Skip to content

Commit

Permalink
[GlobalISel] Check if ShiftAmt is greater then size of operand
Browse files Browse the repository at this point in the history 
matchCombineShlOfExtend did not check if the size of new shift would be
wider then a size of operand. Current condition did not work if the value
being shifted was zero. Updated to support vector splat.

Patch by: Acim Maravic

Differential Revision: https://reviews.llvm.org/D151122
  • Loading branch information
@mbrkusanin
mbrkusanin committed Jun 8, 2023
1 parent 7815166 commit 792667d
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 160 deletions.
9 changes: 5 additions & 4 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1722,9 +1722,9 @@ bool CombinerHelper::matchCombineShlOfExtend(MachineInstr &MI,
!mi_match(LHS, MRI, m_GSExt(m_Reg(ExtSrc))))
return false;

// TODO: Should handle vector splat.
Register RHS = MI.getOperand(2).getReg();
auto MaybeShiftAmtVal = getIConstantVRegValWithLookThrough(RHS, MRI);
MachineInstr *MIShiftAmt = MRI.getVRegDef(RHS);
auto MaybeShiftAmtVal = isConstantOrConstantSplatVector(*MIShiftAmt, MRI);
if (!MaybeShiftAmtVal)
return false;

Expand All @@ -1739,12 +1739,13 @@ bool CombinerHelper::matchCombineShlOfExtend(MachineInstr &MI,
return false;
}

int64_t ShiftAmt = MaybeShiftAmtVal->Value.getSExtValue();
int64_t ShiftAmt = MaybeShiftAmtVal->getSExtValue();
MatchData.Reg = ExtSrc;
MatchData.Imm = ShiftAmt;

unsigned MinLeadingZeros = KB->getKnownZeroes(ExtSrc).countl_one();
return MinLeadingZeros >= ShiftAmt;
unsigned SrcTySize = MRI.getType(ExtSrc).getScalarSizeInBits();
return MinLeadingZeros >= ShiftAmt && ShiftAmt < SrcTySize;
}

void CombinerHelper::applyCombineShlOfExtend(MachineInstr &MI,
Expand Down
12 changes: 7 additions & 5 deletions llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-crash.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,20 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
; CHECK-NEXT: [[DEF:%[0-9]+]]:_(s1) = G_IMPLICIT_DEF
; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 32
; CHECK-NEXT: G_BRCOND [[DEF]](s1), %bb.2
; CHECK-NEXT: G_BR %bb.1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1:
; CHECK-NEXT: successors: %bb.2(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[C1]](s32)
; CHECK-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 -1
; CHECK-NEXT: [[FREEZE:%[0-9]+]]:_(s64) = G_FREEZE [[C2]]
; CHECK-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[C2]](s32)
; CHECK-NEXT: [[C3:%[0-9]+]]:_(s64) = G_CONSTANT i64 -1
; CHECK-NEXT: [[FREEZE:%[0-9]+]]:_(s64) = G_FREEZE [[C3]]
; CHECK-NEXT: [[UDIV:%[0-9]+]]:_(s64) = G_UDIV [[FREEZE]], [[C]]
; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s64) = G_ADD [[ZEXT]], [[UDIV]]
; CHECK-NEXT: [[SHL:%[0-9]+]]:_(s64) = G_SHL [[ZEXT]], [[C1]](s64)
; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s64) = G_ADD [[SHL]], [[UDIV]]
; CHECK-NEXT: G_STORE [[ADD]](s64), [[COPY]](p0) :: (store (s64))
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2:
Expand Down
102 changes: 102 additions & 0 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shl-from-extend-narrow.postlegal.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,105 @@ body: |
%shl:_(s64) = G_SHL %extend, %shiftamt
$vgpr0_vgpr1 = COPY %shl
...

---
name: do_not_shl_s32_zero_by_16_from_zext_s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0
; GFX6-LABEL: name: do_not_shl_s32_zero_by_16_from_zext_s16
; GFX6: liveins: $vgpr0
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX6-NEXT: %extend:_(s32) = G_ZEXT %zero(s16)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX6-NEXT: $vgpr0 = COPY %shl(s32)
; GFX9-LABEL: name: do_not_shl_s32_zero_by_16_from_zext_s16
; GFX9: liveins: $vgpr0
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX9-NEXT: %extend:_(s32) = G_ZEXT %zero(s16)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX9-NEXT: $vgpr0 = COPY %shl(s32)
%zero:_(s16) = G_CONSTANT i16 0
%extend:_(s32) = G_ZEXT %zero:_(s16)
%shiftamt:_(s16) = G_CONSTANT i16 16
%shl:_(s32) = G_SHL %extend, %shiftamt(s16)
$vgpr0 = COPY %shl
...

