Skip to content

Commit

Permalink
[Attributes] Support int attributes with zero value
Browse files Browse the repository at this point in the history
This regularly comes up as a stumbling stone when adding int
attributes: They currently need to be encoded in a way to avoids
the zero value.

This adds support for zero-value int attributes by a) making the
ctor determine int/enum attribute based on attribute kind, not
whether the value is non-zero and b) switching getRawIntAttr()
to return an Optional, so that it's possible to distinguish a zero
value from non-existence.

Differential Revision: https://reviews.llvm.org/D135572
  • Loading branch information
nikic committed Oct 11, 2022
1 parent 24b1340 commit 2569767
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 17 deletions.
12 changes: 6 additions & 6 deletions llvm/include/llvm/IR/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1096,30 +1096,30 @@ class AttrBuilder {
/// invalid if the Kind is not present in the builder.
Attribute getAttribute(StringRef Kind) const;

/// Return raw (possibly packed/encoded) value of integer attribute or 0 if
/// Return raw (possibly packed/encoded) value of integer attribute or None if
/// not set.
uint64_t getRawIntAttr(Attribute::AttrKind Kind) const;
Optional<uint64_t> getRawIntAttr(Attribute::AttrKind Kind) const;

/// Retrieve the alignment attribute, if it exists.
MaybeAlign getAlignment() const {
return MaybeAlign(getRawIntAttr(Attribute::Alignment));
return MaybeAlign(getRawIntAttr(Attribute::Alignment).value_or(0));
}

/// Retrieve the stack alignment attribute, if it exists.
MaybeAlign getStackAlignment() const {
return MaybeAlign(getRawIntAttr(Attribute::StackAlignment));
return MaybeAlign(getRawIntAttr(Attribute::StackAlignment).value_or(0));
}

/// Retrieve the number of dereferenceable bytes, if the
/// dereferenceable attribute exists (zero is returned otherwise).
uint64_t getDereferenceableBytes() const {
return getRawIntAttr(Attribute::Dereferenceable);
return getRawIntAttr(Attribute::Dereferenceable).value_or(0);
}

/// Retrieve the number of dereferenceable_or_null bytes, if the
/// dereferenceable_or_null attribute exists (zero is returned otherwise).
uint64_t getDereferenceableOrNullBytes() const {
return getRawIntAttr(Attribute::DereferenceableOrNull);
return getRawIntAttr(Attribute::DereferenceableOrNull).value_or(0);
}

/// Retrieve type for the given type attribute.
Expand Down
10 changes: 8 additions & 2 deletions llvm/lib/IR/AttributeImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class AttributeImpl : public FoldingSetNode {

void Profile(FoldingSetNodeID &ID) const {
if (isEnumAttribute())
Profile(ID, getKindAsEnum(), static_cast<uint64_t>(0));
Profile(ID, getKindAsEnum());
else if (isIntAttribute())
Profile(ID, getKindAsEnum(), getValueAsInt());
else if (isStringAttribute())
Expand All @@ -85,10 +85,16 @@ class AttributeImpl : public FoldingSetNode {
Profile(ID, getKindAsEnum(), getValueAsType());
}

static void Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind) {
assert(Attribute::isEnumAttrKind(Kind) && "Expected enum attribute");
ID.AddInteger(Kind);
}

static void Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind,
uint64_t Val) {
assert(Attribute::isIntAttrKind(Kind) && "Expected int attribute");
ID.AddInteger(Kind);
if (Val) ID.AddInteger(Val);
ID.AddInteger(Val);
}

static void Profile(FoldingSetNodeID &ID, StringRef Kind, StringRef Values) {
Expand Down
22 changes: 13 additions & 9 deletions llvm/lib/IR/Attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,23 +89,25 @@ unpackVScaleRangeArgs(uint64_t Value) {

Attribute Attribute::get(LLVMContext &Context, Attribute::AttrKind Kind,
uint64_t Val) {
if (Val)
assert(Attribute::isIntAttrKind(Kind) && "Not an int attribute");
else
assert(Attribute::isEnumAttrKind(Kind) && "Not an enum attribute");
bool IsIntAttr = Attribute::isIntAttrKind(Kind);
assert((IsIntAttr || Attribute::isEnumAttrKind(Kind)) &&
"Not an enum or int attribute");

LLVMContextImpl *pImpl = Context.pImpl;
FoldingSetNodeID ID;
ID.AddInteger(Kind);
if (Val) ID.AddInteger(Val);
if (IsIntAttr)
ID.AddInteger(Val);
else
assert(Val == 0 && "Value must be zero for enum attributes");

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.
if (!Val)
if (!IsIntAttr)
PA = new (pImpl->Alloc) EnumAttributeImpl(Kind);
else
PA = new (pImpl->Alloc) IntAttributeImpl(Kind, Val);
Expand Down Expand Up @@ -1638,10 +1640,12 @@ AttrBuilder &AttrBuilder::removeAttribute(StringRef A) {
return *this;
}

uint64_t AttrBuilder::getRawIntAttr(Attribute::AttrKind Kind) const {
Optional<uint64_t> AttrBuilder::getRawIntAttr(Attribute::AttrKind Kind) const {
assert(Attribute::isIntAttrKind(Kind) && "Not an int attribute");
Attribute A = getAttribute(Kind);
return A.isValid() ? A.getValueAsInt() : 0;
if (A.isValid())
return A.getValueAsInt();
return None;
}

AttrBuilder &AttrBuilder::addRawIntAttr(Attribute::AttrKind Kind,
Expand All @@ -1650,7 +1654,7 @@ AttrBuilder &AttrBuilder::addRawIntAttr(Attribute::AttrKind Kind,
}

std::pair<unsigned, Optional<unsigned>> AttrBuilder::getAllocSizeArgs() const {
return unpackAllocSizeArgs(getRawIntAttr(Attribute::AllocSize));
return unpackAllocSizeArgs(getRawIntAttr(Attribute::AllocSize).value_or(0));
}

AttrBuilder &AttrBuilder::addAlignmentAttr(MaybeAlign Align) {
Expand Down

0 comments on commit 2569767

Please sign in to comment.