public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][MSP430][0/4] Reduce code size when performing bit shifts
@ 2019-06-04 13:03 Jozef Lawrynowicz
  2019-06-04 13:07 ` [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections Jozef Lawrynowicz
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-04 13:03 UTC (permalink / raw)
  To: gcc-patches; +Cc: nickc

The following series of patches reduces the code size of MSP430 programs that
make use of bit shifts.
The MSP430 does not have a native instruction for shifting integers, but in
some cases the native rotate instruction can be substituted. In other cases,
assembly code in libgcc is used to emulate the shift.

Successfully regtested the changes with the GCC and G++ testsuites for
msp430-elf, in both the small and large memory models.
I've included some details about the achieved code size reduction in the
individual patch submissions.

Ok for trunk?

jozefl (4):
  Put libgcc shift functions in their own sections
  Emulate 16-bit shifts with rotate insn when src operand is originally in
    memory
  Disable performance optimal library code shifts when optimizing for size
  Implement 64-bit shifts in assembly code

 gcc/config/msp430/msp430.c                         | 13 ++++-
 gcc/config/msp430/msp430.md                        | 66 ++++++++++++++++++----
 gcc/testsuite/gcc.target/msp430/emulate-slli.c     | 15 +++++
 gcc/testsuite/gcc.target/msp430/emulate-srai.c     | 15 +++++
 gcc/testsuite/gcc.target/msp430/emulate-srli.c     | 15 +++++
 gcc/testsuite/gcc.target/msp430/mspabi_sllll.c     | 10 ++++
 gcc/testsuite/gcc.target/msp430/mspabi_srall.c     | 10 ++++
 gcc/testsuite/gcc.target/msp430/mspabi_srlll.c     | 10 ++++
 .../gcc.target/msp430/size-optimized-shifts.c      | 26 +++++++++
 libgcc/config/msp430/slli.S                        | 41 +++++++++++++-
 libgcc/config/msp430/srai.S                        | 42 +++++++++++++-
 libgcc/config/msp430/srli.S                        | 43 +++++++++++++-
 12 files changed, 286 insertions(+), 20 deletions(-)

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

* [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections
  2019-06-04 13:03 [PATCH][MSP430][0/4] Reduce code size when performing bit shifts Jozef Lawrynowicz
@ 2019-06-04 13:07 ` Jozef Lawrynowicz
  2019-06-05 22:20   ` Jeff Law
  2019-06-04 13:11 ` [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory Jozef Lawrynowicz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-04 13:07 UTC (permalink / raw)
  To: gcc-patches; +Cc: nickc

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

This patch reduces code size by putting each of the shift library functions from
libgcc in their own section. This means that, for example, a 16-bit logical
left shift does not result in code to perform a 32-bit logical left shift being
included in the final executable, as the linker can now garbage collect unused
sections.

For the following program, the below code size reduction is observed:
  int a, b;

  int
  main (void)
  {
    a = a << b;
    return 0;
  }

Current trunk:
   text    data     bss     dec     hex filename
    572      12      22     606     25e a.out
With patch:
   text    data     bss     dec     hex filename
    466      12      22     500     1f4 a.out

Ok for trunk?

[-- Attachment #2: 0001-MSP430-Put-the-library-functions-for-bitwise-shifts-.patch --]
[-- Type: text/x-patch, Size: 3800 bytes --]

From 8017a4b453ae1b07bbeb75f7f7613a5bc5605159 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 13 May 2019 17:42:08 +0100
Subject: [PATCH 1/4] MSP430: Put the library functions for bitwise shifts in
 their own sections

libgcc/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/slli.S (__mspabi_slli_n): Put function in its own
	section.
	(__mspabi_slli): Likewise.
	(__mspabi_slll_n): Likewise.
	(__mspabi_slll): Likewise.
	* config/msp430/srai.S (__mspabi_srai_n): Likewise.
	(__mspabi_srai): Likewise.
	(__mspabi_sral_n): Likewise.
	(__mspabi_sral): Likewise.
	* config/msp430/srli.S (__mspabi_srli_n): Likewise.
	(__mspabi_srli): Likewise.
	(__mspabi_srll_n): Likewise.
	(__mspabi_srll): Likewise.
---
 libgcc/config/msp430/slli.S | 8 ++++++--
 libgcc/config/msp430/srai.S | 8 ++++++--
 libgcc/config/msp430/srli.S | 8 ++++++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/libgcc/config/msp430/slli.S b/libgcc/config/msp430/slli.S
index 9d151a97f5d..89ca35a9304 100644
--- a/libgcc/config/msp430/slli.S
+++ b/libgcc/config/msp430/slli.S
@@ -22,8 +22,9 @@
 	
 	.text
 
-/* Logical Left Shift - R12 -> R12 */
+/* Logical Left Shift - R12 -> R12.  */
 
+	.section	.text.__mspabi_slli_n
 	.macro	_slli n
 	.global __mspabi_slli_\n
 __mspabi_slli_\n:
@@ -51,6 +52,7 @@ __mspabi_slli_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_slli
 1:	ADD.W	#-1,R13
 	ADD.W	R12,R12
 	.global	__mspabi_slli
@@ -63,8 +65,9 @@ __mspabi_slli:
 	RET
 #endif
 
-/* Logical Left Shift - R12:R13 -> R12:R13 */
+/* Logical Left Shift - R12:R13 -> R12:R13.  */
 
+	.section	.text.__mspabi_slll_n
 	.macro	_slll	n
 	.global	__mspabi_slll_\n
 __mspabi_slll_\n:
@@ -93,6 +96,7 @@ __mspabi_slll_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_slll
 1:	ADD.W	#-1,R14
 	ADD.W	R12,R12
 	ADDC.W	R13,R13
diff --git a/libgcc/config/msp430/srai.S b/libgcc/config/msp430/srai.S
index 33c9b5ee62d..564f7989a8c 100644
--- a/libgcc/config/msp430/srai.S
+++ b/libgcc/config/msp430/srai.S
@@ -22,13 +22,14 @@
 	
 	.text
 
+	.section	.text.__mspabi_srai_n
 	.macro	_srai n
 	.global __mspabi_srai_\n
 __mspabi_srai_\n:
 	RRA.W	R12
 	.endm
 
-/* Logical Right Shift - R12 -> R12 */
+/* Arithmetic Right Shift - R12 -> R12.  */
 	_srai	15
 	_srai	14
 	_srai	13
@@ -50,6 +51,7 @@ __mspabi_srai_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_srai
 1:	ADD.W	#-1,R13
 	RRA.W	R12,R12
 	.global	__mspabi_srai
@@ -62,8 +64,9 @@ __mspabi_srai:
 	RET
 #endif
 
-/* Logical Right Shift - R12:R13 -> R12:R13 */
+/* Arithmetic Right Shift - R12:R13 -> R12:R13.  */
 
+	.section	.text.__mspabi_sral_n
 	.macro	_sral	n
 	.global	__mspabi_sral_\n
 __mspabi_sral_\n:
@@ -92,6 +95,7 @@ __mspabi_sral_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_sral
 1:	ADD.W	#-1,R14
 	RRA.W	R13
 	RRC.W	R12
diff --git a/libgcc/config/msp430/srli.S b/libgcc/config/msp430/srli.S
index dbe37f67a7d..4dd32ea4002 100644
--- a/libgcc/config/msp430/srli.S
+++ b/libgcc/config/msp430/srli.S
@@ -22,6 +22,7 @@
 	
 	.text
 
+	.section	.text.__mspabi_srli_n
 	.macro	_srli n
 	.global __mspabi_srli_\n
 __mspabi_srli_\n:
@@ -29,7 +30,7 @@ __mspabi_srli_\n:
 	RRC.W	R12
 	.endm
 
-/* Logical Right Shift - R12 -> R12 */
+/* Logical Right Shift - R12 -> R12.  */
 	_srli	15
 	_srli	14
 	_srli	13
@@ -51,6 +52,7 @@ __mspabi_srli_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_srli
 1:	ADD.W	#-1,R13
 	CLRC
 	RRC.W	R12,R12
@@ -64,8 +66,9 @@ __mspabi_srli:
 	RET
 #endif
 
-/* Logical Right Shift - R12:R13 -> R12:R13 */
+/* Logical Right Shift - R12:R13 -> R12:R13.  */
 
+	.section	.text.__mspabi_srll_n
 	.macro	_srll	n
 	.global	__mspabi_srll_\n
 __mspabi_srll_\n:
@@ -95,6 +98,7 @@ __mspabi_srll_\n:
 	RET
 #endif
 
+	.section	.text.__mspabi_srll
 1:	ADD.W	#-1,R14
 	CLRC
 	RRC.W	R13
-- 
2.17.1


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

* [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory
  2019-06-04 13:03 [PATCH][MSP430][0/4] Reduce code size when performing bit shifts Jozef Lawrynowicz
  2019-06-04 13:07 ` [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections Jozef Lawrynowicz
@ 2019-06-04 13:11 ` Jozef Lawrynowicz
  2019-06-05 22:21   ` Jeff Law
  2019-06-04 13:14 ` [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size Jozef Lawrynowicz
  2019-06-04 13:17 ` [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code Jozef Lawrynowicz
  3 siblings, 1 reply; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-04 13:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: nickc

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

This patch reduces code size by enabling the emulation of some 16-bit shift
instructions with the native rotate instructions, when the source operand is in
memory. This is achieved by forcing the source operand into a register.

For the following program, the below code size reduction is observed:
  int a;

  int
  main (void)
  {
    a = a << 4;
    return 0;
  }

With shift patch 1:
   text    data     bss     dec     hex filename
    484      12      20     516     204 a.out
With new patch:
   text    data     bss     dec     hex filename
    452      12      20     484     1e4 a.out

Ok for trunk?

[-- Attachment #2: 0002-MSP430-Force-the-src-operand-of-a-HImode-shift-into-.patch --]
[-- Type: text/x-patch, Size: 4309 bytes --]

From e609f63d49227ce385316896dde6a476f5f27db7 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 13 May 2019 17:48:00 +0100
Subject: [PATCH 2/4] MSP430: Force the src operand of a HImode shift into a
 register if it is in memory

gcc/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.md (ashlhi3): Force shift src operand into a
	register if it is in memory, so the shift can be emulated with a rotate
	instruction.
	(ashrhi3): Likewise.
	(lshrhi3): Likewise.

gcc/testsuite/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc.target/msp430/emulate-slli.c: New test.
	* gcc.target/msp430/emulate-srai.c: New test.
	* gcc.target/msp430/emulate-srli.c: New test.
---
 gcc/config/msp430/msp430.md                    | 15 +++++++++------
 gcc/testsuite/gcc.target/msp430/emulate-slli.c | 15 +++++++++++++++
 gcc/testsuite/gcc.target/msp430/emulate-srai.c | 15 +++++++++++++++
 gcc/testsuite/gcc.target/msp430/emulate-srli.c | 15 +++++++++++++++
 4 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/emulate-slli.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/emulate-srai.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/emulate-srli.c

diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index 344d21d9378..58c1f4edc9c 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -756,8 +756,9 @@
 		   (match_operand:HI 2 "general_operand")))]
   ""
   {
-    if (GET_CODE (operands[1]) == SUBREG
-        && REG_P (XEXP (operands[1], 0)))
+    if ((GET_CODE (operands[1]) == SUBREG
+	 && REG_P (XEXP (operands[1], 0)))
+	|| MEM_P (operands[1]))
       operands[1] = force_reg (HImode, operands[1]);
     if (msp430x
         && REG_P (operands[0])
@@ -828,8 +829,9 @@
 		     (match_operand:HI 2 "general_operand")))]
   ""
   {
-    if (GET_CODE (operands[1]) == SUBREG
-        && REG_P (XEXP (operands[1], 0)))
+    if ((GET_CODE (operands[1]) == SUBREG
+	 && REG_P (XEXP (operands[1], 0)))
+	|| MEM_P (operands[1]))
       operands[1] = force_reg (HImode, operands[1]);
     if (msp430x
         && REG_P (operands[0])
@@ -916,8 +918,9 @@
 		     (match_operand:HI 2 "general_operand")))]
   ""
   {
-    if (GET_CODE (operands[1]) == SUBREG
-        && REG_P (XEXP (operands[1], 0)))
+    if ((GET_CODE (operands[1]) == SUBREG
+	 && REG_P (XEXP (operands[1], 0)))
+	|| MEM_P (operands[1]))
       operands[1] = force_reg (HImode, operands[1]);
     if (msp430x
         && REG_P (operands[0])
diff --git a/gcc/testsuite/gcc.target/msp430/emulate-slli.c b/gcc/testsuite/gcc.target/msp430/emulate-slli.c
new file mode 100644
index 00000000000..0ed09d55d8c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/emulate-slli.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler-not "mspabi_slli" } } */
+/* { dg-final { scan-assembler "rlax" } } */
+
+/* Ensure that HImode shifts with source operand in memory are emulated with a
+   rotate instructions.  */
+
+int a;
+
+void
+foo (void)
+{
+  a = a << 4;
+}
diff --git a/gcc/testsuite/gcc.target/msp430/emulate-srai.c b/gcc/testsuite/gcc.target/msp430/emulate-srai.c
new file mode 100644
index 00000000000..66291717a02
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/emulate-srai.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler-not "mspabi_srai" } } */
+/* { dg-final { scan-assembler "rrax" } } */
+
+/* Ensure that HImode shifts with source operand in memory are emulated with a
+   rotate instructions.  */
+
+int a;
+
+void
+foo (void)
+{
+  a = a >> 4;
+}
diff --git a/gcc/testsuite/gcc.target/msp430/emulate-srli.c b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
new file mode 100644
index 00000000000..c10f30b2779
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler-not "mspabi_srli" } } */
+/* { dg-final { scan-assembler "rrum" } } */
+
+/* Ensure that HImode shifts with source operand in memory are emulated with a
+   rotate instructions.  */
+
+unsigned int a;
+
+void
+foo (void)
+{
+  a = a >> 4;
+}
-- 
2.17.1


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

* [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size
  2019-06-04 13:03 [PATCH][MSP430][0/4] Reduce code size when performing bit shifts Jozef Lawrynowicz
  2019-06-04 13:07 ` [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections Jozef Lawrynowicz
  2019-06-04 13:11 ` [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory Jozef Lawrynowicz
@ 2019-06-04 13:14 ` Jozef Lawrynowicz
  2019-06-05 22:22   ` Jeff Law
  2019-06-04 13:17 ` [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code Jozef Lawrynowicz
  3 siblings, 1 reply; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-04 13:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: nickc

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

This patch reduces code size by disabling the performance optimized,
"const_variant" of shift library functions when optimization for size is
enabled.

For the following program, the below code size reduction is observed:
  long a;

  int
  main (void)
  {
    a = a >> 4;
    return 0;
  }

With shift patch 2:
   text    data     bss     dec     hex filename
    522      12      22     556     22c a.out
New patch:
   text    data     bss     dec     hex filename
    474      12      22     508     1fc a.out

Ok for trunk?

[-- Attachment #2: 0003-MSP430-Do-not-use-the-performance-optimized-variant-.patch --]
[-- Type: text/x-patch, Size: 4011 bytes --]

From 894b6809822ba3a3a1bab3750abe29e03f2a3ad6 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 13 May 2019 17:52:19 +0100
Subject: [PATCH 3/4] MSP430: Do not use the performance optimized variant of a
 shift by constant amount when optimizing for size

gcc/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.md (ashlhi3): Use the const_variant of shift
	library functions only when not optimizing for size.
	(ashlsi3): Likewise.
	(ashrhi3): Likewise.
	(ashrsi3): Likewise.
	(lshrhi3): Likewise.
	(lshrsi3): Likewise.

gcc/testsuite/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc.target/msp430/size-optimized-shifts.c: New test.
---
 gcc/config/msp430/msp430.md                   | 15 ++++++-----
 .../gcc.target/msp430/size-optimized-shifts.c | 26 +++++++++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/size-optimized-shifts.c

diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index 58c1f4edc9c..76296a2f317 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -769,7 +769,10 @@
 	     && INTVAL (operands[2]) == 1)
       emit_insn (gen_slli_1 (operands[0], operands[1]));
     else		 
-      msp430_expand_helper (operands, \"__mspabi_slli\", true);
+      /* The const variants of mspabi shifts have larger code size than the
+	 generic version, so use the generic version if optimizing for
+	 size.  */
+      msp430_expand_helper (operands, \"__mspabi_slli\", !optimize_size);
     DONE;
   }
 )
@@ -815,7 +818,7 @@
 	(ashift:SI (match_operand:SI 1 "general_operand")
 		   (match_operand:SI 2 "general_operand")))]
   ""
-  "msp430_expand_helper (operands, \"__mspabi_slll\", true);
+  "msp430_expand_helper (operands, \"__mspabi_slll\", !optimize_size);
    DONE;"
 )
 
@@ -842,7 +845,7 @@
 	     && INTVAL (operands[2]) == 1)
       emit_insn (gen_srai_1 (operands[0], operands[1]));
     else		 
-       msp430_expand_helper (operands, \"__mspabi_srai\", true);
+       msp430_expand_helper (operands, \"__mspabi_srai\", !optimize_size);
    DONE;
    }
 )
@@ -904,7 +907,7 @@
 	(ashiftrt:SI (match_operand:SI 1 "general_operand")
 		     (match_operand:SI 2 "general_operand")))]
   ""
