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 C9A15384406C for ; Tue, 27 Jul 2021 08:19:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C9A15384406C Received: by mail-ed1-x529.google.com with SMTP id h8so14257585ede.4 for ; Tue, 27 Jul 2021 01:19:54 -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=Zg7pGXQ7QEBfJ6gHPXyrmG1xG5EuUFsBlxv9utOSXFU=; b=HZ5AZe+KMaDLpoDW6D0i1z6rtW6MgbsoMwzuts1lzOiRG8v2DwM5VIBChJco8VB6aF TX22SDCzu3iZM1GUQI20ZvyM3M55+u9XbB6qlai6yO3ezpZefX6Za99tKYvdcg+Dv/Rw 3f97EK60FF1aCXwMMaER6eTMePBuU2+dtEHzq5hPCvsyRh7RCx0nFeMv5ZRWmoEhQqlN k9JNAahiqPK2vEaW6D03SveAiTtjpjMZdW7rGlTRjZpONYpkRl8PXrwDro4UlHFkJz3b Y6pXEr0ZeZ+Fr/hdq6FhwA+24xxlap0O/WrfoFiybW3JStspLjALS2e1G8rNw6yRb+it aiGA== X-Gm-Message-State: AOAM532i/YrQt1ffuK2NWg2Sy/dbT+hUkCQ05CSXF/eywJ0X/diz70h8 PV84Y2ukeVC1odK4JXaYZRYveEr77JGyvGTAhi8= X-Google-Smtp-Source: ABdhPJzHRSjCf6IGWBBl7Kv7/fe7keBBnQYuD+KkntUa0ytvYeYBgV9FrxLj/+/4bTcWIKhsL+1v/XwKmKfmI/58cQA= X-Received: by 2002:a05:6402:31a6:: with SMTP id dj6mr27042764edb.248.1627373993799; Tue, 27 Jul 2021 01:19:53 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Tue, 27 Jul 2021 10:19:42 +0200 Message-ID: Subject: Re: [RFC] Adding a new attribute to function param to mark it as constant To: Prathamesh Kulkarni Cc: Andrew Pinski , GCC Development , Richard Earnshaw Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Jul 2021 08:19:56 -0000 On Mon, Jul 26, 2021 at 11:06 AM Prathamesh Kulkarni via Gcc wrote: > > On Fri, 23 Jul 2021 at 23:29, Andrew Pinski wrote: > > > > On Fri, Jul 23, 2021 at 3:55 AM Prathamesh Kulkarni via Gcc > > wrote: > > > > > > Hi, > > > Continuing from this thread, > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575920.html > > > The proposal is to provide a mechanism to mark a parameter in a > > > function as a literal constant. > > > > > > Motivation: > > > Consider the following intrinsic vshl_n_s32 from arrm/arm_neon.h: > > > > > > __extension__ extern __inline int32x2_t > > > __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) > > > vshl_n_s32 (int32x2_t __a, const int __b) > > > { > > > return (int32x2_t)__builtin_neon_vshl_nv2si (__a, __b); > > > } > > > > > > and it's caller: > > > > > > int32x2_t f (int32x2_t x) > > > { > > > return vshl_n_s32 (x, 1); > > > } > > > > Can't you do similar to what is done already in the aarch64 back-end: > > #define __AARCH64_NUM_LANES(__v) (sizeof (__v) / sizeof (__v[0])) > > #define __AARCH64_LANE_CHECK(__vec, __idx) \ > > __builtin_aarch64_im_lane_boundsi (sizeof(__vec), > > sizeof(__vec[0]), __idx) > > > > ? > > Yes this is about lanes but you could even add one for min/max which > > is generic and such; add an argument to say the intrinsics name even. > > You could do this as a non-target builtin if you want and reuse it > > also for the aarch64 backend. > Hi Andrew, > Thanks for the suggestions. IIUC, we could use this approach to check > if the argument > falls within a certain range (min / max), but I am not sure how it > will help to determine > if the arg is a constant immediate ? AFAIK, vshl_n intrinsics require > that the 2nd arg is immediate ? > > Even the current RTL builtin checking is not consistent across > optimization levels: > For eg: > int32x2_t f(int32_t *restrict a) > { > int32x2_t v = vld1_s32 (a); > int b = 2; > return vshl_n_s32 (v, b); > } > > With pristine trunk, compiling with -O2 results in no errors because > constant propagation replaces 'b' with 2, and during expansion, > expand_builtin_args is happy. But at -O0, it results in the error - > "argument 2 must be a constant immediate". > > So I guess we need some mechanism to mark a parameter as a constant ? I guess you want to mark it in a way that the frontend should force constant evaluation and error if that's not possible? C++ doesn't allow to declare a parameter as 'constexpr' but something like void foo (consteval int i); since I guess you do want to allow passing constexpr arguments in C++ or in C extended forms of constants like static const int a[4]; foo (a[1]); ? But yes, this looks useful to me. Richard. > > Thanks, > Prathamesh > > > > Thanks, > > Andrew Pinski > > > > > > > > The constraint here is that, vshl_n intrinsics require that the > > > second arg (__b), > > > should be an immediate value. > > > Currently, this check is performed by arm_expand_builtin_args, and if > > > a non-constant > > > value gets passed, it emits the following diagnostic: > > > > > > ../armhf-build/gcc/include/arm_neon.h:4904:10: error: argument 2 must > > > be a constant immediate > > > 4904 | return (int32x2_t)__builtin_neon_vshl_nv2si (__a, __b); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > However, we're trying to replace builtin calls with gcc's C vector > > > extensions where > > > possible (PR66791), because the builtins are opaque to the optimizers. > > > > > > Unfortunately, we lose type checking of immediate value if we replace > > > the builtin > > > with << operator: > > > > > > __extension__ extern __inline int32x2_t > > > __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) > > > vshl_n_s32 (int32x2_t __a, const int __b) > > > { > > > return __a << __b; > > > } > > > > > > So, I was wondering if we should have an attribute for a parameter to > > > specifically > > > mark it as a constant value with optional range value info ? > > > As Richard suggested, sth like: > > > void foo(int x __attribute__((literal_constant (min_val, max_val))); > > > > > > Thanks, > > > Prathamesh