[PATCH] D59339: [GISel][Legalizer][WIP][RFC] Retry legalization for failing instrs when artifacts are around

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet created this revision.
qcolombet added reviewers: arsenm, aditya_nandakumar, volkan.
Herald added subscribers: Petar.Avramovic, wdng.
Herald added a project: LLVM.

This is a prototype based on the discussion I had with @arsenm in D59227 <https://reviews.llvm.org/D59227>.

Basically what the patch does is when we cannot legalize an instruction but there are still legalization artifacts around, push it into a retry list instead of aborting right away. The rationale is that the legalization may have been prevented because the legalization of the instruction couldn't cope with the artifacts in the middle.

This is an RFC/WIP patch, in particular, there are no check whether the element in the retry list got removed, no check for loops (e.g., failed legalization actually inserts a bunch of artifacts and will keep being reapplied over and over) and so on and so forth.

The initial motivation for this patch was to simplify something like `opToLegalize (zext(trunc cst))` into `opToLegalize cst`, i.e., the `zext(trunc)` chain (which is a legalization artifact) is in the way of the constant whereas `opToLegalize` may rely on those not being here.

Unfortunately this is not quite enough because legalization artifacts are turned into copies: `opToLegalize zext(trunc cst)` => `opToLegalize (copy cst)`, so the legalization of `opToLegalize` still has to look through something.

@arsenm @volkan @aditya_nandakumar, just adding you as reviewers so that you are aware of this trial. I don't plan to push that.


Repository:
  rL LLVM

https://reviews.llvm.org/D59339

Files:
  lib/CodeGen/GlobalISel/Legalizer.cpp


Index: lib/CodeGen/GlobalISel/Legalizer.cpp
===================================================================
--- lib/CodeGen/GlobalISel/Legalizer.cpp
+++ lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -142,6 +142,7 @@
 
   // Populate Insts
   InstListTy InstList;
+  SmallVector<MachineInstr *, 16> RetryInstList;
   ArtifactListTy ArtifactList;
   ReversePostOrderTraversal<MachineFunction *> RPOT(&MF);
   // Perform legalization bottom up so we can DCE as we legalize.
@@ -195,6 +196,8 @@
   };
   bool Changed = false;
   do {
+    assert(RetryInstList.empty() &&
+           "Retry instructions should have been pushed into InstList");
     while (!InstList.empty()) {
       MachineInstr &MI = *InstList.pop_back_val();
       assert(isPreISelGenericOpcode(MI.getOpcode()) && "Expecting generic opcode");
@@ -209,10 +212,14 @@
       // Error out if we couldn't legalize this instruction. We may want to
       // fall back to DAG ISel instead in the future.
       if (Res == LegalizerHelper::UnableToLegalize) {
-        Helper.MIRBuilder.stopObservingChanges();
-        reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
-                           "unable to legalize instruction", MI);
-        return false;
+        if (ArtifactList.empty()) {
+          Helper.MIRBuilder.stopObservingChanges();
+          reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
+                             "unable to legalize instruction", MI);
+          return false;
+        }
+        LLVM_DEBUG(dbgs() << "Artifacts may prevent legalization to happen\n");
+        RetryInstList.push_back(&MI);
       }
       Changed |= Res == LegalizerHelper::Legalized;
     }
@@ -241,6 +248,9 @@
       else
         InstList.insert(&MI);
     }
+    for (MachineInstr *MI : RetryInstList)
+      InstList.insert(MI);
+    RetryInstList.clear();
   } while (!InstList.empty());
 
   // For now don't support if new blocks are inserted - we would need to fix the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59339.190528.patch
Type: text/x-patch
Size: 1952 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190313/a251e707/attachment.bin>