<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 25, 2014 at 9:19 AM, 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:0 0 0 .8ex;border-left:1px #ccc 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>
> Sent: Wednesday, September 24, 2014 11:22:47 PM<br>
</span><div><div class="h5">> Subject: Re: r217349 - Add __builtin_assume and __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> > 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 @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 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>
> 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 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 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 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 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 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 is<br>
> never<br>
> +evaluated, so any side effects of the expression will be 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) argument<br>
> 2 here too, in case they have side-effects or are non-constant.<br>
<br>
</div></div>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 don't think it can have side-effects. Maybe this is not quite right, but currently we get:<br>
 error: argument to '__builtin_assume_aligned' must be a constant 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 argument is constant, I don't see things being folded (and, thus, I 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) == (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 dropped side effect). Maybe this related to the fact that Expr::HasSideEffects always returns true for CallExprClass?</blockquote><div><br></div><div>Try this (in C++11 onwards):</div><div><br></div><div>  int n;</div><div>  constexpr int *p = 0;</div><div>  constexpr int k =  __builtin_assume_aligned(p, 16, n = 5);</div><div><br></div><div>We should reject; I suspect we won't.</div><div><br></div><div>Hmm, also... should __builtin_assume* be non-constant when the assumption does not hold? In C++11 we have a general rule that undefined behavior is non-constant, and assumption failure seems like it should be UB.</div></div></div></div>