public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
@ 2017-05-19 13:39 Jozef Lawrynowicz
  2017-07-26 16:58 ` Jeff Law
  0 siblings, 1 reply; 9+ messages in thread
From: Jozef Lawrynowicz @ 2017-05-19 13:39 UTC (permalink / raw)
  To: gcc-patches

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

Original post: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01030.html

The attached patch fixes an issue for the msp430 target where the TYPE_SIZE of
the __int20 type was set using the precision (20 bits) instead of the in-memory
size (32 bits) of the type. This bug caused an ICE as reported in PR78849:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78849

The patch passed bootstrap and regression testing with no regressions
on recent trunk (r247020) for x86_64-pc-linux-gnu.

The patch passed regression testing with "-mcpu=msp430x/-mlarge" for
msp430-elf on the gcc-6-branch (r247086). Trunk doesn't build with C++
support for msp430-elf which is why gcc-6-branch was used.

If the patch is acceptable I would appreciate if someone could commit it for me
as I don't have write access.

[-- Attachment #2: 0001-Use-GET_MODE_BITSIZE-when-setting-TYPE_SIZE.patch --]
[-- Type: application/octet-stream, Size: 4402 bytes --]

From 81ee936dcdde4f4a7d4036479dbbff77da1e72bb Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@somniumtech.com>
Date: Wed, 12 Apr 2017 14:45:45 +0000
Subject: [PATCH] Use GET_MODE_BITSIZE when setting TYPE_SIZE

2017-05-XX	Jozef Lawrynowicz	<jozef.l@somniumtech.com>

	gcc/
	PR target/78849
	* stor-layout.c (initialize_sizetypes): Use GET_MODE_BITSIZE when setting TYPE_SIZE.
	* tree.c (build_common_tree_nodes): Likewise.

	gcc/testsuite
	PR target/78849
	* gcc.target/msp430/pr78849.c: New test.
	* gcc.target/msp430/msp430.exp: Remove -pedantic-errors option from DEFAULT_CFLAGS.
---
 gcc/stor-layout.c                          |  5 +++--
 gcc/testsuite/gcc.target/msp430/msp430.exp | 13 +++++++++----
 gcc/testsuite/gcc.target/msp430/pr78849.c  | 21 +++++++++++++++++++++
 gcc/tree.c                                 |  6 ++++--
 4 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/pr78849.c

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 10e9a32..1dbaba0 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -2602,13 +2602,14 @@ initialize_sizetypes (void)
   /* Now layout both types manually.  */
   SET_TYPE_MODE (sizetype, smallest_mode_for_size (precision, MODE_INT));
   SET_TYPE_ALIGN (sizetype, GET_MODE_ALIGNMENT (TYPE_MODE (sizetype)));
-  TYPE_SIZE (sizetype) = bitsize_int (precision);
+  TYPE_SIZE (sizetype) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (sizetype)));
   TYPE_SIZE_UNIT (sizetype) = size_int (GET_MODE_SIZE (TYPE_MODE (sizetype)));
   set_min_and_max_values_for_integral_type (sizetype, precision, UNSIGNED);
 
   SET_TYPE_MODE (bitsizetype, smallest_mode_for_size (bprecision, MODE_INT));
   SET_TYPE_ALIGN (bitsizetype, GET_MODE_ALIGNMENT (TYPE_MODE (bitsizetype)));
-  TYPE_SIZE (bitsizetype) = bitsize_int (bprecision);
+  TYPE_SIZE (bitsizetype)
+    = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (bitsizetype)));
   TYPE_SIZE_UNIT (bitsizetype)
     = size_int (GET_MODE_SIZE (TYPE_MODE (bitsizetype)));
   set_min_and_max_values_for_integral_type (bitsizetype, bprecision, UNSIGNED);
diff --git a/gcc/testsuite/gcc.target/msp430/msp430.exp b/gcc/testsuite/gcc.target/msp430/msp430.exp
index e54d531..ce5c3dc 100644
--- a/gcc/testsuite/gcc.target/msp430/msp430.exp
+++ b/gcc/testsuite/gcc.target/msp430/msp430.exp
@@ -24,10 +24,15 @@ if { ![istarget msp430-*-*] } then {
 # Load support procs.
 load_lib gcc-dg.exp
 
-# If a testcase doesn't have special options, use these.
+# The '-pedantic-errors' option in the global variable DEFAULT_CFLAGS that is
+# set by other drivers causes an error when the __int20 type is used, so remove
+# this option from DEFAULT_CFLAGS for the msp430 tests.
 global DEFAULT_CFLAGS
-if ![info exists DEFAULT_CFLAGS] then {
-    set DEFAULT_CFLAGS ""
+if [info exists DEFAULT_CFLAGS] then {
+    set MSP430_DEFAULT_CFLAGS \
+      [ string map { "-pedantic-errors" "" } $DEFAULT_CFLAGS ]
+} else {
+    set MSP430_DEFAULT_CFLAGS ""
 }
 
 # Initialize `dg'.
