Skip to content

AMDGPU/GlobalISel: Fix G_PTR_ADD handling in getBaseWithConstantOffset - #213968

Merged
petar-avramovic merged 1 commit into
mainfrom
users/petar-avramovic/ptr-add-nuw
Aug 5, 2026
Merged

AMDGPU/GlobalISel: Fix G_PTR_ADD handling in getBaseWithConstantOffset#213968
petar-avramovic merged 1 commit into
mainfrom
users/petar-avramovic/ptr-add-nuw

Conversation

@petar-avramovic

Copy link
Copy Markdown
Contributor

There was a discrepancy compared to SDAG when G_PTR_ADD was being matched
and there was no nuw flag check. In case of G_PTR_ADD, nuw flag comes
from inbounds flag on getelementptr. For reference, SDAG does not have
pointers so PTR_ADD is integer ADD in SDAG.

petar-avramovic commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-backend-amdgpu

Author: Petar Avramovic (petar-avramovic)

Changes

There was a discrepancy compared to SDAG when G_PTR_ADD was being matched
and there was no nuw flag check. In case of G_PTR_ADD, nuw flag comes
from inbounds flag on getelementptr. For reference, SDAG does not have
pointers so PTR_ADD is integer ADD in SDAG.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp (+7-2)
  • (modified) llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll (+2-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp b/llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp
index 9dce9df2ad99f..c3b497a8ba79d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp
@@ -64,8 +64,13 @@ AMDGPU::getBaseWithConstantOffset(MachineRegisterInfo &MRI, Register Reg,
   // Handle G_PTRTOINT (G_PTR_ADD base, const) case
   if (Def->getOpcode() == TargetOpcode::G_PTRTOINT) {
     MachineInstr *Base;
-    if (mi_match(Def->getOperand(1).getReg(), MRI,
-                 m_GPtrAdd(m_MInstr(Base), m_ICst(Offset)))) {
+    Register PtrAdd = Def->getOperand(1).getReg();
+    if (mi_match(PtrAdd, MRI, m_GPtrAdd(m_MInstr(Base), m_ICst(Offset)))) {
+      // Same check as for G_ADD; nuw comes from getelementptr inbounds.
+      if (CheckNUW && !MRI.getVRegDef(PtrAdd)->getFlag(MachineInstr::NoUWrap)) {
+        assert(MRI.getType(Reg).getScalarSizeInBits() == 32);
+        return std::pair(Reg, 0);
+      }
       // If Base was int converted to pointer, simply return int and offset.
       if (Base->getOpcode() == TargetOpcode::G_INTTOPTR)
         return std::pair(Base->getOperand(1).getReg(), Offset);
diff --git a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll
index 721122c97d07a..901afb865a81e 100644
--- a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll
+++ b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll
@@ -2188,7 +2188,8 @@ define amdgpu_ps i32 @s_buffer_load_ptr_soffset(<4 x i32> inreg %desc, ptr addrs
 ; GFX1250-GISEL:       ; %bb.0:
 ; GFX1250-GISEL-NEXT:    global_prefetch_b8 v0, s[0:1] scope:SCOPE_SE
 ; GFX1250-GISEL-NEXT:    v_nop
-; GFX1250-GISEL-NEXT:    s_buffer_load_b32 s0, s[0:3], s4 offset:0x34 nv
+; GFX1250-GISEL-NEXT:    s_add_co_u32 s4, s4, 52
+; GFX1250-GISEL-NEXT:    s_buffer_load_b32 s0, s[0:3], s4 offset:0x0 nv
 ; GFX1250-GISEL-NEXT:    s_wait_kmcnt 0x0
 ; GFX1250-GISEL-NEXT:    ; return to shader part epilog
   %gep = getelementptr i8, ptr addrspace(6) %p, i32 52

@krzysz00 krzysz00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the nuw come from inbounds? I thought that was why GEP had a separate nuw flag on it.

@petar-avramovic

Copy link
Copy Markdown
Contributor Author

Does the nuw come from inbounds? I thought that was why GEP had a separate nuw flag on it.

yes, see #151708
getelementptr inbounds -> nuw nusw inbounds G_PTR_ADD

Base automatically changed from users/petar-avramovic/ptr-soffset to main August 5, 2026 10:48
There was a discrepancy compared to SDAG when G_PTR_ADD was being matched
and there was no nuw flag check. In case of G_PTR_ADD, nuw flag comes
from inbounds flag on getelementptr. For reference, SDAG does not have
pointers so PTR_ADD is integer ADD in SDAG.
@petar-avramovic
petar-avramovic force-pushed the users/petar-avramovic/ptr-add-nuw branch from f0bda69 to 88e7a79 Compare August 5, 2026 12:03
@petar-avramovic
petar-avramovic merged commit b2b88d9 into main Aug 5, 2026
8 of 12 checks passed
@petar-avramovic
petar-avramovic deleted the users/petar-avramovic/ptr-add-nuw branch August 5, 2026 12:04
jinge90 pushed a commit to jinge90/llvm-project that referenced this pull request Aug 6, 2026
llvm#213968)

There was a discrepancy compared to SDAG when G_PTR_ADD was being matched
and there was no nuw flag check. In case of G_PTR_ADD, nuw flag comes
from inbounds flag on getelementptr. For reference, SDAG does not have
pointers so PTR_ADD is integer ADD in SDAG.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants