[PATCH] [AArch64] Enhance rematerialization by adding a new API isAsCheapAsAMove in TargetInstroInfo

Quentin Colombet qcolombet at apple.com
Mon Jul 7 07:04:24 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
----------------
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?

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:558
@@ +557,3 @@
+      if (MI->getOperand(3).getImm() != 0)
+        return false;
+      else
----------------
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.

http://reviews.llvm.org/D4361






More information about the llvm-commits mailing list