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

Eric Christopher echristo at gmail.com
Wed Jul 9 16:42:53 PDT 2014


Hi Quentin, Jiangning,

I'm not sure about this patch (though I love the results!) -

It looks like we're just conflating isRematerializable and
isAsCheapAsAMove, especially in the somewhat random addition of
isRematerializable to some of the instructions and then adding the
rest to isAsCheapAsAMove (and are they universally cheap or is it
really subtarget dependent here?) and I haven't really seen an answer
to Quentin's question about that? Are these actually rematerializable
or is it just cheaper/etc?

Thanks!

-eric


On Wed, Jul 9, 2014 at 7:28 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> 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
>
> http://reviews.llvm.org/D4361
>
>




More information about the llvm-commits mailing list