[SelectionDAG] Ignore LegalTypes parameter in TargetLoweringBase::getShiftAmountTy.

[topperc]

When this flag was false, getShiftAmountTy would return PointerTy instead of the target's preferred shift amount type for scalar shifts.

This used to be needed when the target's preferred type wasn't large enough to support the shift amount needed for an illegal type. For example, any scalar type larger than i256 on X86 since X86's preferred shift amount type is i8.

For a while now, we've had code that uses MVT::i32 if LegalTypes is true, but the target's preferred type is too small. This fixed a repeated cause of crashes where the LegalTypes flag wasn't set to false when illegal types could be present.

This has made it unnecessary to set the LegalTypes flag correctly, and as a result more and more places don't. So I think its time for this flag to go away.

This first patch just disconnects the flag. The interface and all callers will be cleaned up in follow up patches.

The X86 test change is because we now have the same shift type for both shifts in a (srl (sub C, (shl X, 32), 32) sequence. This makes the shift amounts appear equal in value and type which is needed to enable a combine.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Craig Topper (topperc)

Changes

When this flag was false, getShiftAmountTy would return PointerTy instead of the target's preferred shift amount type for scalar shifts.

This used to be needed when the target's preferred type wasn't large enough to support the shift amount needed for an illegal type. For example, any scalar type larger than i256 on X86 since X86's preferred shift amount type is i8.

For a while now, we've had code that uses MVT::i32 if LegalTypes is true, but the target's preferred type is too small. This fixed a repeated cause of crashes where the LegalTypes flag wasn't set to false when illegal types could be present.

This has made it unnecessary to set the LegalTypes flag correctly, and as a result more and more places don't. So I think its time for this flag to go away.

This first patch just disconnects the flag. The interface and all callers will be cleaned up in follow up patches.

The X86 test change is because we now have the same shift type for both shifts in a (srl (sub C, (shl X, 32), 32) sequence. This makes the shift amounts appear equal in value and type which is needed to enable a combine.

---

Full diff: https://github.com/llvm/llvm-project/pull/97645.diff

2 Files Affected:

• (modified) llvm/lib/CodeGen/TargetLoweringBase.cpp (+1-2)
• (modified) llvm/test/CodeGen/X86/shift-combine.ll (+4-6)

diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index ff684c7cb6bba..c1a4e5d198ac4 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -1057,8 +1057,7 @@ EVT TargetLoweringBase::getShiftAmountTy(EVT LHSTy, const DataLayout &DL,
   assert(LHSTy.isInteger() && "Shift amount is not an integer type!");
   if (LHSTy.isVector())
     return LHSTy;
-  MVT ShiftVT =
-      LegalTypes ? getScalarShiftAmountTy(DL, LHSTy) : getPointerTy(DL);
+  MVT ShiftVT = getScalarShiftAmountTy(DL, LHSTy);
   // If any possible shift value won't fit in the prefered type, just use
   // something safe. Assume it will be legalized when the shift is expanded.
   if (ShiftVT.getSizeInBits() < Log2_32_Ceil(LHSTy.getSizeInBits()))
diff --git a/llvm/test/CodeGen/X86/shift-combine.ll b/llvm/test/CodeGen/X86/shift-combine.ll
index 30c3d53dd37c9..c9edd3f3e9048 100644
--- a/llvm/test/CodeGen/X86/shift-combine.ll
+++ b/llvm/test/CodeGen/X86/shift-combine.ll
@@ -444,12 +444,10 @@ define i64 @ashr_add_neg_shl_i32(i64 %r) nounwind {
 define i64 @ashr_add_neg_shl_i8(i64 %r) nounwind {
 ; X86-LABEL: ashr_add_neg_shl_i8:
 ; X86:       # %bb.0:
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    shll $24, %eax
-; X86-NEXT:    movl $33554432, %edx # imm = 0x2000000
-; X86-NEXT:    subl %eax, %edx
-; X86-NEXT:    movl %edx, %eax
-; X86-NEXT:    sarl $24, %eax
+; X86-NEXT:    movb $2, %al
+; X86-NEXT:    subb {{[0-9]+}}(%esp), %al
+; X86-NEXT:    movsbl %al, %eax
+; X86-NEXT:    movl %eax, %edx
 ; X86-NEXT:    sarl $31, %edx
 ; X86-NEXT:    retl
 ;

[RKSimon]

LGTM