[PATCH] D23942: Do not abort on unresolved conflicts when joining subranges

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:2342-2377
@@ -2341,5 +2341,38 @@
                  << '@' << LR.getValNumInfo(i)->def << '\n');
-    if (SubRangeJoin)
-      return false;
+    if (SubRangeJoin) {
+      // Unresolved subrange conflicts may remain here as a result of merging
+      // registers where subregister definitions of one register are
+      // overwritten by corresponding definitions of the other register.
+      // For example, in this case, whem coalescing vreg140 into vreg131:
+      //   704B     %vreg140<def> = ...
+      //   720B     %vreg131<def> = COPY %vreg140
+      //   736B     %vreg131:sub1<def> = COPY %vreg48
+      //   768B     %vreg131:sub2<def> = COPY %vreg23
+      //   800B     %vreg131:sub3<def> = COPY %vreg24
+      //   832B     %vreg131:sub4<def> = COPY %vreg25
+      //   864B     %vreg131:sub5<def> = COPY %vreg26
+      //   896B     %vreg131:sub6<def> = COPY %vreg40
+      //   928B     %vreg131:sub7<def> = COPY %vreg42
+      //   960B     %vreg131:sub8<def> = COPY %vreg44
+      //   976B     ... = use %vreg131
+      //   1008B    %vreg140:sub1<def> = COPY %vreg54
+      //   1040B    %vreg140:sub2<def> = COPY %vreg23
+      //   1072B    %vreg140:sub3<def> = COPY %vreg24
+      //   1104B    %vreg140:sub4<def> = COPY %vreg25
+      //   1136B    %vreg140:sub5<def> = COPY %vreg26
+      //   1168B    %vreg140:sub6<def> = COPY %vreg50
+      //   1200B    %vreg140:sub7<def> = COPY %vreg51
+      //   1232B    %vreg140:sub8<def> = COPY %vreg52
+      //   1248B    ... = use %vreg140
+      // The conflict resolution for the main live ranges of both registers
+      // can determine that the coalescing is legal and may proceed, but the
+      // subregister ranges will still contain conflicts, since the subranges
+      // will overlap between the two virtual registers. Since repeating of
+      // the resolution code below for subregisters could only result in
+      // CR_Replace, and the legality has already been determined, assume
+      // the resolution to be CR_Replace without repeating the work.
+      V.Resolution = CR_Replace;
+      continue;
+    }
 
     ++NumLaneConflicts;
----------------
I would not expect the example in the comment to pass the usesLanes checks because of the lack of undef flags, right?
I.e., that does not seem correct to jump to the Replace conclusion.

>  They are considered "used" for the purpose of liveness tracking, but are not actually used by the program. 

But if they are alive that means they are used right? I mean we couldn't merge them unless vreg140:sub1 has read-undef, could we?

Basically, I don't like us making assumptions on what the liveness models compared to what the program actually uses. I am afraid it may work for some targets, but not all of them. Therefore, I would rather be conservatively correct (that we have right now) than guessing around the liveness.


Repository:
  rL LLVM

https://reviews.llvm.org/D23942