[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:135
+        if (MI.hasUnmodeledSideEffects() && Next != MBB.end())
+          BuildInstOrderMap(Next, IOM);
+        continue;
----------------
danielcdh wrote:
> skatkov wrote:
> > Why don't you refresh SawStore here?
> Not sure if I understand the comment. SawStore is updated in MI.isSafeToMove. Do you mean that I can remove the previous "SawStore = BuildInstOrderMap", and set SawStore's initial value to "false"?
My understanding is that if you meet that MI has side effect and it is a barrier you will not move later instruction through this one. SawStore is also a kind of barrier and you cannot move instruction which intersects with the store instruction. After you set the new barrier you can reset SawStore. Say if you saw store before new barrier and there is no stores after the barrier you are free to move instruction which do not intersect with stores. However if SawStore has been set to true once I do not see the place where it can be to reset to false. Do I miss anything?

However the overall logic around SawStore is not clear to me. SawStore is set in BuildInstOrderMap. So it detects where there is a store in the tail. It would prevent us re-ordering instruction (we move instruction up always) even if store is located later. Is it expected?


https://reviews.llvm.org/D32563