<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 20 March 2015 at 12:25, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">If you want this to work we have serious problems, since the same can be said for simple structs as well:<span class=""><br><div><br></div><div>struct S00 {</div><div> char mC_0;</div><div> short mS_1;</div><div> int mI_2;</div><div> int mI_3;</div><div>};</div></span><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px">void test_s00b(struct S00* a, struct S00* b)</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px">{</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"> a->mI_2=1;</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"> b->mI_3=2;</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"> // c++11: NoAlias</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"> // llvm: NoAlias</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"> // future: NoAlias</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px">}</span><br style="color:rgb(33,33,33);font-family:monospace,monospace;font-size:13.1999998092651px;line-height:19.7999992370605px"><div><div><br></div><div>You can play the same trick here, and change offset of b so that mi_2 and mi_3 are in the same place, and both are ints.</div><div><br></div><div><br></div><div>(Jeroen produced these results saying they are NoAlias).<br></div></div></div></blockquote><div><br></div><div>Oh. :-( Yeah, we need this to work. There isn't any prohibition against using getelementptr with negative indices to go from an element pointer to the container pointer (eg. from int S00::*mI_2 to struct S00*). std::vector does this, for one. Sorry.</div><div><br></div><div>There may be ways to achieve this optimization anyways. If "struct S00* a" means that it needs to be a valid struct S00* pointer at the language level, we can annotate it somehow ala. TBAA. Also, we can try to look at the alignment and based on that deduce that the incoming pointer can't have been arranged to conflict (ie., pointers have 16-byte alignment, the members are 1-byte apart).</div><div><br></div><div>Nick</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 20, 2015 at 12:09 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On 20 March 2015 at 11:58, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Mar 20, 2015 at 11:41 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:979<br>
@@ -978,1 +978,3 @@<br>
<br>
+/// isActualObject - Return true if V represents an actual object where we can<br>
+/// reason about the offsets directly, like an argument, alloca, or global<br>
----------------<br>
I really don't understand this comment, either from "actual" or "reason about the offsets directly".<br>
<br>
Looking at the uses, I don't see any reason not to use llvm::isIdentifiedObject. I think what you need to know is that [V, V+Offset] does not overlap any other object. isIdentifiedObject should be sufficient for that.<br></blockquote><div><br></div></span><div>It would be, except isIdentifiedObject does not consider Arguments to be identified unless they are noalias or byval.</div><div><br></div><div>We don't care.</div><div>The only case noalias could matter is if they are in a union, and as we've been through on this thread, we don't consider passing addresses of union members to save you.</div><span><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:983<br>
@@ +982,3 @@<br>
+static bool isActualObject(const Value *V) {<br>
+ if (isa<GlobalValue>(V) || isa<Argument>(V) || isa<AllocaInst>(V))<br>
+ return true;<br>
----------------<br>
What about GlobalAlias? GlobalAlias isa GlobalValue.<br></blockquote></span><div>Agreed.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1063<br>
@@ -1054,1 +1062,3 @@<br>
+ // If they have the same offsets, and the underlying object is noalias,<br>
+ // they must not alias<br>
if (GEP1BaseOffset == GEP2BaseOffset &&<br>
----------------<br>
Full stop after "alias".<br></blockquote></span><div>Fixed</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1069<br>
@@ +1068,3 @@<br>
+ } else {<br>
+ // If these are both arguments, global variable, or alloca, and the<br>
+ // [offset, offset+size] ranges don't overlap, they can't alias<br>
----------------<br>
Extra space between "both" and "arguments".<br></blockquote></span><div>Fixed </div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1069<br>
@@ +1068,3 @@<br>
+ } else {<br>
+ // If these are both arguments, global variable, or alloca, and the<br>
+ // [offset, offset+size] ranges don't overlap, they can't alias<br>
----------------<br>
nlewycky wrote:<br>
> Extra space between "both" and "arguments".<br>
Tense agreement between "arguments" and "global variable" and "alloca".<br></blockquote><div> </div></span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Fixed<br></blockquote><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
================<br>
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1070<br>
@@ +1069,3 @@<br>
+ // If these are both arguments, global variable, or alloca, and the<br>
+ // [offset, offset+size] ranges don't overlap, they can't alias<br>
<span>+ if (isActualObject(UnderlyingV1) && isActualObject(UnderlyingV2) &&<br>
</span>----------------<br>
Full stop after "alias".<br></blockquote></span><div>Fixed.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: test/Analysis/BasicAA/gep-alias.ll:235<br>
@@ +234,3 @@<br>
+<br>
+; We should be able to determine the second store and the load do not conflict,<br>
+; but the first store and the load are must alias<br>
----------------<br>
But they can, I just need to call @test13 with %a = %b + 13 . </blockquote><div><br></div><div><br></div></span><div>I'm not sure i see how this works.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Did you mean to check whether arguments are noalias?<br>
<br></blockquote></span><div>No.</div><div>The original testcase is:<br><div>// struct</div><span><div>struct S00 {</div><div> char mC_0;</div><div> short mS_1;</div><div> int mI_2;</div><div> int mI_3;</div><div>};</div><div><br></div></span><div>// array member</div><div>// ------------</div><span><div>struct S03 {</div><div> short mS_0;</div><div> struct S00 mS00_1[4];</div><div> short mS_2;</div><div>};</div><div><br></div><div>int test_s03c(struct S03* a, struct S03* b)</div><div>{</div><div> a->mS00_1[0].mI_2=1;</div><div> b->mS00_1[1].mI_2=2;</div><div> return a->mS00_1[0].mI_2;</div><div>} <br></div></span></div><div><br></div><div>I don't see how you an make these conflict, no matter what you pass in, because the [1] prevents it.</div></div></div></div></blockquote><div><br></div></div></div><div>All I need to do is arrange that &a->mS00_1[0].mI_2 == &b->mS00_1[1].mI_2.</div><div><br></div><div><div>void test() {</div><div> int mI_2;</div><div> char *a = (char*)&mI_2, *b = (char*)&mI_2;</div><div> a -= offsetof(S03, mS00_1[0].mI_2);</div><div> b -= offsetof(S03, mS00_1[1].mI_2);</div><div> test_s03c((S03*)a, (S03*)b);</div><div>}</div></div><span><font color="#888888"><div><br></div><div>Nick<br></div></font></span><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
%6 is %b+17 (aka gep %5, i32 0, i32 1, i32 2)<br>
%10 loads %a+5 (aka gep %a, i32 0, i32 1, i32 0, i32 2)<br>
<br></blockquote><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
[I assume one byte of padding in %struct.S00 after the i16.]<br>
<br>
================<br>
Comment at: test/Analysis/BasicAA/gep-alias.ll:236<br>
@@ +235,3 @@<br>
+; We should be able to determine the second store and the load do not conflict,<br>
+; but the first store and the load are must alias<br>
+define i32 @test13(%struct.S03* %a, %struct.S03* %b) {<br>
----------------<br>
Full stop after "alias".<br>
<div><div><br>
<a href="http://reviews.llvm.org/D8487" target="_blank">http://reviews.llvm.org/D8487</a><br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></span></div><br></div></div>
</blockquote></span></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div>