https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87691
--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> --- On Tue, 23 Oct 2018, jozef.l at mittosystems dot com wrote: > 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. That makes sense.
