Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMDGPU: Fix inst-selection of large scratch offsets with sgpr base #110256

Merged

Conversation

petar-avramovic
Copy link
Collaborator

Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.

Copy link
Collaborator Author

petar-avramovic commented Sep 27, 2024

@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Petar Avramovic (petar-avramovic)

Changes

Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.


Full diff: https://github.com/llvm/llvm-project/pull/110256.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp (+1-1)
  • (modified) llvm/test/CodeGen/AMDGPU/flat-scratch.ll (+2-2)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
index d3d5bc924525fc..48971a6840c779 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
@@ -1911,7 +1911,7 @@ bool AMDGPUDAGToDAGISel::SelectScratchSAddr(SDNode *Parent, SDValue Addr,
                     0);
   }
 
-  Offset = CurDAG->getTargetConstant(COffsetVal, DL, MVT::i16);
+  Offset = CurDAG->getTargetConstant(COffsetVal, DL, MVT::i32);
 
   return true;
 }
diff --git a/llvm/test/CodeGen/AMDGPU/flat-scratch.ll b/llvm/test/CodeGen/AMDGPU/flat-scratch.ll
index 667a8a38c62ecc..496ac80a3dfbcf 100644
--- a/llvm/test/CodeGen/AMDGPU/flat-scratch.ll
+++ b/llvm/test/CodeGen/AMDGPU/flat-scratch.ll
@@ -4926,7 +4926,7 @@ define amdgpu_gs void @sgpr_base_large_offset(ptr addrspace(1) %out, ptr addrspa
 ;
 ; GFX12-LABEL: sgpr_base_large_offset:
 ; GFX12:       ; %bb.0: ; %entry
-; GFX12-NEXT:    scratch_load_b32 v2, off, s0 offset:-24
+; GFX12-NEXT:    scratch_load_b32 v2, off, s0 offset:65512
 ; GFX12-NEXT:    s_wait_loadcnt 0x0
 ; GFX12-NEXT:    global_store_b32 v[0:1], v2, off
 ; GFX12-NEXT:    s_nop 0
@@ -4985,7 +4985,7 @@ define amdgpu_gs void @sgpr_base_large_offset(ptr addrspace(1) %out, ptr addrspa
 ;
 ; GFX12-PAL-LABEL: sgpr_base_large_offset:
 ; GFX12-PAL:       ; %bb.0: ; %entry
-; GFX12-PAL-NEXT:    scratch_load_b32 v2, off, s0 offset:-24
+; GFX12-PAL-NEXT:    scratch_load_b32 v2, off, s0 offset:65512
 ; GFX12-PAL-NEXT:    s_wait_loadcnt 0x0
 ; GFX12-PAL-NEXT:    global_store_b32 v[0:1], v2, off
 ; GFX12-PAL-NEXT:    s_nop 0

@petar-avramovic petar-avramovic force-pushed the users/petar-avramovic/scratch-large-offset-test branch from 41189ad to 43076c2 Compare September 27, 2024 16:03
@petar-avramovic petar-avramovic force-pushed the users/petar-avramovic/scratch-large-offset-fix branch from dcec930 to 2ea25b2 Compare September 27, 2024 16:04
Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Please also backport to release/19.x.

Base automatically changed from users/petar-avramovic/scratch-large-offset-test to main September 30, 2024 08:39
Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.
@petar-avramovic petar-avramovic force-pushed the users/petar-avramovic/scratch-large-offset-fix branch from 2ea25b2 to fd2e866 Compare September 30, 2024 08:41
Copy link
Collaborator Author

petar-avramovic commented Sep 30, 2024

Merge activity

@petar-avramovic petar-avramovic merged commit 83fe851 into main Sep 30, 2024
5 of 7 checks passed
@petar-avramovic petar-avramovic deleted the users/petar-avramovic/scratch-large-offset-fix branch September 30, 2024 08:45
@petar-avramovic
Copy link
Collaborator Author

/cherry-pick e9d12a6 83fe851

@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2024

/cherry-pick e9d12a6 83fe851

Error: Command failed due to missing milestone.

@arsenm arsenm added this to the LLVM 19.X Release milestone Sep 30, 2024
@arsenm
Copy link
Contributor

arsenm commented Sep 30, 2024

/cherry-pick e9d12a6 83fe851

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Sep 30, 2024
…lvm#110256)

Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.

(cherry picked from commit 83fe851)
@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2024

/pull-request #110470

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
…lvm#110256)

Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.

(cherry picked from commit 83fe851)
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
…lvm#110256)

Use i32 for offset instead of i16, this way it does not get interpreted
as negative 16 bit offset.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants