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

2023-08-09 Thread Edwin Lu

Hi Vineet,

On 8/8/2023 2:02 PM, Vineet Gupta wrote:


Maybe add a comment that in absence of -m[no-]strict-align, we use the 
cpu tune param -> slow_unaligned_access and that default mcpu is rocket 
which has that set to _slow.




That sounds good to me!


+#if defined(__riscv_unaligned_avoid)
+#error "__riscv_unaligned_avoid is unexpectedly set"
+#endif


Lets first check what is really expected.
#if !defined (_slow) #error


+
+/* Check __riscv_unaligned_slow xor __riscv_unaligned_fast is set.  */
+#if !defined(__riscv_unaligned_slow) && !defined(__riscv_unaligned_fast)
+#error "either __riscv_unaligned_slow or__riscv_unaligned_fast is not 
set"

+#endif
+
+#if defined(__riscv_unaligned_slow) && defined(__riscv_unaligned_fast)
+#error "both __riscv_unaligned_slow and__riscv_unaligned_fast are set"
+#endif
+


I think we can simplify this a bit (sorry I if wasn't clear enough in 
our off-list discussions).
We now need to ensure that unexpected toggles are NOT defined: #if 
defined(_avoid) || defined(_fast) #error
I don't think we need to check for the combinations as that is covered 
by first one and this.


While separate #error prints for 2 failures are ideal, personally it 
feels excessive given that the current implementation will only ever 
generate 1 of them. If a future code change breaks that assumption, the 
onus is on that change to fix/update the errors.




That makes sense to me. I was thinking that adding some more checks 
would help clarify the assumptions but it did just make things overly 
verbose.




Same as my comment for attribute-1. Please recheck all of them.



Thanks for the feedback! I will be sure to update all of them in the 
next iteration.


Edwin



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

2023-08-08 Thread Vineet Gupta




On 8/8/23 13:24, Edwin Lu wrote:

diff --git a/gcc/testsuite/gcc.target/riscv/attribute-1.c 
b/gcc/testsuite/gcc.target/riscv/attribute-1.c
index bc919c586b6..4b077c980a4 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-1.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-1.c
@@ -2,5 +2,20 @@
  /* { dg-options "-mriscv-attribute" } */
  int foo()
  {
+


Maybe add a comment that in absence of -m[no-]strict-align, we use the 
cpu tune param -> slow_unaligned_access and that default mcpu is rocket 
which has that set to _slow.



+#if defined(__riscv_unaligned_avoid)
+#error "__riscv_unaligned_avoid is unexpectedly set"
+#endif


Lets first check what is really expected.
#if !defined (_slow) #error


+
+/* Check __riscv_unaligned_slow xor __riscv_unaligned_fast is set.  */
+#if !defined(__riscv_unaligned_slow) && !defined(__riscv_unaligned_fast)
+#error "either __riscv_unaligned_slow or__riscv_unaligned_fast is not set"
+#endif
+
+#if defined(__riscv_unaligned_slow) && defined(__riscv_unaligned_fast)
+#error "both __riscv_unaligned_slow and__riscv_unaligned_fast are set"
+#endif
+


I think we can simplify this a bit (sorry I if wasn't clear enough in 
our off-list discussions).
We now need to ensure that unexpected toggles are NOT defined: #if 
defined(_avoid) || defined(_fast) #error
I don't think we need to check for the combinations as that is covered 
by first one and this.


While separate #error prints for 2 failures are ideal, personally it 
feels excessive given that the current implementation will only ever 
generate 1 of them. If a future code change breaks that assumption, the 
onus is on that change to fix/update the errors.



+return 0;
  }
  /* { dg-final { scan-assembler ".attribute arch" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-4.c 
b/gcc/testsuite/gcc.target/riscv/attribute-4.c
index 7c565c4963e..c505dbae2aa 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-4.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-4.c
@@ -2,5 +2,19 @@
  /* { dg-options "-mriscv-attribute -mstrict-align" } */
  int foo()
  {
+
+#if !defined(__riscv_unaligned_avoid)
+#error "__riscv_unaligned_avoid is not set"
+#endif
+
+#if defined(__riscv_unaligned_fast)
+#error "__riscv_unaligned_fast is unexpectedly set"
+#endif
+
+#if defined(__riscv_unaligned_slow)
+#error "__riscv_unaligned_slow is unexpectedly set"
+#endif
+
+  return 0;
  }
  /* { dg-final { scan-assembler ".attribute unaligned_access, 0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-5.c 
b/gcc/testsuite/gcc.target/riscv/attribute-5.c
index ee9cf693be6..45afa6c464d 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-5.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-5.c
@@ -2,5 +2,20 @@
  /* { dg-options "-mriscv-attribute -mno-strict-align" } */
  int foo()
  {
+
+#if defined(__riscv_unaligned_avoid)
+#error "__riscv_unaligned_avoid is unexpectedly set"
+#endif
+
+/* Check __riscv_unaligned_slow xor __riscv_unaligned_fast is set.  */
+#if !defined(__riscv_unaligned_slow) && !defined(__riscv_unaligned_fast)
+#error "either __riscv_unaligned_slow or__riscv_unaligned_fast is not set"
+#endif
+
+#if defined(__riscv_unaligned_slow) && defined(__riscv_unaligned_fast)
+#error "both __riscv_unaligned_slow and__riscv_unaligned_fast are set"
+#endif


Same as my comment for attribute-1. Please recheck all of them.


+
+return 0;
  }
  /* { dg-final { scan-assembler ".attribute unaligned_access, 1" } } */
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 000..2a889dd9284
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-mtune=thead-c906" } */
+
+int main() {
+
+/* thead-c906 default is cpu tune param unaligned access fast */
+#if !defined(__riscv_unaligned_fast)
+#error "__riscv_unaligned_fast is not set"
+#endif
+
+#if defined(__riscv_unaligned_slow)
+#error "__riscv_unaligned_slow is unexpectedly set"
+#endif
+
+#if defined(__risvc_unaligned_avoid)
+#error "__riscv_unaligned_avoid is unexpectedly set"
+#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 000..76cbce60d9f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/predef-align-2.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-mtune=thead-c906 -mstrict-align" } */
+
+int main() {
+
+#if !defined(__riscv_unaligned_avoid)
+#error "__riscv_unaligned_avoid is not set"
+#endif
+
+#if defined(__riscv_unaligned_fast)
+#error "__riscv_unaligned_fast is unexpectedly set"
+#endif
+
+#if defined(__riscv_unaligned_slow)
+#error "__riscv_unaligned_slow is unexpectedly set"
+#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 000..f8caef48180
---