[PATCH] D86183: [NFC][DAGCombine] Remove dead node when it is created by getNegatedExpression

[Qing Shan Zhang via Phabricator via llvm-commits]

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