[llvm] [IR] Reorder Value fields to put the SubclassID first (PR #75634)

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 8 15:54:36 PST 2024


https://github.com/rnk updated https://github.com/llvm/llvm-project/pull/75634

>From a2d0933b14fad77f41dcd383b2bc2468dc5e8c2b Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at google.com>
Date: Thu, 14 Dec 2023 22:48:57 +0000
Subject: [PATCH] [IR] Reorder Value fields to put the SubclassID first

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.
---
 llvm/include/llvm/IR/Value.h | 12 ++++++------
 llvm/lib/IR/Value.cpp        |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index 8fce56e12c5235..61f9d34eef3567 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -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?
 
@@ -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;
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 94b0ae7435c949..c4e47975b18221 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -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



More information about the llvm-commits mailing list