Skip to content

Add the 'initializes' attribute langref and support #84803

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

Merged
merged 39 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
2554c82
Add 'initialized' attribute langref
haopliu Mar 11, 2024
a9256bf
Update the LangRef
haopliu Mar 12, 2024
9150ece
Refactor the argument uses collecting to a function
haopliu Mar 12, 2024
206b707
Add 'initialized' attribute support and undo the FunctionAttrs refact…
haopliu Mar 14, 2024
eaf5ca0
Update the LangRef (round 2)
haopliu Mar 14, 2024
52cc649
Update FoldingSet.h
haopliu Mar 14, 2024
f7fddb0
Change the attribute type to ConstantRangeList
haopliu Apr 5, 2024
3b5955a
Rename attr to initializes
haopliu Apr 5, 2024
6a1df7a
Update CRL about accessing the range of an EmptySet/FullSet
haopliu Apr 9, 2024
fe2e0a5
Change CRL.insert to be ordered and no overlapping
haopliu Apr 12, 2024
f73df04
Optimize CRL::insert for common cases
haopliu Apr 16, 2024
63c45f6
Update ConstantRangeList
haopliu Apr 16, 2024
dcc2b38
Add unit tests for BitcodeReader/Verifier/Assembler
haopliu Apr 17, 2024
9723322
Add a space after commas
haopliu Apr 17, 2024
e70988d
Update CRL and unit tests
haopliu Apr 18, 2024
9782c31
Update CRL to merge consecutive ranges
haopliu Apr 23, 2024
a924dd3
Handle a common case in CRL::insert and add CRL::dump
haopliu Apr 23, 2024
45cdc34
Nit: change dbgs() to llvm::dbgs()
haopliu Apr 24, 2024
4eeba08
Change the attr impl to TrailingObjects<ConstantRange>
haopliu Apr 25, 2024
502e062
Update LLVMContextImpl.h
haopliu Apr 26, 2024
fac03b8
Merge branch 'llvm:main' into dse-commit
haopliu May 8, 2024
ea3e7c5
Rebase to the latest main
haopliu May 8, 2024
7630510
Change getInitializes() to return ArrayRef<ConstantRange>
haopliu May 8, 2024
a604b4f
Remove llvm::
haopliu May 8, 2024
f941b24
Update LangRef and ConstantRangeList
haopliu May 9, 2024
e52fd9d
Update BitCodeReader/Writer
haopliu May 9, 2024
8e64fa9
Encode bitwidth once for ConstantRangeList kind attr
haopliu May 9, 2024
9ae0fca
Use SpecificBumpPtrAllocator to allocate mem and make sure to call de…
haopliu May 16, 2024
d4809ab
Refactor readConstantRange and update BitcodeReader
haopliu May 17, 2024
99d36cc
Merge branch 'main' into dse-commit
haopliu May 28, 2024
96191cd
Change the attr imple back to ConstantRangeList
haopliu May 30, 2024
645f577
Add getConstantRangeList unittest
haopliu May 30, 2024
2a8ea9a
Add Bitwidth in FoldingSetNodeId
haopliu Jun 11, 2024
37edecd
Merge branch 'main' into dse-commit
haopliu Jun 11, 2024
80306d7
Clean up a redundant check in BitcodeReader
haopliu Jun 11, 2024
67445fe
Change attr impl to normal Alloc w/ a vector and call dtor for each a…
haopliu Jun 18, 2024
68643a5
nit: use uninitialized_copy in the ConstantRangeListAttributeImpl ctor
haopliu Jun 18, 2024
46e8d87
Remove redundant BitWidth in the attr Profile as APInt::Profile alrea…
haopliu Jun 20, 2024
714d02f
Use ConstantRange::contains() in ConstantRangeList::insert()
haopliu Jun 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions llvm/include/llvm/ADT/FoldingSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,21 @@ class FoldingSetNodeID {
}
}

