[PATCH] D46775: [LICM] Preserve DT and LoopInfo specifically

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin added inline comments.


================
Comment at: test/Transforms/LICM/pr37323.ll:1-2
+;RUN: opt -loop-simplify -postdomtree -licm -adce -S -o - %s | FileCheck %s
+;RUN: opt -passes='loop-simplify,require<postdomtree>,require<opt-remark-emit>,loop(licm),function(adce)' -S -o - %s | FileCheck %s
+
----------------
junbuml wrote:
> junbuml wrote:
> > mzolotukhin wrote:
> > > junbuml wrote:
> > > > mzolotukhin wrote:
> > > > > By the way, do we have something like `-verify-dom-tree` for PostDT? It would be cleaner to explicitly verify postdom tree here than calling ADCE and hoping it crashes.
> > > > The same tag (-verify-dom-info) seem to cover PostDT's verifyAnalysis as well.  Using -verify-dom-info, I confirmed that this test crash in PostDT's verifyAnalysis without this change.
> > > Thanks for checking! Also, speaking of reducing the test case further- can we get rid of `-loop-simplify` (i.e. replace the test with the output of `loop-simplify`) and `-adce`?
> > Without this patch, I tried to reproduce the crash in this test case without -loop-simplify and -adce. However, we need both to reproduce the crash.  If we remove -loop-simplify and put passes just like "-postdomtree -licm -adce", then the PDT executed before LICM will be invalidated (not preserved) after "Canonicalize natural loops".  Therefore, we cannot reproduce the crash because the postdom will be re-executed before ADCE. Please see the pass order for "-postdomtree -licm -adce" : 
> >  
> >       Post-Dominator Tree Construction     <-- execute postdom
> >       Dominator Tree Construction
> >       Natural Loop Information
> >       Canonicalize natural loops           <-- postdom is invalidated and not preserved
> >       LCSSA Verifier
> >       Loop-Closed SSA Form Pass
> >       Basic Alias Analysis (stateless AA impl)
> >       Function Alias Analysis Results
> >       Scalar Evolution Analysis
> >       Loop Pass Manager
> >         Loop Invariant Code Motion
> >       Post-Dominator Tree Construction    <--- re-execute postdom
> >       Aggressive Dead Code Elimination    <--- cannot reproduce the crash
> > 
> > We should make the postdom executed after "Canonicalize natural loops".
> > 
> > Even with -verify-loop-info, after licm, we still need a pass which requires postdom (e.g., adce) to verify postdom. Otherwise, PostDT's verifyAnalysis wont be invoked. 
> > 
> Hi Michael,
> 
> Can you please let me know if this test is okay with loop-simplify and adce ? 
> Thanks,
> Jun
Yes, it's ok, thanks for the investigation and explanation!


https://reviews.llvm.org/D46775