[Bitcode] Limit bits used for CallingConv::ID in Function, update tests

[vsk]

Summary

• Use 10 bits to represent calling convention ID's instead of 13.
• Update the bitcode compatibility tests to reflect this change.

We currently implement about 80 calling conventions. This change lowers the cap to 1023 CC's (from 8191). This gives us back reserve bits in Function's subclass data field, which I plan on using soon.

As a follow-up, I will fix syntax errors in the compatibility tests caused by accidental 3.6 -> 3.8 IR conversion.

[rnk]

Technically this is a bitcode backwards compatibility break, but I don't think anyone has ever used a calling convention ID greater than 1023 except in these test cases. I think we could get away with rejecting large calling convention IDs in the bitcode reader, though. After the reader, we can assert that truncation should not occur in setCallingConv for Function, InvokeInst, and CallInst.

include/llvm/IR/Function.h

167–169

Should this assert that CC < 1024?

[vsk]

• Assert when we try to set invalid calling conventions (addresses rnk's comment).
• Error out in the bitcode reader when we see a bad calling conv (this is the compatibility break).

[rnk]

Can we hoist 0xCFF / 1023 into the CallingConv.h header, and call it something like CallingConv::MaxID? Otherwise, this looks good.

[vsk]

Thanks Reid, I placed the constant in CallingConv::MaxID as suggested. r251452.

include/llvm/IR/Function.h

private:
   mutable ArgumentListType ArgumentList;  ///< The formal arguments
   ValueSymbolTable *SymTab;               ///< Symbol table of args/instructions
   AttributeSet AttributeSets;             ///< Parameter attributes
   FunctionType *Ty;
 
   /*
    * Value::SubclassData
    *
-   * bit 0  : HasLazyArguments
-   * bit 1  : HasPrefixData
-   * bit 2  : HasPrologueData
-   * bit 3-6: CallingConvention
+   * bit 0      : HasLazyArguments
+   * bit 1      : HasPrefixData
+   * bit 2      : HasPrologueData
+   * bit 3      : [reserved]
+   * bits 4-13  : CallingConvention
+   * bits 14-15 : [reserved]
    */
 
   /// Bits from GlobalObject::GlobalObjectSubclassData.
   enum {
     /// Whether this function is materializable.
     IsMaterializableBit = 1 << 0,
     HasMetadataHashEntryBit = 1 << 1
   };
public:
   /// Note, this method does not need to be called directly, as it is called
   /// from Value::setName() whenever the name of this function changes.
   void recalculateIntrinsicID();
 
   /// getCallingConv()/setCallingConv(CC) - These method get and set the
   /// calling convention of this function.  The enum values for the known
   /// calling conventions are defined in CallingConv.h.
   CallingConv::ID getCallingConv() const {
-    return static_cast<CallingConv::ID>(getSubclassDataFromValue() >> 3);
+    return static_cast<CallingConv::ID>((getSubclassDataFromValue() >> 4) &
+                                        0x3ff);
   }
   void setCallingConv(CallingConv::ID CC) {
-    setValueSubclassData((getSubclassDataFromValue() & 7) |
-                         (static_cast<unsigned>(CC) << 3));
+    auto ID = static_cast<unsigned>(CC);
+    assert(!(ID & ~0x3ff) && "Unsupported calling convention");
+    setValueSubclassData((getSubclassDataFromValue() & 0xc00f) | (ID << 4));
   }
 
   /// @brief Return the attribute list for this Function.
   AttributeSet getAttributes() const { return AttributeSets; }
 
   /// @brief Set the attribute list for this Function.
   void setAttributes(AttributeSet attrs) { AttributeSets = attrs; }
 

include/llvm/IR/Instructions.h

   }
 
   /// getCallingConv/setCallingConv - Get or set the calling convention of this
   /// function call.
   CallingConv::ID getCallingConv() const {
     return static_cast<CallingConv::ID>(getSubclassDataFromInstruction() >> 2);
   }
   void setCallingConv(CallingConv::ID CC) {
+    auto ID = static_cast<unsigned>(CC);
+    assert(!(ID & ~0x3ff) && "Unsupported calling convention");
     setInstructionSubclassData((getSubclassDataFromInstruction() & 3) |
-                               (static_cast<unsigned>(CC) << 2));
+                               (ID << 2));
   }
 
   /// getAttributes - Return the parameter attributes for this call.
   ///
   const AttributeSet &getAttributes() const { return AttributeList; }
 
   /// setAttributes - Set the parameter attributes for this call.
   ///

   }
 
   /// getCallingConv/setCallingConv - Get or set the calling convention of this
   /// function call.
   CallingConv::ID getCallingConv() const {
     return static_cast<CallingConv::ID>(getSubclassDataFromInstruction());
   }
   void setCallingConv(CallingConv::ID CC) {
-    setInstructionSubclassData(static_cast<unsigned>(CC));
+    auto ID = static_cast<unsigned>(CC);
+    assert(!(ID & ~0x3ff) && "Unsupported calling convention");
+    setInstructionSubclassData(ID);
   }
 
   /// getAttributes - Return the parameter attributes for this invoke.
   ///
   const AttributeSet &getAttributes() const { return AttributeList; }
 
   /// setAttributes - Set the parameter attributes for this invoke.
   ///

