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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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

Why?

[mcberg2017]

include/llvm/IR/Module.h

189 ↗

(On Diff #134095)

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 ↗

(On Diff #134095)

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

614–615

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/Bitcode/LLVMBitCodes.h

 
 /// OverflowingBinaryOperatorOptionalFlags - Flags for serializing
 /// OverflowingBinaryOperator's SubclassOptionalData contents.
 enum OverflowingBinaryOperatorOptionalFlags {
   OBO_NO_UNSIGNED_WRAP = 0,
   OBO_NO_SIGNED_WRAP = 1
 };
 
+/// FastMath Flags
+/// This is a fixed layout derived from the bitcode emitted by LLVM 5.0
+/// intended to decouple the in-memory representation from the serialization.
+enum FastMathMap {
+  UnsafeAlgebra   = (1 << 0), // Legacy
+  NoNaNs          = (1 << 1),
+  NoInfs          = (1 << 2),
+  NoSignedZeros   = (1 << 3),
+  AllowReciprocal = (1 << 4),
+  AllowContract   = (1 << 5),
+  ApproxFunc      = (1 << 6),
+  AllowReassoc    = (1 << 7)
+};
+
 /// PossiblyExactOperatorOptionalFlags - Flags for serializing
 /// PossiblyExactOperator's SubclassOptionalData contents.
 enum PossiblyExactOperatorOptionalFlags { PEO_EXACT = 0 };
 
 /// Encoded AtomicOrdering values.
 enum AtomicOrderingCodes {
   ORDERING_NOTATOMIC = 0,
   ORDERING_UNORDERED = 1,

lib/Bitcode/Reader/BitcodeReader.cpp

 BitcodeReaderBase::parseVersionRecord(ArrayRef<uint64_t> Record) {
   if (Record.empty())
     return error("Invalid record");
   unsigned ModuleVersion = Record[0];
   if (ModuleVersion > 2)
     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_NO_DUPLICATES:
     return Comdat::NoDuplicates;
   case bitc::COMDAT_SELECTION_KIND_SAME_SIZE:
     return Comdat::SameSize;
   }
 }
 
 static FastMathFlags getDecodedFastMathFlags(unsigned Val) {
   FastMathFlags FMF;
-  if (0 != (Val & FastMathFlags::AllowReassoc))
+  if (0 != (Val & bitc::UnsafeAlgebra))
+    FMF.setFast();
+  if (0 != (Val & bitc::AllowReassoc))
     FMF.setAllowReassoc();
-  if (0 != (Val & FastMathFlags::NoNaNs))
+  if (0 != (Val & bitc::NoNaNs))
     FMF.setNoNaNs();
-  if (0 != (Val & FastMathFlags::NoInfs))
+  if (0 != (Val & bitc::NoInfs))
     FMF.setNoInfs();
-  if (0 != (Val & FastMathFlags::NoSignedZeros))
+  if (0 != (Val & bitc::NoSignedZeros))
     FMF.setNoSignedZeros();
-  if (0 != (Val & FastMathFlags::AllowReciprocal))
+  if (0 != (Val & bitc::AllowReciprocal))
     FMF.setAllowReciprocal();
-  if (0 != (Val & FastMathFlags::AllowContract))
+  if (0 != (Val & bitc::AllowContract))
     FMF.setAllowContract(true);
-  if (0 != (Val & FastMathFlags::ApproxFunc))
+  if (0 != (Val & bitc::ApproxFunc))
     FMF.setApproxFunc();
   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;

lib/Bitcode/Writer/BitcodeWriter.cpp

if (OBO->hasNoSignedWrap())
       Flags |= 1 << bitc::OBO_NO_SIGNED_WRAP;
     if (OBO->hasNoUnsignedWrap())
       Flags |= 1 << bitc::OBO_NO_UNSIGNED_WRAP;
   } else if (const auto *PEO = dyn_cast<PossiblyExactOperator>(V)) {
     if (PEO->isExact())
       Flags |= 1 << bitc::PEO_EXACT;
   } else if (const auto *FPMO = dyn_cast<FPMathOperator>(V)) {
     if (FPMO->hasAllowReassoc())
-      Flags |= FastMathFlags::AllowReassoc;
+      Flags |= bitc::AllowReassoc;
     if (FPMO->hasNoNaNs())
-      Flags |= FastMathFlags::NoNaNs;
+      Flags |= bitc::NoNaNs;
     if (FPMO->hasNoInfs())
-      Flags |= FastMathFlags::NoInfs;
+      Flags |= bitc::NoInfs;
     if (FPMO->hasNoSignedZeros())
-      Flags |= FastMathFlags::NoSignedZeros;
+      Flags |= bitc::NoSignedZeros;
     if (FPMO->hasAllowReciprocal())
-      Flags |= FastMathFlags::AllowReciprocal;
+      Flags |= bitc::AllowReciprocal;
     if (FPMO->hasAllowContract())
-      Flags |= FastMathFlags::AllowContract;
+      Flags |= bitc::AllowContract;
     if (FPMO->hasApproxFunc())
-      Flags |= FastMathFlags::ApproxFunc;
+      Flags |= bitc::ApproxFunc;
   }
 
   return Flags;
 }
 
 void ModuleBitcodeWriter::writeValueAsMetadata(
     const ValueAsMetadata *MD, SmallVectorImpl<uint64_t> &Record) {
   // Mimic an MDNode with a value as one operand.
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
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8)); // 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

