r217349 - Add __builtin_assume and __builtin_assume_aligned using @llvm.assume.

[Hal Finkel]

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

Agreed; Thanks for all of your help with this. r218992 should address this (and the evaluation in general) in a more-proper manner.

 -Hal

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory