public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Improve MSP430 hardware multiply support
@ 2020-11-15 21:14 Jozef Lawrynowicz
  2020-11-15 21:16 ` [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders Jozef Lawrynowicz
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jozef Lawrynowicz @ 2020-11-15 21:14 UTC (permalink / raw)
  To: gcc-patches

The attached patch series improves MSP430 hardware multiply support, by
improving code generation when setting up the 16-bit and 32-bit hardware
multiply functions, and adding a 64-bit hardware multiply library
function for devices that have a 32-bit hardware multiplier.

Successfully regtested GCC and G++ testsuites for:
    msp430-sim
    msp430-sim/-mcpu=msp430
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=f5series/-mlarge/-mdata-region=either/-mcode-region=either
    msp430-sim/-mlarge
    msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either

Additionally regtested GCC execute.exp for:
    msp430-sim/-mhwmult=16bit
    msp430-sim/-mhwmult=32bit
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=none
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=16bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=32bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=f5series
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=none

Ok for trunk?

Jozef Lawrynowicz (2):
  MSP430: Add mulhi, mulsi and {u,}mulsidi3  expanders
  MSP430: Add 64-bit hardware multiply support

 gcc/config/msp430/msp430.md       | 61 ++++++++++++++++++++++--
 libgcc/config/msp430/lib2hw_mul.S | 77 +++++++++++++++++++++++++++++--
 libgcc/config/msp430/lib2mul.c    | 52 +++++++++++++++++++++
 libgcc/config/msp430/t-msp430     |  5 ++
 4 files changed, 186 insertions(+), 9 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders
  2020-11-15 21:14 [PATCH 0/2] Improve MSP430 hardware multiply support Jozef Lawrynowicz
@ 2020-11-15 21:16 ` Jozef Lawrynowicz
  2020-11-15 21:18 ` [PATCH 2/2] MSP430: Add 64-bit hardware multiply support Jozef Lawrynowicz
  2020-11-17  1:36 ` [PATCH 0/2] Improve MSP430 " Jeff Law
  2 siblings, 0 replies; 6+ messages in thread
From: Jozef Lawrynowicz @ 2020-11-15 21:16 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]

GCC generates better code when multiplication operations, which require
library functions to perform, are caught in early in RTL, rather than
leaving the operation to be mapped to a library function later on.

When there is hardware multiply support, it is more efficient to perform
widening multiplication using the hardware multiplier instead of letting
GCC widen the arguments before calling the multiplication routine in the
wider mode.

Successfully regtested GCC and G++ testsuites for:
    msp430-sim
    msp430-sim/-mcpu=msp430
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=f5series/-mlarge/-mdata-region=either/-mcode-region=either
    msp430-sim/-mlarge
    msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either

Additionally regtested GCC execute.exp for:
    msp430-sim/-mhwmult=16bit
    msp430-sim/-mhwmult=32bit
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=none
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=16bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=32bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=f5series
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=none

Ok for trunk?

