This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] PR target/65491: Classify V1TF vectors as AAPCS64 short vectors rather than composite types
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Fri, 22 May 2015 15:39:25 +0100
- Subject: Re: [PATCH][AArch64] PR target/65491: Classify V1TF vectors as AAPCS64 short vectors rather than composite types
- Authentication-results: sourceware.org; auth=none
- References: <000001d07b53$012f1830$038d4890$ at arm dot com> <20150519111854 dot GA31361 at arm dot com>
Hi James,
On 19/05/15 12:18, James Greenhalgh wrote:
On Mon, Apr 20, 2015 at 11:16:02AM +0100, Kyrylo Tkachov wrote:
Hi all,
The ICE in the PR happens when we pass a 1x(128-bit float) vector as an
argument.
The aarch64 backend erroneously classifies it as a composite type when in
fact it
is a short vector according to AAPCS64
(section 4.1.2 from
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.p
df).
Agreed.
The solution in this patch is to check aarch64_composite_type_p for a short
vector with
aarch64_short_vector_p rather than the other way around (check for
aarch64_short_vector_p
in aarch64_composite_type_p).
I think I understand what you are saying, but your patch does the
opposite (ADDS a check for aarch64_short_vector_p in
aarch64_composite_type_p, REMOVES a check for aarch64_composite_type_p,
in aarch64_short_vector_p)...
Yeah, I just worded it wrong in the cover letter, sorry about that.
As you say, the logic is pretty hairy.
This logic is pretty hairy, and I'm struggling to convince myself that
your change only hits the bug you described above. I think I've worked
it through and it does, but if you can find any additional ABI tests
which stress the Vector/Floating-Point passing rules that would help
settle my nerves.
The aapcs64.exp stuff seems to test the existing rules
quite well...
The patch is OK. I wouldn't think we would want to backport it to
release branches as there is no regression to fix.
Ok, I've committed it with r223577.
I agree that it's not a regression fix, so messing with ABI code
in the release branches is not desirable.
Thanks for the review.
Kyrill
Thanks,
James
2015-04-20 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/65491
* config/aarch64/aarch64.c (aarch64_short_vector_p): Move above
aarch64_composite_type_p. Remove check for aarch64_composite_type_p.
(aarch64_composite_type_p): Return false if given type and mode are
for a short vector.
2015-04-20 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/65491
* gcc.target/aarch64/pr65491_1.c: New test.
* gcc.target/aarch64/aapcs64/type-def.h (vlf1_t): New typedef.
* gcc.target/aarch64/aapcs64/func-ret-1.c: Add test for vlf1_t.