From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96269 invoked by alias); 18 Jan 2016 11:36:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 96232 invoked by uid 89); 18 Jan 2016 11:36:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=type_mode, TYPE_MODE, fragile, complementary X-HELO: mail-yk0-f180.google.com Received: from mail-yk0-f180.google.com (HELO mail-yk0-f180.google.com) (209.85.160.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 18 Jan 2016 11:36:49 +0000 Received: by mail-yk0-f180.google.com with SMTP id v14so516194563ykd.3 for ; Mon, 18 Jan 2016 03:36:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=Aw3IPti24dSlfOAt21HDNWKK1Fw0bQYS8bRovN8zNSE=; b=j1tlziFnIMJct7Mfk8+iLBBh2hwMA5Qp4j3MIV+gauVkjDdiLPutTXGdxnvwN7tlId 4F5jKXpejvuORKCKw31w1NVj4tkSv+OfRywJNltD60B0fbEvkuo/ndsCOLqjMYyEgf5D hN9phmuYyGPvsDtkwFu9jzDGM6vpchqe/4qswylNvbYqXmQAqxqeDfYdVtO2yVBd4oL0 B2C53HZ8nK/dvlJEND3bmyT/x6/F5xO29Gl+FK2VmjgiPsmuqRcC3po3lmGB6a4nK9qg iG0iqYEsqaBVcSZkJBGqDf3/UBr87396LM11SwXBRP31Bzohl7Gfp5fhmrdoFX5t7hRD Pikg== X-Gm-Message-State: ALoCoQl7r++Wzn7SFgqX9oOmdGTIU5f1sqfh2xJ11Y38ys5eqvI2BqznG4WDXbXpRO7IMuII4zLO9n+EMpVBqlr4tKxXKBbE1g== MIME-Version: 1.0 X-Received: by 10.129.20.212 with SMTP id 203mr15284610ywu.68.1453117007169; Mon, 18 Jan 2016 03:36:47 -0800 (PST) Received: by 10.37.202.82 with HTTP; Mon, 18 Jan 2016 03:36:47 -0800 (PST) In-Reply-To: <568FB9AA.5020706@st.com> References: <568FB9AA.5020706@st.com> Date: Mon, 18 Jan 2016 11:36:00 -0000 Message-ID: Subject: Re: [PATCH][ARM,AARCH64] target/PR68674: relayout vector_types in expand_expr From: Richard Biener To: Christian Bruel Cc: kyrylo.tkachov@foss.arm.com, Richard Earnshaw , ramana.radhakrishnan@foss.arm.com, bschmidt@redhat.com, GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg01259.txt.bz2 On Fri, Jan 8, 2016 at 2:29 PM, Christian Bruel wrote: > When compiling code with attribute targets on arm or aarch64, > vector_type_mode returns different results (eg Vmode or BLKmode) depending > on the current simd flags that are not set between functions. > > for example the following code: > > #include > > extern int8x8_t a; > extern int8x8_t b; > > int16x8_t > __attribute__ ((target("fpu=neon"))) > foo(void) > { > return vaddl_s8 (a, b); > } > > Triggers gcc_asserts in copy_to_mode_regs while expanding NEON builtins , > because the mismatch and DECL_MODE current's TYPE_MODE used in > expand_builtin for global variables. > > but the best explanation is in the vector_type_mode: > /* Vector types need to re-check the target flags each time we report > the machine mode. We need to do this because attribute target can > change the result of vector_mode_supported_p and have_regs_of_mode > on a per-function basis. Thus the TYPE_MODE of a VECTOR_TYPE can > change on a per-function basis. */ > > I first tried to hack the 2 machine descriptions to insert convert_to_mode > or relayout_decls here and there, but I found this very fragile. Instead a > more central relayout the of type while expanding gave good results, as > proposed here. > > bootstraped and tested with no regression for arm, aarch64 and i586. > > Does this look to be the right approach ? > > nb: for testing this patch is complementary with > > https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00332.html > https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00248.html > > thanks for your comments. A x86 specific testcase that ICEs as well: typedef int v8si __attribute__((vector_size(32))); v8si a; v8si __attribute__((target("avx"))) foo() { return a; } in your patch not using the shared DECL_RTL of the global var "fixes" this so I think a conceptually better fix would be to "adjust" DECL_RTL from globals via a adjust_address (or so). Also given that we do /* ... fall through ... */ case FUNCTION_DECL: case RESULT_DECL: decl_rtl = DECL_RTL (exp); expand_decl_rtl: gcc_assert (decl_rtl); decl_rtl = copy_rtx (decl_rtl); thus always "unshare" DECL_RTL anyway it might be not so bad to simply do decl_rtl = adjust_address (decl_rtl, TYPE_MODE (type), 0); instead of that to avoid one copy. Index: expr.c =================================================================== --- expr.c (revision 232496) +++ expr.c (working copy) @@ -9597,7 +9597,10 @@ expand_expr_real_1 (tree exp, rtx target decl_rtl = DECL_RTL (exp); expand_decl_rtl: gcc_assert (decl_rtl); - decl_rtl = copy_rtx (decl_rtl); + if (MEM_P (decl_rtl)) + decl_rtl = adjust_address (decl_rtl, TYPE_MODE (type), 0); + else + decl_rtl = copy_rtx (decl_rtl); /* Record writes to register variables. */ if (modifier == EXPAND_WRITE && REG_P (decl_rtl) untested apart from on the x86_64 testcase (which it fixes). One could guard this further to only apply on vector typed decls with mismatched mode of course. I think that re-layouting globals is not very good design. Richard. > > > > > >