<div dir="ltr">If you want this to work we have serious problems, since the same can be said for simple structs as well:<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 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><br></div><div><br></div><div><br></div><div><br></div></div></div><div class="gmail_extra"><br><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 class="h5">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 class="HOEnZb"><font color="#888888"><div><br></div><div>Nick<br></div></font></span><span class=""><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>