"Yangfei (Felix)" <felix.y...@huawei.com> writes: >> -----Original Message----- >> From: Richard Sandiford [mailto:richard.sandif...@arm.com] >> Sent: Tuesday, April 21, 2020 4:01 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 >> >> "Yangfei (Felix)" <felix.y...@huawei.com> writes: >> > Hi, >> > >> > It looks like there are several issues out there for sve codegen with - >> mgeneral-regs-only. >> > I have created a bug for that: >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94678 >> > >> > We do ISA extension checks for SVE in >> check_required_extensions(aarch64-sve-builtins.cc). >> > I think we may also need to check -mgeneral-regs-only there and issue an >> error message when this option is specified. >> > This would be cheaper as compared with adding && >> TARGET_GENERAL_REGS_ONLY to TARGET_SVE and similar macros. >> >> We should probably do both. >> >> The middle end should never try to use vector patterns when the vector >> modes have been disabled by !have_regs_of_mode. But it's still wrong for >> the target to provide patterns that would inevitably lead to spill failure >> due to >> lack of registers. So I think we should check !TARGET_GENERAL_REGS_ONLY >> in TARGET_SVE. > > Yes, that's right. And I have a question here: > Should aarch64_sve::init_builtins ()/aarch64_sve::handle_arm_sve_h () be > guarded by TARGET_SVE? > > I mean in aarch64_init_builtins: > if (TARGET_SVE) > aarch64_sve::init_builtins (); > > and in aarch64_pragma_aarch64: > if (TARGET_SVE) > aarch64_sve::handle_arm_sve_h (); > > It looks to me that this is not wanted from the following two tests: > ./gcc.target/aarch64/sve/acle/general/nosve_1.c > ./gcc.target/aarch64/sve/acle/general/nosve_2.c > > 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. 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, Richard
