[PATCH] D39060: AMDGPU: Lower buffer store and atomic intrinsics manually

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 23 09:41:02 PDT 2017


arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4264-4265
+
+    switch (IntrID) {
+    case Intrinsic::amdgcn_buffer_atomic_swap:
+      Opcode = AMDGPUISD::BUFFER_ATOMIC_SWAP;
----------------
Move to a separate function?


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4566-4568
+      MachineMemOperand::MOStore |
+      MachineMemOperand::MODereferenceable |
+      MachineMemOperand::MOVolatile,
----------------
nhaehnle wrote:
> Unfortunately, there's not a lot of documentation on MOVolatile, but I suspect this should not be set at least when GLC == SLC == 0. And I image that that would fix the issue with D39012 as well... (which means the order of patches should be reversed).
You should not be setting MOVolatile out of nowhere. Adding that defeats what you are trying to accomplish. I also think we aren't setting volatile directly to GLC and the memory legalizer pass is supposed to set GLC.


https://reviews.llvm.org/D39060





More information about the llvm-commits mailing list