From patchwork Wed Feb 3 22:30:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve Ellcey X-Patchwork-Id: 578440 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5D14214076E for ; Thu, 4 Feb 2016 09:31:01 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=mR+Ibxkd; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:reply-to:to:cc:date:in-reply-to :references:content-type:content-transfer-encoding:mime-version; q=dns; s=default; b=FMrimRykPiJmsePnvgEFNA1A4yaRDOaetyB11kMYpIR JprAzGw69w/CTgy3VZbDZzo2OgL9QkazM4eNIXII6B5WQ8jyTiOIt3QJdS+ktZKv JG981h7sLl8G7VVvTItu9AZcGw7EamP9fjbXFco/dGITbLArus5vfRLgRcB8pkgI = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:reply-to:to:cc:date:in-reply-to :references:content-type:content-transfer-encoding:mime-version; s=default; bh=KnqZaMeoqr/G4hUbdqlZHeTNJNQ=; b=mR+Ibxkddi4CBjbga FavM21wcl/XL3vgo655aa4omR09qHIifa4LRdJFAjywSH1bgF0bfuevBOsGAJyB1 vz+AQDVajRJ7B5vgcrsAuaIG1/vJGPor0W1NayOr1VFpSbwjNIbcr324p8BI5Qzk UBgIbNW2n+SFSEneJ55hx+6FD8= Received: (qmail 86508 invoked by alias); 3 Feb 2016 22:30:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 86486 invoked by uid 89); 3 Feb 2016 22:30:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=conventions, defect, x3d, 1913 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 Feb 2016 22:30:44 +0000 Received: from hhmail02.hh.imgtec.org (unknown [10.100.10.20]) by Websense Email Security Gateway with ESMTPS id AA4271A847CB6; Wed, 3 Feb 2016 22:30:37 +0000 (GMT) Received: from BAMAIL02.ba.imgtec.org (10.20.40.28) by hhmail02.hh.imgtec.org (10.100.10.20) with Microsoft SMTP Server (TLS) id 14.3.266.1; Wed, 3 Feb 2016 22:30:41 +0000 Received: from [10.20.3.214] (10.20.3.214) by bamail02.ba.imgtec.org (10.20.40.28) with Microsoft SMTP Server (TLS) id 14.3.174.1; Wed, 3 Feb 2016 14:30:38 -0800 Message-ID: <1454538638.3232.125.camel@ubuntu-sellcey> Subject: Re: [Patch, MIPS] Fix PR target/68273, passing args in wrong regs From: Steve Ellcey Reply-To: To: Richard Biener , "Moore, Catherine" , "Maciej W. Rozycki" , "Matthew Fortune" CC: GCC Patches , Richard Sandiford , Eric Botcazou Date: Wed, 3 Feb 2016 14:30:38 -0800 In-Reply-To: References: <87egczwclj.fsf@googlemail.com> MIME-Version: 1.0 Here is a new patch for PR target/68273. It makes the GCC calling conventions compatible with LLVM so that the two agree with each other in all the cases I could think of testing and it fixes the reported defect. I couldn't get the GCC compatibility test to work (see https://gcc.gnu.org/ml/gcc/2016-02/msg00017.html) so I wasn't able to use that for compatibility testing, instead I hand examined routines with various argument types (structures, ints, complex, etc) to see what registers GCC and LLVM were accessing. This change does introduce an ABI/compatibility change with GCC itself and that is obviously a concern. Basically, any type that is passed by value and has an external alignment applied to it may get passed in different registers because we strip off that alignment (via TYPE_MAIN_VARIANT) before determining the alignment of the variable. If we don't want to break the GCC compatibility then we will continue to have an incompatibility with LLVM and we will need to find another way to deal with the aligned int variable that SRA is creating and passing to a function that expects a 'normal' integer. One thought I had was that we could compare TYPE_ALIGN (type) and TYPE_ALIGN (TYPE_MAIN_VARIANT (type) and if they are different, issue a warning during compilation about a possible incompatibility with older objects. Steve Ellcey sellcey@imgtec.com 2016-02-03 Steve Ellcey PR target/68273 * config/mips/mips.c (mips_function_arg_boundary): Fix argument alignment. diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 697abc2..4aa215f 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -5644,7 +5644,10 @@ mips_function_arg_boundary (machine_mode mode, const_tree type) { unsigned int alignment; - alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); + alignment = type + ? TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) + : GET_MODE_ALIGNMENT (mode); + if (alignment < PARM_BOUNDARY) alignment = PARM_BOUNDARY; if (alignment > STACK_BOUNDARY) 2016-02-03 Steve Ellcey PR target/68273 * gcc.c-torture/execute/pr68273-1.c: New test. * gcc.c-torture/execute/pr68273-2.c: New test. diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c index e69de29..3ce07c6 100644 --- a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c +++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c @@ -0,0 +1,74 @@ +/* Make sure that the alignment attribute on an argument passed by + value does not affect the calling convention and what registers + arguments are passed in. */ + +extern void exit (int); +extern void abort (void); + +typedef int alignedint __attribute__((aligned(8))); + +int __attribute__((noinline)) +foo1 (int a, alignedint b) +{ return a + b; } + +int __attribute__((noinline)) +foo2 (int a, int b) +{ + return a + b; +} + +int __attribute__((noinline)) +bar1 (alignedint x) +{ + return foo1 (1, x); +} + +int __attribute__((noinline)) +bar2 (alignedint x) +{ + return foo1 (1, (alignedint) 99); +} + +int __attribute__((noinline)) +bar3 (alignedint x) +{ + return foo1 (1, x + (alignedint) 1); +} + +alignedint q = 77; + +int __attribute__((noinline)) +bar4 (alignedint x) +{ + return foo1 (1, q); +} + + +int __attribute__((noinline)) +bar5 (alignedint x) +{ + return foo2 (1, x); +} + +int __attribute__((noinline)) +use_arg_regs (int i, int j, int k) +{ + return i+j-k; +} + +int main() +{ + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (foo1 (19,13) != 32) abort (); + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (bar1 (-33) != -32) abort (); + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (bar2 (1) != 100) abort (); + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (bar3 (17) != 19) abort (); + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (bar4 (-33) != 78) abort (); + if (use_arg_regs (999, 999, 999) != 999) abort (); + if (bar5 (-84) != -83) abort (); + exit (0); +} diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c b/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c index e69de29..1661be9 100644 --- a/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c +++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c @@ -0,0 +1,109 @@ +/* Make sure that the alignment attribute on an argument passed by + value does not affect the calling convention and what registers + arguments are passed in. */ + +extern void exit (int); +extern void abort (void); + +typedef struct s { + char c; + char d; +} t1; +typedef t1 t1_aligned8 __attribute__((aligned(8))); +typedef t1 t1_aligned16 __attribute__((aligned(16))); +typedef t1 t1_aligned32 __attribute__((aligned(32))); + +int bar1(int a, t1 b) +{ + return a + b.c + b.d; +} + +int bar2(int a, t1_aligned8 b) +{ + return a + b.c + b.d; +} + +int bar3(int a, t1_aligned16 b) +{ + return a + b.c + b.d; +} + +int bar4(int a, t1_aligned32 b) +{ + return a + b.c + b.d; +} + +#define FOODEF(n,m,type) \ +int __attribute__((noinline)) \ +foo##n (int i, type b) \ + { \ + return bar##m (i, b); \ + } + +FOODEF(1, 1, t1) +FOODEF(2, 1, t1_aligned8) +FOODEF(3, 1, t1_aligned16) +FOODEF(4, 1, t1_aligned32) +FOODEF(5, 2, t1) +FOODEF(6, 2, t1_aligned8) +FOODEF(7, 2, t1_aligned16) +FOODEF(8, 2, t1_aligned32) +FOODEF(9, 3, t1) +FOODEF(10, 3, t1_aligned8) +FOODEF(11, 3, t1_aligned16) +FOODEF(12, 3, t1_aligned32) +FOODEF(13, 4, t1) +FOODEF(14, 4, t1_aligned8) +FOODEF(15, 4, t1_aligned16) +FOODEF(16, 4, t1_aligned32) + +int __attribute__((noinline)) +use_arg_regs (int i, int j, int k) +{ + return i+j-k; +} + +#define FOOCALL(i) \ + c = i*11 + 39; \ + x1.c = i + 5; \ + x1.d = i*2 + 3; \ + x2.c = x1.c + 1; \ + x2.d = x1.d + 1; \ + x3.c = x2.c + 1; \ + x3.d = x2.d + 1; \ + x4.c = x3.c + 1; \ + x4.d = x3.d + 1; \ + if (use_arg_regs (999,999,999) != 999) abort (); \ + if (foo##i (c, x1) != c + x1.c + x1.d) abort (); \ + if (use_arg_regs (999,999,999) != 999) abort (); \ + if (foo##i (c, x2) != c + x2.c + x2.d) abort (); \ + if (use_arg_regs (999,999,999) != 999) abort (); \ + if (foo##i (c, x3) != c + x3.c + x3.d) abort (); \ + if (use_arg_regs (999,999,999) != 999) abort (); \ + if (foo##i (c, x4) != c + x4.c + x4.d) abort (); + +int main() +{ + int c; + t1 x1; + t1_aligned8 x2; + t1_aligned16 x3; + t1_aligned32 x4; + FOOCALL(1); + FOOCALL(2); + FOOCALL(3); + FOOCALL(4); + FOOCALL(5); + FOOCALL(6); + FOOCALL(7); + FOOCALL(8); + FOOCALL(9); + FOOCALL(10); + FOOCALL(11); + FOOCALL(12); + FOOCALL(13); + FOOCALL(14); + FOOCALL(15); + FOOCALL(16); + exit (0); +}