Richard Biener <richard.guent...@gmail.com> writes:

> I don't think it reliably works the way you implement it.  It's also having
> more side-effects than what you document, in particular

Yeah, I made a 'minimal' patch that had the effect I needed, but it's
clearly in the wrong place as it disables the matching of builtins
against the incoming source code instead of the generation of new
builtin references from the tree.

> I think you may want to instead change builtin_decl_implicit
> to avoid code-generating a specific builtin.

Yup, I looked at that and there are numerous places which assume that
will work, so it will be a more complicated patch.

> Generally we'd also want sth like the clang attribute and _not_
> use optimize("") for this or a global flag_*, so the behavior can
> be more readily encoded in the IL.  In fact a flag on the call
> statement could be added to denote the desired effect on it.

Agreed, using the existing optimize attribute was a short-cut to
leverage the existing code handling that case. If we think providing
something that matches the clang attribute would be useful, it makes
sense to provide it using the same syntax.

> I also don't see the advantage compared to -fno-builtin[-foo].
> Declaring the function should be something that's already done.

The semantic of the clang option is not to completely disable access
to the given builtin function, but rather to stop the optimizer from
creating new builtin function references (either to a specific builtin,
or to all builtins).

If I could use "no-builtin" in a function attribute, I probably wouldn't
have bothered looking to implement the clang semantics, but -fno-builtin
isn't supported in this way. But, now that I think I understand the
behavior of attribute((no_builtin)) in clang, I think it has value
beyond what -fno-builtin performs as you can still gain access to
builtin functions when they are directly named.

I'll go implement changes in builtin_decl_implicit and all of the
affected call sites and see what that looks like.

Thanks much for your review!

-- 
-keith

Attachment: signature.asc
Description: PGP signature

Reply via email to