On Sat, 20 Oct 2018 at 11:03, Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Iain Buclaw <ibuc...@gdcproject.org> writes:
> > On 14 October 2018 at 17:29, Richard Sandiford
> > <rdsandif...@googlemail.com> wrote:
> >> [Sorry if this turns out to do be a dup]
> >>
> >> Iain Buclaw <ibuc...@gdcproject.org> writes:
> >>> +/* Clear the DECL_BUILT_IN_CLASS flag on the function in CALLEXP. */
> >>> +
> >>> +static void
> >>> +clear_intrinsic_flag (tree callexp)
> >>> +{
> >>> + tree decl = CALL_EXPR_FN (callexp);
> >>> +
> >>> + if (TREE_CODE (decl) == ADDR_EXPR)
> >>> + decl = TREE_OPERAND (decl, 0);
> >>> +
> >>> + gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
> >>> +
> >>> + DECL_INTRINSIC_CODE (decl) = INTRINSIC_NONE;
> >>> + DECL_BUILT_IN_CLASS (decl) = NOT_BUILT_IN;
> >>> + DECL_FUNCTION_CODE (decl) = BUILT_IN_NONE;
> >>> +}
> >>
> >> It seems wrong that a particular call to a function would change the
> >> function's properties for all calls, and I don't think there's any
> >> expectation in the midend that this kind of change might happen.
> >>
> >> Why's it needed? Whatever the reason, I think it needs to be done in a
> >> different way.
> >>
> >
> > There are some D library math functions that are only treated as
> > built-in during compile-time only. When it comes to run-time code
> > generation, we want to call the D library functions, not the C library
> > (or built-ins).
>
> But e.g. we can treat printf as a built-in and still call the real printf
> without doing this.
>
There's a doing_semantic_analysis_p variable I added a year after I
did this particular function. Said variable is set to true from the
beginning till the end of the semantic passes, so that can be used as
a gate for expanding CTFE-only intrinsics. So I think that would be
problem solved here.
> >>> +static tree
> >>> +expand_intrinsic_sqrt (intrinsic_code intrinsic, tree callexp)
> >>> +{
> >>> + tree arg = CALL_EXPR_ARG (callexp, 0);
> >>> + tree type = TYPE_MAIN_VARIANT (TREE_TYPE (arg));
> >>> +
> >>> + /* arg is an integral type - use double precision. */
> >>> + if (INTEGRAL_TYPE_P (type))
> >>> + arg = fold_convert (double_type_node, arg);
> >>> +
> >>> + /* Which variant of __builtin_sqrt* should we call? */
> >>> + built_in_function code = (intrinsic == INTRINSIC_SQRT) ? BUILT_IN_SQRT
> >>> :
> >>> + (intrinsic == INTRINSIC_SQRTF) ? BUILT_IN_SQRTF :
> >>> + (intrinsic == INTRINSIC_SQRTL) ? BUILT_IN_SQRTL : END_BUILTINS;
> >>> +
> >>> + gcc_assert (code != END_BUILTINS);
> >>> + return call_builtin_fn (callexp, code, 1, arg);
> >>> +}
> >>
> >> Converting integers to double precision isn't right for SQRTF and SQRTL.
> >> But how do such calls reach here? It looks like the deco strings in the
> >> function entries provide types for the 3 sqrt intrinsics, is that right?
> >> Does that translate to a well-typed FUNCTION_DECL, or do the function
> >> types use some opaque types instead? If the intrinsic function itself
> >> is well-typed then an incoming CALLEXP with integer arguments would be
> >> invalid.
> >>
> >
> > As with pow(), I'll have to check by running this through the
> > testsuite to see what fails and why.
> >
> > From memory, the reason is likely some requirement of CTFE, where this
> > call *must* be const folded at compile-time, which may not happen if
> > the types are wrong.
>
> The reason for picking this one in particular is that the INTRINSIC_SQRT{,F,L}
> FUNCTION_DECLs looked like they would have correct types, so any CALL_EXPR
> tree with mismatched types would be invalid in terms of GCC internals.
> If that happens, it sounds like there's a bug earlier on.
>
I think for this, it's just an initial lack of trust that D's CTFE
will pass to us the right thing.
Yes, the deco (function signature) matches explictly the float,
double, and real overloads, and calling `sqrt(2)` will result in a
compile-time error, as `int` is an ambiguous match.
I've just removed this though stopped should of adding hard asserts
that `!INTEGRAL_TYPE_P (type)`. If the parameter passed somehow ends
up being an integer, no folding will occur, resulting in a compiler
error that the built-in was not evaluated.
Regards.
--
Iain
