[PATCH] [AArch64] Enhance rematerialization by adding a new API isAsCheapAsAMove in TargetInstroInfo
Jiangning Liu
liujiangning1 at gmail.com
Mon Jul 7 23:30:20 PDT 2014
================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:1615
@@ -1614,3 +1614,3 @@
SDPatternOperator OpNode = null_frag> {
- let hasSideEffects = 0 in {
+ let hasSideEffects = 0, isReMaterializable = 1 in {
// Add/Subtract immediate
----------------
Quentin Colombet wrote:
> Tim Northover wrote:
> > Quentin Colombet wrote:
> > > Since we are overriding the behavior of isAsCheapAsMove, I think it would make sense to set the isAsCheapAsMove flag as well as the isRematerializable flag.
> > >
> > > What do you think?
> > >
> > > Also, we may want to check the uses of isAsCheapAsMove to see if we should "promote" them to the new target hook.
> > Careful if you do that, I don't think it is as cheap as a move on Cyclone.
> Indeed!
> That's why isAsCheapAsMove queries the subtarget to decide whether or not this is true.
>
> If we go into the "promote" direction I was talking about, perhaps we should update the comment in MCDescInstr.h for isAsCheapAsMove to make the fact that this flag will be checked by the subtarget more obvious.
>
> Jiangning, Tim, what do you think?
OK. I can add one more sentence to highlight this.
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:558
@@ +557,3 @@
+ if (MI->getOperand(3).getImm() != 0)
+ return false;
+ else
----------------
Quentin Colombet wrote:
> This if/else construction should be replaced by a simpler:
> return MI->getOperand(3).getImm() == 0;
>
> If you do not like this construction, you should at least remove the "else" to match LLVM guidelines.
OK. I will make the change.
http://reviews.llvm.org/D4361
More information about the llvm-commits
mailing list