[SelectionDAG] Fix the assertion failure in Release build after #91747 #93416
Conversation
…91747 In llvm#91747, we change the SDNode from `X86ISD::SUB` (FROM) to `X86ISD::CCMP` (TO) in the DAGCombine. The value type of X86ISD::SUB can be `i8, i32` while the value type of X86ISD::CCMP is `i32`. That means the `SDValue(FROM, 0)` is unused and may be removed. However, `transferDbgValues` assumes the value is not null, which is called by `ReplaceAllUsesWith(SDNode *, const SDValue *)`. So we need to check if the value has any use before calling the function. Note: We already have same check in `ReplaceAllUsesWith(SDNode *, SDNode *)`. This fix the error ``` SelectionDAG.cpp:10942: void llvm::SelectionDAG::transferDbgValues(llvm::SDValue, llvm::SDValue, unsigned int, unsigned int, bool): Assertion `FromNode && ToNode && "Can't modify dbg values"' failed. ``` for tests llvm/test/CodeGen/X86/apx/ccmp.ll llvm/test/CodeGen/X86/apx/ctest.ll in Release build when LLVM_ENABLE_ASSERTIONS is on.
|
@llvm/pr-subscribers-llvm-selectiondag Author: Shengchen Kan (KanRobert) ChangesIn #91747, we change the SDNode from That means the Note: We already have same check in This fix the error for tests llvm/test/CodeGen/X86/apx/ccmp.ll in Release build when LLVM_ENABLE_ASSERTIONS is on. Full diff: https://github.com/llvm/llvm-project/pull/93416.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index b05649c6ce955..f21b17102bde5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -11293,10 +11293,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, const SDValue *To) {
return ReplaceAllUsesWith(SDValue(From, 0), To[0]);
for (unsigned i = 0, e = From->getNumValues(); i != e; ++i) {
- // Preserve Debug Info.
- transferDbgValues(SDValue(From, i), To[i]);
- // Preserve extra info.
- copyExtraInfo(From, To[i].getNode());
+ if (From->hasAnyUseOfValue(i)) {
+ // Preserve Debug Info.
+ transferDbgValues(SDValue(From, i), To[i]);
+ // Preserve extra info.
+ copyExtraInfo(From, To[i].getNode());
+ }
}
// Iterate over just the existing users of From. See the comments in
|
| // Preserve Debug Info. | ||
| transferDbgValues(SDValue(From, i), To[i]); | ||
| // Preserve extra info. | ||
| copyExtraInfo(From, To[i].getNode()); |
There was a problem hiding this comment.
It looks like you're reading uninitialized memory here. The number of results must match exactly, which is not true when you're replacing (i8, i32) with just (i32).
|
See the new solution #93434 |
In #91747, we changed the SDNode from
X86ISD::SUB(FROM) toX86ISD::CCMP(TO) in the DAGCombine. The value type of X86ISD::SUB can bei8, i32while the value type of X86ISD::CCMP isi32.That means the
SDValue(FROM, 0)is unused and may be removed. However,transferDbgValuesassumes the value is not null, which is called byReplaceAllUsesWith(SDNode *, const SDValue *). So we need to check if the value has any use before calling the function.Note: We already have same check in
ReplaceAllUsesWith(SDNode *, SDNode *).This fix the error
for tests
llvm/test/CodeGen/X86/apx/ccmp.ll
llvm/test/CodeGen/X86/apx/ctest.ll
in Release build when LLVM_ENABLE_ASSERTIONS is on.