Bug 56025 - ARM NEON polynomial types have broken overload resolution
Summary: ARM NEON polynomial types have broken overload resolution
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ABI
Depends on:
Blocks:
 
Reported: 2013-01-18 09:38 UTC by Tim Northover
Modified: 2015-01-07 14:37 UTC (History)
2 users (show)

See Also:
Host:
Target: arm-eabi, arm-linux-gnueabi
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-01-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Northover 2013-01-18 09:38:22 UTC
While investigating bug #56024, we discovered this problem in the same area. Essentially, GCC has semi-special builtin types to cover poly8_t and poly16_t defined in arm_neon.h.

These types are used by G++ when calculating the overload resolution. The following two functions can both be defined with no issues in the front-end:

#include <arm_neon.h>
void foo(short s) {}
void foo(__builtin_neon_poly16 s) {}

However, in the resulting assembly they are both mangled as _Z3foos, which causes a conflict.

This mangling area is likely to be affected by any change fixing 56024, so a sensible combined solution might be a good idea.
Comment 1 Richard Earnshaw 2013-01-18 09:51:40 UTC
Agreed.  If there's a conflict in the mangling, it really should be detectable at definition time if the two definitions appear in the same translation unit.
Comment 2 Tejas Belagod 2015-01-06 17:14:11 UTC
(In reply to Tim Northover from comment #0)
> While investigating bug #56024, we discovered this problem in the same area.
> Essentially, GCC has semi-special builtin types to cover poly8_t and
> poly16_t defined in arm_neon.h.
> 
> These types are used by G++ when calculating the overload resolution. The
> following two functions can both be defined with no issues in the front-end:
> 
> #include <arm_neon.h>
> void foo(short s) {}
> void foo(__builtin_neon_poly16 s) {}
> 
> However, in the resulting assembly they are both mangled as _Z3foos, which
> causes a conflict.
> 
> This mangling area is likely to be affected by any change fixing 56024, so a
> sensible combined solution might be a good idea.

This should now be fixed. Now mangled as _Z3foos and _Z3foo10__Poly16_t respectively.

	.cpu generic+fp+simd
	.file	"gpp.cpp"
	.text
	.align	2
	.p2align 3,,7
	.global	_Z3foos
	.type	_Z3foos, %function
_Z3foos:
.LFB3026:
	.cfi_startproc
	ret
	.cfi_endproc
.LFE3026:
	.size	_Z3foos, .-_Z3foos
	.align	2
	.p2align 3,,7
	.global	_Z3foo10__Poly16_t
	.type	_Z3foo10__Poly16_t, %function
_Z3foo10__Poly16_t:
.LFB3027:
	.cfi_startproc
	ret
	.cfi_endproc
.LFE3027:
	.size	_Z3foo10__Poly16_t, .-_Z3foo10__Poly16_t
	.ident	"GCC: (unknown) 5.0.0 20141229 (experimental)"
Comment 3 Tejas Belagod 2015-01-06 17:21:38 UTC
(In reply to Tejas Belagod from comment #2)
> (In reply to Tim Northover from comment #0)
> > While investigating bug #56024, we discovered this problem in the same area.
> > Essentially, GCC has semi-special builtin types to cover poly8_t and
> > poly16_t defined in arm_neon.h.
> > 
> > These types are used by G++ when calculating the overload resolution. The
> > following two functions can both be defined with no issues in the front-end:
> > 
> > #include <arm_neon.h>
> > void foo(short s) {}
> > void foo(__builtin_neon_poly16 s) {}
> > 
> > However, in the resulting assembly they are both mangled as _Z3foos, which
> > causes a conflict.
> > 
> > This mangling area is likely to be affected by any change fixing 56024, so a
> > sensible combined solution might be a good idea.
> 
> This should now be fixed. Now mangled as _Z3foos and _Z3foo10__Poly16_t
> respectively.
> 
> 	.cpu generic+fp+simd
> 	.file	"gpp.cpp"
> 	.text
> 	.align	2
> 	.p2align 3,,7
> 	.global	_Z3foos
> 	.type	_Z3foos, %function
> _Z3foos:
> .LFB3026:
> 	.cfi_startproc
> 	ret
> 	.cfi_endproc
> .LFE3026:
> 	.size	_Z3foos, .-_Z3foos
> 	.align	2
> 	.p2align 3,,7
> 	.global	_Z3foo10__Poly16_t
> 	.type	_Z3foo10__Poly16_t, %function
> _Z3foo10__Poly16_t:
> .LFB3027:
> 	.cfi_startproc
> 	ret
> 	.cfi_endproc
> .LFE3027:
> 	.size	_Z3foo10__Poly16_t, .-_Z3foo10__Poly16_t
> 	.ident	"GCC: (unknown) 5.0.0 20141229 (experimental)"

Sorry, wrong target. Still doesn't seem to be fixed for aarch32.
Comment 4 Tejas Belagod 2015-01-06 17:36:04 UTC
The ABI does not define an internal type name for scalar poly types - http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042e/IHI0042E_aapcs.pdf. This will need an ABI update.
Comment 5 Tejas Belagod 2015-01-07 14:37:37 UTC
ACLE states that

"poly8_t and poly16_t are defined as unsigned integer types. It is unspecified whether these are the same type as uint8_t and  uint16_t for overloading and mangling purposes"

Presently, poly types are implemented as distinct types in the ARM backend with default mangling which is wrong. This obviates the need for their own mangling scheme in the ABI which it does not specify. Therefore, it cannot be modelled as a distinct type until the ABI says that it is a distinct type. Also, poly8_t and poly16_t are presently modelled as signed types which is incorrect. When we change this to an unsigned type, it will be incompatible with legacy anyway wrt mangling among other things. So, till the time the ABI decides to make poly types as distinct types, we can model it as an unsigned type or as another type name for uint8_t or uint16_t. This will disallow overloading of uint8_t and poly8_t for eg and break current overloads, but correcting poly to unsigned type is a greater good. This is also future-proof because if the ABI decides to model poly and unsigned types as distinct types, previously disallowed overloading will then be allowed which isn't breaking legacy in the broad sense.