[PATCH] D109748: [GlobalISel][Legalizer] Use ArtifactValueFinder first for unmerge combines before trying others.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In D109748#3000346 <https://reviews.llvm.org/D109748#3000346>, @arsenm wrote:

> In D109748#3000156 <https://reviews.llvm.org/D109748#3000156>, @aemerson wrote:
>
>> In D109748#2999614 <https://reviews.llvm.org/D109748#2999614>, @arsenm wrote:
>>
>>> The MIR test changes are all clearly worse, but the IR tests are small improvements. The MIR tests all look like unfortunate legalization ordering changes that just result in more artifact churn (some of which should result in more instructions in the end). This is probably from no longer considering the legalization action of the proposed new unmerge. I'm not really sure what to do here. I'm still not 100% clear on how artifact legalization rules should be used, and whether the artifact combiner should be fully responsible for handling them
>>
>> Can these regressions be cleaned up by a later (postlegalize) combine for AMDGPU?
>
> In principle you can always go backwards. There are already a lot of intermediate steps for these artifacts already, and this would add yet another set on top of them.
>
> The changes I think are ugliest look like legalize-smin.mir. This is a trivial merge of legal register types, but now this turns into a mess of bitshifting and bitmasking. I think there would be improvements by changing the legalization order, or somehow considering the specific legalization of newly created artifacts

At least some of those changes seem to actually be due to different dead code being left behind by the legalizer. I've put D109858 <https://reviews.llvm.org/D109858> to allow us to clean it up for tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109748