<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 18 June 2015 at 23:07, Iain Buclaw <span dir="ltr"><<a href="mailto:ibuclaw@gdcproject.org" target="_blank">ibuclaw@gdcproject.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div class="h5"><div class="gmail_extra"><div class="gmail_quote">On 18 June 2015 at 22:18, Johannes Pfau via D.gnu <span dir="ltr"><<a href="mailto:d.gnu@puremagic.com" target="_blank">d.gnu@puremagic.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Am Tue, 16 Jun 2015 22:05:44 +0200<br>
schrieb Johannes Pfau <<a href="mailto:nospam@example.com" target="_blank">nospam@example.com</a>>:<br>
<div><div><br>
> Am Thu, 11 Jun 2015 22:30:39 +0200<br>
> schrieb "Iain Buclaw via D.gnu" <<a href="mailto:d.gnu@puremagic.com" target="_blank">d.gnu@puremagic.com</a>>:<br>
><br>
> > On 11 June 2015 at 20:27, Johannes Pfau via D.gnu<br>
> > <<a href="mailto:d.gnu@puremagic.com" target="_blank">d.gnu@puremagic.com</a>> wrote:<br>
> ><br>
> > > Some recent change in GDC broke ARM cross compiler builds. Same<br>
> > > error with gcc-5.1 and gcc-4.9:<br>
> > ><br>
> > > <a href="https://gist.github.com/jpf91/1de81d6ff55587d702ae" rel="noreferrer" target="_blank">https://gist.github.com/jpf91/1de81d6ff55587d702ae</a><br>
> > ><br>
> > > I'm not sure if this only happens for ARM cross compilers or for<br>
> > > all cross compilers. But it doesn't seem to happen with native<br>
> > > builds (or maybe it's target specific and it doesn't happen for<br>
> > > x86 builds).<br>
> > ><br>
> > ><br>
> > > Iain, any clue what could cause this? Otherwise I'll have to debug<br>
> > > this later on.<br>
> > ><br>
> ><br>
> > The comment above says:<br>
> ><br>
> > If the DECL isn't in memory, then the DECL wasn't properly<br>
> > marked TREE_ADDRESSABLE, which will be either a front-end<br>
> > or a tree optimizer bug.<br>
> ><br>
> > Peek returns a ubyte[4] - so my initial guess would be the recent<br>
> > NRVO change. Perhaps it wasn't quite as bullet-proof as I hoped.<br>
> ><br>
><br>
> Small update: It's indeed the NRVO changes, reduced test case:<br>
> ------------------<br>
> private union EndianSwapper<br>
> {<br>
> uint value;<br>
> ubyte[4] array;<br>
> }<br>
><br>
> struct CRC32<br>
> {<br>
> public:<br>
> @trusted pure nothrow finish()<br>
> {<br>
> auto tmp = peek();<br>
> return tmp;<br>
> }<br>
><br>
> @trusted pure nothrow peek()<br>
> {<br>
> EndianSwapper es;<br>
> es.value = 0;<br>
> return es.array;<br>
> }<br>
> }<br>
> ------------------<br>
><br>
<br>
</div></div>@Iain please read my questions at the bottom of this message, the first<br>
(debugging) part probably isn't important.<br>
<br>
<br>
Debugging showed that the backend assigns a REG:SI rtx for tmp.<br>
However, tmp does have the ADDRESSABLE flag set:<br>
<br>
--------------------------------------------------------<br>
<result_decl 0x7ffff7ff5078 tmp<br>
type <array_type 0x7ffff71f1738<br>
type <integer_type 0x7ffff73e5f18 ubyte public unsigned QI<br>
size <integer_cst 0x7ffff73cdf00 constant 8><br>
unit size <integer_cst 0x7ffff73cdf18 constant 1><br>
align 8 symtab 0 alias set -1 canonical type 0x7ffff73e5f18<br>
precision 8 min <integer_cst 0x7ffff73dd2e8 0> max <integer_cst<br>
0x7ffff73dd2b8 255> pointer_to_this <pointer_type 0x7ffff71f17e0>><br>
no-force-blk BLK size <integer_cst 0x7ffff73cddc8 constant 32><br>
unit size <integer_cst 0x7ffff73cdde0 constant 4><br>
align 8 symtab 0 alias set -1 canonical type 0x7ffff71f1738<br>
domain <integer_type 0x7ffff71f1690 type <integer_type<br>
0x7ffff73d10a8 sizetype> public SI size <integer_cst 0x7ffff73cddc8<br>
32> unit size <integer_cst 0x7ffff73cdde0 4> align 32 symtab 0 alias<br>
set -1 canonical type 0x7ffff71f1690 precision 32 min <integer_cst<br>
0x7ffff73cddf8 0> max <integer_cst 0x7ffff71e34c8 3>> pointer_to_this<br>
<pointer_type 0x7ffff71fb2a0>> addressable used ignored regdecl BLK<br>
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<br>
line 10 col 14 size <integer_cst 0x7ffff73cddc8 32> unit size<br>
<integer_cst 0x7ffff73cdde0 4> align 8 context <function_decl<br>
0x7ffff71f4000 finish> (reg:SI 110 [ <retval> ])><br>
--------------------------------------------------------<br>
<br>
<br>
The wrong RTL is generated in function.c:expand_function_start.<br>
expand_function_start checks aggregate_value_p (DECL_RESULT (subr),<br>
subr). aggregate_value_p does not look at TREE_ADDRESSABLE of the<br>
result decl, it only looks at TREE_ADDRESSABLE for the result type and<br>
the function type. (which could be a gcc bug?)<br>
<br>
Because of this we might have to set DECL_BY_REFERENCE for the return<br>
value as well. Will test this tomorrow.<br>
<br>
X86 might avoid this issue because of this:<br>
if (flag_pcc_struct_return && AGGREGATE_TYPE_P (type))<br>
or this<br>
if (targetm.calls.return_in_memory (type, fntype))<br>
<br>
<br>
<br>
However, writing this down and thinking about it now I've got one<br>
fundamental question: Can we even legally do NRVO in the posted test<br>
case? The return types are PODs and fit in a register. So from an API<br>
perspective they should be returned in registers unless otherwise<br>
requested by the user. We can probably set DECL_REFERENCE on the result<br>
to force this. But if we do this we change the ABI (register vs memory<br>
pointer) so we also need to be able to determine whether this function<br>
returns in memory when calling it. I don't see anything<br>
special about the function signature in this example which would allow<br>
this.<br>
<br>
Could it be possible the<br>
else if (TREE_CODE (return_type) == ARRAY_TYPE) ...BLKmode<br>
check isn't doing what it's supposed to do?<br>
Maybe we have to call if (targetm.calls.return_in_memory (type, fntype))<br>
instead, like aggregate_value_p does.<br>
<br>
Also why is the extra array check necessary? Shouldn't<br>
aggregate_value_p handle arrays as well?<br>
</blockquote></div><br></div></div></div><div class="gmail_extra">For aggregate_value_p to work correctly, I think the function type should be marked as addressable.<br><br></div><div class="gmail_extra">I was just building gdc in a qemu raspbian chroot for other reasons, and have just hit this. Will have a look.<span class=""><font color="#888888"><br><br></font></span></div><span class=""><font color="#888888"><div class="gmail_extra">Iain.<br></div><div class="gmail_extra"><br></div></font></span></div>
</blockquote></div><br></div><div class="gmail_extra">This is what I've settled with: <a href="https://github.com/D-Programming-GDC/GDC/pull/107">https://github.com/D-Programming-GDC/GDC/pull/107</a><br><br></div><div class="gmail_extra">I've fixed the one failing testsuite test because GDC actually returns int[3] in registers (on 64bit at least).<br><br></div><div class="gmail_extra">Iain<br></div></div>