> -----Original Message----- > From: Richard Sandiford [mailto:richard.sandif...@arm.com] > Sent: Tuesday, April 21, 2020 6:11 PM > To: Yangfei (Felix) <felix.y...@huawei.com> > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH PR94678] aarch64: unexpected result with -mgeneral- > regs-only and sve > > Should aarch64_sve::init_builtins ()/aarch64_sve::handle_arm_sve_h () be > guarded by TARGET_SVE? > > > > Could you please confirm that? > > Yeah, that's right. As Jakub says, the SVE stuff is (deliberately) registered > unconditionally because it's possible to switch SVE on and off later. Also, > protecting it with just TARGET_SVE would mean that we'd continue to > register SVE2 functions even if SVE2 isn't currently enabled.
Well, I was thinking maybe we can call aarch64_sve::init_builtins () in
aarch64_pragma_target_parse when SVE is switched on.
But I think I will leave this alone.
> So what matters is whether SVE is enabled at the point of use, not the point
> of the #include. FWIW, arm_neon.h works the same way: the same
> functions are defined regardless of what the current prevailing architecture
> is,
> and what matters is whether the necessary features are enabled when the
> functions are called. (Inlining fails if they're not.) But because we're
> implementing arm_sve.h directly in the compiler, we don't need the
> overhead of a full target switch when defining the functions.
>
> And like you say, the second of the above tests makes sure that turning SVE
> on later does indeed work, while the first makes sure that we get an
> appropriate error if SVE is disabled at the point of use.
Thanks for the details, it helps :-)
I have modified accordingly and attached please find the adapted v2 patch.
Bootstrap and tested on aarch64 Linux platform. Does it look better?
Note that we need to disable TARGET_GENERAL_REGS_ONLY before
register_builtin_types.
Otherwise we got ICEs like:
<built-in>: internal compiler error: in register_builtin_types, at
config/aarch64/aarch64-sve-builtins.cc:3336
0x185bcfb register_builtin_types
../../gcc-git/gcc/config/aarch64/aarch64-sve-builtins.cc:3332
0x185c467 aarch64_sve::init_builtins()
../../gcc-git/gcc/config/aarch64/aarch64-sve-builtins.cc:3375
0x17c075b aarch64_init_builtins
../../gcc-git/gcc/config/aarch64/aarch64.c:13086
, where TYPE_MODE (vectype) is BLKmode.
The reason is targetm.vector_mode_supported_p (mode) and
have_regs_of_mode[mode] are false when TARGET_GENERAL_REGS_ONLY is enabled.
(gdb) bt
#0 vector_type_mode (t=0xffffb7aa2f18) at ../../gcc-git/gcc/tree.c:13825
#1 0x0000000001297ee0 in layout_type (type=0xffffb7aa2f18) at
../../gcc-git/gcc/stor-layout.c:2400
#2 0x00000000016e19d8 in make_vector_type (innertype=0xffffb7aa2a80,
nunits=..., mode=E_VNx16BImode) at ../../gcc-git/gcc/tree.c:9984
#3 0x00000000016e79b4 in build_truth_vector_type_for_mode (nunits=...,
mask_mode=E_VNx16BImode) at ../../gcc-git/gcc/tree.c:10929
#4 0x000000000185bb00 in aarch64_sve::register_builtin_types () at
../../gcc-git/gcc/config/aarch64/aarch64-sve-builtins.cc:3331
#5 0x000000000185c468 in aarch64_sve::init_builtins () at
../../gcc-git/gcc/config/aarch64/aarch64-sve-builtins.cc:3375
#6 0x00000000017c075c in aarch64_init_builtins () at
../../gcc-git/gcc/config/aarch64/aarch64.c:13086
#7 0x0000000000a69cb4 in c_define_builtins
(va_list_ref_type_node=0xffffb79ea540, va_list_arg_type_node=0xffffb79e79d8)
at ../../gcc-git/gcc/c-family/c-common.c:3973
Felix
pr94678-v2.patch
Description: pr94678-v2.patch
