-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[FunctionAttrs] Add the "initializes" attribute inference #97373
base: main
Are you sure you want to change the base?
Changes from 1 commit
56345c1
d4d49d3
07f73dd
cb144c0
9585684
0863cce
e9578c6
fa5df76
21debb5
b1841e1
08ef400
b82d3c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ | |||||
#include "llvm/Transforms/IPO/FunctionAttrs.h" | ||||||
#include "llvm/ADT/ArrayRef.h" | ||||||
#include "llvm/ADT/DenseMap.h" | ||||||
#include "llvm/ADT/PostOrderIterator.h" | ||||||
#include "llvm/ADT/SCCIterator.h" | ||||||
#include "llvm/ADT/STLExtras.h" | ||||||
#include "llvm/ADT/SetVector.h" | ||||||
|
@@ -36,6 +37,7 @@ | |||||
#include "llvm/IR/Attributes.h" | ||||||
#include "llvm/IR/BasicBlock.h" | ||||||
#include "llvm/IR/Constant.h" | ||||||
#include "llvm/IR/ConstantRangeList.h" | ||||||
#include "llvm/IR/Constants.h" | ||||||
#include "llvm/IR/Function.h" | ||||||
#include "llvm/IR/InstIterator.h" | ||||||
|
@@ -580,6 +582,205 @@ struct ArgumentUsesTracker : public CaptureTracker { | |||||
const SCCNodeSet &SCCNodes; | ||||||
}; | ||||||
|
||||||
struct ArgumentUse { | ||||||
Use *U; | ||||||
std::optional<int64_t> Offset; | ||||||
}; | ||||||
|
||||||
// A struct of argument access info. "Unknown" accesses are the cases like | ||||||
// unrecognized instructions, instructions that have more than one use of | ||||||
// the argument, or volatile memory accesses. "Unknown" implies "IsClobber" | ||||||
// and an empty access range. | ||||||
// Write or Read accesses can be clobbers as well for example, a Load with | ||||||
// scalable type. | ||||||
struct ArgumentAccessInfo { | ||||||
enum AccessType { Write, Read, Unknown }; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. enum class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
AccessType ArgAccessType; | ||||||
ConstantRangeList AccessRanges; | ||||||
bool IsClobber = false; | ||||||
jvoung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}; | ||||||
|
||||||
struct UsesPerBlockInfo { | ||||||
DenseMap<Instruction *, ArgumentAccessInfo> Insts; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How large is Insts typically? DenseMap jumps from 0 to 64: https://llvm.org/docs/ProgrammersManual.html#llvm-adt-densemap-h SmallDenseMap<..., 8> or something might help make it more gradual 0 -> 8 -> 64 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! According to an internal benchmark (muppet), 90% have a <= 2 size. Changed to SmallDenseMap<..., 2>. Similarly to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's <= 2, and a non-trivial amount are == 2, you may want to provide 3 inline buckets to avoid having to grow() when the 2nd entry is inserted. llvm-project/llvm/include/llvm/ADT/DenseMap.h Line 612 in c1f1aab
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good point! Changed the SmallDenseMap size here to 4 and the size of BTW, the size must be a power of 2 so choose 4 and 16. llvm-project/llvm/include/llvm/ADT/DenseMap.h Line 933 in 8f15909
|
||||||
bool HasWrites; | ||||||
bool HasClobber; | ||||||
}; | ||||||
|
||||||
ArgumentAccessInfo GetArgmentAccessInfo(const Instruction *I, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const ArgumentUse &IU, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Would it make more sense to name the variable "ArgUse"? Unclear to me why "I" in the name? Similar below "ArgumentUse IU = ..." at line 756 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, legacy names. Changed to "ArgUse". |
||||||
const DataLayout &DL) { | ||||||
auto GetTypeAccessRange = | ||||||
[&DL](Type *Ty, | ||||||
std::optional<int64_t> Offset) -> std::optional<ConstantRange> { | ||||||
auto TypeSize = DL.getTypeStoreSize(Ty); | ||||||
if (!TypeSize.isScalable() && Offset.has_value()) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in many if-statements, you write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Thanks! |
||||||
int64_t Size = TypeSize.getFixedValue(); | ||||||
return ConstantRange(APInt(64, Offset.value(), true), | ||||||
APInt(64, Offset.value() + Size, true)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To access the value of optionals, you often write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
} | ||||||
return std::nullopt; | ||||||
}; | ||||||
auto GetConstantIntRange = | ||||||
[](Value *Length, | ||||||
std::optional<int64_t> Offset) -> std::optional<ConstantRange> { | ||||||
auto *ConstantLength = dyn_cast<ConstantInt>(Length); | ||||||
if (ConstantLength && Offset.has_value()) { | ||||||
return ConstantRange( | ||||||
APInt(64, Offset.value(), true), | ||||||
APInt(64, Offset.value() + ConstantLength->getSExtValue(), true)); | ||||||
} | ||||||
return std::nullopt; | ||||||
}; | ||||||
if (auto *SI = dyn_cast<StoreInst>(I)) { | ||||||
aeubanks marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (&SI->getOperandUse(1) == IU.U) { | ||||||
// Get the fixed type size of "SI". Since the access range of a write | ||||||
// will be unioned, if "SI" doesn't have a fixed type size, we just set | ||||||
// the access range to empty. | ||||||
ConstantRangeList AccessRanges; | ||||||
auto TypeAccessRange = GetTypeAccessRange(SI->getAccessType(), IU.Offset); | ||||||
if (TypeAccessRange.has_value()) | ||||||
AccessRanges.insert(TypeAccessRange.value()); | ||||||
return {ArgumentAccessInfo::AccessType::Write, AccessRanges, | ||||||
/*IsClobber=*/false}; | ||||||
} | ||||||
} else if (auto *LI = dyn_cast<LoadInst>(I)) { | ||||||
if (&LI->getOperandUse(0) == IU.U) { | ||||||
// Get the fixed type size of "LI". Different from Write, if "LI" | ||||||
// doesn't have a fixed type size, we conservatively set as a clobber | ||||||
// with an empty access range. | ||||||
auto TypeAccessRange = GetTypeAccessRange(LI->getAccessType(), IU.Offset); | ||||||
if (TypeAccessRange.has_value()) | ||||||
return {ArgumentAccessInfo::AccessType::Read, | ||||||
{TypeAccessRange.value()}, | ||||||
/*IsClobber=*/false}; | ||||||
else | ||||||
return {ArgumentAccessInfo::AccessType::Read, {}, /*IsClobber=*/true}; | ||||||
} | ||||||
} else if (auto *MemSet = dyn_cast<MemSetInst>(I)) { | ||||||
if (!MemSet->isVolatile()) { | ||||||
ConstantRangeList AccessRanges; | ||||||
auto AccessRange = GetConstantIntRange(MemSet->getLength(), IU.Offset); | ||||||
if (AccessRange.has_value()) | ||||||
AccessRanges.insert(AccessRange.value()); | ||||||
return {ArgumentAccessInfo::AccessType::Write, AccessRanges, | ||||||
/*IsClobber=*/false}; | ||||||
} | ||||||
} else if (auto *MemCpy = dyn_cast<MemCpyInst>(I)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be MemTransferInst to cover memmove too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point! Changed to MemTransferInst and added unit test for memmove as well. |
||||||
if (!MemCpy->isVolatile()) { | ||||||
if (&MemCpy->getOperandUse(0) == IU.U) { | ||||||
ConstantRangeList AccessRanges; | ||||||
auto AccessRange = GetConstantIntRange(MemCpy->getLength(), IU.Offset); | ||||||
if (AccessRange.has_value()) | ||||||
AccessRanges.insert(AccessRange.value()); | ||||||
return {ArgumentAccessInfo::AccessType::Write, AccessRanges, | ||||||
/*IsClobber=*/false}; | ||||||
} else if (&MemCpy->getOperandUse(1) == IU.U) { | ||||||
auto AccessRange = GetConstantIntRange(MemCpy->getLength(), IU.Offset); | ||||||
if (AccessRange.has_value()) | ||||||
return {ArgumentAccessInfo::AccessType::Read, | ||||||
{AccessRange.value()}, | ||||||
/*IsClobber=*/false}; | ||||||
else | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No else after return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
return {ArgumentAccessInfo::AccessType::Read, {}, /*IsClobber=*/true}; | ||||||
} | ||||||
} | ||||||
} else if (auto *CB = dyn_cast<CallBase>(I)) { | ||||||
if (CB->isArgOperand(IU.U)) { | ||||||
unsigned ArgNo = CB->getArgOperandNo(IU.U); | ||||||
bool IsInitialize = CB->paramHasAttr(ArgNo, Attribute::Initializes); | ||||||
// Argument is only not clobbered when parameter is writeonly/readnone | ||||||
// and nocapture. | ||||||
bool IsClobber = !(CB->onlyWritesMemory(ArgNo) && | ||||||
CB->paramHasAttr(ArgNo, Attribute::NoCapture)); | ||||||
ConstantRangeList AccessRanges; | ||||||
if (IsInitialize && IU.Offset.has_value()) { | ||||||
Attribute Attr = CB->getParamAttr(ArgNo, Attribute::Initializes); | ||||||
if (!Attr.isValid()) { | ||||||
Attr = CB->getCalledFunction()->getParamAttribute( | ||||||
ArgNo, Attribute::Initializes); | ||||||
} | ||||||
ConstantRangeList CBCRL = Attr.getValueAsConstantRangeList(); | ||||||
for (ConstantRange &CR : CBCRL) { | ||||||
AccessRanges.insert(ConstantRange(CR.getLower() + IU.Offset.value(), | ||||||
CR.getUpper() + IU.Offset.value())); | ||||||
} | ||||||
return {ArgumentAccessInfo::AccessType::Write, AccessRanges, IsClobber}; | ||||||
} | ||||||
} | ||||||
} | ||||||
// Unrecognized instructions are considered clobbers. | ||||||
return {ArgumentAccessInfo::AccessType::Unknown, {}, /*IsClobber=*/true}; | ||||||
} | ||||||
|
||||||
std::pair<bool, bool> CollectArgumentUsesPerBlock( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add some doc for the function? What does the return value represent? UsesPerBlock updated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this return |
||||||
Argument &A, Function &F, | ||||||
DenseMap<const BasicBlock *, UsesPerBlockInfo> &UsesPerBlock) { | ||||||
auto &DL = F.getParent()->getDataLayout(); | ||||||
auto PointerSize = | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
DL.getIndexSizeInBits(A.getType()->getPointerAddressSpace()); | ||||||
|
||||||
bool HasAnyWrite = false; | ||||||
bool HasWriteOutsideEntryBB = false; | ||||||
|
||||||
BasicBlock &EntryBB = F.getEntryBlock(); | ||||||
SmallVector<ArgumentUse, 4> Worklist; | ||||||
for (Use &U : A.uses()) | ||||||
Worklist.push_back({&U, 0}); | ||||||
|
||||||
auto UpdateUseInfo = [&UsesPerBlock](Instruction *I, | ||||||
jvoung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
ArgumentAccessInfo Info) { | ||||||
auto *BB = I->getParent(); | ||||||
auto &BBInfo = UsesPerBlock.getOrInsertDefault(BB); | ||||||
jvoung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
bool AlreadyVisitedInst = BBInfo.Insts.contains(I); | ||||||
auto &IInfo = BBInfo.Insts[I]; | ||||||
|
||||||
// Instructions that have more than one use of the argument are considered | ||||||
// as clobbers. | ||||||
if (AlreadyVisitedInst) { | ||||||
IInfo = {ArgumentAccessInfo::AccessType::Unknown, {}, true}; | ||||||
BBInfo.HasClobber = true; | ||||||
return false; | ||||||
} | ||||||
|
||||||
IInfo = Info; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
BBInfo.HasClobber |= IInfo.IsClobber; | ||||||
BBInfo.HasWrites |= | ||||||
(IInfo.ArgAccessType == ArgumentAccessInfo::AccessType::Write && | ||||||
!IInfo.AccessRanges.empty()); | ||||||
return !IInfo.AccessRanges.empty(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the return value mean "HasWrite" or is it more subtle than that? If it means "HasWrite", should it match the "BBInfo.HasWrites " above in that it also checks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! Updated. |
||||||
}; | ||||||
|
||||||
// No need for a visited set because we don't look through phis, so there are | ||||||
// no cycles. | ||||||
while (!Worklist.empty()) { | ||||||
ArgumentUse IU = Worklist.pop_back_val(); | ||||||
User *U = IU.U->getUser(); | ||||||
// Add GEP uses to worklist. | ||||||
// If the GEP is not a constant GEP, set IsInitialize to false. | ||||||
jvoung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (auto *GEP = dyn_cast<GEPOperator>(U)) { | ||||||
APInt Offset(PointerSize, 0, /*isSigned=*/true); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The flag is not meaningful for a zero value. |
||||||
bool IsConstGEP = GEP->accumulateConstantOffset(DL, Offset); | ||||||
std::optional<int64_t> NewOffset = std::nullopt; | ||||||
if (IsConstGEP && IU.Offset.has_value()) { | ||||||
NewOffset = *IU.Offset + Offset.getSExtValue(); | ||||||
} | ||||||
for (Use &U : GEP->uses()) | ||||||
Worklist.push_back({&U, NewOffset}); | ||||||
continue; | ||||||
} | ||||||
|
||||||
auto *I = cast<Instruction>(U); | ||||||
bool HasWrite = UpdateUseInfo(I, GetArgmentAccessInfo(I, IU, DL)); | ||||||
|
||||||
HasAnyWrite |= HasWrite; | ||||||
|
||||||
if (HasWrite && I->getParent() != &EntryBB) { | ||||||
HasWriteOutsideEntryBB = true; | ||||||
} | ||||||
} | ||||||
return {HasAnyWrite, HasWriteOutsideEntryBB}; | ||||||
} | ||||||
|
||||||
} // end anonymous namespace | ||||||
|
||||||
namespace llvm { | ||||||
|
@@ -866,9 +1067,132 @@ static bool addAccessAttr(Argument *A, Attribute::AttrKind R) { | |||||
return true; | ||||||
} | ||||||
|
||||||
static bool inferInitializes(Argument &A, Function &F) { | ||||||
DenseMap<const BasicBlock *, UsesPerBlockInfo> UsesPerBlock; | ||||||
auto [HasAnyWrite, HasWriteOutsideEntryBB] = | ||||||
CollectArgumentUsesPerBlock(A, F, UsesPerBlock); | ||||||
// No write anywhere in the function, bail. | ||||||
if (!HasAnyWrite) | ||||||
return false; | ||||||
|
||||||
BasicBlock &EntryBB = F.getEntryBlock(); | ||||||
DenseMap<const BasicBlock *, ConstantRangeList> Initialized; | ||||||
auto VisitBlock = [&](const BasicBlock *BB) -> ConstantRangeList { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add a comment on what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done and found a corner case :-) "If this block has uses and none are writes, the argument is not initialized in this block." |
||||||
auto UPB = UsesPerBlock.find(BB); | ||||||
|
||||||
// If this block has uses and none are writes, the argument is not | ||||||
// initialized in this block. | ||||||
if (UPB != UsesPerBlock.end() && !UPB->second.HasWrites) | ||||||
return ConstantRangeList(); | ||||||
|
||||||
ConstantRangeList CRL; | ||||||
|
||||||
// Start with intersection of successors. | ||||||
// If this block has any clobbering use, we're going to clear out the | ||||||
// ranges at some point in this block anyway, so don't bother looking at | ||||||
// successors. | ||||||
if (UPB == UsesPerBlock.end() || !UPB->second.HasClobber) { | ||||||
bool HasAddedSuccessor = false; | ||||||
for (auto *Succ : successors(BB)) { | ||||||
if (auto SuccI = Initialized.find(Succ); SuccI != Initialized.end()) { | ||||||
if (HasAddedSuccessor) { | ||||||
CRL = CRL.intersectWith(SuccI->second); | ||||||
} else { | ||||||
CRL = SuccI->second; | ||||||
HasAddedSuccessor = true; | ||||||
} | ||||||
} else { | ||||||
CRL = ConstantRangeList(); | ||||||
break; | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
if (UPB != UsesPerBlock.end()) { | ||||||
// Sort uses in this block by instruction order. | ||||||
SmallVector<std::pair<Instruction *, ArgumentAccessInfo>, 2> Insts; | ||||||
append_range(Insts, UPB->second.Insts); | ||||||
sort(Insts, [](std::pair<Instruction *, ArgumentAccessInfo> &LHS, | ||||||
std::pair<Instruction *, ArgumentAccessInfo> &RHS) { | ||||||
return LHS.first->comesBefore(RHS.first); | ||||||
}); | ||||||
|
||||||
// From the end of the block to the beginning of the block, set | ||||||
// initializes ranges. | ||||||
for (auto [_, Info] : reverse(Insts)) { | ||||||
jvoung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (Info.IsClobber) { | ||||||
CRL = ConstantRangeList(); | ||||||
} | ||||||
if (!Info.AccessRanges.empty()) { | ||||||
if (Info.ArgAccessType == ArgumentAccessInfo::AccessType::Write) { | ||||||
CRL = CRL.unionWith(Info.AccessRanges); | ||||||
} else { | ||||||
assert(Info.ArgAccessType == ArgumentAccessInfo::AccessType::Read); | ||||||
for (const auto &ReadRange : Info.AccessRanges) { | ||||||
CRL.subtract(ReadRange); | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
return CRL; | ||||||
}; | ||||||
|
||||||
ConstantRangeList EntryCRL; | ||||||
// If all write instructions are in the EntryBB, or if the EntryBB has | ||||||
// a clobbering use, we only need to look at EntryBB. | ||||||
bool OnlyScanEntryBlock = !HasWriteOutsideEntryBB; | ||||||
if (!OnlyScanEntryBlock) { | ||||||
if (auto EntryUPB = UsesPerBlock.find(&EntryBB); | ||||||
EntryUPB != UsesPerBlock.end()) { | ||||||
OnlyScanEntryBlock = EntryUPB->second.HasClobber; | ||||||
} | ||||||
} | ||||||
if (OnlyScanEntryBlock) { | ||||||
EntryCRL = VisitBlock(&EntryBB); | ||||||
if (EntryCRL.empty()) { | ||||||
return false; | ||||||
} | ||||||
} else { | ||||||
// Visit successors before predecessors with a post-order walk of the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you expand the comment to include the reasoning behind this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
// blocks. | ||||||
for (const BasicBlock *BB : post_order(&F)) { | ||||||
ConstantRangeList CRL = VisitBlock(BB); | ||||||
if (!CRL.empty()) { | ||||||
Initialized[BB] = CRL; | ||||||
} | ||||||
} | ||||||
|
||||||
auto EntryCRLI = Initialized.find(&EntryBB); | ||||||
if (EntryCRLI == Initialized.end()) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you cleanup the instances of https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, thanks for reminding! Done. |
||||||
return false; | ||||||
} | ||||||
|
||||||
EntryCRL = EntryCRLI->second; | ||||||
} | ||||||
|
||||||
assert(!EntryCRL.empty() && | ||||||
"should have bailed already if EntryCRL is empty"); | ||||||
|
||||||
if (A.hasAttribute(Attribute::Initializes)) { | ||||||
ConstantRangeList PreviousCRL = | ||||||
A.getAttribute(Attribute::Initializes).getValueAsConstantRangeList(); | ||||||
if (PreviousCRL == EntryCRL) { | ||||||
return false; | ||||||
} | ||||||
EntryCRL = EntryCRL.unionWith(PreviousCRL); | ||||||
} | ||||||
|
||||||
A.addAttr(Attribute::get(A.getContext(), Attribute::Initializes, | ||||||
EntryCRL.rangesRef())); | ||||||
|
||||||
return true; | ||||||
} | ||||||
|
||||||
/// Deduce nocapture attributes for the SCC. | ||||||
static void addArgumentAttrs(const SCCNodeSet &SCCNodes, | ||||||
SmallSet<Function *, 8> &Changed) { | ||||||
SmallSet<Function *, 8> &Changed, | ||||||
bool SkipInitializes) { | ||||||
ArgumentGraph AG; | ||||||
|
||||||
// Check each function in turn, determining which pointer arguments are not | ||||||
|
@@ -936,6 +1260,10 @@ static void addArgumentAttrs(const SCCNodeSet &SCCNodes, | |||||
if (addAccessAttr(&A, R)) | ||||||
Changed.insert(F); | ||||||
} | ||||||
if (!SkipInitializes && !A.onlyReadsMemory()) { | ||||||
if (inferInitializes(A, *F)) | ||||||
Changed.insert(F); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -1844,13 +2172,13 @@ deriveAttrsInPostOrder(ArrayRef<Function *> Functions, AARGetterT &&AARGetter, | |||||
|
||||||
SmallSet<Function *, 8> Changed; | ||||||
if (ArgAttrsOnly) { | ||||||
addArgumentAttrs(Nodes.SCCNodes, Changed); | ||||||
addArgumentAttrs(Nodes.SCCNodes, Changed, /*SkipInitializes=*/true); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment as to why SkipInitializes in the ArgAttrsOnly case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done :-D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would simplify the comment a bit, this is too much detail. The important thing is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
return Changed; | ||||||
} | ||||||
|
||||||
addArgumentReturnedAttrs(Nodes.SCCNodes, Changed); | ||||||
addMemoryAttrs(Nodes.SCCNodes, AARGetter, Changed); | ||||||
addArgumentAttrs(Nodes.SCCNodes, Changed); | ||||||
addArgumentAttrs(Nodes.SCCNodes, Changed, /*SkipInitializes=*/false); | ||||||
inferConvergent(Nodes.SCCNodes, Changed); | ||||||
addNoReturnAttrs(Nodes.SCCNodes, Changed); | ||||||
addWillReturn(Nodes.SCCNodes, Changed); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reminding! Done.