void AddRanges(const SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use ArrayRef or something to not need specific SmallVector inline size of 16 here? "Prefer to use ArrayRef or SmallVectorImpl as a parameter type." under https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h

unsigned Size = Ranges.size();

unsigned NumsToInsert = 1 + 2 * Size;
Bits.reserve(Bits.size() + NumsToInsert);
Bits.push_back(Size);
if (Size == 0)
return;

for (const auto &[Start, End] : Ranges) {
Bits.push_back(Start);
Bits.push_back(End);
}
}

void AddBoolean(bool B) { AddInteger(B ? 1U : 0U); }
void AddString(StringRef String);
void AddNodeID(const FoldingSetNodeID &ID);
Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/AsmParser/LLParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ namespace llvm {
Loc = Lex.getLoc();
return parseUInt64(Val);
}
bool parseInt64(int64_t &Val);
bool parseFlag(unsigned &Val);

bool parseStringAttribute(AttrBuilder &B);
Expand Down Expand Up @@ -307,6 +308,10 @@ namespace llvm {
bool AllowParens = false);
bool parseOptionalCodeModel(CodeModel::Model &model);
bool parseOptionalDerefAttrBytes(lltok::Kind AttrKind, uint64_t &Bytes);
bool parseConstRange(std::pair<int64_t, int64_t> &Range);
bool parseInitializedRanges(
lltok::Kind AttrKind,
Copy link
Contributor

Choose a reason for hiding this comment

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

can the AttrKind be hardcoded within the function instead of taking as parameter? (unlike DerefAttrBytes, there is only one variation of the attribute, I believe?)

SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges);
bool parseOptionalUWTableKind(UWTableKind &Kind);
bool parseAllocKind(AllocFnKind &Kind);
std::optional<MemoryEffects> parseMemoryAttr();
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,7 @@ enum AttributeKindCodes {
ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE = 90,
ATTR_KIND_DEAD_ON_UNWIND = 91,
ATTR_KIND_RANGE = 92,
ATTR_KIND_INITIALIZED = 93,
};

