[PATCH] D47383: [AMDGPU] Avoid using divergent value in mubuf addr64 descriptor

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 15 03:09:20 PDT 2018


nhaehnle added a comment.

Thanks. I feel like some of this could perhaps be improved with a computeKnownBits if there is a 64-bit uniform base and a 32-bit non-uniform offset, but that doesn't have to be part of this change.



================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:370-374
+  SDNode *Lo = CurDAG->getMachineNode(AMDGPU::S_MOV_B32, DL, MVT::i32,
+                              CurDAG->getConstant(Imm & 0xFFFFFFFF, DL,
+                                                  MVT::i32));
+  SDNode *Hi = CurDAG->getMachineNode(AMDGPU::S_MOV_B32, DL, MVT::i32,
+                              CurDAG->getConstant(Imm >> 32, DL, MVT::i32));
----------------
Have you run clang-format on this? It looks a bit off.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1006
 
+
 bool AMDGPUDAGToDAGISel::SelectMUBUF(SDValue Addr, SDValue &Ptr,
----------------
Spurious whitespace change.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1083-1098
+    if (N0->isDivergent()) {
+      if (N1->isDivergent()) {
+        // Both N0 and N1 are divergent. Use the result of the add as the
+        // addr64, and construct the resource from a 0 address.
+        Ptr = SDValue(buildSMovImm64(DL, 0, MVT::v2i32), 0);
+        VAddr = Addr;
+      } else {
----------------
This has a lot of redundancy with the isBaseWithConstantOffset case above. Perhaps the cases can be combined?


Repository:
  rL LLVM

https://reviews.llvm.org/D47383





More information about the llvm-commits mailing list