[AMDGPU] SelectionDAG divergence tracking should take into account Target divergency. #147560
Conversation
…teOperands changed." This reverts commit 1c2b4ce.
…ds change. One more chance.
…LIT test updated.
…eBits.IsDivergent initialization removed
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…d Release build. Formatting.
|
I think this is also causing crashes in ELD's ci which builds against tip of LLVM-project: https://github.com/qualcomm/eld/actions/runs/16155428606/job/45596726140 - the traces are pointing to:
I think this might need to be reverted? |
|
llc is crashing on the test cases from #146407 |
|
I think I see the problem. |
llvm#147560 changed when the legacy SelectionDAG pass needs TargetTransformInfoWrapperPass to always require it (rather than only when assertions are enabled). `SelectionDAGISelLegacy::getAnalysisUsage` was not updated in that PR, which was causing crashes on assertions-disabled builds, which are hard to track down. This makes the required update, which should avoid crashes being seen on some buildbots and by some users.
#147560 changed when the legacy SelectionDAG pass needs TargetTransformInfoWrapperPass to always require it (rather than only when assertions are enabled). `SelectionDAGISelLegacy::getAnalysisUsage` was not updated in that PR, which was causing crashes on assertions-disabled builds, which are hard to track down. This makes the required update, which should avoid crashes being seen on some buildbots and by some users.
llvm/llvm-project#147560 changed when the legacy SelectionDAG pass needs TargetTransformInfoWrapperPass to always require it (rather than only when assertions are enabled). `SelectionDAGISelLegacy::getAnalysisUsage` was not updated in that PR, which was causing crashes on assertions-disabled builds, which are hard to track down. This makes the required update, which should avoid crashes being seen on some buildbots and by some users.
|
@lenary , thank you so much for handling this! My apologies for inaccuracy :( |
|
No worries, debugging things is hard, especially when it depends on the kind of build (asserts vs none) |
|
This change is causing us issues downstream - we've had to revert it there. Does this need reverting here as well - it can break in the presence of wwm intrinsics. |
5 participants
This is the next attempt to upstream this: #144947
The las one caused build errors in AArch64.
Issue was resolved.