---
name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0
; GFX6-LABEL: name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
; GFX6: liveins: $vgpr0
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX6-NEXT: %zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero(s16), %zero(s16)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt(s16), %shiftamt(s16)
; GFX6-NEXT: %extend:_(<2 x s32>) = G_ZEXT %zerovector(<2 x s16>)
; GFX6-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector(<2 x s16>)
; GFX6-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
; GFX9-LABEL: name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
; GFX9: liveins: $vgpr0
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX9-NEXT: %zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero(s16), %zero(s16)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt(s16), %shiftamt(s16)
; GFX9-NEXT: %extend:_(<2 x s32>) = G_ZEXT %zerovector(<2 x s16>)
; GFX9-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector(<2 x s16>)
; GFX9-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
%zero:_(s16) = G_CONSTANT i16 0
%zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero, %zero:_(s16)
%shiftamt:_(s16) = G_CONSTANT i16 16
%shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt, %shiftamt:_(s16)
%extend:_(<2 x s32>) = G_ZEXT %zerovector:_(<2 x s16>)
%shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector
$vgpr0_vgpr1 = COPY %shl
...

---
name: do_not_shl_s32_by_16_from_zext_s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0
; GFX6-LABEL: name: do_not_shl_s32_by_16_from_zext_s16
; GFX6: liveins: $vgpr0
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %argument:_(s32) = COPY $vgpr0
; GFX6-NEXT: %truncate:_(s16) = G_TRUNC %argument(s32)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %extend:_(s32) = G_ZEXT %truncate(s16)
; GFX6-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX6-NEXT: $vgpr0 = COPY %shl(s32)
; GFX9-LABEL: name: do_not_shl_s32_by_16_from_zext_s16
; GFX9: liveins: $vgpr0
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %argument:_(s32) = COPY $vgpr0
; GFX9-NEXT: %truncate:_(s16) = G_TRUNC %argument(s32)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %extend:_(s32) = G_ZEXT %truncate(s16)
; GFX9-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX9-NEXT: $vgpr0 = COPY %shl(s32)
%argument:_(s32) = COPY $vgpr0
%truncate:_(s16) = G_TRUNC %argument:_(s32)
%shiftamt:_(s16) = G_CONSTANT i16 16
%extend:_(s32) = G_ZEXT %truncate:_(s16)
%shl:_(s32) = G_SHL %extend, %shiftamt(s16)
$vgpr0 = COPY %shl
...
126 changes: 114 additions & 12 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shl-from-extend-narrow.prelegal.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ body: |
; GFX6-NEXT: %masklow14:_(s16) = G_CONSTANT i16 16383
; GFX6-NEXT: %masklow14vec:_(<2 x s16>) = G_BUILD_VECTOR %masklow14(s16), %masklow14(s16)
; GFX6-NEXT: %masked:_(<2 x s16>) = G_AND %narrow, %masklow14vec
; GFX6-NEXT: %extend:_(<2 x s32>) = G_ZEXT %masked(<2 x s16>)
; GFX6-NEXT: %shiftamt:_(s32) = G_CONSTANT i32 2
; GFX6-NEXT: %shiftamtvec:_(<2 x s32>) = G_BUILD_VECTOR %shiftamt(s32), %shiftamt(s32)
; GFX6-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvec(<2 x s32>)
; GFX6-NEXT: [[C:%[0-9]+]]:_(s16) = G_CONSTANT i16 2
; GFX6-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<2 x s16>) = G_BUILD_VECTOR [[C]](s16), [[C]](s16)
; GFX6-NEXT: [[SHL:%[0-9]+]]:_(<2 x s16>) = G_SHL %masked, [[BUILD_VECTOR]](<2 x s16>)
; GFX6-NEXT: %shl:_(<2 x s32>) = G_ZEXT [[SHL]](<2 x s16>)
; GFX6-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
; GFX9-LABEL: name: narrow_shl_v2s32_by_2_from_zext_v2s16
; GFX9: liveins: $vgpr0
Expand All @@ -146,10 +146,10 @@ body: |
; GFX9-NEXT: %masklow14:_(s16) = G_CONSTANT i16 16383
; GFX9-NEXT: %masklow14vec:_(<2 x s16>) = G_BUILD_VECTOR %masklow14(s16), %masklow14(s16)
; GFX9-NEXT: %masked:_(<2 x s16>) = G_AND %narrow, %masklow14vec
; GFX9-NEXT: %extend:_(<2 x s32>) = G_ZEXT %masked(<2 x s16>)
; GFX9-NEXT: %shiftamt:_(s32) = G_CONSTANT i32 2
; GFX9-NEXT: %shiftamtvec:_(<2 x s32>) = G_BUILD_VECTOR %shiftamt(s32), %shiftamt(s32)
; GFX9-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvec(<2 x s32>)
; GFX9-NEXT: [[C:%[0-9]+]]:_(s16) = G_CONSTANT i16 2
; GFX9-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<2 x s16>) = G_BUILD_VECTOR [[C]](s16), [[C]](s16)
; GFX9-NEXT: [[SHL:%[0-9]+]]:_(<2 x s16>) = G_SHL %masked, [[BUILD_VECTOR]](<2 x s16>)
; GFX9-NEXT: %shl:_(<2 x s32>) = G_ZEXT [[SHL]](<2 x s16>)
; GFX9-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
%narrow:_(<2 x s16>) = COPY $vgpr0
%masklow14:_(s16) = G_CONSTANT i16 16383
Expand All @@ -176,10 +176,10 @@ body: |
; GFX6-NEXT: %masklow30:_(s32) = G_CONSTANT i32 1073741823
; GFX6-NEXT: %masklow30vec:_(<2 x s32>) = G_BUILD_VECTOR %masklow30(s32), %masklow30(s32)
; GFX6-NEXT: %masked:_(<2 x s32>) = G_AND %narrow, %masklow30vec
; GFX6-NEXT: %extend:_(<2 x s64>) = G_ANYEXT %masked(<2 x s32>)
; GFX6-NEXT: %shiftamt:_(s32) = G_CONSTANT i32 2
; GFX6-NEXT: %shiftamtvec:_(<2 x s32>) = G_BUILD_VECTOR %shiftamt(s32), %shiftamt(s32)
; GFX6-NEXT: %shl:_(<2 x s64>) = G_SHL %extend, %shiftamtvec(<2 x s32>)
; GFX6-NEXT: [[SHL:%[0-9]+]]:_(<2 x s32>) = G_SHL %masked, %shiftamtvec(<2 x s32>)
; GFX6-NEXT: %shl:_(<2 x s64>) = G_ZEXT [[SHL]](<2 x s32>)
; GFX6-NEXT: $vgpr0_vgpr1_vgpr2_vgpr3 = COPY %shl(<2 x s64>)
; GFX9-LABEL: name: narrow_shl_v2s64_by_2_from_anyext_v2s32
; GFX9: liveins: $vgpr0_vgpr1
Expand All @@ -188,10 +188,10 @@ body: |
; GFX9-NEXT: %masklow30:_(s32) = G_CONSTANT i32 1073741823
; GFX9-NEXT: %masklow30vec:_(<2 x s32>) = G_BUILD_VECTOR %masklow30(s32), %masklow30(s32)
; GFX9-NEXT: %masked:_(<2 x s32>) = G_AND %narrow, %masklow30vec
; GFX9-NEXT: %extend:_(<2 x s64>) = G_ANYEXT %masked(<2 x s32>)
; GFX9-NEXT: %shiftamt:_(s32) = G_CONSTANT i32 2
; GFX9-NEXT: %shiftamtvec:_(<2 x s32>) = G_BUILD_VECTOR %shiftamt(s32), %shiftamt(s32)
; GFX9-NEXT: %shl:_(<2 x s64>) = G_SHL %extend, %shiftamtvec(<2 x s32>)
; GFX9-NEXT: [[SHL:%[0-9]+]]:_(<2 x s32>) = G_SHL %masked, %shiftamtvec(<2 x s32>)
; GFX9-NEXT: %shl:_(<2 x s64>) = G_ZEXT [[SHL]](<2 x s32>)
; GFX9-NEXT: $vgpr0_vgpr1_vgpr2_vgpr3 = COPY %shl(<2 x s64>)
%narrow:_(<2 x s32>) = COPY $vgpr0_vgpr1
%masklow30:_(s32) = G_CONSTANT i32 1073741823
Expand All @@ -203,3 +203,105 @@ body: |
%shl:_(<2 x s64>) = G_SHL %extend, %shiftamtvec
$vgpr0_vgpr1_vgpr2_vgpr3 = COPY %shl
...

