diff mbox series

RISC-V: Promote type correctly for libcalls

Message ID 1564660110-29983-1-git-send-email-kito.cheng@sifive.com
State New
Headers show
Series RISC-V: Promote type correctly for libcalls | expand

Commit Message

Kito Cheng Aug. 1, 2019, 11:48 a.m. UTC 
- argument and return value for libcall won't promote at
   default_promote_function_mode_always_promote, however we expect it
   should sign-extend as normal function.

 - Witout this patch, this test case will fail at -march=rv64i -mabi=lp64.

 - The implementation of riscv_promote_function_mode is borrowed from MIPS.

gcc/ChangeLog

	* config/riscv/riscv.c (riscv_promote_function_mode): New.
	(TARGET_PROMOTE_FUNCTION_MODE): Use riscv_promote_function_mode.

gcc/testsuite/ChangeLog

	* gcc.target/riscv/promote-type-for-libcall.c: New.
---
 gcc/config/riscv/riscv.c                           | 28 +++++++++++++++-
 .../gcc.target/riscv/promote-type-for-libcall.c    | 37 ++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/promote-type-for-libcall.c

Comments

Jim Wilson Aug. 2, 2019, 4:25 a.m. UTC | #1 
On Thu, Aug 1, 2019 at 4:48 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> gcc/ChangeLog
>         * config/riscv/riscv.c (riscv_promote_function_mode): New.
>         (TARGET_PROMOTE_FUNCTION_MODE): Use riscv_promote_function_mode.
> gcc/testsuite/ChangeLog
>         * gcc.target/riscv/promote-type-for-libcall.c: New.

Yes, this looks correct to me, though I couldn't get the testcase to
fail until I thought to try gcc-8.3.  It turns out that gcc-9
optimizes away the __floatsidf2 calls, and that prevents the bug from
triggering.  The bug fix is still right though and should still go in.
It failure is just a little harder to trigger than expected.

> +/* Implement TARGET_PROMOTE_FUNCTION_MODE */

Missing a period at the end of the sentence, and the second space
before the comment close.  The MIPS port has the same comment typo as
you copied it from there.  You can fix the typo in the MIPS port too
if you want.

Jim
Kito Cheng Aug. 2, 2019, 7:01 a.m. UTC | #2 
> > gcc/ChangeLog
> >         * config/riscv/riscv.c (riscv_promote_function_mode): New.
> >         (TARGET_PROMOTE_FUNCTION_MODE): Use riscv_promote_function_mode.
> > gcc/testsuite/ChangeLog
> >         * gcc.target/riscv/promote-type-for-libcall.c: New.
>
> Yes, this looks correct to me, though I couldn't get the testcase to
> fail until I thought to try gcc-8.3.  It turns out that gcc-9
> optimizes away the __floatsidf2 calls, and that prevents the bug from
> triggering.  The bug fix is still right though and should still go in.
> It failure is just a little harder to trigger than expected.

My mistake during reduce the testcase, I'll send v2 patch to update test case,
let it able trigger on trunk.


> > +/* Implement TARGET_PROMOTE_FUNCTION_MODE */
>
> Missing a period at the end of the sentence, and the second space
> before the comment close.  The MIPS port has the same comment typo as
> you copied it from there.  You can fix the typo in the MIPS port too
> if you want.
>
> Jim
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index e274f1b..7d80fe1 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -4910,6 +4910,32 @@  riscv_constant_alignment (const_tree exp, HOST_WIDE_INT align)
   return align;
 }
 
+/* Implement TARGET_PROMOTE_FUNCTION_MODE */
+
+/* This function is equivalent to default_promote_function_mode_always_promote
+   except that it returns a promoted mode even if type is NULL_TREE.  This is
+   needed by libcalls which have no type (only a mode) such as fixed conversion
+   routines that take a signed or unsigned char/short/int argument and convert
+   it to a fixed type.  */
+
+static machine_mode
+riscv_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
+			     machine_mode mode,
+			     int *punsignedp ATTRIBUTE_UNUSED,
+			     const_tree fntype ATTRIBUTE_UNUSED,
+			     int for_return ATTRIBUTE_UNUSED)
+{
+  int unsignedp;
+
+  if (type != NULL_TREE)
+    return promote_mode (type, mode, punsignedp);
+
+  unsignedp = *punsignedp;
+  PROMOTE_MODE (mode, unsignedp, type);
+  *punsignedp = unsignedp;
+  return mode;
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -4951,7 +4977,7 @@  riscv_constant_alignment (const_tree exp, HOST_WIDE_INT align)
 #define TARGET_EXPAND_BUILTIN_VA_START riscv_va_start
 
 #undef  TARGET_PROMOTE_FUNCTION_MODE
-#define TARGET_PROMOTE_FUNCTION_MODE default_promote_function_mode_always_promote
+#define TARGET_PROMOTE_FUNCTION_MODE riscv_promote_function_mode
 
 #undef TARGET_RETURN_IN_MEMORY
 #define TARGET_RETURN_IN_MEMORY riscv_return_in_memory
diff --git a/gcc/testsuite/gcc.target/riscv/promote-type-for-libcall.c b/gcc/testsuite/gcc.target/riscv/promote-type-for-libcall.c
new file mode 100644
index 0000000..5d37029
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/promote-type-for-libcall.c
@@ -0,0 +1,37 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+
+#include <stdio.h>
+#include <stdlib.h>
+#define N 4
+volatile float f[N];
+int x[N] __attribute__((aligned(8)));
+int main() {
+  int i;
+  x[0] = -1;
+  x[1] = 2;
+  x[2] = -2;
+  x[3] = 2;
+
+  for (i=0;i<N;++i){
+    f[i] = x[i];
+  }
+
+  if (f[0] != -1.0f) {
+    abort();
+  }
+
+  if (f[1] != 2.0f) {
+    abort();
+  }
+
+  if (f[2] != -2.0f) {
+    abort();
+  }
+
+  if (f[3] != 2.0f) {
+    abort();
+  }
+
+  return 0;
+}