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) _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
