www.digitalmars.com         C & C++   DMDScript  

D.gnu - gdc master is broken for arm cross-compilers

reply Johannes Pfau <nospam example.com> writes:
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.
Jun 11 2015
parent reply "Iain Buclaw via D.gnu" <d.gnu puremagic.com> writes:
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.
Jun 11 2015
parent reply Johannes Pfau <nospam example.com> writes:
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; } } ------------------ Only happens with certain optimizations (-O or higher). Using -O -fno-tree-sra or -O -tree-no-inline hides the problem. Tree dump before crash: ------------------ finish (struct CRC32 & this) { union EndianSwapper es.0; <bb 2>: es.0 ={v} {CLOBBER}; MEM[(ubyte[4] *)&<retval>] = 0; return <retval>; }
Jun 16 2015
parent reply Johannes Pfau <nospam example.com> writes:
Am Tue, 16 Jun 2015 22:05:44 +0200
schrieb Johannes Pfau <nospam 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?
Jun 18 2015
next sibling parent "Iain Buclaw via D.gnu" <d.gnu puremagic.com> writes:
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 <nospam 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.
Jun 18 2015
prev sibling parent "Iain Buclaw via D.gnu" <d.gnu puremagic.com> writes:
On 18 June 2015 at 23:07, Iain Buclaw <ibuclaw gdcproject.org> wrote:

 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 <nospam 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.
This is what I've settled with: https://github.com/D-Programming-GDC/GDC/pull/107 I've fixed the one failing testsuite test because GDC actually returns int[3] in registers (on 64bit at least). Iain
Jun 18 2015