[PATCH] D45644: [GlobalISel][Legalizer] Look thro copies while combining G_UNMERGE's

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 13 16:19:28 PDT 2018


rtereshin added inline comments.


================
Comment at: test/CodeGen/AArch64/GlobalISel/legalize-combines.mir:94
+    %5:_(s32) = G_ADD %3, %4
+    $w0 = COPY %5
+...
----------------
`test_combines_5` prior to the patch is an exact copy of `test_combines_3`, so just changing it in place doesn't reduce the coverage nor it does create tests that test more than one thing.


================
Comment at: test/CodeGen/AArch64/GlobalISel/legalize-combines.mir:119
+    %8:_(s64) = COPY %7
+    %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %8
     %5:_(s32) = G_ADD %3, %4
----------------
same as `test_combines_5`, but uses one of the copies in the middle of the COPY-chain to see how look-thro-copies thing would deal with it.


================
Comment at: test/CodeGen/AArch64/GlobalISel/legalize-nonpowerof2eltsvec.mir:25
     ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
     ; CHECK: [[MV:%[0-9]+]]:_(<3 x s64>) = G_MERGE_VALUES [[COPY]](s64), [[COPY]](s64), [[COPY]](s64)
+    ; CHECK: $x0 = COPY [[COPY]](s64)
----------------
My understanding of the test is that it needs to see how the `G_MERGE_VALUES` would be legalized as an instruction, not as a legalization artifact. To make sure it doesn't get combined out as a legalization artifact instead, we need to use the merged vector value more reliably than by just inserting a copy between `G_MERGE_VALUES` and `G_UNMERGE_VALUES`


Repository:
  rL LLVM

https://reviews.llvm.org/D45644





More information about the llvm-commits mailing list