-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[FunctionAttrs] Add the "initializes" attribute inference #97373
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
Changes from 10 commits
56345c1
d4d49d3
07f73dd
cb144c0
9585684
0863cce
e9578c6
fa5df76
21debb5
b1841e1
08ef400
b82d3c4
eb2d9cf
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,206 @@ struct ArgumentUsesTracker : public CaptureTracker { | |||||
| const SCCNodeSet &SCCNodes; | ||||||
| }; | ||||||
|
|
||||||
| // A struct of argument use: a Use and the offset it accesses. This struct | ||||||
| // is to track uses inside function via GEP. If GEP has a non-constant index, | ||||||
| // the Offset field is nullopt. | ||||||
| struct ArgumentUse { | ||||||
|
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. missing documentation.
Contributor
Author
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. Thanks for reminding! Done. |
||||||
| 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 class AccessType : uint8_t { Write, Read, Unknown }; | ||||||
| AccessType ArgAccessType; | ||||||
| bool IsClobber = false; | ||||||
jvoung marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| ConstantRangeList AccessRanges; | ||||||
| }; | ||||||
|
|
||||||
| // A struct to wrap the argument use info per block. | ||||||
| struct UsesPerBlockInfo { | ||||||
| SmallDenseMap<Instruction *, ArgumentAccessInfo, 4> Insts; | ||||||
| bool HasWrites = false; | ||||||
| bool HasClobber = false; | ||||||
| }; | ||||||
|
|
||||||
| // A struct to summarize the argument use info in a function. | ||||||
| struct ArgumentUsesSummary { | ||||||
| bool HasAnyWrite = false; | ||||||
| bool HasWriteOutsideEntryBB = false; | ||||||
| SmallDenseMap<const BasicBlock *, UsesPerBlockInfo, 16> UsesPerBlock; | ||||||
| }; | ||||||
|
|
||||||
| ArgumentAccessInfo GetArgmentAccessInfo(const Instruction *I, | ||||||
|
||||||
| ArgumentAccessInfo GetArgmentAccessInfo(const Instruction *I, | |
| ArgumentAccessInfo getArgmentAccessInfo(const Instruction *I, |
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.
Done.
aeubanks marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
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.
These isVolatile() checks should probably be isSimple() checks? Presumably you don't want to handle atomics.
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.
Ah, just noticed isSimple(). Replaced with isSimple() checks.
Outdated
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.
| /*IsClobber=*/false, AccessRanges}; | |
| /*IsClobber=*/false, std::move(AccessRanges)}; |
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.
Done.
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.
we should add a test where the argument is passed as an operand bundle to a call, rather than an argument
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.
Done!
Outdated
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.
| ArgumentUsesSummary CollectArgumentUsesPerBlock(Argument &A, Function &F) { | |
| ArgumentUsesSummary collectArgumentUsesPerBlock(Argument &A, Function &F) { |
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.
Done.
Outdated
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.
| auto PointerSize = | |
| unsigned PointerSize = |
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.
Done.
Outdated
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.
| IInfo = Info; | |
| IInfo = std::move(Info); |
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.
Done.
Outdated
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.
| APInt Offset(PointerSize, 0, /*isSigned=*/true); | |
| APInt Offset(PointerSize, 0); |
The flag is not meaningful for a zero value.
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, done.
Outdated
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.
If ArgUse.Offset is already nullopt, you can skip the accumulateConstantOffset() call.
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.
Good point! Refactored code to check if ArgUse.Offset is nullopt first.
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.
can you add a comment on what VisitBlock does
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.
Done and found a corner case :-)
"If this block has uses and none are writes, the argument is not initialized in this block."
Removed this early return. See the test.
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.
Please use doxygen comments (
///) for classes.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.
Done.