test/Bitcode/compatibility-3.6.ll

define void @fastmathflags(float %op1, float %op2) {
   %f.nnan = fadd nnan float %op1, %op2
   ; CHECK: %f.nnan = fadd nnan float %op1, %op2
   %f.ninf = fadd ninf float %op1, %op2
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
   %f.fast = fadd fast float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ;; Type System
 %opaquety = type opaque
 define void @typesystem() {
   %p0 = bitcast i8* null to i32 (i32)*
   ; CHECK: %p0 = bitcast i8* null to i32 (i32)*

test/Bitcode/compatibility-3.7.ll

define void @fastmathflags(float %op1, float %op2) {
   ; CHECK: %f.nnan = fadd nnan float %op1, %op2
   %f.ninf = fadd ninf float %op1, %op2
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
   %f.fast = fadd fast float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ;; Type System
 %opaquety = type opaque
 define void @typesystem() {
   %p0 = bitcast i8* null to i32 (i32)*
   ; CHECK: %p0 = bitcast i8* null to i32 (i32)*

test/Bitcode/compatibility-3.8.ll

define void @fastmathflags(float %op1, float %op2) {
   ; CHECK: %f.nnan = fadd nnan float %op1, %op2
   %f.ninf = fadd ninf float %op1, %op2
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
   %f.fast = fadd fast float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ; Check various fast math flags and floating-point types on calls.
 
 declare float @fmf1()
 declare double @fmf2()
 declare <4 x double> @fmf3()
 
 ; CHECK-LABEL: fastMathFlagsForCalls(
 define void @fastMathFlagsForCalls(float %f, double %d1, <4 x double> %d2) {
   %call.fast = call fast float @fmf1()
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'aml' bits set, so this is not fully 'fast'.
-  ; CHECK: %call.fast = call reassoc nnan ninf nsz arcp float @fmf1()
+  ; CHECK: %call.fast = call fast float @fmf1()
 
   ; Throw in some other attributes to make sure those stay in the right places.
 
   %call.nsz.arcp = notail call nsz arcp double @fmf2()
   ; CHECK: %call.nsz.arcp = notail call nsz arcp double @fmf2()
 
   %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()
   ; CHECK: %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()

test/Bitcode/compatibility-3.9.ll

define void @fastmathflags(float %op1, float %op2) {
   ; CHECK: %f.nnan = fadd nnan float %op1, %op2
   %f.ninf = fadd ninf float %op1, %op2
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
   %f.fast = fadd fast float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ; Check various fast math flags and floating-point types on calls.
 
 declare float @fmf1()
 declare double @fmf2()
 declare <4 x double> @fmf3()
 
 ; CHECK-LABEL: fastMathFlagsForCalls(
 define void @fastMathFlagsForCalls(float %f, double %d1, <4 x double> %d2) {
   %call.fast = call fast float @fmf1()
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %call.fast = call reassoc nnan ninf nsz arcp float @fmf1()
+  ; CHECK: %call.fast = call fast float @fmf1()
 
   ; Throw in some other attributes to make sure those stay in the right places.
 
   %call.nsz.arcp = notail call nsz arcp double @fmf2()
   ; CHECK: %call.nsz.arcp = notail call nsz arcp double @fmf2()
 
   %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()
   ; CHECK: %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()

test/Bitcode/compatibility-4.0.ll

define void @fastmathflags(float %op1, float %op2) {
   %f.nnan = fadd nnan float %op1, %op2
   ; CHECK: %f.nnan = fadd nnan float %op1, %op2
   %f.ninf = fadd ninf float %op1, %op2
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
   %f.fast = fadd fast float %op1, %op2
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ; Check various fast math flags and floating-point types on calls.
 
 declare float @fmf1()
 declare double @fmf2()
 declare <4 x double> @fmf3()
 
 ; CHECK-LABEL: fastMathFlagsForCalls(
 define void @fastMathFlagsForCalls(float %f, double %d1, <4 x double> %d2) {
   %call.fast = call fast float @fmf1()
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'contract' and 'afn' bits set, so this is not fully 'fast'.
-  ; CHECK: %call.fast = call reassoc nnan ninf nsz arcp float @fmf1()
+  ; CHECK: %call.fast = call fast float @fmf1()
 
   ; Throw in some other attributes to make sure those stay in the right places.
 
   %call.nsz.arcp = notail call nsz arcp double @fmf2()
   ; CHECK: %call.nsz.arcp = notail call nsz arcp double @fmf2()
 
   %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()
   ; CHECK: %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()

test/Bitcode/compatibility-5.0.ll

define void @fastmathflags(float %op1, float %op2) {
   ; CHECK: %f.ninf = fadd ninf float %op1, %op2
   %f.nsz = fadd nsz float %op1, %op2
   ; CHECK: %f.nsz = fadd nsz float %op1, %op2
   %f.arcp = fadd arcp float %op1, %op2
   ; CHECK: %f.arcp = fadd arcp float %op1, %op2
   %f.contract = fadd contract float %op1, %op2
   ; CHECK: %f.contract = fadd contract float %op1, %op2
   %f.fast = fadd fast float %op1, %op2
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'afn' bit set, so this is not fully 'fast'.
-  ; CHECK: %f.fast = fadd reassoc nnan ninf nsz arcp contract float %op1, %op2
+  ; CHECK: %f.fast = fadd fast float %op1, %op2
   ret void
 }
 
 ; Check various fast math flags and floating-point types on calls.
 
 declare float @fmf1()
 declare double @fmf2()
 declare <4 x double> @fmf3()
 
 ; CHECK-LABEL: fastMathFlagsForCalls(
 define void @fastMathFlagsForCalls(float %f, double %d1, <4 x double> %d2) {
   %call.fast = call fast float @fmf1()
-  ; 'fast' used to be its own bit, but this changed in Oct 2017.
-  ; The binary test file does not have the newer 'afn' bit set, so this is not fully 'fast'.
-  ; CHECK: %call.fast = call reassoc nnan ninf nsz arcp contract float @fmf1()
+  ; CHECK: %call.fast = call fast float @fmf1()
 
   ; Throw in some other attributes to make sure those stay in the right places.
 
   %call.nsz.arcp = notail call nsz arcp double @fmf2()
   ; CHECK: %call.nsz.arcp = notail call nsz arcp double @fmf2()
 
   %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()
   ; CHECK: %call.nnan.ninf = tail call nnan ninf fastcc <4 x double> @fmf3()