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] Only set Info.memVT when not later overridden #92670

Merged
merged 1 commit into from
May 20, 2024

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented May 18, 2024

For the amdgcn_*_buffer_load_lds intrinsics this field is later overriden, so avoid pointlessly calling MVT::getVT in that case. Importantly, this is also the only case I can find in tree where a PointerType is passed to MVT::getVT, so this will allow us to forbid doing so in future, keeping MVT::iPTR as originating solely from TableGen as was claimed next to its definition in MachineValueType.h (but lost in the autogeneration conversion).

For the amdgcn_*_buffer_load_lds intrinsics this field is later
overriden, so avoid pointlessly calling MVT::getVT in that case.
Importantly, this is also the only case I can find in tree where a
PointerType is passed to MVT::getVT, so this will allow us to forbid
doing so in future, keeping MVT::iPTR as originating solely from
TableGen as was claimed next to its definition in MachineValueType.h
(but lost in the autogeneration conversion).
@llvmbot
Copy link
Collaborator

llvmbot commented May 18, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Jessica Clarke (jrtc27)

Changes

For the amdgcn_*_buffer_load_lds intrinsics this field is later overriden, so avoid pointlessly calling MVT::getVT in that case. Importantly, this is also the only case I can find in tree where a PointerType is passed to MVT::getVT, so this will allow us to forbid doing so in future, keeping MVT::iPTR as originating solely from TableGen as was claimed next to its definition in MachineValueType.h (but lost in the autogeneration conversion).


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 89e83babcfef4..801ab4b65cbc0 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -1233,13 +1233,13 @@ bool SITargetLowering::getTgtMemIntrinsic(IntrinsicInfo &Info,
       // Atomic
       Info.opc = CI.getType()->isVoidTy() ? ISD::INTRINSIC_VOID :
                                             ISD::INTRINSIC_W_CHAIN;
-      Info.memVT = MVT::getVT(CI.getArgOperand(0)->getType());
       Info.flags |= MachineMemOperand::MOLoad |
                     MachineMemOperand::MOStore |
                     MachineMemOperand::MODereferenceable;
 
       switch (IntrID) {
       default:
+        Info.memVT = MVT::getVT(CI.getArgOperand(0)->getType());
         // XXX - Should this be volatile without known ordering?
         Info.flags |= MachineMemOperand::MOVolatile;
         break;

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff ba8a2ade84f4c1bfc531fe3673470377c038f31d 20b5ad9f94db4456953abb21b63b0fa9cf3e4e1c -- llvm/lib/Target/AMDGPU/SIISelLowering.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 801ab4b65c..b62aff0fb6 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -1231,8 +1231,8 @@ bool SITargetLowering::getTgtMemIntrinsic(IntrinsicInfo &Info,
       Info.flags |= MachineMemOperand::MOStore;
     } else {
       // Atomic
-      Info.opc = CI.getType()->isVoidTy() ? ISD::INTRINSIC_VOID :
-                                            ISD::INTRINSIC_W_CHAIN;
+      Info.opc = CI.getType()->isVoidTy() ? ISD::INTRINSIC_VOID
+                                          : ISD::INTRINSIC_W_CHAIN;
       Info.flags |= MachineMemOperand::MOLoad |
                     MachineMemOperand::MOStore |
                     MachineMemOperand::MODereferenceable;

@jrtc27 jrtc27 merged commit 7529fe2 into llvm:main May 20, 2024
4 of 6 checks passed
@jrtc27 jrtc27 deleted the amdgpu-memvt-pointer-redundant branch May 20, 2024 06:08
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.

None yet

3 participants