[PATCH] D26114: [AMDGPU] Allow hoisting of comparisons out of a loop and eliminate condition copies

Valery Pykhtin via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 07:12:01 PDT 2016


vpykhtin added a comment.

Mostly LGTM



================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:133
+          MachineInstr *Use = RegUses.pop_back_val();
+          if (Use->getOperand(1).getReg() == Dst.getReg()) {
+            unsigned FromReg = Use->getOperand(0).getReg();
----------------
if we already know that Use is a copy and it uses Dst why it's needed to check that Operand(1) is Dst? Maybe assert is enough here.


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:134
+          if (Use->getOperand(1).getReg() == Dst.getReg()) {
+            unsigned FromReg = Use->getOperand(0).getReg();
+            Use->eraseFromParent();
----------------
FromReg variable name is a bit confusing here. It seems that the name comes from "void replaceRegWith(unsigned **FromReg**, unsigned ToReg)" but in fact this register is a copy destination reg and by its nature is a duplicate of original SGPR64 we would like to propagate. Maybe it worth to name it something like DuplicateReg or something.


Repository:
  rL LLVM

https://reviews.llvm.org/D26114





More information about the llvm-commits mailing list