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

[Quentin Colombet]

Hi Jiangning,

A few comments on the second patch (the first one looks good).

+// FIXME: this implementation should be micro-architecture dependent, so a
+// micro-architecture target hook should be introduced here in future.
+bool AArch64InstrInfo::isAsCheapAsAMove(const MachineInstr *MI) const {
+  if (Subtarget.isCortexA57() || Subtarget.isCortexA53()) {
+    switch (MI->getOpcode()) {
+    default:
+      return false;
[…]
+  }
+
+  return MI->isAsCheapAsAMove();
+}
+

To match LLVM guidelines, use an early exit when the subtarget does not match:
if (!Subtarget.isCortexA57() && !Subtarget.isCortexA53())
  return MI->isAsCheapAsAMove();
switch (MI->getOpcode()) {
[…]

+    case AArch64::ANDSWri:
+    case AArch64::ANDSXri:

I believe this is a remaining of the initial patch. The S variant are not rematerializable.

Thanks,
-Quentin

> On Jul 7, 2014, at 11:34 PM, Jiangning Liu <liujiangning1 at gmail.com> wrote:
> 
> Hi Eric,
> 
> OK. Attached are two separate patches.
> 
> Hi Qunentin,
> 
> I modified the code following your new comments.
> 
> Since I don't know how phabaricator will work for two patches uploaded, I'm
> simply reply this mail by attached two separate patches. I don't want to
> create a new review entry in phabaricator to confuse the people who want to
> review the patch.
> 
> Thanks,
> -Jiangning
> 
> 
> 2014-07-08 5:25 GMT+08:00 Eric Christopher <echristo at gmail.com>:
> 
>> You could also split the patch into adding the API and using it in the
>> next commit. I.e. there's no reason why isRematerializable can't be set for
>> the instructions separate from adding TII->isCheapAsAMove()
>> 
>> http://reviews.llvm.org/D4361
>> 
>> 
>> 
> 
> - {F91790, layout=link}
> - {F91789, layout=link}
> 
> http://reviews.llvm.org/D4361
> 
>