[llvm] 2182665 - [Bitcode] Don't confuse type attributes on declaration and call

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-03-11T17:32:40+01:00
New Revision: 2182665305d90ae7d0d2b573dced9ef0ef414a9a

URL: https://github.com/llvm/llvm-project/commit/2182665305d90ae7d0d2b573dced9ef0ef414a9a
DIFF: https://github.com/llvm/llvm-project/commit/2182665305d90ae7d0d2b573dced9ef0ef414a9a.diff

LOG: [Bitcode] Don't confuse type attributes on declaration and call

We should not be using APIs here that try to fetch the attribute
from both the call attributes and the function attributes. Otherwise
we'll try to upgrade a non-existent sret attribute on the call using
the attribute on the function.

Added: 
    llvm/test/Bitcode/sret-only-on-declaration.ll

Modified: 
    llvm/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/test/Bitcode/invalid.test

Removed: 
    llvm/test/Bitcode/Inputs/missing-element-type-for-attribute.bc


################################################################################
diff  --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index f6f0c924cf222..5a667f55948f0 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -4086,15 +4086,14 @@ Error BitcodeReader::typeCheckLoadStoreInst(Type *ValType, Type *PtrType) {
 
 Error BitcodeReader::propagateAttributeTypes(CallBase *CB,
                                              ArrayRef<unsigned> ArgTyIDs) {
+  AttributeList Attrs = CB->getAttributes();
   for (unsigned i = 0; i != CB->arg_size(); ++i) {
     for (Attribute::AttrKind Kind : {Attribute::ByVal, Attribute::StructRet,
                                      Attribute::InAlloca}) {
-      if (!CB->paramHasAttr(i, Kind) ||
-          CB->getParamAttr(i, Kind).getValueAsType())
+      if (!Attrs.hasParamAttr(i, Kind) ||
+          Attrs.getParamAttr(i, Kind).getValueAsType())
         continue;
 
-      CB->removeParamAttr(i, Kind);
-
       Type *PtrEltTy = getPtrElementTypeByID(ArgTyIDs[i]);
       if (!PtrEltTy)
         return error("Missing element type for typed attribute upgrade");
@@ -4114,7 +4113,7 @@ Error BitcodeReader::propagateAttributeTypes(CallBase *CB,
         llvm_unreachable("not an upgraded type attribute");
       }
 
-      CB->addParamAttr(i, NewAttr);
+      Attrs = Attrs.addParamAttribute(Context, i, NewAttr);
     }
   }
 
@@ -4125,12 +4124,13 @@ Error BitcodeReader::propagateAttributeTypes(CallBase *CB,
       if (!CI.hasArg())
         continue;
 
-      if (CI.isIndirect && !CB->getParamElementType(ArgNo)) {
+      if (CI.isIndirect && !Attrs.getParamElementType(ArgNo)) {
         Type *ElemTy = getPtrElementTypeByID(ArgTyIDs[ArgNo]);
         if (!ElemTy)
           return error("Missing element type for inline asm upgrade");
-        CB->addParamAttr(
-            ArgNo, Attribute::get(Context, Attribute::ElementType, ElemTy));
+        Attrs = Attrs.addParamAttribute(
+            Context, ArgNo,
+            Attribute::get(Context, Attribute::ElementType, ElemTy));
       }
 
       ArgNo++;
@@ -4140,18 +4140,19 @@ Error BitcodeReader::propagateAttributeTypes(CallBase *CB,
   switch (CB->getIntrinsicID()) {
   case Intrinsic::preserve_array_access_index:
   case Intrinsic::preserve_struct_access_index:
-    if (!CB->getParamElementType(0)) {
+    if (!Attrs.getParamElementType(0)) {
       Type *ElTy = getPtrElementTypeByID(ArgTyIDs[0]);
       if (!ElTy)
         return error("Missing element type for elementtype upgrade");
       Attribute NewAttr = Attribute::get(Context, Attribute::ElementType, ElTy);
-      CB->addParamAttr(0, NewAttr);
+      Attrs = Attrs.addParamAttribute(Context, 0, NewAttr);
     }
     break;
   default:
     break;
   }
 
+  CB->setAttributes(Attrs);
   return Error::success();
 }
 

diff  --git a/llvm/test/Bitcode/Inputs/missing-element-type-for-attribute.bc b/llvm/test/Bitcode/Inputs/missing-element-type-for-attribute.bc
deleted file mode 100644
index 5661399fab91f..0000000000000
Binary files a/llvm/test/Bitcode/Inputs/missing-element-type-for-attribute.bc and /dev/null 
diff er

diff  --git a/llvm/test/Bitcode/invalid.test b/llvm/test/Bitcode/invalid.test
index 4e8c7eda3004e..bd2e6ceaab34b 100644
--- a/llvm/test/Bitcode/invalid.test
+++ b/llvm/test/Bitcode/invalid.test
@@ -292,10 +292,6 @@ RUN:   FileCheck --check-prefix=INVALID-DIIMPORTEDENTITY-RECORD %s
 
 INVALID-DIIMPORTEDENTITY-RECORD: Invalid DIImportedEntity record
 
-RUN: not llvm-dis -disable-output -opaque-pointers %p/Inputs/missing-element-type-for-attribute.bc 2>&1 | \
-RUN:   FileCheck --check-prefix=MISSING-ELEMENT-TYPE-FOR-ATTRIBUTE %s
-
-MISSING-ELEMENT-TYPE-FOR-ATTRIBUTE: Missing element type for typed attribute upgrade
 RUN: not llvm-dis -disable-output -opaque-pointers %p/Inputs/invalid-forward-declare.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-FORWARD-DECLARE %s
 

diff  --git a/llvm/test/Bitcode/sret-only-on-declaration.ll b/llvm/test/Bitcode/sret-only-on-declaration.ll
new file mode 100644
index 0000000000000..bc3e61352dfee
--- /dev/null
+++ b/llvm/test/Bitcode/sret-only-on-declaration.ll
@@ -0,0 +1,11 @@
+; RUN: llvm-as -opaque-pointers < %s | llvm-dis -opaque-pointers | FileCheck %s
+
+; CHECK: declare void @decl(ptr sret(i64))
+; CHECK: call void @decl(ptr %arg)
+
+declare void @decl(i64* sret(i64))
+
+define void @test(i64* %arg) {
+  call void @decl(i64* %arg)
+  ret void
+}