---
name: do_not_shl_s32_zero_by_16_from_zext_s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0
; GFX6-LABEL: name: do_not_shl_s32_zero_by_16_from_zext_s16
; GFX6: liveins: $vgpr0
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX6-NEXT: %extend:_(s32) = G_ZEXT %zero(s16)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX6-NEXT: $vgpr0 = COPY %shl(s32)
; GFX9-LABEL: name: do_not_shl_s32_zero_by_16_from_zext_s16
; GFX9: liveins: $vgpr0
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX9-NEXT: %extend:_(s32) = G_ZEXT %zero(s16)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX9-NEXT: $vgpr0 = COPY %shl(s32)
%zero:_(s16) = G_CONSTANT i16 0
%extend:_(s32) = G_ZEXT %zero:_(s16)
%shiftamt:_(s16) = G_CONSTANT i16 16
%shl:_(s32) = G_SHL %extend, %shiftamt(s16)
$vgpr0 = COPY %shl
...

---
name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0, $vgpr1
; GFX6-LABEL: name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
; GFX6: liveins: $vgpr0, $vgpr1
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX6-NEXT: %zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero(s16), %zero(s16)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt(s16), %shiftamt(s16)
; GFX6-NEXT: %extend:_(<2 x s32>) = G_ZEXT %zerovector(<2 x s16>)
; GFX6-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector(<2 x s16>)
; GFX6-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
; GFX9-LABEL: name: do_not_shl_v2s32_zero_by_16_from_zext_v2s16
; GFX9: liveins: $vgpr0, $vgpr1
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %zero:_(s16) = G_CONSTANT i16 0
; GFX9-NEXT: %zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero(s16), %zero(s16)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt(s16), %shiftamt(s16)
; GFX9-NEXT: %extend:_(<2 x s32>) = G_ZEXT %zerovector(<2 x s16>)
; GFX9-NEXT: %shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector(<2 x s16>)
; GFX9-NEXT: $vgpr0_vgpr1 = COPY %shl(<2 x s32>)
%zero:_(s16) = G_CONSTANT i16 0
%zerovector:_(<2 x s16>) = G_BUILD_VECTOR %zero, %zero:_(s16)
%shiftamt:_(s16) = G_CONSTANT i16 16
%shiftamtvector:_(<2 x s16>) = G_BUILD_VECTOR %shiftamt, %shiftamt:_(s16)
%extend:_(<2 x s32>) = G_ZEXT %zerovector:_(<2 x s16>)
%shl:_(<2 x s32>) = G_SHL %extend, %shiftamtvector
$vgpr0_vgpr1 = COPY %shl
...

