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

--- Comment #7 from Jozef Lawrynowicz <jozef.l at mittosystems dot com> ---
(In reply to Richard Biener from comment #6)
> 
> 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?

I tweaked the above source a little:

float foo(union U u)
{
  u.i = u.i + 10;
  return u.f;
}

For this, GCC fills u.i then spills after modifying it. I therefore assume in
general that if u.f is alive then modifications to u.i will always be spilled
after.

> 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?

It actually depends on the order of the fields in the union, which is not good.
Since __int20 and float have the same BITSIZE, the mode of the last field in
the union will be used for the mode of the union. So for the above code with my
current patch, the union TYPE_MODE is PSImode. This causes wrong code to be
generated, as the function foo starts by only pushing the __int20 value of the
union onto the stack, rather than the full 32-bits.

I had the following additional patch which didn't seem to affect the
testresults, but fixes this issue so is clearly beneficial:

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 6449f16..3bb0bbc 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -1834,7 +1834,13 @@ compute_record_mode (tree type)
       /* If this field is the whole struct, remember its mode so
         that, say, we can put a double in a class into a DF
         register instead of forcing it to live in the stack.  */
-      if (simple_cst_equal (TYPE_SIZE (type), DECL_SIZE (field)))
+      if (simple_cst_equal (TYPE_SIZE (type), DECL_SIZE (field))
+         /* Partial int types (e.g. __int20) may have TYPE_SIZE equal to
+            wider types (e.g. int32), despite precision being less.  Ensure
+            that the TYPE_MODE of the struct does not get set to the partial
+            int mode if there is a wider type also in the struct.  */
+         && known_gt (GET_MODE_PRECISION (DECL_MODE (field)),
+                      GET_MODE_PRECISION (mode)))
        mode = DECL_MODE (field);

       /* With some targets, it is sub-optimal to access an aligned

This fixes the above issue, so the union TYPE_MODE is always SFmode, regardless
of the ordering of the fields.

Reply via email to