diff mbox series

RISC-V: xtheadmemidx: Document inline asm issue with memory constraint

Message ID 20231205151644.3203029-1-christoph.muellner@vrull.eu
State Accepted
Headers
Series RISC-V: xtheadmemidx: Document inline asm issue with memory constraint |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Christoph Müllner Dec. 5, 2023, 3:16 p.m. UTC 
  The XTheadMemIdx support relies on the fact that memory operands that
can be expressed by XTheadMemIdx instructions, will only appear as
operands of such instructions.  For internal instruction generation
this is guaranteed by the implemenation.  However, in case of inline
assembly, this guarantee is not given and we cannot differentiate
these two cases when printing the operand:

  asm volatile ("sd	%1,%0" : "=m"(*tmp) : "r"(val));
  asm volatile ("th.srd %1,%0" : "=m"(*tmp) : "r"(val));

If XTheadMemIdx is enabled, then the address will be printed as if an
XTheadMemIdx instruction is emitted, which is obviously wrong in the
first case.

There might be solutions to handle this (e.g. using TARGET_MEM_CONSTRAINT
or extending the mnemonics to accept the standard operands for
XTheadMemIdx instructions), but let's document this behavior for now
as a known issue by adding xfail tests until we have an acceptable fix.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xtheadmemidx-inline-asm-1.c: New test.

Reported-by: Jin Ma <jinma@linux.alibaba.com>
Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
---
 .../riscv/xtheadmemidx-inline-asm-1.c         | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c

Comments

Kito Cheng Dec. 6, 2023, 7:18 a.m. UTC | #1 
LGTM

On Tue, Dec 5, 2023 at 11:16 PM Christoph Müllner
<christoph.muellner@vrull.eu> wrote:
>
> The XTheadMemIdx support relies on the fact that memory operands that
> can be expressed by XTheadMemIdx instructions, will only appear as
> operands of such instructions.  For internal instruction generation
> this is guaranteed by the implemenation.  However, in case of inline
> assembly, this guarantee is not given and we cannot differentiate
> these two cases when printing the operand:
>
>   asm volatile ("sd     %1,%0" : "=m"(*tmp) : "r"(val));
>   asm volatile ("th.srd %1,%0" : "=m"(*tmp) : "r"(val));
>
> If XTheadMemIdx is enabled, then the address will be printed as if an
> XTheadMemIdx instruction is emitted, which is obviously wrong in the
> first case.
>
> There might be solutions to handle this (e.g. using TARGET_MEM_CONSTRAINT
> or extending the mnemonics to accept the standard operands for
> XTheadMemIdx instructions), but let's document this behavior for now
> as a known issue by adding xfail tests until we have an acceptable fix.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/xtheadmemidx-inline-asm-1.c: New test.
>
> Reported-by: Jin Ma <jinma@linux.alibaba.com>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  .../riscv/xtheadmemidx-inline-asm-1.c         | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c
>
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c b/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c
> new file mode 100644
> index 00000000000..da52433feb7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Og" } } */
> +/* { dg-options "-march=rv64gc_xtheadmemidx" } */
> +
> +/* XTheadMemIdx support is implemented such that reg+reg addressing mode
> +   loads/stores are preferred over standard loads/stores.
> +   If this order changed using inline assembly, the result will be invalid
> +   instructions.  This test serves the purpose of documenting this
> +   limitation until a solution is available.  */
> +
> +void foo (void *p, unsigned long off, unsigned long val)
> +{
> +  unsigned long *tmp = (unsigned long*)(p + off);
> +  asm volatile ("sd    %1,%0" : "=m"(*tmp) : "r"(val));
> +}
> +
> +void bar (void *p, unsigned long off, unsigned long val)
> +{
> +  unsigned long *tmp = (unsigned long*)(p + off);
> +  asm volatile ("th.srd        %1,%0" : "=m"(*tmp) : "r"(val));
> +}
> +
> +/* { dg-final { scan-assembler "sd\t\[a-z\]\[0-9\]+,0\\(\[a-z\]\[0-9\]+\\)" { xfail *-*-* } } } */
> +/* { dg-final { scan-assembler-not "sd\t\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,0" { xfail *-*-* } } } */
> +/* { dg-final { scan-assembler "th\.srd\t\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,0" } } */
> +/* { dg-final { scan-assembler-not "th\.srd\t\[a-z\]\[0-9\]+,0\\(\[a-z\]\[0-9\]+\\)" } } */
> --
> 2.43.0
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c b/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c
new file mode 100644
index 00000000000..da52433feb7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadmemidx-inline-asm-1.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Og" } } */
+/* { dg-options "-march=rv64gc_xtheadmemidx" } */
+
+/* XTheadMemIdx support is implemented such that reg+reg addressing mode
+   loads/stores are preferred over standard loads/stores.
+   If this order changed using inline assembly, the result will be invalid
+   instructions.  This test serves the purpose of documenting this
+   limitation until a solution is available.  */
+
+void foo (void *p, unsigned long off, unsigned long val)
+{
+  unsigned long *tmp = (unsigned long*)(p + off);
+  asm volatile ("sd	%1,%0" : "=m"(*tmp) : "r"(val));
+}
+
+void bar (void *p, unsigned long off, unsigned long val)
+{
+  unsigned long *tmp = (unsigned long*)(p + off);
+  asm volatile ("th.srd	%1,%0" : "=m"(*tmp) : "r"(val));
+}
+
+/* { dg-final { scan-assembler "sd\t\[a-z\]\[0-9\]+,0\\(\[a-z\]\[0-9\]+\\)" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "sd\t\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,0" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler "th\.srd\t\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,\[a-z\]\[0-9\]+,0" } } */
+/* { dg-final { scan-assembler-not "th\.srd\t\[a-z\]\[0-9\]+,0\\(\[a-z\]\[0-9\]+\\)" } } */