Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IR] Add new Range attribute using new ConstantRange Attribute type #83171

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

andjo403
Copy link
Contributor

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 27, 2024

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-adt

@llvm/pr-subscribers-llvm-ir

Author: Andreas Jonson (andjo403)

Changes

implementation as discussed in https://discourse.llvm.org/t/rfc-metadata-attachments-for-function-arguments/76420

CC @nikic


Patch is 27.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83171.diff

17 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+18)
  • (modified) llvm/include/llvm/ADT/FoldingSet.h (+7)
  • (modified) llvm/include/llvm/AsmParser/LLParser.h (+1)
  • (modified) llvm/include/llvm/Bitcode/LLVMBitCodes.h (+1)
  • (modified) llvm/include/llvm/IR/Attributes.h (+23)
  • (modified) llvm/include/llvm/IR/Attributes.td (+6)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+50)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+30)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+39-20)
  • (modified) llvm/lib/IR/AttributeImpl.h (+27-1)
  • (modified) llvm/lib/IR/Attributes.cpp (+78-2)
  • (modified) llvm/lib/IR/Verifier.cpp (+5)
  • (modified) llvm/lib/Transforms/Utils/CodeExtractor.cpp (+1)
  • (added) llvm/test/Assembler/range-attribute-invalid.ll (+8)
  • (modified) llvm/test/Bitcode/attributes.ll (+10)
  • (added) llvm/test/Verifier/range-attr.ll (+11)
  • (modified) llvm/utils/TableGen/Attributes.cpp (+6-3)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 60e682ae328a8f..425013c581b378 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1634,6 +1634,24 @@ Currently, only the following parameter attributes are defined:
 
     This attribute cannot be applied to return values.
 
+``range(<ty>,<a>,<b>)``
+    This attribute expresses the possible range the parameter is in. If the 
+    value of the parameter is not in the specified range, a poison value is 
+    returned instead.
+    The argument passed to ``range`` has the following properties:
+
+-  The type must match the scalar type of the parameter.
+-  The pair ``a,b`` represents the range ``[a,b)``.
+-  Both ``a`` and ``b`` are constants.
+-  The range is allowed to wrap.
+-  The range should not represent the full or empty set. That is,
+   ``a!=b``.
+    
+    This attribute may only be applied to parameters with integer or vector of 
+    integer types.
+    
+    For vector-typed parameters, the range is applied element-wise.
+
 .. _gc:
 
 Garbage Collector Strategy Names
diff --git a/llvm/include/llvm/ADT/FoldingSet.h b/llvm/include/llvm/ADT/FoldingSet.h
index f82eabd5044b22..55f56a224cb446 100644
--- a/llvm/include/llvm/ADT/FoldingSet.h
+++ b/llvm/include/llvm/ADT/FoldingSet.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_ADT_FOLDINGSET_H
 #define LLVM_ADT_FOLDINGSET_H
 
+#include "llvm/ADT/APInt.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/SmallVector.h"
@@ -354,6 +355,12 @@ class FoldingSetNodeID {
     AddInteger(unsigned(I));
     AddInteger(unsigned(I >> 32));
   }
+  void AddInteger(APInt Int) {
+    const auto *Parts = Int.getRawData();
+    for (int i = 0, N = Int.getNumWords(); i < N; ++i) {
+      AddInteger(Parts[i]);
+    }
+  }
 
   void AddBoolean(bool B) { AddInteger(B ? 1U : 0U); }
   void AddString(StringRef String);
diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h
index f07f4c61f9d649..887580c1c5893f 100644
--- a/llvm/include/llvm/AsmParser/LLParser.h
+++ b/llvm/include/llvm/AsmParser/LLParser.h
@@ -366,6 +366,7 @@ namespace llvm {
     bool parseFnAttributeValuePairs(AttrBuilder &B,
                                     std::vector<unsigned> &FwdRefAttrGrps,
                                     bool inAttrGrp, LocTy &BuiltinLoc);
+    bool parseRangeAttr(AttrBuilder &B);
     bool parseRequiredTypeAttr(AttrBuilder &B, lltok::Kind AttrToken,
                                Attribute::AttrKind AttrKind);
 
diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
index c6f0ddf29a6da8..c0a52d64a101d0 100644
--- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -724,6 +724,7 @@ enum AttributeKindCodes {
   ATTR_KIND_WRITABLE = 89,
   ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE = 90,
   ATTR_KIND_DEAD_ON_UNWIND = 91,
+  ATTR_KIND_RANGE = 92,
 };
 
 enum ComdatSelectionKindCodes {
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index a4ebe5d732f568..0c2a02514ba0e6 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -37,6 +37,7 @@ class AttributeMask;
 class AttributeImpl;
 class AttributeListImpl;
 class AttributeSetNode;
+class ConstantRange;
 class FoldingSetNodeID;
 class Function;
 class LLVMContext;
@@ -103,6 +104,9 @@ class Attribute {
   static bool isTypeAttrKind(AttrKind Kind) {
     return Kind >= FirstTypeAttr && Kind <= LastTypeAttr;
   }
+  static bool isConstantRangeAttrKind(AttrKind Kind) {
+    return Kind >= FirstConstantRangeAttr && Kind <= LastConstantRangeAttr;
+  }
 
   static bool canUseAsFnAttr(AttrKind Kind);
   static bool canUseAsParamAttr(AttrKind Kind);
@@ -125,6 +129,8 @@ class Attribute {
   static Attribute get(LLVMContext &Context, StringRef Kind,
                        StringRef Val = StringRef());
   static Attribute get(LLVMContext &Context, AttrKind Kind, Type *Ty);
+  static Attribute get(LLVMContext &Context, AttrKind Kind,
+                       const ConstantRange &CR);
 
   /// Return a uniquified Attribute object that has the specific
   /// alignment set.
@@ -180,6 +186,9 @@ class Attribute {
   /// Return true if the attribute is a type attribute.
   bool isTypeAttribute() const;
 
+  /// Return true if the attribute is a ConstantRange attribute.
+  bool isConstantRangeAttribute() const;
+
   /// Return true if the attribute is any kind of attribute.
   bool isValid() const { return pImpl; }
 
@@ -213,6 +222,10 @@ class Attribute {
   /// a type attribute.
   Type *getValueAsType() const;
 
+  /// Return the attribute's value as a ConstantRange. This requires the
+  /// attribute to be a ConstantRange attribute.
+  ConstantRange getValueAsConstantRange() const;
+
   /// Returns the alignment field of an attribute as a byte alignment
   /// value.
   MaybeAlign getAlignment() const;
@@ -251,6 +264,9 @@ class Attribute {
   /// Return the FPClassTest for nofpclass
   FPClassTest getNoFPClass() const;
 
+  /// Returns the value of the range attribute.
+  ConstantRange getRange() const;
+
   /// The Attribute is converted to a string of equivalent mnemonic. This
   /// is, presumably, for writing out the mnemonics for the assembly writer.
   std::string getAsString(bool InAttrGrp = false) const;
@@ -1189,6 +1205,13 @@ class AttrBuilder {
   // Add nofpclass attribute
   AttrBuilder &addNoFPClassAttr(FPClassTest NoFPClassMask);
 
+  /// Add a ConstantRange attribute with the given range.
+  AttrBuilder &addConstantRangeAttr(Attribute::AttrKind Kind,
+                                    const ConstantRange &CR);
+
+  /// Add range attribute.
+  AttrBuilder &addRangeAttr(const ConstantRange &CR);
+
   ArrayRef<Attribute> attrs() const { return Attrs; }
 
   bool operator==(const AttrBuilder &B) const;
diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td
index 08afecf3201512..c5b68d25363282 100644
--- a/llvm/include/llvm/IR/Attributes.td
+++ b/llvm/include/llvm/IR/Attributes.td
@@ -44,6 +44,9 @@ class StrBoolAttr<string S> : Attr<S, []>;
 /// Arbitrary string attribute.
 class ComplexStrAttr<string S, list<AttrProperty> P> : Attr<S, P>;
 
+/// ConstantRange attribute.
+class ConstantRangeAttr<string S, list<AttrProperty> P> : Attr<S, P>;
+
 /// Target-independent enum attributes.
 
 /// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias.
@@ -218,6 +221,9 @@ def OptimizeNone : EnumAttr<"optnone", [FnAttr]>;
 /// Similar to byval but without a copy.
 def Preallocated : TypeAttr<"preallocated", [FnAttr, ParamAttr]>;
 
+/// Function does not access memory.
+def Range : ConstantRangeAttr<"range", [ParamAttr, RetAttr]>;
+
 /// Function does not access memory.
 def ReadNone : EnumAttr<"readnone", [ParamAttr]>;
 
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index a91e2f690999e0..8d2c2cabf457f3 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -1585,6 +1585,9 @@ bool LLParser::parseEnumAttribute(Attribute::AttrKind Attr, AttrBuilder &B,
 
     return true;
   }
+  case Attribute::Range: {
+    return parseRangeAttr(B);
+  }
   default:
     B.addAttribute(Attr);
     Lex.Lex();
@@ -2997,6 +3000,53 @@ bool LLParser::parseRequiredTypeAttr(AttrBuilder &B, lltok::Kind AttrToken,
   return false;
 }
 
+/// parseRangeAttr
+///   ::= range(<ty>,<n>,<n>)
+bool LLParser::parseRangeAttr(AttrBuilder &B) {
+  Lex.Lex();
+
+  APInt Lower;
+  APInt Upper;
+  Type *Ty = nullptr;
+  LocTy TyLoc;
+
+  auto ParseAPSInt = [&](llvm::TypeSize BitWidth, APInt &Val) {
+    if (Lex.getKind() != lltok::APSInt)
+      return tokError("expected integer");
+    if (Lex.getAPSIntVal().getBitWidth() > BitWidth)
+      return tokError("integer is to large for the BitWidth");
+    Val = Lex.getAPSIntVal().extend(BitWidth);
+    Lex.Lex();
+    return false;
+  };
+
+  if (!EatIfPresent(lltok::lparen))
+    return tokError("expected '('");
+  if (parseType(Ty, TyLoc))
+    return true;
+  if (!Ty->isIntegerTy())
+    return error(TyLoc, "must have integer type");
+
+  auto BitWidth = Ty->getPrimitiveSizeInBits();
+
+  if (!EatIfPresent(lltok::comma))
+    return tokError("expected ','");
+  if (ParseAPSInt(BitWidth, Lower))
+    return true;
+  if (!EatIfPresent(lltok::comma))
+    return tokError("expected ','");
+  if (ParseAPSInt(BitWidth, Upper))
+    return true;
+  if (Lower == Upper)
+    return tokError("The range should not represent the full or empty set!");
+
+  if (!EatIfPresent(lltok::rparen))
+    return tokError("expected ')'");
+
+  B.addRangeAttr(ConstantRange(Lower, Upper));
+  return false;
+}
+
 /// parseOptionalOperandBundles
 ///    ::= /*empty*/
 ///    ::= '[' OperandBundle [, OperandBundle ]* ']'
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 832907a3f53f5f..148dcdbdf221dc 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -2103,6 +2103,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
     return Attribute::CoroDestroyOnlyWhenComplete;
   case bitc::ATTR_KIND_DEAD_ON_UNWIND:
     return Attribute::DeadOnUnwind;
+  case bitc::ATTR_KIND_RANGE:
+    return Attribute::Range;
   }
 }
 
@@ -2272,6 +2274,34 @@ Error BitcodeReader::parseAttributeGroupBlock() {
             return error("Not a type attribute");
 
           B.addTypeAttr(Kind, HasType ? getTypeByID(Record[++i]) : nullptr);
+        } else if (Record[i] == 7 || Record[i] == 8) {
+          bool WideAPInt = Record[i++] == 8;
+          Attribute::AttrKind Kind;
+          if (Error Err = parseAttrKind(Record[i++], &Kind))
+            return Err;
+          if (!Attribute::isConstantRangeAttrKind(Kind))
+            return error("Not a ConstantRange attribute");
+
+          unsigned ValueBitWidth = Record[i++];
+          unsigned ActiveWords = 1;
+          if (WideAPInt)
+            ActiveWords = Record[i++];
+          APInt Lower =
+              readWideAPInt(ArrayRef(&Record[i], ActiveWords), ValueBitWidth);
+          i += ActiveWords;
+          ActiveWords = 1;
+          if (WideAPInt)
+            ActiveWords = Record[i++];
+          APInt Upper =
+              readWideAPInt(ArrayRef(&Record[i], ActiveWords), ValueBitWidth);
+          i += ActiveWords - 1;
+
+          if (Lower == Upper)
+            return error(
+                "The range should not represent the full or empty set!");
+
+          ConstantRange Range(Lower, Upper);
+          B.addConstantRangeAttr(Kind, Range);
         } else {
           return error("Invalid attribute group entry");
         }
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 656f2a6ce870f5..d12f959861a748 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -844,6 +844,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
     return bitc::ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE;
   case Attribute::DeadOnUnwind:
     return bitc::ATTR_KIND_DEAD_ON_UNWIND;
+  case Attribute::Range:
+    return bitc::ATTR_KIND_RANGE;
   case Attribute::EndAttrKinds:
     llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:
@@ -856,6 +858,24 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   llvm_unreachable("Trying to encode unknown attribute");
 }
 
+static void emitSignedInt64(SmallVectorImpl<uint64_t> &Vals, uint64_t V) {
+  if ((int64_t)V >= 0)
+    Vals.push_back(V << 1);
+  else
+    Vals.push_back((-V << 1) | 1);
+}
+
+static void emitWideAPInt(SmallVectorImpl<uint64_t> &Vals, const APInt &A) {
+  // We have an arbitrary precision integer value to write whose
+  // bit width is > 64. However, in canonical unsigned integer
+  // format it is likely that the high bits are going to be zero.
+  // So, we only write the number of active words.
+  unsigned NumWords = A.getActiveWords();
+  const uint64_t *RawData = A.getRawData();
+  for (unsigned i = 0; i < NumWords; i++)
+    emitSignedInt64(Vals, RawData[i]);
+}
+
 void ModuleBitcodeWriter::writeAttributeGroupTable() {
   const std::vector<ValueEnumerator::IndexAndAttrSet> &AttrGrps =
       VE.getAttributeGroups();
@@ -889,13 +909,30 @@ void ModuleBitcodeWriter::writeAttributeGroupTable() {
           Record.append(Val.begin(), Val.end());
           Record.push_back(0);
         }
-      } else {
-        assert(Attr.isTypeAttribute());
+      } else if (Attr.isTypeAttribute()) {
         Type *Ty = Attr.getValueAsType();
         Record.push_back(Ty ? 6 : 5);
         Record.push_back(getAttrKindEncoding(Attr.getKindAsEnum()));
         if (Ty)
           Record.push_back(VE.getTypeID(Attr.getValueAsType()));
+      } else {
+        assert(Attr.isConstantRangeAttribute());
+        ConstantRange Range = Attr.getValueAsConstantRange();
+        bool WideAPInt = Range.getBitWidth() > 64;
+        Record.push_back(WideAPInt ? 8 : 7);
+        Record.push_back(getAttrKindEncoding(Attr.getKindAsEnum()));
+        Record.push_back(Range.getBitWidth());
+        if (WideAPInt) {
+          const APInt &Lower = Range.getLower();
+          Record.push_back(Lower.getActiveWords());
+          emitWideAPInt(Record, Lower);
+          const APInt &Upper = Range.getUpper();
+          Record.push_back(Upper.getActiveWords());
+          emitWideAPInt(Record, Upper);
+        } else {
+          emitSignedInt64(Record, *Range.getLower().getRawData());
+          emitSignedInt64(Record, *Range.getUpper().getRawData());
+        }
       }
     }
 
@@ -1716,24 +1753,6 @@ void ModuleBitcodeWriter::writeDIGenericSubrange(
   Record.clear();
 }
 
-static void emitSignedInt64(SmallVectorImpl<uint64_t> &Vals, uint64_t V) {
-  if ((int64_t)V >= 0)
-    Vals.push_back(V << 1);
-  else
-    Vals.push_back((-V << 1) | 1);
-}
-
-static void emitWideAPInt(SmallVectorImpl<uint64_t> &Vals, const APInt &A) {
-  // We have an arbitrary precision integer value to write whose
-  // bit width is > 64. However, in canonical unsigned integer
-  // format it is likely that the high bits are going to be zero.
-  // So, we only write the number of active words.
-  unsigned NumWords = A.getActiveWords();
-  const uint64_t *RawData = A.getRawData();
-  for (unsigned i = 0; i < NumWords; i++)
-    emitSignedInt64(Vals, RawData[i]);
-}
-
 void ModuleBitcodeWriter::writeDIEnumerator(const DIEnumerator *N,
                                             SmallVectorImpl<uint64_t> &Record,
                                             unsigned Abbrev) {
diff --git a/llvm/lib/IR/AttributeImpl.h b/llvm/lib/IR/AttributeImpl.h
index 78496786b0ae95..9a6427bbc3d557 100644
--- a/llvm/lib/IR/AttributeImpl.h
+++ b/llvm/lib/IR/AttributeImpl.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Attributes.h"
+#include "llvm/IR/ConstantRange.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <cassert>
 #include <cstddef>
@@ -46,6 +47,7 @@ class AttributeImpl : public FoldingSetNode {
     IntAttrEntry,
     StringAttrEntry,
     TypeAttrEntry,
+    ConstantRangeAttrEntry,
   };
 
   AttributeImpl(AttrEntryKind KindID) : KindID(KindID) {}
@@ -59,6 +61,9 @@ class AttributeImpl : public FoldingSetNode {
   bool isIntAttribute() const { return KindID == IntAttrEntry; }
   bool isStringAttribute() const { return KindID == StringAttrEntry; }
   bool isTypeAttribute() const { return KindID == TypeAttrEntry; }
+  bool isConstantRangeAttribute() const {
+    return KindID == ConstantRangeAttrEntry;
+  }
 
   bool hasAttribute(Attribute::AttrKind A) const;
   bool hasAttribute(StringRef Kind) const;
@@ -72,6 +77,8 @@ class AttributeImpl : public FoldingSetNode {
 
   Type *getValueAsType() const;
 
+  ConstantRange getValueAsConstantRange() const;
+
   /// Used when sorting the attributes.
   bool operator<(const AttributeImpl &AI) const;
 
@@ -82,8 +89,10 @@ class AttributeImpl : public FoldingSetNode {
       Profile(ID, getKindAsEnum(), getValueAsInt());
     else if (isStringAttribute())
       Profile(ID, getKindAsString(), getValueAsString());
-    else
+    else if (isTypeAttribute())
       Profile(ID, getKindAsEnum(), getValueAsType());
+    else
+      Profile(ID, getKindAsEnum(), getValueAsConstantRange());
   }
 
   static void Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind) {
@@ -108,6 +117,13 @@ class AttributeImpl : public FoldingSetNode {
     ID.AddInteger(Kind);
     ID.AddPointer(Ty);
   }
+
+  static void Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind,
+                      const ConstantRange &CR) {
+    ID.AddInteger(Kind);
+    ID.AddInteger(CR.getLower());
+    ID.AddInteger(CR.getUpper());
+  }
 };
 
 static_assert(std::is_trivially_destructible<AttributeImpl>::value,
@@ -196,6 +212,16 @@ class TypeAttributeImpl : public EnumAttributeImpl {
   Type *getTypeValue() const { return Ty; }
 };
 
+class ConstantRangeAttributeImpl : public EnumAttributeImpl {
+  ConstantRange CR;
+
+public:
+  ConstantRangeAttributeImpl(Attribute::AttrKind Kind, const ConstantRange &CR)
+      : EnumAttributeImpl(ConstantRangeAttrEntry, Kind), CR(CR) {}
+
+  ConstantRange getConstantRangeValue() const { return CR; }
+};
+
 class AttributeBitSet {
   /// Bitset with a bit for each available attribute Attribute::AttrKind.
   uint8_t AvailableAttrs[12] = {};
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index 00acbbe7989d8a..6cd78641584f33 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -24,6 +24,7 @@
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/AttributeMask.h"
+#include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Type.h"
@@ -165,6 +166,30 @@ Attribute Attribute::get(LLVMContext &Context, Attribute::AttrKind Kind,
   return Attribute(PA);
 }
 
+Attribute Attribute::get(LLVMContext &Context, Attribute::AttrKind Kind,
+                         const ConstantRange &CR) {
+  assert(Attribute::isConstantRangeAttrKind(Kind) &&
+         "Not a ConstantRange attribute");
+  LLVMContextImpl *pImpl = Context.pImpl;
+  FoldingSetNodeID ID;
+  ID.AddInteger(Kind);
+  ID.AddInteger(CR.getLower());
+  ID.AddInteger(CR.getUpper());
+
+  void *InsertPoint;
+  AttributeImpl *PA = pImpl->AttrsSet.FindNodeOrInsertPos(ID, InsertPoint);
+
+  if (!PA) {
+    // If we didn't find any existing attributes of the same shape then create a
+    // new one and insert it.
+    PA = new (pImpl->Alloc) ConstantRangeAttributeImpl(Kind, CR);
+    pImpl->AttrsSet.InsertNode(PA, InsertPoint);
+  }
+
+  // Return the Attribute that we found or created.
+  return Attribute(PA);
+}
+
 Attribute Attribute::getWithAlignment(LLVMContext &Context, Align A) {
   assert(A <= llvm::Value::MaximumAlignment && "Alignment too large.");
   return get(Context, Alignment, A.value());
@@ -287,9 +312,14 @@ bool Attribute::isTypeAttribute() const {
   return pImpl && pImpl->isTypeAttribute();
 }
 
+bool Attribute::isConstantRangeAttribute() const {
+  return pImpl && pImpl->isConstantRangeAttribute();
+}
+
 Attribute::AttrKind Attribute::getKindAsEnum() const {
   if (!pImpl) return None;
-  assert((isEnumAttribute() || isIntAttribute() || isTypeAttribute()) &&
+  assert((isEnumAttribute() || isIntAttribute() || isTypeAttribute() ||
+          isConstantRangeAttribute()) &&
          "Invalid attribute type to get the kind as an enum!");
   return pImpl->getKindAsEnum();
 }
@@ -329,6 +359,11 @@ Type *Attribute::getValueAsType() const {
   return pImpl->getValueAsType();
 }
 
+ConstantRange Attribute::getValueAsConstantRange() const {
+  assert(isConstantRangeAttribute() &&
+         "Invalid attribute type to get the value as a ConstantRange!");
+  return pImpl->getValueAsConstantRange();
+}
 
 bool Attribute::hasAttribute(AttrKind Kind) const {
   return (pImpl && pImpl->hasAttribute(Kind)) || (!pImpl && Kind == None);
@@ -408,6 +443,12 @@ FPClassTest Attribute::getNoFPClass() const {
   return static_cast<FPClassTest>(pImpl->getValueAsInt());
 }
 
+ConstantRange Attribute::getRange() const {
+  assert(hasAttribute(Attribute::Range) &&
+         "Trying to get range args from non-range attribute");
+  return pImpl->getValueAsConstantRange();
+}
+
...
[truncated]

Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@andjo403
Copy link
Contributor Author

force pushed to fix the email.

@jdoerfert jdoerfert requested a review from nikic February 27, 2024 21:25
llvm/docs/LangRef.rst Outdated Show resolved Hide resolved
``range(<ty>,<a>,<b>)``
This attribute expresses the possible range the parameter is in. If the
value of the parameter is not in the specified range, a poison value is
returned instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like "is returned", this is not a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was copied from the range metadata but I agree it sounds strange, how about:
The value of the parameter is in the specified range or is poison.

; CHECK: The range should not represent the full or empty set!
define void @range_empty(i8 range(i8,0,0) %a) {
ret void
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check mismatching types in various combinations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some more tests

@andjo403
Copy link
Contributor Author

andjo403 commented Mar 6, 2024

ping @jdoerfert @nikic do you think that more is needed for this commit?

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty reasonable to me.

llvm/docs/LangRef.rst Outdated Show resolved Hide resolved
llvm/include/llvm/ADT/FoldingSet.h Outdated Show resolved Hide resolved
llvm/include/llvm/IR/Attributes.td Outdated Show resolved Hide resolved
} else {
emitSignedInt64(Record, *Range.getLower().getRawData());
emitSignedInt64(Record, *Range.getUpper().getRawData());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a ConstantRange encoding that can be reused for other purposes as well. The way this is implemented now this will be a bit hard, as the wide distinction is part of the attribute kind.

I have some local changes that also needed this, and these are the implementations I used:

// BitcodeWriter
static void emitConstantRange(SmallVectorImpl<uint64_t> &Record,
                              const ConstantRange &CR) {
  unsigned BitWidth = CR.getBitWidth();
  Record.push_back(BitWidth);
  if (BitWidth > 64) {
    Record.push_back(CR.getLower().getActiveWords() |
        (uint64_t(CR.getUpper().getActiveWords()) << 32));
    emitWideAPInt(Record, CR.getLower());
    emitWideAPInt(Record, CR.getUpper());
  } else {
    emitSignedInt64(Record, CR.getLower().getSExtValue());
    emitSignedInt64(Record, CR.getUpper().getSExtValue());
  }
}

// BitcodeReader
Expected<ConstantRange> readConstantRange(ArrayRef<uint64_t> Record,
                                          unsigned &OpNum) {
  if (Record.size() - OpNum < 3)
    return error("Too few records for range");
  unsigned BitWidth = Record[OpNum++];
  if (BitWidth > 64) {
    unsigned LowerActiveWords = Record[OpNum];
    unsigned UpperActiveWords = Record[OpNum++] >> 32;
    if (Record.size() - OpNum < LowerActiveWords + UpperActiveWords)
      return error("Too few records for range");
    APInt Lower =
        readWideAPInt(ArrayRef(&Record[OpNum], LowerActiveWords), BitWidth);
    OpNum += LowerActiveWords;
    APInt Upper =
        readWideAPInt(ArrayRef(&Record[OpNum], UpperActiveWords), BitWidth);
    OpNum += UpperActiveWords;
    return ConstantRange(Lower, Upper);
  } else {        
    int64_t Start = BitcodeReader::decodeSignRotatedValue(Record[OpNum++]);
    int64_t End = BitcodeReader::decodeSignRotatedValue(Record[OpNum++]);
    return ConstantRange(APInt(BitWidth, Start), APInt(BitWidth, End));
  }
} 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes much nicer to have separat functions was trying to restrict number of used bytes as other types did it.

llvm/lib/AsmParser/LLParser.cpp Outdated Show resolved Hide resolved
llvm/lib/AsmParser/LLParser.cpp Outdated Show resolved Hide resolved
llvm/lib/AsmParser/LLParser.cpp Outdated Show resolved Hide resolved
llvm/lib/AsmParser/LLParser.cpp Outdated Show resolved Hide resolved
llvm/lib/AsmParser/LLParser.cpp Outdated Show resolved Hide resolved
llvm/test/Bitcode/attributes.ll Outdated Show resolved Hide resolved
@andjo403
Copy link
Contributor Author

andjo403 commented Mar 6, 2024

@nikic thanks for all the comments have addressed them in the latest commit.

the parameter is in the specified range or is poison.
The arguments passed to ``range`` has the following properties:

- The type must match the scalar type of the parameter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any downside in having the range just apply to all vector elems?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does apply to all vector elements:

For vector-typed parameters, the range is applied element-wise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see, for vector types you pull out the element type when creating the range.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes on the LangRef wording, otherwise this looks fine to me.

the parameter is in the specified range or is poison.
The arguments passed to ``range`` has the following properties:

- The type must match the scalar type of the parameter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The type must match the scalar type of the parameter.
- The type must match the scalar type of the parameter or return value.

Comment on lines 1648 to 1649
This attribute may only be applied to parameters with integer or vector of
integer types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This attribute may only be applied to parameters with integer or vector of
integer types.
This attribute may only be applied to parameters or return values with integer or vector of
integer types.

``range(<ty> <a>, <b>)``
This attribute expresses the possible range the parameter is in. The value of
the parameter is in the specified range or is poison.
The arguments passed to ``range`` has the following properties:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The arguments passed to ``range`` has the following properties:
The arguments passed to ``range`` have the following properties:

Comment on lines 1638 to 1639
This attribute expresses the possible range the parameter is in. The value of
the parameter is in the specified range or is poison.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This attribute expresses the possible range the parameter is in. The value of
the parameter is in the specified range or is poison.
This attribute expresses the possible range of the parameter or return value.
If the value is not in the specified range, it is converted to poison.

@andjo403
Copy link
Contributor Author

andjo403 commented Mar 7, 2024

Updated with the suggested changes to LangRef.

is the next step to squash the commits or is that done during the merge to main?

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic
Copy link
Contributor

nikic commented Mar 8, 2024

is the next step to squash the commits or is that done during the merge to main?

It will be squashed when merging.

@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 8, 2024

@andjo403 Would you like to add the range attribute support in llvm::computeConstantRange?

@dtcxzyw dtcxzyw linked an issue Mar 8, 2024 that may be closed by this pull request
@andjo403
Copy link
Contributor Author

andjo403 commented Mar 8, 2024

@dtcxzyw I have started to prepare a PR that will use the new attribute in among other things llvm::computeConstantRange will have more time this weekend to work on it.

I also need help with merging this PR as I do not have permission to do that.

@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 8, 2024

@dtcxzyw I have started to prepare a PR that will use the new attribute in among other things llvm::computeConstantRange will have more time this weekend to work on it.

I also need help with merging this PR as I do not have permission to do that.

Thank you!

@dtcxzyw dtcxzyw merged commit e0d4906 into llvm:main Mar 8, 2024
4 of 5 checks passed
Copy link

github-actions bot commented Mar 8, 2024

@andjo403 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

@fmayer
Copy link
Contributor

fmayer commented Mar 8, 2024

This has a memory leak detected by our sanitizer bot https://lab.llvm.org/buildbot/#/builders/168/builds/19110/steps/10/logs/stdio

FAIL: LLVM :: Bitcode/attributes.ll (24010 of 81188)
******************** TEST 'LLVM :: Bitcode/attributes.ll' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
RUN: at line 1: /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-as < /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/Bitcode/attributes.ll | /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-dis | /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/Bitcode/attributes.ll
+ /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-dis
+ /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/Bitcode/attributes.ll
+ /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-as
=================================================================
==1078367==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x55605c69b9bd in operator new[](unsigned long) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:89:3
    #1 0x55605cbebe8c in getMemory /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:45:10
    #2 0x55605cbebe8c in llvm::APInt::initSlowCase(llvm::APInt const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:86:12
    #3 0x55605c8d8de4 in APInt /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/ADT/APInt.h:158:7
    #4 0x55605c8d8de4 in ConstantRange /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/IR/ConstantRange.h:47:21
    #5 0x55605c8d8de4 in ConstantRangeAttributeImpl /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/AttributeImpl.h:220:58
    #6 0x55605c8d8de4 in llvm::Attribute::get(llvm::LLVMContext&, llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:185:29
    #7 0x55605c8e830c in addConstantRangeAttr /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1947:23
    #8 0x55605c8e830c in llvm::AttrBuilder::addRangeAttr(llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1951:10
    #9 0x55605c6e5101 in llvm::LLParser::parseRangeAttr(llvm::AttrBuilder&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:3050:5
    #10 0x55605c6e0d57 in llvm::LLParser::parseEnumAttribute(llvm::Attribute::AttrKind, llvm::AttrBuilder&, bool) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:1600:12
    #11 0x55605c6e83b9 in llvm::LLParser::parseOptionalParamOrReturnAttrs(llvm::AttrBuilder&, bool) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:1964:9
    #12 0x55605c6f0200 in parseOptionalParamAttrs /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/AsmParser/LLParser.h:294:14
    #13 0x55605c6f0200 in llvm::LLParser::parseArgumentList(llvm::SmallVectorImpl<llvm::LLParser::ArgInfo>&, llvm::SmallVectorImpl<unsigned int>&, bool&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:3146:31
    #14 0x55605c6c977e in llvm::LLParser::parseFunctionHeader(llvm::Function*&, bool, unsigned int&, llvm::SmallVectorImpl<unsigned int>&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:6203:7
    #15 0x55605c6c1a46 in llvm::LLParser::parseDefine() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:716:10
    #16 0x55605c6baa55 in llvm::LLParser::parseTopLevelEntities() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:522:11
    #17 0x55605c6b9fa6 in llvm::LLParser::Run(bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:99:10
    #18 0x55605c6a6c89 in parseAssemblyInto(llvm::MemoryBufferRef, llvm::Module*, llvm::ModuleSummaryIndex*, llvm::SMDiagnostic&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:34:8
    #19 0x55605c6a7a16 in parseAssemblyWithIndex(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:83:7
    #20 0x55605c6a7ffa in parseAssemblyFileWithIndex(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:113:10
    #21 0x55605c6a7cf9 in llvm::parseAssemblyFileWithIndex(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:122:10
    #22 0x55605c6a289d in main /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-as/llvm-as.cpp:134:22
    #23 0x7fbde3e23a8f  (/lib/x86_64-linux-gnu/libc.so.6+0x23a8f) (BuildId: d320ce4e63925d698610ed423fc4b1f0e8ed51f1)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x55605c69b9bd in operator new[](unsigned long) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:89:3
    #1 0x55605cbebe8c in getMemory /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:45:10
    #2 0x55605cbebe8c in llvm::APInt::initSlowCase(llvm::APInt const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:86:12
    #3 0x55605c8d8d5f in APInt /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/ADT/APInt.h:158:7
    #4 0x55605c8d8d5f in ConstantRange /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/IR/ConstantRange.h:47:21
    #5 0x55605c8d8d5f in ConstantRangeAttributeImpl /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/AttributeImpl.h:220:58
    #6 0x55605c8d8d5f in llvm::Attribute::get(llvm::LLVMContext&, llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:185:29
    #7 0x55605c8e830c in addConstantRangeAttr /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1947:23
    #8 0x55605c8e830c in llvm::AttrBuilder::addRangeAttr(llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1951:10
    #9 0x55605c6e5101 in llvm::LLParser::parseRangeAttr(llvm::AttrBuilder&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:3050:5
    #10 0x55605c6e0d57 in llvm::LLParser::parseEnumAttribute(llvm::Attribute::AttrKind, llvm::AttrBuilder&, bool) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:1600:12
    #11 0x55605c6e83b9 in llvm::LLParser::parseOptionalParamOrReturnAttrs(llvm::AttrBuilder&, bool) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:1964:9
    #12 0x55605c6f0200 in parseOptionalParamAttrs /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/AsmParser/LLParser.h:294:14
    #13 0x55605c6f0200 in llvm::LLParser::parseArgumentList(llvm::SmallVectorImpl<llvm::LLParser::ArgInfo>&, llvm::SmallVectorImpl<unsigned int>&, bool&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:3146:31
    #14 0x55605c6c977e in llvm::LLParser::parseFunctionHeader(llvm::Function*&, bool, unsigned int&, llvm::SmallVectorImpl<unsigned int>&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:6203:7
    #15 0x55605c6c1a46 in llvm::LLParser::parseDefine() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:716:10
    #16 0x55605c6baa55 in llvm::LLParser::parseTopLevelEntities() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:522:11
    #17 0x55605c6b9fa6 in llvm::LLParser::Run(bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:99:10
    #18 0x55605c6a6c89 in parseAssemblyInto(llvm::MemoryBufferRef, llvm::Module*, llvm::ModuleSummaryIndex*, llvm::SMDiagnostic&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:34:8
    #19 0x55605c6a7a16 in parseAssemblyWithIndex(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:83:7
    #20 0x55605c6a7ffa in parseAssemblyFileWithIndex(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, bool, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:113:10
    #21 0x55605c6a7cf9 in llvm::parseAssemblyFileWithIndex(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, llvm::function_ref<std::__1::optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> (llvm::StringRef, llvm::StringRef)>) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:122:10
    #22 0x55605c6a289d in main /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-as/llvm-as.cpp:134:22
    #23 0x7fbde3e23a8f  (/lib/x86_64-linux-gnu/libc.so.6+0x23a8f) (BuildId: d320ce4e63925d698610ed423fc4b1f0e8ed51f1)
SUMMARY: AddressSanitizer: 32 byte(s) leaked in 2 allocation(s).
=================================================================
==1078369==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x5582992d4fed in operator new[](unsigned long) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:89:3
    #1 0x55829974557c in getMemory /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:45:10
    #2 0x55829974557c in llvm::APInt::initSlowCase(llvm::APInt const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:86:12
    #3 0x55829942bf84 in APInt /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/ADT/APInt.h:158:7
    #4 0x55829942bf84 in ConstantRange /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/IR/ConstantRange.h:47:21
    #5 0x55829942bf84 in ConstantRangeAttributeImpl /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/AttributeImpl.h:220:58
    #6 0x55829942bf84 in llvm::Attribute::get(llvm::LLVMContext&, llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:185:29
    #7 0x55829943cea4 in llvm::AttrBuilder::addConstantRangeAttr(llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1947:23
    #8 0x5582993476f1 in parseAttributeGroupBlock /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2315:13
    #9 0x5582993476f1 in (anonymous namespace)::BitcodeReader::parseModule(unsigned long, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4357:25
    #10 0x5582992ebd74 in parseBitcodeInto /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4596:10
    #11 0x5582992ebd74 in llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:8062:22
    #12 0x5582992eeb50 in llvm::BitcodeModule::getLazyModule(llvm::LLVMContext&, bool, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:8082:10
    #13 0x5582992dc460 in main /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-dis/llvm-dis.cpp:205:16
    #14 0x7f361ce23a8f  (/lib/x86_64-linux-gnu/libc.so.6+0x23a8f) (BuildId: d320ce4e63925d698610ed423fc4b1f0e8ed51f1)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x5582992d4fed in operator new[](unsigned long) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:89:3
    #1 0x55829974557c in getMemory /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:45:10
    #2 0x55829974557c in llvm::APInt::initSlowCase(llvm::APInt const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/APInt.cpp:86:12
    #3 0x55829942beff in APInt /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/ADT/APInt.h:158:7
    #4 0x55829942beff in ConstantRange /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/IR/ConstantRange.h:47:21
    #5 0x55829942beff in ConstantRangeAttributeImpl /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/AttributeImpl.h:220:58
    #6 0x55829942beff in llvm::Attribute::get(llvm::LLVMContext&, llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:185:29
    #7 0x55829943cea4 in llvm::AttrBuilder::addConstantRangeAttr(llvm::Attribute::AttrKind, llvm::ConstantRange const&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/Attributes.cpp:1947:23
    #8 0x5582993476f1 in parseAttributeGroupBlock /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2315:13
    #9 0x5582993476f1 in (anonymous namespace)::BitcodeReader::parseModule(unsigned long, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4357:25
    #10 0x5582992ebd74 in parseBitcodeInto /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4596:10
    #11 0x5582992ebd74 in llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:8062:22
    #12 0x5582992eeb50 in llvm::BitcodeModule::getLazyModule(llvm::LLVMContext&, bool, bool, llvm::ParserCallbacks) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp:8082:10
    #13 0x5582992dc460 in main /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-dis/llvm-dis.cpp:205:16
    #14 0x7f361ce23a8f  (/lib/x86_64-linux-gnu/libc.so.6+0x23a8f) (BuildId: d320ce4e63925d698610ed423fc4b1f0e8ed51f1)
SUMMARY: AddressSanitizer: 32 byte(s) leaked in 2 allocation(s).

@nikic
Copy link
Contributor

nikic commented Mar 8, 2024

Hm yeah, unlike the other attributes, this one has a non-trivial dtor, so we can't just stick in to the general LLVMContext BumpPtrAllocator.

@andjo403
Copy link
Contributor Author

andjo403 commented Mar 8, 2024

yes this assert fails so think that is correct:

static_assert(std::is_trivially_destructible<ConstantRangeAttributeImpl>::value,
              "ConstantRangeAttributeImpl should be trivially destructible");

StringAttributeImpl use some TrailingObjects base class maybe some thing that we need to use also?
private TrailingObjects<ConstantRangeAttributeImpl, APInt::WordType> and then add unsigned LowerNumWords and UpperNumWords

@nikic
Copy link
Contributor

nikic commented Mar 8, 2024

@andjo403 I don't think trying to manually manage APInt memory here is worthwhile. It's probably easiest to add an extra SpecificBumpPtrAllocator<ConstantRangeAttributeImpl> in LLVMContextImpl so that it takes care of destruction.

@andjo403
Copy link
Contributor Author

andjo403 commented Mar 8, 2024

hmm ok will try to look in to that my initial thought was something like

class ConstantRangeAttributeImpl final
    : public EnumAttributeImpl,
      private TrailingObjects<ConstantRangeAttributeImpl, APInt::WordType> {
  friend TrailingObjects;

  unsigned NumBits;
  unsigned LowerNumWords;
  unsigned UpperNumWords;

public:
  ConstantRangeAttributeImpl(Attribute::AttrKind Kind, const ConstantRange &CR)
      : EnumAttributeImpl(ConstantRangeAttrEntry, Kind),
        NumBits(CR.getBitWidth()), LowerNumWords(CR.getLower().getNumWords()),
        UpperNumWords(CR.getUpper().getNumWords()) {
    APInt::WordType *TrailingAPInt = getTrailingObjects<APInt::WordType>();
    // Some users rely on zero-termination.
    llvm::copy(ArrayRef<uint64_t>(CR.getLower().getRawData(),
                                  CR.getLower().getNumWords()),
               TrailingAPInt);
    llvm::copy(ArrayRef<uint64_t>(CR.getLower().getRawData(),
                                  CR.getLower().getNumWords()),
               &TrailingAPInt[LowerNumWords]);
  }

  ConstantRange getConstantRangeValue() const {
    return {
        APInt(NumBits, ArrayRef<uint64_t>(getTrailingObjects<APInt::WordType>(),
                                          LowerNumWords)),
        APInt(NumBits,
              ArrayRef<uint64_t>(getTrailingObjects<APInt::WordType>() +
                                     LowerNumWords,
                                 UpperNumWords))};
  }
};

dtcxzyw pushed a commit that referenced this pull request Mar 9, 2024
…e type (#84617)

The only change from #83171 is the
change of the allocator so the destructor is called for
ConstantRangeAttributeImpl.

reverts #84549
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support !range metadata for by-value function arguments
7 participants