[llvm] 93e1568 - [DAG] Fix a miscompile in insert_subvector undef (insert_subvector undef, ..), idx combine (#73587)

[via llvm-commits]

Author: Philip Reames
Date: 2023-11-27T16:45:29-08:00
New Revision: 93e156833bfadfbabfa5be2afad4f002c019257f

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

LOG: [DAG] Fix a miscompile in insert_subvector undef (insert_subvector undef, ..), idx combine (#73587)

The combine was implicitly assuming that the index on the outer
insert_subvector meant the same thing when the source was switched to be
the index of the inner insert_subvector. This is not true if the
innermost sub-vector is fixed, and the outer subvector is scalable.

I could do a less restrictive fix here - i.e. allow the case where the
scalability of the subvectors are the same - but there's no test
coverage which shows this transform actually has profit. Given that, go
for the simplest fix.

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 41d36e7d16d2e14..2a3425a42607e72 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -26153,10 +26153,11 @@ SDValue DAGCombiner::visitINSERT_SUBVECTOR(SDNode *N) {
                        N1, N2);
 
   // Eliminate an intermediate insert into an undef vector:
-  // insert_subvector undef, (insert_subvector undef, X, 0), N2 -->
-  // insert_subvector undef, X, N2
+  // insert_subvector undef, (insert_subvector undef, X, 0), 0 -->
+  // insert_subvector undef, X, 0
   if (N0.isUndef() && N1.getOpcode() == ISD::INSERT_SUBVECTOR &&
-      N1.getOperand(0).isUndef() && isNullConstant(N1.getOperand(2)))
+      N1.getOperand(0).isUndef() && isNullConstant(N1.getOperand(2)) &&
+      isNullConstant(N2))
     return DAG.getNode(ISD::INSERT_SUBVECTOR, SDLoc(N), VT, N0,
                        N1.getOperand(1), N2);
 

diff  --git a/llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll b/llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll
index 57de8341cb89cef..8a368e7161c3f2a 100644
--- a/llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll
@@ -505,6 +505,33 @@ define <vscale x 2 x i64> @insert_nxv2i64_nxv3i64(<3 x i64> %sv) #0 {
   ret <vscale x 2 x i64> %vec
 }
 
+; This shows a case where we were miscompiling because the index of the
+; outer expects a scalable inner and the inner most subvector is fixed length.
+; The code generated happens to be correct if VLEN=128, but is wrong if
+; VLEN=256.
+define <vscale x 8 x i32> @insert_insert_combine(<2 x i32> %subvec) {
+; CHECK-LABEL: insert_insert_combine:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vmv1r.v v10, v8
+; CHECK-NEXT:    ret
+  %inner = call <vscale x 4 x i32> @llvm.vector.insert.nxv4i32.v2i32(<vscale x 4 x i32> undef, <2 x i32> %subvec, i64 0)
+  %outer = call <vscale x 8 x i32> @llvm.vector.insert.nxv4i32.nxv8i32(<vscale x 8 x i32> undef, <vscale x 4 x i32> %inner, i64 4)
+  ret <vscale x 8 x i32> %outer
+}
+
+; We can combine these two (even with non-zero index on the outer) because
+; the vector must be an even multiple.
+define <vscale x 8 x i32> @insert_insert_combine2(<vscale x 2 x i32> %subvec) {
+; CHECK-LABEL: insert_insert_combine2:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vmv1r.v v10, v8
+; CHECK-NEXT:    ret
+  %inner = call <vscale x 4 x i32> @llvm.vector.insert.nxv2i32.nxv4i32(<vscale x 4 x i32> undef, <vscale x 2 x i32> %subvec, i64 0)
+  %outer = call <vscale x 8 x i32> @llvm.vector.insert.nxv4i32.nxv8i32(<vscale x 8 x i32> undef, <vscale x 4 x i32> %inner, i64 4)
+  ret <vscale x 8 x i32> %outer
+}
+
+
 attributes #0 = { vscale_range(2,1024) }
 
 declare <vscale x 4 x i1> @llvm.vector.insert.nxv1i1.nxv4i1(<vscale x 4 x i1>, <vscale x 1 x i1>, i64)
@@ -517,6 +544,9 @@ declare <vscale x 32 x half> @llvm.vector.insert.nxv2f16.nxv32f16(<vscale x 32 x
 
 declare <vscale x 4 x i8> @llvm.vector.insert.nxv1i8.nxv4i8(<vscale x 4 x i8>, <vscale x 1 x i8>, i64 %idx)
 
+declare <vscale x 4 x i32> @llvm.vector.insert.nxv2i32.nxv4i32(<vscale x 4 x i32>, <vscale x 2 x i32>, i64)
+declare <vscale x 4 x i32> @llvm.vector.insert.nxv4i32.v2i32(<vscale x 4 x i32>, <2 x i32>, i64)
+
 declare <vscale x 8 x i32> @llvm.vector.insert.nxv2i32.nxv8i32(<vscale x 8 x i32>, <vscale x 2 x i32>, i64 %idx)
 declare <vscale x 8 x i32> @llvm.vector.insert.nxv4i32.nxv8i32(<vscale x 8 x i32>, <vscale x 4 x i32>, i64 %idx)