On Thu, 23 Nov 2023 01:28:40 GMT, Xiaohong Gong <xg...@openjdk.org> wrote:
>> Ah, now I se what you are trying to do here. First of all, in the detection >> part, only set `SVE_FEATURE_SUPPORT`. Then you can handle the `SVE_CFLAGS` >> addition elsewhere/later. >> >> Secondly, you should not mix these `SVE_CFLAGS` with the spleef C flags. >> Keeping them separate will allow for LIBSLEEF_CFLAGS to be named just that. >> >> Thirdly, I do not like at all how you just come crashing in setting `-march` >> like that. The `-march` flag is handled by `FLAGS_SETUP_ABI_PROFILE`. >> >> Actually, now that I think of it, this is just completely wrong! You are >> checking on features on the build machine, to determine what target machine >> code to generate, with no way to override. >> >> You need to break out the -march handling separately. It should be moved to >> FLAGS_SETUP_ABI_PROFILE. I'm guessing you will need to make something like a >> `aarch64-sve` profile, and possibly try to auto-select it based on the >> result of the sve test program above. But changing >> `OPENJDK_TARGET_ABI_PROFILE` can have further consequences; I do not know >> the full extent on the top of my head. > > Thanks for the advice! I will take a consideration for it. > Thirdly, I do not like at all how you just come crashing in setting -march > like that. The -march flag is handled by FLAGS_SETUP_ABI_PROFILE. `-march=armv8-a+sve` is just used in this new added module, which may not expect to be used for other libraries. Per my understanding, flags handled by `FLAGS_SETUP_ABI_PROFILE` is not used for a specified module? > Actually, now that I think of it, this is just completely wrong! You are > checking on features on the build machine, to determine what target machine > code to generate, with no way to override. Yes, that's be a risk, although the usage to the SVE functions are controlled by SVE feature as well in runtime. I need time to find a better solution. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1403052964