enum ComdatSelectionKindCodes {
Expand Down
19 changes: 19 additions & 0 deletions llvm/include/llvm/IR/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class Attribute {

static const unsigned NumIntAttrKinds = LastIntAttr - FirstIntAttr + 1;
static const unsigned NumTypeAttrKinds = LastTypeAttr - FirstTypeAttr + 1;
static const unsigned NumConstRangeListAttrKinds =
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used by anything (if not, can remove)? I don't see the ConstantRangeAttrKind version of it, and hard to see "NumTypeAttrKinds" used

LastConstRangeListAttr - FirstConstRangeListAttr + 1;

static bool isEnumAttrKind(AttrKind Kind) {
return Kind >= FirstEnumAttr && Kind <= LastEnumAttr;
Expand All @@ -107,6 +109,9 @@ class Attribute {
static bool isConstantRangeAttrKind(AttrKind Kind) {
return Kind >= FirstConstantRangeAttr && Kind <= LastConstantRangeAttr;
}
static bool isConstRangeListAttrKind(AttrKind Kind) {
return Kind >= FirstConstRangeListAttr && Kind <= LastConstRangeListAttr;
}

static bool canUseAsFnAttr(AttrKind Kind);
static bool canUseAsParamAttr(AttrKind Kind);
Expand All @@ -131,6 +136,8 @@ class Attribute {
static Attribute get(LLVMContext &Context, AttrKind Kind, Type *Ty);
static Attribute get(LLVMContext &Context, AttrKind Kind,
const ConstantRange &CR);
static Attribute get(LLVMContext &Context, AttrKind Kind,
SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges);

/// Return a uniquified Attribute object that has the specific
/// alignment set.
Expand Down Expand Up @@ -186,6 +193,9 @@ class Attribute {
/// Return true if the attribute is a type attribute.
bool isTypeAttribute() const;

/// Return true if the attribute is a const range list attribute.
bool isConstRangeListAttribute() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: wonder if should s/Const/Constant/ to be consistent with isConstantRangeAttribute, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, no need to abbreviate


/// Return true if the attribute is a ConstantRange attribute.
bool isConstantRangeAttribute() const;

Expand Down Expand Up @@ -226,6 +236,10 @@ class Attribute {
/// attribute to be a ConstantRange attribute.
ConstantRange getValueAsConstantRange() const;

/// Return the attribute's value as a const range list. This requires the
/// attribute to be a const range list attribute.
SmallVector<std::pair<int64_t, int64_t>, 16> getValueAsRanges() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use ConstantRange instead of std::pair, especially since it already has intersection/union methods

Copy link
Contributor

Choose a reason for hiding this comment

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

also, 16 elements for the small vector optimization seems very high, I feel the vast majority of cases will be at most 4. I'd choose 4 (or even 2)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to return an ArrayRef instead of a copy? Will the underlying storage lifetime work out?


/// Returns the alignment field of an attribute as a byte alignment
/// value.
MaybeAlign getAlignment() const;
Expand Down Expand Up @@ -1169,6 +1183,11 @@ class AttrBuilder {
/// Add a type attribute with the given type.
AttrBuilder &addTypeAttr(Attribute::AttrKind Kind, Type *Ty);

/// Add a const range list attribute with the given ranges.
AttrBuilder &
addConstRangeListAttr(Attribute::AttrKind Kind,
SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges);

/// This turns a byval type into the form used internally in Attribute.
AttrBuilder &addByValAttr(Type *Ty);

Expand Down
6 changes: 6 additions & 0 deletions llvm/include/llvm/IR/Attributes.td
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class IntAttr<string S, list<AttrProperty> P> : Attr<S, P>;
/// Type attribute.
class TypeAttr<string S, list<AttrProperty> P> : Attr<S, P>;

/// Const range list attribute.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: put closer to the ConstantRangeAttr?

class ConstRangeListAttr<string S, list<AttrProperty> P>: Attr<S, P>;

/// StringBool attribute.
class StrBoolAttr<string S> : Attr<S, []>;

Expand Down Expand Up @@ -318,6 +321,9 @@ def Writable : EnumAttr<"writable", [ParamAttr]>;
/// Function only writes to memory.
def WriteOnly : EnumAttr<"writeonly", [ParamAttr]>;

/// Pointer argument memory [%p+LoN, %p+HiN) is initialized.
Copy link
Contributor

Choose a reason for hiding this comment

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

The list looks almost sorted, so may be better to put it near InAlloca or ?

def Initialized : ConstRangeListAttr<"initialized", [ParamAttr]>;

/// Zero extended before/after call.
def ZExt : EnumAttr<"zeroext", [ParamAttr, RetAttr]>;

Expand Down
65 changes: 65 additions & 0 deletions llvm/lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,13 @@ bool LLParser::parseEnumAttribute(Attribute::AttrKind Attr, AttrBuilder &B,
B.addDereferenceableOrNullAttr(Bytes);
return false;
}
case Attribute::Initialized: {
SmallVector<std::pair<int64_t, int64_t>, 16> Ranges;
if (parseInitializedRanges(lltok::kw_initialized, Ranges))
return true;
B.addConstRangeListAttr(Attribute::Initialized, Ranges);
return false;
}
case Attribute::UWTable: {
UWTableKind Kind;
if (parseOptionalUWTableKind(Kind))
Expand Down Expand Up @@ -1850,6 +1857,16 @@ bool LLParser::parseUInt64(uint64_t &Val) {
return false;
}

/// parseInt64
/// ::= int64_t
bool LLParser::parseInt64(int64_t &Val) {
if (Lex.getKind() != lltok::APSInt)
return tokError("expected signed integer");
Val = Lex.getAPSIntVal().extend(64).getSExtValue();
Lex.Lex();
return false;
}

/// parseTLSModel
/// := 'localdynamic'
/// := 'initialexec'
Expand Down Expand Up @@ -2364,6 +2381,54 @@ bool LLParser::parseOptionalDerefAttrBytes(lltok::Kind AttrKind,
return false;
}

bool LLParser::parseConstRange(std::pair<int64_t, int64_t> &Range) {
LocTy LparenLoc = Lex.getLoc();
if (!EatIfPresent(lltok::lparen))
return error(LparenLoc, "expected'('");

if (parseInt64(Range.first))
return true;

if (EatIfPresent(lltok::comma)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be an error to be missing the comma?

if (parseInt64(Range.second)) {
return true;
}
}

LocTy RparenLoc = Lex.getLoc();
if (!EatIfPresent(lltok::rparen))
return error(RparenLoc, "expected ')'");

return false;
}

bool LLParser::parseInitializedRanges(
lltok::Kind AttrKind,
SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges) {
assert(AttrKind == lltok::kw_initialized);
Ranges.clear();

if (!EatIfPresent(AttrKind))
return false;

LocTy LparenLoc = Lex.getLoc();
if (!EatIfPresent(lltok::lparen))
return error(LparenLoc, "expected '('");

// Parse each range.
do {
std::pair<int64_t, int64_t> Range;
if (parseConstRange(Range))
return true;
Ranges.push_back(Range);
} while (EatIfPresent(lltok::comma));

LocTy RparenLoc = Lex.getLoc();
if (!EatIfPresent(lltok::rparen))
return error(RparenLoc, "expected ')'");
return false;
}

bool LLParser::parseOptionalUWTableKind(UWTableKind &Kind) {
Lex.Lex();
Kind = UWTableKind::Default;
Expand Down
18 changes: 18 additions & 0 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
return Attribute::DeadOnUnwind;
case bitc::ATTR_KIND_RANGE:
return Attribute::Range;
case bitc::ATTR_KIND_INITIALIZED:
return Attribute::Initialized;
}
}

Expand Down Expand Up @@ -2313,6 +2315,22 @@ Error BitcodeReader::parseAttributeGroupBlock() {
i--;

B.addConstantRangeAttr(Kind, MaybeCR.get());
} else if (Record[i] == 8 || Record[i] == 9) {
Attribute::AttrKind Kind;
if (Error Err = parseAttrKind(Record[++i], &Kind))
return Err;
if (!Attribute::isConstRangeListAttrKind(Kind))
return error("Not a const range list attribute");

SmallVector<std::pair<int64_t, int64_t>, 16> Ranges;
int RangeSize = Record[++i];
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe assert i + (RangeSize * 2) < e
(please double check if I got that right)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

each loop iter up to RangeSize will bump i twice, so should it be * 2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, good catch! Done.

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
int RangeSize = Record[++i];
unsigned RangeSize = Record[++i];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

for (int Idx = 0; Idx < RangeSize; ++Idx) {
int Start = Record[++i];
int End = Record[++i];
Ranges.push_back(std::make_pair(Start, End));
}

B.addConstRangeListAttr(Kind, Ranges);
} else {
return error("Invalid attribute group entry");
}
Expand Down
17 changes: 16 additions & 1 deletion llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
return bitc::ATTR_KIND_DEAD_ON_UNWIND;
case Attribute::Range:
return bitc::ATTR_KIND_RANGE;
case Attribute::Initialized:
return bitc::ATTR_KIND_INITIALIZED;
case Attribute::EndAttrKinds:
llvm_unreachable("Can not encode end-attribute kinds marker.");
case Attribute::None:
Expand Down Expand Up @@ -930,11 +932,24 @@ void ModuleBitcodeWriter::writeAttributeGroupTable() {
Record.push_back(getAttrKindEncoding(Attr.getKindAsEnum()));
if (Ty)
Record.push_back(VE.getTypeID(Attr.getValueAsType()));
} else {
} else if (Attr.isConstantRangeAttribute()) {
assert(Attr.isConstantRangeAttribute());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can remove the assert?

Record.push_back(7);
Record.push_back(getAttrKindEncoding(Attr.getKindAsEnum()));
emitConstantRange(Record, Attr.getValueAsConstantRange());
} else {
assert(Attr.isConstRangeListAttribute());
const auto &Ranges = Attr.getValueAsRanges();

Record.push_back(Ranges.empty() ? 8 : 9);
Record.push_back(getAttrKindEncoding(Attr.getKindAsEnum()));
Record.push_back(Ranges.size());
if (!Ranges.empty()) {
for (const auto &Range : Ranges) {
Record.push_back(Range.first);
Record.push_back(Range.second);
}
}
}
}

Expand Down
31 changes: 30 additions & 1 deletion llvm/lib/IR/AttributeImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class AttributeImpl : public FoldingSetNode {
StringAttrEntry,
TypeAttrEntry,
ConstantRangeAttrEntry,
ConstRangeListAttrEntry,
};

AttributeImpl(AttrEntryKind KindID) : KindID(KindID) {}
Expand All @@ -64,6 +65,9 @@ class AttributeImpl : public FoldingSetNode {
bool isConstantRangeAttribute() const {
return KindID == ConstantRangeAttrEntry;
}
bool isConstRangeListAttribute() const {
return KindID == ConstRangeListAttrEntry;
}

bool hasAttribute(Attribute::AttrKind A) const;
bool hasAttribute(StringRef Kind) const;
Expand All @@ -79,6 +83,8 @@ class AttributeImpl : public FoldingSetNode {

ConstantRange getValueAsConstantRange() const;

SmallVector<std::pair<int64_t, int64_t>, 16> getValueAsRanges() const;

/// Used when sorting the attributes.
bool operator<(const AttributeImpl &AI) const;

Expand All @@ -91,8 +97,10 @@ class AttributeImpl : public FoldingSetNode {
Profile(ID, getKindAsString(), getValueAsString());
else if (isTypeAttribute())
Profile(ID, getKindAsEnum(), getValueAsType());
else
else if (isConstantRangeAttribute())
Profile(ID, getKindAsEnum(), getValueAsConstantRange());
else
Profile(ID, getKindAsEnum(), getValueAsRanges());
}

static void Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind) {
Expand Down Expand Up @@ -124,6 +132,13 @@ class AttributeImpl : public FoldingSetNode {
ID.AddInteger(CR.getLower());
ID.AddInteger(CR.getUpper());
}

static void
Profile(FoldingSetNodeID &ID, Attribute::AttrKind Kind,
const SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges) {
ID.AddInteger(Kind);
ID.AddRanges(Ranges);
}
};

static_assert(std::is_trivially_destructible<AttributeImpl>::value,
Expand Down Expand Up @@ -222,6 +237,20 @@ class ConstantRangeAttributeImpl : public EnumAttributeImpl {
ConstantRange getConstantRangeValue() const { return CR; }
};

class ConstRangeListAttributeImpl : public EnumAttributeImpl {
SmallVector<std::pair<int64_t, int64_t>, 16> Ranges;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something smaller than 16? What is the typical number of ranges?


public:
ConstRangeListAttributeImpl(
Attribute::AttrKind Kind,
SmallVector<std::pair<int64_t, int64_t>, 16> &Ranges)
: EnumAttributeImpl(ConstRangeListAttrEntry, Kind), Ranges(Ranges) {}

SmallVector<std::pair<int64_t, int64_t>, 16> getRangesValue() const {
return Ranges;
}
};

class AttributeBitSet {
/// Bitset with a bit for each available attribute Attribute::AttrKind.
uint8_t AvailableAttrs[12] = {};
Expand Down
Loading