From ddf167c442018e807819aa0b2e56546977870333 Mon Sep 17 00:00:00 2001 From: Amara Emerson Date: Sat, 18 Feb 2023 09:51:17 -0800 Subject: [PATCH] [GlobalISel] Fix G_ZEXTLOAD being converted to G_SEXTLOAD incorrectly. The extending loads combine tries to prefer sign-extends folding into loads vs zexts, and in cases where a G_ZEXTLOAD is first used by a G_ZEXT, and then used by a G_SEXT, it would select the G_SEXT even though the load is already zero-extending. Fixes issue #59630 --- llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 11 +++++++---- .../prelegalizercombiner-extending-loads.mir | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp index d83bf24..ec4e45e 100644 --- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp @@ -399,7 +399,8 @@ namespace { /// Select a preference between two uses. CurrentUse is the current preference /// while *ForCandidate is attributes of the candidate under consideration. -PreferredTuple ChoosePreferredUse(PreferredTuple &CurrentUse, +PreferredTuple ChoosePreferredUse(MachineInstr &LoadMI, + PreferredTuple &CurrentUse, const LLT TyForCandidate, unsigned OpcodeForCandidate, MachineInstr *MIForCandidate) { @@ -425,8 +426,10 @@ PreferredTuple ChoosePreferredUse(PreferredTuple &CurrentUse, return {TyForCandidate, OpcodeForCandidate, MIForCandidate}; // Prefer sign extensions to zero extensions as sign-extensions tend to be - // more expensive. - if (CurrentUse.Ty == TyForCandidate) { + // more expensive. Don't do this if the load is already a zero-extend load + // though, otherwise we'll rewrite a zero-extend load into a sign-extend + // later. + if (!isa(LoadMI) && CurrentUse.Ty == TyForCandidate) { if (CurrentUse.ExtendOpcode == TargetOpcode::G_SEXT && OpcodeForCandidate == TargetOpcode::G_ZEXT) return CurrentUse; @@ -566,7 +569,7 @@ bool CombinerHelper::matchCombineExtendingLoads(MachineInstr &MI, .Action != LegalizeActions::Legal) continue; } - Preferred = ChoosePreferredUse(Preferred, + Preferred = ChoosePreferredUse(MI, Preferred, MRI.getType(UseMI.getOperand(0).getReg()), UseMI.getOpcode(), &UseMI); } diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads.mir index abefe43..47c85f7 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads.mir @@ -459,3 +459,24 @@ body: | $w0 = COPY %2(s32) RET_ReallyLR implicit $w0 ... +--- +name: test_dont_zextload_to_sextload +body: | + bb.0: + liveins: $x0 + ; CHECK-LABEL: name: test_dont_zextload_to_sextload + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x0 + ; CHECK-NEXT: [[ZEXTLOAD:%[0-9]+]]:_(s64) = G_ZEXTLOAD [[COPY]](p0) :: (load (s8)) + ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[ZEXTLOAD]](s64) + ; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s32) + ; CHECK-NEXT: $x0 = COPY [[ZEXTLOAD]](s64) + ; CHECK-NEXT: $x1 = COPY [[SEXT]](s64) + %0:_(p0) = COPY $x0 + %1:_(s32) = G_ZEXTLOAD %0 :: (load (s8)) + %2:_(s64) = G_ZEXT %1 + %3:_(s64) = G_SEXT %1 + $x0 = COPY %2 + $x1 = COPY %3 +... -- 2.7.4