diff mbox series

riscv: generate builtin macro for compilation with strict alignment

Message ID 20230117225908.1604948-1-vineetg@rivosinc.com
State New
Headers show
Series riscv: generate builtin macro for compilation with strict alignment | expand

Commit Message

Vineet Gupta Jan. 17, 2023, 10:59 p.m. UTC 
This could be useful for library writers who want to write code variants
for fast vs. slow unaligned accesses.

We distinguish explicit -mstrict-align (1) vs. slow_unaligned_access
cpu tune param (2) for even more code divesity.

gcc/ChangeLog:

	* config/riscv-c.cc (riscv_cpu_cpp_builtins):
	  Generate __riscv_strict_align with value 1 or 2.
	* config/riscv/riscv.cc: Define riscv_user_wants_strict_align.
	  (riscv_option_override) Set riscv_user_wants_strict_align to
	  TARGET_STRICT_ALIGN.
	* config/riscv/riscv.h: Declare riscv_user_wants_strict_align.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/attribute.c: Check for
	  __riscv_strict_align=1.
	* gcc.target/riscv/predef-align-1.c: New test.
	* gcc.target/riscv/predef-align-2.c: New test.
	* gcc.target/riscv/predef-align-3.c: New test.
	* gcc.target/riscv/predef-align-4.c: New test.
	* gcc.target/riscv/predef-align-5.c: New test.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gcc/config/riscv/riscv-c.cc                     | 11 +++++++++++
 gcc/config/riscv/riscv.cc                       |  9 +++++++++
 gcc/config/riscv/riscv.h                        |  1 +
 gcc/testsuite/gcc.target/riscv/attribute-4.c    |  9 +++++++++
 gcc/testsuite/gcc.target/riscv/predef-align-1.c | 12 ++++++++++++
 gcc/testsuite/gcc.target/riscv/predef-align-2.c | 11 +++++++++++
 gcc/testsuite/gcc.target/riscv/predef-align-3.c | 15 +++++++++++++++
 gcc/testsuite/gcc.target/riscv/predef-align-4.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/riscv/predef-align-5.c | 16 ++++++++++++++++
 9 files changed, 100 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-5.c

Comments