@@ -35,7 +40,7 @@ dg-init
 
 # Main loop.
 dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
-	"" $DEFAULT_CFLAGS
+	"" $MSP430_DEFAULT_CFLAGS
 
 # All done.
 dg-finish
diff --git a/gcc/testsuite/gcc.target/msp430/pr78849.c b/gcc/testsuite/gcc.target/msp430/pr78849.c
new file mode 100644
index 0000000..97792e9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78849.c
@@ -0,0 +1,21 @@
+/* { dg-do link } */
+
+struct s_1
+{
+  __int20 array[2];
+  char elem;
+};
+
+struct s_1 instance =
+{
+    {
+      0,
+      1,
+    },
+    2
+};
+
+int main (void)
+{
+  return 0;
+}
diff --git a/gcc/tree.c b/gcc/tree.c
index 72dbba4..12f2635 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10285,8 +10285,10 @@ build_common_tree_nodes (bool signed_char)
     {
       int_n_trees[i].signed_type = make_signed_type (int_n_data[i].bitsize);
       int_n_trees[i].unsigned_type = make_unsigned_type (int_n_data[i].bitsize);
-      TYPE_SIZE (int_n_trees[i].signed_type) = bitsize_int (int_n_data[i].bitsize);
-      TYPE_SIZE (int_n_trees[i].unsigned_type) = bitsize_int (int_n_data[i].bitsize);
+      TYPE_SIZE (int_n_trees[i].signed_type)
+	= bitsize_int (GET_MODE_BITSIZE (int_n_data[i].m));
+      TYPE_SIZE (int_n_trees[i].unsigned_type)
+	= bitsize_int (GET_MODE_BITSIZE (int_n_data[i].m));
 
       if (int_n_data[i].bitsize > LONG_LONG_TYPE_SIZE
 	  && int_n_enabled_p[i])
-- 
1.8.3.1


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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-05-19 13:39 ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array Jozef Lawrynowicz
@ 2017-07-26 16:58 ` Jeff Law
  2017-07-31 23:08   ` Joseph Myers
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Law @ 2017-07-26 16:58 UTC (permalink / raw)
  To: Jozef Lawrynowicz, gcc-patches

On 05/19/2017 07:35 AM, Jozef Lawrynowicz wrote:
> Original post: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01030.html
> 
> The attached patch fixes an issue for the msp430 target where the TYPE_SIZE of
> the __int20 type was set using the precision (20 bits) instead of the in-memory
> size (32 bits) of the type. This bug caused an ICE as reported in PR78849:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78849
> 
> The patch passed bootstrap and regression testing with no regressions
> on recent trunk (r247020) for x86_64-pc-linux-gnu.
> 
> The patch passed regression testing with "-mcpu=msp430x/-mlarge" for
> msp430-elf on the gcc-6-branch (r247086). Trunk doesn't build with C++
> support for msp430-elf which is why gcc-6-branch was used.
> 
> If the patch is acceptable I would appreciate if someone could commit it for me
> as I don't have write access.
> 
> 
> 0001-Use-GET_MODE_BITSIZE-when-setting-TYPE_SIZE.patch
> 
> 
> From 81ee936dcdde4f4a7d4036479dbbff77da1e72bb Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@somniumtech.com>
> Date: Wed, 12 Apr 2017 14:45:45 +0000
> Subject: [PATCH] Use GET_MODE_BITSIZE when setting TYPE_SIZE
> 
> 2017-05-XX	Jozef Lawrynowicz	<jozef.l@somniumtech.com>
> 
> 	gcc/
> 	PR target/78849
> 	* stor-layout.c (initialize_sizetypes): Use GET_MODE_BITSIZE when setting TYPE_SIZE.
> 	* tree.c (build_common_tree_nodes): Likewise.
> 
> 	gcc/testsuite
> 	PR target/78849
> 	* gcc.target/msp430/pr78849.c: New test.
> 	* gcc.target/msp430/msp430.exp: Remove -pedantic-errors option from DEFAULT_CFLAGS.
TYPE_SIZE, according to my understanding, should be a tree for the size
of the expression in bits.

The problem is for msp430 that size varies depending on where it's used.
 ie, in a register an object might have a bitsize of 20 bits, but in
memory its size is 32 bits.

I don't think that TYPE_SIZE has any concept that the use context is
relevant to selecting the proper size.

I wonder if we would be better off keeping TYPE_SIZE as-is and instead
looking at the end use points where we might have that contextual
information.

Thoughts?
jeff

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-07-26 16:58 ` Jeff Law
@ 2017-07-31 23:08   ` Joseph Myers
  2017-08-01 16:32     ` Jozef Lawrynowicz
  2017-08-02 16:21     ` Jeff Law
  0 siblings, 2 replies; 9+ messages in thread
From: Joseph Myers @ 2017-07-31 23:08 UTC (permalink / raw)
  To: Jeff Law; +Cc: Jozef Lawrynowicz, gcc-patches

On Wed, 26 Jul 2017, Jeff Law wrote:

> TYPE_SIZE, according to my understanding, should be a tree for the size
> of the expression in bits.
> 
> The problem is for msp430 that size varies depending on where it's used.
>  ie, in a register an object might have a bitsize of 20 bits, but in
> memory its size is 32 bits.
> 
> I don't think that TYPE_SIZE has any concept that the use context is
> relevant to selecting the proper size.

TYPE_SIZE_UNIT is unambiguously the memory size, including padding; it's 
used for sizeof.  TYPE_SIZE may be less clear.  We've had issues before 
with unions with x87 long double, which has 80-bit precision in registers 
but is 12-byte or 16-byte in memory; that was wrong code (bug 71522) 
rather than an ICE, but the long double case may be useful for comparison 
of what various type properties are set to in such cases.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-07-31 23:08   ` Joseph Myers
@ 2017-08-01 16:32     ` Jozef Lawrynowicz
  2017-08-02 16:33       ` Jeff Law
  2017-08-02 16:21     ` Jeff Law
  1 sibling, 1 reply; 9+ messages in thread
From: Jozef Lawrynowicz @ 2017-08-01 16:32 UTC (permalink / raw)
  To: Joseph Myers, Jeff Law; +Cc: gcc-patches

On 01/08/2017 00:08, Joseph Myers wrote:
> On Wed, 26 Jul 2017, Jeff Law wrote:
> 
>> TYPE_SIZE, according to my understanding, should be a tree for the size
>> of the expression in bits.
>>
>> The problem is for msp430 that size varies depending on where it's used.
>>   ie, in a register an object might have a bitsize of 20 bits, but in
>> memory its size is 32 bits.
>>
>> I don't think that TYPE_SIZE has any concept that the use context is
>> relevant to selecting the proper size.
> 
> TYPE_SIZE_UNIT is unambiguously the memory size, including padding; it's
> used for sizeof.  TYPE_SIZE may be less clear.  We've had issues before
> with unions with x87 long double, which has 80-bit precision in registers
> but is 12-byte or 16-byte in memory; that was wrong code (bug 71522)
> rather than an ICE, but the long double case may be useful for comparison
> of what various type properties are set to in such cases.
> 

Thanks for the responses.

Could it be possible to use "variable_size" to help the compiler choose
which size to use for TYPE_SIZE? Although the problem I see with this
right away is that variable_size is never executed on an INTEGER_CST,
perhaps this is part of the problem?

Zooming out a bit from TYPE_SIZE, the value that ends up being incorrect
as a result of TYPE_SIZE is rli->bitpos, this value is used a lot in
stor_layout.c:place_field, perhaps I need to look deeper in there.

For this specific bug, rli->bitpos is 20*ARRAY_SIZE, if I modify it in
a GDB session to be 32*ARRAY_SIZE, the case no longer ICEs.

Any suggestions on where I should be looking given this information?
Is this fix for this likely to be in the back-end?

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-07-31 23:08   ` Joseph Myers
  2017-08-01 16:32     ` Jozef Lawrynowicz
@ 2017-08-02 16:21     ` Jeff Law
  1 sibling, 0 replies; 9+ messages in thread
From: Jeff Law @ 2017-08-02 16:21 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Jozef Lawrynowicz, gcc-patches

On 07/31/2017 05:08 PM, Joseph Myers wrote:
> On Wed, 26 Jul 2017, Jeff Law wrote:
> 
>> TYPE_SIZE, according to my understanding, should be a tree for the size
>> of the expression in bits.
>>
>> The problem is for msp430 that size varies depending on where it's used.
>>  ie, in a register an object might have a bitsize of 20 bits, but in
>> memory its size is 32 bits.
>>
>> I don't think that TYPE_SIZE has any concept that the use context is
>> relevant to selecting the proper size.
> 
> TYPE_SIZE_UNIT is unambiguously the memory size, including padding; it's 
> used for sizeof.  TYPE_SIZE may be less clear.  We've had issues before 
> with unions with x87 long double, which has 80-bit precision in registers 
> but is 12-byte or 16-byte in memory; that was wrong code (bug 71522) 
> rather than an ICE, but the long double case may be useful for comparison 
> of what various type properties are set to in such cases.
Thanks for the clarification.  I wandered around a bit before commenting
on Jozef's patch and didn't come across anything which made it
unambiguous.

And yes, x87 long doubles are a good example to compare/contrast against.

jeff

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-08-01 16:32     ` Jozef Lawrynowicz
@ 2017-08-02 16:33       ` Jeff Law
  2017-08-02 16:46         ` Joseph Myers
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Law @ 2017-08-02 16:33 UTC (permalink / raw)
  To: Jozef Lawrynowicz, Joseph Myers; +Cc: gcc-patches

On 08/01/2017 10:26 AM, Jozef Lawrynowicz wrote:
> On 01/08/2017 00:08, Joseph Myers wrote:
>> On Wed, 26 Jul 2017, Jeff Law wrote:
>>
>>> TYPE_SIZE, according to my understanding, should be a tree for the size
>>> of the expression in bits.
>>>
>>> The problem is for msp430 that size varies depending on where it's used.
>>>   ie, in a register an object might have a bitsize of 20 bits, but in
>>> memory its size is 32 bits.
>>>
>>> I don't think that TYPE_SIZE has any concept that the use context is
>>> relevant to selecting the proper size.
>>
>> TYPE_SIZE_UNIT is unambiguously the memory size, including padding; it's
>> used for sizeof.  TYPE_SIZE may be less clear.  We've had issues before
>> with unions with x87 long double, which has 80-bit precision in registers
>> but is 12-byte or 16-byte in memory; that was wrong code (bug 71522)
>> rather than an ICE, but the long double case may be useful for comparison
>> of what various type properties are set to in such cases.
>>
> 
> Thanks for the responses.
> 
> Could it be possible to use "variable_size" to help the compiler choose
> which size to use for TYPE_SIZE? Although the problem I see with this
> right away is that variable_size is never executed on an INTEGER_CST,
> perhaps this is part of the problem?
I suspect variable_size is more for things like VLAs where the size of
an array may vary at runtime.

What we're dealing here is more that the size of an object varies
depending on if it's in a register or in memory.


> Zooming out a bit from TYPE_SIZE, the value that ends up being incorrect
> as a result of TYPE_SIZE is rli->bitpos, this value is used a lot in
> stor_layout.c:place_field, perhaps I need to look deeper in there.
I think Joseph's suggestion for looking at partial float handling may be
useful, though ia64's RFmode may be more interesting as it's not a
multiple of 8 in bitsize.  IF/KF modes on the ppc port have similar
properties.

If you could declare a type that corresponds to one of those modes, then
build an array of them and follow how that code works it might give you
some hints on how to fix intXX.
jeff

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-08-02 16:33       ` Jeff Law
@ 2017-08-02 16:46         ` Joseph Myers
  2017-08-22 11:34           ` Jozef Lawrynowicz
  0 siblings, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2017-08-02 16:46 UTC (permalink / raw)
  To: Jeff Law; +Cc: Jozef Lawrynowicz, gcc-patches

On Wed, 2 Aug 2017, Jeff Law wrote:

> I think Joseph's suggestion for looking at partial float handling may be
> useful, though ia64's RFmode may be more interesting as it's not a
> multiple of 8 in bitsize.  IF/KF modes on the ppc port have similar
> properties.

The key issue those floating-point modes engage is the meaning of 
precision.  IFmode and KFmode have precision set to 106 and 113 to 
distinguish them.  But that's precision in the sense of number of mantissa 
bits; as normally understood in GCC, precision should be the number of 
significant bits, so 128 for both those modes (but having multiple 
different binary floating-point modes with the same precision would 
require changing how we deal with laying out long double, so the target 
specifies a mode for floating-point types rather than leaving it to be 
deduced from values such as LONG_DOUBLE_TYPE_SIZE).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-08-02 16:46         ` Joseph Myers
@ 2017-08-22 11:34           ` Jozef Lawrynowicz
  2017-08-24 12:40             ` Jozef Lawrynowicz
  0 siblings, 1 reply; 9+ messages in thread
From: Jozef Lawrynowicz @ 2017-08-22 11:34 UTC (permalink / raw)
  To: Joseph Myers, Jeff Law; +Cc: gcc-patches

On 02/08/2017 17:45, Joseph Myers wrote:
> On Wed, 2 Aug 2017, Jeff Law wrote:
> 
>> I think Joseph's suggestion for looking at partial float handling may be
>> useful, though ia64's RFmode may be more interesting as it's not a
>> multiple of 8 in bitsize.  IF/KF modes on the ppc port have similar
>> properties.
> 
> The key issue those floating-point modes engage is the meaning of
> precision.  IFmode and KFmode have precision set to 106 and 113 to
> distinguish them.  But that's precision in the sense of number of mantissa
> bits; as normally understood in GCC, precision should be the number of
> significant bits, so 128 for both those modes (but having multiple
> different binary floating-point modes with the same precision would
> require changing how we deal with laying out long double, so the target
> specifies a mode for floating-point types rather than leaving it to be
> deduced from values such as LONG_DOUBLE_TYPE_SIZE).
> 

Thanks for the advice, I'm looking into how the ppc KFmode behaves in
this situation now.

I also looked through the front end code a bit more, and the behaviour
of stor-layout.c:layout_type for RECORD_TYPE looks likes a smoking gun
to me.
For BOOLEAN_TYPE, INTEGER_TYPE, ENUMERAL_TYPE, REAL_TYPE,
FIXED_POINT_TYPE etc. layout_type sets:

TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type)));

