<div dir="ltr"><div>Ah, I'm sorry. "free" will need to be marked ArgMemOnly, I didn't mention that in the original mail.<br><br>>why do you need to mark printf() as argmemonly?<br></div>Because
printf can access memory pointed to by its arguments (and in the rare
case that James Molloy pointed out, it can modify it too).</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><div dir="ltr"> - Vaivaswatha<br></div></div></div>
<br><div class="gmail_quote">On Mon, Dec 14, 2015 at 11:34 AM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
Vaivaswatha Nagaraj wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>You'll have to mark free as argmemonly + read-write, since the<br>
The intention is to say that free can modify the memory pointed to in its argument. Doesn't ArgMemOnly cover that<br>
already?<br>
</blockquote>
<br></span>
Yes, but I thought you were suggesting against marking free as argmemonly, since you said "Mark malloc/free as (HasInaccessibleState, ReadNone)"?<br>
<br>
A question that will help me assess if I've understood what you have in mind: why do you need to mark printf() as argmemonly?<br>
<br>
-- Sanjoy<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Vaivaswatha<div><div class="h5"><br>
<br>
On Mon, Dec 14, 2015 at 10:59 AM, Sanjoy Das <<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a> <mailto:<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>>> wrote:<br>
<br>
Vaivaswatha Nagaraj via llvm-dev wrote:<br>
> >I'm against adding this as a "subtractive" attribute. We need to add these as new attributes, not as an attribute<br>
that<br>
> makes readonly a little less read only. I believe we're in agreement on this point.<br>
> Just to make sure I understood right, below are the things that need to be done:<br>
> (Approach A)<br>
> 1. We define a new a attribute "HasInaccessibleState" to denote "this function might access globals, but none of these<br>
> globals can alias with any memory location accessible from the IR being optimized".<br>
> 2. Mark malloc/free as (HasInaccessibleState, ReadNone) and printf as (HasInaccessibleState, ArgMemOnly) ...<br>
(similarly<br>
> other libc functions).<br>
<br>
You'll have to mark free as argmemonly + read-write, since the<br>
following transform needs to be illegal:<br>
<br>
val = *ptr<br>
free(ptr)<br>
<br>
=><br>
<br>
free(ptr)<br>
val = *ptr<br>
<br>
In fact, I think you can mark @free as argmemonly *today* without any<br>
problems. There is a theoretical issue on whether a "write" can make<br>
a location no longer dereferenceable, but that's not new.<br>
<br>
> 3. Any function whose definition is not available needs to be marked with "HasInaccessibleState" (conservatively).<br>
> 4. Propagate the flag "HasInaccessibleState" upwards in the call graph. (Do this in FunctionAttrs.cpp?).<br>
> 5. Since ReadNone and ArgMemOnly has now been redfined, all optimizations that rely on these flags for safety now also<br>
> need to check the new "HasInaccessibleState" flag and make sure it isn't present.<br>
> 6. GlobalsAA will be modified according to the diff in the first mail in this email thread.<br>
><br>
> The alternative approach that was discussed would involve the following changes:<br>
> (Approach B)<br>
> 1. Define new attributes AlmostReadNone and AlmostArgMemOnly, (with the "Almost" part denoting that the function may<br>
> accesses globals that are not part of the IR).<br>
> 2. malloc/free would have AlmostReadNone set and printf would have AlmostArgMemOnly set ... (and similarly other libc<br>
> calls).<br>
> 3. In the diff I originally posted for GlobalsAA, the code would check for AlmostReadNone or AlmostArgMemOnly too<br>
(along<br>
> with ReadNone or ArgMemOnly).<br>
><br>
> Approach B seems simpler to me, but I understand the concern about having a "subtractive" attribute which is new<br>
to the<br>
> framework. If there is a consensus on which of these two approaches is the way to go, I can submit a quick prototype<br>
> patch for further review/discussion.<br>
<br>
A third approach is to extend the noalias metadata [1] (I mentioned<br>
this earlier in the thread, but did not do a good job on<br>
expanding on it). Currently noalias metadata can be used to specify<br>
that a certain call or invoke does not read / modify memory locations<br>
belonging to certain *specific* alias domains. We could generalize<br>
this mechanism and specify a new kind of alias domain called (say)<br>
!ir-mem. Calls to @malloc and @printf could be marked with "!noalias<br>
!ir-mem" [2], and we could specify (as an axiom) that no loads and<br>
stores visible at the IR level alias with the !ir-mem domain. The<br>
advantage of this approach is that we won't have to introduce any<br>
fundamentally new first class concepts to LLVM IR (though it would<br>
still be a fair amount of work).<br>
<br>
[1]: <a href="http://llvm.org/docs/LangRef.html#noalias-and-alias-scope-metadata" rel="noreferrer" target="_blank">http://llvm.org/docs/LangRef.html#noalias-and-alias-scope-metadata</a><br>
[2]: We'll also have to introduce an "NoAliasIRMem" attribute, that<br>
can be used denote that all calls to the marked function are !noalias<br>
!ir-mem.<br>
<br>
-- Sanjoy<br>
<br>
<br>
><br>
> Thanks,<br>
><br>
><br>
> - Vaivaswatha<br>
><br>
> On Sat, Dec 12, 2015 at 3:21 AM, Hal Finkel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br></div></div><span class="">
<mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>>> wrote:<br>
><br>
> ----- Original Message -----<br></span><span class="">
> > From: "Joseph Tremoulet" <<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a><br>
<mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>>>><br></span><span class="">
> > To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>>>,<br>
"Mehdi Amini" <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>><br></span><div><div class="h5">
> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>>>><br>
> > Cc: "llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>>><br>
> > Sent: Friday, December 11, 2015 3:35:38 PM<br>
> > Subject: RE: [llvm-dev] RFC: New function attribute HasInaccessibleState<br>
> ><br>
> > Yeah, I'd agree (rewording slightly) that "state which is only<br>
> > modified by external code" is well-defined (and likely to be in the<br>
> > "other" bucket of any individual analysis). I do, as other have,<br>
> > find it odd to redefine readonly and argmemonly in terms of this and<br>
> > require its propagation up the call graph, as opposed to introducing<br>
> > new "writes only external" and "writes only arg and external"<br>
> > attributes.<br>
><br>
> As I stated in some other e-mail, I'm against adding this as a "subtractive" attribute. We need to add these<br>
as new<br>
> attributes, not as an attribute that makes readonly a little less read only. I believe we're in agreement on<br>
this point.<br>
><br>
> -Hal<br>
><br>
> ><br>
> > Thanks<br>
> > -Joseph<br>
> ><br>
> > -----Original Message-----<br></div></div><span class="">
> > From: Hal Finkel [mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>>]<br>
> > Sent: Friday, December 11, 2015 4:00 PM<br></span><span class="">
> > To: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a><br>
<mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>>>><br></span><span class="">
> > Cc: llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>>>; Joseph Tremoulet<br></span><span class="">
> > <<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>>>><br>
> > Subject: Re: [llvm-dev] RFC: New function attribute<br>
> > HasInaccessibleState<br>
> ><br>
> > ----- Original Message -----<br></span><span class="">
> > > From: "Mehdi Amini" <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>> <mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a><br>
<mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>>>><br></span><span class="">
> > > To: "Joseph Tremoulet" <<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a><br>
<mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>>>><br></span><span class="">
> > > Cc: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>>>,<br>
"llvm-dev"<br></span><span class="">
> > > <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>>><br>
> > > Sent: Friday, December 11, 2015 1:28:05 PM<br>
> > > Subject: Re: [llvm-dev] RFC: New function attribute<br>
> > > HasInaccessibleState<br>
> > ><br>
> > ><br>
> > > > On Dec 11, 2015, at 11:16 AM, Joseph Tremoulet<br></span>
> > > > <<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>> <mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a><div><div class="h5"><br>
<mailto:<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>>>> wrote:<br>
> > > ><br>
> > > > <<<<br>
> > > > I may misunderstand, but it seems to me that this solves only<br>
> > > > query<br>
> > > > for aliasing with a pointer known to be pointing only to globals<br>
> > > > defined in the current compilation unit.<br>
> > > > For any pointer which "may point somewhere else”, you won’t be<br>
> > > > able<br>
> > > > to resolve the non-aliasing with the “internal state” for<br>
> > > > malloc/free, right?<br>
> > > ><br>
> > > > To take the original example in this thread:<br>
> > > ><br>
> > > > int *x = malloc(4);<br>
> > > > *x = 2;<br>
> > > > int *y = malloc(4);<br>
> > > > *y = 4;<br>
> > > ><br>
> > > > A pointer analysis can solve this case, but I’m not sure it scale<br>
> > > > inter procedurally and will have a limited impact outside of LTO<br>
> > > > anyway.<br>
> > > >>>><br>
> > > ><br>
> > > > I think you're understanding correctly, but I don't understand<br>
> > > > what<br>
> > > > you're saying will go badly with the malloc example. Quoting the<br>
> > > > start of the thread:<br>
> > > ><br>
> > > > <<<<br>
> > > > The intention behind introducing this attribute is to relax the<br>
> > > > conditions in GlobalsAA as below:<br>
> > > > (this code is in GlobalsAAResult::AnalyzeCallGraph)<br>
> > > > if (F->isDeclaration()) {<br>
> > > > // Try to get mod/ref behaviour from function attributes.<br>
> > > > - if (F->doesNotAccessMemory()) {<br>
> > > > + if (F->doesNotAccessMemory() ||<br>
> > > > F->onlyAccessesArgMemory()) {<br>
> > > > // Can't do better than that!<br>
> > > > } else if (F->onlyReadsMemory()) {<br>
> > > > FunctionEffect |= Ref;<br>
> > > > if (!F->isIntrinsic())<br>
> > > > // This function might call back into the module and<br>
> > > > read a global -<br>
> > > > // consider every global as possibly being read by<br>
> > > > this<br>
> > > > function.<br>
> > > > FR.MayReadAnyGlobal = true;<br>
> > > > } else {<br>
> > > > FunctionEffect |= ModRef;<br>
> > > > // Can't say anything useful unless it's an intrinsic -<br>
> > > > they don't<br>
> > > > // read or write global variables of the kind<br>
> > > > considered<br>
> > > > here.<br>
> > > > KnowNothing = !F->isIntrinsic();<br>
> > > > }<br>
> > > > continue;<br>
> > > > }<br>
> > > > This relaxation allows functions that (transitively) call library<br>
> > > > functions (such as printf/malloc) to still maintain and propagate<br>
> > > > GlobalsAA info. In general, this adds more precision to the<br>
> > > > description of these functions.<br>
> > > > Concerns regarding impact on other optimizations (I'm repeating a<br>
> > > > few examples that Hal mentioned earlier).<br>
> > > ><br>
> > > > 1.<br>
> > > >> A readnone function is one whose output is a function only of<br>
> > > >> its<br>
> > > >> inputs, and if you have this:<br>
> > > >><br>
> > > >> int *x = malloc(4);<br>
> > > >> *x = 2;<br>
> > > >> int *y = malloc(4);<br>
> > > >> *y = 4;<br>
> > > >> you certainly don't want EarlyCSE to replace the second call to<br>
> > > >> malloc with the result of the first (which it will happily do if<br>
> > > >> you mark malloc as readnone).<br>
> > > >>>><br>
> > > ><br>
> > > > It sounded like improving GlobalsAA (and thus disambiguation<br>
> > > > against<br>
> > > > globals) was the explicit goal, and that the concern with the<br>
> > > > malloc<br>
> > > > case was that you don't want EarlyCSE to start combining those<br>
> > > > two<br>
> > > > calls; I may be misunderstanding the code, but I wouldn't expect<br>
> > > > EarlyCSE to start combining those calls just because they have a<br>
> > > > new<br>
> > > > meaningful-only-to-GlobalsAA "almost-readnone" attribute.<br>
> > ><br>
> > > Sure, my point is not that your solution would enable CSE where we<br>
> > > don’t want, but rather that it is not as powerful as what the<br>
> > > attribute “HasInaccessibleState” would model, which I saw as "this<br>
> > > function might access globals, but none of these globals can alias<br>
> > > with any memory location accessible from the IR being optimized”.<br>
> ><br>
> > This is also, essentially, what I had in mind. I think it is<br>
> > sufficiently well defined in this form.<br>
> ><br>
> > -Hal<br>
> ><br>
> > > For instance:<br>
> > ><br>
> > > void foo(int *x) {<br>
> > > int *y = malloc(4);<br>
> > > *x = 2;<br>
> > > }<br>
> > ><br>
> > > If you don’t know anything about x, can you execute the write to *x<br>
> > > before the call to malloc?<br>
> > > This is something that the HasInaccessibleState would allow, but I<br>
> > > don’t believe would be possible with your categorization.<br>
> > ><br>
> > > I’m don’t know how much it matters in practice, but I’d rather be<br>
> > > sure<br>
> > > we’re on the same track about the various tradeoff.<br>
> > ><br>
> > > —<br>
> > > Mehdi<br>
> > ><br>
> > ><br>
> > ><br>
> > > ><br>
> > > ><br>
> > > > To the larger point of whether there are other similar cases that<br>
> > > > extending GlobalsAA wouldn't allow us to optimize -- yes,<br>
> > > > certainly.<br>
> > > > I'm just saying that I think that the notion of "external state"<br>
> > > > is<br>
> > > > much easier to define in the context of a particular analysis<br>
> > > > than<br>
> > > > the IR as a whole, and that I'd expect that coordinating the<br>
> > > > notion<br>
> > > > across analyses would require methods on the analysis API<br>
> > > > explicitly<br>
> > > > for that coordination.<br>
> > > ><br>
> > > ><br>
> > > ><br>
> > > > —<br>
> > > > Mehdi<br>
> > > ><br>
> > ><br>
> > ><br>
> ><br>
> > --<br>
> > Hal Finkel<br>
> > Assistant Computational Scientist<br>
> > Leadership Computing Facility<br>
> > Argonne National Laboratory<br>
> ><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
> _______________________________________________<br>
> LLVM Developers mailing list<br></div></div>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><span class=""><br>
<mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
><br>
><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br></span>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
<br>
</blockquote>
</blockquote></div><br></div>