[IR] Value: Fix OpCode checks

[samitolvanen]

Summary

Value::SubclassID cannot be directly compared to Instruction enums, such as
Instruction::{Call,Invoke,CallBr}. We have to first subtract InstructionVal
from the SubclassID to get the OpCode, similar to Instruction::getOpCode().

[nickdesaulniers]

Based on the comments above Value::getValueID, this seems fine to me. ie. no such opcode == 0 and InstructionVal enumerator must be the highest valued enumerator in the ValueTy enum.

[samitolvanen]

Thanks, Nick. I don't have commit access, so would you mind committing the patch? I confirmed that check-all passes for me with the patch applied.

[craig.topper]

llvm/lib/IR/Value.cpp

67

Can we change this to CallBase while we're here? Only one of those opcodes is CallInst, the others are InvokeInst and CallBrInst. They all inherit from CallBase.

I was hoping there was a better constructor on CallBase to stash this in to simplify things, but I don't think there is.

[samitolvanen]

Changed the message to refer to CallBase instead of CallInst.

[nickdesaulniers]

llvm/lib/IR/Value.cpp

67

I think there's a 4th class derived from CallBase: GCStatePointInst, though there's nothing in langref about it; seems like an intrinsic. Probably should be fine. The text of the assert can be cleaned up, too.

llvm/lib/IR/Value.cpp

 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) {
   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
   // constructed.
-  if (SubclassID == Instruction::Call || SubclassID == Instruction::Invoke ||
-      SubclassID == Instruction::CallBr)
+  unsigned OpCode = 0;
+  if (SubclassID >= InstructionVal)
+    OpCode = SubclassID - InstructionVal;
+  if (OpCode == Instruction::Call || OpCode == Instruction::Invoke ||
+      OpCode == Instruction::CallBr)
     assert((VTy->isFirstClassType() || VTy->isVoidTy() || VTy->isStructTy()) &&
-           "invalid CallInst type!");
+           "invalid CallBase type!");
   else if (SubclassID != BasicBlockVal &&
            (/*SubclassID < ConstantFirstVal ||*/ SubclassID > ConstantLastVal))
     assert((VTy->isFirstClassType() || VTy->isVoidTy()) &&
            "Cannot create non-first-class values except for constants!");
   static_assert(sizeof(Value) == 2 * sizeof(void *) + 2 * sizeof(unsigned),
                 "Value too big");
 }