bitcode support change for fast flags compatibility

[mcberg2017]

Summary

The discussion and as per need, each vendor needs a way to keep the old fast flags and the new fast flags in the auto upgrade path of the IR upgrader. This revision addresses that issue.

[mcberg2017]

This comment was removed by mcberg2017.

[mcberg2017]

Uploaded above are the 6 bitcode files we will need to replace for check to run successfully as these files had the old version number in them.

[mehdi_amini]

In D43253#1006664, @mcberg2017 wrote:

Uploaded above are the 6 bitcode files we will need to replace for check to run successfully as these files had the old version number in them.

Weren't these files generated with the corresponding version of LLVM? We should check that they upgrade correctly, not prevent them from upgrading I believe.

[mehdi_amini]

include/llvm/IR/Module.h

189

Why does it make sense to have on the Module?

lib/Bitcode/Reader/BitcodeReader.cpp

426

I'm not familiar with this record, why use this instead of bitc::BITCODE_CURRENT_EPOCH?

[mcberg2017]

For the 6 bitcode files, they are special context, as they require the current fast sub flags format for part of their use, so we have to bring them up into the current context as they require the use of the new flags selectively with the bumped version number.

[mcberg2017]

include/llvm/IR/Module.h

189

We had it before, we just never held the value in storage (we only checked it), and I believe (someone correct me if I misspeak), this is where we bumped the bitcode version.

[mehdi_amini]

In D43253#1006726, @mcberg2017 wrote:

For the 6 bitcode files, they are special context, as they require the current fast sub flags format for part of their use, so we have to bring them up into the current context as they require the use of the new flags selectively with the bumped version number.

Can you elaborate what you mean by "they require the current fast sub flags format for part of their use"?

include/llvm/IR/Module.h

189

That's not answering why it makes sense to add it here, and now.

[mehdi_amini]

lib/Bitcode/Reader/BitcodeReader.cpp

426

just to make you didn't miss this: I don't know why bumping ModuleVersion instead of bitc::BITCODE_CURRENT_EPOCH?

[mehdi_amini]

Actually if we're making this change, we consider that bitcode generated between the introduction of the new fast-math flags and now isn't supported anymore.
If so then we could instead of bumping a version, change the encoding of the FMF record (or create a new record type), so that the old encoding is upgraded but not the new one.
That would seem to me to be more in line with the usual handling of such situation in the bitcode.

[mcberg2017]

include/llvm/IR/Module.h

189

I'll attempt to answer both comments here (lines 189/426): My interpretation of {b738d340, the review for adding EPOCH and other components} is that with IDENTIFICATION_CODE_VERSION removed and major and minor version number are no longer maintained and with no vehicle comparing LLVM_VERSION_MAJOR/LLVM_VERSION_MINOR in this context for bitcode versioning that the action was open to interpretation for a plausible solution. Subsequently that using the Version Record and giving it storage would work well as it is already overloaded to modify actions regarding what and how to read bitcode.

[mehdi_amini]

include/llvm/IR/Module.h

189

What is b738d340? Can you use the SVN rev please?

I don't understand your answer, like almost anything. I'll try to clarify what I'm asking:

1. At the bitcode level why is EPOCH not the right thing to use?
2. Even if you were to use the module version record in the bitcode. That does not justify by itself to expose it on the IR structure that is the class Module. I'm missing the motivation to change anything outside of IR.

(also, this discussion is unnecessary if we follow my other suggestion which is to change the encoding of the FMF record).

[mcberg2017]

include/llvm/IR/Module.h

189

The comment above pertains to this review: https://reviews.llvm.org/D13666

and this svn rev: https://gitlab.kitware.com/third-party/llvm/commit/b738d340fa9958dbefe8d23a4d4659c2255b823a

Changing the encoding format for the FMF record is a non starter.

[mehdi_amini]

include/llvm/IR/Module.h

189

