[llvm] [SelectionDAG] Ignore IsLegal parameter in TargetLoweringBase::getShiftAmountTy. (PR #97645)

[Craig Topper via llvm-commits]

https://github.com/topperc created https://github.com/llvm/llvm-project/pull/97645

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.

>From c37aca5a950fd283c0c0db8243feb92426bb8972 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 3 Jul 2024 14:29:11 -0700
Subject: [PATCH] [SelectionDAG] Ignore IsLegal parameter in
 TargetLoweringBase::getShiftAmountTy.

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.
---
 llvm/lib/CodeGen/TargetLoweringBase.cpp |  3 +--
 llvm/test/CodeGen/X86/shift-combine.ll  | 10 ++++------
 2 files changed, 5 insertions(+), 8 deletions(-)

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
 ;