Jeff Law April 20, 2023, 4:56 p.m. UTC | #1 
On 1/17/23 15:59, Vineet Gupta wrote:
> This could be useful for library writers who want to write code variants
> for fast vs. slow unaligned accesses.
> 
> We distinguish explicit -mstrict-align (1) vs. slow_unaligned_access
> cpu tune param (2) for even more code divesity.
> 
> gcc/ChangeLog:
> 
> 	* config/riscv-c.cc (riscv_cpu_cpp_builtins):
> 	  Generate __riscv_strict_align with value 1 or 2.
> 	* config/riscv/riscv.cc: Define riscv_user_wants_strict_align.
> 	  (riscv_option_override) Set riscv_user_wants_strict_align to
> 	  TARGET_STRICT_ALIGN.
> 	* config/riscv/riscv.h: Declare riscv_user_wants_strict_align.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/attribute.c: Check for
> 	  __riscv_strict_align=1.
> 	* gcc.target/riscv/predef-align-1.c: New test.
> 	* gcc.target/riscv/predef-align-2.c: New test.
> 	* gcc.target/riscv/predef-align-3.c: New test.
> 	* gcc.target/riscv/predef-align-4.c: New test.
> 	* gcc.target/riscv/predef-align-5.c: New test.
> 
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
>   gcc/config/riscv/riscv-c.cc                     | 11 +++++++++++
>   gcc/config/riscv/riscv.cc                       |  9 +++++++++
>   gcc/config/riscv/riscv.h                        |  1 +
>   gcc/testsuite/gcc.target/riscv/attribute-4.c    |  9 +++++++++
>   gcc/testsuite/gcc.target/riscv/predef-align-1.c | 12 ++++++++++++
>   gcc/testsuite/gcc.target/riscv/predef-align-2.c | 11 +++++++++++
>   gcc/testsuite/gcc.target/riscv/predef-align-3.c | 15 +++++++++++++++
>   gcc/testsuite/gcc.target/riscv/predef-align-4.c | 16 ++++++++++++++++
>   gcc/testsuite/gcc.target/riscv/predef-align-5.c | 16 ++++++++++++++++
>   9 files changed, 100 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-1.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-2.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-3.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-4.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-5.c
> 
> diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
> index 826ae0067bb8..47a396501d74 100644
> --- a/gcc/config/riscv/riscv-c.cc
> +++ b/gcc/config/riscv/riscv-c.cc
> @@ -102,6 +102,17 @@ riscv_cpu_cpp_builtins (cpp_reader *pfile)
>   
>       }
>   
> +  /* TARGET_STRICT_ALIGN does not cover all cases.  */
> +  if (riscv_slow_unaligned_access_p)
> +    {
> +      /* Explicit -mstruct-align preceedes cpu tune param
> +         slow_unaligned_access=true.  */
Did you mean "-mstrict-align" above?


> +      if (riscv_user_wants_strict_align)
> +        builtin_define_with_int_value ("__riscv_strict_align", 1);
> +      else
> +        builtin_define_with_int_value ("__riscv_strict_align", 2);
So I don't understand why we're testing "riscv_user_wants_strict_align" 
instead of TARGET_STRICT_ALIGN here.  AFAICT they're equivalent.  But 
maybe there's something subtle I'm missing.

Jeff
Vineet Gupta April 28, 2023, 9:37 p.m. UTC | #2 
On 4/20/23 09:56, Jeff Law via Gcc-patches wrote:
>
>
> On 1/17/23 15:59, Vineet Gupta wrote:
>> This could be useful for library writers who want to write code variants
>> for fast vs. slow unaligned accesses.
>>
>> We distinguish explicit -mstrict-align (1) vs. slow_unaligned_access
>> cpu tune param (2) for even more code divesity.
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv-c.cc (riscv_cpu_cpp_builtins):
>>       Generate __riscv_strict_align with value 1 or 2.
>>     * config/riscv/riscv.cc: Define riscv_user_wants_strict_align.
>>       (riscv_option_override) Set riscv_user_wants_strict_align to
>>       TARGET_STRICT_ALIGN.
>>     * config/riscv/riscv.h: Declare riscv_user_wants_strict_align.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.target/riscv/attribute.c: Check for
>>       __riscv_strict_align=1.
>>     * gcc.target/riscv/predef-align-1.c: New test.
>>     * gcc.target/riscv/predef-align-2.c: New test.
>>     * gcc.target/riscv/predef-align-3.c: New test.
>>     * gcc.target/riscv/predef-align-4.c: New test.
>>     * gcc.target/riscv/predef-align-5.c: New test.
>>
>> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
>> ---
>>   gcc/config/riscv/riscv-c.cc                     | 11 +++++++++++
>>   gcc/config/riscv/riscv.cc                       |  9 +++++++++
>>   gcc/config/riscv/riscv.h                        |  1 +
>>   gcc/testsuite/gcc.target/riscv/attribute-4.c    |  9 +++++++++
>>   gcc/testsuite/gcc.target/riscv/predef-align-1.c | 12 ++++++++++++
>>   gcc/testsuite/gcc.target/riscv/predef-align-2.c | 11 +++++++++++
>>   gcc/testsuite/gcc.target/riscv/predef-align-3.c | 15 +++++++++++++++
>>   gcc/testsuite/gcc.target/riscv/predef-align-4.c | 16 ++++++++++++++++
>>   gcc/testsuite/gcc.target/riscv/predef-align-5.c | 16 ++++++++++++++++
>>   9 files changed, 100 insertions(+)
>>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-1.c
>>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-2.c
>>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-3.c
>>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-4.c
>>   create mode 100644 gcc/testsuite/gcc.target/riscv/predef-align-5.c
>>
>> diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
>> index 826ae0067bb8..47a396501d74 100644
>> --- a/gcc/config/riscv/riscv-c.cc
>> +++ b/gcc/config/riscv/riscv-c.cc
>> @@ -102,6 +102,17 @@ riscv_cpu_cpp_builtins (cpp_reader *pfile)
>>         }
>>   +  /* TARGET_STRICT_ALIGN does not cover all cases.  */
>> +  if (riscv_slow_unaligned_access_p)
>> +    {
>> +      /* Explicit -mstruct-align preceedes cpu tune param
>> +         slow_unaligned_access=true.  */
> Did you mean "-mstrict-align" above?

Doh sorry yes.

>
>
>> +      if (riscv_user_wants_strict_align)
>> +        builtin_define_with_int_value ("__riscv_strict_align", 1);
>> +      else
>> +        builtin_define_with_int_value ("__riscv_strict_align", 2);
> So I don't understand why we're testing 
> "riscv_user_wants_strict_align" instead of TARGET_STRICT_ALIGN here.  
> AFAICT they're equivalent.  But maybe there's something subtle I'm 
> missing.

The missing part is slightly over-engineered unaligned access signaling 
in RV gcc frontend IMHO.

Thing is -mno-strict-align can be over-ruled by the cpu tune param 
slow_unaligned_access=true (and behave as if -mstrict-align was passed)
And I wanted the macro to reflect this (for future proofing) by being 
defined but with different values.

There's some renewed discussion with Kito on [1] so I need to respin 
this after getting the agreed upon specification in there.

Thx,
-Vineet

[1] https://github.com/riscv-non-isa/riscv-c-api-doc/issues/32
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
index 826ae0067bb8..47a396501d74 100644
--- a/gcc/config/riscv/riscv-c.cc
+++ b/gcc/config/riscv/riscv-c.cc
@@ -102,6 +102,17 @@  riscv_cpu_cpp_builtins (cpp_reader *pfile)
 
     }
 
