https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87691

--- Comment #5 from Jozef Lawrynowicz <jozef.l at mittosystems dot com> ---
(In reply to Richard Biener from comment #4)

Thanks for the pointers.

> What happens if you make the attribute work for a MODE_INT union with a
> MODE_PARTIAL_INT first field that has MODE_SIZE of the union mode?
>
> Is there
> a generic way to query SImode for PSImode as defined in
> 
> PARTIAL_INT_MODE (SI, 20, PSI);
> 
> ?  Or does one need to compare MODE_SIZE?

I believe you just have to compare MODE_SIZE as I did below.

>  Can there be multiple non-partial integer modes with the same size?

Currently for the targets using PARTIAL_INT_MODE there is only a one-to-one
mapping from the "full" mode to the partial mode. But where it can get tricky
is when examining the BITSIZE of a partial int mode, as depending on the
context, the bitsize could be the bitsize of the SI mode or the true bitsize of
the PSImode i.e. the precision.

I tried this (there may well be a neater way):

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 4416b50..74f6a76 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -1280,7 +1280,11 @@ handle_transparent_union_attribute (tree *node, tree
name,
          tree first = first_field (type);
          if (first == NULL_TREE
              || DECL_ARTIFICIAL (first)
-             || TYPE_MODE (type) != DECL_MODE (first))
+             || (TYPE_MODE (type) != DECL_MODE (first)
+                 && !(GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT
+                      && GET_MODE_CLASS (DECL_MODE(first)) == MODE_PARTIAL_INT
+                      && known_eq (GET_MODE_BITSIZE (TYPE_MODE (type)),
+                                   GET_MODE_BITSIZE (DECL_MODE (first))))))
            goto ignored;
        }

It fixes most of those failing DejaGNU tests, except pr34885.c ends up ICE-ing
at "-O3 -g".

during RTL pass: final^M
/home/jozef/msp430/gcc/gcc/testsuite/gcc.c-torture/compile/pr34885.c: In
function 'send':^M
/home/jozef/msp430/gcc/gcc/testsuite/gcc.c-torture/compile/pr34885.c:9:1:
internal compiler error: Segmentation fault^M
0xb58f4f crash_signal^M
  ../../gcc/toplev.c:325^M
0x7bf92d gen_subprogram_die^M
  ../../gcc/dwarf2out.c:23270^M
0x7c1c5b gen_decl_die^M
  ../../gcc/dwarf2out.c:26197^M
0x7c3f9e dwarf2out_decl^M
  ../../gcc/dwarf2out.c:26765^M
0x7d707e dwarf2out_function_decl^M
  ../../gcc/dwarf2out.c:26780^M
0x83e757 rest_of_handle_final^M
  ../../gcc/final.c:4681^M
0x83e757 execute^M
  ../../gcc/final.c:4723^M

I haven't investigated the above ICE as the below alternative approach fixes
the issue and offers codegen benefits.

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 58a3aa3..6449f16 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -1844,10 +1844,15 @@ compute_record_mode (tree type)
     }

   /* If we only have one real field; use its mode if that mode's size
-     matches the type's size.  This only applies to RECORD_TYPE.  This
-     does not apply to unions.  */
+     matches the type's size.  This generally only applies to RECORD_TYPE.
+     As an exception, if the union will be passed by reference then it is
+     acceptable to use the mode of the widest field for the mode of the
+     union.  */
   poly_uint64 type_size;
-  if (TREE_CODE (type) == RECORD_TYPE
+  if (((TREE_CODE (type) == RECORD_TYPE)
+       || (TREE_CODE (type) == UNION_TYPE
+          && targetm.calls.pass_by_reference (pack_cumulative_args (NULL),
+                                              mode, type, 0)))
       && mode != VOIDmode
       && poly_int_tree_p (TYPE_SIZE (type), &type_size)
       && known_eq (GET_MODE_BITSIZE (mode), type_size))

The codegen benefits are that the target might have instructions to
specifically operate on the partial int mode, which it can use if the mode of
the union is set to this partial int mode.

So for msp430 if the widest type in a union is __int20 (PSImode), then allowing
the union mode to be PSImode results in fewer instructions being used to
manipulate the union, compared to if the union mode was SImode.

I'll go ahead and test the second patch, unless there is a reason not to allow
non-MODE_INT unions. If, for example, we'd rather not have unions be MODE_FLOAT
I could always restrict the above logic further to only allow the union mode to
be set to a MODE_PARTIAL_INT in addition to the currently allowable MODE_INT.

Reply via email to