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

[Jiangning Liu]

Hi Quentin,

>
> Going back to the concern that we are conflating isRematerializable and
> isAsCheapAsAMove, I can see why we want to keep both. That said, I still
> think that if we are overriding isAsCheapAsMove with this new hook for
> isRematerializable instructions, those instructions should be marked as
> isAsCheapAsMove too.
>
> This means two things:
> 1. All the uses of MachineInstr::isAsCheapAsMove should be promoted to use
> of TargetInstrInfo::isAsCheapAsMove.
> 2. If an instruction is rematerializable but not as cheap as move, then it
> shouldn’t appear in the isAsCheapAsMove target hook.
>
> If you have an instruction matching #2 and still want to make it appear in
> TargetInstrInfo::isAsCheapAsMove, it means you are trying to abuse the
> register coalescer to do rematerialization and that’s probably not the
> right solution.
>
> What do you think?
>

For case #2, you are right, so if we want to make make it rematerializable,
actually we could override isReallyTriviallyReMaterializable(MI, AA)
instead.

    /// isTriviallyReMaterializable - Return true if the instruction is
trivially
    /// rematerializable, meaning it has no side effects and requires no
operands


    /// that aren't always available.
    bool isTriviallyReMaterializable(const MachineInstr *MI,
                                     AliasAnalysis *AA = nullptr) const {
      return MI->getOpcode() == TargetOpcode::IMPLICIT_DEF ||
              (MI->getDesc().isRematerializable() &&
              (isReallyTriviallyReMaterializable(MI, AA) ||
               isReallyTriviallyReMaterializableGeneric(MI, AA)));
    }

For register coalesce pass, so far the algorithm could only rematerialize
instructions that are as cheap as move, because of the code in register
coalesce pass I pasted previously,

  if (!DefMI->isAsCheapAsAMove())
    return false;
  if (!TII->isTriviallyReMaterializable(DefMI, AA))



    return false;

If we want to change it, I'd suggest doing it separately.

If we add flag isAsCheapAsMove in .td file, we needn't to override
TargetInstrInfo::isAsCheapAsMove() at all for this patch. Previously I
proposed this solution at
http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/194325, and Tim
gave me feedback of not adding micro-architecture dependent things like
this in .td file, and I was convinced this is a good design level choice.
After all, for other micro-architectures, the instructions I'm highlighting
right now may not be as cheap as move.

Attached is the patch making the small change as you suggested.

Thanks,
-Jiangning


>
> Cheers,
> -Quentin
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/9c026bc5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Implement-AArch64-TTI-interface-isAsCheapAsAMove.patch
Type: text/x-patch
Size: 5918 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/9c026bc5/attachment.bin>