+  /* TARGET_STRICT_ALIGN does not cover all cases.  */
+  if (riscv_slow_unaligned_access_p)
+    {
+      /* Explicit -mstruct-align preceedes cpu tune param
+         slow_unaligned_access=true.  */
+      if (riscv_user_wants_strict_align)
+        builtin_define_with_int_value ("__riscv_strict_align", 1);
+      else
+        builtin_define_with_int_value ("__riscv_strict_align", 2);
+    }
+
   if (TARGET_MIN_VLEN != 0)
     builtin_define_with_int_value ("__riscv_v_min_vlen", TARGET_MIN_VLEN);
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 9a53999a39de..d6a40d043584 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -255,6 +255,9 @@  struct riscv_tune_info {
 /* Whether unaligned accesses execute very slowly.  */
 bool riscv_slow_unaligned_access_p;
 
+/* Whether use explcitly passed -mstrict-align.  */
+bool riscv_user_wants_strict_align;
+
 /* Stack alignment to assume/maintain.  */
 unsigned riscv_stack_boundary;
 
@@ -6047,6 +6050,12 @@  riscv_option_override (void)
      -m[no-]strict-align is left unspecified, heed -mtune's advice.  */
   riscv_slow_unaligned_access_p = (cpu->tune_param->slow_unaligned_access
 				   || TARGET_STRICT_ALIGN);
+
+  /* Make a note if user explicitly passed -mstrict-align for later
+     builtin macro generation. Can't use  target_flags_explicit since
+     it is set even for -mno-strict-align.  */
+  riscv_user_wants_strict_align = TARGET_STRICT_ALIGN;
+
   if ((target_flags_explicit & MASK_STRICT_ALIGN) == 0
       && cpu->tune_param->slow_unaligned_access)
     target_flags |= MASK_STRICT_ALIGN;
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 0ab739bd6ebf..c55546656b7d 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -1030,6 +1030,7 @@  while (0)
 #ifndef USED_FOR_TARGET
 extern const enum reg_class riscv_regno_to_class[];
 extern bool riscv_slow_unaligned_access_p;
+extern bool riscv_user_wants_strict_align;
 extern unsigned riscv_stack_boundary;
 extern unsigned riscv_bytes_per_vector_chunk;
 extern poly_uint16 riscv_vector_chunks;
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-4.c b/gcc/testsuite/gcc.target/riscv/attribute-4.c
index 7c565c4963ec..ce7f1929e6a6 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-4.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-4.c
@@ -2,5 +2,14 @@ 
 /* { dg-options "-mriscv-attribute -mstrict-align" } */
 int foo()
 {
+
+#if !defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#if __riscv_strict_align != 1
+#error "__riscv_strict_align != 1"
+#endif
+#endif
+
+  return 0;
 }
 /* { dg-final { scan-assembler ".attribute unaligned_access, 0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/predef-align-1.c b/gcc/testsuite/gcc.target/riscv/predef-align-1.c
new file mode 100644
index 000000000000..49153a8efc20
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-1.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mtune=thead-c906" } */
+
+int main () {
+
+/* thead-c906 default is cpu tune param unaligned access fast.  */
+#if defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#endif
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/predef-align-2.c b/gcc/testsuite/gcc.target/riscv/predef-align-2.c
new file mode 100644
index 000000000000..b1c6ee5606e8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-2.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mtune=thead-c906 -mno-strict-align" } */
+
+int main () {
+
+#if defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#endif
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/predef-align-3.c b/gcc/testsuite/gcc.target/riscv/predef-align-3.c
new file mode 100644
index 000000000000..7a417ec8ff7e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-3.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mtune=thead-c906 -mstrict-align" } */
+
+int main () {
+
+#if !defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#else
+#if __riscv_strict_align != 1
+#error "__riscv_strict_align != 1"
+#endif
+#endif
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/predef-align-4.c b/gcc/testsuite/gcc.target/riscv/predef-align-4.c
new file mode 100644
index 000000000000..a1d6da8053a4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-4.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mtune=rocket" } */
+
+int main () {
+
+/* rocket default is cpu tune param unaligned access slow.  */
+#if !defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#else
+#if __riscv_strict_align != 2
+#error "__riscv_strict_align != 2"
+#endif
+#endif
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/predef-align-5.c b/gcc/testsuite/gcc.target/riscv/predef-align-5.c
new file mode 100644
index 000000000000..a8d239be9afd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-5.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mtune=rocket -mno-strict-align" } */
+
+int main () {
+
+/* -mno-strict-align override due to cpu tune param.  */
+#if !defined(__riscv_strict_align)
+#error "__riscv_strict_align"
+#else
+#if __riscv_strict_align != 2
+#error "__riscv_strict_align != 2"
+#endif
+#endif
+
+  return 0;
+}