Changing the encoding format for the FMF record is a non starter.

Why?

[mcberg2017]

include/llvm/IR/Module.h

189

First lets step back a bit.

If one objection is the use of storage in Module.h, then I can carry the state around in the reader.

By using EPOCH do you mean changing the value from 0 to 1 to interpret the effective mapping of the fast math flags as old verses new?

[mehdi_amini]

include/llvm/IR/Module.h

189

First lets step back a bit.

That's what I did and lead me to look at the FMF encoding instead.
The FMF is a uint64_t, there is likely a bit that could be used for versioning here, or other means of encoding that would help with the upgrade.

By using EPOCH do you mean changing the value from 0 to 1 to interpret the effective mapping of the fast math flags as old verses new?

I looked back at EPOCH and it won't help: it is not designed to help with upgrade, it is designed as a barrier for not to read an old bitcode entirely.

[mehdi_amini]

I quickly put a patch to change the encoding, may be easier to discuss with code at hand: https://reviews.llvm.org/D43348
(I didn't try to build it, just a quick draft)

[mehdi_amini]

In D43253#1007097, @mehdi_amini wrote:

In D43253#1006726, @mcberg2017 wrote:

For the 6 bitcode files, they are special context, as they require the current fast sub flags format for part of their use, so we have to bring them up into the current context as they require the use of the new flags selectively with the bumped version number.

Can you elaborate what you mean by "they require the current fast sub flags format for part of their use"?

Ping on this.

[mcberg2017]

So the compatibility .bc files in check test have usages of the current flag layout with reassoc, afn and the new layout, as has been the case since last Nov. These tests are not old fast format but sub flags tests in the new format, and require the new mapping.

[mehdi_amini]

In D43253#1009275, @mcberg2017 wrote:

So the compatibility .bc files in check test have usages of the current flag layout with reassoc, afn and the new layout, as has been the case since last Nov. These tests are not old fast format but sub flags tests in the new format, and require the new mapping.

Please be very specific and provide example of what is broken in the test/Bitcode/compatibility-3.6.ll for example.

[mcberg2017]

Case in point:

./bin/llvm-dis < /Users/michael_c_berg/llvm/llvm/test/Bitcode/compatibility-3.6.ll.bc | ./bin/FileCheck ../llvm/test/Bitcode/compatibility-3.6.ll
../llvm/test/Bitcode/compatibility-3.6.ll:617:11: error: expected string not found in input
; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2

^

<stdin>:445:2: note: scanning from here
%f.fast = fadd fast float %op1, %op2

in this case we are using my current patch with module version at 2 in the key .bc file, however we need the mapping as:

reassoc
nnan
ninf
nsz
arch
contract
afn

the new format writes reassoc in position 0 and aliases to UnsafeAlgebra incorrectly in this case, causing us to map fast, when less than fast was specified in the IR.
This will still be the case with the other format.
^

[mehdi_amini]

In D43253#1009292, @mcberg2017 wrote:

Case in point:

./bin/llvm-dis < /Users/michael_c_berg/llvm/llvm/test/Bitcode/compatibility-3.6.ll.bc | ./bin/FileCheck ../llvm/test/Bitcode/compatibility-3.6.ll
../llvm/test/Bitcode/compatibility-3.6.ll:617:11: error: expected string not found in input
; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2

^

<stdin>:445:2: note: scanning from here
%f.fast = fadd fast float %op1, %op2

in this case we are using my current patch with module version at 2 in the key .bc file, however we need the mapping as:

reassoc
nnan
ninf
nsz
arch
contract
afn

the new format writes reassoc in position 0 and aliases to UnsafeAlgebra incorrectly in this case, causing us to map fast, when less than fast was specified in the IR.
This will still be the case with the other format.
^

Thanks for being specific.
The right fix is to change the check line, not the bitcode.

[mcberg2017]

Strongly disagree. The new test's purpose is to provide functional testing on the new flags in the new format. Sanjay, now would be a good time to chime in.

[mehdi_amini]

In D43253#1009298, @mcberg2017 wrote:

Strongly disagree. The new test's purpose is to provide functional testing on the new flags in the new format. Sanjay, now would be a good time to chime in.

We don't have the same understanding I believe: my take on this is that this test is supposed to ensure that LLVM upgrade correctly old bitcode. So changing the bitcode is a huge red flag to begin with,
The CHECK line on the other is intended to be adjusted: it is testing what is the current output of LLVM for an old bitcode.

So the question is: for the bitcode generated by LLVM 3.6 with a "fast" flag. What do you want printed by LLVM today?

Ping @vsk as well.

[vsk]

In D43253#1009299, @mehdi_amini wrote:

In D43253#1009298, @mcberg2017 wrote:

Strongly disagree. The new test's purpose is to provide functional testing on the new flags in the new format. Sanjay, now would be a good time to chime in.

We don't have the same understanding I believe: my take on this is that this test is supposed to ensure that LLVM upgrade correctly old bitcode. So changing the bitcode is a huge red flag to begin with,
The CHECK line on the other is intended to be adjusted: it is testing what is the current output of LLVM for an old bitcode.

+ 1, yes exactly. The point of checking in the bitcode generated by the 3.6 compiler is that future compilers must understand it gracefully :). This is fundamental for allowing submission of bitcode to Apple's app store and is a documented requirement: https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility.

