AMDGPU: Make llvm.amdgcn.make.buffer.rsrc propagate poison

[arsenm]

No description provided.

[arsenm]

• AMDGPU: Make llvm.amdgcn.make.buffer.rsrc propagate poison #141913 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp (+6)
• (modified) llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll (+21)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
index 5f6ab24182d5e..16f27533a0c19 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
@@ -1527,6 +1527,12 @@ GCNTTIImpl::instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II) const {
       return IC.replaceInstUsesWith(II, ConstantInt::getFalse(II.getType()));
     break;
   }
+  case Intrinsic::amdgcn_make_buffer_rsrc: {
+    Value *Src = II.getArgOperand(0);
+    if (isa<PoisonValue>(Src))
+      return IC.replaceInstUsesWith(II, PoisonValue::get(II.getType()));
+    return std::nullopt;
+  }
   case Intrinsic::amdgcn_raw_buffer_store_format:
   case Intrinsic::amdgcn_struct_buffer_store_format:
   case Intrinsic::amdgcn_raw_tbuffer_store:
diff --git a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
index 89c73c2161e35..9375d56e900e0 100644
--- a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
@@ -6520,3 +6520,24 @@ define amdgpu_kernel void @ds_bpermute_uniform_lane(ptr addrspace(1) %out, i32 %
   store i32 %v, ptr addrspace(1) %out
   ret void
 }
+
+; --------------------------------------------------------------------
+; llvm.amdgcn.make.buffer.rsrc.p8
+; --------------------------------------------------------------------
+
+define ptr addrspace(8) @make_buffer_rsrc_poison() {
+; CHECK-LABEL: @make_buffer_rsrc_poison(
+; CHECK-NEXT:    ret ptr addrspace(8) poison
+;
+  %rsrc = call ptr addrspace(8) @llvm.amdgcn.make.buffer.rsrc.p8.p1(ptr addrspace(1) poison, i16 0, i32 1234, i32 5678)
+  ret ptr addrspace(8) %rsrc
+}
+
+define ptr addrspace(8) @make_buffer_rsrc_undef() {
+; CHECK-LABEL: @make_buffer_rsrc_undef(
+; CHECK-NEXT:    [[RSRC:%.*]] = call ptr addrspace(8) @llvm.amdgcn.make.buffer.rsrc.p8.p1(ptr addrspace(1) undef, i16 0, i32 1234, i32 5678)
+; CHECK-NEXT:    ret ptr addrspace(8) [[RSRC]]
+;
+  %rsrc = call ptr addrspace(8) @llvm.amdgcn.make.buffer.rsrc.p8.p1(ptr addrspace(1) undef, i16 0, i32 1234, i32 5678)
+  ret ptr addrspace(8) %rsrc
+}

[github-actions]

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:

git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

The following files introduce new uses of undef:

• llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

[shiltian]

Look reasonable.

[krzysz00]

We really should get around to the CantGeneratePoison or whatever we want to call it annotation on intrinsics ... (but that's unrelated)