Modifying other AbstractAttributes to use Liveness AA and skip dead instructions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 35703 Build 35702: arc lint + arc unit
Event Timeline
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
676 | SmallPtrSetImpl<ReturnInst *> & | |
llvm/lib/Transforms/IPO/Attributor.cpp | ||
658 | Do not modify UniqueRV and do return true to indicate this return value was handled and all is OK. The idea is, when dead then ignore the return value. | |
681 | & missing | |
724 | Move into a helper function. |
- adding instruction-wise is Assumed/Known Dead checks. This resolves FIXME in TEST 2.
- addressing comments
It should be noted that there is still a problem in AAReturnedValues. I will have to do a debug build, which will take quiet a lot of time, to investigate further.
some comments, no major problem though.
llvm/lib/Transforms/IPO/Attributor.cpp | ||
---|---|---|
582 | Maybe move this as a member into AAIsDead with the argument beeing a ArrayRef<Instruction *> so we can reuse it | |
592 | return true; | |
761 | I'll commit these changes as a fix today. | |
1245 | Let's move this logic into the checkForAllCallSites call so all users benefit. | |
1577 | please add an assert (also above) to verify I (or BB) is in the AnchoredScope. | |
1601 | return getKnown() && isAssumedDead(I) to reduce duplication of logic. | |
1713 | // At least one new instruction encountered. | |
llvm/test/Transforms/FunctionAttrs/arg_returned.ll | ||
751 ↗ | (On Diff #212298) | Please keep these and add the additional attribute sets instead. |
Almost there. I added a last set of comments, most minor.
Could you add one more test case:
internal function foo, called 2 times but one call site is dead. We want to derive something about foo from the call site.
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
866 | Thanks for this generic version. Please add doxygen documentation as well. | |
llvm/lib/Transforms/IPO/Attributor.cpp | ||
648 | If you want LivenessAA might not be available, check for null. | |
724 | Test for nullptr. | |
983 | Comments here and elsewhere say "blocks" but check instructions. | |
1264 | Pass this as reference (as we always do in the Attributor). | |
1762 | Nit: CS->getFunction() or CS.getInstruction()->getFunction() | |
1766 | not return true but continue! Only this call site is OK if it is dead. | |
llvm/test/Transforms/FunctionAttrs/liveness.ll | ||
64 | remove the comment. |
One minor thing and one change request + test. Other than that, LGTM.
llvm/lib/Transforms/IPO/Attributor.cpp | ||
---|---|---|
1264 | Please pass this as a reference not pointer. | |
1768 | Move the liveness stuff before the CallSite CS stuff. If the user is dead, it does not matter if it is a call site, a direct call site, etc., it is dead after all. Also add a test for this please, e.g., pass the function pointer to a another unknown function or store it away but the instruction that does so is dead. |
SmallPtrSetImpl<ReturnInst *> &