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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:652
+          (StartBit + Size) >= (InsertOffset + InsertedRegTy.getSizeInBits()))
+        return Register();
+
----------------
These conditions don't look right. You are only disallowing:
```
+-----------+-----+-----------+
| CONTAINER | INS | CONTAINER |
+-----------+-----+-----------+
        |            |
        SB           EB
```
but there are lots of other cases of partial overlap. I would suggest rewriting it...


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:693
+      Register SrcRegToUse;
+      if (InsertOffset <= StartBit && StartBit < InsertedEndBit) {
+        // Scenarios A and D.
----------------
... like this:
```
if (EndBit <= InsertOffset || InsertedEndBit <= StartBit)
  SrcRegToUse = ContainerSrcReg;
else if (InsertOffset <= StartBit && EndBit <= InsertedEndBit)
  SrcRegToUse = InsertedReg;
else
  return Register();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104355