But for the individual field types in RECORD_TYPE, UNION_TYPE or
QUAL_UNION_TYPE, this same setting of TYPE_SIZE and friends is not
performed.
So a field in a RECORD_TYPE might be an INTEGER_TYPE, but TYPE_SIZE for
this INTEGER_TYPE will not be set as it would have been had the type not
been a field in a RECORD_TYPE.

So the fix to me looks to be to ensure that the code for the base types
(BOOLEAN_TYPE, INTEGER_TYPE etc.) from layout_type is also executed for
each type that happens to be a field in a RECORD/UNION/QUAL_UNION_TYPE.

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

* Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array
  2017-08-22 11:34           ` Jozef Lawrynowicz
@ 2017-08-24 12:40             ` Jozef Lawrynowicz
  0 siblings, 0 replies; 9+ messages in thread
From: Jozef Lawrynowicz @ 2017-08-24 12:40 UTC (permalink / raw)
  To: Joseph Myers, Jeff Law; +Cc: gcc-patches

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

On 22/08/2017 11:57, Jozef Lawrynowicz wrote:
> On 02/08/2017 17:45, Joseph Myers wrote:
>> On Wed, 2 Aug 2017, Jeff Law wrote:
>>
>>> I think Joseph's suggestion for looking at partial float handling may be
>>> useful, though ia64's RFmode may be more interesting as it's not a
>>> multiple of 8 in bitsize.  IF/KF modes on the ppc port have similar
>>> properties.
>>
>> The key issue those floating-point modes engage is the meaning of
>> precision.  IFmode and KFmode have precision set to 106 and 113 to
>> distinguish them.  But that's precision in the sense of number of 
>> mantissa
>> bits; as normally understood in GCC, precision should be the number of
>> significant bits, so 128 for both those modes (but having multiple
>> different binary floating-point modes with the same precision would
>> require changing how we deal with laying out long double, so the target
>> specifies a mode for floating-point types rather than leaving it to be
>> deduced from values such as LONG_DOUBLE_TYPE_SIZE).
>>
> 
> Thanks for the advice, I'm looking into how the ppc KFmode behaves in
> this situation now.
> 
> I also looked through the front end code a bit more, and the behaviour
> of stor-layout.c:layout_type for RECORD_TYPE looks likes a smoking gun
> to me.
> For BOOLEAN_TYPE, INTEGER_TYPE, ENUMERAL_TYPE, REAL_TYPE,
> FIXED_POINT_TYPE etc. layout_type sets:
> 
> TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type)));
> 
> But for the individual field types in RECORD_TYPE, UNION_TYPE or
> QUAL_UNION_TYPE, this same setting of TYPE_SIZE and friends is not
> performed.
> So a field in a RECORD_TYPE might be an INTEGER_TYPE, but TYPE_SIZE for
> this INTEGER_TYPE will not be set as it would have been had the type not
> been a field in a RECORD_TYPE.
> 
> So the fix to me looks to be to ensure that the code for the base types
> (BOOLEAN_TYPE, INTEGER_TYPE etc.) from layout_type is also executed for
> each type that happens to be a field in a RECORD/UNION/QUAL_UNION_TYPE.

