On 19/04/2024 13:45, Alexandre Oliva wrote:
> On Apr 16, 2024, "Richard Earnshaw (lists)" <richard.earns...@arm.com> wrote:
>
>> The require-effective-target flags test whether a specific set of
>> flags will make the compilation work, so they need to be used in
>> conjunction with the corresponding dg-add-options flags that then
>> apply those options.
>
> *nod*, that's the theory. Problem is the architectures suported by
> [add_options_for_]arm_arch_*[_ok] do not match exactly those expected by
> the tests, and I can't quite tell whether the subtle changes they would
> introduce would change what they intend to test, or even whether the
> differences are irrelevant, or would be sensible to add as variants to
> the dg machinery. I think it would take someone more familiar than I am
> with all of the ARM variants to do this correctly. I don't even know
> how these changes would need to be tested to be sure they remain
> correct.
It's ok to add additional variations to the table of variants in
target-supports.exp, but we should avoid writing new specific run-time
functions unless we really want an executable test.
I started doing some cleanup of the Arm tests infrastructure during phase 3,
but stopped during phase 4 as I wanted to minimise the changes being made now.
I plan to go back and work on it some more once stage 1 re-opens.
>
> Would you be willing to take it from here, or would you accept the patch
> as an incremental yet imperfect improvement, or would you prefer to
> guide me in making it correct, and in verifying it (there are questions
> below)? I don't have a lot of cycles to put into this (we've already
> worked around the testsuite bugs we ran into), but it would be desirable
> to get a fix into GCC as well, if we can converge on one without
> unreasonably burdening anyone.
>
>
> v8_1m_main "-march=armv8.1-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__
> v8_1m_main_pacbti "-march=armv8.1-m.main+pacbti+fp -mthumb"
> "__ARM_ARCH_8M_MAIN__ && __ARM_FEATURE_BTI &&
> __ARM_FEATURE_PAUTH
>
> Why do these have +fp in -march but not in the v8_1m* arch name?
It's ... complicated :)
The +fp is there because, with the move to having -mfpu=auto as the default, we
need to avoid problems when the compiler has been configured with
--with-float=hard, which requires the extension register set (fp or vector
support) even if the test code itself doesn't care. The best way to handle
this in most cases is to give the architecture strings a default FPU
specification (ie +fp).
>
>
> gcc/testsuite/g++.target/arm/pac-1.C:
> /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret
> -mthumb -mfloat-abi=hard -g -O0" } */
>
> v8_1m_main_pacbti plus +mve minus +fp.
> Do we need a dg arch for that?
I'd be inclined to drop +mve from this one; there's nothing I can see in the
test that would generate mve instructions, so I think it's irrelevant. We can
use the existing v8_1m_main_pacbti operations.
>
>
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c:
> /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps
> -mfloat-abi=hard" } */
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c:
> /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */
>
> v8_1m_main_pacbti minus -mthumb.
> AFAICT the -mthumb is redundant.
Nearly, but not quite. Although the gcc driver knows that m-profile
architectures require thumb, that's not enough to override an explicit -marm
from a testsuite configuration run, so if your site.exp file adds -marm in a
test configuration we need to override that or the test will fail. But the
table based list of options will do that for you.
>
>
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c:
> /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */
>
> v8_1m_main minus -mthumb.
> AFAICT the -mthumb is redundant.
As above
>
>
> gcc/testsuite/gcc.target/arm/bti-1.c:
> /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp
> -mbranch-protection=bti --save-temps" } */
> gcc/testsuite/gcc.target/arm/bti-2.c:
> /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp
> -mbranch-protection=bti --save-temps" } */
>
> v8_1m_main minus +fp.>
> Can these be bumped to +fp, or do we need an extra dg arch?
>
> Are these missing +pacbti?
The tests themselves do not require fp, but if we use the effective-target
rules (arm_arch_v8_1m_main), we can remove the -march, -mthumb and -mfloat-abi
flags from these tests.
These tests for BTI should NOT have +pacbti: they're testing that the compiler
generates the right nop-based implementation that is backwards compatible with
CPUs that do not have this feature.[1]
R.
>
>
> Thanks,
>
[1] The PAC and BTI features define the behaviour of some architectural NOP
instructions: on older CPUs these instructions have no effect (they are NOPs),
but on newer CPUs these NOPs take on a new behaviour that implements the
feature (PAC or BTI).
R.
