[PATCH] D12588: Introduce target hook for optimizing register copies

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: include/llvm/Target/TargetRegisterInfo.h:512
@@ +511,3 @@
+                             const TargetRegisterClass *SrcRC,
+                             unsigned SrcSubReg) const;
+
----------------
qcolombet wrote:
> Does it make sense to expose this API here?
> I.e., do we expect more users of this?
> If not, I would rather keep it where it was as a static function or at least being private.
This was to keep it as the default implementation of shouldRewriteCopySrc. I was thinking a target might want to insert an additional constraint on top of sharesSameRegisterFile in shouldRewriteCopySrc although I don't think I have a use for this.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:667
@@ -696,5 +666,3 @@
       const TargetRegisterClass *SrcRC = MRI->getRegClass(CurSrcPair.Reg);
-
-      // If this source does not incur a cross register bank copy, use it.
-      ShouldRewrite = shareSameRegisterFile(*TRI, DefRC, SubReg, SrcRC,
-                                           CurSrcPair.SubReg);
+      ShouldRewrite = TRI->shouldRewriteCopySrc(DefRC, SubReg, SrcRC,
+                                                CurSrcPair.SubReg);
----------------
qcolombet wrote:
> shareSameRegisterFile is supposed to produce register coalescer friendly copies and should benefit every target.
> So I would have expect this check to be
> shareSameRegisterFile || TRI->shouldRewriteCopySrc.
> 
> I may be wrong, but I want to be sure the change is intentional.
This is intentional. The problem is that this returns true and not false in this case. For most every legal case on AMDGPU, shareSameRegisterFile is going to be true, so it decides to rewrite the copy earlier and doesn't continue up the use-def chain to the value inserted into the reg_sequence. Even for the cases where this would return false, we sometimes would often prefer a copy from what this would consider to be one register file to another. We don't have cross register file copies, because only one direction is ever legal and we don't have any type distinctions between register types.


http://reviews.llvm.org/D12588