From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id E7277385800F for ; Mon, 27 Jan 2025 16:31:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E7277385800F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E7277385800F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737995477; cv=none; b=UsfpWryXEywrdSOfNTq6DH9aJIDmYyl5scn2IQK3XBy1gPtGFxXVBQl9CoYVuwchZQtxrwFZj2B67mIPJtxO982kLriiv1HEKkGhlX6kO4cSipPhqD1aMd4LWrdDA4coxcesy5RTHOTd/kBbTnZp8XMKoBW8SSCZigExeEvh+cQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737995477; c=relaxed/simple; bh=qjN6N3KQ5CSAmyk1ApeSzLdkFjs988klmnqC6lWYee4=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=EUgWHeLSMp1iE3SM5yq431YSthVqNlAkcRZMxze8pYu9o8oDQJq3DOOwNQHgKAkTa4rBq7n8ZZxtVcOaG4rRmbwg30yLhouJNkoDwWHN1tI2Jsy3vh0qF8F8/cpC716sf6/076y6xrguhsdSdLq8pnZWSycwDMO4ZD8GqlGWsTA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E7277385800F Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2717F497; Mon, 27 Jan 2025 08:31:43 -0800 (PST) Received: from [10.2.78.71] (e120077-lin.cambridge.arm.com [10.2.78.71]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5EBE13F528; Mon, 27 Jan 2025 08:31:15 -0800 (PST) Message-ID: <21fadba4-7b81-4d82-a42e-d9ce1b02437f@arm.com> Date: Mon, 27 Jan 2025 16:31:13 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 05/65] Arm: use is_whitespace() To: Jan Beulich , Binutils Cc: Nick Clifton , "ramana.radhakrishnan@arm.com" , Richard Earnshaw References: <2316ac5c-7870-4b46-9c80-eaecaef93a31@suse.com> <83414a2b-bfad-4381-8903-152ec0552e88@suse.com> From: "Richard Earnshaw (lists)" Content-Language: en-GB In-Reply-To: <83414a2b-bfad-4381-8903-152ec0552e88@suse.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3489.7 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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: On 27/01/2025 15:50, Jan Beulich wrote: > Wherever blanks are permissible in input, tabs ought to be permissible, > too. This is particularly relevant when -f is passed to gas (alongside > appropriate input). At the same time use is_end_of_stmt() instead of an > open-coded nul char check. > > In parse_neon_type() be more aggressive and remove the special casing of > certain characters altogether. The original default case simply having > "break" can't have been correct. > --- > I don't think I see why parse_qfloat_immediate() checks for '\n'. If > that was needed, "line" (really: statement) separators would need > checking for, too. Yet an easy experiment demonstrates that this case is > working correctly despite the lack of a check for ';'. > > The check for "0x" in parse_qfloat_immediate() seems fishy, too: If it > was actually needed, "0X" would apparently also checking form. Yet again > experimentally that's properly refused anyway, by atof_ieee() I guess. > > While for parse_neon_type() the change improves the handling of this set > of (bad) examples (including the case of passing -f to gas): > > vcvt.bf016.f32 d0, q0 > vcvt.bf16.f032 d0, q0 > vcvt.b16.f32 d0, q0 > vcvt.b f16.f32 d0, q0 > vcvt.bf 16.f32 d0, q0 > vcvt.bf16.f 32 d0, q0 > vcvt.b f16.f32 d0, q0 > vcvt.b 16.f32 d0, q0 > vcvt.b 32.f32 d0, q0 > vcvt.bf 16.f32 d0, q0 > > several are left which imo also ought to be rejected. Yet that will want > sorting separately. > --- > v2: Also replace ISSPACE(). > > --- a/gas/config/tc-arm.c > +++ b/gas/config/tc-arm.c > @@ -1081,7 +1081,7 @@ const char FLT_CHARS[] = "rRsSfFdDxXeEpP > > /* Separator character handling. */ > > -#define skip_whitespace(str) do { if (*(str) == ' ') ++(str); } while (0) > +#define skip_whitespace(str) do { if (is_whitespace (*(str))) ++(str); } while (0) > > enum fp_16bit_format > { > @@ -1510,13 +1510,9 @@ parse_neon_type (struct neon_type *type, > return FAIL; > } > goto done; > - case '0': case '1': case '2': case '3': case '4': > - case '5': case '6': case '7': case '8': case '9': > - case ' ': case '.': > + default: > as_bad (_("unexpected type character `b' -- did you mean `bf'?")); > return FAIL; > - default: > - break; > } This entire switch statement has now degenerated into 'f' or error. So I think it would be better to just replace it with an if-else. > break; > default: > @@ -5055,7 +5051,8 @@ set_fp16_format (int dummy ATTRIBUTE_UNU > new_format = ARM_FP16_FORMAT_DEFAULT; > > name = input_line_pointer; > - while (*input_line_pointer && !ISSPACE (*input_line_pointer)) > + while (!is_end_of_stmt (*input_line_pointer) > + && !is_whitespace (*input_line_pointer)) > input_line_pointer++; > > saved_char = *input_line_pointer; > @@ -5366,7 +5363,7 @@ parse_qfloat_immediate (char **ccp, int > return FAIL; > else > { > - for (; *fpnum != '\0' && *fpnum != ' ' && *fpnum != '\n'; fpnum++) > + for (; *fpnum != '\0' && !is_whitespace (*fpnum) && *fpnum != '\n'; fpnum++) > if (*fpnum == '.' || *fpnum == 'e' || *fpnum == 'E') > { > found_fpchar = 1; > @@ -22450,7 +22447,7 @@ opcode_lookup (char **str) > /* Scan up to the end of the mnemonic, which must end in white space, > '.' (in unified mode, or for Neon/VFP instructions), or end of string. */ > for (base = end = *str; *end != '\0'; end++) > - if (*end == ' ' || *end == '.') > + if (is_whitespace (*end) || *end == '.') > break; > > if (end == base) > @@ -22481,7 +22478,7 @@ opcode_lookup (char **str) > if (parse_neon_type (&inst.vectype, str) == FAIL) > return NULL; > } > - else if (end[offset] != '\0' && end[offset] != ' ') > + else if (end[offset] != '\0' && !is_whitespace (end[offset])) > return NULL; > } > else > OK with that change. R.