On 18 June 2015 at 22:18, Johannes Pfau via D.gnu <d.gnu@puremagic.com> wrote:
> Am Tue, 16 Jun 2015 22:05:44 +0200 > schrieb Johannes Pfau <nos...@example.com>: > > > Am Thu, 11 Jun 2015 22:30:39 +0200 > > schrieb "Iain Buclaw via D.gnu" <d.gnu@puremagic.com>: > > > > > On 11 June 2015 at 20:27, Johannes Pfau via D.gnu > > > <d.gnu@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.
