[PATCH] D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 7 21:24:56 PST 2018


mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:198
+
+    if (LoopIsLive) {
+      BlocksInLoopAfterFolding.insert(L.getLoopLatch());
----------------
anna wrote:
> mkazantsev wrote:
> > anna wrote:
> > > reames wrote:
> > > > Not clear to me why you need this bit of code in this patch.  
> > > If `LoopIsLive` is false, we should just return at this point now, right? For this patch, I believe this chunk of code is not needed. Am I missing something?
> > You are right. But this patch is just collecting all required data for follow-up patches https://reviews.llvm.org/D54023 https://reviews.llvm.org/D54025
> > 
> > My idea was to collect *all* required data in this patch and then add transforms that use it one by one, without adding more analysis. If you think it would be easier to review if the data collection is also added incrementally, I can rework the change.
> pls add this with the patch that deletes the loop, instead of in this patch. I'd like to reduce the complexity of this patch.
Sorry for misleading you earlier. This logic works if we *do not* delete the loop. I cannot remove it. It is required to detect the cases we are not supporting yet.



https://reviews.llvm.org/D54021





More information about the llvm-commits mailing list