[PATCH] D68635: [AMDGPU] Come back patch for the 'Assign register class for cross block values according to the divergence.'

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 14 09:57:26 PDT 2019


nhaehnle added a comment.

Thank you for persisting through this.



================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:746-747
+      break;
+    }
+    else if (Def->isCopy() &&
+      TRI->isVGPR(*MRI, Def->getOperand(1).getReg())) {
----------------
LLVM coding style is to have the `else` on the same line as the `}`.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:760-761
+    TII->moveToVALU(MI);
+  }
+  else {
+    LLVM_DEBUG(dbgs() << "Legalizing PHI: " << MI);
----------------
Same coding style remark.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10903
+
+static bool hasCFUser(const Value *V, SmallPtrSet<const Value *, 16> &Visited) {
+  if (!Visited.insert(V).second)
----------------
The logic in this function seems needlessly convoluted. Instead of tracking `Result`, it should be possible to just `return true` in a number of places, right?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68635/new/

https://reviews.llvm.org/D68635





More information about the llvm-commits mailing list