<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 08/27/2015 07:40 PM, David Majnemer
via llvm-commits wrote:<br>
</div>
<blockquote
cite="mid:CAL7bZ_cgWuD+Q7VLUd+Yvi+WB_KvFNybvkT6jkkYP6eyBfuZeg@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Aug 27, 2015 at 5:37 PM, Hal
Finkel <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"><span
class="">----- Original Message -----<br>
> From: "David Majnemer" <<a
moz-do-not-send="true"
href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>><br>
> To: "Hal Finkel" <<a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Cc: "llvm-commits" <<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>><br>
</span>
<div>
<div class="h5">> Sent: Thursday, August 27, 2015
7:16:40 PM<br>
> Subject: Re: [llvm] r246232 - [ValueTracking]
readnone CallInsts are fair game for speculation<br>
><br>
><br>
> On Thu, Aug 27, 2015 at 5:09 PM, Hal Finkel <
<a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>
><br>
> wrote:<br>
><br>
><br>
> ----- Original Message -----<br>
> > From: "David Majnemer" < <a
moz-do-not-send="true"
href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>
><br>
> > To: "Hal Finkel" < <a
moz-do-not-send="true" href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>
><br>
> > Cc: "llvm-commits" < <a
moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>
><br>
> > Sent: Thursday, August 27, 2015 6:57:32 PM<br>
> > Subject: Re: [llvm] r246232 -
[ValueTracking] readnone CallInsts<br>
> > are fair game for speculation<br>
> ><br>
> > On Thu, Aug 27, 2015 at 4:20 PM, Hal Finkel
< <a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>
><br>
> > wrote:<br>
> ><br>
> ><br>
> > Hi David,<br>
> ><br>
> > I'd really like this to be a correct change,
but I'm not sure that<br>
> > it<br>
> > is. I'd like it to be correct because we add
readnone for functions<br>
> > that, at the C level, have
__attribute__((const)), and I believe<br>
> > those can be speculated (assuming my
understanding of GCC's<br>
> > semantics is correct). However, in terms of
readnone itself, we<br>
> > need<br>
> > to be careful about undefined behavior.
Specifically,<br>
> ><br>
> > int divide(int x, int y) { return x/y; }<br>
> ><br>
> > is readnone, according to our definition,
but could trap if<br>
> > speculated with y == 0.<br>
> ><br>
> > Thanks again,<br>
> > Hal<br>
> ><br>
> > Would it be correct for a function which
non-deterministically<br>
> > traps<br>
> > (but which does not read or write memory) to
be marked readnone?<br>
><br>
> I believe this is correct, as the IR is currently
defined. Plus<br>
> FunctionAttrs will happily do this today (that
divide function is<br>
> marked readnone).<br>
><br>
> > If<br>
> > so, I think our optimizer is in quite a bit
of trouble.<br>
><br>
> :-)<br>
><br>
> ><br>
> > I'll also note that we go out of our way not
to mark things like<br>
> > rdrand/rdseed as readnone despite them not
reading any memory. I'd<br>
> > prefer that readnone should imply that
speculation, CSE, etc. are<br>
> > safe. Would it be problematic to let
'divide' not be readnone?<br>
> ><br>
><br>
> I'd really prefer that we kept the speculation
safety and the pointer<br>
> aliasing properties separate, and so I don't want
to mark divide as<br>
> potentially accessing memory. I think that we
should just add a<br>
> safe_to_speculate attribute (modulo
bikeshedding), and infer it by<br>
> running isSafeToSpeculativelyExecute over the
function in<br>
> FunctionAttrs just like we do for readnone and
mayWriteToMemory,<br>
> etc.<br>
><br>
><br>
><br>
> I'm not at all opposed to this resolution.
However, I am bit<br>
> confused. The langref states:<br>
> "It does not write through any pointer arguments
(including byval<br>
> arguments) and never changes any state visible to
callers. This<br>
> means that it cannot unwind exceptions by calling
the C++ exception<br>
> throwing methods."<br>
><br>
><br>
> It seems that readnone functions imply nounwind
and "never changes<br>
> any state visible to callers". Would trapping
count?<br>
><br>
<br>
</div>
</div>
I believe that it did not count prior to r246232 :-)<br>
<br>
Seriously, however, I don't disagree with you. Trapping
certainly seems like it could be considered a behavior
visible to the caller. However, you could also argue that
trapping terminates the program, and the caller can't
actually observe that. This latter viewpoint seems to be
the one guiding what we've currently implemented, and it
seems like a relatively-useful distinction.<br>
</blockquote>
<div><br>
</div>
<div>I'm starting to split this gordian knot. One last
question: the 'nounwind' implication should be part of the
new attribute, not the old one right?<br>
</div>
</div>
</div>
</div>
</blockquote>
Sounds like you've reached the conclusion I would hope for. Thanks
for doing the work to introduce the speculatable attribute. That
will be really useful. <br>
<br>
For the record, let me say that having readnone imply that a
function couldn't fault would be really problematic for me. A
couple of examples:<br>
- readnone and readonly are documented as only describing memory
*visible to the program*. Functions so marked can still read and
write memory which is provably noalias with every pointer visible to
the program. This is really useful for caching and memoization
schemes inside runtime code where the implementation details aren't
visible to compiled code. Just because the memory isn't visible
(noalias) doesn't imply the dereferenceability of that memory isn't
controlled by actions taken by the compiled code.<br>
- The division by zero case that already got mentioned. In
particular, there might have been a guarded divide within the
function whose guard was disproved by interprocedural analysis.
Then lifting the faulting divide above the control dependence in the
caller would be bad. We could require that all attributes be
stripped when removing the guard within the function, but that seems
less than helpful from an optimization perspective.<br>
<br>
Philip<br>
</body>
</html>