-  "msp430_expand_helper (operands, \"__mspabi_sral\", true);
+  "msp430_expand_helper (operands, \"__mspabi_sral\", !optimize_size);
    DONE;"
 )
 
@@ -931,7 +934,7 @@
 	     && INTVAL (operands[2]) == 1)
       emit_insn (gen_srli_1 (operands[0], operands[1]));
     else		 
-      msp430_expand_helper (operands, \"__mspabi_srli\", true);
+      msp430_expand_helper (operands, \"__mspabi_srli\", !optimize_size);
     DONE;
   }
 )
@@ -983,7 +986,7 @@
 	(lshiftrt:SI (match_operand:SI 1 "general_operand")
 		     (match_operand:SI 2 "general_operand")))]
   ""
-  "msp430_expand_helper (operands, \"__mspabi_srll\", true);
+  "msp430_expand_helper (operands, \"__mspabi_srll\", !optimize_size);
    DONE;"
 )
 
diff --git a/gcc/testsuite/gcc.target/msp430/size-optimized-shifts.c b/gcc/testsuite/gcc.target/msp430/size-optimized-shifts.c
new file mode 100644
index 00000000000..be9509b86cc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/size-optimized-shifts.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler-not "__mspabi_sral_4" } } */
+/* { dg-final { scan-assembler-not "__mspabi_srll_4" } } */
+/* { dg-final { scan-assembler-not "__mspabi_slll_4" } } */
+/* { dg-final { scan-assembler "__mspabi_sral" } } */
+/* { dg-final { scan-assembler "__mspabi_srll" } } */
+/* { dg-final { scan-assembler "__mspabi_slll" } } */
+
+/* Ensure that SImode shifts by a constant amount do not use the const_variant
+   of the shift library code when optimizing for size.  */
+
+long a;
+long b;
+long c;
+long d;
+unsigned long e;
+unsigned long f;
+
+void
+foo (void)
+{
+  a = b >> 4;
+  c = d << 4;
+  e = f >> 4;
+}
-- 
2.17.1


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

