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

Jiangning Liu liujiangning1 at gmail.com
Wed Jul 2 03:22:22 PDT 2014


Hi Quentin,

Thanks for your review!


> This is kind of strange that the patch introduces a target hook for
> isAsCheapAsMove, whereas it uses the flag isRematerializable to mark the
> interesting instructions.
> Indeed, it seems kind of arbitrary which instructions are marked
> rematerializable and which instructions are not. If we want to pursue this
> way, I believe that we have to clarify the semantic of this flag.
>
> The isRematerializable flag is also in a weird situation. Indeed,
> according to the documentation, it is deprecated:
>   /// isRematerializable - Returns true if this instruction is a candidate
> for
>   /// remat.  This flag is deprecated, please don't use it anymore.  If
> this
>   /// flag is set, the isReallyTriviallyReMaterializable() method is
> called to
>   /// verify the instruction is really rematable.
> But it is used in TargetInstrInfo::isTriviallyRematerializable.
>
> Since, you are touching this part, I believe that we should either fix the
> implementation of isTrivallyRematerializable and really deprecate this
> flag, or update the comment to reflect the actual semantic (which needs to
> be defined) of this flag.
>

Sorry that I really didn't notice that this flag is deprecated. I can see
isReMaterializable flag is being used in a lot of .td files. Do you mean
because this flag is being used in an arbitrary manner everywhere, we want
to work out another more centralized solution to describe rematerializable
attribute? Or we needn't this flag at all, and the algorithm of
rematerialization should just check cost like the info provided by
isAsCheapAsMove?

Thanks,
-Jiangning


>
> Thanks,
> -Quentin
>

http://reviews.llvm.org/D4361






More information about the llvm-commits mailing list