On 10/21/24 6:43 AM, Matthew Malcomson wrote:
Ping (re-sending ping because previous message body too large for list --
apologies for duplication to those on Cc).
Attaching update on testsuite to fix testism on Arm that Linaro CI caught.
From: Matthew Malcomson <mmalcom...@nvidia.com>
This commit newly introduces the ability to use overloaded builtins in
C++ SFINAE context.
...
This is done both for resolve_overloaded_builtin and
check_builtin_function_arguments, both of which can be used in SFINAE
contexts.
N.b. I attempted to trigger something using the `reject_gcc_builtin`
function in an SFINAE context. Given the context where this
function is called from the C++ frontend it looks like it may be
possible, but I did not manage to trigger this in template context
by attempting to do something similar to the testcases added around
those calls.
- I would appreciate any feedback on whether this is something that
can happen in a template context, and if so some help writing a
relevant testcase for it.
I'm also having trouble thinking of a case where we could hit
reject_gcc_builtin in SFINAE contexts, it should happen during parsing.
We can always add that later if someone comes up with a testcase.
Incidentally, using "N.b." in comments seems redundant; all comments are
there to give context for what follows.
Both of these functions have target hooks for target specific builtins
that I have updated to take the extra boolean flag. I have not adjusted
the functions implementing those target hooks (except to update the
declarations) so target specific builtins will still error in SFINAE
contexts.
Since you're adding the parameter, why not adjust them to check it?
- I did not pass this new flag through
atomic_bitint_fetch_using_cas_loop since the _BitInt type is not
available in the C++ frontend and I didn't want if conditions that can
not be executed in the source.
I guess that's fine.
- I only test non-compile-time-constant types with SVE types, since I do
not know of a way to get a VLA into a SFINAE context.
Agreed.
- While writing tests I noticed a few differences with clang in this
area. I don't think they are problematic but am mentioning them for
completeness and to allow others to judge if these are a problem).
- atomic_fetch_add on a boolean is allowed by clang.
I don't see a need to allow that.
- With SFINAE GCC is happy with multiple definitions of a differently
typed template as long as they aren't instantiated, while clang is
not. This seems to be a general difference around clang and GCC and
not specific to builtins.
I.e. two template definitions with the same name where one
specialises on `myfunc (std::declval<T>())` and another on
`myfunc (std::declval<T>(), std::declval<T>())` will not give an
error in GCC unless one attempts to instantiate the template.
However it will give an error in clang on the template redefinition.
Can you give a testcase? I'm having trouble understanding what you mean.
- I do not block the warning about using
__builtin_speculation_safe_value on a target that does not have any
active mitigation defined. I do this since when this happens we do
not return error_mark_node, which means that if a user attempted to
use SFINAE to check for this situation that would silently fail if the
warning were suppressed.
N.b. this is also a warning rather than an error.
Similarly I do not block the warning about an invalid memory model
argument in get_atomic_generic_size.
Hmm, I think we want a SFINAE failure in these cases rather than a
warning, like various extensions that we reject in SFINAE but accept
with a pedwarn in other contexts.
Giving a warning when considering a candidate that might not be chosen
for other reasons seems undesirable.
@@ -8356,41 +8473,41 @@ resolve_overloaded_builtin (location_t loc, tree
function,
{
case BUILT_IN_ATOMIC_EXCHANGE:
{
- if (resolve_overloaded_atomic_exchange (loc, function, params,
- &new_return))
- return new_return;
- /* Change to the _N variant. */
- orig_code = BUILT_IN_ATOMIC_EXCHANGE_N;
- break;
+ if (resolve_overloaded_atomic_exchange (loc, function, params,
+ &new_return, complain))
+ return new_return;
+ /* Change to the _N variant. */
+ orig_code = BUILT_IN_ATOMIC_EXCHANGE_N;
+ break;
}
Try not to change the indentation of lines you aren't otherwise changing.
Jason
