[PATCH] D124116: Change Attribute::get to consider 0 value as IntValue

[Anna Thomas via Phabricator via llvm-commits]

anna created this revision.
Herald added subscribers: ormris, dexonsmith, okura, jdoerfert, kuter, hiraditya.
Herald added a project: All.
anna requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: baziotis.
Herald added a project: LLVM.

Alternate approach to D124115 <https://reviews.llvm.org/D124115>. I honestly prefer D124115 <https://reviews.llvm.org/D124115>, but placing
this here to show the alternate way involving fixing locations where we
consider zero value for enum attributes. Note that without the change in
Transforms/IPO/Attributor.cpp, this change fails for the test
test/Transforms/Attributor/nonnull.ll.
There are couple of other failures as well, I just fixed this
Attributor.cpp, but we will need to do the same for other usages where
we make this assumption (enum attribute can contain zero value passed
in).

The benefit of this approach is that it makes the Attribute::get API
clearer and callers will need to honour the API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124116

Files:
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp


Index: llvm/lib/Transforms/IPO/Attributor.cpp
===================================================================
--- llvm/lib/Transforms/IPO/Attributor.cpp
+++ llvm/lib/Transforms/IPO/Attributor.cpp
@@ -942,8 +942,10 @@
       A.getInfoCache().getMustBeExecutedContextExplorer();
   auto EIt = Explorer.begin(getCtxI()), EEnd = Explorer.end(getCtxI());
   for (auto &It : A2K)
-    if (Explorer.findInContextOf(It.first, EIt, EEnd))
-      Attrs.push_back(Attribute::get(Ctx, AK, It.second.Max));
+    if (Explorer.findInContextOf(It.first, EIt, EEnd)) {
+      auto OptVal = It.second.Max == 0 ? None : Optional<uint64_t>(It.second.Max);
+      Attrs.push_back(Attribute::get(Ctx, AK, OptVal));
+    }
   return AttrsSize != Attrs.size();
 }
 
Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -89,7 +89,7 @@
 }
 
 Attribute Attribute::get(LLVMContext &Context, Attribute::AttrKind Kind,
-                         uint64_t Val) {
+                         Optional<uint64_t> Val) {
   if (Val)
     assert(Attribute::isIntAttrKind(Kind) && "Not an int attribute");
   else
@@ -98,7 +98,7 @@
   LLVMContextImpl *pImpl = Context.pImpl;
   FoldingSetNodeID ID;
   ID.AddInteger(Kind);
-  if (Val) ID.AddInteger(Val);
+  if (Val.hasValue()) ID.AddInteger(*Val);
 
   void *InsertPoint;
   AttributeImpl *PA = pImpl->AttrsSet.FindNodeOrInsertPos(ID, InsertPoint);
@@ -106,10 +106,10 @@
   if (!PA) {
     // If we didn't find any existing attributes of the same shape then create a
     // new one and insert it.
-    if (!Val)
+    if (!Val.hasValue())
       PA = new (pImpl->Alloc) EnumAttributeImpl(Kind);
     else
-      PA = new (pImpl->Alloc) IntAttributeImpl(Kind, Val);
+      PA = new (pImpl->Alloc) IntAttributeImpl(Kind, *Val);
     pImpl->AttrsSet.InsertNode(PA, InsertPoint);
   }
 
Index: llvm/include/llvm/IR/Attributes.h
===================================================================
--- llvm/include/llvm/IR/Attributes.h
+++ llvm/include/llvm/IR/Attributes.h
@@ -108,7 +108,8 @@
   //===--------------------------------------------------------------------===//
 
   /// Return a uniquified Attribute object.
-  static Attribute get(LLVMContext &Context, AttrKind Kind, uint64_t Val = 0);
+  static Attribute get(LLVMContext &Context, AttrKind Kind,
+                       Optional<uint64_t> Val = NoneType::None);
   static Attribute get(LLVMContext &Context, StringRef Kind,
                        StringRef Val = StringRef());
   static Attribute get(LLVMContext &Context, AttrKind Kind, Type *Ty);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124116.423998.patch
Type: text/x-patch
Size: 2641 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220420/f9bd71da/attachment.bin>