So the question is: for the bitcode generated by LLVM 3.6 with a "fast" flag. What do you want printed by LLVM today?

Yep. It's fine if new compilers change the semantics of old bitcode (within reason), but the old bitcode must be handled gracefully. We need to continue testing that: i.e we can't replace the old bitcode files, but it's OK to update the check lines in the *.ll file.

[spatel]

In D43253#1009299, @mehdi_amini wrote:

In D43253#1009298, @mcberg2017 wrote:

Strongly disagree. The new test's purpose is to provide functional testing on the new flags in the new format. Sanjay, now would be a good time to chime in.

We don't have the same understanding I believe: my take on this is that this test is supposed to ensure that LLVM upgrade correctly old bitcode. So changing the bitcode is a huge red flag to begin with,
The CHECK line on the other is intended to be adjusted: it is testing what is the current output of LLVM for an old bitcode.

So the question is: for the bitcode generated by LLVM 3.6 with a "fast" flag. What do you want printed by LLVM today?

I haven't commented because I don't have any expertise on the trade-offs as far as the implementation goes, but IIUC the point of this exercise is to get old bitcode that was 'fast' to be 'fast' again. So we want the CHECK lines in the compatibility files that were changed in D39304 to go back to being 'fast'. Ie, we want to reverse the diffs anywhere there's a comment like:

; 'fast' used to be its own bit, but this changed in Oct 2017.

[mcberg2017]

As per feedback I have updated the bitcode reader/writer functionality as per Medhi's feedback along with the compatibility tests as per Sanjay's feedback to fast, and corrected the fast map values so that we get unique flag selection, as the code from D43348 would not have worked without shift values. Also I added a change to the fast math encoding size field from 7 to 8 bits. All works as advertised in the check tests.

[spatel]

lib/Bitcode/Writer/BitcodeWriter.cpp

3199

I don't know this code, so I'm curious how this works, and it might be worth a code comment.

What allows us to increase the width of the field? Are we relying on padding bits? If not, how does this work when a new compiler reads an old IR file with the old smaller field?

test/Bitcode/compatibility-3.6.ll

615–616 ↗

