<br>On Sun, Jul 18, 2010 at 5:40 PM, Eugene Toder <span dir="ltr"><<a href="mailto:eltoder@gmail.com">eltoder@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

Sorry, I misunderstood the question.<br>
The difference between a load and a read-only call is that load can be<br>
used as the value of the memory location. E.g. DeadStoreElimination<br>
pass removes a store that stores a just loaded value back into the<br>
same location. To do this it checks if the stored value is the value<br>
of load. Read-only call cannot be used like this.<br>
This being said, I don't know if this is the reason for the current<br>
logic, and DSE won't break with your change -- it checks if Def is<br>
indeed a load.<br></blockquote><div><br></div><div>Well as a matter of consistency I think a Def is more appropriate.  DSE is just one consumer of memdep analysis, and the analysis I have in mind does not use memdep in the way that DSE or GVN does. </div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
A related question -- it looks like memdep returns Def for store to<br>
load dependence even when they MayAlias. Should it return Clobber for<br>
MayAlias and Def for MustAlias? With the current logic DSE has to<br>
check that SI->getPointerOperand() == DepLoad->getPointerOperand(),<br>
though this is not strictly necessary -- they can be different values<br>
but be MustAlias.<br></blockquote><div><br></div><div>Personally, I find the Def vs. Clobber distinction rather arbitrary and not very intuitive.  A may-alias store "clobbers" but a must-alias store is a "def"?  I understand why there is a distinction, but the naming is poor.  The comments say the naming is "unusual but pragmatic", but I have to disagree and say that the information is pragmatic only for a particular set of consumer analyses/transformations, and not across all possible consumers.  </div>

<div><br></div><div>As an aside, a newcomer's understanding is also compromised trying to compress everything into the two bits inside MemDepResult.  I understand the desire to meet this constraint, but with three bits you could represent may/must-alias with one bit, ref/mod/invalid/nonlocal with the other two bits and it would be easier to work with.</div>

<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<font color="#888888"><br>
Eugene<br>
</font><div><div></div><div class="h5"><br>
On Sun, Jul 18, 2010 at 2:50 AM, Marc de Kruijf <<a href="mailto:dekruijf@cs.wisc.edu">dekruijf@cs.wisc.edu</a>> wrote:<br>
> Yes, I'm not arguing that there is a dependence, just that it's not a<br>
> clobber dependence.  The case of a load is already considered earlier in<br>
> that function and with isLoad == false it returns MemDepResult::getDef().<br>
>  My question is:  why should a read-only call (which yields<br>
> AliasAnalysis::Ref and is handled in this code fragment) be any different<br>
> from e.g. a load.  Isn't a read-only call effectively just a series of loads<br>
> from a memory-dependence perspective?<br>
> In other words, why does this code fragment return<br>
> MemDepResult::getClobber() instead of MemDepResult::getDef() for a read-only<br>
> call?<br>
> On Sat, Jul 17, 2010 at 6:18 PM, Eugene Toder <<a href="mailto:eltoder@gmail.com">eltoder@gmail.com</a>> wrote:<br>
>><br>
>> Since isLoad == false means we're looking at a store, what this does<br>
>> is making the store *p depend on the load *p. This is correct -- you<br>
>> can't move store before load, otherwise load will start returning a<br>
>> different value.<br>
>><br>
>> Eugene<br>
>><br>
>> On Fri, Jul 16, 2010 at 5:43 PM, Marc de Kruijf <<a href="mailto:dekruijf@cs.wisc.edu">dekruijf@cs.wisc.edu</a>><br>
>> wrote:<br>
>> > Hello,<br>
>> ><br>
>> > I'm taking a really good look at the MemoryDependenceAnalysis pass, but<br>
>> > I'm<br>
>> > slightly confused about one little thing.  I think it's a bug but I'm<br>
>> > not<br>
>> > confident enough to submit a bug report.<br>
>> ><br>
>> > Is there a reason why read-only calls are considered to "clobber"<br>
>> > pointers<br>
>> > in the non-load case (which is what gets returned due to the<br>
>> > fall-through in<br>
>> > the switch -- see below).  It seems this should be returning a "def"<br>
>> > instead.  My thinking is the code should look like this (from line 288<br>
>> > on,<br>
>> > +++ marks addition):<br>
>> ><br>
>> >      // See if this instruction (e.g. a call or vaarg) mod/ref's the<br>
>> > pointer.<br>
>> >      switch (AA->getModRefInfo(Inst, MemPtr, MemSize)) {<br>
>> >      case AliasAnalysis::NoModRef:<br>
>> >        // If the call has no effect on the queried pointer, just ignore<br>
>> > it.<br>
>> >        continue;<br>
>> >      case AliasAnalysis::Mod:<br>
>> >        // If we're in an invariant region, we can ignore calls that ONLY<br>
>> >        // modify the pointer.<br>
>> >        if (InvariantTag) continue;<br>
>> >        return MemDepResult::getClobber(Inst);<br>
>> >      case AliasAnalysis::Ref:<br>
>> >        // If the call is known to never store to the pointer, and if<br>
>> > this is<br>
>> > a<br>
>> >        // load query, we can safely ignore it (scan past it).<br>
>> >        if (isLoad)<br>
>> >          continue;<br>
>> > +++    return MemDepResult::getDef(Inst);<br>
>> >      default:<br>
>> >        // Otherwise, there is a potential dependence.  Return a clobber.<br>
>> >        return MemDepResult::getClobber(Inst);<br>
>> >      }<br>
>> ><br>
>> > If this seems right to you too,  I've attached the patch.  If this isn't<br>
>> > right, can someone please explain the logic?<br>
>> ><br>
>> > Thanks,<br>
>> > Marc<br>
>> ><br>
>> > _______________________________________________<br>
>> > LLVM Developers mailing list<br>
>> > <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
>> ><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br>