[llvm-commits] [PATCH] Fix a miscompile in memory dependency analysis

[Chandler Carruth]

Sorry for delays reviewing this, but I've been swamped with other patches.

I'm at a bit of a loss regarding your approach here:

- I don't think MemoryDependenceAnalysis can depend on LoopInfo -- it's
used in *many* places where that isn't available.
- We can't rely on LoopInfo to describe all backedges as LoopInfo doesn't
represent unnatural loops -- essentially it isn't guaranteed to find all
cycles.

I think fundamentally, MemDep needs to use a conservative reachability
analysis. We can't approximate this, or we will just miscompile ever more
subtle code patterns.

On Tue, Dec 18, 2012 at 10:16 AM, Manman Ren <mren at apple.com> wrote:

>
> ping
>
>
>
> On Dec 14, 2012, at 3:01 PM, Manman Ren wrote:
>
> >
> > ping
> >
> > On Dec 13, 2012, at 10:34 AM, Manman Ren <mren at apple.com> wrote:
> >
> >>
> >> Given input:
> >> while.body:
> >> call void @RunInMode(i32 100) nounwind
> >> %1 = load i32* %tasksIdle, align 4
> >> %tobool = icmp eq i32 %1, 0
> >> br i1 %tobool, label %while.cond.backedge, label %if.then
> >>
> >> if.then:
> >> store i32 0, i32* %tasksIdle, align 4
> >> call void @TimerCreate(i32* %shouldExit) nounwind
> >> br label %while.cond.backedge
> >>
> >> while.cond.backedge:
> >> %2 = load i32* %shouldExit, align 4
> >> %cmp = icmp eq i32 %2, 0
> >> br i1 %cmp, label %while.body, label %while.cond.while.end_crit_edge
> >>
> >> Make sure we are not returning "call RunInMode" does not modify
> %shouldExit.
> >> Currently, we optimize this into
> >> while.cond.backedge:                              ; preds = %if.then,
> %while.body
> >> %1 = phi i32 [ %.pre, %if.then ], [ 0, %while.body ]
> >> %cmp = icmp eq i32 %1, 0
> >> br i1 %cmp, label %while.body, label %while.cond.while.end_crit_edge
> >>
> >> The proper fix is to check all uses of %shouldExit as long as there is
> a possible
> >> path from the use to "call RunInMode". The proposed patch uses LoopInfo
> to approximate the reachability analysis.
> >> Suggestions on how to approximate this are welcome.
> >>
> >> Please review, Thanks,
> >>
> >> Manman
> >>
> >> <mem_dep.patch>_______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121218/3da1e5af/attachment.html>