(On Diff #134545)

Here and everywhere else the comment is repeated: delete it because it's no longer valid.

[mehdi_amini]

@hans : this needs to go in LLVM 6.0, is it OK?

lib/Bitcode/Writer/BitcodeWriter.cpp

3199

The bitcode format is described here: http://www.llvm.org/docs/BitCodeFormat.html

The abbreviation describe how the records will be stored and the abbrev is itself emitted. So that the reader knows how to decode. When a new llvm reads an old bitcode it'll see an abbrev and know that only 7 bits needs to be read. In any case the API unpack and returns a uint64.
Does it make sense?

[spatel]

lib/Bitcode/Writer/BitcodeWriter.cpp

3199

Ah, yes - the size itself is encoded in the binary. Thanks.

[mcberg2017]

Comments removed per recommendation.

[spatel]

In D43253#1010355, @mcberg2017 wrote:

Comments removed per recommendation.

Thanks. I don't have any other comments, but someone with a better understanding and a stake in this should give approval.

[steven_wu]

I think this approach is much better. Can you update the title because it is not really a version bump anymore? From my understanding, this is good enough for backwards-compatibility. Thanks!

I don't know enough about fast math flag to fully approve this. @qcolombet, what do you think?

[vsk]

Thanks, this is looking good! It'd be great to hear Quentin chime in.

[qcolombet]

Thanks all for the great feedbacks!

The final solution is much better than a dumb bump of the bitcode version.

LGTM.

[mehdi_amini]

In D43253#1010451, @qcolombet wrote:

Thanks all for the great feedbacks!

The final solution is much better than a dumb bump of the bitcode version.

LGTM.

Note: we need buy-in from LLVM 6.0 release manager (@hans ?) that this will be cherry-picked there before landing this in trunk.

[hans]

In D43253#1010580, @mehdi_amini wrote:

In D43253#1010451, @qcolombet wrote:

Thanks all for the great feedbacks!

The final solution is much better than a dumb bump of the bitcode version.

LGTM.

Note: we need buy-in from LLVM 6.0 release manager (@hans ?) that this will be cherry-picked there before landing this in trunk.

This seems fine for 6.0 (it needs to land soon though), but why does it need to be cherry-picked to the branch before landing on trunk?

[steven_wu]

Michael doesn't have commit right yet so I will commit this for him.

This can cherry-pick after landing on trunk.

[mehdi_amini]

In D43253#1011978, @hans wrote:

In D43253#1010580, @mehdi_amini wrote:

In D43253#1010451, @qcolombet wrote:

Thanks all for the great feedbacks!

The final solution is much better than a dumb bump of the bitcode version.

LGTM.

Note: we need buy-in from LLVM 6.0 release manager (@hans ?) that this will be cherry-picked there before landing this in trunk.

This seems fine for 6.0 (it needs to land soon though), but why does it need to be cherry-picked to the branch before landing on trunk?

Sorry for the ambiguity, I meant that we needed your confirmation before landing this in trunk, not that the cherry-pick has to happen before.
i.e. parenthesis were intended like this "we need (buy-in from LLVM 6.0 release manager that this will be cherry-picked there) before landing this in trunk."

Thanks!

[hans]

Note: we need buy-in from LLVM 6.0 release manager (@hans ?) that this will be cherry-picked there before landing this in trunk.

This seems fine for 6.0 (it needs to land soon though), but why does it need to be cherry-picked to the branch before landing on trunk?

Sorry for the ambiguity, I meant that we needed your confirmation before landing this in trunk, not that the cherry-pick has to happen before.
i.e. parenthesis were intended like this "we need (buy-in from LLVM 6.0 release manager that this will be cherry-picked there) before landing this in trunk."

Makes sense, thanks :-)

I'll let this sit in trunk a few hours more and will merge it later today.

[hans]

In D43253#1012927, @hans wrote:

Note: we need buy-in from LLVM 6.0 release manager (@hans ?) that this will be cherry-picked there before landing this in trunk.

This seems fine for 6.0 (it needs to land soon though), but why does it need to be cherry-picked to the branch before landing on trunk?

