Re: [PATCH] Fix failing assertion in calls.c:store_unaligned_arguments_into_pseudos

[Jeff Law <law at redhat dot com>]

On 11/11/13 12:10, Ulrich Weigand wrote:
> Jeff Law wrote:
> > On 11/11/13 07:32, Ulrich Weigand wrote:
> > > However, looking more closely, it seems store_unaligned_arguments_into_pseudos
> > > is not really useful for PARALLEL arguments in the first place.  What this
> > > routine does is load arguments into args[i].aligned_regs.  But if we have
> > > an argument where args[i].reg is a PARALLEL, args[i].aligned_regs will in
> > > fact never be used later on at all!   Instead, PARALLEL will always be
> > > handled directly via emit_group_move (in load_register_parameters), so
> > > the code generated by store_unaligned_arguments_into_pseudos for such cases
> > > is simply dead anyway.
>
> > Does this work on the PA, particularly the 32bit ABI?
> >            /* Structures 5 to 8 bytes in size are passed in the general
> >                registers in the same manner as other non floating-point
> >                objects.  The data is right-justified and zero-extended
> >                to 64 bits.  This is opposite to the normal justification
> >                used on big endian targets and requires special treatment.
> >                We now define BLOCK_REG_PADDING to pad these objects.
> >                Aggregates, complex and vector types are passed in the same
> >                manner as structures.  */
>
> It seems this is exactly one of the cases I mention above, where current
> code generates code to load up "aligned registers" which are then never
> used again.
Agreed.  I built a cross compiler and managed to get some old grey 
matter activated enough to read the resulting code.


> I'm not really set up to test PA, but I built a cross to hppa-linux,
> and compiled this simple test passing a 5-byte struct:
Few people are, I think the cross testing is fine.


> struct test { char x[5]; };
>
> struct test x;
> void func (struct test);
>
> void caller (void)
> {
>    func (x);
> }
>
> And indeed with my patch, I'm getting a lot less code at -O0.  However,
> building with -O2 both with and without my patch, the resulting assembler
> files are identical again -- which appears to confirm that this extra
> code is in fact dead ...
Just as importantly, if I'm reading the resulting code correctly, the 
values we actually end up passing in the argument registers (%r26,%r25) 
are the same before/after your patch.

Let me take a look at the actual patch again since it seems nobody is 
ready to go out on a limb and approve this stuff :-)

jeff