[PATCH] D157811: [DAGCombiner] Don't merge stores to an illegal integer type

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai created this revision.
nemanjai added reviewers: arsenm, RKSimon, bogner.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nemanjai requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

The DAG Combiner finds the widest available integer and vector type and chooses the wider of the two for the merge. If vector types can't be used, it produces an integer type that is as wide. Due to the logic, this type is necessarily wider than the widest legal integer type, so if this is being combined after legalization, the back end will crash.

This patch fixes the logic to use a type of vector width only if vectors can be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157811

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll


Index: llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mattr=+vsx -ppc-vsr-nums-as-vr \
+; RUN:   -ppc-asm-full-reg-names -mtriple=powerpc64le-unknown-linux-gnu < %s \
+; RUN:   | FileCheck %s
+
+define dso_local void @test(i64* nocapture %arr1) local_unnamed_addr #0 {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    ld r4, 16(r3)
+; CHECK-NEXT:    ld r5, 24(r3)
+; CHECK-NEXT:    std r4, 0(r3)
+; CHECK-NEXT:    std r5, 8(r3)
+; CHECK-NEXT:    blr
+entry:
+%arrayidx = getelementptr inbounds i64, i64* %arr1, i64 2
+%0 = load i64, i64* %arrayidx, align 8
+store i64 %0, i64* %arr1, align 8
+%arrayidx2 = getelementptr inbounds i64, i64* %arr1, i64 3
+%1 = load i64, i64* %arrayidx2, align 8
+%arrayidx3 = getelementptr inbounds i64, i64* %arr1, i64 1
+store i64 %1, i64* %arrayidx3, align 8
+ret void
+}
+
+define dso_local void @test_vec(i64* nocapture %arr1) local_unnamed_addr {
+; CHECK-LABEL: test_vec:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    li r4, 16
+; CHECK-NEXT:    lxvd2x vs0, r3, r4
+; CHECK-NEXT:    stxvd2x vs0, 0, r3
+; CHECK-NEXT:    blr
+entry:
+%arrayidx = getelementptr inbounds i64, i64* %arr1, i64 2
+%0 = load i64, i64* %arrayidx, align 8
+store i64 %0, i64* %arr1, align 8
+%arrayidx2 = getelementptr inbounds i64, i64* %arr1, i64 3
+%1 = load i64, i64* %arrayidx2, align 8
+%arrayidx3 = getelementptr inbounds i64, i64* %arr1, i64 1
+store i64 %1, i64* %arrayidx3, align 8
+ret void
+}
+
+attributes #0 = { noimplicitfloat }
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -20187,8 +20187,7 @@
     // type. If they are the same, use integers.
     bool UseVectorTy =
         LastLegalVectorType > LastLegalIntegerType && AllowVectors;
-    unsigned LastLegalType =
-        std::max(LastLegalVectorType, LastLegalIntegerType);
+    unsigned LastLegalType = UseVectorTy ? LastLegalVectorType : LastLegalIntegerType;
 
     // We add +1 here because the LastXXX variables refer to location while
     // the NumElem refers to array/index size.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157811.549698.patch
Type: text/x-patch
Size: 2518 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230813/e2ee5bf4/attachment.bin>