Skip to content

Commit 121cd7c

Browse files
authored
Re apply 130577 narrow math for and operand (#133896)
Re-apply #130577 Which is reverted in #133880 The old application failed in address sanitizer due to `tryNarrowMathIfNoOverflow` was called after `I.eraseFromParent();` in `AMDGPUCodeGenPrepareImpl::visitBinaryOperator`, it create a use after free failure. To fix this, `tryNarrowMathIfNoOverflow` will be called before and directly return if `tryNarrowMathIfNoOverflow` result in true.
1 parent 2ee7fc0 commit 121cd7c

File tree

6 files changed

+346
-48
lines changed

6 files changed

+346
-48
lines changed

llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,80 @@ void AMDGPUCodeGenPrepareImpl::expandDivRem64(BinaryOperator &I) const {
15591559
llvm_unreachable("not a division");
15601560
}
15611561

1562+
/*
1563+
This will cause non-byte load in consistency, for example:
1564+
```
1565+
%load = load i1, ptr addrspace(4) %arg, align 4
1566+
%zext = zext i1 %load to
1567+
i64 %add = add i64 %zext
1568+
```
1569+
Instead of creating `s_and_b32 s0, s0, 1`,
1570+
it will create `s_and_b32 s0, s0, 0xff`.
1571+
We accept this change since the non-byte load assumes the upper bits
1572+
within the byte are all 0.
1573+
*/
1574+
static bool tryNarrowMathIfNoOverflow(Instruction *I,
1575+
const SITargetLowering *TLI,
1576+
const TargetTransformInfo &TTI,
1577+
const DataLayout &DL) {
1578+
unsigned Opc = I->getOpcode();
1579+
Type *OldType = I->getType();
1580+
1581+
if (Opc != Instruction::Add && Opc != Instruction::Mul)
1582+
return false;
1583+
1584+
unsigned OrigBit = OldType->getScalarSizeInBits();
1585+
1586+
if (Opc != Instruction::Add && Opc != Instruction::Mul)
1587+
llvm_unreachable("Unexpected opcode, only valid for Instruction::Add and "
1588+
"Instruction::Mul.");
1589+
1590+
unsigned MaxBitsNeeded = computeKnownBits(I, DL).countMaxActiveBits();
1591+
1592+
MaxBitsNeeded = std::max<unsigned>(bit_ceil(MaxBitsNeeded), 8);
1593+
Type *NewType = DL.getSmallestLegalIntType(I->getContext(), MaxBitsNeeded);
1594+
if (!NewType)
1595+
return false;
1596+
unsigned NewBit = NewType->getIntegerBitWidth();
1597+
if (NewBit >= OrigBit)
1598+
return false;
1599+
NewType = I->getType()->getWithNewBitWidth(NewBit);
1600+
1601+
// Old cost
1602+
InstructionCost OldCost =
1603+
TTI.getArithmeticInstrCost(Opc, OldType, TTI::TCK_RecipThroughput);
1604+
// New cost of new op
1605+
InstructionCost NewCost =
1606+
TTI.getArithmeticInstrCost(Opc, NewType, TTI::TCK_RecipThroughput);
1607+
// New cost of narrowing 2 operands (use trunc)
1608+
int NumOfNonConstOps = 2;
1609+
if (isa<Constant>(I->getOperand(0)) || isa<Constant>(I->getOperand(1))) {
1610+
// Cannot be both constant, should be propagated
1611+
NumOfNonConstOps = 1;
1612+
}
1613+
NewCost += NumOfNonConstOps * TTI.getCastInstrCost(Instruction::Trunc,
1614+
NewType, OldType,
1615+
TTI.getCastContextHint(I),
1616+
TTI::TCK_RecipThroughput);
1617+
// New cost of zext narrowed result to original type
1618+
NewCost +=
1619+
TTI.getCastInstrCost(Instruction::ZExt, OldType, NewType,
1620+
TTI.getCastContextHint(I), TTI::TCK_RecipThroughput);
1621+
if (NewCost >= OldCost)
1622+
return false;
1623+
1624+
IRBuilder<> Builder(I);
1625+
Value *Trunc0 = Builder.CreateTrunc(I->getOperand(0), NewType);
1626+
Value *Trunc1 = Builder.CreateTrunc(I->getOperand(1), NewType);
1627+
Value *Arith =
1628+
Builder.CreateBinOp((Instruction::BinaryOps)Opc, Trunc0, Trunc1);
1629+
1630+
Value *Zext = Builder.CreateZExt(Arith, OldType);
1631+
I->replaceAllUsesWith(Zext);
1632+
I->eraseFromParent();
1633+
return true;
1634+
}
1635+
15621636
bool AMDGPUCodeGenPrepareImpl::visitBinaryOperator(BinaryOperator &I) {
15631637
if (foldBinOpIntoSelect(I))
15641638
return true;
@@ -1569,6 +1643,9 @@ bool AMDGPUCodeGenPrepareImpl::visitBinaryOperator(BinaryOperator &I) {
15691643

15701644
if (UseMul24Intrin && replaceMulWithMul24(I))
15711645
return true;
1646+
if (tryNarrowMathIfNoOverflow(&I, ST.getTargetLowering(),
1647+
TM.getTargetTransformInfo(F), DL))
1648+
return true;
15721649

15731650
bool Changed = false;
15741651
Instruction::BinaryOps Opc = I.getOpcode();

llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-mul24.ll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,10 @@ define i64 @umul24_i64_2(i64 %lhs, i64 %rhs) {
414414
; DISABLED-LABEL: @umul24_i64_2(
415415
; DISABLED-NEXT: [[LHS24:%.*]] = and i64 [[LHS:%.*]], 65535
416416
; DISABLED-NEXT: [[RHS24:%.*]] = and i64 [[RHS:%.*]], 65535
417-
; DISABLED-NEXT: [[MUL:%.*]] = mul i64 [[LHS24]], [[RHS24]]
417+
; DISABLED-NEXT: [[TMP1:%.*]] = trunc i64 [[LHS24]] to i32
418+
; DISABLED-NEXT: [[TMP2:%.*]] = trunc i64 [[RHS24]] to i32
419+
; DISABLED-NEXT: [[TMP3:%.*]] = mul i32 [[TMP1]], [[TMP2]]
420+
; DISABLED-NEXT: [[MUL:%.*]] = zext i32 [[TMP3]] to i64
418421
; DISABLED-NEXT: ret i64 [[MUL]]
419422
;
420423
%lhs24 = and i64 %lhs, 65535

llvm/test/CodeGen/AMDGPU/atomic_optimizations_global_pointer.ll

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1823,22 +1823,22 @@ define amdgpu_kernel void @add_i64_constant(ptr addrspace(1) %out, ptr addrspace
18231823
; GFX1264: ; %bb.0: ; %entry
18241824
; GFX1264-NEXT: s_load_b128 s[0:3], s[4:5], 0x24
18251825
; GFX1264-NEXT: s_mov_b64 s[6:7], exec
1826-
; GFX1264-NEXT: s_mov_b32 s9, 0
1827-
; GFX1264-NEXT: v_mbcnt_lo_u32_b32 v0, s6, 0
18281826
; GFX1264-NEXT: s_mov_b64 s[4:5], exec
1827+
; GFX1264-NEXT: v_mbcnt_lo_u32_b32 v0, s6, 0
18291828
; GFX1264-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
18301829
; GFX1264-NEXT: v_mbcnt_hi_u32_b32 v2, s7, v0
18311830
; GFX1264-NEXT: ; implicit-def: $vgpr0_vgpr1
18321831
; GFX1264-NEXT: v_cmpx_eq_u32_e32 0, v2
18331832
; GFX1264-NEXT: s_cbranch_execz .LBB3_2
18341833
; GFX1264-NEXT: ; %bb.1:
1835-
; GFX1264-NEXT: s_bcnt1_i32_b64 s8, s[6:7]
1834+
; GFX1264-NEXT: s_bcnt1_i32_b64 s6, s[6:7]
1835+
; GFX1264-NEXT: v_mov_b32_e32 v1, 0
1836+
; GFX1264-NEXT: s_wait_alu 0xfffe
1837+
; GFX1264-NEXT: s_mul_i32 s6, s6, 5
18361838
; GFX1264-NEXT: s_mov_b32 s11, 0x31016000
1837-
; GFX1264-NEXT: s_mul_u64 s[6:7], s[8:9], 5
1838-
; GFX1264-NEXT: s_mov_b32 s10, -1
18391839
; GFX1264-NEXT: s_wait_alu 0xfffe
18401840
; GFX1264-NEXT: v_mov_b32_e32 v0, s6
1841-
; GFX1264-NEXT: v_mov_b32_e32 v1, s7
1841+
; GFX1264-NEXT: s_mov_b32 s10, -1
18421842
; GFX1264-NEXT: s_wait_kmcnt 0x0
18431843
; GFX1264-NEXT: s_mov_b32 s8, s2
18441844
; GFX1264-NEXT: s_mov_b32 s9, s3
@@ -1860,29 +1860,27 @@ define amdgpu_kernel void @add_i64_constant(ptr addrspace(1) %out, ptr addrspace
18601860
; GFX1232-LABEL: add_i64_constant:
18611861
; GFX1232: ; %bb.0: ; %entry
18621862
; GFX1232-NEXT: s_load_b128 s[0:3], s[4:5], 0x24
1863-
; GFX1232-NEXT: s_mov_b32 s7, exec_lo
1864-
; GFX1232-NEXT: s_mov_b32 s5, 0
1865-
; GFX1232-NEXT: v_mbcnt_lo_u32_b32 v2, s7, 0
18661863
; GFX1232-NEXT: s_mov_b32 s6, exec_lo
1864+
; GFX1232-NEXT: s_mov_b32 s4, exec_lo
1865+
; GFX1232-NEXT: v_mbcnt_lo_u32_b32 v2, s6, 0
18671866
; GFX1232-NEXT: ; implicit-def: $vgpr0_vgpr1
18681867
; GFX1232-NEXT: s_delay_alu instid0(VALU_DEP_1)
18691868
; GFX1232-NEXT: v_cmpx_eq_u32_e32 0, v2
18701869
; GFX1232-NEXT: s_cbranch_execz .LBB3_2
18711870
; GFX1232-NEXT: ; %bb.1:
1872-
; GFX1232-NEXT: s_bcnt1_i32_b32 s4, s7
1871+
; GFX1232-NEXT: s_bcnt1_i32_b32 s5, s6
18731872
; GFX1232-NEXT: s_mov_b32 s11, 0x31016000
1874-
; GFX1232-NEXT: s_mul_u64 s[4:5], s[4:5], 5
1873+
; GFX1232-NEXT: s_mul_i32 s5, s5, 5
18751874
; GFX1232-NEXT: s_mov_b32 s10, -1
1876-
; GFX1232-NEXT: v_dual_mov_b32 v0, s4 :: v_dual_mov_b32 v1, s5
1875+
; GFX1232-NEXT: v_dual_mov_b32 v0, s5 :: v_dual_mov_b32 v1, 0
18771876
; GFX1232-NEXT: s_wait_kmcnt 0x0
18781877
; GFX1232-NEXT: s_mov_b32 s8, s2
18791878
; GFX1232-NEXT: s_mov_b32 s9, s3
18801879
; GFX1232-NEXT: buffer_atomic_add_u64 v[0:1], off, s[8:11], null th:TH_ATOMIC_RETURN scope:SCOPE_DEV
18811880
; GFX1232-NEXT: s_wait_loadcnt 0x0
18821881
; GFX1232-NEXT: global_inv scope:SCOPE_DEV
18831882
; GFX1232-NEXT: .LBB3_2:
1884-
; GFX1232-NEXT: s_wait_alu 0xfffe
1885-
; GFX1232-NEXT: s_or_b32 exec_lo, exec_lo, s6
1883+
; GFX1232-NEXT: s_or_b32 exec_lo, exec_lo, s4
18861884
; GFX1232-NEXT: s_wait_kmcnt 0x0
18871885
; GFX1232-NEXT: v_readfirstlane_b32 s3, v1
18881886
; GFX1232-NEXT: v_readfirstlane_b32 s2, v0
@@ -5372,22 +5370,22 @@ define amdgpu_kernel void @sub_i64_constant(ptr addrspace(1) %out, ptr addrspace
53725370
; GFX1264: ; %bb.0: ; %entry
53735371
; GFX1264-NEXT: s_load_b128 s[0:3], s[4:5], 0x24
53745372
; GFX1264-NEXT: s_mov_b64 s[6:7], exec
5375-
; GFX1264-NEXT: s_mov_b32 s9, 0
5376-
; GFX1264-NEXT: v_mbcnt_lo_u32_b32 v0, s6, 0
53775373
; GFX1264-NEXT: s_mov_b64 s[4:5], exec
5374+
; GFX1264-NEXT: v_mbcnt_lo_u32_b32 v0, s6, 0
53785375
; GFX1264-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
53795376
; GFX1264-NEXT: v_mbcnt_hi_u32_b32 v2, s7, v0
53805377
; GFX1264-NEXT: ; implicit-def: $vgpr0_vgpr1
53815378
; GFX1264-NEXT: v_cmpx_eq_u32_e32 0, v2
53825379
; GFX1264-NEXT: s_cbranch_execz .LBB9_2
53835380
; GFX1264-NEXT: ; %bb.1:
5384-
; GFX1264-NEXT: s_bcnt1_i32_b64 s8, s[6:7]
5381+
; GFX1264-NEXT: s_bcnt1_i32_b64 s6, s[6:7]
5382+
; GFX1264-NEXT: v_mov_b32_e32 v1, 0
5383+
; GFX1264-NEXT: s_wait_alu 0xfffe
5384+
; GFX1264-NEXT: s_mul_i32 s6, s6, 5
53855385
; GFX1264-NEXT: s_mov_b32 s11, 0x31016000
5386-
; GFX1264-NEXT: s_mul_u64 s[6:7], s[8:9], 5
5387-
; GFX1264-NEXT: s_mov_b32 s10, -1
53885386
; GFX1264-NEXT: s_wait_alu 0xfffe
53895387
; GFX1264-NEXT: v_mov_b32_e32 v0, s6
5390-
; GFX1264-NEXT: v_mov_b32_e32 v1, s7
5388+
; GFX1264-NEXT: s_mov_b32 s10, -1
53915389
; GFX1264-NEXT: s_wait_kmcnt 0x0
53925390
; GFX1264-NEXT: s_mov_b32 s8, s2
53935391
; GFX1264-NEXT: s_mov_b32 s9, s3
@@ -5412,29 +5410,27 @@ define amdgpu_kernel void @sub_i64_constant(ptr addrspace(1) %out, ptr addrspace
54125410
; GFX1232-LABEL: sub_i64_constant:
54135411
; GFX1232: ; %bb.0: ; %entry
54145412
; GFX1232-NEXT: s_load_b128 s[0:3], s[4:5], 0x24
5415-
; GFX1232-NEXT: s_mov_b32 s7, exec_lo
5416-
; GFX1232-NEXT: s_mov_b32 s5, 0
5417-
; GFX1232-NEXT: v_mbcnt_lo_u32_b32 v2, s7, 0
54185413
; GFX1232-NEXT: s_mov_b32 s6, exec_lo
5414+
; GFX1232-NEXT: s_mov_b32 s4, exec_lo
5415+
; GFX1232-NEXT: v_mbcnt_lo_u32_b32 v2, s6, 0
54195416
; GFX1232-NEXT: ; implicit-def: $vgpr0_vgpr1
54205417
; GFX1232-NEXT: s_delay_alu instid0(VALU_DEP_1)
54215418
; GFX1232-NEXT: v_cmpx_eq_u32_e32 0, v2
54225419
; GFX1232-NEXT: s_cbranch_execz .LBB9_2
54235420
; GFX1232-NEXT: ; %bb.1:
5424-
; GFX1232-NEXT: s_bcnt1_i32_b32 s4, s7
5421+
; GFX1232-NEXT: s_bcnt1_i32_b32 s5, s6
54255422
; GFX1232-NEXT: s_mov_b32 s11, 0x31016000
5426-
; GFX1232-NEXT: s_mul_u64 s[4:5], s[4:5], 5
5423+
; GFX1232-NEXT: s_mul_i32 s5, s5, 5
54275424
; GFX1232-NEXT: s_mov_b32 s10, -1
5428-
; GFX1232-NEXT: v_dual_mov_b32 v0, s4 :: v_dual_mov_b32 v1, s5
5425+
; GFX1232-NEXT: v_dual_mov_b32 v0, s5 :: v_dual_mov_b32 v1, 0
54295426
; GFX1232-NEXT: s_wait_kmcnt 0x0
54305427
; GFX1232-NEXT: s_mov_b32 s8, s2
54315428
; GFX1232-NEXT: s_mov_b32 s9, s3
54325429
; GFX1232-NEXT: buffer_atomic_sub_u64 v[0:1], off, s[8:11], null th:TH_ATOMIC_RETURN scope:SCOPE_DEV
54335430
; GFX1232-NEXT: s_wait_loadcnt 0x0
54345431
; GFX1232-NEXT: global_inv scope:SCOPE_DEV
54355432
; GFX1232-NEXT: .LBB9_2:
5436-
; GFX1232-NEXT: s_wait_alu 0xfffe
5437-
; GFX1232-NEXT: s_or_b32 exec_lo, exec_lo, s6
5433+
; GFX1232-NEXT: s_or_b32 exec_lo, exec_lo, s4
54385434
; GFX1232-NEXT: s_wait_kmcnt 0x0
54395435
; GFX1232-NEXT: v_readfirstlane_b32 s2, v0
54405436
; GFX1232-NEXT: v_mul_u32_u24_e32 v0, 5, v2

llvm/test/CodeGen/AMDGPU/memcpy-crash-issue63986.ll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,13 @@ define void @issue63986_reduced_expanded(i64 %idxprom) {
170170
; CHECK-NEXT: s_cbranch_execnz .LBB1_8
171171
; CHECK-NEXT: .LBB1_5: ; %loop-memcpy-residual.preheader
172172
; CHECK-NEXT: v_mov_b32_e32 v0, s4
173-
; CHECK-NEXT: s_mov_b64 s[6:7], 0
173+
; CHECK-NEXT: s_mov_b64 s[8:9], 0
174+
; CHECK-NEXT: s_mov_b32 s7, 0
174175
; CHECK-NEXT: v_mov_b32_e32 v1, s5
175176
; CHECK-NEXT: .LBB1_6: ; %loop-memcpy-residual
176-
; CHECK-NEXT: s_add_u32 s4, s6, 1
177-
; CHECK-NEXT: s_addc_u32 s5, s7, 0
178-
; CHECK-NEXT: v_cmp_lt_u64_e32 vcc, s[4:5], v[0:1]
179-
; CHECK-NEXT: s_mov_b64 s[6:7], 1
177+
; CHECK-NEXT: s_add_i32 s6, s8, 1
178+
; CHECK-NEXT: v_cmp_lt_u64_e32 vcc, s[6:7], v[0:1]
179+
; CHECK-NEXT: s_mov_b64 s[8:9], 1
180180
; CHECK-NEXT: s_cbranch_vccnz .LBB1_6
181181
; CHECK-NEXT: ; %bb.7: ; %Flow
182182
; CHECK-NEXT: v_mov_b32_e32 v0, 0

0 commit comments

Comments
 (0)