From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 07E233858402 for ; Mon, 18 Oct 2021 15:36:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 07E233858402 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-402-orEvG0r2OyGoSa_4sDKTeg-1; Mon, 18 Oct 2021 11:36:51 -0400 X-MC-Unique: orEvG0r2OyGoSa_4sDKTeg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D93901019983; Mon, 18 Oct 2021 15:36:49 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 728E93CC7; Mon, 18 Oct 2021 15:36:49 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 19IFakpo2439512 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 18 Oct 2021 17:36:46 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 19IFajKp2439511; Mon, 18 Oct 2021 17:36:45 +0200 Date: Mon, 18 Oct 2021 17:36:44 +0200 From: Jakub Jelinek To: Qing Zhao Cc: Richard Biener , gcc-patches Nick Alcock via Subject: Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers. Message-ID: <20211018153644.GB304296@tucnak> Reply-To: Jakub Jelinek References: <89606056-4D5A-467B-A824-9163D882C35A@oracle.com> MIME-Version: 1.0 In-Reply-To: <89606056-4D5A-467B-A824-9163D882C35A@oracle.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Oct 2021 15:36:54 -0000 On Mon, Oct 18, 2021 at 03:04:40PM +0000, Qing Zhao wrote: > 2021-10-16 qing zhao > > * gimplify.c (gimplify_decl_expr): Do not add call to > __BUILTIN_CLEAR_PADDING when a variable is a gimple register The builtin is called __builtin_clear_padding, using __BUILTIN_CLEAR_PADDING makes no sense, either use the lower-case version of it, or use BUILT_IN_CLEAR_PADDING which is the enum built_in_function enumerator used to refer to it in the compiler. > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -1954,8 +1954,14 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p) > pattern initialization. > In order to make the paddings as zeroes for pattern init, We > should add a call to __builtin_clear_padding to clear the > - paddings to zero in compatiple with CLANG. */ > - if (flag_auto_var_init == AUTO_INIT_PATTERN) > + paddings to zero in compatiple with CLANG. > + We cannot insert this call if the variable is a gimple register > + since __BUILTIN_CLEAR_PADDING assumes the variable is in memory. Likewise (and please fix the other spots in gimplify.c that do that too. Furthermore, __builtin_clear_padding doesn't assume anything, but it takes an address of an object as argument and already the taking of the address that gimple_add_padding_init_for_auto_var does makes the var TREE_ADDRESABLE, which is something that needs to be done before the var is ever accessed during gimplification. > + As a result, if a long double/Complex long double variable will Please change Complex to _Complex > + spilled into stack later, its padding is 0XFE. */ > + if (flag_auto_var_init == AUTO_INIT_PATTERN > + && !is_gimple_reg (decl) > + && clear_padding_type_may_have_padding_p (TREE_TYPE (decl))) > gimple_add_padding_init_for_auto_var (decl, is_vla, seq_p); > } > } > @@ -5388,8 +5394,15 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > initialize paddings of object always to zero regardless of > INIT_TYPE. Note, we will not insert this call if the aggregate > variable has be completely cleared already or it's initialized > - with an empty constructor. */ > + with an empty constructor. We cannot insert this call if the > + variable is a gimple register since __BUILTIN_CLEAR_PADDING assumes > + the variable is in memory. As a result, if a long double/Complex long Likewise. > + double variable will be spilled into stack later, its padding cannot > + be cleared with __BUILTIN_CLEAR_PADDING. we should clear its padding s/we/We/ > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr102281.c > @@ -0,0 +1,15 @@ > +/* PR102281 */ > +/* { dg-do compile } */ > +/* { dg-options "-ftrivial-auto-var-init=zero" } */ > +long _mm_set_epi64x___q0; > +__attribute__((__vector_size__(2 * sizeof(long)))) long long _mm_set_epi64x() { > + return (__attribute__((__vector_size__(2 * sizeof(long)))) long long){ > + _mm_set_epi64x___q0}; > +} > + > +float _mm_set1_ps___F; > +__attribute__((__vector_size__(4 * sizeof(float)))) float > +__attribute___mm_set1_ps() { > + return (__attribute__((__vector_size__(4 * sizeof(float)))) float){ > + _mm_set1_ps___F}; > +} If it is a generic testcase, please change the variable and function names so that they don't look like x86 intrinsics, using v, w or var1, var2 etc. instead of _mm_set_epi64x___q0 or _mm_set1_ps___F, and foo/bar instead of _mm_set_epi64x or __attribute___mm_set1_ps will make it certainly more readable. Please add typedefs for the vector types, typedef long long V __attribute__((__vector_size__ (2 * sizeof (long long)))); typedef float W __attribute__((__vector_size__ (4 * sizeof (float)))); and use that (note that I've changed the long in there to long long to make it match. And please use (void) instead of () in the function declarations. Also, the testcase will likely fail on x86-64 with RUNTESTFLAGS='--target_board=unix\{-m32/-mno-sse,-m32/-msse2,-m64\} dg.exp=pr102281.c' because of -Wpsabi warnings. So, you likely want -Wno-psabi or even -Wno-psabi -w in dg-options. Jakub