[-- Attachment #2: 0001-MSP430-Add-mulhi-mulsi-and-u-mulsidi3-expanders.patch --]
[-- Type: text/plain, Size: 4570 bytes --]

From 2f9bbb5dde866627c0dc321ec102ba3ef73d591c Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Sun, 15 Nov 2020 21:03:10 +0000
Subject: [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders

GCC generates better code when multiplication operations, which require
library functions to perform, are caught in early in RTL, rather than
leaving the operation to be mapped to a library function later on.

When there is hardware multiply support, it is more efficient to perform
widening multiplication using the hardware multiplier instead of letting
GCC widen the arguments before calling the multiplication routine in the
wider mode.

gcc/ChangeLog:

	* config/msp430/msp430.md (mulhi3): New.
	(mulsi3): New.
	(mulsidi3): Rename to *mulsidi3_inline.
	(umulsidi3): Rename to *umulsidi3_inline.
	(mulsidi3): New define_expand.
	(umulsidi3): New define_expand.
---
 gcc/config/msp430/msp430.md | 61 ++++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 5 deletions(-)

diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index 65e951774b1..714cd84a095 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -1724,6 +1724,28 @@ (define_insn "delay_cycles_1"
   [(set_attr "length" "2")]
 )
 
+(define_expand "mulhi3"
+  [(set (match_operand:HI			   0 "register_operand" "=r")
+	(mult:HI (match_operand:HI 1 "register_operand" "%0")
+		 (match_operand:HI 2 "register_operand" "r")))]
+  "msp430_has_hwmult ()"
+  {
+  msp430_expand_helper (operands, "__mspabi_mpyi", false);
+  DONE;
+  }
+)
+
+(define_expand "mulsi3"
+  [(set (match_operand:SI			   0 "register_operand" "=r")
+	(mult:SI (match_operand:SI 1 "register_operand" "%0")
+		 (match_operand:SI 2 "register_operand" "r")))]
+  "msp430_has_hwmult ()"
+  {
+  msp430_expand_helper (operands, "__mspabi_mpyl", false);
+  DONE;
+  }
+)
+
 ; libgcc helper functions for widening multiplication aren't currently
 ; generated by gcc, so we can't catch them later and map them to the mspabi
 ; functions.
@@ -1733,9 +1755,7 @@ (define_insn "delay_cycles_1"
 ; If we don't have hardware multiply support, it will generally be slower and
 ; result in larger code to call the mspabi library function to perform the
 ; widening multiplication than just leaving GCC to widen the arguments itself.
-;
-; We don't use library functions for SImode->DImode widening since its always
-; larger and slower than letting GCC widen the arguments inline.
+
 (define_expand "mulhisi3"
   [(set (match_operand:SI			   0 "register_operand" "=r")
 	(mult:SI (sign_extend:SI (match_operand:HI 1 "register_operand" "%0"))
@@ -1766,6 +1786,37 @@ (define_expand "umulhisi3"
   }
 )
 
+(define_expand "mulsidi3"
+  [(set (match_operand:DI			   0 "register_operand" "=r")
+	(mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "%0"))
+		 (sign_extend:DI (match_operand:SI 2 "register_operand" "r"))))]
+  "msp430_has_hwmult ()"
+  {
+    /* Leave the other case for the inline insn.  */
+    if (!(optimize > 2 && msp430_has_hwmult ()))
+    {
+      msp430_expand_helper (operands, "__mspabi_mpysll", false);
+      DONE;
+    }
+  }
+)
+
+(define_expand "umulsidi3"
+  [(set (match_operand:DI			   0 "register_operand" "=r")
+	(mult:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "%0"))
+		 (zero_extend:DI (match_operand:SI 2 "register_operand" "r"))))]
+  "msp430_has_hwmult ()"
+  {
+    /* Leave the other case for the inline insn.  */
+    if (!(optimize > 2 && msp430_has_hwmult ()))
+    {
+      msp430_expand_helper (operands, "__mspabi_mpyull", false);
+      DONE;
+    }
+  }
+)
+
+
 (define_insn "*mulhisi3_inline"
   [(set (match_operand:SI                          0 "register_operand" "=r")
 	(mult:SI (sign_extend:SI (match_operand:HI 1 "register_operand" "%0"))
@@ -1794,7 +1845,7 @@ (define_insn "*umulhisi3_inline"
   [(set_attr "length" "24")]
 )
 
-(define_insn "mulsidi3"
+(define_insn "*mulsidi3_inline"
   [(set (match_operand:DI                          0 "register_operand" "=r")
 	(mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "%0"))
 		 (sign_extend:DI (match_operand:SI 2 "register_operand" "r"))))]
@@ -1808,7 +1859,7 @@ (define_insn "mulsidi3"
   [(set_attr "length" "40")]
 )
 
-(define_insn "umulsidi3"
+(define_insn "*umulsidi3_inline"
   [(set (match_operand:DI                          0 "register_operand" "=r")
 	(mult:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "%0"))
 		 (zero_extend:DI (match_operand:SI 2 "register_operand" "r"))))]
-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] MSP430: Add 64-bit hardware multiply support
  2020-11-15 21:14 [PATCH 0/2] Improve MSP430 hardware multiply support Jozef Lawrynowicz
  2020-11-15 21:16 ` [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders Jozef Lawrynowicz
@ 2020-11-15 21:18 ` Jozef Lawrynowicz
  2020-11-17  1:36 ` [PATCH 0/2] Improve MSP430 " Jeff Law
  2 siblings, 0 replies; 6+ messages in thread
From: Jozef Lawrynowicz @ 2020-11-15 21:18 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Hardware multipliers that support widening 32-bit multiplication can
be used to perform a 64-bit * 64-bit multiplication more efficiently
than a software implementation.

The following equation is used to perform 64-bit multiplication for
devices with "32bit" or "f5series" hardware multiply versions:

  64bit_result = (low32_op0 * lop32_op1)
    + ((low32_op0 * high32_op1) << 32)
       + ((high32_op0 * low32_op1) << 32)

Successfully regtested GCC and G++ testsuites for:
    msp430-sim
    msp430-sim/-mcpu=msp430
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=f5series/-mlarge/-mdata-region=either/-mcode-region=either
    msp430-sim/-mlarge
    msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either

Additionally regtested GCC execute.exp for:
    msp430-sim/-mhwmult=16bit
    msp430-sim/-mhwmult=32bit
    msp430-sim/-mhwmult=f5series
    msp430-sim/-mhwmult=none
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=16bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=32bit
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=f5series
    msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=none

Ok for trunk?

[-- Attachment #2: 0002-MSP430-Add-64-bit-hardware-multiply-support.patch --]
[-- Type: text/plain, Size: 7530 bytes --]

From cb1ea86822cc8c6b0183afd06da42e320034dc43 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Sun, 15 Nov 2020 21:03:14 +0000
Subject: [PATCH 2/2] MSP430: Add 64-bit hardware multiply support

Hardware multipliers that support widening 32-bit multiplication can
be used to perform a 64-bit * 64-bit multiplication more efficiently
than a software implementation.

The following equation is used to perform 64-bit multiplication for
devices with "32bit" or "f5series" hardware multiply versions:

  64bit_result = (low32_op0 * lop32_op1)
    + ((low32_op0 * high32_op1) << 32)
       + ((high32_op0 * low32_op1) << 32)

libgcc/ChangeLog:

	* config/msp430/lib2hw_mul.S (mult64_hw): New.
	(if MUL_32): Use mult64_hw for __muldi3.
	(if MUL_F5): Use mult64_hw for __muldi3.
	* config/msp430/lib2mul.c (__muldi3): New.
	* config/msp430/t-msp430 (LIB2FUNCS_EXCLUDE): Define.
---
 libgcc/config/msp430/lib2hw_mul.S | 77 +++++++++++++++++++++++++++++--
 libgcc/config/msp430/lib2mul.c    | 52 +++++++++++++++++++++
 libgcc/config/msp430/t-msp430     |  5 ++
 3 files changed, 130 insertions(+), 4 deletions(-)

diff --git a/libgcc/config/msp430/lib2hw_mul.S b/libgcc/config/msp430/lib2hw_mul.S
index 0fbafcd8b95..855dcd8bb55 100644
--- a/libgcc/config/msp430/lib2hw_mul.S
+++ b/libgcc/config/msp430/lib2hw_mul.S
@@ -207,6 +207,73 @@
 	MOV.W   &\RES3, R15		; Ready high 16-bits for return
 .endm
 
+.macro mult64_hw  MPY32_LO MPY32_HI OP2_LO OP2_HI RES0 RES1 RES2 RES3
+;* * 64-bit hardware multiply with a 64-bit result
+;*	int64 = int64 * int64
+;*
+;*   - Operand 1 is in R8, R9, R10, R11
+;*   - Operand 2 is in R12, R13, R14, R15
+;*   - Result    is in R12, R13, R14, R15
+;*
+;* 64-bit multiplication is achieved using the 32-bit hardware multiplier with
+;* the following equation:
+;*    R12:R15 = (R8:R9 * R12:R13) + ((R8:R9 * R14:R15) << 32) + ((R10:R11 * R12:R13) << 32)
+;*
+;* The left shift by 32 is handled with minimal cost by saving the two low
+;* words and discarding the two high words.
+;*
+;* To ensure that the multiply is performed atomically, interrupts are
+;* disabled upon routine entry.  Interrupt state is restored upon exit.
+;*
+;*   Registers used:  R6, R7, R8, R9, R10, R11, R12, R13, R14, R15
+;*
+;* Macro arguments are the memory locations of the hardware registers.
+;*
+#if defined(__MSP430X_LARGE__)
+	PUSHM.A	#5, R10
+#elif defined(__MSP430X__)
+	PUSHM.W	#5, R10
+#else
+	PUSH R10 { PUSH R9 { PUSH R8 { PUSH R7 { PUSH R6
+#endif
+	; Multiply the low 32-bits of op0 and the high 32-bits of op1.
+	MOV.W	R8, &\MPY32_LO
+	MOV.W	R9, &\MPY32_HI
+	MOV.W	R14, &\OP2_LO
+	MOV.W	R15, &\OP2_HI
+	; Save the low 32-bits of the result.
+	MOV.W	&\RES0, R6
+	MOV.W	&\RES1, R7
+	; Multiply the high 32-bits of op0 and the low 32-bits of op1.
+	MOV.W	R10, &\MPY32_LO
+	MOV.W	R11, &\MPY32_HI
+	MOV.W	R12, &\OP2_LO
+	MOV.W	R13, &\OP2_HI
+	; Add the low 32-bits of the result to the previously saved result.
+	ADD.W	&\RES0, R6
+	ADDC.W	&\RES1, R7
+	; Multiply the low 32-bits of op0 and op1.
+	MOV.W	R8, &\MPY32_LO
+	MOV.W	R9, &\MPY32_HI
+	MOV.W	R12, &\OP2_LO
+	MOV.W	R13, &\OP2_HI
+	; Write the return values
+	MOV.W	&\RES0, R12
+	MOV.W   &\RES1, R13
+	MOV.W	&\RES2, R14
+	MOV.W   &\RES3, R15
+	; Add the saved low 32-bit results from earlier to the high 32-bits of
+	; this result, effectively shifting those two results left by 32 bits.
+	ADD.W	R6, R14
+	ADDC.W  R7, R15
+#if defined(__MSP430X_LARGE__)
+	POPM.A	#5, R10
+#elif defined(__MSP430X__)
+	POPM.W	#5, R10
+#else
+	POP R6 { POP R7 { POP R8 { POP R9 { POP R10
+#endif
+.endm
 
 ;; EABI mandated names:
 ;; 
@@ -365,8 +432,9 @@
 	mult3264_hw MPY32L, MPY32H, OP2L, OP2H, RES0, RES1, RES2, RES3
 	end_func   __umulsidi2
 
-	;; FIXME: Add a hardware version of this function.
-	fake_func __muldi3    __mspabi_mpyll  __mspabi_mpyll_hw32
+	start_func __muldi3   __mspabi_mpyll __mspabi_mpyll_hw32
+	mult64_hw MPY32L, MPY32H, OP2L, OP2H, RES0, RES1, RES2, RES3
+	end_func __muldi3
 
 #elif defined MUL_F5
 /* The F5xxx series of MCUs support the same 16-bit and 32-bit multiply
@@ -397,8 +465,9 @@
 	mult3264_hw MPY32L_F5, MPY32H_F5, OP2L_F5, OP2H_F5, RES0_F5, RES1_F5, RES2_F5, RES3_F5
 	end_func   __umulsidi2
 
-	;; FIXME: Add a hardware version of this function.
-	fake_func __muldi3   __mspabi_mpyll __mspabi_mpyll_f5hw
+	start_func __muldi3   __mspabi_mpyll __mspabi_mpyll_f5hw
+	mult64_hw MPY32L_F5, MPY32H_F5, OP2L_F5, OP2H_F5, RES0_F5, RES1_F5, RES2_F5, RES3_F5
+	end_func __muldi3
 
 #else
 #error MUL type not defined
diff --git a/libgcc/config/msp430/lib2mul.c b/libgcc/config/msp430/lib2mul.c
index 955e7bb0cf9..ab709b35892 100644
--- a/libgcc/config/msp430/lib2mul.c
+++ b/libgcc/config/msp430/lib2mul.c
@@ -30,6 +30,58 @@ typedef unsigned int  uint08_type   __attribute__ ((mode (QI)));
 #define C3B(a,b,c) a##b##c
 #define C3(a,b,c) C3B(a,b,c)
 
+#if defined (MUL_NONE) || defined (MUL_16)
+/* __muldi3 must be excluded from libgcc.a to prevent multiple-definition
+   errors for the hwmult configurations which have their own definition.
+   But for MUL_NONE and MUL_16, the software version is still required, so
+   the necessary preprocessed output from libgcc2.c to compile that
+   software version of __muldi3 is below.  */
+typedef unsigned int USItype __attribute__ ((mode (SI)));
+typedef int DItype __attribute__ ((mode (DI)));
+typedef int SItype __attribute__ ((mode (SI)));
+struct DWstruct {SItype low, high;};
+
+typedef union
+{
+  struct DWstruct s;
+  DItype ll;
+} DWunion;
+
+DItype __muldi3 (DItype u, DItype v);
+
+DItype
+__muldi3 (DItype u, DItype v)
+{
+  const DWunion uu = {.ll = u};
+  const DWunion vv = {.ll = v};
+  /* The next block of code is expanded from the following line:
+     DWunion w = {.ll = __umulsidi3 (uu.s.low, vv.s.low)};  */
+  DWunion w;
+  USItype __x0, __x1, __x2, __x3;
+  USItype __ul, __vl, __uh, __vh;
+  __ul = ((USItype) (uu.s.low) & (((USItype) 1 << ((4 * 8) / 2)) - 1));
+  __uh = ((USItype) (uu.s.low) >> ((4 * 8) / 2));
+  __vl = ((USItype) (vv.s.low) & (((USItype) 1 << ((4 * 8) / 2)) - 1));
+  __vh = ((USItype) (vv.s.low) >> ((4 * 8) / 2));
+  __x0 = (USItype) __ul * __vl;
+  __x1 = (USItype) __ul * __vh;
+  __x2 = (USItype) __uh * __vl;
+  __x3 = (USItype) __uh * __vh;
+  __x1 += ((USItype) (__x0) >> ((4 * 8) / 2));
+  __x1 += __x2;
+  if (__x1 < __x2)
+    __x3 += ((USItype) 1 << ((4 * 8) / 2));
+  (w.s.high) = __x3 + ((USItype) (__x1) >> ((4 * 8) / 2));
+  (w.s.low) = ((USItype) (__x1) & (((USItype) 1 << ((4 * 8) / 2)) - 1))
+    * ((USItype) 1 << ((4 * 8) / 2))
+    + ((USItype) (__x0) & (((USItype) 1 << ((4 * 8) / 2)) - 1));
+
+  w.s.high += ((USItype) uu.s.low * (USItype) vv.s.high
+	       + (USItype) uu.s.high * (USItype) vv.s.low);
+  return w.ll;
+}
+#endif
+
 #if defined MUL_NONE
 
 /* The software multiply library needs __mspabi_mpyll.  */
diff --git a/libgcc/config/msp430/t-msp430 b/libgcc/config/msp430/t-msp430
index be56e884ba6..9a79b5bbf55 100644
--- a/libgcc/config/msp430/t-msp430
+++ b/libgcc/config/msp430/t-msp430
@@ -40,6 +40,11 @@ LIB2ADD = \
 	$(srcdir)/config/msp430/floathisf.c \
 	$(srcdir)/config/msp430/cmpd.c
 
+# 32-bit and F5series hardware multiply have their own version of this function.
+# To handle the case when there is no hardware multiply or only 16-bit hardware
+# multiply, the libgcc version has been copied to lib2mul.c.
+LIB2FUNCS_EXCLUDE += _muldi3
+
 HOST_LIBGCC2_CFLAGS += -Os -ffunction-sections -fdata-sections -mhwmult=none
 
 crtbegin_no_eh.o: $(srcdir)/crtstuff.c
-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] Improve MSP430 hardware multiply support
  2020-11-15 21:14 [PATCH 0/2] Improve MSP430 hardware multiply support Jozef Lawrynowicz
  2020-11-15 21:16 ` [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders Jozef Lawrynowicz
  2020-11-15 21:18 ` [PATCH 2/2] MSP430: Add 64-bit hardware multiply support Jozef Lawrynowicz
@ 2020-11-17  1:36 ` Jeff Law
  2020-11-17 14:47   ` Jozef Lawrynowicz
  2 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2020-11-17  1:36 UTC (permalink / raw)
  To: gcc-patches



On 11/15/20 2:14 PM, Jozef Lawrynowicz wrote:
> The attached patch series improves MSP430 hardware multiply support, by
> improving code generation when setting up the 16-bit and 32-bit hardware
> multiply functions, and adding a 64-bit hardware multiply library
> function for devices that have a 32-bit hardware multiplier.
>
> Successfully regtested GCC and G++ testsuites for:
>     msp430-sim
>     msp430-sim/-mcpu=msp430
>     msp430-sim/-mhwmult=f5series
>     msp430-sim/-mhwmult=f5series/-mlarge/-mdata-region=either/-mcode-region=either
>     msp430-sim/-mlarge
>     msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either
>
> Additionally regtested GCC execute.exp for:
>     msp430-sim/-mhwmult=16bit
>     msp430-sim/-mhwmult=32bit
>     msp430-sim/-mhwmult=f5series
>     msp430-sim/-mhwmult=none
>     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=16bit
>     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=32bit
>     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=f5series
>     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=none
>
> Ok for trunk?
>
> Jozef Lawrynowicz (2):
>   MSP430: Add mulhi, mulsi and {u,}mulsidi3  expanders
>   MSP430: Add 64-bit hardware multiply support
>
>  gcc/config/msp430/msp430.md       | 61 ++++++++++++++++++++++--
>  libgcc/config/msp430/lib2hw_mul.S | 77 +++++++++++++++++++++++++++++--
>  libgcc/config/msp430/lib2mul.c    | 52 +++++++++++++++++++++
>  libgcc/config/msp430/t-msp430     |  5 ++
>  4 files changed, 186 insertions(+), 9 deletions(-)
Both are fine.

BTW, what would be a reasonable set of multlibs for automated testing? 
My tester has the ability to define them on a per-target basis, but I
haven't tried to do that except for targets that I happen to know
well.   So right now it's just using the default via
-target_board=msp430-sim.    Figure we've probably got a time budget to
add 3 multilibs without causing headaches.  What 3 might you suggest?

Jeff


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] Improve MSP430 hardware multiply support
  2020-11-17  1:36 ` [PATCH 0/2] Improve MSP430 " Jeff Law
@ 2020-11-17 14:47   ` Jozef Lawrynowicz
  2020-11-19 16:07     ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Jozef Lawrynowicz @ 2020-11-17 14:47 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Mon, Nov 16, 2020 at 06:36:17PM -0700, Jeff Law via Gcc-patches wrote:
> 
> 
> On 11/15/20 2:14 PM, Jozef Lawrynowicz wrote:
> > The attached patch series improves MSP430 hardware multiply support, by
> > improving code generation when setting up the 16-bit and 32-bit hardware
> > multiply functions, and adding a 64-bit hardware multiply library
> > function for devices that have a 32-bit hardware multiplier.
> >
> > Successfully regtested GCC and G++ testsuites for:
> >     msp430-sim
> >     msp430-sim/-mcpu=msp430
> >     msp430-sim/-mhwmult=f5series
> >     msp430-sim/-mhwmult=f5series/-mlarge/-mdata-region=either/-mcode-region=either
> >     msp430-sim/-mlarge
> >     msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either
> >
> > Additionally regtested GCC execute.exp for:
> >     msp430-sim/-mhwmult=16bit
> >     msp430-sim/-mhwmult=32bit
> >     msp430-sim/-mhwmult=f5series
> >     msp430-sim/-mhwmult=none
> >     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=16bit
> >     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=32bit
> >     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=f5series
> >     msp430-sim/-mlarge/-mcode-region=either/-mdata-region=either/-mhwmult=none
> >
> > Ok for trunk?
> >
> > Jozef Lawrynowicz (2):
> >   MSP430: Add mulhi, mulsi and {u,}mulsidi3  expanders
> >   MSP430: Add 64-bit hardware multiply support
> >
> >  gcc/config/msp430/msp430.md       | 61 ++++++++++++++++++++++--
> >  libgcc/config/msp430/lib2hw_mul.S | 77 +++++++++++++++++++++++++++++--
> >  libgcc/config/msp430/lib2mul.c    | 52 +++++++++++++++++++++
> >  libgcc/config/msp430/t-msp430     |  5 ++
> >  4 files changed, 186 insertions(+), 9 deletions(-)
> Both are fine.

Thanks.

> 
> BTW, what would be a reasonable set of multlibs for automated testing? 
> My tester has the ability to define them on a per-target basis, but I
> haven't tried to do that except for targets that I happen to know
> well.   So right now it's just using the default via
> -target_board=msp430-sim.    Figure we've probably got a time budget to
> add 3 multilibs without causing headaches.  What 3 might you suggest?

In addition to the default config, I would suggest:
  msp430-sim/-mcpu=msp430
    Test the 430 ISA
  msp430-sim/-mlarge/-mcode-region=either
    Test the large memory model with data assumed to be in the lower
    memory region (default, reduces code size penalty of using -mlarge),
    whilst shuffling code between the upper and lower memory regions to
    make the program fit.
  msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either
   Test the large memory model, shuffling code and data between upper
   and lower memory regions.

I should really use -mlarge/-mcode-region=either, instead of just
-mlarge, as well. -mcode-region=either doesn't change code gen, just
allows the linker shuffling of text sections so more tests build and so
we get better test coverage.

With limited testing capacity, testing hwmult configs is not very useful
unless hwmult behavior is specifically changed. There are msp430
specific tests to verify the options basically work.

Thanks,
Jozef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] Improve MSP430 hardware multiply support
  2020-11-17 14:47   ` Jozef Lawrynowicz
@ 2020-11-19 16:07     ` Jeff Law
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2020-11-19 16:07 UTC (permalink / raw)
  To: gcc-patches



On 11/17/20 7:47 AM, Jozef Lawrynowicz wrote:
> In addition to the default config, I would suggest:
>   msp430-sim/-mcpu=msp430
>     Test the 430 ISA
>   msp430-sim/-mlarge/-mcode-region=either
>     Test the large memory model with data assumed to be in the lower
>     memory region (default, reduces code size penalty of using -mlarge),
>     whilst shuffling code between the upper and lower memory regions to
>     make the program fit.
>   msp430-sim/-mlarge/-mdata-region=either/-mcode-region=either
>    Test the large memory model, shuffling code and data between upper
>    and lower memory regions.
>
> I should really use -mlarge/-mcode-region=either, instead of just
> -mlarge, as well. -mcode-region=either doesn't change code gen, just
> allows the linker shuffling of text sections so more tests build and so
> we get better test coverage.
>
> With limited testing capacity, testing hwmult configs is not very useful
> unless hwmult behavior is specifically changed. There are msp430
> specific tests to verify the options basically work.
ACK.  I've added those multilibs to msp430-elf configuration.

Thanks!

jeff


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-19 16:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-15 21:14 [PATCH 0/2] Improve MSP430 hardware multiply support Jozef Lawrynowicz
2020-11-15 21:16 ` [PATCH 1/2] MSP430: Add mulhi, mulsi and {u,}mulsidi3 expanders Jozef Lawrynowicz
2020-11-15 21:18 ` [PATCH 2/2] MSP430: Add 64-bit hardware multiply support Jozef Lawrynowicz
2020-11-17  1:36 ` [PATCH 0/2] Improve MSP430 " Jeff Law
2020-11-17 14:47   ` Jozef Lawrynowicz
2020-11-19 16:07     ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).