Re: [PATCH] riscv: generate builtin macro for compilation with strict alignment

[Vineet Gupta <vineetg@rivosinc.com>]

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