On Wed, Nov 17, 2021 at 02:58:54PM -0600, Bill Schmidt wrote:
> Hi! This patch is broken out of the previous patch for all the builtins test
> suite adjustments. Here we have some slight changes in error messages due to
> how the internals have changed between the old and new builtins methods.
>
> For scalar-extract-exp-2.c we change:
> error: '__builtin_vec_scalar_extract_exp is not supported in this compiler
> configuration'
>
> to:
> error: '__builtin_vsx_scalar_extract_exp' requires the '-mcpu=power9'
> option and either the '-m64' or '-mpowerpc64' option
> note: builtin '__builtin_vec_scalar_extract_exp' requires builtin
> '__builtin_vsx_scalar_extract_exp'
I don't like that at all. The user didn't write the _vsx thing, and it
isn't documented either (neither is the _vec one, but that is a separate
issue, specific to this builtin).
> The new message provides more information. In both cases, it is less than
> ideal that we don't refer to scalar_extract_exp, which is referenced in
> the source line, but this is because scalar_extract_exp is #define'd to
> __builtin_vec_scalar_extract_exp, so it's unavoidable. Certainly this is no
> worse than before, and arguably better.
It is a macro, enough said there
The __builtin_ implementation should be documented (in the GCC manual,
if not elsewhere). The warnings should talk about _vec, because the
_vsx thing only exists as implementation detail, and we should never
talk about those. We don't have errors about adddi3 either!
> error: '__builtin_vsx_scalar_extract_sig' requires the '-mcpu=power9'
> option and either the '-m64' or '-mpowerpc64' option
> note: builtin '__builtin_vec_scalar_extract_sig' requires builtin
> '__builtin_vsx_scalar_extract_sig'
The rhs in the note does not *exist*, as far as the user is concerned.
One builtin requiring another is all gobbledygook.
> For scalar-test-neg-{2,3,5}.c, we actually change the test case. This is
> because we deliberately removed some undocumented and pointless
> overloads,
> where each overload mapped to a single builtin. These were:
> __builtin_vec_scalar_test_neg_sp
> __builtin_vec_scalar_test_neg_dp
> __builtin_vec_scalar_test_neg_qp
> which are redundant with the "real" overload:
> __builtin_vec_scalar_test_neg
> The latter maps to three builtins of the appropriate type.
Yes. And the new ones are undocumented and useless just as well, they
just have better names.
Segher
