gdc master is broken for arm cross-compilers

[Johannes Pfau via D.gnu]

Am Tue, 16 Jun 2015 22:05:44 +0200
schrieb Johannes Pfau <nospam at example.com>:

> Am Thu, 11 Jun 2015 22:30:39 +0200
> schrieb "Iain Buclaw via D.gnu" <d.gnu at puremagic.com>:
> 
> > On 11 June 2015 at 20:27, Johannes Pfau via D.gnu
> > <d.gnu at puremagic.com> wrote:
> > 
> > > Some recent change in GDC broke ARM cross compiler builds. Same
> > > error with gcc-5.1 and gcc-4.9:
> > >
> > > https://gist.github.com/jpf91/1de81d6ff55587d702ae
> > >
> > > I'm not sure if this only happens for ARM cross compilers or for
> > > all cross compilers. But it doesn't seem to happen with native
> > > builds (or maybe it's target specific and it doesn't happen for
> > > x86 builds).
> > >
> > >
> > > Iain, any clue what could cause this? Otherwise I'll have to debug
> > > this later on.
> > >
> > 
> > The comment above says:
> > 
> >  If the DECL isn't in memory, then the DECL wasn't properly
> >  marked TREE_ADDRESSABLE, which will be either a front-end
> >  or a tree optimizer bug.
> > 
> > Peek returns a ubyte[4] - so my initial guess would be the recent
> > NRVO change.  Perhaps it wasn't quite as bullet-proof as I hoped.
> > 
> 
> Small update: It's indeed the NRVO changes, reduced test case:
> ------------------
> private union EndianSwapper
> {
>     uint value;
>     ubyte[4] array;
> }
> 
> struct CRC32
> {
>     public:
>         @trusted pure nothrow finish()
>         {
>             auto tmp = peek();
>             return tmp;
>         }
> 
>         @trusted pure nothrow peek()
>         {
>             EndianSwapper es;
>             es.value = 0;
>             return es.array;
>         }
> }
> ------------------
> 

@Iain please read my questions at the bottom of this message, the first
(debugging) part probably isn't important.


Debugging showed that the backend assigns a REG:SI rtx for tmp.
However, tmp does have the ADDRESSABLE flag set:

--------------------------------------------------------
 <result_decl 0x7ffff7ff5078 tmp
    type <array_type 0x7ffff71f1738
        type <integer_type 0x7ffff73e5f18 ubyte public unsigned QI
            size <integer_cst 0x7ffff73cdf00 constant 8>
            unit size <integer_cst 0x7ffff73cdf18 constant 1>
            align 8 symtab 0 alias set -1 canonical type 0x7ffff73e5f18
 precision 8 min <integer_cst 0x7ffff73dd2e8 0> max <integer_cst
 0x7ffff73dd2b8 255> pointer_to_this <pointer_type 0x7ffff71f17e0>>
 no-force-blk BLK size <integer_cst 0x7ffff73cddc8 constant 32>
        unit size <integer_cst 0x7ffff73cdde0 constant 4>
        align 8 symtab 0 alias set -1 canonical type 0x7ffff71f1738
        domain <integer_type 0x7ffff71f1690 type <integer_type
 0x7ffff73d10a8 sizetype> public SI size <integer_cst 0x7ffff73cddc8
 32> unit size <integer_cst 0x7ffff73cdde0 4> align 32 symtab 0 alias
 set -1 canonical type 0x7ffff71f1690 precision 32 min <integer_cst
 0x7ffff73cddf8 0> max <integer_cst 0x7ffff71e34c8 3>> pointer_to_this
 <pointer_type 0x7ffff71fb2a0>> addressable used ignored regdecl BLK
 file /home/build/gdc-build-configs/configs/x86_64-linux-gnu/gcc-snapshot/arm-gdcproject-linux-gnueabi/.build/src/gcc-custom/libphobos/src/std/digest/crc.d
 line 10 col 14 size <integer_cst 0x7ffff73cddc8 32> unit size
 <integer_cst 0x7ffff73cdde0 4> align 8 context <function_decl
 0x7ffff71f4000 finish> (reg:SI 110 [ <retval> ])>
--------------------------------------------------------


The wrong RTL is generated in function.c:expand_function_start.
expand_function_start checks aggregate_value_p (DECL_RESULT (subr),
subr). aggregate_value_p does not look at TREE_ADDRESSABLE of the
result decl, it only looks at TREE_ADDRESSABLE for the result type and
the function type. (which could be a gcc bug?)

Because of this we might have to set DECL_BY_REFERENCE for the return
value as well. Will test this tomorrow.

X86 might avoid this issue because of this:
if (flag_pcc_struct_return && AGGREGATE_TYPE_P (type))
or this
if (targetm.calls.return_in_memory (type, fntype))



However, writing this down and thinking about it now I've got one
fundamental question: Can we even legally do NRVO in the posted test
case? The return types are PODs and fit in a register. So from an API
perspective they should be returned in registers unless otherwise
requested by the user. We can probably set DECL_REFERENCE on the result
to force this. But if we do this we change the ABI (register vs memory
pointer) so we also need to be able to determine whether this function
returns in memory when calling it. I don't see anything
special about the function signature in this example which would allow
this.

Could it be possible the
else if (TREE_CODE (return_type) == ARRAY_TYPE) ...BLKmode
check isn't doing what it's supposed to do?
Maybe we have to call if (targetm.calls.return_in_memory (type, fntype))
instead, like aggregate_value_p does.

Also why is the extra array check necessary? Shouldn't
aggregate_value_p handle arrays as well?