[PATCH 0/7] gas: whitespace handling
Jan Beulich
jbeulich@suse.com
Tue Aug 20 13:58:39 GMT 2024
On 20.08.2024 15:02, Jens Remus wrote:
> Am 09.08.2024 um 14:48 schrieb Jan Beulich:
>> As per observations in target specific code (resulting in me stopping the
>> effort there for the moment) there appears to be disagreement across the
>> assembler whether to check for specific characters (blank and tab normally)
>> or whether to use ISSPACE(). It clearly is an option is use ISSPACE()
>> uniformly instead of the new is_whitespace(), just that ISSPACE() also
>> yields "true" for characters we don't really consider whitespace; ISBLANK()
>> would be closer to what the open-coded checks did so far. See also the
>> CR_EOL uses in read.c and app.c.
>
> s390 exclusively uses ISSPACE(). No open coded checks for specific
> whitespace characters (e.g. ' ' or '\t').
>
> Are you suggesting that it would be safe to replace all occurrences of
> ISSPACE() with ISBLANK(), since that is probably the original intention
> of the code?
What I'm suggesting is that in principle wherever ISSPACE() is used when
parsing source code, perhaps ISBLANK() was instead meant to be used. That
ought to be a safe change to make, but I can't reasonably assess this for
all targets. IOW s/would/should/ in what you said.
> What would be the benefit of using is_whitespace() instead of ISBLANK()?
That's the open question; see below.
> Would that get the performance benefit from -f?
No, that's orthogonal. To make -f work, code presently only checking for
blanks needs updating. Whether to is_whitespace() or to ISBLANK() (or,
hopefully not, to ISSPACE()) is a separate question, which I'm putting up
here. If, as you say, s390 code already is consistent in this regard, then
at least from that angle there should be noting in the way of using -f.
(There may be other obstacles, though.)
It just so happened that I started the conversion, and only later noticed
some targets using ISSPACE() already. At that point I started wondering
whether for parsing source code we may indeed use ISSPACE() (libiberty
probably isn't going to change behavior, yet still it feels like a wrong
dependency to have), or whether the assembler should have its own notion
of what it considers whitespace.
Jan
More information about the Binutils
mailing list