[PATCH] D19486: Keep dead inst copy from being deleted only when the inst is rematerializable

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/LiveRangeEdit.cpp:331
@@ -330,1 +330,3 @@
+    // allocations of the func are done.
+    if (isOrigDef && DeadRemats && TII.isTriviallyReMaterializable(MI, AA)) {
       LiveInterval &NewLI = createEmptyIntervalFrom(Dest);
----------------
wmi wrote:
> qcolombet wrote:
> > Don't we already check that MI is triviallyReMaterializable before trying to rematerialize?
> > I.e., this is just an optimization to keep DeadRemats small, not the actual fix, right?
> > 
> > In other words, if this is the proposed fix, I think I miss something!
> This patch is to fix the problems by keeping DeadRemats small.
> 
> For instruction like vreg1 = vreg2 + 3 which is not triviallyReMaterializable because it has a vreg use on rhs, if it is dead, previously we still put it in DeadRemats and fail to update the live interval after we delete it in postOptimization. This will cause verifier error. 
> 
> Now we reduce the DeadRemats set to dead instructions which will truly possible to be used in rematerialization, we don't have to worry live interval update because instructions in DeadRemats won't have vreg use. vreg dest is a dummy register and its live interval has been properly set.  
> 
> In other words, keep instructions in DeadRemats to a limited form enforced by triviallyReMaterializable, we can simplify some live interval update hassle and keep the machine verifier silent. 
That sounds more like a workaround for a more general liveness problem.
Those instructions are not supposed to be there and they *must* not affect the liveness of other the variables referenced in those instructions.

That being said, I could live with the fact that we do not push instructions that we know we won’t be able to rematerialize. But, this should not be considered to be the fix for the other problem. In other words, we need to find a way to properly update the liveness when we kill, but keep those instructions.


Repository:
  rL LLVM

http://reviews.llvm.org/D19486