Actually, the issue turned out to be that TYPE_SIZE is specifically set
in the first place.
When the internal data structures to handle __intN types are initialised
in tree.c, the compiler is also setting TYPE_SIZE.

For the other "standard" types, layout_type sets TYPE_SIZE. So rather
than specifically setting TYPE_SIZE in tree.c, I've removed this code so
TYPE_SIZE will get set like it does for every other type.

If the attached patch is acceptable, I would appreciate if someone could
commit it for me, as I do not have write access.

Thanks,
Jozef

[-- Attachment #2: 0001-MSP430-Dont-specifically-set-TYPE_SIZE-for-__intN-ty.patch --]
[-- Type: text/plain, Size: 3464 bytes --]

From 5437c7ffa48f974c6960a1e308c4cdf0ea0a2648 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@somniumtech.com>
Date: Thu, 24 Aug 2017 11:40:04 +0000
Subject: [PATCH] MSP430: Dont specifically set TYPE_SIZE for __intN types

2017-08-XX	Jozef Lawrynowicz	<jozef.l@somniumtech.com>

	PR target/78849
	* gcc/tree.c (build_common_tree_nodes): Dont set TYPE_SIZE for __intN
	types.
	
gcc/testsuite
2017-08-XX	Jozef Lawrynowicz	<jozef.l@somniumtech.com>

	PR target/78849
	* gcc.target/msp430/msp430.exp: Remove -pedantic-errors from
	DEFAULT_CFLAGS.
	* gcc.target/msp430/pr78849.c: New test.
---
 gcc/testsuite/gcc.target/msp430/msp430.exp | 13 +++++---
 gcc/testsuite/gcc.target/msp430/pr78849.c  | 50 ++++++++++++++++++++++++++++++
 gcc/tree.c                                 |  2 --
 3 files changed, 59 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/pr78849.c

diff --git a/gcc/testsuite/gcc.target/msp430/msp430.exp b/gcc/testsuite/gcc.target/msp430/msp430.exp
index e54d531..3be8711 100644
--- a/gcc/testsuite/gcc.target/msp430/msp430.exp
+++ b/gcc/testsuite/gcc.target/msp430/msp430.exp
@@ -24,10 +24,15 @@ if { ![istarget msp430-*-*] } then {
 # Load support procs.
 load_lib gcc-dg.exp
 
-# If a testcase doesn't have special options, use these.
+# The '-pedantic-errors' option in the global variable DEFAULT_CFLAGS that is
+# set by other drivers causes an error when the __int20 type is used, so remove
+# this option from DEFAULT_CFLAGS for the msp430 tests.
 global DEFAULT_CFLAGS
-if ![info exists DEFAULT_CFLAGS] then {
-    set DEFAULT_CFLAGS ""
+if [info exists DEFAULT_CFLAGS] then {
+    set MSP430_DEFAULT_CFLAGS \
+      [ string map { "-pedantic-errors" "" } $DEFAULT_CFLAGS ]
+} else {
+   set MSP430_DEFAULT_CFLAGS ""
 }
 
 # Initialize `dg'.
@@ -35,7 +40,7 @@ dg-init
 
 # Main loop.
 dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
-	"" $DEFAULT_CFLAGS
+	"" $MSP430_DEFAULT_CFLAGS
 
 # All done.
 dg-finish
diff --git a/gcc/testsuite/gcc.target/msp430/pr78849.c b/gcc/testsuite/gcc.target/msp430/pr78849.c
new file mode 100644
index 0000000..f70f0bb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/pr78849.c
@@ -0,0 +1,50 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler ".size.*instance.*52" } } */
+
+struct t_inner
+{
+  __int20 a;
+  char val1;
+  __int20 b[3];
+  char val2;
+};
+
+struct t_full
+{
+  __int20 array[2];
+  char val1;
+  struct t_inner bb[2];
+  char val2;
+};
+
+struct t_full instance =
+{
+    {
+      4231,
+      3212,
+    },
+    5,
+    {
+        {
+          87680,
+	  20,
+          {
+            2534,
+            3,
+            41,
+          },
+	  55,
+        },
+        {
+          567,
+	  4,
+          {
+            43522,
+            5433,
+            454,
+          },
+	  88,
+        },
+    },
+    8,
+};
diff --git a/gcc/tree.c b/gcc/tree.c
index 4f56892..1c085ba 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -9526,8 +9526,6 @@ build_common_tree_nodes (bool signed_char)
     {
       int_n_trees[i].signed_type = make_signed_type (int_n_data[i].bitsize);
       int_n_trees[i].unsigned_type = make_unsigned_type (int_n_data[i].bitsize);
-      TYPE_SIZE (int_n_trees[i].signed_type) = bitsize_int (int_n_data[i].bitsize);
-      TYPE_SIZE (int_n_trees[i].unsigned_type) = bitsize_int (int_n_data[i].bitsize);
 
       if (int_n_data[i].bitsize > LONG_LONG_TYPE_SIZE
 	  && int_n_enabled_p[i])
-- 
1.8.3.1


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

end of thread, other threads:[~2017-08-24 11:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-19 13:39 ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array Jozef Lawrynowicz
2017-07-26 16:58 ` Jeff Law
2017-07-31 23:08   ` Joseph Myers
2017-08-01 16:32     ` Jozef Lawrynowicz
2017-08-02 16:33       ` Jeff Law
2017-08-02 16:46         ` Joseph Myers
2017-08-22 11:34           ` Jozef Lawrynowicz
2017-08-24 12:40             ` Jozef Lawrynowicz
2017-08-02 16:21     ` 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).