[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