Reorder Value.def to optimize code size

[serge-sans-paille]

Summary

If the first values in Value.def is the range of constant, then the code generated by `isa<Constant>` is smaller by one operation (basically, an add is removed). It turns out this small optimization reduces the size of the statically linked clang binary by 400ko on my laptop. The theoritical performance gain is non visible from my benchmarks, but the size dropdown is.

[grandinj]

include/llvm/IR/Value.def

59–60

Consant -> Constant

[serge-sans-paille]

@grandinj : updated, thanks.

[serge-sans-paille]

@grandinj : are you okay with the change otherwise?

[grandinj]

Sorry, I'm not an LLVM reviewer of any note, can't make any useful decisions

[serge-sans-paille]

Adding pete to the list of subscribers, as accordning to `svn log` he's the original author of the file.

[rnk]

lgtm

I checked, and we wrap this enum in our C API, so there should be no ABI concerns here. See LLVMValueKind LLVMGetValueKind(LLVMValueRef Val) in IR/Core.cpp.

[serge-sans-paille]

Thanks @rnk for the review!

[serge-sans-paille]

Mmmh, build failing due to a funny warning that illustrates the optimization, see http://lab.llvm.org:8011/builders/ubuntu-gcc7.1-werror/builds/3012/steps/build-unified-tree/logs/stdio. Update in r318087.

include/llvm/IR/Value.def

 #ifndef HANDLE_VALUE
 #define HANDLE_VALUE(ValueName)
 #endif
 
 #ifndef HANDLE_CONSTANT_MARKER
 #define HANDLE_CONSTANT_MARKER(MarkerName, ValueName)
 #endif
 
-HANDLE_VALUE(Argument)
-HANDLE_VALUE(BasicBlock)
-
-// FIXME: It's awkward that Value.def knows about classes in Analysis. While
-// this doesn't introduce a strict link or include dependency, we should remove
-// the circular dependency eventually.
-HANDLE_MEMORY_VALUE(MemoryUse)
-HANDLE_MEMORY_VALUE(MemoryDef)
-HANDLE_MEMORY_VALUE(MemoryPhi)
+// Having constant first makes the range check for isa<Constant> faster
+// and smaller by one operation.
 
+// Constant
 HANDLE_GLOBAL_VALUE(Function)
 HANDLE_GLOBAL_VALUE(GlobalAlias)
 HANDLE_GLOBAL_VALUE(GlobalIFunc)
 HANDLE_GLOBAL_VALUE(GlobalVariable)
 HANDLE_CONSTANT(BlockAddress)
 HANDLE_CONSTANT(ConstantExpr)
 
 // ConstantAggregate.
 HANDLE_CONSTANT(ConstantArray)
 HANDLE_CONSTANT(ConstantStruct)
 HANDLE_CONSTANT(ConstantVector)
 
 // ConstantData.
 HANDLE_CONSTANT(UndefValue)
 HANDLE_CONSTANT(ConstantAggregateZero)
 HANDLE_CONSTANT(ConstantDataArray)
 HANDLE_CONSTANT(ConstantDataVector)
 HANDLE_CONSTANT(ConstantInt)
 HANDLE_CONSTANT(ConstantFP)
 HANDLE_CONSTANT(ConstantPointerNull)
 HANDLE_CONSTANT(ConstantTokenNone)
 
-HANDLE_METADATA_VALUE(MetadataAsValue)
-HANDLE_INLINE_ASM_VALUE(InlineAsm)
-
-HANDLE_INSTRUCTION(Instruction)
-// Enum values starting at InstructionVal are used for Instructions;
-// don't add new values here!
-
 HANDLE_CONSTANT_MARKER(ConstantFirstVal, Function)
 HANDLE_CONSTANT_MARKER(ConstantLastVal, ConstantTokenNone)
 HANDLE_CONSTANT_MARKER(ConstantDataFirstVal, UndefValue)
 HANDLE_CONSTANT_MARKER(ConstantDataLastVal, ConstantTokenNone)
 HANDLE_CONSTANT_MARKER(ConstantAggregateFirstVal, ConstantArray)
 HANDLE_CONSTANT_MARKER(ConstantAggregateLastVal, ConstantVector)
 
+HANDLE_VALUE(Argument)
+HANDLE_VALUE(BasicBlock)
+
+
+HANDLE_METADATA_VALUE(MetadataAsValue)
+HANDLE_INLINE_ASM_VALUE(InlineAsm)
+
+// FIXME: It's awkward that Value.def knows about classes in Analysis. While
+// this doesn't introduce a strict link or include dependency, we should remove
+// the circular dependency eventually.
+HANDLE_MEMORY_VALUE(MemoryUse)
+HANDLE_MEMORY_VALUE(MemoryDef)
+HANDLE_MEMORY_VALUE(MemoryPhi)
+
+HANDLE_INSTRUCTION(Instruction)
+// Enum values starting at InstructionVal are used for Instructions;
+// don't add new values here!
+
 #undef HANDLE_MEMORY_VALUE
 #undef HANDLE_GLOBAL_VALUE
 #undef HANDLE_CONSTANT
 #undef HANDLE_INSTRUCTION
 #undef HANDLE_METADATA_VALUE
 #undef HANDLE_INLINE_ASM_VALUE
 #undef HANDLE_VALUE
 #undef HANDLE_CONSTANT_MARKER