When already have (op N0, N2), reassociate (op (op N0, N1), N2) to (op (op N0, N2), N1) to reuse the exist (op N0, N2)
Details
Diff Detail
Event Timeline
llvm/test/CodeGen/X86/masked-iv-safe.ll | ||
---|---|---|
269 ↗ | (On Diff #418434) | This seems regression. The same below. |
llvm/test/CodeGen/X86/masked-iv-safe.ll | ||
---|---|---|
269 ↗ | (On Diff #418434) | Yeah, this change brings some regressions because of two reasons:
But I think they should be fixed on every backend's isReassocProfitable. I prefer to fix these on individual changes. |
llvm/test/CodeGen/X86/masked-iv-safe.ll | ||
---|---|---|
269 ↗ | (On Diff #418434) | We shouldn't commit a patch with any known regressions. An individual fix is fine, but it should be ahead of this one so that we won't bring the regression at any time. |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
1072 | auto *LoadStore |
LGTM - with one more style minor
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
1047 | if (auto *LoadStore = dyn_cast<MemSDNode>(Node);) |
Hi @bcl5980, one of our internal tests started to hit an assertion failure while building which I bisected back to this change. I have filed the details as issue #56170, can you take a look?
This somehow caused a regression in the wasm targets, which I bisected to here. Certain files when compiled with -O1 appear to hang for a very long time (maybe forever). This happened on the Bullet physics engine codebase in the Emscripten test suite, but affects all wasm targets (wasm32-unknown-unknown, -wasi, and -emscripten).
This doesn't affect other targets, which I guess is why it wasn't noticed before it reached our CI. So maybe this uncovered an existing wasm bug.
@tlively is reducing this atm. Posting the original testcase here for future reference.
STR: clang++ -target wasm32-unknown-unknown -O1 -c bug.cpp. That should take less than a second normally, but from this change landing it hangs a very long time (I measured at least 10 seconds during bisection).
Thanks for the case. I try to minimize the test:
target triple = "wasm32-unknown-unknown" define float @reassociate_xor(float %x, float %y) { entry: ; preds = %if.then, %entry %cmp0 = fcmp ule float %x, 0x3E80000000000000 %cmp1 = fcmp ugt float %y, 0x3E80000000000000 %cmp2 = xor i1 %cmp0, %cmp1 br i1 %cmp2, label %if.end.i, label %if.then.i if.then.i: ; preds = %if.end br label %if.end.i if.end.i: ; preds = %if.then.i, %if.end %s = phi i32 [ 1, %entry ], [ 0, %if.then.i ] ret float %x }
It looks rebuildSetCC will visitXor in a loop keep the original dag before reassoicate cause deadloop.
I will continue work on it to find a solution.
llvm/test/Transforms/Reassociate/pr56170.ll | ||
---|---|---|
4 ↗ | (On Diff #439631) | Actually almost every target can trigger this issue. Move the test to x86 is OK for me. |