---
name: do_not_shl_s32_by_16_from_zext_s16
tracksRegLiveness: true
legalized: true

body: |
bb.0:
liveins: $vgpr0
; GFX6-LABEL: name: do_not_shl_s32_by_16_from_zext_s16
; GFX6: liveins: $vgpr0
; GFX6-NEXT: {{ $}}
; GFX6-NEXT: %argument:_(s32) = COPY $vgpr0
; GFX6-NEXT: %truncate:_(s16) = G_TRUNC %argument(s32)
; GFX6-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX6-NEXT: %extend:_(s32) = G_ZEXT %truncate(s16)
; GFX6-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX6-NEXT: $vgpr0 = COPY %shl(s32)
; GFX9-LABEL: name: do_not_shl_s32_by_16_from_zext_s16
; GFX9: liveins: $vgpr0
; GFX9-NEXT: {{ $}}
; GFX9-NEXT: %argument:_(s32) = COPY $vgpr0
; GFX9-NEXT: %truncate:_(s16) = G_TRUNC %argument(s32)
; GFX9-NEXT: %shiftamt:_(s16) = G_CONSTANT i16 16
; GFX9-NEXT: %extend:_(s32) = G_ZEXT %truncate(s16)
; GFX9-NEXT: %shl:_(s32) = G_SHL %extend, %shiftamt(s16)
; GFX9-NEXT: $vgpr0 = COPY %shl(s32)
%argument:_(s32) = COPY $vgpr0
%truncate:_(s16) = G_TRUNC %argument:_(s32)
%shiftamt:_(s16) = G_CONSTANT i16 16
%extend:_(s32) = G_ZEXT %truncate:_(s16)
%shl:_(s32) = G_SHL %extend, %shiftamt(s16)
$vgpr0 = COPY %shl
...

0 comments on commit 792667d

Please sign in to comment.