[llvm] eeb2f72 - [SelectionDAG][X86] Fix the assertion failure in Release build after #91747 (#93434)

[via llvm-commits]

Author: Shengchen Kan
Date: 2024-05-27T11:33:23+08:00
New Revision: eeb2f72a493368d892653203feec03ce484bf17c

URL: https://github.com/llvm/llvm-project/commit/eeb2f72a493368d892653203feec03ce484bf17c
DIFF: https://github.com/llvm/llvm-project/commit/eeb2f72a493368d892653203feec03ce484bf17c.diff

LOG: [SelectionDAG][X86] Fix the assertion failure in Release build after #91747 (#93434)

In #91747, we changed 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. This breaks the
assumption
that the value type should match after the combine and triggers 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.
```

when running tests

llvm/test/CodeGen/X86/apx/ccmp.ll
llvm/test/CodeGen/X86/apx/ctest.ll

in Release build when LLVM_ENABLE_ASSERTIONS is on.

In this patch, we fix it by creating a merged value.

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/lib/Target/X86/X86ISelLowering.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index c7aeb0633e4ba..93d866384b482 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -1801,8 +1801,11 @@ void DAGCombiner::Run(CombineLevel AtLevel) {
 
     if (N->getNumValues() == RV->getNumValues())
       DAG.ReplaceAllUsesWith(N, RV.getNode());
-    else
+    else {
+      assert(N->getValueType(0) == RV.getValueType() &&
+             N->getNumValues() == 1 && "Type mismatch");
       DAG.ReplaceAllUsesWith(N, &RV);
+    }
 
     // Push the new node and any users onto the worklist.  Omit this if the
     // new node is the EntryToken (e.g. if a store managed to get optimized

diff  --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 7d90296a3eea6..0e514ca1246b0 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -49253,8 +49253,10 @@ static SDValue combineX86SubCmpForFlags(SDNode *N, SDValue Flag,
     return SDValue();
 
   SDValue X = SetCC.getOperand(1);
-  // Replace API is called manually here b/c the number of results may change.
-  DAG.ReplaceAllUsesOfValueWith(Flag, X);
+  // sub has two results while X only have one. DAG combine assumes the value
+  // type matches.
+  if (N->getOpcode() == X86ISD::SUB)
+    X = DAG.getMergeValues({N->getOperand(0), X}, SDLoc(N));
 
   SDValue CCN = SetCC.getOperand(0);
   X86::CondCode CC =