* [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code
  2019-06-04 13:03 [PATCH][MSP430][0/4] Reduce code size when performing bit shifts Jozef Lawrynowicz
                   ` (2 preceding siblings ...)
  2019-06-04 13:14 ` [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size Jozef Lawrynowicz
@ 2019-06-04 13:17 ` Jozef Lawrynowicz
  2019-06-05 22:35   ` Jeff Law
  3 siblings, 1 reply; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-04 13:17 UTC (permalink / raw)
  To: gcc-patches; +Cc: nickc

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

This patch implements 64-bit shifts in assembly code. Previously, generic C
library code from libgcc would be used to perform the shifts, which was much
more costly in terms of code size.

I observed 700 PASS->FAIL regressions from the GCC testsuite alone when these
64-bit shifts were implemented incorrectly, hence I've assumed there is
already adequate test coverage that shifts operate correctly, and I have not
added new tests to verify their correct execution.

For the following program, the below code size reduction is observed:
  long long a;

  int
  main (void)
  {
    a = a >> 4;
    return 0;
  }

With shift patch 3:
   text    data     bss     dec     hex filename
    670      12      26     708     2c4 a.out
With new patch:
   text    data     bss     dec     hex filename
    512      12      26     550     226 a.out

Ok for trunk?

[-- Attachment #2: 0004-MSP430-Implement-64-bit-shifts-in-assembly-code.patch --]
[-- Type: text/x-patch, Size: 9193 bytes --]

From 3b34b3d005ea63b37cf6a277395a048e55d854b2 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 13 May 2019 17:55:27 +0100
Subject: [PATCH 4/4] MSP430: Implement 64-bit shifts in assembly code

gcc/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.c (msp430_expand_helper): Setup arguments which
	describe how to perform MSPABI compliant 64-bit shift.
	* config/msp430/msp430.md (ashldi3): New define_expand.
	(ashrdi3): New define_expand.
	(lshrdi3): New define_expand.

libgcc/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/slli.S (__mspabi_sllll): New library function for
	performing a logical left shift of a 64-bit value.
	(__mspabi_srall): New library function for
	performing a arithmetic right shift of a 64-bit value.
	(__mspabi_srlll): New library function for
	performing a logical right shift of a 64-bit value.

gcc/testsuite/ChangeLog

2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc.target/msp430/mspabi_sllll.c: New test.
	* gcc.target/msp430/mspabi_srall.c: New test.
	* gcc.target/msp430/mspabi_srlll.c: New test.
---
 gcc/config/msp430/msp430.c                    | 13 +++++--
 gcc/config/msp430/msp430.md                   | 36 +++++++++++++++++++
 .../gcc.target/msp430/mspabi_sllll.c          | 10 ++++++
 .../gcc.target/msp430/mspabi_srall.c          | 10 ++++++
 .../gcc.target/msp430/mspabi_srlll.c          | 10 ++++++
 libgcc/config/msp430/slli.S                   | 33 +++++++++++++++++
 libgcc/config/msp430/srai.S                   | 34 ++++++++++++++++++
 libgcc/config/msp430/srli.S                   | 35 ++++++++++++++++++
 8 files changed, 179 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/mspabi_sllll.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/mspabi_srall.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/mspabi_srlll.c

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 020e980b8cc..365e9eba747 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -3046,6 +3046,7 @@ msp430_expand_helper (rtx *operands, const char *helper_name, bool const_variant
 {
   rtx c, f;
   char *helper_const = NULL;
+  int arg1 = 12;
   int arg2 = 13;
   int arg1sz = 1;
   machine_mode arg0mode = GET_MODE (operands[0]);
@@ -3079,6 +3080,13 @@ msp430_expand_helper (rtx *operands, const char *helper_name, bool const_variant
       arg2 = 14;
       arg1sz = 2;
     }
+  else if (arg1mode == DImode)
+    {
+      /* Shift value in R8:R11, shift amount in R12.  */
+      arg1 = 8;
+      arg1sz = 4;
+      arg2 = 12;
+    }
 
   if (const_variants
       && CONST_INT_P (operands[2])
@@ -3091,7 +3099,7 @@ msp430_expand_helper (rtx *operands, const char *helper_name, bool const_variant
       snprintf (helper_const, len, "%s_%d", helper_name, (int) INTVAL (operands[2]));
     }
 
-  emit_move_insn (gen_rtx_REG (arg1mode, 12),
+  emit_move_insn (gen_rtx_REG (arg1mode, arg1),
 		  operands[1]);
   if (!helper_const)
     emit_move_insn (gen_rtx_REG (arg2mode, arg2),
@@ -3104,12 +3112,13 @@ msp430_expand_helper (rtx *operands, const char *helper_name, bool const_variant
   RTL_CONST_CALL_P (c) = 1;
 
   f = 0;
-  use_regs (&f, 12, arg1sz);
+  use_regs (&f, arg1, arg1sz);
   if (!helper_const)
     use_regs (&f, arg2, 1);
   add_function_usage_to (c, f);
 
   emit_move_insn (operands[0],
+		  /* Return value will always start in R12.  */
 		  gen_rtx_REG (arg0mode, 12));
 }
 
diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index 76296a2f317..f6d688950cb 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -822,6 +822,18 @@
    DONE;"
 )
 
+(define_expand "ashldi3"
+  [(set (match_operand:DI	     0 "nonimmediate_operand")
+	(ashift:DI (match_operand:DI 1 "general_operand")
+		   (match_operand:DI 2 "general_operand")))]
+  ""
+  {
+    /* No const_variant for 64-bit shifts.  */
+    msp430_expand_helper (operands, \"__mspabi_sllll\", false);
+    DONE;
+  }
+)
+
 ;;----------
 
 ;; signed A >> C
@@ -911,6 +923,18 @@
    DONE;"
 )
 
+(define_expand "ashrdi3"
+  [(set (match_operand:DI	     0 "nonimmediate_operand")
+	(ashift:DI (match_operand:DI 1 "general_operand")
+		   (match_operand:DI 2 "general_operand")))]
+  ""
+  {
+    /* No const_variant for 64-bit shifts.  */
+    msp430_expand_helper (operands, \"__mspabi_srall\", false);
+    DONE;
+  }
+)
+
 ;;----------
 
 ;; unsigned A >> C
@@ -990,6 +1014,18 @@
    DONE;"
 )
 
+(define_expand "lshrdi3"
+  [(set (match_operand:DI	     0 "nonimmediate_operand")
+	(ashift:DI (match_operand:DI 1 "general_operand")
+		   (match_operand:DI 2 "general_operand")))]
+  ""
+  {
+    /* No const_variant for 64-bit shifts.  */
+    msp430_expand_helper (operands, \"__mspabi_srlll\", false);
+    DONE;
+  }
+)
+
 ;;------------------------------------------------------------
 ;; Function Entry/Exit
 
diff --git a/gcc/testsuite/gcc.target/msp430/mspabi_sllll.c b/gcc/testsuite/gcc.target/msp430/mspabi_sllll.c
new file mode 100644
index 00000000000..b88a8be73ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/mspabi_sllll.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-not "ashldi3" } } */
+/* { dg-final { scan-assembler "__mspabi_sllll" } } */
+
+long long
+foo (long long a)
+{
+  return a << 4;
+}
+
diff --git a/gcc/testsuite/gcc.target/msp430/mspabi_srall.c b/gcc/testsuite/gcc.target/msp430/mspabi_srall.c
new file mode 100644
index 00000000000..a0aba3d43d7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/mspabi_srall.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-not "ashrdi3" } } */
+/* { dg-final { scan-assembler "__mspabi_srall" } } */
+
+long long
+foo (long long a)
+{
+  return a >> 4;
+}
+
diff --git a/gcc/testsuite/gcc.target/msp430/mspabi_srlll.c b/gcc/testsuite/gcc.target/msp430/mspabi_srlll.c
new file mode 100644
index 00000000000..cb9a3744b77
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/mspabi_srlll.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-not "lshrdi3" } } */
+/* { dg-final { scan-assembler "__mspabi_srlll" } } */
+
+unsigned long long
+foo (unsigned long long a)
+{
+  return a >> 4;
+}
+
diff --git a/libgcc/config/msp430/slli.S b/libgcc/config/msp430/slli.S
index 89ca35a9304..9210fe6e934 100644
--- a/libgcc/config/msp430/slli.S
+++ b/libgcc/config/msp430/slli.S
@@ -110,3 +110,36 @@ __mspabi_slll:
 	RET
 #endif
 
+/* Logical Left Shift - R8:R11 -> R12:R15
+   A 64-bit argument would normally be passed in R12:R15, but __mspabi_sllll has
+   special conventions, so the 64-bit value to shift is passed in R8:R11.
+   According to the MSPABI, the shift amount is a 64-bit value in R12:R15, but
+   we only use the low word in R12.  */
+
+	.section	.text.__mspabi_sllll
+	.global __mspabi_sllll
+__mspabi_sllll:
+	MOV R11, R15 ; Free up R11 first
+	MOV R12, R11 ; Save the shift amount in R11
+	MOV R10, R14
+	MOV R9, R13
+	MOV R8, R12
+	CMP #0,R11
+	JNZ 1f
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
+1:
+	RLA R12
+	RLC R13
+	RLC R14
+	RLC R15
+	ADD #-1,R11
+	JNZ 1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
diff --git a/libgcc/config/msp430/srai.S b/libgcc/config/msp430/srai.S
index 564f7989a8c..ed5c6a5ad7c 100644
--- a/libgcc/config/msp430/srai.S
+++ b/libgcc/config/msp430/srai.S
@@ -108,3 +108,37 @@ __mspabi_sral:
 #else
 	RET
 #endif
+
+/* Arithmetic Right Shift - R8:R11 -> R12:R15
+   A 64-bit argument would normally be passed in R12:R15, but __mspabi_srall has
+   special conventions, so the 64-bit value to shift is passed in R8:R11.
+   According to the MSPABI, the shift amount is a 64-bit value in R12:R15, but
+   we only use the low word in R12.  */
+
+	.section	.text.__mspabi_srall
+	.global __mspabi_srall
+__mspabi_srall:
+	MOV R11, R15 ; Free up R11 first
+	MOV R12, R11 ; Save the shift amount in R11
+	MOV R10, R14
+	MOV R9, R13
+	MOV R8, R12
+	CMP #0, R11
+	JNZ 1f
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
+1:
+	RRA R15
+	RRC R14
+	RRC R13
+	RRC R12
+	ADD #-1,R11
+	JNZ 1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
diff --git a/libgcc/config/msp430/srli.S b/libgcc/config/msp430/srli.S
index 4dd32ea4002..bc1b034e4b9 100644
--- a/libgcc/config/msp430/srli.S
+++ b/libgcc/config/msp430/srli.S
@@ -112,3 +112,38 @@ __mspabi_srll:
 #else
 	RET
 #endif
+
+/* Logical Right Shift - R8:R11 -> R12:R15
+   A 64-bit argument would normally be passed in R12:R15, but __mspabi_srlll has
+   special conventions, so the 64-bit value to shift is passed in R8:R11.
+   According to the MSPABI, the shift amount is a 64-bit value in R12:R15, but
+   we only use the low word in R12.  */
+
+	.section	.text.__mspabi_srlll
+	.global __mspabi_srlll
+__mspabi_srlll:
+	MOV R11, R15 ; Free up R11 first
+	MOV R12, R11 ; Save the shift amount in R11
+	MOV R10, R14
+	MOV R9, R13
+	MOV R8, R12
+	CMP #0,R11
+	JNZ 1f
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
+1:
+	CLRC
+	RRC R15
+	RRC R14
+	RRC R13
+	RRC R12
+	ADD #-1,R11
+	JNZ 1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif
-- 
2.17.1


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

* Re: [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections
  2019-06-04 13:07 ` [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections Jozef Lawrynowicz
@ 2019-06-05 22:20   ` Jeff Law
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2019-06-05 22:20 UTC (permalink / raw)
  To: Jozef Lawrynowicz, gcc-patches; +Cc: nickc

On 6/4/19 7:07 AM, Jozef Lawrynowicz wrote:
> This patch reduces code size by putting each of the shift library functions from
> libgcc in their own section. This means that, for example, a 16-bit logical
> left shift does not result in code to perform a 32-bit logical left shift being
> included in the final executable, as the linker can now garbage collect unused
> sections.
> 
> For the following program, the below code size reduction is observed:
>   int a, b;
> 
>   int
>   main (void)
>   {
>     a = a << b;
>     return 0;
>   }
> 
> Current trunk:
>    text    data     bss     dec     hex filename
>     572      12      22     606     25e a.out
> With patch:
>    text    data     bss     dec     hex filename
>     466      12      22     500     1f4 a.out
> 
> Ok for trunk?
> 
> 
> 0001-MSP430-Put-the-library-functions-for-bitwise-shifts-.patch
> 
> From 8017a4b453ae1b07bbeb75f7f7613a5bc5605159 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 13 May 2019 17:42:08 +0100
> Subject: [PATCH 1/4] MSP430: Put the library functions for bitwise shifts in
>  their own sections
> 
> libgcc/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/slli.S (__mspabi_slli_n): Put function in its own
> 	section.
> 	(__mspabi_slli): Likewise.
> 	(__mspabi_slll_n): Likewise.
> 	(__mspabi_slll): Likewise.
> 	* config/msp430/srai.S (__mspabi_srai_n): Likewise.
> 	(__mspabi_srai): Likewise.
> 	(__mspabi_sral_n): Likewise.
> 	(__mspabi_sral): Likewise.
> 	* config/msp430/srli.S (__mspabi_srli_n): Likewise.
> 	(__mspabi_srli): Likewise.
> 	(__mspabi_srll_n): Likewise.
> 	(__mspabi_srll): Likewise.
OK.
jeff

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

* Re: [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory
  2019-06-04 13:11 ` [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory Jozef Lawrynowicz
@ 2019-06-05 22:21   ` Jeff Law
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2019-06-05 22:21 UTC (permalink / raw)
  To: Jozef Lawrynowicz, gcc-patches; +Cc: nickc

On 6/4/19 7:11 AM, Jozef Lawrynowicz wrote:
> This patch reduces code size by enabling the emulation of some 16-bit shift
> instructions with the native rotate instructions, when the source operand is in
> memory. This is achieved by forcing the source operand into a register.
> 
> For the following program, the below code size reduction is observed:
>   int a;
> 
>   int
>   main (void)
>   {
>     a = a << 4;
>     return 0;
>   }
> 
> With shift patch 1:
>    text    data     bss     dec     hex filename
>     484      12      20     516     204 a.out
> With new patch:
>    text    data     bss     dec     hex filename
>     452      12      20     484     1e4 a.out
> 
> Ok for trunk?
> 
> 
> 0002-MSP430-Force-the-src-operand-of-a-HImode-shift-into-.patch
> 
> From e609f63d49227ce385316896dde6a476f5f27db7 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 13 May 2019 17:48:00 +0100
> Subject: [PATCH 2/4] MSP430: Force the src operand of a HImode shift into a
>  register if it is in memory
> 
> gcc/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.md (ashlhi3): Force shift src operand into a
> 	register if it is in memory, so the shift can be emulated with a rotate
> 	instruction.
> 	(ashrhi3): Likewise.
> 	(lshrhi3): Likewise.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc.target/msp430/emulate-slli.c: New test.
> 	* gcc.target/msp430/emulate-srai.c: New test.
> 	* gcc.target/msp430/emulate-srli.c: New test.
OK
jeff
> ---

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

* Re: [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size
  2019-06-04 13:14 ` [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size Jozef Lawrynowicz
@ 2019-06-05 22:22   ` Jeff Law
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2019-06-05 22:22 UTC (permalink / raw)
  To: Jozef Lawrynowicz, gcc-patches; +Cc: nickc

On 6/4/19 7:14 AM, Jozef Lawrynowicz wrote:
> This patch reduces code size by disabling the performance optimized,
> "const_variant" of shift library functions when optimization for size is
> enabled.
> 
> For the following program, the below code size reduction is observed:
>   long a;
> 
>   int
>   main (void)
>   {
>     a = a >> 4;
>     return 0;
>   }
> 
> With shift patch 2:
>    text    data     bss     dec     hex filename
>     522      12      22     556     22c a.out
> New patch:
>    text    data     bss     dec     hex filename
>     474      12      22     508     1fc a.out
> 
> Ok for trunk?
> 
> 
> 0003-MSP430-Do-not-use-the-performance-optimized-variant-.patch
> 
> From 894b6809822ba3a3a1bab3750abe29e03f2a3ad6 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 13 May 2019 17:52:19 +0100
> Subject: [PATCH 3/4] MSP430: Do not use the performance optimized variant of a
>  shift by constant amount when optimizing for size
> 
> gcc/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.md (ashlhi3): Use the const_variant of shift
> 	library functions only when not optimizing for size.
> 	(ashlsi3): Likewise.
> 	(ashrhi3): Likewise.
> 	(ashrsi3): Likewise.
> 	(lshrhi3): Likewise.
> 	(lshrsi3): Likewise.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc.target/msp430/size-optimized-shifts.c: New test.
OK
jeff

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

* Re: [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code
  2019-06-04 13:17 ` [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code Jozef Lawrynowicz
@ 2019-06-05 22:35   ` Jeff Law
  2019-06-06 12:42     ` Jozef Lawrynowicz
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2019-06-05 22:35 UTC (permalink / raw)
  To: Jozef Lawrynowicz, gcc-patches; +Cc: nickc

On 6/4/19 7:17 AM, Jozef Lawrynowicz wrote:
> This patch implements 64-bit shifts in assembly code. Previously, generic C
> library code from libgcc would be used to perform the shifts, which was much
> more costly in terms of code size.
> 
> I observed 700 PASS->FAIL regressions from the GCC testsuite alone when these
> 64-bit shifts were implemented incorrectly, hence I've assumed there is
> already adequate test coverage that shifts operate correctly, and I have not
> added new tests to verify their correct execution.
> 
> For the following program, the below code size reduction is observed:
>   long long a;
> 
>   int
>   main (void)
>   {
>     a = a >> 4;
>     return 0;
>   }
> 
> With shift patch 3:
>    text    data     bss     dec     hex filename
>     670      12      26     708     2c4 a.out
> With new patch:
>    text    data     bss     dec     hex filename
>     512      12      26     550     226 a.out
> 
> Ok for trunk?
> 
> 
> 0004-MSP430-Implement-64-bit-shifts-in-assembly-code.patch
> 
> From 3b34b3d005ea63b37cf6a277395a048e55d854b2 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 13 May 2019 17:55:27 +0100
> Subject: [PATCH 4/4] MSP430: Implement 64-bit shifts in assembly code
> 
> gcc/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.c (msp430_expand_helper): Setup arguments which
> 	describe how to perform MSPABI compliant 64-bit shift.
> 	* config/msp430/msp430.md (ashldi3): New define_expand.
> 	(ashrdi3): New define_expand.
> 	(lshrdi3): New define_expand.
> 
> libgcc/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/slli.S (__mspabi_sllll): New library function for
> 	performing a logical left shift of a 64-bit value.
> 	(__mspabi_srall): New library function for
> 	performing a arithmetic right shift of a 64-bit value.
> 	(__mspabi_srlll): New library function for
> 	performing a logical right shift of a 64-bit value.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* gcc.target/msp430/mspabi_sllll.c: New test.
> 	* gcc.target/msp430/mspabi_srall.c: New test.
> 	* gcc.target/msp430/mspabi_srlll.c: New test.
Going to assume your assembly routines are correct :-)

OK
jeff

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

* Re: [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code
  2019-06-05 22:35   ` Jeff Law
@ 2019-06-06 12:42     ` Jozef Lawrynowicz
  2019-06-06 17:33       ` Jeff Law
  0 siblings, 1 reply; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-06 12:42 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, nickc

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

On Wed, 5 Jun 2019 16:35:14 -0600
Jeff Law <law@redhat.com> wrote:

> On 6/4/19 7:17 AM, Jozef Lawrynowicz wrote:
> > libgcc/ChangeLog
> > 
> > 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> > 
> > 	* config/msp430/slli.S (__mspabi_sllll): New library function for
> > 	performing a logical left shift of a 64-bit value.
> > 	(__mspabi_srall): New library function for
> > 	performing a arithmetic right shift of a 64-bit value.
> > 	(__mspabi_srlll): New library function for
> > 	performing a logical right shift of a 64-bit value.
> > 
> Going to assume your assembly routines are correct :-)
> 
> OK
> jeff

I assume I implemented them correctly based on the clean regtest of
GCC/G++ testsuites. But in case there might be a gap in the coverage
somewhere, how about the attached new torture test to explicitly check 64-bit
shifts work as expected?
Passes for x86_64-linux-gnu and msp430-elf.

Thanks for the reviews.
Jozef

[-- Attachment #2: gcc-shiftdi-2.diff --]
[-- Type: text/x-patch, Size: 3520 bytes --]

diff --git a/gcc/testsuite/gcc.c-torture/execute/shiftdi-2.c b/gcc/testsuite/gcc.c-torture/execute/shiftdi-2.c
new file mode 100644
index 00000000000..63d1fe90830
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/shiftdi-2.c
@@ -0,0 +1,23 @@
+__INT64_TYPE__ a = 568513516876543756;
+__INT64_TYPE__ b = -754324895235774564;
+__UINT64_TYPE__ c = 156789543257562457;
+
+__INT64_TYPE__ expected_a[64] = {568513516876543756, 1137027033753087512, 2274054067506175024, 4548108135012350048, 9096216270024700096, -254311533660151424, -508623067320302848, -1017246134640605696, -2034492269281211392, -4068984538562422784, -8137969077124845568, 2170805919459860480, 4341611838919720960, 8683223677839441920, -1080296718030667776, -2160593436061335552, -4321186872122671104, -8642373744245342208, 1161996585218867200, 2323993170437734400, 4647986340875468800, -9150771391958614016, 145201289792323584, 290402579584647168, 580805159169294336, 1161610318338588672, 2323220636677177344, 4646441273354354688, -9153861527000842240, 139021019707867136, 278042039415734272, 556084078831468544, 1112168157662937088, 2224336315325874176, 4448672630651748352, 8897345261303496704, -652053551102558208, -1304107102205116416, -2608214204410232832, -5216428408820465664, 8013887256068620288, -2418969561572311040, -4837939123144622080, 8770865827420307456, -905012418868936704, -1810024837737873408, -3620049675475746816, -7240099350951493632, 3966545371806564352, 7933090743613128704, -2580562586483294208, -5161125172966588416, 8124493727776374784, -2197756618156802048, -4395513236313604096, -8791026472627208192, 864691128455135232, 1729382256910270464, 3458764513820540928, 6917529027641081856, -4611686018427387904, -9223372036854775808ULL, 0, 0};
+__INT64_TYPE__ expected_b[64] = {-754324895235774564, -377162447617887282, -188581223808943641, -94290611904471821, -47145305952235911, -23572652976117956, -11786326488058978, -5893163244029489, -2946581622014745, -1473290811007373, -736645405503687, -368322702751844, -184161351375922, -92080675687961, -46040337843981, -23020168921991, -11510084460996, -5755042230498, -2877521115249, -1438760557625, -719380278813, -359690139407, -179845069704, -89922534852, -44961267426, -22480633713, -11240316857, -5620158429, -2810079215, -1405039608, -702519804, -351259902, -175629951, -87814976, -43907488, -21953744, -10976872, -5488436, -2744218, -1372109, -686055, -343028, -171514, -85757, -42879, -21440, -10720, -5360, -2680, -1340, -670, -335, -168, -84, -42, -21, -11, -6, -3, -2, -1, -1, -1, -1};
+__UINT64_TYPE__ expected_c[64] = {156789543257562457, 78394771628781228, 39197385814390614, 19598692907195307, 9799346453597653, 4899673226798826, 2449836613399413, 1224918306699706, 612459153349853, 306229576674926, 153114788337463, 76557394168731, 38278697084365, 19139348542182, 9569674271091, 4784837135545, 2392418567772, 1196209283886, 598104641943, 299052320971, 149526160485, 74763080242, 37381540121, 18690770060, 9345385030, 4672692515, 2336346257, 1168173128, 584086564, 292043282, 146021641, 73010820, 36505410, 18252705, 9126352, 4563176, 2281588, 1140794, 570397, 285198, 142599, 71299, 35649, 17824, 8912, 4456, 2228, 1114, 557, 278, 139, 69, 34, 17, 8, 4, 2, 1, 0, 0, 0, 0, 0, 0};
+
+int
+main (void)
+{
+  int i;
+  for (i = 0; i < 64; i++)
+  {
+    if ((a << i) != expected_a[i])
+      __builtin_abort ();
+    else if ((b >> i) != expected_b[i])
+      __builtin_abort ();
+    else if ((c >> i) != expected_c[i])
+      __builtin_abort ();
+  }
+  return 0;
+}

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

* Re: [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code
  2019-06-06 12:42     ` Jozef Lawrynowicz
@ 2019-06-06 17:33       ` Jeff Law
  2019-06-16 21:42         ` Jozef Lawrynowicz
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2019-06-06 17:33 UTC (permalink / raw)
  To: Jozef Lawrynowicz; +Cc: gcc-patches, nickc

On 6/6/19 6:42 AM, Jozef Lawrynowicz wrote:
> On Wed, 5 Jun 2019 16:35:14 -0600
> Jeff Law <law@redhat.com> wrote:
> 
>> On 6/4/19 7:17 AM, Jozef Lawrynowicz wrote:
>>> libgcc/ChangeLog
>>>
>>> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
>>>
>>> 	* config/msp430/slli.S (__mspabi_sllll): New library function for
>>> 	performing a logical left shift of a 64-bit value.
>>> 	(__mspabi_srall): New library function for
>>> 	performing a arithmetic right shift of a 64-bit value.
>>> 	(__mspabi_srlll): New library function for
>>> 	performing a logical right shift of a 64-bit value.
>>>
>> Going to assume your assembly routines are correct :-)
>>
>> OK
>> jeff
> I assume I implemented them correctly based on the clean regtest of
> GCC/G++ testsuites. But in case there might be a gap in the coverage
> somewhere, how about the attached new torture test to explicitly check 64-bit
> shifts work as expected?
> Passes for x86_64-linux-gnu and msp430-elf.
I suspect this needs to be conditional on 64bit integer support (check
either at runtime with sizeof or via dejagnu effective target stuff).
With that fixed this is OK.

jeff

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

* Re: [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code
  2019-06-06 17:33       ` Jeff Law
@ 2019-06-16 21:42         ` Jozef Lawrynowicz
  0 siblings, 0 replies; 12+ messages in thread
From: Jozef Lawrynowicz @ 2019-06-16 21:42 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, nickc

On Thu, 6 Jun 2019 11:32:49 -0600
Jeff Law <law@redhat.com> wrote:

> On 6/6/19 6:42 AM, Jozef Lawrynowicz wrote:
> > On Wed, 5 Jun 2019 16:35:14 -0600
> > Jeff Law <law@redhat.com> wrote:
> >   
> >> On 6/4/19 7:17 AM, Jozef Lawrynowicz wrote:  
> >>> libgcc/ChangeLog
> >>>
> >>> 2019-06-04  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> >>>
> >>> 	* config/msp430/slli.S (__mspabi_sllll): New library function for
> >>> 	performing a logical left shift of a 64-bit value.
> >>> 	(__mspabi_srall): New library function for
> >>> 	performing a arithmetic right shift of a 64-bit value.
> >>> 	(__mspabi_srlll): New library function for
> >>> 	performing a logical right shift of a 64-bit value.
> >>>  
> >> Going to assume your assembly routines are correct :-)
> >>
> >> OK
> >> jeff  
> > I assume I implemented them correctly based on the clean regtest of
> > GCC/G++ testsuites. But in case there might be a gap in the coverage
> > somewhere, how about the attached new torture test to explicitly check 64-bit
> > shifts work as expected?
> > Passes for x86_64-linux-gnu and msp430-elf.  
> I suspect this needs to be conditional on 64bit integer support (check
> either at runtime with sizeof or via dejagnu effective target stuff).
> With that fixed this is OK.
> 
> jeff

Since it seems that the use of long long when a 64-bit type is required is much
more prevalent in the testsuite that __INT64_TYPE__ (which may not be supported
on all systems), I changed the types to use long long, and made the test
require the longlong64 effective target keyword. Committed.

Thanks,
Jozef

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

end of thread, other threads:[~2019-06-16 21:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-04 13:03 [PATCH][MSP430][0/4] Reduce code size when performing bit shifts Jozef Lawrynowicz
2019-06-04 13:07 ` [PATCH][MSP430][1/4] Put libgcc shift functions in their own sections Jozef Lawrynowicz
2019-06-05 22:20   ` Jeff Law
2019-06-04 13:11 ` [PATCH][MSP430][2/4] Emulate 16-bit shifts with rotate insn when src operand is originally in memory Jozef Lawrynowicz
2019-06-05 22:21   ` Jeff Law
2019-06-04 13:14 ` [PATCH][MSP430][3/4] Disable performance optimal library code shifts when optimizing for size Jozef Lawrynowicz
2019-06-05 22:22   ` Jeff Law
2019-06-04 13:17 ` [PATCH][MSP430][4/4] Implement 64-bit shifts in assembly code Jozef Lawrynowicz
2019-06-05 22:35   ` Jeff Law
2019-06-06 12:42     ` Jozef Lawrynowicz
2019-06-06 17:33       ` Jeff Law
2019-06-16 21:42         ` Jozef Lawrynowicz

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).