PR12696: Attribute bits above 1<<30 are not encoded in bitcode

Attribute bits above 1<<30 are now encoded correctly.  Additionally,
the encoding/decoding functionality has been hoisted to helper functions
in Attributes.h in an effort to help the encoding/decoding to stay in
sync with the Attribute bitcode definitions.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@157581 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 211 insertions, 26 deletions

diff --git a/include/llvm/Attributes.h b/include/llvm/Attributes.h
index 7b17cd2..950a516 100644
--- a/include/llvm/Attributes.h
+++ b/include/llvm/Attributes.h
@@ -223,6 +223,50 @@ inline unsigned getStackAlignmentFromAttrs(Attributes A) {
   return 1U << ((StackAlign.Raw() >> 26) - 1);
 }
 
+/// This returns an integer containing an encoding of all the
+/// LLVM attributes found in the given attribute bitset.  Any
+/// change to this encoding is a breaking change to bitcode
+/// compatibility.
+inline uint64_t encodeLLVMAttributesForBitcode(Attributes Attrs) {
+  // FIXME: It doesn't make sense to store the alignment information as an
+  // expanded out value, we should store it as a log2 value.  However, we can't
+  // just change that here without breaking bitcode compatibility.  If this ever
+  // becomes a problem in practice, we should introduce new tag numbers in the
+  // bitcode file and have those tags use a more efficiently encoded alignment
+  // field.
+
+  // Store the alignment in the bitcode as a 16-bit raw value instead of a
+  // 5-bit log2 encoded value. Shift the bits above the alignment up by
+  // 11 bits.
+
+  uint64_t EncodedAttrs = Attrs.Raw() & 0xffff;
+  if (Attrs & Attribute::Alignment)
+    EncodedAttrs |= (1ull << 16) <<
+      (((Attrs & Attribute::Alignment).Raw()-1) >> 16);
+  EncodedAttrs |= (Attrs.Raw() & (0xfffull << 21)) << 11;
+
+  return EncodedAttrs;
+}
+
+/// This returns an attribute bitset containing the LLVM attributes
+/// that have been decoded from the given integer.  This function
+/// must stay in sync with 'encodeLLVMAttributesForBitcode'.
+inline Attributes decodeLLVMAttributesForBitcode(uint64_t EncodedAttrs) {
+  // The alignment is stored as a 16-bit raw value from bits 31--16.
+  // We shift the bits above 31 down by 11 bits.
+
+  unsigned Alignment = (EncodedAttrs & (0xffffull << 16)) >> 16;
+  assert((!Alignment || isPowerOf2_32(Alignment)) &&
+         "Alignment must be a power of two.");
+
+  Attributes Attrs(EncodedAttrs & 0xffff);
+  if (Alignment)
+    Attrs |= Attribute::constructAlignmentFromInt(Alignment);
+  Attrs |= Attributes((EncodedAttrs & (0xfffull << 32)) >> 11);
+
+  return Attrs;
+}
+
 
 /// The set of Attributes set in Attributes is converted to a
 /// string of equivalent mnemonics. This is, presumably, for writing out
diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index 2b98f01..0e78dce 100644
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -463,20 +463,8 @@ bool BitcodeReader::ParseAttributeBlock() {
         return Error("Invalid ENTRY record");
 
       for (unsigned i = 0, e = Record.size(); i != e; i += 2) {
-        // FIXME: remove in LLVM 3.0
-        // The alignment is stored as a 16-bit raw value from bits 31--16.
-        // We shift the bits above 31 down by 11 bits.
-
-        unsigned Alignment = (Record[i+1] & (0xffffull << 16)) >> 16;
-        if (Alignment && !isPowerOf2_32(Alignment))
-          return Error("Alignment is not a power of two.");
-
-        Attributes ReconstitutedAttr(Record[i+1] & 0xffff);
-        if (Alignment)
-          ReconstitutedAttr |= Attribute::constructAlignmentFromInt(Alignment);
-        ReconstitutedAttr |=
-            Attributes((Record[i+1] & (0xffffull << 32)) >> 11);
-
+        Attributes ReconstitutedAttr =
+          Attribute::decodeLLVMAttributesForBitcode(Record[i+1]);
         Record[i+1] = ReconstitutedAttr.Raw();
       }
 
diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp
index c680866..d454964 100644
--- a/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -177,18 +177,7 @@ static void WriteAttributeTable(const ValueEnumerator &VE,
     for (unsigned i = 0, e = A.getNumSlots(); i != e; ++i) {
       const AttributeWithIndex &PAWI = A.getSlot(i);
       Record.push_back(PAWI.Index);
-
-      // FIXME: remove in LLVM 3.0
-      // Store the alignment in the bitcode as a 16-bit raw value instead of a
-      // 5-bit log2 encoded value. Shift the bits above the alignment up by
-      // 11 bits.
-      uint64_t FauxAttr = PAWI.Attrs.Raw() & 0xffff;
-      if (PAWI.Attrs & Attribute::Alignment)
-        FauxAttr |= (1ull<<16)<<
-            (((PAWI.Attrs & Attribute::Alignment).Raw()-1) >> 16);
-      FauxAttr |= (PAWI.Attrs.Raw() & (0x3FFull << 21)) << 11;
-
-      Record.push_back(FauxAttr);
+      Record.push_back(Attribute::encodeLLVMAttributesForBitcode(PAWI.Attrs));
     }
 
     Stream.EmitRecord(bitc::PARAMATTR_CODE_ENTRY, Record);
diff --git a/test/Bitcode/attributes.ll b/test/Bitcode/attributes.ll
new file mode 100644
index 0000000..502e967
--- /dev/null
+++ b/test/Bitcode/attributes.ll
@@ -0,0 +1,164 @@
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+; PR12696
+
+define void @f1(i8 zeroext)
+; CHECK: define void @f1(i8 zeroext)
+{
+        ret void;
+}
+
+define void @f2(i8 signext)
+; CHECK: define void @f2(i8 signext)
+{
+        ret void;
+}
+
+define void @f3() noreturn
+; CHECK: define void @f3() noreturn
+{
+        ret void;
+}
+
+define void @f4(i8 inreg)
+; CHECK: define void @f4(i8 inreg)
+{
+        ret void;
+}
+
+define void @f5(i8* sret)
+; CHECK: define void @f5(i8* sret)
+{
+        ret void;
+}
+
+define void @f6() nounwind
+; CHECK: define void @f6() nounwind
+{
+        ret void;
+}
+
+define void @f7(i8* noalias)
+; CHECK: define void @f7(i8* noalias)
+{
+        ret void;
+}
+
+define void @f8(i8* byval)
+; CHECK: define void @f8(i8* byval)
+{
+        ret void;
+}
+
+define void @f9(i8* nest)
+; CHECK: define void @f9(i8* nest)
+{
+        ret void;
+}
+
+define void @f10() readnone
+; CHECK: define void @f10() readnone
+{
+        ret void;
+}
+
+define void @f11() readonly
+; CHECK: define void @f11() readonly
+{
+        ret void;
+}
+
+define void @f12() noinline
+; CHECK: define void @f12() noinline
+{
+        ret void;
+}
+
+define void @f13() alwaysinline
+; CHECK: define void @f13() alwaysinline
+{
+        ret void;
+}
+
+define void @f14() optsize
+; CHECK: define void @f14() optsize
+{
+        ret void;
+}
+
+define void @f15() ssp
+; CHECK: define void @f15() ssp
+{
+        ret void;
+}
+
+define void @f16() sspreq
+; CHECK: define void @f16() sspreq
+{
+        ret void;
+}
+
+define void @f17(i8 align 4)
+; CHECK: define void @f17(i8 align 4)
+{
+        ret void;
+}
+
+define void @f18(i8* nocapture)
+; CHECK: define void @f18(i8* nocapture)
+{
+        ret void;
+}
+
+define void @f19() noredzone
+; CHECK: define void @f19() noredzone
+{
+        ret void;
+}
+
+define void @f20() noimplicitfloat
+; CHECK: define void @f20() noimplicitfloat
+{
+        ret void;
+}
+
+define void @f21() naked
+; CHECK: define void @f21() naked
+{
+        ret void;
+}
+
+define void @f22() inlinehint
+; CHECK: define void @f22() inlinehint
+{
+        ret void;
+}
+
+define void @f23() alignstack(4)
+; CHECK: define void @f23() alignstack(4)
+{
+        ret void;
+}
+
+define void @f24() returns_twice
+; CHECK: define void @f24() returns_twice
+{
+        ret void;
+}
+
+define void @f25() uwtable
+; CHECK: define void @f25() uwtable
+{
+        ret void;
+}
+
+define void @f26() nonlazybind
+; CHECK: define void @f26() nonlazybind
+{
+        ret void;
+}
+
+define void @f27() address_safety
+; CHECK: define void @f27() address_safety
+{
+        ret void;
+}