Sorry for the ambiguity, I meant that we needed your confirmation before landing this in trunk, not that the cherry-pick has to happen before.
i.e. parenthesis were intended like this "we need (buy-in from LLVM 6.0 release manager that this will be cherry-picked there) before landing this in trunk."

Makes sense, thanks :-)

I'll let this sit in trunk a few hours more and will merge it later today.

Merged to 6.0 in r325592.

include/llvm/IR/Module.h

private:
   Materializer;                   ///< Used to materialize GlobalValues
   std::string ModuleID;           ///< Human readable identifier for the module
   std::string SourceFileName;     ///< Original source file name for module,
                                   ///< recorded in bitcode.
   std::string TargetTriple;       ///< Platform target triple Module compiled on
                                   ///< Format: (arch)(sub)-(vendor)-(sys0-(abi)
   void *NamedMDSymTab;            ///< NamedMDNode names.
   DataLayout DL;                  ///< DataLayout associated with the module
+  unsigned VersionNum;            ///< Module version number determined from parsing
 
   friend class Constant;
 
 /// @}
 /// @name Constructors
 /// @{
 public:
   /// The Module constructor. Note that there is no default constructor. You
/// @{
   /// Get the global data context.
   /// @returns LLVMContext - a container for LLVM's global information
   LLVMContext &getContext() const { return Context; }
 
   /// Get any module-scope inline assembly blocks.
   /// @returns a string containing the module-scope inline assembly blocks.
   const std::string &getModuleInlineAsm() const { return GlobalScopeAsm; }
 
+  /// Get module version number.
+  const unsigned &getModuleVersionNum() const { return  VersionNum; }
+
   /// Get a RandomNumberGenerator salted for use with this module. The
   /// RNG can be seeded via -rng-seed=<uint64> and is salted with the
   /// ModuleID and the provided pass salt. The returned RNG should not
   /// be shared across threads or passes.
   ///
   /// A unique RNG per pass ensures a reproducible random stream even
   /// when other randomness consuming passes are added or removed. In
   /// addition, the random stream will be reproducible across LLVM
/// @{
   /// Set the module-scope inline assembly blocks.
   /// A trailing newline is added if the input doesn't have one.
   void setModuleInlineAsm(StringRef Asm) {
     GlobalScopeAsm = Asm;
     if (!GlobalScopeAsm.empty() && GlobalScopeAsm.back() != '\n')
       GlobalScopeAsm += '\n';
   }
 
+  /// Set the module version number.
+  void setModuleVersionNum(unsigned V) { VersionNum = V; }
+
   /// Append to the module-scope inline assembly blocks.
   /// A trailing newline is added if the input doesn't have one.
   void appendModuleInlineAsm(StringRef Asm) {
     GlobalScopeAsm += Asm;
     if (!GlobalScopeAsm.empty() && GlobalScopeAsm.back() != '\n')
       GlobalScopeAsm += '\n';
   }
 

lib/Bitcode/Reader/BitcodeReader.cpp

Error BitcodeReaderBase::error(const Twine &Message) {
   return ::error(FullMsg);
 }
 
 Expected<unsigned>
 BitcodeReaderBase::parseVersionRecord(ArrayRef<uint64_t> Record) {
   if (Record.empty())
     return error("Invalid record");
   unsigned ModuleVersion = Record[0];
-  if (ModuleVersion > 2)
+  if (ModuleVersion > 3)
     return error("Invalid value");
   UseStrtab = ModuleVersion >= 2;
   return ModuleVersion;
 }
 
 std::pair<StringRef, ArrayRef<uint64_t>>
 BitcodeReaderBase::readNameFromStrtab(ArrayRef<uint64_t> Record) {
   if (!UseStrtab)
     return {"", Record};
   // Invalid reference. Let the caller complain about the record being empty.
   if (Record[0] + Record[1] > Strtab.size())
     return {"", {}};
case bitc::COMDAT_SELECTION_KIND_LARGEST:
     return Comdat::Largest;
   case bitc::COMDAT_SELECTION_KIND_NO_DUPLICATES:
     return Comdat::NoDuplicates;
   case bitc::COMDAT_SELECTION_KIND_SAME_SIZE:
     return Comdat::SameSize;
   }
 }
 
