From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [IPv6:2a00:1450:4864:20::52f]) by sourceware.org (Postfix) with ESMTPS id 412233852221; Mon, 21 Nov 2022 22:27:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 412233852221 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x52f.google.com with SMTP id v8so7202999edi.3; Mon, 21 Nov 2022 14:27:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=q7aK0Rw5F8C9sYJCEHPPmGqTn5uldutfiVyT7wO2Tto=; b=hxrqrJJQYBIWzU3s7H/TuwQ/j/Fp701+YS4vyuhZvORflVDDm3e8t2ZHFhBcPcclNg oXFaHWRj1tqkwtRitFHFd+rL9Fyczwcr7rFFzNUynVXtAVHadp5VMtFnyL/WfpAc8tvq ByeOwteh1nM8Tqg/SpMbENndeDKQuvFH3plw5QvQZJx04W1LEsweYSpuH/zup8Nc2sH2 Z/AK2AMJTRPtL7pMnPguhR+Dau2J1UI52/zRbqNOK/mc5I2Fq5b29iV6S1yAxmVioeTO y+/2kVtr/CceZSS7VPINgelQeG1/BJK2tlwTcnF+ypgeE9WINfzOuRMnICB5+ftqhyNA ScaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=q7aK0Rw5F8C9sYJCEHPPmGqTn5uldutfiVyT7wO2Tto=; b=6z9nuF5khjSQaA+v7JWnz1F+1Tgdcl4oOqTP4//OyqEriidI7cyTPNB3JwweVn489u mLxdg60R4Pgkdlvih1opbFwbHWvlVeMn9oRYRQ3gcaq695deK/NIMXxdjRqBaf9gEklq 2Knjh9TCBwBCKI1Cp/w88+/MsEKbOh+FMxed7ViGNv+Yl6LcKq/hHS+Kll0FpYmDkBO2 XiW+s4uIg/5/wWUCKq/jfGag16MZ0lRvaI+wbU81LwZHE1PxXFfVxGC5nw6CZVrXX4Gj jkTxA2lcrcwa8N7Ktlca9/exZ7Dtqc3dk1fmnU1oghj4OFpVpAgZ+qG+Y0aX2/+Uny6N nyow== X-Gm-Message-State: ANoB5plZOF4opxAOAj2GDpBJEPX6gv3HxlCCFDXUIsiRysLKXVo3iPnC e9Eq+Sf8xs/bXYoQoXGX7sM= X-Google-Smtp-Source: AA0mqf6Ucy2r+hTcFUV/xX7yhsxLp5lETFda45pwrOrvrdFTEQWZvCDi48f/LjZFaeel1kgdkqwjzA== X-Received: by 2002:a05:6402:e04:b0:469:e6ef:9164 with SMTP id h4-20020a0564020e0400b00469e6ef9164mr25867edh.185.1669069650654; Mon, 21 Nov 2022 14:27:30 -0800 (PST) Received: from nbbrfq (80-110-214-113.static.upcbusiness.at. [80.110.214.113]) by smtp.gmail.com with ESMTPSA id g11-20020a170906594b00b007af75e6b6fesm5503990ejr.147.2022.11.21.14.27.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Nov 2022 14:27:30 -0800 (PST) Date: Mon, 21 Nov 2022 23:26:13 +0100 From: Bernhard Reutner-Fischer To: Mikael Morin Cc: rep.dot.nop@gmail.com, gcc-patches@gcc.gnu.org, Bernhard Reutner-Fischer , gfortran ML Subject: Re: [PATCH 2/2] Fortran: add attribute target_clones Message-ID: <20221121232613.66c79474@nbbrfq> In-Reply-To: References: <20221109190225.96037-1-aldot@gcc.gnu.org> <20221109190225.96037-3-aldot@gcc.gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3.8 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 21 Nov 2022 20:13:40 +0100 Mikael Morin wrote: > Hello, >=20 > Le 09/11/2022 =C3=A0 20:02, Bernhard Reutner-Fischer via Fortran a =C3=A9= crit=C2=A0: > > Hi! > >=20 > > Add support for attribute target_clones: > > !GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubrout= ine > > +/* Internal helper to parse attribute argument list. > > + If REQUIRE_STRING is true, then require a string. > > + If ALLOW_MULTIPLE is true, allow more than one arg. > > + If multiple arguments are passed, require braces around them. > > + Returns a tree_list of arguments or NULL_TREE. */ > > +static tree > > +gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple) > > + do { > > + } while (num_quotes % 2 && gfc_match_eos () !=3D MATCH_YES); =20 > The do-while loops are wrongly indented. > It should be: > do > { > ... > } > while (...) oops, right. > > + tree str =3D build_string (pos, name); > > + /* Compare with c-family/c-common.cc: fix_string_type. */ > > + tree i_type =3D build_index_type (size_int (pos)); > > + tree a_type =3D build_array_type (char_type_node, i_type); > > + TREE_TYPE (str) =3D a_type; > > + TREE_READONLY (str) =3D 1; > > + TREE_STATIC (str) =3D 1; > > + attr_arg =3D build_tree_list (NULL_TREE, str); > > + attr_args =3D chainon (attr_args, attr_arg); =20 > Same comment as for the flatten attribute: > please no tree stuff out of the trans-*.cc files. yes ok, noted. It's a pity in this context, where we purely pass a blob on to the ME but ok. > This includes gfortran.h, so the attribute arguments need to be carried=20 > around using the front-end structures (gfc_actual_arglist for example). That's a sane rule of thumb, yes. Usually, the parser deals with language grammar and not with pure passthrough remarks, so that's fair. Not so much in the case of such attribs but i see your point :) =20 > > + if (allow_multiple && gfc_match_char (')') !=3D MATCH_YES) > > + { > > + gfc_error ("expected ')' at %C"); > > + return NULL_TREE; > > + } > > + > > + return attr_args; > > +} =20 > I'm not sure this function need to do all the parsing manually. > I would rather use gfc_match_actual_arglist, or maybe implement the=20 > function as a wrapper around it. > What is allowed here? Are non-literal constants allowed, for example=20 > parameter variables? Is line continuation supported ? Line continuation is supported i think. Parameter variables supposedly are or should not be supported. Why would you do that in the context of an attribute target decl? Either way, if the ME does not find such an fndecl, it will complain and ignore the attribute. I don't understand non-literal constants in this context. This very attribute applies to decls, so the existing code supposedly matches a comma separated list of identifiers. The usual dollar-ok caveats apply. As to gfc_match_actual_arglist, probably. target_clones has + { "target_clones", 1, -1, true, false, false, false, + dummy, NULL }, with tree-core.h struct attribute_spec, so name, min=3D1, max=3Dunbounded, decl_required=3Dyes, ...ignore... hence applies to functions and subroutines and the like. It does take an unbounded list of strings, isa1, isa2, isa4, default. We could add "default" unless seen, but i'd rather want it spelled out by the user for the user is supposed to know what she's doing, as in c or c++. The ME has code to sanity-check the attributes, including conflicting (ME) attributes. The reason why i contemplated with a separate parser was that for stuff like regparm or sseregparm, you would want to require a single number for the equivalent of __attribute__((regparm(3),stdcall) which you would provide in 2 separate !GCC$ attributes i assume. >=20 > Nothing (bad) to say about the rest, but there is enough to change with=20 > the above comments. Yes, many thanks for your comments. I think there is no other non-intrusive way to pass the data through the frontend. So for an acceptable way this means touching quite some spots for every single ME attribute anybody would like to add in the future. But that's how it is.