From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2062.outbound.protection.outlook.com [40.107.21.62]) by sourceware.org (Postfix) with ESMTPS id B93433857732 for ; Tue, 6 Jun 2023 12:01:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B93433857732 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=39kCtyACPrgPVJFxtCT+K3uIZvv7k9AMM6ITCoDMPWY=; b=yWUKXkhsUXtrcgiP5P1dnKGDqmEyG16GPu3e5w0YlLm6wBfEfIHXllzjtq4YdSUrxT9zodZM2x0MBPN0Fdc87lzHOKvWYJ1PGtnMWFx0j4QMwXyh5N4t1KjXmWo01BDYToevSbV60Tuni7aD6NFotqrSGi7/wvK2/1Psh5lrIm0= Received: from AS9PR05CA0205.eurprd05.prod.outlook.com (2603:10a6:20b:495::12) by AS8PR08MB8733.eurprd08.prod.outlook.com (2603:10a6:20b:565::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.32; Tue, 6 Jun 2023 12:01:08 +0000 Received: from AM7EUR03FT020.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:495:cafe::b6) by AS9PR05CA0205.outlook.office365.com (2603:10a6:20b:495::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.33 via Frontend Transport; Tue, 6 Jun 2023 12:01:08 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM7EUR03FT020.mail.protection.outlook.com (100.127.140.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6477.19 via Frontend Transport; Tue, 6 Jun 2023 12:01:08 +0000 Received: ("Tessian outbound 5bb4c51d5a1f:v136"); Tue, 06 Jun 2023 12:01:08 +0000 X-CR-MTA-TID: 64aa7808 Received: from d86b25db9454.3 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 731ACCE3-2298-4F29-92AC-0239263104CE.1; Tue, 06 Jun 2023 12:00:58 +0000 Received: from EUR05-AM6-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id d86b25db9454.3 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 06 Jun 2023 12:00:58 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AE/koYe7ruuqCbuvq/RaLho6r4x2TVca68u8qdXh3DqlEl2NJnvyfnsrGxlmHJ3fdn95CK8gK/aJNmWiK64CZpP2yjK7lk8zuGCLEsvQARRzyrO/rCywarXonTmuApBT9o0Cq03v1VvLEbTW7zsVWMHb74pdWR11q3DFeDuOBAnaMVhyXdQIWpqi4SyYHAFG8uEDvSE+LLuWAT5Jst4xGT74kTdP3wHSLFlg77ZvsYlbHfHo3Hs3bYP+jJp4JRa1mo+TVO2D39q0BLspswNhY1ywvTGv3nL2NN92Hr9IjVrMn2SvJDT4nD+YAF5PLXSehYCT4gfoIqEIFxDXTkVkAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=39kCtyACPrgPVJFxtCT+K3uIZvv7k9AMM6ITCoDMPWY=; b=VPxYjNQqkRo1vGruDJ1qv1ywg66PSg/ct7tZ0BfWvrx0HYIIavXBmm95DbkS3G9PXkif/BAPq4tvy7fkpuYFula89X7tfrT7sRlEe147mEAY08uCDtboUmi9nezywPe3ebJUoWgKJZzIsFh+tBjI8Q4LKmHX6PPVQd0VnXRoHuhCai76icxXWwDD5k3IrdPCBryQ1kvaajMC1eGDiyKQeU5UHPK+pqz08BIBn41mq3gut5biFpxnu8mdsbAWcf6kpleR6vQ1JOE7MZN1oXyIjW0lSjePIU25qMtPLvTUewMbvjEo2Pd7cpJNuKuhnkHe395e5qbq2VRd2iDP1qCodQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=39kCtyACPrgPVJFxtCT+K3uIZvv7k9AMM6ITCoDMPWY=; b=yWUKXkhsUXtrcgiP5P1dnKGDqmEyG16GPu3e5w0YlLm6wBfEfIHXllzjtq4YdSUrxT9zodZM2x0MBPN0Fdc87lzHOKvWYJ1PGtnMWFx0j4QMwXyh5N4t1KjXmWo01BDYToevSbV60Tuni7aD6NFotqrSGi7/wvK2/1Psh5lrIm0= Received: from VI1PR08MB5325.eurprd08.prod.outlook.com (2603:10a6:803:13e::17) by PAWPR08MB10060.eurprd08.prod.outlook.com (2603:10a6:102:35a::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.32; Tue, 6 Jun 2023 12:00:55 +0000 Received: from VI1PR08MB5325.eurprd08.prod.outlook.com ([fe80::2301:1cde:cfe7:eaf0]) by VI1PR08MB5325.eurprd08.prod.outlook.com ([fe80::2301:1cde:cfe7:eaf0%6]) with mapi id 15.20.6455.030; Tue, 6 Jun 2023 12:00:55 +0000 From: Tamar Christina To: Richard Sandiford CC: "gcc-patches@gcc.gnu.org" , nd , Richard Earnshaw Subject: RE: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions. Thread-Topic: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions. Thread-Index: AQHZl+08kf326nq/x0q2PMtkKgB+m699qeyA Date: Tue, 6 Jun 2023 12:00:55 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: 9F1ACC1D7D450C4F80644D6746BFFCAC.0 x-checkrecipientchecked: true Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; x-ms-traffictypediagnostic: VI1PR08MB5325:EE_|PAWPR08MB10060:EE_|AM7EUR03FT020:EE_|AS8PR08MB8733:EE_ X-MS-Office365-Filtering-Correlation-Id: 91614407-4347-43ea-f246-08db6685b6f3 x-checkrecipientrouted: true nodisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: rl5HxvSLvwcWy0OVMbOOWA8FGc2/C60RyG1rLlhjo1d8j1CoRtPyzBBZwl2Rt794zXVpLOzKvpTXHkLs/PQzFnS+tcsMyDfEhHDtU/rBFJg7l2bFrYjcddvienvUTbk16aeek50NnIVxkVDynBqcvW6bnhkgSFXL73Kb6VKAlKCuXM/5eFr42P0ZHHhqOxnRC8u/WlZNot6dMoxwCE1R2K/yoGT+k8qJsRE9UvtdgzmhB14183Fdo1XVnhsUDpuzp9buw3YVNP2VfholUukVL5MCaFCUBMLrW/xZDoHF1qYp8/YEDIow+iZrxvhbbiLUkr33X2k9FJjM7n0hxucDINA5/+Ga8Dohxse2MFPC5XtpseFpcKDpmblMFeyTdyu1MswCsKkZ4y+QAGxA0GkYiTKDhZ4Uy0xcnWHQcoH7V895384aCHXLQ6ZjrnMxI+70SrkayswIRsZNMY8KjmOZmTkH6i4/ReVaqNycUz9T5rY7EIyPlBavDc3FRFddqqfA8CqLYN9kJlYC43cDktTEEh3KVVmufSspte1xaF9rn890Xg5+9V4YIpDnRiJdYKliKZvCWxQYZAQ37LmYqtFvU/DtZD7aLmvO2chaXdjxEkhS2aiDGgE1Yv868tYHmAPG X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR08MB5325.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(376002)(346002)(136003)(396003)(39860400002)(366004)(451199021)(41300700001)(316002)(54906003)(122000001)(52536014)(5660300002)(478600001)(6636002)(6862004)(2906002)(30864003)(4326008)(76116006)(66556008)(66476007)(64756008)(66446008)(8936002)(8676002)(66946007)(71200400001)(38070700005)(7696005)(86362001)(186003)(38100700002)(26005)(55016003)(9686003)(33656002)(6506007)(83380400001);DIR:OUT;SFP:1101; Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB10060 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM7EUR03FT020.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 965a0b5e-4485-450d-1f2b-08db6685aec8 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TQKsknafYViQhQKAnzQQKqWgJXHxXgkVPw4EjPGkURFZ7JVEVU6JWfQlC64LJKCYRKFoccUbBRSm+m0bL0yVBV0Jou8frTSbzhMAks75mEvUTP/upOH6VPsPfcu1hiKafsmCJ4K+bu0xPi5eOOuXxqcYpT+WTnSfAraCfj3ZpnKCU+Ij1MWXBRz4VLcFVJlpcBtZc8pm8pwz4IojeyV1UZESxxh+T2ahZ5ta+ULatuCLX8UvIDM9PQF2CwE5mCNNVEiGmAPnhetdOToON9ptO8v99tJVHOFYx3v4Y0vc2HomISN8WnNrUmHCBAgkuikY0RLP/IRRCTcwP9+37GxlbcbYSMejDs1XZHuQcyH4mrN+8YcTbTluhjq/z2vILzP7Q7OX760Z/qDTxUTRMQTBM3OpMyvcIS4+blHyq9On6AX6txE+7oSEzQ2EhX67sN0wK8Hup5yQrBuXIfnHQUDsSZ5LL7SQEZSdKNQ9an5ZvQvgc5z/h2JNWr9+E+H0IIvu4IGtROmh3pNz9KoAGw/Di0S8yA771TAL3BV5MiWCaH6eiMzbWRb2AvqYZgRY38rWOFbvDHnX3l3hnbhio441hfuL75Oz1gISYVyiUPUPpTtcDIibHS0+LXM1lZUvn1c9l2YKpyN5RvgRuonMkDjTcr577cf5NeMY/OcqFroEj6J6iMf3D9SPdBZtUIj9AWpsdq6HdOVuWzEBnyU7NtJNqdpFo1/4fcS2CzpjP8SJ6EQefKlbdPuwyx9PTRBG1IuL X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230028)(4636009)(39860400002)(346002)(136003)(376002)(396003)(451199021)(46966006)(40470700004)(36840700001)(47076005)(26005)(9686003)(6506007)(83380400001)(41300700001)(36860700001)(7696005)(336012)(186003)(40460700003)(478600001)(54906003)(2906002)(6636002)(82740400003)(70586007)(82310400005)(40480700001)(55016003)(356005)(316002)(70206006)(81166007)(4326008)(52536014)(8936002)(5660300002)(6862004)(8676002)(33656002)(30864003)(86362001);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2023 12:01:08.8400 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 91614407-4347-43ea-f246-08db6685b6f3 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM7EUR03FT020.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB8733 X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,FORGED_SPF_HELO,KAM_DMARC_NONE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,UNPARSEABLE_RELAY autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, Thanks for the review, just some quick responses before I make the changes: > > int operand_number; /* Operand index in the big array. */ > > int output_format; /* INSN_OUTPUT_FORMAT_*. */ > > + bool compact_syntax_p; > > struct operand_data operand[MAX_MAX_OPERANDS]; }; > > > > @@ -700,12 +702,57 @@ process_template (class data *d, const char > *template_code) > > if (sp !=3D ep) > > message_at (d->loc, "trailing whitespace in output template"); > > > > - while (cp < sp) > > + /* Check for any unexpanded iterators. */ > > + if (bp[0] !=3D '*' && d->compact_syntax_p) >=20 > I assume the bp[0] !=3D '*' condition skips the check for C code blocks. > Genuine question, but are you sure we want that? C code often includes a= sm > strings (in quotes), such as for the SVE CNT[BHWD] example. >=20 > Extending the check would mean that any use of <...> for C++ templates wi= ll > need to be quoted, but explicit instantiation is pretty rare in .md files= . It would > also look weird for conditions. >=20 > Either way is fine, just asking. I excluded it entirely to avoid also running afoul of the binary operators.= So e.g. * a < b && b > c ? foo : bar shouldn't trigger it. It seemed more trouble= than it's worth to try to get correct. > > + } > > + > > + /* Adds a character to the end of the string. */ void add (char > > + c) { > > + con +=3D c; > > + } > > + > > + /* Output the string in the form of a brand-new char *, then effecti= vely > > + clear the internal string by resetting len to 0. */ char * out > > + () >=20 > Formatting: no need for a space before "out". >=20 > > + { > > + /* Final character is always a trailing comma, so strip it out. > > + */ >=20 > trailing ',', ';' or ']', rather than just a comma? Ah no, this is a bit of a lazy intercalate, when the alternatives are pushe= d in it's not easy to tell how many there will be (because we don't keep track of it = in this part), so we just always add a trailing "," and ignore the last char on output. V= alidation of the alternative counts themselves is done later by the normal machinery. >=20 > > + char * q; >=20 > Similarly no space before "q" here. >=20 > > + if (modifier.empty ()) > > + q =3D xstrndup (con.c_str (), con.size () - 1); >=20 > Could just be "xstrdup (con.c_str ())". >=20 > > + else > > + { > > + int len =3D con.size () + modifier.size (); > > + q =3D XNEWVEC (char, len); > > + strncpy (q, modifier.c_str (), modifier.size ()); > > + strncpy (q + modifier.size (), con.c_str (), con.size ()); > > + q[len -1] =3D '\0'; > > + } >=20 > Do we need the separation between "modifier" and "cons"? It looks like t= he > code completes the initialisation of "modifier" before it writes to "cons= ", and > so we could just use a single string. Fair point. > > + { > > + if (XSTR (part, 1) && XSTR (part, 1)[0] !=3D '\0') > > + { > > + error_at (loc, "can't mix normal and compact attribute syntax")= ; > > + break; > > + } > > + XSTR (part, 1) =3D attrs[index].out (); > > + > > + ++index; > > + if (index =3D=3D attrs.size ()) > > + break; > > + } >=20 > It looks like you forgive mixing new-style and old-style syntax, since th= ere's no > "else error" here. But the documentation said that that wasn't allowed. >=20 > Either way seems OK to me, but see the next comment. >=20 > > + } > > + > > + return index; > > +} > > + > > +/* Modify the attributes list to make space for the implicitly declare= d > > + attributes in the attrs: list. */ > > + > > +static void > > +create_missing_attributes (rtx x, file_location /* loc */, > > +vec_conlist &attrs) { > > + if (attrs.empty ()) > > + return; > > + > > + unsigned int attr_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 4 : 3; > > + vec_conlist missing; > > + > > + /* This is an O(n*m) loop but it's fine, both n and m will always be= very > > + small. */ >=20 > Agreed that quadraticness isn't a problem. But I wonder how many people > would write an explicit placeholder set_attr. Unlike match_operand and > match_scratch, a placeholder set_attr doesn't carry any additional > information. >=20 > It might be simpler to drop add_attributes and add all attributes > unconditionally in this function instead. If the user tries to specify t= he same > attribute using both syntaxes, the pattern would end up with two definiti= ons > of the same attribute, which ought to be flagged by existing code. >=20 This was done to support the (in arm backend) common thing of having attrib= utes which are either too complex to add inline in the new syntax or that just r= epeat a value. i.e. it's to allow cases like this: [(set_attr "length") (set_attr "predicable" "yes") (set_attr "predicable_short_it") (set_attr "arch") (set (attr "type") (if_then_else (match_operand 2 "const_int_operand" ""= ) (const_string "alu_imm") (const_string "alu_sreg"))) Where your attrs contains: {@ [cons: =3D0, 1, 2; attrs: length, predicable_short_it, arch] However you're right, I could simply say that you must omit the set_attr in= attrs and just merge the two lists? I think that's what you were alluding to? Cheers, Tamar > > + for (conlist cl : attrs) > > + { > > + bool found =3D false; > > + for (int i =3D 0; XVEC (x, attr_index) && i < XVECLEN (x, attr_i= ndex); ++i) > > + { > > + rtx part =3D XVECEXP (x, attr_index, i); > > + > > + if (GET_CODE (part) !=3D SET_ATTR > > + || cl.name.compare (XSTR (part, 0)) =3D=3D 0) > > + { > > + found =3D true; > > + break; > > + } > > + } > > + > > + if (!found) > > + missing.push_back (cl); > > + } > > + > > + rtvec orig =3D XVEC (x, attr_index); > > + size_t n_curr =3D orig ? XVECLEN (x, attr_index) : 0; rtvec copy = =3D > > + rtvec_alloc (n_curr + missing.size ()); > > + > > + /* Create a shallow copy of existing entries. */ memcpy > > + (©->elem[missing.size ()], &orig->elem[0], sizeof (rtx) * > > + n_curr); XVEC (x, attr_index) =3D copy; > > + > > + /* Create the new elements. */ > > + for (unsigned i =3D 0; i < missing.size (); i++) > > + { > > + rtx attr =3D rtx_alloc (SET_ATTR); > > + XSTR (attr, 0) =3D xstrdup (attrs[i].name.c_str ()); > > + XSTR (attr, 1) =3D NULL; > > + XVECEXP (x, attr_index, i) =3D attr; > > + } > > + > > + return; > > +} > > + > > +/* Consumes spaces and tabs. */ > > + > > +static inline void > > +skip_spaces (const char **str) > > +{ > > + while (**str =3D=3D ' ' || **str =3D=3D '\t') >=20 > ISSPACE here too. >=20 > > + (*str)++; > > +} > > + > > +/* Consumes the given character, if it's there. */ > > + > > +static inline bool > > +expect_char (const char **str, char c) { > > + if (**str !=3D c) > > + return false; > > + (*str)++; > > + return true; > > +} > > + > > +/* Parses the section layout that follows a "{@}" if using new syntax.= Builds > > + a vector for a single section. E.g. if we have "attrs: length arch)= ..." > > + then list will have two elements, the first for "length" and the se= cond > > + for "arch". */ > > + > > +static void > > +parse_section_layout (const char **templ, const char *label, > > + vec_conlist &list, bool numeric) { > > + const char *name_start; > > + size_t label_len =3D strlen (label); > > + if (strncmp (label, *templ, label_len) =3D=3D 0) > > + { > > + *templ +=3D label_len; > > + > > + /* Gather the names. */ > > + while (**templ !=3D ';' && **templ !=3D ']') > > + { > > + skip_spaces (templ); > > + name_start =3D *templ; > > + int len =3D 0; > > + char val =3D (*templ)[len]; > > + while (val !=3D ',' && val !=3D ';' && val !=3D ']') > > + val =3D (*templ)[++len]; > > + *templ +=3D len; > > + if (val =3D=3D ',') > > + (*templ)++; > > + list.push_back (conlist (name_start, len, numeric)); > > + } > > + } > > +} > > + > > +/* Parse a section, a section is defined as a named space separated li= st, e.g. > > + > > + foo: a b c >=20 > Now comma-separated rather than space-separated. Applies to the example > too. >=20 > > + > > + is a section named "foo" with entries a,b and c. */ > > + > > +static void > > +parse_section (const char **templ, unsigned int n_elems, unsigned int > alt_no, > > + vec_conlist &list, file_location loc, const char *name) { > > + unsigned int i; > > + > > + /* Go through the list, one character at a time, adding said charact= er > > + to the correct string. */ > > + for (i =3D 0; **templ !=3D ']' && **templ !=3D ';'; (*templ)++) > > + { > > + if (**templ !=3D ' ' && **templ !=3D '\t') >=20 > !ISSPACE >=20 > > + { > > + list[i].add(**templ); >=20 > Formatting: should be a space before "(". >=20 > > + if (**templ =3D=3D ',') > > + { > > + ++i; > > + if (i =3D=3D n_elems) > > + fatal_at (loc, "too many %ss in alternative %d: expected %d", > > + name, alt_no, n_elems); > > + } > > + } > > + } > > + > > + if (i + 1 < n_elems) > > + fatal_at (loc, "too few %ss in alternative %d: expected %d, got %d= ", > > + name, alt_no, n_elems, i); > > + > > + list[i].add(','); > > +} > > + > > +/* The compact syntax has more convience syntaxes. As such we post > process > > + the lines to get them back to something the normal syntax > > +understands. */ > > + > > +static void > > +preprocess_compact_syntax (file_location loc, int alt_no, std::string = &line, > > + std::string &last_line) > > +{ > > + /* Check if we're copying the last statement. */ > > + if (line.find ("^") =3D=3D 0 && line.size () =3D=3D 1) > > + { > > + if (last_line.empty ()) > > + fatal_at (loc, "found instruction to copy previous line (^) in" > > + "alternative %d but no previous line to copy", alt_no); > > + line =3D last_line; > > + return; > > + } > > + > > + std::string result; > > + std::string buffer; > > + /* Check if we have << which means return c statement. */ if > > + (line.find ("<<") =3D=3D 0) > > + { > > + result.append ("* return "); > > + result.append (line.substr (3)); >=20 > Seems like this should be line.substr (2) or that the find() should inclu= de a > space after "<<". As it stands, we'd accept <=20 > > + } > > + else > > + result.append (line); > > + > > + line =3D result; > > + return; > > +} > > + > > +/* Converts an rtx from compact syntax to normal syntax if possible. > > +*/ > > + > > +static void > > +convert_syntax (rtx x, file_location loc) { > > + int alt_no; > > + unsigned int index, templ_index; > > + const char *templ; > > + vec_conlist tconvec, convec, attrvec; > > + > > + templ_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 3 : 2; > > + > > + templ =3D XTMPL (x, templ_index); > > + > > + /* Templates with constraints start with "{@". */ if (strncmp > > + ("*{@", templ, 3)) > > + return; > > + > > + /* Get the layout for the template. */ templ +=3D 3; skip_spaces > > + (&templ); > > + > > + if (!expect_char (&templ, '[')) > > + fatal_at (loc, "expecing `[' to begin section list"); > > + > > + parse_section_layout (&templ, "cons:", tconvec, true); > > + convec.resize (tconvec.size ()); > > + > > + /* Check for any duplicate cons entries and sort based on i. */ > > + for (unsigned i =3D 0; i < tconvec.size (); i++) > > + { > > + int idx =3D tconvec[i].idx; > > + if (convec[idx].idx >=3D 0) > > + fatal_at (loc, "duplicate cons number found: %d", idx); > > + convec[idx] =3D tconvec[i]; > > + } > > + tconvec.clear (); >=20 > "convec.resize (tconvec.size ());" isn't guaranteed to be enough if the c= ons: > skips operands. Either we need to calculate the maximum idx first, or we= need > to grow convec on demand. >=20 > > + > > + >=20 > Nit: excess whitespace >=20 > > + if (*templ !=3D ']') > > + { > > + if (*templ =3D=3D ';') > > + skip_spaces (&(++templ)); > > + parse_section_layout (&templ, "attrs:", attrvec, false); > > + create_missing_attributes (x, loc, attrvec); > > + } > > + > > + if (!expect_char (&templ, ']')) > > + { > > + fatal_at (loc, "expecting `]` to end section list - section list= " > > + "must have cons first, attrs second"); > > + } >=20 > Formatting nit: unnecessary braces >=20 > > + > > + /* We will write the un-constrainified template into new_templ. */ > > + std::string new_templ; new_templ.append ("@"); > > + > > + /* Skip to the first proper line. */ while (*templ++ !=3D '\n'); >=20 > This seems to allow anything to follow the "]". Should we instead use > skip_spaces and then require a '\n'? >=20 > > + alt_no =3D 0; > > + > > + std::string last_line; > > + > > + /* Process the alternatives. */ > > + while (*(templ - 1) !=3D '\0') > > + { > > + /* Copy leading whitespace. */ > > + std::string buffer; > > + while (*templ =3D=3D ' ' || *templ =3D=3D '\t') > > + buffer +=3D *templ++; >=20 > Why do we need to do that? The '@' handling in genoutput.cc seems to ski= p > whatever space is present. >=20 > I was wondering if it was so that column numbers matched in compiler erro= r > messages against "<<" lines, but those would already be off because of th= e "* > return" transformation (not an issue that needs to be fixed). >=20 > > + > > + /* Check if we're at the end. */ > > + if (templ[0] =3D=3D '}' && templ[1] =3D=3D '\0') > > + break; > > + > > + new_templ +=3D '\n'; > > + new_templ.append (buffer); > > + > > + if (expect_char (&templ, '[')) > > + { > > + /* Parse the constraint list, then the attribute list. */ > > + if (convec.size () > 0) > > + parse_section (&templ, convec.size (), alt_no, convec, loc, > > + "constraint"); > > + > > + if (attrvec.size () > 0) > > + { > > + if (convec.size () > 0 && !expect_char (&templ, ';')) > > + fatal_at (loc, "expected `;' to separate constraints " > > + "and attributes in alternative %d", alt_no); > > + > > + parse_section (&templ, attrvec.size (), alt_no, > > + attrvec, loc, "attribute"); > > + } > > + > > + if (!expect_char (&templ, ']')) > > + fatal_at (loc, "expected end of constraint/attribute list but " > > + "missing an ending `]' in alternative %d", alt_no); > > + } > > + else if (templ[0] =3D=3D '/' && templ[1] =3D=3D '/') > > + { > > + templ+=3D2; >=20 > Formatting: should be spaces around "+=3D". But here, and... >=20 > > + /* Glob till newline or end of string. */ > > + while (*templ !=3D '\n' || *templ !=3D '\0') > > + templ++; > > + } > > + else if (templ[0] =3D=3D '/' && templ[1] =3D=3D '*') > > + { > > + templ+=3D2; > > + /* Glob till newline or end of multiline comment. */ > > + while (templ[0] !=3D '*' && templ[1] !=3D '/') > > + templ++; > > + templ++; >=20 > ...especially here, I think we should instead completely skip lines with > comments and then "continue", without adding anything to new_templ for > that iteration of the loop. That would ensure > that: >=20 > (a) multi-line // comments work correctly > (b) a comment at the end gets silently dropped without adding a > line to the new template >=20 > > + } > > + else > > + fatal_at (loc, "expected constraint/attribute list at beginning of " > > + "alternative %d but missing a starting `['", alt_no); > > + > > + /* Skip whitespace between list and asm. */ > > + ++templ; > > + skip_spaces (&templ); > > + > > + /* Copy asm to new template. */ > > + std::string line; > > + while (*templ !=3D '\n' && *templ !=3D '\0') > > + line +=3D *templ++; > > + > > + /* Apply any pre-processing needed to the line. */ > > + preprocess_compact_syntax (loc, alt_no, line, last_line); > > + new_templ.append (line); > > + last_line =3D line; > > + > > + /* The processing is very sensitive to whitespace, so preserve > > + all but the trailing ones. */ > > + if (templ[0] =3D=3D '\n') > > + *templ++; >=20 > Is the point here that we allow the closing "}" to be on its own line? > It might be worth calling that out explicitly if so. >=20 > In other words, I'd understood this to mean something like: >=20 > /* Normal "*..." syntax expects the closing quote to be on the final > line of asm, whereas we allow the closing "}" to be on its own lin= e. > Postpone copying the '\n' until we know that there is another > alternative in the list. */ >=20 > > + ++alt_no; > > + } > > + > > + /* Write the constraints and attributes into their proper places. > > +*/ > > + if (convec.size () > 0) > > + { > > + index =3D add_constraints (x, loc, 0, convec); > > + if (index < convec.size ()) > > + fatal_at (loc, "could not find match_operand/scratch with id %d", > > + convec[index].idx); > > + } > > + > > + if (attrvec.size () > 0) > > + { > > + index =3D add_attributes (x, loc, attrvec); > > + if (index < attrvec.size ()) > > + fatal_at (loc, "could not find set_attr for attribute %s", > > + attrvec[index].name.c_str ()); > > + } > > + > > + /* Copy over the new un-constrainified template. */ XTMPL (x, > > + templ_index) =3D xstrdup (new_templ.c_str ()); > > + > > + /* Register for later checks during iterator expansions. */ > > + compact_syntax.add (x); > > + > > +#if DEBUG > > + print_rtl_single (stderr, x); > > +#endif >=20 > IMO it'd be better to drop this. It's easy enough to add locally if that= 's what > someone wants. ("make mddump" would also be useful for debugging this.) >=20 > Thanks, > Richard >=20 > > +} > > + > > /* Process a top level rtx in some way, queuing as appropriate. */ > > > > static void > > @@ -553,10 +1121,12 @@ process_rtx (rtx desc, file_location loc) > > switch (GET_CODE (desc)) > > { > > case DEFINE_INSN: > > + convert_syntax (desc, loc); > > queue_pattern (desc, &define_insn_tail, loc); > > break; > > > > case DEFINE_COND_EXEC: > > + convert_syntax (desc, loc); > > queue_pattern (desc, &define_cond_exec_tail, loc); > > break; > > > > @@ -631,6 +1201,7 @@ process_rtx (rtx desc, file_location loc) > > attr =3D XVEC (desc, split_code + 1); > > PUT_CODE (desc, DEFINE_INSN); > > XVEC (desc, 4) =3D attr; > > + convert_syntax (desc, loc); > > > > /* Queue them. */ > > insn_elem =3D queue_pattern (desc, &define_insn_tail, loc);