[PATCH] D106408: Allow rematerialization of virtual reg uses

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

>> Are they all expected to keep working with the change here?
>
> Overall yes. The only per-requestity is MachineLICM change in the D107677 <https://reviews.llvm.org/D107677> at this point.

Nice one

>> I feel like the new behavior isn't really "Trivial" any more, and it may be worth keeping the old method for trivial cases, which is just the base case plus a virtual reg use check. Essentially MachineLICMBase::isTriviallyReMaterializable from D107677 <https://reviews.llvm.org/D107677>. I'm not sure what to call the new method though, and not sure its worth it if all the uses above are OK as-is.
>
> The uses above are OK as far as I can tell. I can see how that is possible to wrap one method into another, but as usual have problem with names. We already have isRematerializable, isTriviallyRematerializable, and isReallyTriviallyRematerializable. Creating something like isReallyReallyTriviallyRematerializable looks like going down a rabbit hole ;) But I am open to opinions.

I was thinking of something like "isSimpleRematerializable" vs "isTriviallyRematerializable" (but I didn't really like "simple"). If we don't have a user of it (other than Machine LICM which has it's own function), I'd say it is fine as-is.

Thanks for checking. Providing there are not other comment, this SGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106408/new/

https://reviews.llvm.org/D106408