On 08/10/2021 13:18, Tejas Belagod via Gcc-patches wrote:
Hi,

This patch implements target feature macros when PACBTI is
enabled through the -march option or -mbranch-protection.

Tested on arm-none-eabi. OK for trunk?

2021-10-04  Tejas Belagod  <tbela...@arm.com>

gcc/ChangeLog:

        * config/arm/arm-c.c (arm_cpu_builtins): Define
        __ARM_FEATURE_BTI_DEFAULT and __ARM_FEATURE_PAC_DEFAULT.

gcc/testsuite/ChangeLog:

        * gcc.target/arm/acle/pacbti-m-predef-2.c: New test.
        * gcc.target/arm/acle/pacbti-m-predef-4.c: New test.
        * gcc.target/arm/acle/pacbti-m-predef-5.c: New test.


I presume the specification for this is ACLE - please say so rather than making me guess.


+  cpp_undef (pfile, "__ARM_FEATURE_BTI_DEFAULT");
+  cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT");
+  if (TARGET_HAVE_PACBTI)
+    {
+      builtin_define_with_int_value ("__ARM_FEATURE_BTI_DEFAULT",
+                                    arm_enable_pacbti & 0x1);

My reading of the ACLE specification would suggest this shouldn't be defined if it would have a value of 0, but that's not what this code does. I think it would be better to move this outside the TARGET_HAVE_PACBTI and use the def_or_undef approach.

+      builtin_define_with_int_value ("__ARM_FEATURE_PAC_DEFAULT",
+                                    arm_enable_pacbti >> 1);

This one is less clear, could the value ever be zero? I guess exactly one of a-key and b-key must be defined and each has a separate bit.

+    }
+
+

Not more than one blank line at the end of a block.


diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c


Given what I've said above, I think you need to also test that __ARM_FEATURE_BTI_DEFAULT is defined before testing the value (and emitting #error if it isn't).

R.

Reply via email to