[PATCH] D117552: [Attributes] Make attribute addition behavior consistent

[Nikita Popov via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGda61cb019eb2: [Attributes] Make attribute addition behavior consistent (authored by nikic).

Changed prior to commit:
  https://reviews.llvm.org/D117552?vs=400785&id=401155#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117552/new/

https://reviews.llvm.org/D117552

Files:
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/test/Transforms/Inline/ret_attr_update.ll
  llvm/test/Transforms/InstCombine/deref-alloc-fns.ll


Index: llvm/test/Transforms/InstCombine/deref-alloc-fns.ll
===================================================================
--- llvm/test/Transforms/InstCombine/deref-alloc-fns.ll
+++ llvm/test/Transforms/InstCombine/deref-alloc-fns.ll
@@ -134,7 +134,7 @@
 
 define noalias i8* @malloc_constant_size2() {
 ; CHECK-LABEL: @malloc_constant_size2(
-; CHECK-NEXT:    [[CALL:%.*]] = tail call noalias dereferenceable_or_null(80) i8* @malloc(i64 40)
+; CHECK-NEXT:    [[CALL:%.*]] = tail call noalias dereferenceable_or_null(40) i8* @malloc(i64 40)
 ; CHECK-NEXT:    ret i8* [[CALL]]
 ;
   %call = tail call noalias dereferenceable_or_null(80) i8* @malloc(i64 40)
Index: llvm/test/Transforms/Inline/ret_attr_update.ll
===================================================================
--- llvm/test/Transforms/Inline/ret_attr_update.ll
+++ llvm/test/Transforms/Inline/ret_attr_update.ll
@@ -142,7 +142,7 @@
 
 ; deref attributes have different values on the callee and the call feeding into
 ; the return.
-; AttrBuilder chooses the already existing value and does not overwrite it.
+; AttrBuilder overwrites the existing value.
 define internal i8* @callee6(i8* %p) alwaysinline {
   %r = call dereferenceable_or_null(16) i8* @foo(i8* %p)
   %v = call i8* @baz(i8* %p)
@@ -153,7 +153,7 @@
 define i8* @test6(i8* %ptr, i64 %x) {
 ; CHECK-LABEL: @test6(
 ; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds i8, i8* [[PTR:%.*]], i64 [[X:%.*]]
-; CHECK-NEXT:    [[R_I:%.*]] = call dereferenceable_or_null(16) i8* @foo(i8* [[GEP]])
+; CHECK-NEXT:    [[R_I:%.*]] = call dereferenceable_or_null(12) i8* @foo(i8* [[GEP]])
 ; CHECK-NEXT:    [[V_I:%.*]] = call i8* @baz(i8* [[GEP]])
 ; CHECK-NEXT:    ret i8* [[R_I]]
 ;
Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -627,11 +627,9 @@
   if (!AS.hasAttributes())
     return *this;
 
-  AttrBuilder B(C, AS);
-  for (const auto &I : *this)
-    B.addAttribute(I);
-
- return get(C, B);
+  AttrBuilder B(C, *this);
+  B.merge(AttrBuilder(C, AS));
+  return get(C, B);
 }
 
 AttributeSet AttributeSet::removeAttribute(LLVMContext &C,
@@ -1250,15 +1248,6 @@
   if (!pImpl)
     return AttributeList::get(C, {{Index, AttributeSet::get(C, B)}});
 
-#ifndef NDEBUG
-  // FIXME it is not obvious how this should work for alignment. For now, say
-  // we can't change a known alignment.
-  const MaybeAlign OldAlign = getAttributes(Index).getAlignment();
-  const MaybeAlign NewAlign = B.getAlignment();
-  assert((!OldAlign || !NewAlign || OldAlign == NewAlign) &&
-         "Attempt to change alignment!");
-#endif
-
   AttrBuilder Merged(C, getAttributes(Index));
   Merged.merge(B);
   return setAttributesAtIndex(C, Index, AttributeSet::get(C, Merged));
@@ -1744,13 +1733,12 @@
 }
 
 AttrBuilder &AttrBuilder::merge(const AttrBuilder &B) {
-  // FIXME: What if both have an int/type attribute, but they don't match?!
   for (unsigned Index = 0; Index < Attribute::NumIntAttrKinds; ++Index)
-    if (!IntAttrs[Index])
+    if (B.IntAttrs[Index])
       IntAttrs[Index] = B.IntAttrs[Index];
 
   for (unsigned Index = 0; Index < Attribute::NumTypeAttrKinds; ++Index)
-    if (!TypeAttrs[Index])
+    if (B.TypeAttrs[Index])
       TypeAttrs[Index] = B.TypeAttrs[Index];
 
   Attrs |= B.Attrs;
Index: llvm/include/llvm/IR/Attributes.h
===================================================================
--- llvm/include/llvm/IR/Attributes.h
+++ llvm/include/llvm/IR/Attributes.h
@@ -1048,7 +1048,8 @@
       return removeAttribute(A.getKindAsEnum());
   }
 
-  /// Add the attributes from the builder.
+  /// Add the attributes from the builder. Attributes in the passed builder
+  /// overwrite attributes in this builder if they have the same key.
   AttrBuilder &merge(const AttrBuilder &B);
 
   /// Remove the attributes from the builder.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117552.401155.patch
Type: text/x-patch
Size: 3899 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220119/9941edee/attachment.bin>