-static FastMathFlags getDecodedFastMathFlags(unsigned Val) {
+static FastMathFlags getDecodedFastMathFlags(unsigned Val, bool UseOldFast) {
   FastMathFlags FMF;
   if (0 != (Val & FastMathFlags::AllowReassoc))
     FMF.setAllowReassoc();
   if (0 != (Val & FastMathFlags::NoNaNs))
     FMF.setNoNaNs();
   if (0 != (Val & FastMathFlags::NoInfs))
     FMF.setNoInfs();
   if (0 != (Val & FastMathFlags::NoSignedZeros))
     FMF.setNoSignedZeros();
   if (0 != (Val & FastMathFlags::AllowReciprocal))
     FMF.setAllowReciprocal();
   if (0 != (Val & FastMathFlags::AllowContract))
     FMF.setAllowContract(true);
   if (0 != (Val & FastMathFlags::ApproxFunc))
     FMF.setApproxFunc();
+  // Support the old fast math, by bitcode version, as a mapping to reassoc 
+  // which equates to UnsafeAlgebra, use this to upgrade to current fast.
+  if (FMF.allowReassoc() && UseOldFast)
+    FMF.setFast();
   return FMF;
 }
 
 static void upgradeDLLImportExportLinkage(GlobalValue *GV, unsigned Val) {
   switch (Val) {
   case 5: GV->setDLLStorageClass(GlobalValue::DLLImportStorageClass); break;
   case 6: GV->setDLLStorageClass(GlobalValue::DLLExportStorageClass); break;
   }
while (true) {
     auto BitCode = Stream.readRecord(Entry.ID, Record);
     switch (BitCode) {
     default: break;  // Default behavior, ignore unknown content.
     case bitc::MODULE_CODE_VERSION: {
       Expected<unsigned> VersionOrErr = parseVersionRecord(Record);
       if (!VersionOrErr)
         return VersionOrErr.takeError();
       UseRelativeIDs = *VersionOrErr >= 1;
+      TheModule->setModuleVersionNum(*VersionOrErr);
       break;
     }
     case bitc::MODULE_CODE_TRIPLE: {  // TRIPLE: [strchr x N]
       std::string S;
       if (convertToString(Record, 0, S))
         return error("Invalid record");
       TheModule->setTargetTriple(S);
       break;
Error BitcodeReader::parseFunctionBody(Function *F) {
 
   // Unexpected unresolved metadata when parsing function.
   if (MDLoader->hasFwdRefs())
     return error("Invalid function metadata: incoming forward references");
 
   InstructionList.clear();
   unsigned ModuleValueListSize = ValueList.size();
   unsigned ModuleMDLoaderSize = MDLoader->size();
+  bool UseOldFast = (TheModule->getModuleVersionNum() < 3);
 
   // Add all the function arguments to the value table.
   for (Argument &I : F->args())
     ValueList.push_back(&I);
 
   unsigned NextValueNo = ValueList.size();
   BasicBlock *CurBB = nullptr;
   unsigned CurBBNo = 0;
case bitc::FUNC_CODE_INST_BINOP: {    // BINOP: [opval, ty, opval, opcode]
             cast<BinaryOperator>(I)->setHasNoUnsignedWrap(true);
         } else if (Opc == Instruction::SDiv ||
                    Opc == Instruction::UDiv ||
                    Opc == Instruction::LShr ||
                    Opc == Instruction::AShr) {
           if (Record[OpNum] & (1 << bitc::PEO_EXACT))
             cast<BinaryOperator>(I)->setIsExact(true);
         } else if (isa<FPMathOperator>(I)) {
-          FastMathFlags FMF = getDecodedFastMathFlags(Record[OpNum]);
+          FastMathFlags FMF = getDecodedFastMathFlags(Record[OpNum], UseOldFast);
           if (FMF.any())
             I->setFastMathFlags(FMF);
         }
 
       }
       break;
     }
     case bitc::FUNC_CODE_INST_CAST: {    // CAST: [opval, opty, destty, castopc]
case bitc::FUNC_CODE_INST_CMP2: { // CMP2: [opty, opval, opval, pred]
       if (getValueTypePair(Record, OpNum, NextValueNo, LHS) ||
           popValue(Record, OpNum, NextValueNo, LHS->getType(), RHS))
         return error("Invalid record");
 
       unsigned PredVal = Record[OpNum];
       bool IsFP = LHS->getType()->isFPOrFPVectorTy();
       FastMathFlags FMF;
       if (IsFP && Record.size() > OpNum+1)
-        FMF = getDecodedFastMathFlags(Record[++OpNum]);
+        FMF = getDecodedFastMathFlags(Record[++OpNum], UseOldFast);
 
       if (OpNum+1 != Record.size())
         return error("Invalid record");
 
       if (LHS->getType()->isFPOrFPVectorTy())
         I = new FCmpInst((FCmpInst::Predicate)PredVal, LHS, RHS);
       else
         I = new ICmpInst((ICmpInst::Predicate)PredVal, LHS, RHS);
case bitc::FUNC_CODE_INST_CALL: {
         return error("Invalid record");
 
       unsigned OpNum = 0;
       AttributeList PAL = getAttributes(Record[OpNum++]);
       unsigned CCInfo = Record[OpNum++];
 
       FastMathFlags FMF;
       if ((CCInfo >> bitc::CALL_FMF) & 1) {
-        FMF = getDecodedFastMathFlags(Record[OpNum++]);
+        FMF = getDecodedFastMathFlags(Record[OpNum++], UseOldFast);
         if (!FMF.any())
           return error("Fast math flags indicator set for call with no FMF");
       }
 
       FunctionType *FTy = nullptr;
       if (CCInfo >> bitc::CALL_EXPLICIT_TYPE & 1 &&
           !(FTy = dyn_cast<FunctionType>(getTypeByID(Record[OpNum++]))))
         return error("Explicit call type is not a function type");

lib/Bitcode/Writer/BitcodeWriter.cpp

 
 protected:
   void writeBitcodeHeader();
   void writeModuleVersion();
 };
 
 void BitcodeWriterBase::writeModuleVersion() {
   // VERSION: [version#]
-  Stream.EmitRecord(bitc::MODULE_CODE_VERSION, ArrayRef<uint64_t>{2});
+  Stream.EmitRecord(bitc::MODULE_CODE_VERSION, ArrayRef<uint64_t>{3});
 }
 
 /// Base class to manage the module bitcode writing, currently subclassed for
 /// ModuleBitcodeWriter and ThinLinkBitcodeWriter.
 class ModuleBitcodeWriterBase : public BitcodeWriterBase {
 protected:
   /// The Module to write to bitcode.
   const Module &M;
if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID, Abbv) !=
       llvm_unreachable("Unexpected abbrev ordering!");
   }
   { // INST_BINOP_FLAGS abbrev for FUNCTION_BLOCK.
     auto Abbv = std::make_shared<BitCodeAbbrev>();
     Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_INST_BINOP));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LHS
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // RHS
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 4)); // opc
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 7)); // flags
     if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID, Abbv) !=
         FUNCTION_INST_BINOP_FLAGS_ABBREV)
       llvm_unreachable("Unexpected abbrev ordering!");
   }
   { // INST_CAST abbrev for FUNCTION_BLOCK.
     auto Abbv = std::make_shared<BitCodeAbbrev>();
     Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_INST_CAST));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));    // OpVal