Skip to content

Commit

Permalink
[IR] Reorder Value fields to put the SubclassID first (#53520)
Browse files Browse the repository at this point in the history
Placing the class id at offset 0 should make `isa` and `dyn_cast` faster
by eliminating the field offset (previously 0x10) from the memory
operand, saving encoding space on x86, and, in theory, an add micro-op.
You can see the load encodes one byte smaller here:
https://godbolt.org/z/Whvz4can9

The compile time tracker shows some modestly positive results in the
on the `cycle` metric and in the final clang binary size metric:
https://llvm-compile-time-tracker.com/compare.php?from=33b54f01fe32030ff60d661a7a951e33360f82ee&to=2530347a57401744293c54f92f9781fbdae3d8c2&stat=cycles
Clicking through to the per-library size breakdown shows that
instcombine size reduces by 0.68%, which is meaningful, and I believe
instcombine is known to be a hotspot.

It is, however, potentially noise. I still think we should do this,
because notionally, the class id really acts as the vptr of the Value,
and conventionally the vptr is always at offset 0.
  • Loading branch information
rnk committed Jan 12, 2024
1 parent dc1e279 commit 21a77e8
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
12 changes: 6 additions & 6 deletions llvm/include/llvm/IR/Value.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ using ValueName = StringMapEntry<Value *>;
/// objects that watch it and listen to RAUW and Destroy events. See
/// llvm/IR/ValueHandle.h for details.
class Value {
Type *VTy;
Use *UseList;

friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase;

const unsigned char SubclassID; // Subclass identifier (for isa/dyn_cast)
unsigned char HasValueHandle : 1; // Has a ValueHandle pointing to this?

Expand Down Expand Up @@ -121,6 +115,12 @@ class Value {
unsigned HasDescriptor : 1;

private:
Type *VTy;
Use *UseList;

friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.

template <typename UseT> // UseT == 'Use' or 'const Use'
class use_iterator_impl {
friend class Value;
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/IR/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ static inline Type *checkType(Type *Ty) {
}

Value::Value(Type *ty, unsigned scid)
: VTy(checkType(ty)), UseList(nullptr), SubclassID(scid), HasValueHandle(0),
SubclassOptionalData(0), SubclassData(0), NumUserOperands(0),
IsUsedByMD(false), HasName(false), HasMetadata(false) {
: SubclassID(scid), HasValueHandle(0), SubclassOptionalData(0),
SubclassData(0), NumUserOperands(0), IsUsedByMD(false), HasName(false),
HasMetadata(false), VTy(checkType(ty)), UseList(nullptr) {
static_assert(ConstantFirstVal == 0, "!(SubclassID < ConstantFirstVal)");
// FIXME: Why isn't this in the subclass gunk??
// Note, we cannot call isa<CallInst> before the CallInst has been
Expand Down

0 comments on commit 21a77e8

Please sign in to comment.