<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Oct 2, 2014 at 2:41 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</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"><span class="">----- Original Message -----<br>
> From: "Richard Smith" <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Cc: "cfe commits" <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>><br>
</span><div><div class="h5">> Sent: Monday, September 29, 2014 2:09:07 PM<br>
> Subject: Re: r217349 - Add __builtin_assume and __builtin_assume_aligned using @llvm.assume.<br>
><br>
> On Thu, Sep 25, 2014 at 9:19 AM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>
> wrote:<br>
><br>
><br>
> ----- Original Message -----<br>
> > From: "Richard Smith" < <a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a> ><br>
> > To: "Hal Finkel" < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>
> > Cc: "cfe commits" < <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a> ><br>
> > Sent: Wednesday, September 24, 2014 11:22:47 PM<br>
><br>
><br>
> > Subject: Re: r217349 - Add __builtin_assume and<br>
> > __builtin_assume_aligned using @llvm.assume.<br>
> ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> > On Sun, Sep 7, 2014 at 3:58 PM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>
> > wrote:<br>
> ><br>
> ><br>
> > Author: hfinkel<br>
> > Date: Sun Sep 7 17:58:14 2014<br>
> > New Revision: 217349<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217349&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217349&view=rev</a><br>
> > Log:<br>
> > Add __builtin_assume and __builtin_assume_aligned using<br>
> > @llvm.assume.<br>
> ><br>
> > This makes use of the recently-added @llvm.assume intrinsic to<br>
> > implement a<br>
> > __builtin_assume(bool) intrinsic (to provide additional information<br>
> > to the<br>
> > optimizer). This hooks up __assume in MS-compatibility mode to<br>
> > mirror<br>
> > __builtin_assume (the semantics have been intentionally kept<br>
> > compatible), and<br>
> > implements GCC's __builtin_assume_aligned as assume((p - o) & mask<br>
> > ==<br>
> > 0). LLVM<br>
> > now contains special logic to deal with assumptions of this form.<br>
> ><br>
> > Added:<br>
> > cfe/trunk/test/CodeGen/builtin-assume-aligned.c<br>
> > cfe/trunk/test/Sema/builtin-assume-aligned.c<br>
> > cfe/trunk/test/SemaCXX/builtin-assume-aligned-tmpl.cpp<br>
> > Modified:<br>
> > cfe/trunk/docs/LanguageExtensions.rst<br>
> > cfe/trunk/include/clang/Basic/Builtins.def<br>
> > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> > cfe/trunk/include/clang/Sema/Sema.h<br>
> > cfe/trunk/lib/AST/ExprConstant.cpp<br>
> > cfe/trunk/lib/CodeGen/CGBuiltin.cpp<br>
> > cfe/trunk/lib/CodeGen/CGExpr.cpp<br>
> > cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
> > cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> > cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
> > cfe/trunk/test/CodeGen/builtin-assume.c<br>
> > cfe/trunk/test/Sema/builtin-assume.c<br>
> ><br>
> > Modified: cfe/trunk/docs/LanguageExtensions.rst<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=217349&r1=217348&r2=217349&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=217349&r1=217348&r2=217349&view=diff</a><br>
> > ==============================================================================<br>
> > --- cfe/trunk/docs/LanguageExtensions.rst (original)<br>
> > +++ cfe/trunk/docs/LanguageExtensions.rst Sun Sep 7 17:58:14 2014<br>
> > @@ -1228,8 +1228,9 @@ Builtin Functions<br>
> > Clang supports a number of builtin library functions with the same<br>
> > syntax as<br>
> > GCC, including things like ``__builtin_nan``,<br>
> > ``__builtin_constant_p``,<br>
> > ``__builtin_choose_expr``, ``__builtin_types_compatible_p``,<br>
> > -``__sync_fetch_and_add``, etc. In addition to the GCC builtins,<br>
> > Clang supports<br>
> > -a number of builtins that GCC does not, which are listed here.<br>
> > +``__builtin_assume_aligned``, ``__sync_fetch_and_add``, etc. In<br>
> > addition to<br>
> > +the GCC builtins, Clang supports a number of builtins that GCC<br>
> > does<br>
> > not, which<br>
> > +are listed here.<br>
> ><br>
> > Please note that Clang does not and will not support all of the GCC<br>
> > builtins<br>
> > for vector operations. Instead of using builtins, you should use<br>
> > the<br>
> > functions<br>
> > @@ -1239,6 +1240,42 @@ implemented directly in terms of :ref:`e<br>
> > <langext-vectors>` instead of builtins, in order to reduce the<br>
> > number<br>
> > of<br>
> > builtins that we need to implement.<br>
> ><br>
> > +``__builtin_assume``<br>
> > +------------------------------<br>
> > +<br>
> > +``__builtin_assume`` is used to provide the optimizer with a<br>
> > boolean<br>
> > +invariant that is defined to be true.<br>
> > +<br>
> > +**Syntax**:<br>
> > +<br>
> > +.. code-block:: c++<br>
> > +<br>
> > + __builtin_assume(bool)<br>
> > +<br>
> > +**Example of Use**:<br>
> > +<br>
> > +.. code-block:: c++<br>
> > +<br>
> > + int foo(int x) {<br>
> > + __builtin_assume(x != 0);<br>
> > +<br>
> > + // The optimizer may short-circuit this check using the<br>
> > invariant.<br>
> > + if (x == 0)<br>
> > + return do_something();<br>
> > +<br>
> > + return do_something_else();<br>
> > + }<br>
> > +<br>
> > +**Description**:<br>
> > +<br>
> > +The boolean argument to this function is defined to be true. The<br>
> > optimizer may<br>
> > +analyze the form of the expression provided as the argument and<br>
> > deduce from<br>
> > +that information used to optimize the program. If the condition is<br>
> > violated<br>
> > +during execution, the behavior is undefined. The argument itself<br>
> > is<br>
> > never<br>
> > +evaluated, so any side effects of the expression will be<br>
> > discarded.<br>
> > +<br>
> > +Query for this feature with ``__has_builtin(__builtin_assume)``.<br>
> > +<br>
> > ``__builtin_readcyclecounter``<br>
> > ------------------------------<br>
> ><br>
> ><br>
> > Modified: cfe/trunk/include/clang/Basic/Builtins.def<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=217349&r1=217348&r2=217349&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=217349&r1=217348&r2=217349&view=diff</a><br>
> > ==============================================================================<br>
> > --- cfe/trunk/include/clang/Basic/Builtins.def (original)<br>
> > +++ cfe/trunk/include/clang/Basic/Builtins.def Sun Sep 7 17:58:14<br>
> > 2014<br>
> > @@ -412,6 +412,7 @@ BUILTIN(__builtin_va_start, "vA.", "nt")<br>
> > BUILTIN(__builtin_va_end, "vA", "n")<br>
> > BUILTIN(__builtin_va_copy, "vAA", "n")<br>
> > BUILTIN(__builtin_stdarg_start, "vA.", "n")<br>
> > +BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")<br>
> > BUILTIN(__builtin_bcmp, "iv*v*z", "n")<br>
> > BUILTIN(__builtin_bcopy, "vv*v*z", "n")<br>
> > BUILTIN(__builtin_bzero, "vv*z", "nF")<br>
> > @@ -1173,6 +1174,9 @@ LIBBUILTIN(_Block_object_dispose, "vvC*i<br>
> > // Annotation function<br>
> > BUILTIN(__builtin_annotation, "v.", "tn")<br>
> ><br>
> > +// Invariants<br>
> > +BUILTIN(__builtin_assume, "vb", "n")<br>
> > +<br>
> > // Multiprecision Arithmetic Builtins.<br>
> > BUILTIN(__builtin_addcb, "UcUcCUcCUcCUc*", "n")<br>
> > BUILTIN(__builtin_addcs, "UsUsCUsCUsCUs*", "n")<br>
> ><br>
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=217349&r1=217348&r2=217349&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=217349&r1=217348&r2=217349&view=diff</a><br>
> > ==============================================================================<br>
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Sep 7<br>
> > 17:58:14 2014<br>
> > @@ -451,7 +451,7 @@ def note_strncat_wrong_size : Note<<br>
> > "the terminating null byte">;<br>
> ><br>
> > def warn_assume_side_effects : Warning<<br>
> > - "the argument to __assume has side effects that will be<br>
> > discarded">,<br>
> > + "the argument to %0 has side effects that will be discarded">,<br>
> > InGroup<DiagGroup<"assume">>;<br>
> ><br>
> > /// main()<br>
> > @@ -2078,8 +2078,9 @@ def err_no_accessor_for_property : Error<br>
> > def error_cannot_find_suitable_accessor : Error<<br>
> > "cannot find suitable %select{getter|setter}0 for property %1">;<br>
> ><br>
> > -def err_attribute_aligned_not_power_of_two : Error<<br>
> > +def err_alignment_not_power_of_two : Error<<br>
> > "requested alignment is not a power of 2">;<br>
> > +<br>
> > def err_attribute_aligned_too_great : Error<<br>
> > "requested alignment must be %0 bytes or smaller">;<br>
> > def warn_redeclaration_without_attribute_prev_attribute_ignored :<br>
> > Warning<<br>
> ><br>
> > Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=217349&r1=217348&r2=217349&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=217349&r1=217348&r2=217349&view=diff</a><br>
> > ==============================================================================<br>
> > --- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
> > +++ cfe/trunk/include/clang/Sema/Sema.h Sun Sep 7 17:58:14 2014<br>
> > @@ -8375,6 +8375,7 @@ public:<br>
> > private:<br>
> > bool SemaBuiltinPrefetch(CallExpr *TheCall);<br>
> > bool SemaBuiltinAssume(CallExpr *TheCall);<br>
> > + bool SemaBuiltinAssumeAligned(CallExpr *TheCall);<br>
> > bool SemaBuiltinLongjmp(CallExpr *TheCall);<br>
> > ExprResult SemaBuiltinAtomicOverloaded(ExprResult TheCallResult);<br>
> > ExprResult SemaAtomicOpsOverloaded(ExprResult TheCallResult,<br>
> ><br>
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=217349&r1=217348&r2=217349&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=217349&r1=217348&r2=217349&view=diff</a><br>
> > ==============================================================================<br>
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)<br>
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Sun Sep 7 17:58:14 2014<br>
> > @@ -6092,6 +6092,7 @@ bool IntExprEvaluator::VisitCallExpr(con<br>
> > return Success(Operand, E);<br>
> > }<br>
> ><br>
> > + case Builtin::BI__builtin_assume_aligned:<br>
> > case Builtin::BI__builtin_expect:<br>
> > return Visit(E->getArg(0));<br>
> ><br>
> ><br>
> ><br>
> > We should evaluate (and discard) argument 1 and (if present)<br>
> > argument<br>
> > 2 here too, in case they have side-effects or are non-constant.<br>
><br>
> Actually, I'm having trouble coming up with a test-case for this.<br>
><br>
> First, argument 1 is restricted to being a constant integer, and I<br>
> don't think it can have side-effects. Maybe this is not quite right,<br>
> but currently we get:<br>
> error: argument to '__builtin_assume_aligned' must be a constant<br>
> integer<br>
> return __builtin_assume_aligned((int*) 0xfffff000, (++i, 16), i);<br>
> ^ ~~~~~~~~~<br>
><br>
> For argument 2, this can have side effects, but even when the pointer<br>
> argument is constant, I don't see things being folded (and, thus, I<br>
> don't see side-effects as being dropped). For example:<br>
><br>
> int foob() {<br>
> if (((int*) 0xfffff000) == (int*)0)<br>
> return 0;<br>
> else<br>
> return 1;<br>
> }<br>
><br>
> produces (-O3 -mllvm -disable-llvm-optzns):<br>
><br>
> define i32 @foob() #0 {<br>
> entry:<br>
> ret i32 1<br>
> }<br>
><br>
> but this:<br>
><br>
> int foo5(int i) {<br>
> if ((int*)__builtin_assume_aligned((int*) 0xfffff000, 16, i) ==<br>
> (int*)0)<br>
> return 0;<br>
> else<br>
> return 1;<br>
> }<br>
><br>
> does not fold (and changing argument 2 from i to ++i does lead to a<br>
> dropped side effect). Maybe this related to the fact that<br>
> Expr::HasSideEffects always returns true for CallExprClass?<br>
><br>
><br>
> Try this (in C++11 onwards):<br>
><br>
><br>
> int n;<br>
> constexpr int *p = 0;<br>
> constexpr int k = __builtin_assume_aligned(p, 16, n = 5);<br>
><br>
><br>
> We should reject; I suspect we won't.<br>
<br>
</div></div>But, as it turns out, we do:<br>
<br>
$ cat /tmp/bac.cpp<br>
<span class="">int n;<br>
constexpr int *p = 0;<br>
</span>constexpr int *k =  (int *) __builtin_assume_aligned(p, 16, n = 5);<br>
<br>
$ clang++ -std=c++11 -S -emit-llvm -o - /tmp/bac.cpp<br>
/tmp/bac.cpp:3:29: error: constexpr variable 'k' must be initialized by a constant expression<br>
constexpr int *k =  (int *) __builtin_assume_aligned(p, 16, n = 5);<br>
                    ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
1 error generated.<br>
<span class=""><br>
><br>
><br>
> Hmm, also... should __builtin_assume* be non-constant when the<br>
> assumption does not hold? In C++11 we have a general rule that<br>
> undefined behavior is non-constant, and assumption failure seems<br>
> like it should be UB.<br>
<br>
</span>Agreed, although it seems to be currently not an issue because it does not work regardless.</blockquote><div><br></div><div>Oh, I see. It doesn't work because this code was added to the wrong place:</div><div><br></div><div><span style="font-family:arial,sans-serif;font-size:13px">Modified: cfe/trunk/lib/AST/</span><span style="font-family:arial,sans-serif;font-size:13px">ExprConstant.cpp</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">URL: </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=217349&r1=217348&r2=217349&view=diff" target="_blank" style="font-family:arial,sans-serif;font-size:13px">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=217349&r1=217348&r2=217349&view=diff</a><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">==============================</span><span style="font-family:arial,sans-serif;font-size:13px">==============================</span><span style="font-family:arial,sans-serif;font-size:13px">==================</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">--- cfe/trunk/lib/AST/</span><span style="font-family:arial,sans-serif;font-size:13px">ExprConstant.cpp (original)</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">+++ cfe/trunk/lib/AST/</span><span style="font-family:arial,sans-serif;font-size:13px">ExprConstant.cpp Sun Sep  7 17:58:14 2014</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">@@ -6092,6 +6092,7 @@ bool IntExprEvaluator::</span><span style="font-family:arial,sans-serif;font-size:13px">VisitCallExpr(con</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">     return Success(Operand, E);</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">   }</span><br style="font-family:arial,sans-serif;font-size:13px"><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">+  case Builtin::BI__builtin_assume_</span><span style="font-family:arial,sans-serif;font-size:13px">aligned:</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">   case Builtin::BI__builtin_expect:</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">     return Visit(E->getArg(0));</span><br></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">This is inside IntExprEvaluator, but a call to __builtin_assume_aligned does not have integral type. Maybe revert this hunk?</span></div></div></div></div>