Hello,
I'm taking a really good look at the MemoryDependenceAnalysis pass, but
I'm
slightly confused about one little thing. I think it's a bug but I'm
not
confident enough to submit a bug report.
Is there a reason why read-only calls are considered to "clobber"
pointers
in the non-load case (which is what gets returned due to the fall-through in
the switch -- see below). It seems this should be returning a "def"
instead. My thinking is the code should look like this (from line 288 on,
+++ marks addition):
// See if this instruction (e.g. a call or vaarg) mod/ref's the
pointer.
switch (AA->getModRefInfo(Inst, MemPtr, MemSize)) {
case AliasAnalysis::NoModRef:
// If the call has no effect on the queried pointer, just ignore it.
continue;
case AliasAnalysis::Mod:
// If we're in an invariant region, we can ignore calls that ONLY
// modify the pointer.
if (InvariantTag) continue;
return MemDepResult::getClobber(Inst);
case AliasAnalysis::Ref:
// If the call is known to never store to the pointer, and if this is
a
// load query, we can safely ignore it (scan past it).
if (isLoad)
continue;
+++ return MemDepResult::getDef(Inst);
default:
// Otherwise, there is a potential dependence. Return a clobber.
return MemDepResult::getClobber(Inst);
}
If this seems right to you too, I've attached the patch. If this isn't
right, can someone please explain the logic?
Thanks,
Marc
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100716/9a6642ab/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MDA.patch
Type: text/x-patch
Size: 537 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100716/9a6642ab/attachment.bin>
Since isLoad == false means we're looking at a store, what this does is making the store *p depend on the load *p. This is correct -- you can't move store before load, otherwise load will start returning a different value. Eugene On Fri, Jul 16, 2010 at 5:43 PM, Marc de Kruijf <dekruijf at cs.wisc.edu> wrote:> Hello, > > I'm taking a really good look at the MemoryDependenceAnalysis pass, but I'm > slightly confused about one little thing. I think it's a bug but I'm not > confident enough to submit a bug report. > > Is there a reason why read-only calls are considered to "clobber" pointers > in the non-load case (which is what gets returned due to the fall-through in > the switch -- see below). It seems this should be returning a "def" > instead. My thinking is the code should look like this (from line 288 on, > +++ marks addition): > > // See if this instruction (e.g. a call or vaarg) mod/ref's the > pointer. > switch (AA->getModRefInfo(Inst, MemPtr, MemSize)) { > case AliasAnalysis::NoModRef: > // If the call has no effect on the queried pointer, just ignore it. > continue; > case AliasAnalysis::Mod: > // If we're in an invariant region, we can ignore calls that ONLY > // modify the pointer. > if (InvariantTag) continue; > return MemDepResult::getClobber(Inst); > case AliasAnalysis::Ref: > // If the call is known to never store to the pointer, and if this is > a > // load query, we can safely ignore it (scan past it). > if (isLoad) > continue; > +++ return MemDepResult::getDef(Inst); > default: > // Otherwise, there is a potential dependence. Return a clobber. > return MemDepResult::getClobber(Inst); > } > > If this seems right to you too, I've attached the patch. If this isn't > right, can someone please explain the logic? > > Thanks, > Marc > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >
Yes, I'm not arguing that there is a dependence, just that it's not a clobber dependence. The case of a load is already considered earlier in that function and with isLoad == false it returns MemDepResult::getDef(). My question is: why should a read-only call (which yields AliasAnalysis::Ref and is handled in this code fragment) be any different from e.g. a load. Isn't a read-only call effectively just a series of loads from a memory-dependence perspective? In other words, why does this code fragment return MemDepResult::getClobber() instead of MemDepResult::getDef() for a read-only call? On Sat, Jul 17, 2010 at 6:18 PM, Eugene Toder <eltoder at gmail.com> wrote:> Since isLoad == false means we're looking at a store, what this does > is making the store *p depend on the load *p. This is correct -- you > can't move store before load, otherwise load will start returning a > different value. > > Eugene > > On Fri, Jul 16, 2010 at 5:43 PM, Marc de Kruijf <dekruijf at cs.wisc.edu> > wrote: > > Hello, > > > > I'm taking a really good look at the MemoryDependenceAnalysis pass, but > I'm > > slightly confused about one little thing. I think it's a bug but I'm not > > confident enough to submit a bug report. > > > > Is there a reason why read-only calls are considered to "clobber" > pointers > > in the non-load case (which is what gets returned due to the fall-through > in > > the switch -- see below). It seems this should be returning a "def" > > instead. My thinking is the code should look like this (from line 288 > on, > > +++ marks addition): > > > > // See if this instruction (e.g. a call or vaarg) mod/ref's the > > pointer. > > switch (AA->getModRefInfo(Inst, MemPtr, MemSize)) { > > case AliasAnalysis::NoModRef: > > // If the call has no effect on the queried pointer, just ignore > it. > > continue; > > case AliasAnalysis::Mod: > > // If we're in an invariant region, we can ignore calls that ONLY > > // modify the pointer. > > if (InvariantTag) continue; > > return MemDepResult::getClobber(Inst); > > case AliasAnalysis::Ref: > > // If the call is known to never store to the pointer, and if this > is > > a > > // load query, we can safely ignore it (scan past it). > > if (isLoad) > > continue; > > +++ return MemDepResult::getDef(Inst); > > default: > > // Otherwise, there is a potential dependence. Return a clobber. > > return MemDepResult::getClobber(Inst); > > } > > > > If this seems right to you too, I've attached the patch. If this isn't > > right, can someone please explain the logic? > > > > Thanks, > > Marc > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100717/bde953fa/attachment.html>