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

[Manman Ren]

On Dec 18, 2012, at 10:26 AM, Chandler Carruth wrote:

> 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.

Yes, MemDep should conservatively approximate the reachability analysis. And looks like LoopInfo does not always provide a conservative answer.
I will try to come up with a conservative answer.

Thanks for reviewing,
Manman

> 
> 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/acafce03/attachment.html>