[SeparateConstOffsetFromGEP] Don't set unsound inbounds flag #130616
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-globalisel Author: Fabian Ritter (ritter-x2a) ChangesThe language reference says about inbounds geps that "if the If (gep inbounds p, (a + 5)) is translated to (gep [inbounds] (gep p, We might want to use ValueTracking to check if a is known to be For the AMDGPU tests with scratch instructions, removing the unsound For SWDEV-516125. Patch is 100.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130616.diff 14 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index 319b9e14fc21d..138a71ce79cef 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -1092,7 +1092,7 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
// is transformed to:
//
// addr2 = gep float, float* p, i64 a ; inbounds removed
- // addr = gep inbounds float, float* addr2, i64 5
+ // addr = gep float, float* addr2, i64 5 ; inbounds removed
//
// If a is -4, although the old index b is in bounds, the new index a is
// off-bound. http://llvm.org/docs/LangRef.html#id181 says "if the
@@ -1103,7 +1103,7 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
// TODO(jingyue): do some range analysis to keep as many inbounds as
// possible. GEPs with inbounds are more friendly to alias analysis.
// TODO(gep_nowrap): Preserve nuw at least.
- bool GEPWasInBounds = GEP->isInBounds();
+ auto NewGEPFlags = GEPNoWrapFlags::none();
GEP->setNoWrapFlags(GEPNoWrapFlags::none());
// Lowers a GEP to either GEPs with a single index or arithmetic operations.
@@ -1153,7 +1153,7 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
IRBuilder<> Builder(GEP);
NewGEP = cast<Instruction>(Builder.CreatePtrAdd(
NewGEP, ConstantInt::get(PtrIdxTy, AccumulativeByteOffset, true),
- GEP->getName(), GEPWasInBounds));
+ GEP->getName(), NewGEPFlags));
NewGEP->copyMetadata(*GEP);
GEP->replaceAllUsesWith(NewGEP);
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/flat-scratch.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/flat-scratch.ll
index a02e0b37479a0..3460d0db6a3ba 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/flat-scratch.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/flat-scratch.ll
@@ -179,12 +179,15 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; GFX9-NEXT: s_add_u32 flat_scratch_lo, s8, s13
; GFX9-NEXT: s_addc_u32 flat_scratch_hi, s9, 0
; GFX9-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; GFX9-NEXT: v_mov_b32_e32 v2, 15
+; GFX9-NEXT: v_mov_b32_e32 v3, 15
; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
-; GFX9-NEXT: scratch_store_dword v1, v2, off
+; GFX9-NEXT: v_mov_b32_e32 v2, 0
+; GFX9-NEXT: scratch_store_dword v1, v3, off
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc
+; GFX9-NEXT: v_mov_b32_e32 v1, 0x7c
+; GFX9-NEXT: v_add3_u32 v0, v2, v0, v1
+; GFX9-NEXT: scratch_load_dword v0, v0, off glc
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: s_endpgm
;
@@ -195,12 +198,14 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_LO), s8
; GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_HI), s9
; GFX10-NEXT: v_sub_nc_u32_e32 v1, 0, v0
+; GFX10-NEXT: v_mov_b32_e32 v2, 0x7c
; GFX10-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX10-NEXT: v_mov_b32_e32 v2, 15
+; GFX10-NEXT: v_mov_b32_e32 v3, 15
; GFX10-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; GFX10-NEXT: scratch_store_dword v0, v2, off
+; GFX10-NEXT: scratch_store_dword v0, v3, off
; GFX10-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX10-NEXT: scratch_load_dword v0, v1, off offset:124 glc dlc
+; GFX10-NEXT: v_add3_u32 v1, 0, v1, v2
+; GFX10-NEXT: scratch_load_dword v0, v1, off glc dlc
; GFX10-NEXT: s_waitcnt vmcnt(0)
; GFX10-NEXT: s_endpgm
;
@@ -213,21 +218,25 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; GFX942-NEXT: scratch_store_dword v1, v2, off sc0 sc1
; GFX942-NEXT: s_waitcnt vmcnt(0)
; GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX942-NEXT: scratch_load_dword v0, v0, off offset:124 sc0 sc1
+; GFX942-NEXT: v_mov_b32_e32 v1, 0
+; GFX942-NEXT: v_mov_b32_e32 v2, 0x7c
+; GFX942-NEXT: v_add3_u32 v0, v1, v0, v2
+; GFX942-NEXT: scratch_load_dword v0, v0, off sc0 sc1
; GFX942-NEXT: s_waitcnt vmcnt(0)
; GFX942-NEXT: s_endpgm
;
; GFX11-LABEL: store_load_vindex_kernel:
; GFX11: ; %bb.0: ; %bb
-; GFX11-NEXT: v_and_b32_e32 v0, 0x3ff, v0
-; GFX11-NEXT: v_mov_b32_e32 v2, 15
+; GFX11-NEXT: v_dual_mov_b32 v3, 15 :: v_dual_and_b32 v0, 0x3ff, v0
+; GFX11-NEXT: v_mov_b32_e32 v2, 0x7c
; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_2)
; GFX11-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; GFX11-NEXT: v_lshlrev_b32_e32 v0, 2, v0
; GFX11-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; GFX11-NEXT: scratch_store_b32 v0, v2, off dlc
+; GFX11-NEXT: scratch_store_b32 v0, v3, off dlc
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: scratch_load_b32 v0, v1, off offset:124 glc dlc
+; GFX11-NEXT: v_add3_u32 v1, 0, v1, v2
+; GFX11-NEXT: scratch_load_b32 v0, v1, off glc dlc
; GFX11-NEXT: s_waitcnt vmcnt(0)
; GFX11-NEXT: s_endpgm
;
@@ -249,12 +258,15 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; UNALIGNED_GFX9-NEXT: s_add_u32 flat_scratch_lo, s8, s13
; UNALIGNED_GFX9-NEXT: s_addc_u32 flat_scratch_hi, s9, 0
; UNALIGNED_GFX9-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v2, 15
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v3, 15
; UNALIGNED_GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
-; UNALIGNED_GFX9-NEXT: scratch_store_dword v1, v2, off
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v2, 0
+; UNALIGNED_GFX9-NEXT: scratch_store_dword v1, v3, off
; UNALIGNED_GFX9-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v1, 0x7c
+; UNALIGNED_GFX9-NEXT: v_add3_u32 v0, v2, v0, v1
+; UNALIGNED_GFX9-NEXT: scratch_load_dword v0, v0, off glc
; UNALIGNED_GFX9-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX9-NEXT: s_endpgm
;
@@ -265,12 +277,14 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; UNALIGNED_GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_LO), s8
; UNALIGNED_GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_HI), s9
; UNALIGNED_GFX10-NEXT: v_sub_nc_u32_e32 v1, 0, v0
+; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v2, 0x7c
; UNALIGNED_GFX10-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v2, 15
+; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v3, 15
; UNALIGNED_GFX10-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; UNALIGNED_GFX10-NEXT: scratch_store_dword v0, v2, off
+; UNALIGNED_GFX10-NEXT: scratch_store_dword v0, v3, off
; UNALIGNED_GFX10-NEXT: s_waitcnt_vscnt null, 0x0
-; UNALIGNED_GFX10-NEXT: scratch_load_dword v0, v1, off offset:124 glc dlc
+; UNALIGNED_GFX10-NEXT: v_add3_u32 v1, 0, v1, v2
+; UNALIGNED_GFX10-NEXT: scratch_load_dword v0, v1, off glc dlc
; UNALIGNED_GFX10-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX10-NEXT: s_endpgm
;
@@ -283,21 +297,25 @@ define amdgpu_kernel void @store_load_vindex_kernel() {
; UNALIGNED_GFX942-NEXT: scratch_store_dword v1, v2, off sc0 sc1
; UNALIGNED_GFX942-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX942-NEXT: scratch_load_dword v0, v0, off offset:124 sc0 sc1
+; UNALIGNED_GFX942-NEXT: v_mov_b32_e32 v1, 0
+; UNALIGNED_GFX942-NEXT: v_mov_b32_e32 v2, 0x7c
+; UNALIGNED_GFX942-NEXT: v_add3_u32 v0, v1, v0, v2
+; UNALIGNED_GFX942-NEXT: scratch_load_dword v0, v0, off sc0 sc1
; UNALIGNED_GFX942-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX942-NEXT: s_endpgm
;
; UNALIGNED_GFX11-LABEL: store_load_vindex_kernel:
; UNALIGNED_GFX11: ; %bb.0: ; %bb
-; UNALIGNED_GFX11-NEXT: v_and_b32_e32 v0, 0x3ff, v0
-; UNALIGNED_GFX11-NEXT: v_mov_b32_e32 v2, 15
+; UNALIGNED_GFX11-NEXT: v_dual_mov_b32 v3, 15 :: v_dual_and_b32 v0, 0x3ff, v0
+; UNALIGNED_GFX11-NEXT: v_mov_b32_e32 v2, 0x7c
; UNALIGNED_GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_2)
; UNALIGNED_GFX11-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; UNALIGNED_GFX11-NEXT: v_lshlrev_b32_e32 v0, 2, v0
; UNALIGNED_GFX11-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; UNALIGNED_GFX11-NEXT: scratch_store_b32 v0, v2, off dlc
+; UNALIGNED_GFX11-NEXT: scratch_store_b32 v0, v3, off dlc
; UNALIGNED_GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; UNALIGNED_GFX11-NEXT: scratch_load_b32 v0, v1, off offset:124 glc dlc
+; UNALIGNED_GFX11-NEXT: v_add3_u32 v1, 0, v1, v2
+; UNALIGNED_GFX11-NEXT: scratch_load_b32 v0, v1, off glc dlc
; UNALIGNED_GFX11-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX11-NEXT: s_endpgm
;
@@ -788,14 +806,16 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; GFX9-NEXT: scratch_load_dword v1, off, s0 glc
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
; GFX9-NEXT: v_add_u32_e32 v1, 0x100, v1
-; GFX9-NEXT: v_mov_b32_e32 v2, 15
-; GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX9-NEXT: scratch_store_dword v1, v2, off
+; GFX9-NEXT: v_mov_b32_e32 v3, 15
+; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
+; GFX9-NEXT: v_mov_b32_e32 v2, 0x100
+; GFX9-NEXT: scratch_store_dword v1, v3, off
; GFX9-NEXT: s_waitcnt vmcnt(0)
-; GFX9-NEXT: v_add_u32_e32 v0, 0x100, v0
-; GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc
+; GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; GFX9-NEXT: v_mov_b32_e32 v1, 0x7c
+; GFX9-NEXT: v_add3_u32 v0, v2, v0, v1
+; GFX9-NEXT: scratch_load_dword v0, v0, off glc
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: s_endpgm
;
@@ -807,15 +827,16 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_HI), s9
; GFX10-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; GFX10-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX10-NEXT: v_mov_b32_e32 v2, 15
-; GFX10-NEXT: scratch_load_dword v3, off, off glc dlc
-; GFX10-NEXT: s_waitcnt vmcnt(0)
+; GFX10-NEXT: v_mov_b32_e32 v2, 0x7c
+; GFX10-NEXT: v_mov_b32_e32 v3, 15
; GFX10-NEXT: v_lshlrev_b32_e32 v1, 2, v1
; GFX10-NEXT: v_add_nc_u32_e32 v0, 0x100, v0
-; GFX10-NEXT: v_add_nc_u32_e32 v1, 0x100, v1
-; GFX10-NEXT: scratch_store_dword v0, v2, off
+; GFX10-NEXT: v_add3_u32 v1, 0x100, v1, v2
+; GFX10-NEXT: scratch_load_dword v2, off, off glc dlc
+; GFX10-NEXT: s_waitcnt vmcnt(0)
+; GFX10-NEXT: scratch_store_dword v0, v3, off
; GFX10-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX10-NEXT: scratch_load_dword v0, v1, off offset:124 glc dlc
+; GFX10-NEXT: scratch_load_dword v0, v1, off glc dlc
; GFX10-NEXT: s_waitcnt vmcnt(0)
; GFX10-NEXT: s_endpgm
;
@@ -825,30 +846,33 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; GFX942-NEXT: s_waitcnt vmcnt(0)
; GFX942-NEXT: v_and_b32_e32 v0, 0x3ff, v0
; GFX942-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; GFX942-NEXT: v_sub_u32_e32 v0, 0, v0
; GFX942-NEXT: v_mov_b32_e32 v2, 15
-; GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; GFX942-NEXT: v_sub_u32_e32 v0, 0, v0
; GFX942-NEXT: scratch_store_dword v1, v2, off offset:256 sc0 sc1
; GFX942-NEXT: s_waitcnt vmcnt(0)
-; GFX942-NEXT: v_add_u32_e32 v0, 0x100, v0
-; GFX942-NEXT: scratch_load_dword v0, v0, off offset:124 sc0 sc1
+; GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; GFX942-NEXT: v_mov_b32_e32 v1, 0x100
+; GFX942-NEXT: v_mov_b32_e32 v2, 0x7c
+; GFX942-NEXT: v_add3_u32 v0, v1, v0, v2
+; GFX942-NEXT: scratch_load_dword v0, v0, off sc0 sc1
; GFX942-NEXT: s_waitcnt vmcnt(0)
; GFX942-NEXT: s_endpgm
;
; GFX11-LABEL: store_load_vindex_small_offset_kernel:
; GFX11: ; %bb.0: ; %bb
-; GFX11-NEXT: v_and_b32_e32 v0, 0x3ff, v0
-; GFX11-NEXT: scratch_load_b32 v3, off, off glc dlc
-; GFX11-NEXT: s_waitcnt vmcnt(0)
-; GFX11-NEXT: v_mov_b32_e32 v2, 15
+; GFX11-NEXT: v_dual_mov_b32 v3, 15 :: v_dual_and_b32 v0, 0x3ff, v0
+; GFX11-NEXT: v_mov_b32_e32 v2, 0x7c
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_2)
; GFX11-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; GFX11-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2)
; GFX11-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; GFX11-NEXT: scratch_store_b32 v0, v2, off offset:256 dlc
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_add3_u32 v1, 0x100, v1, v2
+; GFX11-NEXT: scratch_load_b32 v2, off, off glc dlc
+; GFX11-NEXT: s_waitcnt vmcnt(0)
+; GFX11-NEXT: scratch_store_b32 v0, v3, off offset:256 dlc
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_add_nc_u32_e32 v1, 0x100, v1
-; GFX11-NEXT: scratch_load_b32 v0, v1, off offset:124 glc dlc
+; GFX11-NEXT: scratch_load_b32 v0, v1, off glc dlc
; GFX11-NEXT: s_waitcnt vmcnt(0)
; GFX11-NEXT: s_endpgm
;
@@ -875,14 +899,16 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; UNALIGNED_GFX9-NEXT: scratch_load_dword v1, off, s0 glc
; UNALIGNED_GFX9-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX9-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; UNALIGNED_GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
; UNALIGNED_GFX9-NEXT: v_add_u32_e32 v1, 0x100, v1
-; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v2, 15
-; UNALIGNED_GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX9-NEXT: scratch_store_dword v1, v2, off
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v3, 15
+; UNALIGNED_GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v2, 0x100
+; UNALIGNED_GFX9-NEXT: scratch_store_dword v1, v3, off
; UNALIGNED_GFX9-NEXT: s_waitcnt vmcnt(0)
-; UNALIGNED_GFX9-NEXT: v_add_u32_e32 v0, 0x100, v0
-; UNALIGNED_GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc
+; UNALIGNED_GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; UNALIGNED_GFX9-NEXT: v_mov_b32_e32 v1, 0x7c
+; UNALIGNED_GFX9-NEXT: v_add3_u32 v0, v2, v0, v1
+; UNALIGNED_GFX9-NEXT: scratch_load_dword v0, v0, off glc
; UNALIGNED_GFX9-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX9-NEXT: s_endpgm
;
@@ -894,15 +920,16 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; UNALIGNED_GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_HI), s9
; UNALIGNED_GFX10-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; UNALIGNED_GFX10-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v2, 15
-; UNALIGNED_GFX10-NEXT: scratch_load_dword v3, off, off glc dlc
-; UNALIGNED_GFX10-NEXT: s_waitcnt vmcnt(0)
+; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v2, 0x7c
+; UNALIGNED_GFX10-NEXT: v_mov_b32_e32 v3, 15
; UNALIGNED_GFX10-NEXT: v_lshlrev_b32_e32 v1, 2, v1
; UNALIGNED_GFX10-NEXT: v_add_nc_u32_e32 v0, 0x100, v0
-; UNALIGNED_GFX10-NEXT: v_add_nc_u32_e32 v1, 0x100, v1
-; UNALIGNED_GFX10-NEXT: scratch_store_dword v0, v2, off
+; UNALIGNED_GFX10-NEXT: v_add3_u32 v1, 0x100, v1, v2
+; UNALIGNED_GFX10-NEXT: scratch_load_dword v2, off, off glc dlc
+; UNALIGNED_GFX10-NEXT: s_waitcnt vmcnt(0)
+; UNALIGNED_GFX10-NEXT: scratch_store_dword v0, v3, off
; UNALIGNED_GFX10-NEXT: s_waitcnt_vscnt null, 0x0
-; UNALIGNED_GFX10-NEXT: scratch_load_dword v0, v1, off offset:124 glc dlc
+; UNALIGNED_GFX10-NEXT: scratch_load_dword v0, v1, off glc dlc
; UNALIGNED_GFX10-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX10-NEXT: s_endpgm
;
@@ -912,30 +939,33 @@ define amdgpu_kernel void @store_load_vindex_small_offset_kernel() {
; UNALIGNED_GFX942-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX942-NEXT: v_and_b32_e32 v0, 0x3ff, v0
; UNALIGNED_GFX942-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; UNALIGNED_GFX942-NEXT: v_sub_u32_e32 v0, 0, v0
; UNALIGNED_GFX942-NEXT: v_mov_b32_e32 v2, 15
-; UNALIGNED_GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; UNALIGNED_GFX942-NEXT: v_sub_u32_e32 v0, 0, v0
; UNALIGNED_GFX942-NEXT: scratch_store_dword v1, v2, off offset:256 sc0 sc1
; UNALIGNED_GFX942-NEXT: s_waitcnt vmcnt(0)
-; UNALIGNED_GFX942-NEXT: v_add_u32_e32 v0, 0x100, v0
-; UNALIGNED_GFX942-NEXT: scratch_load_dword v0, v0, off offset:124 sc0 sc1
+; UNALIGNED_GFX942-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; UNALIGNED_GFX942-NEXT: v_mov_b32_e32 v1, 0x100
+; UNALIGNED_GFX942-NEXT: v_mov_b32_e32 v2, 0x7c
+; UNALIGNED_GFX942-NEXT: v_add3_u32 v0, v1, v0, v2
+; UNALIGNED_GFX942-NEXT: scratch_load_dword v0, v0, off sc0 sc1
; UNALIGNED_GFX942-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX942-NEXT: s_endpgm
;
; UNALIGNED_GFX11-LABEL: store_load_vindex_small_offset_kernel:
; UNALIGNED_GFX11: ; %bb.0: ; %bb
-; UNALIGNED_GFX11-NEXT: v_and_b32_e32 v0, 0x3ff, v0
-; UNALIGNED_GFX11-NEXT: scratch_load_b32 v3, off, off glc dlc
-; UNALIGNED_GFX11-NEXT: s_waitcnt vmcnt(0)
-; UNALIGNED_GFX11-NEXT: v_mov_b32_e32 v2, 15
+; UNALIGNED_GFX11-NEXT: v_dual_mov_b32 v3, 15 :: v_dual_and_b32 v0, 0x3ff, v0
+; UNALIGNED_GFX11-NEXT: v_mov_b32_e32 v2, 0x7c
+; UNALIGNED_GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_2)
; UNALIGNED_GFX11-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; UNALIGNED_GFX11-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; UNALIGNED_GFX11-NEXT: s_delay_alu instid0(VALU_DEP_2)
; UNALIGNED_GFX11-NEXT: v_lshlrev_b32_e32 v1, 2, v1
-; UNALIGNED_GFX11-NEXT: scratch_store_b32 v0, v2, off offset:256 dlc
+; UNALIGNED_GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; UNALIGNED_GFX11-NEXT: v_add3_u32 v1, 0x100, v1, v2
+; UNALIGNED_GFX11-NEXT: scratch_load_b32 v2, off, off glc dlc
+; UNALIGNED_GFX11-NEXT: s_waitcnt vmcnt(0)
+; UNALIGNED_GFX11-NEXT: scratch_store_b32 v0, v3, off offset:256 dlc
; UNALIGNED_GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; UNALIGNED_GFX11-NEXT: v_add_nc_u32_e32 v1, 0x100, v1
-; UNALIGNED_GFX11-NEXT: scratch_load_b32 v0, v1, off offset:124 glc dlc
+; UNALIGNED_GFX11-NEXT: scratch_load_b32 v0, v1, off glc dlc
; UNALIGNED_GFX11-NEXT: s_waitcnt vmcnt(0)
; UNALIGNED_GFX11-NEXT: s_endpgm
;
@@ -1377,14 +1407,16 @@ define amdgpu_kernel void @store_load_vindex_large_offset_kernel() {
; GFX9-NEXT: scratch_load_dword v1, off, s0 offset:4 glc
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
; GFX9-NEXT: v_add_u32_e32 v1, 0x4004, v1
-; GFX9-NEXT: v_mov_b32_e32 v2, 15
-; GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX9-NEXT: scratch_store_dword v1, v2, off
+; GFX9-NEXT: v_mov_b32_e32 v3, 15
+; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0
+; GFX9-NEXT: v_mov_b32_e32 v2, 0x4004
+; GFX9-NEXT: scratch_store_dword v1, v3, off
; GFX9-NEXT: s_waitcnt vmcnt(0)
-; GFX9-NEXT: v_add_u32_e32 v0, 0x4004, v0
-; GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc
+; GFX9-NEXT: v_lshlrev_b32_e32 v0, 2, v0
+; GFX9-NEXT: v_mov_b32_e32 v1, 0x7c
+; GFX9-NEXT: v_add3_u32 v0, v2, v0, v1
+; GFX9-NEXT: scratch_load_dword v0, v0, off glc
; GFX9-NEXT: s_waitcnt vmcnt(0)
; GFX9-NEXT: s_endpgm
;
@@ -1396,15 +1428,16 @@ define amdgpu_kernel void @store_load_vindex_large_offset_kernel() {
; GFX10-NEXT: s_setreg_b32 hwreg(HW_REG_FLAT_SCR_HI), s9
; GFX10-NEXT: v_sub_nc_u32_e32 v1, 0, v0
; GFX10-NEXT: v_lshlrev_b32_e32 v0, 2, v0
-; GFX10-NEXT: v_mov_b32_e32 v2, 15
-; GFX10-NEXT: scratch_load_dword v3, off, off offset:4 glc dlc
-; GFX10-NEXT: s_waitcnt vmcnt(0)
+; GFX10-NEXT: v_mov_b32_e32 v2, 0x7c
+; GFX10-NEXT: v_mov_b32_e32 v3, 15
; GFX10-NEXT: v_lshlrev_b32_e32 v1, 2, v1
; GFX10-NEXT: v_add_nc_u32_e32 v0, 0x4004, v0
-; GFX10-NEXT: v_add_nc_u32_e32 v1, 0x4004, v1
-; GFX10-NEXT: scratch_store_dword v0, v2, off
+; GFX10-NEXT: v_add3_u32 v1, 0x4004, v1, v2
+; GFX10-NEXT: scratch_load_dword v2, off, off offset:4 glc dlc
+; GFX10-NEXT: s_waitcnt vmcnt(0)
+; GFX10-NEXT: scratch_store_dword v0, v3, off
; GFX10-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX10-NEXT: scratch_load_dword v0, v1, off offset:124 glc dlc
+; GFX10-NEXT: scratch_load_dword v0, v1, off glc dlc
; GFX10-NEXT: s_waitcnt vmcnt(0)
; GFX10-NEXT: s_endpgm
;
@@ -1414,32 +1447,35 @@ define amdgpu_kernel void @store_load_vindex_large_offset_kernel() {
; GFX942-NEXT: s_waitcnt vmcnt(0)
; GFX942-NEXT: v_and_b32_e32 v0, 0x3ff, v0
; GFX942-NEXT: v_lshlrev_b32_e32 v1, 2, v0
-; GFX942-NEXT: v_sub_u32_e32 v0, 0, v0
; GFX942-NEXT: v_mov_b32_e32 v2, 15
...
[truncated]
|
|
Here is an alive2 counter example for the transformation as it was before this PR is applied, on the test in preserve-inbounds.ll: https://alive2.llvm.org/ce/z/2unNTZ |
arsenm
reviewed
Mar 11, 2025
| ; GFX9-NEXT: scratch_store_dword v1, v2, off | ||
| ; GFX9-NEXT: s_waitcnt vmcnt(0) | ||
| ; GFX9-NEXT: v_sub_u32_e32 v0, 0, v0 | ||
| ; GFX9-NEXT: scratch_load_dword v0, v0, off offset:124 glc |
There was a problem hiding this comment.
Change to preserve offset fold
There was a problem hiding this comment.
Isn't that offset actually just wrong here (at least pre-gfx12)? The v_sub computes -workitemid*4 in v0, which is negative (except for 0), whereas the hardware wants an unsigned vgpr_offset.
There was a problem hiding this comment.
Wrong or not, this patch is not a solution if this was an incorrect addressing mode fold
There was a problem hiding this comment.
Adjusted this test and the global ISel version by manually applying the unsound transformation and increasing alloca sizes so that the tests are not statically guaranteed to show UB.
There was a problem hiding this comment.
Should come back to check this matching though
arsenm
reviewed
Mar 14, 2025
ritter-x2a
force-pushed
the
users/ritter-x2a/03-10-_separateconstoffsetfromgep_don_t_set_unsound_inbounds_flag
branch
from
54f5ec1 to
07a9924
Compare
|
Rebased to resolve merge conflicts. |
arsenm
approved these changes
Mar 18, 2025
The language reference says about inbounds geps that "if the getelementptr has any non-zero indices[...] [t]he base pointer has an in bounds address of the allocated object that it is based on [and] [d]uring the successive addition of offsets to the address, the resulting pointer must remain in bounds of the allocated object at each step." If (gep inbounds p, (a + 5)) is translated to (gep [inbounds] (gep p, a), 5) with p pointing to the beginning of an object and a=-4, as the example in the comments suggests, that's the case for neither of the resulting geps. Therefore, we need to clear the inbounds flag for both geps. We might want to use ValueTracking to check if a is known to be non-negative to preserve the inbounds flags. For the AMDGPU tests with scratch instructions, removing the unsound inbounds flag means that AMDGPUDAGToDAGISel::isFlatScratchBaseLegal sees no NUW flag at the pointer add, which prevents generation of scratch instructions with immediate offsets. It's not clear to me what test/CodeGen/AMDGPU/constant-address-space-32bit.ll tests and if my fix preserves that. For SWDEV-516125.
...where they were folded previously, by manually applying the unsound version of the transformation and, where necessary, increasing the allocation size so that the transformed code is not statically guaranteed to cause UB.
…nbounds flag on offset folding.
…ive names, use MUBUF&FLATSCR.
ritter-x2a
force-pushed
the
users/ritter-x2a/03-10-_separateconstoffsetfromgep_don_t_set_unsound_inbounds_flag
branch
from
07a9924 to
92a516e
Compare
ritter-x2a
deleted the
users/ritter-x2a/03-10-_separateconstoffsetfromgep_don_t_set_unsound_inbounds_flag
branch
This was referenced Mar 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The language reference says about inbounds geps that "if the
getelementptr has any non-zero indices[...] [t]he base pointer has an in
bounds address of the allocated object that it is based on [and]
[d]uring the successive addition of offsets to the address, the
resulting pointer must remain in bounds of the allocated object at each
step."
If (gep inbounds p, (a + 5)) is translated to (gep [inbounds] (gep p,
a), 5) with p pointing to the beginning of an object and a=-4, as the
example in the comments suggests, that's the case for neither of the
resulting geps. Therefore, we need to clear the inbounds flag for both
geps.
We might want to use ValueTracking to check if a is known to be
non-negative to preserve the inbounds flags.
For the AMDGPU tests with scratch instructions, removing the unsound
inbounds flag means that AMDGPUDAGToDAGISel::isFlatScratchBaseLegal sees
no NUW flag at the pointer add, which prevents generation of scratch
instructions with immediate offsets.
For SWDEV-516125.