From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x529.google.com (mail-ed1-x529.google.com [IPv6:2a00:1450:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id B5416389441E for ; Thu, 22 Apr 2021 12:55:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B5416389441E Received: by mail-ed1-x529.google.com with SMTP id j12so28297418edy.3 for ; Thu, 22 Apr 2021 05:55:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=X7FIAEI4uKisltqtMXId8/yAMOYa3aFnkQhHV69SYa0=; b=seQDko4vrgPwzNFSuNqL5+0PXP+e3jwCxXEiQc4U0DVv+/FfW7g2WEHGDbkPspGuUM OBTZCoIP+uDST6ZiyfXJO+53n2462G9EqBCSFCF6htY70ZKBq0QL4tm6WfCRvhr8Rt5Q 79on7jNkIYZyEcTcKSIUeOsX9kez2PxJgqbEsDPUzwalyTi/S6wa1KMfaNsXb7YkrpuG tNPBOA8vmjzxIe2vCIfnmYG8cCm/0xazsRNGEqI4lSh7aDkwy/6tnFUR+Bz/Wlsa+mCP eIKOLu/Q5XKY5lglD9UmuYlhAyQEF5F6B8mR+59KuuPvV55GEIkVTeGCsH4Lb81b0xLn X/Qw== X-Gm-Message-State: AOAM532SiLSvvIBbnwEw/uO3UvnTYhDC/WPlbUiaOd3drPcaXFhnRLAa 9kvqmlkH7WK9450kw9CfPoc7kZCWcooLLosfHZf669OY0oc= X-Google-Smtp-Source: ABdhPJzxd/v+RsevE7sfJgOcXZnBghYlqq6ttWVsDutMdMnEcoe/WRzs78+5Rw1e3gzasCp2BZSNeW/mHRXoNk/yGMg= X-Received: by 2002:a05:6402:b66:: with SMTP id cb6mr3771923edb.248.1619096141637; Thu, 22 Apr 2021 05:55:41 -0700 (PDT) MIME-Version: 1.0 References: <20210414223918.230495-1-hjl.tools@gmail.com> <20210414223918.230495-3-hjl.tools@gmail.com> <20210422090201.GO1179226@tucnak> <20210422122202.GP1179226@tucnak> In-Reply-To: From: Richard Biener Date: Thu, 22 Apr 2021 14:55:30 +0200 Message-ID: Subject: Re: [PATCH v4 2/2] x86: Add general_regs_only function attribute To: Jakub Jelinek Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 22 Apr 2021 12:55:44 -0000 On Thu, Apr 22, 2021 at 2:52 PM Richard Biener wrote: > > On Thu, Apr 22, 2021 at 2:22 PM Jakub Jelinek wrote: > > > > On Thu, Apr 22, 2021 at 01:23:20PM +0200, Richard Biener via Gcc-patches wrote: > > > > The question is if the pragma GCC target right now behaves incrementally > > > > or not, whether > > > > #pragma GCC target("avx2") > > > > adds -mavx2 to options if it was missing before and nothing otherwise, or if > > > > it switches other options off. If it is incremental, we could e.g. try to > > > > use the second least significant bit of global_options_set.x_* to mean > > > > this option has been set explicitly by some surrounding #pragma GCC target. > > > > The normal tests - global_options_set.x_flag_whatever could still work > > > > fine because they wouldn't care if the option was explicit from anywhere > > > > (command line or GCC target or target attribute) and just & 2 would mean > > > > it was explicit from pragma GCC target; though there is the case of > > > > bitfields... And then the inlining decision could check the & 2 flags to > > > > see what is required and what is just from command line. > > > > Or we can have some other pragma GCC that would be like target but would > > > > have flags that are explicit (and could e.g. be more restricted, to ISA > > > > options only, and let those use in addition to #pragma GCC target. > > > > > > I'm still curious as to what you think will break if always-inline does what > > > it is documented to do. > > > > We will silently accept calling intrinsics that must be used only in certain > > ISA contexts, which will lead to people writing non-portable code. > > > > So -O2 -mno-avx > > #include > > > > void > > foo (__m256 *x) > > { > > x[0] = _mm256_sub_ps (x[1], x[2]); > > } > > etc. will now be accepted when it shouldn't be. > > clang rejects it like gcc with: > > 1.c:6:10: error: always_inline function '_mm256_sub_ps' requires target feature 'avx', but would be inlined into function 'foo' that is compiled without support for 'avx' > > x[0] = _mm256_sub_ps (x[1], x[2]); > > ^ > > > > Note, if I do: > > #include > > > > __attribute__((target ("no-sse3"))) void > > foo (__m256 *x) > > { > > x[0] = _mm256_sub_ps (x[1], x[2]); > > } > > and compile > > clang -S -O2 -mavx2 1.c > > 1.c:6:10: error: always_inline function '_mm256_sub_ps' requires target feature 'avx', but would be inlined into function 'foo' that is compiled without support for 'avx' > > x[0] = _mm256_sub_ps (x[1], x[2]); > > ^ > > then from the error message it seems that unlike GCC, clang remembers > > the exact target features that are needed for the intrinsics and checks just > > those. > > Though, looking at the preprocessed source, seems it uses > > static __inline __m256 __attribute__((__always_inline__, __nodebug__, __target__("avx"), __min_vector_width__(256))) > > _mm256_sub_ps(__m256 __a, __m256 __b) > > { > > return (__m256)((__v8sf)__a-(__v8sf)__b); > > } > > and not target pragmas. > > > > Anyway, if we tweak our intrinsic headers so that > > -#ifndef __AVX__ > > #pragma GCC push_options > > #pragma GCC target("avx") > > -#define __DISABLE_AVX__ > > -#endif /* __AVX__ */ > > > > ... > > -#ifdef __DISABLE_AVX__ > > -#undef __DISABLE_AVX__ > > #pragma GCC pop_options > > -#endif /* __DISABLE_AVX__ */ > > and do the opts_set->x_* & 2 stuff on explicit options coming out of > > target/optimize pragmas and attributes, perhaps we don't even need > > to introduce a new attribute and can handle everything magically: Oh, and any such changes will likely interact with Martins ideas to rework how optimize and target attributes work (aka adding ontop of the commandline options). That is, attribute target will then not be enough to remember the exact set of needed ISA features (as opposed to what likely clang implements?) > > 1) if it is gnu_inline extern inline, allow indirect calls, otherwise > > disallow them for always_inline functions > > There are a lot of intrinsics using extern inline __gnu_inline though... > > > 2) for the isa flags and option mismatches, only disallow opts_set->x_* & 2 > > stuff > > This will keep both intrinsics and glibc fortify macros working fine > > in all the needed use cases. > > Yes, see my example in the other mail. > > I think before we add any new attributes we should sort out the > current mess, eventually adding some testcases for desired > diagnostic. > > Richard. > > > Jakub > >