https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87691
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jozef Lawrynowicz from comment #5)
> (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.
I think it's better to, at this place, simply allow MODE_INT or
MODE_PARTIAL_INT
for unions. Allowing MODE_INT is what we'd fall back to anyways and you
want to allow MODE_PARTIAL_INT which is already allowed for RECORD_TYPEs.
What I'm not sure is if the target is happy about, say,
union U { float f; __int20 i; };
float foo()
{
union U u;
u.i = 10;
return u.f;
}
iff we were to put u into a MODE_PARTIAL_INT register are targets to be
expected to be able to extract a MODE_FLOAT from it or are we going
to spill here anyways?
How is TYPE_SIZE / GET_MODE_BITSIZE of this union anyways? The loop
in stor-layout checks TYPE_SIZE vs. DECL_SIZE - is DECL_SIZE always
matching GET_MODE_BITSIZE?