lib/Bitcode/Reader/BitcodeReader.cpp

       Type *Ty = getTypeByID(Record[0]);
       if (!Ty)
         return error("Invalid record");
       if (auto *PTy = dyn_cast<PointerType>(Ty))
         Ty = PTy->getElementType();
       auto *FTy = dyn_cast<FunctionType>(Ty);
       if (!FTy)
         return error("Invalid type for value");
+      auto CC = static_cast<CallingConv::ID>(Record[1]);
+      if (CC & ~0x3ff)
+        return error("Invalid calling convention ID");
 
       Function *Func = Function::Create(FTy, GlobalValue::ExternalLinkage,
                                         "", TheModule);
 
-      Func->setCallingConv(static_cast<CallingConv::ID>(Record[1]));
+      Func->setCallingConv(CC);
       bool isProto = Record[2];
       uint64_t RawLinkage = Record[3];
       Func->setLinkage(getDecodedLinkage(RawLinkage));
       Func->setAttributes(getAttributes(Record[4]));
 
       unsigned Alignment;
       if (std::error_code EC = parseAlignmentValue(Record[5], Alignment))
         return EC;

           Ops.push_back(Op);
         }
       }
 
       I = InvokeInst::Create(Callee, NormalBB, UnwindBB, Ops, OperandBundles);
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<InvokeInst>(I)
-          ->setCallingConv(static_cast<CallingConv::ID>(~(1U << 13) & CCInfo));
+          ->setCallingConv(static_cast<CallingConv::ID>(0x3ff & CCInfo));
       cast<InvokeInst>(I)->setAttributes(PAL);
       break;
     }
     case bitc::FUNC_CODE_INST_RESUME: { // RESUME: [opval]
       unsigned Idx = 0;
       Value *Val = nullptr;
       if (getValueTypePair(Record, Idx, NextValueNo, Val))
         return error("Invalid record");

           Args.push_back(Op);
         }
       }
 
       I = CallInst::Create(FTy, Callee, Args, OperandBundles);
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<CallInst>(I)->setCallingConv(
-          static_cast<CallingConv::ID>((~(1U << 14) & CCInfo) >> 1));
+          static_cast<CallingConv::ID>((0x7ff & CCInfo) >> 1));
       CallInst::TailCallKind TCK = CallInst::TCK_None;
       if (CCInfo & 1)
         TCK = CallInst::TCK_Tail;
       if (CCInfo & (1 << 14))
         TCK = CallInst::TCK_MustTail;
       cast<CallInst>(I)->setTailCallKind(TCK);
       cast<CallInst>(I)->setAttributes(PAL);
       break;

test/Bitcode/compatibility-3.6.ll

 declare cc79 void @f.cc79()
 ; CHECK: declare x86_64_win64cc void @f.cc79()
 declare x86_64_win64cc void @f.x86_64_win64cc()
 ; CHECK: declare x86_64_win64cc void @f.x86_64_win64cc()
 declare cc80 void @f.cc80()
 ; CHECK: declare x86_vectorcallcc void @f.cc80()
 declare x86_vectorcallcc void @f.x86_vectorcallcc()
 ; CHECK: declare x86_vectorcallcc void @f.x86_vectorcallcc()
