Skip to content

IR: Reorder ConstantData enum values #138638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2025

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented May 6, 2025

This sorts ConstantData to the low values, so we can perform
a hasUseList check in a single compare instead of requiring 2
compares plus an and for the range check.

Copy link
Contributor Author

arsenm commented May 6, 2025

@arsenm arsenm marked this pull request as ready for review May 6, 2025 06:45
@llvmbot
Copy link
Member

llvmbot commented May 6, 2025

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

This sorts ConstantData to the low values, so we can perform
a hasUseList check in a single compare instead of requiring 2
compares plus an and for the range check.


Full diff: https://github.com/llvm/llvm-project/pull/138638.diff

1 Files Affected:

  • (modified) llvm/include/llvm/IR/Value.def (+20-18)
diff --git a/llvm/include/llvm/IR/Value.def b/llvm/include/llvm/IR/Value.def
index 160e0f8513e2a..34b8d4967b28a 100644
--- a/llvm/include/llvm/IR/Value.def
+++ b/llvm/include/llvm/IR/Value.def
@@ -69,24 +69,11 @@
 #define HANDLE_CONSTANT_EXCLUDE_LLVM_C_API(ValueName)
 #endif
 
-// Having constant first makes the range check for isa<Constant> faster
-// and smaller by one operation.
+// Having constant first makes the range check for isa<Constant> faster and
+// smaller by one operation. Further, keep ConstantData as the first subset so
+// it's also as fast.
 
 // Constant
-HANDLE_GLOBAL_VALUE(Function)
-HANDLE_GLOBAL_VALUE(GlobalAlias)
-HANDLE_GLOBAL_VALUE(GlobalIFunc)
-HANDLE_GLOBAL_VALUE(GlobalVariable)
-HANDLE_CONSTANT(BlockAddress)
-HANDLE_CONSTANT(ConstantExpr)
-HANDLE_CONSTANT_EXCLUDE_LLVM_C_API(DSOLocalEquivalent)
-HANDLE_CONSTANT_EXCLUDE_LLVM_C_API(NoCFIValue)
-HANDLE_CONSTANT(ConstantPtrAuth)
-
-// ConstantAggregate.
-HANDLE_CONSTANT(ConstantArray)
-HANDLE_CONSTANT(ConstantStruct)
-HANDLE_CONSTANT(ConstantVector)
 
 // ConstantData.
 HANDLE_CONSTANT(UndefValue)
@@ -100,8 +87,23 @@ HANDLE_CONSTANT(ConstantTargetNone)
 HANDLE_CONSTANT(ConstantPointerNull)
 HANDLE_CONSTANT(ConstantTokenNone)
 
-HANDLE_CONSTANT_MARKER(ConstantFirstVal, Function)
-HANDLE_CONSTANT_MARKER(ConstantLastVal, ConstantTokenNone)
+// ConstantAggregate.
+HANDLE_CONSTANT(ConstantArray)
+HANDLE_CONSTANT(ConstantStruct)
+HANDLE_CONSTANT(ConstantVector)
+
+HANDLE_GLOBAL_VALUE(Function)
+HANDLE_GLOBAL_VALUE(GlobalAlias)
+HANDLE_GLOBAL_VALUE(GlobalIFunc)
+HANDLE_GLOBAL_VALUE(GlobalVariable)
+HANDLE_CONSTANT(BlockAddress)
+HANDLE_CONSTANT(ConstantExpr)
+HANDLE_CONSTANT_EXCLUDE_LLVM_C_API(DSOLocalEquivalent)
+HANDLE_CONSTANT_EXCLUDE_LLVM_C_API(NoCFIValue)
+HANDLE_CONSTANT(ConstantPtrAuth)
+
+HANDLE_CONSTANT_MARKER(ConstantFirstVal, UndefValue)
+HANDLE_CONSTANT_MARKER(ConstantLastVal, ConstantPtrAuth)
 HANDLE_CONSTANT_MARKER(ConstantDataFirstVal, UndefValue)
 HANDLE_CONSTANT_MARKER(ConstantDataLastVal, ConstantTokenNone)
 HANDLE_CONSTANT_MARKER(ConstantAggregateFirstVal, ConstantArray)

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peculiarly, this has a small negative effect on stage1 builds using gcc and a small positive effect on stage2 builds using clang: https://llvm-compile-time-tracker.com/compare.php?from=420eca364b07bad78dc0a5d21da5980493798df0&to=00be79cbc7dea09fcd8a57ea51b3e800564fd986&stat=instructions:u

@arsenm arsenm force-pushed the users/arsenm/ir/remove-constantdata-reference-counts branch from b582820 to f04af6e Compare May 6, 2025 11:45
@arsenm arsenm force-pushed the users/arsenm/ir/reorder-constant-data-enum-values branch from 6346af3 to 936319a Compare May 6, 2025 11:45
@arsenm arsenm force-pushed the users/arsenm/ir/reorder-constant-data-enum-values branch from 936319a to 5317c2e Compare May 6, 2025 12:32
@arsenm arsenm force-pushed the users/arsenm/ir/remove-constantdata-reference-counts branch from a029e76 to 1105d7c Compare May 6, 2025 15:20
Base automatically changed from users/arsenm/ir/remove-constantdata-reference-counts to main May 6, 2025 15:24
@arsenm arsenm force-pushed the users/arsenm/ir/reorder-constant-data-enum-values branch 2 times, most recently from 7c7350f to 010c0ad Compare May 8, 2025 16:52
@arsenm
Copy link
Contributor Author

arsenm commented May 12, 2025

ping

Copy link
Contributor Author

arsenm commented May 19, 2025

Merge activity

  • May 19, 6:10 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • May 19, 6:12 AM EDT: Graphite rebased this pull request as part of a merge.
  • May 19, 6:14 AM EDT: @arsenm merged this pull request with Graphite.

This sorts ConstantData to the low values, so we can perform
a hasUseList check in a single compare instead of requiring 2
compares plus an and for the range check.
@arsenm arsenm force-pushed the users/arsenm/ir/reorder-constant-data-enum-values branch from 010c0ad to dc6faab Compare May 19, 2025 10:12
@arsenm arsenm merged commit 1ead4a8 into main May 19, 2025
6 of 10 checks passed
@arsenm arsenm deleted the users/arsenm/ir/reorder-constant-data-enum-values branch May 19, 2025 10:15
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
This sorts ConstantData to the low values, so we can perform
a hasUseList check in a single compare instead of requiring 2
compares plus an and for the range check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants