[PATCH] D86183: [NFC][DAGCombine] Remove dead node when it is created by getNegatedExpression
Qing Shan Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 18:48:13 PDT 2020
steven.zhang added a comment.
In D86183#2227988 <https://reviews.llvm.org/D86183#2227988>, @qiucf wrote:
> The original test case is too big to get something useful. I happened to grab a basic block from it. It's not small enough to be a test, but it would be much easier to analyze: https://reviews.llvm.org/P8230
Thank you for doing this! The test is small enough to demonstrate the problem I think.
> The test shows that we are not actually infinite looping; we are exponentially increasing compile time with each additional set of FMA combines.
> If I limit the live code in the test by changing the final store from "store float %288, float* %80, align 4" and time it:
> store float %233, float* %80, align 4 --> "time llc" --> user 0m30.640s
> store float %229, float* %80, align 4 --> "time llc" --> user 0m19.956s
> store float %224, float* %80, align 4 --> "time llc" --> user 0m10.881s
> store float %217, float* %80, align 4 --> "time llc" --> user 0m5.750s
> I don't know if that changes the solution or the potential test. Maybe we could add or check some debug statement to track the number of nodes created?
Good catch. And it also explains more clear on why the fix works as we are removing the unnecessary dead nodes we created that increase the problem size of the DAGCombine. As the final DAG is the same w/o the fix and I am worried the test is easy to be broken if we check the temp node we created, I tend to have chaofang's test to verify we are not increasing the compiling time. Does it make sense ?
And thank you for all your help on the tests!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86183/new/
https://reviews.llvm.org/D86183
More information about the llvm-commits
mailing list