[PATCH] D104355: [GlobalISel] Add a new artifact combiner for unmerge which looks through general artifact expressions.

Thu Jun 17 09:46:11 PDT 2021

qcolombet added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/legalize-inserts.mir:864
+    ; CHECK: $x0 = COPY [[MV]](s64)
+    ; CHECK: $x1 = COPY [[MV1]](s64)
     %0:_(s64) = COPY $x0
----------------
aemerson wrote:
> qcolombet wrote:
> > Is this new sequence what we really want?
> > 
> > It seems complicated compared to what we have before that diff.
> This looks to be the result of marking G_INSERT an artifact, which changes the order legalization vs artifact combines. It's not related to the ArtifactValueFinder, so it's collateral damage unfortunately.
> 
> In fact, all of these test changes except for the new tests I added are changes as a result of G_INSERT being an artifact.
What's the plan to fix that?

I would have expected that by delaying the legalization of G_INSERT (i.e., make that an artifact) we would have strictly improved codegen. Clearly, that's not what is happening :).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104355