[PATCH] [PATCH]Select wide immediate offset into [Base+XReg] addressing mode

Hao Liu Hao.Liu at arm.com
Fri Oct 10 18:49:10 PDT 2014


Hi Chad,

I think 


Thanks,
-Hao

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:789
@@ -788,3 +803,1 @@
 
-  // We don't want to match immediate adds here, because they are better lowered
-  // to the register-immediate addressing modes.
----------------
mcrosier wrote:
> Thanks for the update, Hao.  I'm still particularly concerned that this bit of logic is being removed entirely.  Can you please elaborate on why this is being removed?
Sure.

Firstly, only RHS can be constant. There is no situation that both LHS and RHS are constants (Such ADD will be optimized). Also, when there is only one constant operand in ADD, it will always adjust the constant to the RHS operand  (See the test in my patch @t9). So my patch, only checks whether RHS is a constant node.

Then, the original logic checks and returns when the operand is constant. My patch improves this logic. When the constant is too wide to be encoded in [Register+Immdediate] addressing mode or in ADD/SUB, it will be selected in [Register+Register] addressing mode. Otherwise, it will still return false as original logic.

Thanks,
-Hao

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:839
@@ -803,1 +838,3 @@
+  }
+
   // Remember if it is worth folding N when it produces extended register.
----------------
mcrosier wrote:
> Don't we want to keep this bit of logic? (lines 789-793)
> 
> ---------------------------------------------------------------------------------------------------------------------
> // We don't want to match immediate adds here, because they are better lowered 		​
> // to the register-immediate addressing modes. 		​
> if (isa<ConstantSDNode>(LHS) || isa<ConstantSDNode>(RHS))
>   return false;
I think there is no need keep such logic. This patch will check whether the ADD can be matched by the register-immediate addressing modes, if so, it will return false and do nothing.
See the if statement:
     if ((ImmOff % Size == 0 && ImmOff >= 0 && ImmOff < (0x1000 << Scale))

http://reviews.llvm.org/D5477






More information about the llvm-commits mailing list