gdc master is broken for arm cross-compilers

[Iain Buclaw via D.gnu]

On 18 June 2015 at 22:18, Johannes Pfau via D.gnu <d.gnu at puremagic.com>
wrote:

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

For aggregate_value_p to work correctly, I think the function type should
be marked as addressable.

I was just building gdc in a qemu raspbian chroot for other reasons, and
have just hit this.  Will have a look.

Iain.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/d.gnu/attachments/20150618/c6688f4f/attachment.html>