-declare cc8191 void @f.cc8191()
-; CHECK: declare cc8191 void @f.cc8191()
+declare cc1023 void @f.cc1023()
+; CHECK: declare cc1023 void @f.cc1023()
 
 ; Functions -- ret attrs (Return attributes)
 declare zeroext i64 @f.zeroext()
 ; CHECK: declare zeroext i64 @f.zeroext()
 declare signext i64 @f.signext()
 ; CHECK: declare signext i64 @f.signext()
 declare inreg i32* @f.inreg()
 ; CHECK: declare inreg i32* @f.inreg()

test/Bitcode/compatibility-3.7.ll

 declare cc79 void @f.cc79()
 ; CHECK: declare x86_64_win64cc void @f.cc79()
 declare x86_64_win64cc void @f.x86_64_win64cc()
 ; CHECK: declare x86_64_win64cc void @f.x86_64_win64cc()
 declare cc80 void @f.cc80()
 ; CHECK: declare x86_vectorcallcc void @f.cc80()
 declare x86_vectorcallcc void @f.x86_vectorcallcc()
 ; CHECK: declare x86_vectorcallcc void @f.x86_vectorcallcc()
-declare cc8191 void @f.cc8191()
-; CHECK: declare cc8191 void @f.cc8191()
+declare cc1023 void @f.cc1023()
+; CHECK: declare cc1023 void @f.cc1023()
 
 ; Functions -- ret attrs (Return attributes)
 declare zeroext i64 @f.zeroext()
 ; CHECK: declare zeroext i64 @f.zeroext()
 declare signext i64 @f.signext()
 ; CHECK: declare signext i64 @f.signext()
 declare inreg i32* @f.inreg()
 ; CHECK: declare inreg i32* @f.inreg()

test/Bitcode/compatibility.ll

 declare cc79 void @f.cc79()
 ; CHECK: declare x86_64_win64cc void @f.cc79()
 declare x86_64_win64cc void @f.x86_64_win64cc()
 ; CHECK: declare x86_64_win64cc void @f.x86_64_win64cc()
 declare cc80 void @f.cc80()
 ; CHECK: declare x86_vectorcallcc void @f.cc80()
 declare x86_vectorcallcc void @f.x86_vectorcallcc()
 ; CHECK: declare x86_vectorcallcc void @f.x86_vectorcallcc()
-declare cc8191 void @f.cc8191()
-; CHECK: declare cc8191 void @f.cc8191()
+declare cc1023 void @f.cc1023()
+; CHECK: declare cc1023 void @f.cc1023()
 
 ; Functions -- ret attrs (Return attributes)
 declare zeroext i64 @f.zeroext()
 ; CHECK: declare zeroext i64 @f.zeroext()
 declare signext i64 @f.signext()
 ; CHECK: declare signext i64 @f.signext()
 declare inreg i32* @f.inreg()
 ; CHECK: declare inreg i32* @f.inreg()

test/Bitcode/tailcall.ll

 ; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder < %s
 
 ; Check that musttail and tail roundtrip.
 
-declare cc8191 void @t1_callee()
-define cc8191 void @t1() {
-; CHECK: tail call cc8191 void @t1_callee()
-  tail call cc8191 void @t1_callee()
+declare cc1023 void @t1_callee()
+define cc1023 void @t1() {
+; CHECK: tail call cc1023 void @t1_callee()
+  tail call cc1023 void @t1_callee()
   ret void
 }
 
-declare cc8191 void @t2_callee()
-define cc8191 void @t2() {
-; CHECK: musttail call cc8191 void @t2_callee()
-  musttail call cc8191 void @t2_callee()
+declare cc1023 void @t2_callee()
+define cc1023 void @t2() {
+; CHECK: musttail call cc1023 void @t2_callee()
+  musttail call cc1023 void @t2_callee()
   ret void
 }