[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

dmgreen added a comment. I hadn't realized this came from someone at Arm. The performance results I had were overall roughly flat, with some improvements and regressions. I think there were still some people working through some fixes for some of the knock-on effects but with those nothing larg

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

craig.topper added a comment. After this patch, I'm seeing a lot of `invariant.gep` created by LICM. For example, in `LBM_performStreamCollide` in 470.lbm there are 65 of them. On RISC-V, these all get created in registers outside the loop and get spilled. Is ARM seeing anything like this or do

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

fiigii added a comment. That would be fine. Thanks for explaining. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added a comment. In D155688#4653520 , @fiigii wrote: >> The reverse transform is only done if A + B simplifies. > > Looks like`simplifyAddInst` may give add expressions, so I guess this patch > may make IC run into infinite loops. simplifyAddInst

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov added a comment. We have some improvements with the patch, most notable: 549.fotonik_3d improves about 6%. @nikic Should we revert the patch and try another location for it (in LICM pass, as you previously suggested)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

fiigii added a comment. > The reverse transform is only done if A + B simplifies. Looks like`simplifyAddInst` may give add expressions, so I guess this patch may make IC run into infinite loops. Additionally, this change could make longer GEP chains that could hurt other optimizations by excee

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added a comment. In D155688#4653347 , @fiigii wrote: > How does this patch work with `visitGEPOfGEP` that does a reverse > transformation? > > // Replace: gep (gep %P, long B), long A, ... > // With:T = long A+B; gep %P, T, ... The reverse

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

fiigii added a comment. How does this patch work with `visitGEPOfGEP` that does a reverse transformation? // Replace: gep (gep %P, long B), long A, ... // With:T = long A+B; gep %P, T, ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1556

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

This revision was automatically updated to reflect the committed changes. Closed by commit rGe13bed4c5f35: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP (authored by d-smirnov, committed by MatsPetersson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov marked an inline comment as done. d-smirnov added a comment. Updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 ___ cfe-commits mailing list cfe-

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov updated this revision to Diff 557609. d-smirnov added a comment. Updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 Files: clang/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp llvm/lib/

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM We should give this a try, but I think there is a fairly large chance that this will cause regressions somewhere and a more targeted change may be necessary (e.g. only do this for loop-inv

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov marked an inline comment as done. d-smirnov added a comment. @nikic Amended. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 ___ cfe-commits mailing l

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov updated this revision to Diff 557538. d-smirnov marked 2 inline comments as done. d-smirnov added a comment. amended Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 Files: clang/test/CodeGenCX

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov updated this revision to Diff 557537. d-smirnov added a comment. amended Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 Files: clang/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp llvm/lib/

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2338-2340 +if (BinaryOperator *Idx = +dyn_cast_or_null(GEP.getOperand(1))) + if ((Idx->getOpcode() == Instruction::Add) && Idx->hasOneUse()) {

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov added a comment. @nikic Updated. Please review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

d-smirnov updated this revision to Diff 557187. d-smirnov added a comment. comment updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 Files: clang/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp l