[PING,MSP430] PR79242: Implement Complex Partial Integers

Message ID CAOe4MZbvbd1smaBhfWxdS5E2X9Cs_ugHbNbkKhpP625z7Jbbkw@mail.gmail.com
State New
Headers show
Series
  • [PING,MSP430] PR79242: Implement Complex Partial Integers
Related show

Commit Message

Jozef Lawrynowicz Feb. 8, 2018, 4:54 p.m.
ping x1

Complex Partial Integers are unimplemented, resulting in an ICE when
attempting to use them. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242
This results in GCC7/8 for msp430-elf failing to build.

typedef _Complex __int20 C;

C
foo (C x, C y)
{
  return x + y;
}

(Thanks Jakub - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242#c2)

../../gcc/testsuite/gcc.target/msp430/pr79242.c: In function 'foo':
../../gcc/testsuite/gcc.target/msp430/pr79242.c:8:1: internal compiler
error: in make_decl_rtl, at varasm.c:1304
 foo (C x, C y)
 ^~~
0xc07b29 make_decl_rtl(tree_node*)
        ../../gcc/varasm.c:1303
0x67523c set_parm_rtl(tree_node*, rtx_def*)
        ../../gcc/cfgexpand.c:1274
0x79ffb9 expand_function_start(tree_node*)
        ../../gcc/function.c:5166
0x6800e1 execute
        ../../gcc/cfgexpand.c:6250

The attached patch defines a new complex mode for PARTIAL_INT.
You may notice that genmodes.c:complex_class returns MODE_COMPLEX_INT for
MODE_PARTIAL_INT rather than MODE_COMPLEX_PARTIAL_INT. I reviewed the uses of
MODE_COMPLEX_INT and it doesn't looked like a Complex Partial Int requires any
different behaviour to MODE_COMPLEX_INT.

msp430_hard_regno_nregs now returns 2 for CPSImode, but I feel like this may be
better handled in the front-end. PSImode is already defined to only use 1
register, so for a CPSI shouldn't the front-end should be able to work out that
double the amount of registers are required? Thoughts?
Without the definition for CPSI in msp430_hard_regno_nregs,
rtlanal.c:subreg_get_info thinks that a CPSI requires 4 registers of size 2,
instead of 2 registers of size 4.

Successfully bootstrapped and tested for c,c++,fortran,lto,objc on
x86_64-pc-linux-gnu with no regressions on gcc-7-branch.

With this patch gcc-7-branch now builds for msp430-elf. A further bug prevents
trunk from building for msp430-elf.

If the attached patch is acceptable, I would appreciate if someone would commit
it for me (to trunk and gcc-7-branch), as I do not have write access.

Comments

Jeff Law Feb. 14, 2018, 7:25 a.m. | #1
On 02/08/2018 09:54 AM, Jozef Lawrynowicz wrote:
> ping x1
> 
> Complex Partial Integers are unimplemented, resulting in an ICE when
> attempting to use them. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242
> This results in GCC7/8 for msp430-elf failing to build.
> 
> typedef _Complex __int20 C;
> 
> C
> foo (C x, C y)
> {
>   return x + y;
> }
> 
> (Thanks Jakub - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242#c2)
> 
> ../../gcc/testsuite/gcc.target/msp430/pr79242.c: In function 'foo':
> ../../gcc/testsuite/gcc.target/msp430/pr79242.c:8:1: internal compiler
> error: in make_decl_rtl, at varasm.c:1304
>  foo (C x, C y)
>  ^~~
> 0xc07b29 make_decl_rtl(tree_node*)
>         ../../gcc/varasm.c:1303
> 0x67523c set_parm_rtl(tree_node*, rtx_def*)
>         ../../gcc/cfgexpand.c:1274
> 0x79ffb9 expand_function_start(tree_node*)
>         ../../gcc/function.c:5166
> 0x6800e1 execute
>         ../../gcc/cfgexpand.c:6250
> 
> The attached patch defines a new complex mode for PARTIAL_INT.
> You may notice that genmodes.c:complex_class returns MODE_COMPLEX_INT for
> MODE_PARTIAL_INT rather than MODE_COMPLEX_PARTIAL_INT. I reviewed the uses of
> MODE_COMPLEX_INT and it doesn't looked like a Complex Partial Int requires any
> different behaviour to MODE_COMPLEX_INT.
> 
> msp430_hard_regno_nregs now returns 2 for CPSImode, but I feel like this may be
> better handled in the front-end. PSImode is already defined to only use 1
> register, so for a CPSI shouldn't the front-end should be able to work out that
> double the amount of registers are required? Thoughts?
> Without the definition for CPSI in msp430_hard_regno_nregs,
> rtlanal.c:subreg_get_info thinks that a CPSI requires 4 registers of size 2,
> instead of 2 registers of size 4.
> 
> Successfully bootstrapped and tested for c,c++,fortran,lto,objc on
> x86_64-pc-linux-gnu with no regressions on gcc-7-branch.
> 
> With this patch gcc-7-branch now builds for msp430-elf. A further bug prevents
> trunk from building for msp430-elf.
> 
> If the attached patch is acceptable, I would appreciate if someone would commit
> it for me (to trunk and gcc-7-branch), as I do not have write access.
> 
> 
> 0001-Add-support-for-Complex-Partial-Integers-CPSImode.patch
> 
> 
> From 31d8554ebb6afeb2d8f235cf3d3c262236aa5e32 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozefl.gcc@gmail.com>
> Date: Fri, 12 Jan 2018 13:23:40 +0000
> Subject: [PATCH] Add support for Complex Partial Integers - CPSImode
> 
> 2018-01-XX  Jozef Lawrynowicz <jozefl.gcc@gmail.com>
> 
> gcc/
>   PR target/79242
>   * machmode.def: Define a complex mode for PARTIAL_INT.
>   * genmodes.c (complex_class): Return MODE_COMPLEX_INT for
>     MODE_PARTIAL_INT.
>   * doc/rtl.texi: Document CSPImode.
>   * config/msp430/msp430.c (msp430_hard_regno_nregs): Add CPSImode
>     handling.
>     (msp430_hard_regno_nregs_with_padding): Likewise.
> 
> gcc/testsuite/
>   PR target/79242
>   * gcc.target/msp430/pr79242.c: New test.
Thanks.  Installed.

jeff
Jozef Lawrynowicz Feb. 14, 2018, 12:48 p.m. | #2
On 14/02/18 07:25, Jeff Law wrote:
> On 02/08/2018 09:54 AM, Jozef Lawrynowicz wrote:
>> ping x1
>>
>> Complex Partial Integers are unimplemented, resulting in an ICE when
>> attempting to use them. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242
>> This results in GCC7/8 for msp430-elf failing to build.
>>
>> typedef _Complex __int20 C;
>>
>> C
>> foo (C x, C y)
>> {
>>    return x + y;
>> }
>>
>> (Thanks Jakub - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242#c2)
>>
>> ../../gcc/testsuite/gcc.target/msp430/pr79242.c: In function 'foo':
>> ../../gcc/testsuite/gcc.target/msp430/pr79242.c:8:1: internal compiler
>> error: in make_decl_rtl, at varasm.c:1304
>>   foo (C x, C y)
>>   ^~~
>> 0xc07b29 make_decl_rtl(tree_node*)
>>          ../../gcc/varasm.c:1303
>> 0x67523c set_parm_rtl(tree_node*, rtx_def*)
>>          ../../gcc/cfgexpand.c:1274
>> 0x79ffb9 expand_function_start(tree_node*)
>>          ../../gcc/function.c:5166
>> 0x6800e1 execute
>>          ../../gcc/cfgexpand.c:6250
>>
>> The attached patch defines a new complex mode for PARTIAL_INT.
>> You may notice that genmodes.c:complex_class returns MODE_COMPLEX_INT for
>> MODE_PARTIAL_INT rather than MODE_COMPLEX_PARTIAL_INT. I reviewed the uses of
>> MODE_COMPLEX_INT and it doesn't looked like a Complex Partial Int requires any
>> different behaviour to MODE_COMPLEX_INT.
>>
>> msp430_hard_regno_nregs now returns 2 for CPSImode, but I feel like this may be
>> better handled in the front-end. PSImode is already defined to only use 1
>> register, so for a CPSI shouldn't the front-end should be able to work out that
>> double the amount of registers are required? Thoughts?
>> Without the definition for CPSI in msp430_hard_regno_nregs,
>> rtlanal.c:subreg_get_info thinks that a CPSI requires 4 registers of size 2,
>> instead of 2 registers of size 4.
>>
>> Successfully bootstrapped and tested for c,c++,fortran,lto,objc on
>> x86_64-pc-linux-gnu with no regressions on gcc-7-branch.
>>
>> With this patch gcc-7-branch now builds for msp430-elf. A further bug prevents
>> trunk from building for msp430-elf.
>>
>> If the attached patch is acceptable, I would appreciate if someone would commit
>> it for me (to trunk and gcc-7-branch), as I do not have write access.
>>
>>
>> 0001-Add-support-for-Complex-Partial-Integers-CPSImode.patch
>>
>>
>>  From 31d8554ebb6afeb2d8f235cf3d3c262236aa5e32 Mon Sep 17 00:00:00 2001
>> From: Jozef Lawrynowicz <jozefl.gcc@gmail.com>
>> Date: Fri, 12 Jan 2018 13:23:40 +0000
>> Subject: [PATCH] Add support for Complex Partial Integers - CPSImode
>>
>> 2018-01-XX  Jozef Lawrynowicz <jozefl.gcc@gmail.com>
>>
>> gcc/
>>    PR target/79242
>>    * machmode.def: Define a complex mode for PARTIAL_INT.
>>    * genmodes.c (complex_class): Return MODE_COMPLEX_INT for
>>      MODE_PARTIAL_INT.
>>    * doc/rtl.texi: Document CSPImode.
>>    * config/msp430/msp430.c (msp430_hard_regno_nregs): Add CPSImode
>>      handling.
>>      (msp430_hard_regno_nregs_with_padding): Likewise.
>>
>> gcc/testsuite/
>>    PR target/79242
>>    * gcc.target/msp430/pr79242.c: New test.
> Thanks.  Installed.
>
> jeff

Thanks! Do you mind applying to gcc-7-branch as well?

Jozef

Patch

From 31d8554ebb6afeb2d8f235cf3d3c262236aa5e32 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozefl.gcc@gmail.com>
Date: Fri, 12 Jan 2018 13:23:40 +0000
Subject: [PATCH] Add support for Complex Partial Integers - CPSImode

2018-01-XX  Jozef Lawrynowicz <jozefl.gcc@gmail.com>

gcc/
  PR target/79242
  * machmode.def: Define a complex mode for PARTIAL_INT.
  * genmodes.c (complex_class): Return MODE_COMPLEX_INT for
    MODE_PARTIAL_INT.
  * doc/rtl.texi: Document CSPImode.
  * config/msp430/msp430.c (msp430_hard_regno_nregs): Add CPSImode
    handling.
    (msp430_hard_regno_nregs_with_padding): Likewise.

gcc/testsuite/
  PR target/79242
  * gcc.target/msp430/pr79242.c: New test.
---
 gcc/config/msp430/msp430.c                |  4 ++++
 gcc/doc/rtl.texi                          |  5 +++--
 gcc/genmodes.c                            |  1 +
 gcc/machmode.def                          |  1 +
 gcc/testsuite/gcc.target/msp430/pr79242.c | 11 +++++++++++
 5 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/pr79242.c

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 710a97b..c1f0d5b 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -905,6 +905,8 @@  msp430_hard_regno_nregs (int regno ATTRIBUTE_UNUSED,
 {
   if (mode == PSImode && msp430x)
     return 1;
+  if (mode == CPSImode && msp430x)
+    return 2;
   return ((GET_MODE_SIZE (mode) + UNITS_PER_WORD - 1)
 	  / UNITS_PER_WORD);
 }
@@ -927,6 +929,8 @@  msp430_hard_regno_nregs_with_padding (int regno ATTRIBUTE_UNUSED,
 {
   if (mode == PSImode)
     return 2;
+  if (mode == CPSImode)
+    return 4;
   return msp430_hard_regno_nregs (regno, mode);
 }
 
diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index b02e5a1..ebe2a63 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -1291,10 +1291,11 @@  point values.  The floating point values are in @code{QFmode},
 @findex CDImode
 @findex CTImode
 @findex COImode
-@item CQImode, CHImode, CSImode, CDImode, CTImode, COImode
+@findex CPSImode
+@item CQImode, CHImode, CSImode, CDImode, CTImode, COImode, CPSImode
 These modes stand for a complex number represented as a pair of integer
 values.  The integer values are in @code{QImode}, @code{HImode},
-@code{SImode}, @code{DImode}, @code{TImode}, and @code{OImode},
+@code{SImode}, @code{DImode}, @code{TImode}, @code{OImode}, and @code{PSImode},
 respectively.
 
 @findex BND32mode
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index e56c08b..2af6556 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -116,6 +116,7 @@  complex_class (enum mode_class c)
   switch (c)
     {
     case MODE_INT: return MODE_COMPLEX_INT;
+    case MODE_PARTIAL_INT: return MODE_COMPLEX_INT;
     case MODE_FLOAT: return MODE_COMPLEX_FLOAT;
     default:
       error ("no complex class for class %s", mode_class_names[c]);
diff --git a/gcc/machmode.def b/gcc/machmode.def
index afe6851..6c84488 100644
--- a/gcc/machmode.def
+++ b/gcc/machmode.def
@@ -243,6 +243,7 @@  UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
 
 /* Complex modes.  */
 COMPLEX_MODES (INT);
+COMPLEX_MODES (PARTIAL_INT);
 COMPLEX_MODES (FLOAT);
 
 /* Decimal floating point modes.  */
diff --git a/gcc/testsuite/gcc.target/msp430/pr79242.c b/gcc/testsuite/gcc.target/msp430/pr79242.c
new file mode 100644
index 0000000..d7ff8d3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr79242.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { "*-*-*" } { "-mcpu=msp430" "-msmall" } { "" } } */
+/* { dg-options "-mcpu=msp430x" } */
+
+typedef _Complex __int20 C;
+
+C
+foo (C x, C y)
+{
+  return x + y;
+}
-- 
2.7.4