On Tue, Nov 27, 2018 at 3:52 AM Martin Sebor <mse...@gmail.com> wrote:
>
> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01759.html
>
> If there are no objections or suggestions for tweaks I'll commit
> this updated comment this week.
Please do not commit such changes w/o approval.
Does the new comments match existing usage, for example in gimplify.c
where we do
static void
build_stack_save_restore (gcall **save, gcall **restore)
{
tree tmp_var;
*save = gimple_build_call (builtin_decl_implicit (BUILT_IN_STACK_SAVE), 0);
tmp_var = create_tmp_var (ptr_type_node, "saved_stack");
gimple_call_set_lhs (*save, tmp_var);
*restore
= gimple_build_call (builtin_decl_implicit (BUILT_IN_STACK_RESTORE),
1, tmp_var);
}
?
+/* Return the tree node for the built-in function declaration corresponding
+ to FNCODE if its IMPLICIT_P flag has been set or NULL otherwise.
+ IMPLICIT_P is clear for library built-ins that GCC implements but that
+ may not be implemented in the runtime library on the target. See also
+ the DEF_BUILTIN macro in builtins.def. */
+
it isn't exactly helpful to specify when IMPLICIT_P is not set. When is it
actually set?
My understanding of the difference is that the compiler may create a call
to a builtin_decl_implicit decl but _not_ to a builtin_decl_explicit one
unless a call to such function already existed.
Note we set the implicit flag during gimplification if we see such decl
referenced and it is marked as properly declared by the FE.
Overall your patch is an improvement I think but maybe you can
refer to builtins.def in the documentation for builtin_decl_implicit?
Richard.
> On 11/20/18 1:24 PM, Martin Sebor wrote:
> > On 11/20/2018 11:02 AM, Martin Sebor wrote:
> >> Would the updated comments in the attached patch more accurately
> >> describe the purpose of the IMPLICIT_P flag and
> >> the builtin_decl_explicit() and builtin_decl_implicit() functions?
> >>
> >> I ended up here while trying to understand the differences between
> >> the functions on different targets and decide which one should be
> >> used to diagnose bugs like the one below:
> >>
> >> long double fabsl (); // missing prototype
> >>
> >> long double f (int x)
> >> {
> >> return fabsl (x); // want a warning here
> >> }
> >>
> >> I think we want the warning regardless of IMPLICIT_P so the warning
> >> code should call builtin_decl_explicit() to obtain fabsl's expected
> >> type, even if the target's runtime doesn't support the function on
> >> the basis of the comment:
> >>
> >> When a program uses floorf we may assume that the floorf function
> >> has the expected meaning
> >
> > Actually, some more testing suggests the comment in builtins.def
> > (either the original or the patched one) isn't entirely accurate
> > or helpful to understanding the purpose of the flag:
> >
> > IMPLICIT specifies condition when the builtin can be produced by
> > compiler. For instance C90 reserves floorf function, but does not
> > define it's meaning. When user uses floorf we may assume that the
> > floorf has the meaning we expect, but we can't produce floorf by
> > simplifying floor((double)float) since the runtime need not
> > implement it.
> >
> > Given the following code:
> >
> > float floorf ();
> >
> > void f (void)
> > {
> > if (floorf (0.0f))
> > __builtin_abort ();
> > }
> >
> > in C90 mode, BUILT_IN_FLOORF's IMPLICIT flag is clear and GCC
> > doesn't seem to assume anything about the call to the function,
> > contrary to the comment ("we may assume the meaning we expect").
> > The comment also doesn't explain when IMPLICIT may be set.
> >
> > I've updated the comment a bit more to more accurately describe
> > when I think the flag is set or clear, and how it's used.
> > Corrections or further clarification are appreciated.
> >
> > Thanks
> > Martin
>
