The module slice describes which functions we can analyze and transform
while working on an SCC as part of the Attributor-CGSCC pass. So far we
simply restricted it to the SCC.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I went to some tests and left some comments. tomorrow we go through the rest.
llvm/test/Transforms/Attributor/ArgumentPromotion/X86/attributes.ll | ||
---|---|---|
1 | Please pre-commit the additional flags and updates to the tests as NFC (no review) | |
llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll | ||
19 | This is the expected result, the CGSCC and module pass are producing the same in NPM and OPM respectively. | |
122 | It's really important that argument privatization works now in the CGSCC pass! yay! | |
llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll | ||
92–95 | This looks wrong though. Need to verify. |
This is interesting, I thought you couldn't change the IR outside of the current SCC.
Was this intended to go on top of D86081 ?
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
765 | I don't think this is needed. If you just seed the SCC, the AAs won't query anything outside of this anyways. | |
1085–1090 | Documentation | |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
3035–3042 | I think this to not spend too much time on functions that are outside of the current SCC right ? |
We can't delete functions outside of the SCC, the rest should be fine.
Was this intended to go on top of D86081 ?
At first yes. However we need to have manifest in module slice since we need to manifest stuff (if any) and we don't allow deleting functions outside of the SCC. You will probably need to make that allow manifest only on the module slice (but probably unlikely it will ever happen outside of it).
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
765 | While that is right, module slice won't be used by the attributor exclusively, that's why it is in the InformationCache. We'll use this in the OpenMPOpt as well. | |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
3035–3042 | Yes, we don't want to compute it twice. |
Rebasing after recent changes to AANoUndef
llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll | ||
---|---|---|
92–95 | Thanks again for taking a look! |
LGTM, one nit
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
1092–1095 | make this a proper query method, isInModuleSlice or something. |
I don't think this is needed. If you just seed the SCC, the AAs won't query anything outside of this anyways.