Skip to content

Conversation

@Icohedron
Copy link
Contributor

@Icohedron Icohedron commented Nov 27, 2025

Fixes an LLVM DirectX codegen test after it broke due to #169141

The CBuffer loads and GEPs are no longer duplicated when there are two or more accesses within the same basic block.
This PR removes the duplicate check for CBuffer load and GEP from the original test function @f and adds a new test function @g which places duplicate CBuffer loads into separate basic blocks.

@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2025

@llvm/pr-subscribers-backend-directx

Author: Deric C. (Icohedron)

Changes

Fixes an LLVM DirectX codegen test after it broke due to #169141

The CBuffer loads and GEPs are no longer duplicated when in there are two or more accesses within the same basic block.
This PR removes the duplicate CBuffer load and GEP from the original test function @<!-- -->f and adds a new test function @<!-- -->g which places duplicate CBuffer loads into separate basic blocks.


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

1 Files Affected:

  • (modified) llvm/test/CodeGen/DirectX/CBufferAccess/gep-ce-two-uses.ll (+15)
diff --git a/llvm/test/CodeGen/DirectX/CBufferAccess/gep-ce-two-uses.ll b/llvm/test/CodeGen/DirectX/CBufferAccess/gep-ce-two-uses.ll
index 4eda6353f47ed..751f1d3ebcfa9 100644
--- a/llvm/test/CodeGen/DirectX/CBufferAccess/gep-ce-two-uses.ll
+++ b/llvm/test/CodeGen/DirectX/CBufferAccess/gep-ce-two-uses.ll
@@ -21,6 +21,21 @@ entry:
   %a1 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4
   store float %a1, ptr %dst, align 32
 
+  %a2 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4
+  store float %a2, ptr %dst, align 32
+
+  ret void
+}
+
+define void @g(ptr %dst) {
+entry:
+  ; CHECK: [[PTR:%.*]] = call ptr addrspace(2) @llvm.dx.resource.getpointer.{{.*}}(target("dx.CBuffer", %__cblayout_CB) {{%.*}}, i32 0)
+  ; CHECK: getelementptr inbounds nuw i8, ptr addrspace(2) [[PTR]], i32 16
+  %a1 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4
+  store float %a1, ptr %dst, align 32
+  br label %next
+
+next:
   ; CHECK: [[PTR:%.*]] = call ptr addrspace(2) @llvm.dx.resource.getpointer.{{.*}}(target("dx.CBuffer", %__cblayout_CB) {{%.*}}, i32 0)
   ; CHECK: getelementptr inbounds nuw i8, ptr addrspace(2) [[PTR]], i32 16
   %a2 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4

@Icohedron Icohedron merged commit a1f30c2 into llvm:main Nov 27, 2025
9 of 10 checks passed
store float %a1, ptr %dst, align 32

%a2 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4
store float %a2, ptr %dst, align 32
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably add checks for the two stores to show that they're reusing the same GEP

; CHECK: [[PTR:%.*]] = call ptr addrspace(2) @llvm.dx.resource.getpointer.{{.*}}(target("dx.CBuffer", %__cblayout_CB) {{%.*}}, i32 0)
; CHECK: getelementptr inbounds nuw i8, ptr addrspace(2) [[PTR]], i32 16
%a1 = load float, ptr addrspace(2) getelementptr inbounds nuw (i8, ptr addrspace(2) @a1, i32 16), align 4
store float %a1, ptr %dst, align 32
Copy link
Contributor

Choose a reason for hiding this comment

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

and I guess for completeness checking that the two stores use their own GEPs here might make sense.

Icohedron added a commit that referenced this pull request Nov 27, 2025
…ses.ll` more strict (#169855)

Continuation of PR #169848 to address PR comments.

This PR makes the test more strict by adding CHECKs to ensure the loads
are indeed using the same or different GEPs.
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 27, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-mlir-rhel-clang running on ppc64le-mlir-rhel-test while building llvm at step 6 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/33995

Here is the relevant piece of the build log for the reference
Step 6 (test-build-check-mlir-build-only-check-mlir) failure: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
...
PASS: MLIR-Unit :: IR/./MLIRIRTests/100/130 (3689 of 3700)
PASS: MLIR-Unit :: IR/./MLIRIRTests/101/130 (3690 of 3700)
PASS: MLIR-Unit :: IR/./MLIRIRTests/0/130 (3691 of 3700)
PASS: MLIR-Unit :: IR/./MLIRIRTests/38/130 (3692 of 3700)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/12/22 (3693 of 3700)
PASS: MLIR-Unit :: Pass/./MLIRPassTests/10/13 (3694 of 3700)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/13/22 (3695 of 3700)
PASS: MLIR-Unit :: IR/./MLIRIRTests/39/130 (3696 of 3700)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/11/22 (3697 of 3700)
PASS: MLIR :: mlir-reduce/dce-test.mlir (3698 of 3700)
command timed out: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=4210.605909

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.

5 participants