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

[nemanjai]

Summary

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.

Fixes: https://github.com/llvm/llvm-project/issues/39641

[arsenm]

llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll

16

Test should use opaque pointers

25

Can drop attributes and local_unnamed_addr

[nemanjai]

llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll

16

Ah, good point. This was added to the bug report before the switch. I'll update it.

25

I kept the attributes on one version so we can test the noimplicitfloat and a regular one in two functions within the same invocation of llc. Would you prefer that I just have one function with no attributes and just use two RUN lines?

[arsenm]

llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll

25

The attributes that matter should stay, don’t need multiple invocations

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

for (unsigned i = 1; i < LoadNodes.size(); ++i) {
         }
       }
     }
 
     // Only use vector types if the vector type is larger than the integer
     // 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.
     unsigned NumElem = std::min(NumConsecutiveStores, LastConsecutiveLoad + 1);
     NumElem = std::min(LastLegalType, NumElem);
     Align FirstLoadAlign = FirstLoad->getAlign();
 
     if (NumElem < 2) {

llvm/test/CodeGen/PowerPC/merge_stores_post_legalization_crash.ll

+; 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 }