[PATCH] D12588: Introduce target hook for optimizing register copies
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 8 12:06:54 PDT 2015
qcolombet added a comment.
I would have expected shareSameRegisterFile to be a win for every target, but you seem to drop that check.
Is that intentional?
I am mostly fine with the change, just not sure about what to do with shareSameRegisterFile. See my inline comment for further discussions.
Comment at: include/llvm/Target/TargetRegisterInfo.h:512
@@ +511,3 @@
+ const TargetRegisterClass *SrcRC,
+ unsigned SrcSubReg) const;
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.
Comment at: include/llvm/Target/TargetRegisterInfo.h:523
@@ +522,3 @@
+ // If this source does not incur a cross register bank copy, use it.
+ return shareSameRegisterFile(DefRC, DefSubReg, SrcRC, SrcSubReg);
Just return false, see further for the complete suggestion.
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,
+ ShouldRewrite = TRI->shouldRewriteCopySrc(DefRC, SubReg, SrcRC,
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.
More information about the llvm-commits