* [PATCH 0/3] Restore macro with expression argument
@ 2024-08-11 23:11 H.J. Lu
2024-08-11 23:11 ` [PATCH 1/3] Revert "gas: drop scrubber states 14 and 15" H.J. Lu
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: H.J. Lu @ 2024-08-11 23:11 UTC (permalink / raw)
To: binutils; +Cc: amodra, jbeulich, nickc
commit 6ae8a30d44f016cafb46a75843b5109316eb1996
Author: Jan Beulich <jbeulich@suse.com>
Date: Fri Aug 9 11:59:31 2024 +0200
gas: have scrubber retain more whitespace
breaks GCC builds for ARM, AVR, PRU and others as well as x86-64 kernel
build. Revert
7dd0dfbde7e gas: drop scrubber states 14 and 15
6ae8a30d44f gas: have scrubber retain more whitespace
to restore macro with expression argument and add a testcase.
H.J. Lu (3):
Revert "gas: drop scrubber states 14 and 15"
Revert "gas: have scrubber retain more whitespace"
gas: Add a macro test with expression argument
gas/NEWS | 7 -
gas/app.c | 200 ++++++++++++++++++-------
gas/config/tc-aarch64.c | 19 +--
gas/config/tc-arm.c | 2 -
gas/config/tc-crx.c | 6 +-
gas/config/tc-csky.c | 23 +--
gas/config/tc-ia64.h | 3 +
gas/config/tc-pru.c | 10 +-
gas/config/tc-sparc.c | 48 ++----
gas/config/tc-tic6x.h | 2 +
gas/config/tc-v850.c | 4 -
gas/testsuite/gas/all/macro.l | 10 --
gas/testsuite/gas/all/macro.s | 4 +-
gas/testsuite/gas/i386/x86-64-apx-nf.s | 40 ++---
gas/testsuite/gas/macros/arg1.d | 8 +
gas/testsuite/gas/macros/arg1.s | 7 +
gas/testsuite/gas/macros/macros.exp | 2 +
opcodes/cgen-asm.in | 52 +++----
opcodes/epiphany-asm.c | 52 +++----
opcodes/fr30-asm.c | 52 +++----
opcodes/frv-asm.c | 52 +++----
opcodes/ip2k-asm.c | 52 +++----
opcodes/iq2000-asm.c | 52 +++----
opcodes/lm32-asm.c | 52 +++----
opcodes/m32c-asm.c | 52 +++----
opcodes/m32r-asm.c | 52 +++----
opcodes/mep-asm.c | 52 +++----
opcodes/mt-asm.c | 52 +++----
opcodes/nds32-asm.c | 5 -
opcodes/or1k-asm.c | 52 +++----
opcodes/xstormy16-asm.c | 52 +++----
31 files changed, 456 insertions(+), 620 deletions(-)
create mode 100644 gas/testsuite/gas/macros/arg1.d
create mode 100644 gas/testsuite/gas/macros/arg1.s
--
2.46.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] Revert "gas: drop scrubber states 14 and 15"
2024-08-11 23:11 [PATCH 0/3] Restore macro with expression argument H.J. Lu
@ 2024-08-11 23:11 ` H.J. Lu
2024-08-11 23:11 ` [PATCH 2/3] Revert "gas: have scrubber retain more whitespace" H.J. Lu
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2024-08-11 23:11 UTC (permalink / raw)
To: binutils; +Cc: amodra, jbeulich, nickc
This reverts commit 7dd0dfbde7ee31167a3b2e192a575493d26b7b0a.
---
gas/app.c | 29 +++++++++++++++++++++++++++++
gas/config/tc-ia64.h | 3 +++
gas/config/tc-tic6x.h | 2 ++
3 files changed, 34 insertions(+)
diff --git a/gas/app.c b/gas/app.c
index a47276e4816..b88b4c96137 100644
--- a/gas/app.c
+++ b/gas/app.c
@@ -485,6 +485,12 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
13: After seeing a vertical bar, looking for a second
vertical bar as a parallel expression separator.
#endif
+#ifdef TC_PREDICATE_START_CHAR
+ 14: After seeing a predicate start character at state 0, looking
+ for a predicate end character as predicate.
+ 15: After seeing a predicate start character at state 1, looking
+ for a predicate end character as predicate.
+#endif
#ifdef TC_Z80
16: After seeing an 'a' or an 'A' at the start of a symbol
17: After seeing an 'f' or an 'F' in state 16
@@ -771,6 +777,29 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
/* flushchar: */
ch = GET ();
+#ifdef TC_PREDICATE_START_CHAR
+ if (ch == TC_PREDICATE_START_CHAR && (state == 0 || state == 1))
+ {
+ state += 14;
+ PUT (ch);
+ continue;
+ }
+ else if (state == 14 || state == 15)
+ {
+ if (ch == TC_PREDICATE_END_CHAR)
+ {
+ state -= 14;
+ PUT (ch);
+ ch = GET ();
+ }
+ else
+ {
+ PUT (ch);
+ continue;
+ }
+ }
+#endif
+
recycle:
#if defined TC_ARM && defined OBJ_ELF
diff --git a/gas/config/tc-ia64.h b/gas/config/tc-ia64.h
index 665272adde3..8ab05373f6e 100644
--- a/gas/config/tc-ia64.h
+++ b/gas/config/tc-ia64.h
@@ -78,6 +78,9 @@ extern const char *ia64_target_format (void);
#define LEX_QM (LEX_NAME|LEX_BEGIN_NAME) /* allow `?' inside name */
#define LEX_HASH LEX_END_NAME /* allow `#' ending a name */
+#define TC_PREDICATE_START_CHAR '('
+#define TC_PREDICATE_END_CHAR ')'
+
extern const char ia64_symbol_chars[];
#define tc_symbol_chars ia64_symbol_chars
diff --git a/gas/config/tc-tic6x.h b/gas/config/tc-tic6x.h
index f21673e2d36..0b4223961e9 100644
--- a/gas/config/tc-tic6x.h
+++ b/gas/config/tc-tic6x.h
@@ -24,6 +24,8 @@
#define DOUBLEBAR_PARALLEL
#define DWARF2_LINE_MIN_INSN_LENGTH 2
#define MD_APPLY_SYM_VALUE(FIX) 0
+#define TC_PREDICATE_START_CHAR '['
+#define TC_PREDICATE_END_CHAR ']'
/* For TI C6X, we keep spaces in what the preprocessor considers
operands as they may separate functional unit specifiers from
operands. */
--
2.46.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] Revert "gas: have scrubber retain more whitespace"
2024-08-11 23:11 [PATCH 0/3] Restore macro with expression argument H.J. Lu
2024-08-11 23:11 ` [PATCH 1/3] Revert "gas: drop scrubber states 14 and 15" H.J. Lu
@ 2024-08-11 23:11 ` H.J. Lu
2024-08-11 23:11 ` [PATCH 3/3] gas: Add a macro test with expression argument H.J. Lu
2024-08-11 23:20 ` [PATCH 0/3] Restore macro " Sam James
3 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2024-08-11 23:11 UTC (permalink / raw)
To: binutils; +Cc: amodra, jbeulich, nickc
This reverts commit 6ae8a30d44f016cafb46a75843b5109316eb1996.
---
gas/NEWS | 7 -
gas/app.c | 171 +++++++++++++++++--------
gas/config/tc-aarch64.c | 19 +--
gas/config/tc-arm.c | 2 -
gas/config/tc-crx.c | 6 +-
gas/config/tc-csky.c | 23 +---
gas/config/tc-pru.c | 10 +-
gas/config/tc-sparc.c | 48 ++-----
gas/config/tc-v850.c | 4 -
gas/testsuite/gas/all/macro.l | 10 --
gas/testsuite/gas/all/macro.s | 4 +-
gas/testsuite/gas/i386/x86-64-apx-nf.s | 40 +++---
opcodes/cgen-asm.in | 52 +++-----
opcodes/epiphany-asm.c | 52 +++-----
opcodes/fr30-asm.c | 52 +++-----
opcodes/frv-asm.c | 52 +++-----
opcodes/ip2k-asm.c | 52 +++-----
opcodes/iq2000-asm.c | 52 +++-----
opcodes/lm32-asm.c | 52 +++-----
opcodes/m32c-asm.c | 52 +++-----
opcodes/m32r-asm.c | 52 +++-----
opcodes/mep-asm.c | 52 +++-----
opcodes/mt-asm.c | 52 +++-----
opcodes/nds32-asm.c | 5 -
opcodes/or1k-asm.c | 52 +++-----
opcodes/xstormy16-asm.c | 52 +++-----
26 files changed, 405 insertions(+), 620 deletions(-)
diff --git a/gas/NEWS b/gas/NEWS
index 66f8cc5cc8f..69c6317c486 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -2,13 +2,6 @@
* Add support for RISC-V CORE-V extension (XCvBitmanip) with version 1.0.
-* The scrubber (pre-processor) now leaves in place more whitespace, to permit
- various constructs not fitting the basic "insn opnd1,opnd2[,...]" scheme to
- work. This, however, means that macro invocations like "m 1 + 2", i.e. not
- using double quotes or parentheses around the (apparently) sole argument,
- will now be treated as passing three arguments. Such lack of quotation /
- parenthesization was never reliable to use.
-
Changes in 2.43:
* Add support for LoongArch .option for fine-grained control of assembly
diff --git a/gas/app.c b/gas/app.c
index b88b4c96137..41ba4163a2a 100644
--- a/gas/app.c
+++ b/gas/app.c
@@ -467,18 +467,16 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
/*State 0: beginning of normal line
1: After first whitespace on line (flush more white)
- 2: After first non-white (opcode or maybe label when they're followed
- by colons) on line (keep 1white)
- 3: after subsequent white on line (typically into operands)
- (flush more white)
+ 2: After first non-white (opcode) on line (keep 1white)
+ 3: after second white on line (into operands) (flush white)
4: after putting out a .linefile, put out digits
5: parsing a string, then go to old-state
6: putting out \ escape in a "d string.
7: no longer used
8: no longer used
- 9: After seeing non-white in state 3 (keep 1white)
- 10: no longer used
- 11: After seeing a non-white character in state 0 (eg a label definition)
+ 9: After seeing symbol char in state 3 (keep 1white after symchar)
+ 10: After seeing whitespace in state 9 (keep white before symchar)
+ 11: After seeing a symbol character in state 0 (eg a label definition)
-1: output string in out_string and go to the state in old_state
12: no longer used
#ifdef DOUBLEBAR_PARALLEL
@@ -941,11 +939,7 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
&& (state < 1 || strchr (tc_comment_chars, ch)))
|| IS_NEWLINE (ch)
|| IS_LINE_SEPARATOR (ch)
- || IS_PARALLEL_SEPARATOR (ch)
- /* See comma related comment near the bottom of the function.
- Reasoning equally applies to whitespace preceding a comma in
- most cases. */
- || (ch == ',' && state > 2 && state != 11))
+ || IS_PARALLEL_SEPARATOR (ch))
{
if (scrub_m68k_mri)
{
@@ -988,7 +982,6 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
character at the beginning of a line. */
goto recycle;
case 2:
- case 9:
state = 3;
if (to + 1 < toend)
{
@@ -1012,6 +1005,20 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
break;
}
goto recycle; /* Sp in operands */
+ case 9:
+ case 10:
+#ifndef TC_KEEP_OPERAND_SPACES
+ if (scrub_m68k_mri)
+#endif
+ {
+ /* In MRI mode, we keep these spaces. */
+ state = 3;
+ UNGET (ch);
+ PUT (' ');
+ break;
+ }
+ state = 10; /* Sp after symbol char */
+ goto recycle;
case 11:
if (LABELS_WITHOUT_COLONS || flag_m68k_mri)
state = 1;
@@ -1082,17 +1089,27 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
{
if (ch2 != EOF)
UNGET (ch2);
- if (state == 1)
- state = 2;
- else if (state == 3)
- state = 9;
+ if (state == 9 || state == 10)
+ state = 3;
PUT (ch);
}
break;
case LEX_IS_STRINGQUOTE:
quotechar = ch;
- if (state == 3)
+ if (state == 10)
+ {
+ /* Preserve the whitespace in foo "bar". */
+ UNGET (ch);
+ state = 3;
+ PUT (' ');
+
+ /* PUT didn't jump out. We could just break, but we
+ know what will happen, so optimize a bit. */
+ ch = GET ();
+ old_state = 9;
+ }
+ else if (state == 3)
old_state = 9;
else if (state == 0)
old_state = 11; /* Now seeing label definition. */
@@ -1113,6 +1130,14 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
UNGET (c);
}
#endif
+ if (state == 10)
+ {
+ /* Preserve the whitespace in foo 'b'. */
+ UNGET (ch);
+ state = 3;
+ PUT (' ');
+ break;
+ }
ch = GET ();
if (ch == EOF)
{
@@ -1147,7 +1172,10 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
PUT (out_buf[0]);
break;
}
- old_state = state;
+ if (state == 9)
+ old_state = 3;
+ else
+ old_state = state;
state = -1;
out_string = out_buf;
PUT (*out_string++);
@@ -1157,10 +1185,10 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
#ifdef KEEP_WHITE_AROUND_COLON
state = 9;
#else
- if (state == 2 || state == 11)
+ if (state == 9 || state == 10)
+ state = 3;
+ else if (state != 3)
state = 1;
- else
- state = 9;
#endif
PUT (ch);
break;
@@ -1285,6 +1313,20 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
break;
}
+#ifdef TC_D10V
+ /* All insns end in a char for which LEX_IS_SYMBOL_COMPONENT is true.
+ Trap is the only short insn that has a first operand that is
+ neither register nor label.
+ We must prevent exef0f ||trap #1 to degenerate to exef0f ||trap#1 .
+ We can't make '#' LEX_IS_SYMBOL_COMPONENT because it is
+ already LEX_IS_LINE_COMMENT_START. However, it is the
+ only character in line_comment_chars for d10v, hence we
+ can recognize it as such. */
+ /* An alternative approach would be to reset the state to 1 when
+ we see '||', '<'- or '->', but that seems to be overkill. */
+ if (state == 10)
+ PUT (' ');
+#endif
/* We have a line comment character which is not at the
start of a line. If this is also a normal comment
character, fall through. Otherwise treat it as a default
@@ -1348,6 +1390,17 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
/* Fall through. */
case LEX_IS_SYMBOL_COMPONENT:
+ if (state == 10)
+ {
+ /* This is a symbol character following another symbol
+ character, with whitespace in between. We skipped
+ the whitespace earlier, so output it now. */
+ UNGET (ch);
+ state = 3;
+ PUT (' ');
+ break;
+ }
+
#ifdef TC_Z80
/* "af'" is a symbol containing '\''. */
if (state == 3 && (ch == 'a' || ch == 'A'))
@@ -1373,16 +1426,7 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
}
}
#endif
-
- /* Fall through. */
- default:
- de_fault:
- /* Some relatively `normal' character. */
- if (state == 0)
- state = 11; /* Now seeing label definition. */
- else if (state == 1)
- state = 2; /* Ditto. */
- else if (state == 3)
+ if (state == 3)
state = 9;
/* This is a common case. Quickly copy CH and all the
@@ -1391,10 +1435,6 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
&& mri_state == NULL
#if defined TC_ARM && defined OBJ_ELF
&& symver_state == NULL
-#endif
-#ifdef TC_Z80
- /* See comma related comment below. */
- && ch != ','
#endif
)
{
@@ -1410,12 +1450,6 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
if (type != 0
&& type != LEX_IS_SYMBOL_COMPONENT)
break;
-#ifdef TC_Z80
- /* Need to split at commas, to be able to enter state 16
- when needed. */
- if (ch2 == ',')
- break;
-#endif
}
if (s > from)
@@ -1440,15 +1474,52 @@ do_scrub_chars (size_t (*get) (char *, size_t), char *tostart, size_t tolen,
}
}
- /* As a special case, to limit the delta to previous behavior, e.g.
- also affecting listings, go straight to state 3 when seeing a
- comma. Commas are special: While they can be used to separate
- macro parameters or arguments, they cannot (on their own, i.e.
- without quoting) be arguments (or parameter default values).
- Hence successive whitespace is not meaningful there. */
- if (ch == ',' && state == 9)
- state = 3;
+ /* Fall through. */
+ default:
+ de_fault:
+ /* Some relatively `normal' character. */
+ if (state == 0)
+ {
+ state = 11; /* Now seeing label definition. */
+ }
+ else if (state == 1)
+ {
+ state = 2; /* Ditto. */
+ }
+ else if (state == 9)
+ {
+ if (!IS_SYMBOL_COMPONENT (ch))
+ state = 3;
+ }
+ else if (state == 10)
+ {
+ if (ch == '\\')
+ {
+ /* Special handling for backslash: a backslash may
+ be the beginning of a formal parameter (of a
+ macro) following another symbol character, with
+ whitespace in between. If that is the case, we
+ output a space before the parameter. Strictly
+ speaking, correct handling depends upon what the
+ macro parameter expands into; if the parameter
+ expands into something which does not start with
+ an operand character, then we don't want to keep
+ the space. We don't have enough information to
+ make the right choice, so here we are making the
+ choice which is more likely to be correct. */
+ if (to + 1 >= toend)
+ {
+ /* If we're near the end of the buffer, save the
+ character for the next time round. Otherwise
+ we'll lose our state. */
+ UNGET (ch);
+ goto tofull;
+ }
+ *to++ = ' ';
+ }
+ state = 3;
+ }
PUT (ch);
break;
}
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index c6ac66bcc5c..e94a0cff406 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -643,7 +643,6 @@ const char FLT_CHARS[] = "rRsSfFdDxXeEpPhHb";
static inline bool
skip_past_char (char **str, char c)
{
- skip_whitespace (*str);
if (**str == c)
{
(*str)++;
@@ -894,7 +893,6 @@ parse_reg (char **ccp)
start++;
#endif
- skip_whitespace (start);
p = start;
if (!ISALPHA (*p) || !is_name_beginner (*p))
return NULL;
@@ -1204,17 +1202,13 @@ parse_typed_reg (char **ccp, aarch64_reg_type type,
struct vector_type_el *typeinfo, unsigned int flags)
{
char *str = *ccp;
- bool is_alpha;
- const reg_entry *reg;
+ bool is_alpha = ISALPHA (*str);
+ const reg_entry *reg = parse_reg (&str);
struct vector_type_el atype;
struct vector_type_el parsetype;
bool is_typed_vecreg = false;
unsigned int err_flags = (flags & PTR_IN_REGLIST) ? SEF_IN_REGLIST : 0;
- skip_whitespace (str);
- is_alpha = ISALPHA (*str);
- reg = parse_reg (&str);
-
atype.defined = 0;
atype.type = NT_invtype;
atype.width = -1;
@@ -1435,7 +1429,10 @@ parse_vector_reg_list (char **ccp, aarch64_reg_type type,
do
{
if (in_range)
- val_range = val;
+ {
+ str++; /* skip over '-' */
+ val_range = val;
+ }
const reg_entry *reg;
if (has_qualifier)
@@ -1497,8 +1494,7 @@ parse_vector_reg_list (char **ccp, aarch64_reg_type type,
in_range = 0;
ptr_flags |= PTR_GOOD_MATCH;
}
- while (skip_past_comma (&str)
- || (in_range = 1, skip_past_char (&str, '-')));
+ while (skip_past_comma (&str) || (in_range = 1, *str == '-'));
skip_whitespace (str);
if (*str != '}')
@@ -8293,7 +8289,6 @@ parse_operands (char *str, const aarch64_opcode *opcode)
}
/* Check if we have parsed all the operands. */
- skip_whitespace (str);
if (*str != '\0' && ! error_p ())
{
/* Set I to the index of the last present operand; this is
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 14d729558c5..540ab487384 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -1148,8 +1148,6 @@ my_get_expression (expressionS * ep, char ** str, int prefix_mode)
prefix_mode = (prefix_mode == GE_OPT_PREFIX_BIG) ? prefix_mode
: GE_OPT_PREFIX;
- skip_whitespace (*str);
-
switch (prefix_mode)
{
case GE_NO_PREFIX: break;
diff --git a/gas/config/tc-crx.c b/gas/config/tc-crx.c
index 4673c947f2f..3a47354c473 100644
--- a/gas/config/tc-crx.c
+++ b/gas/config/tc-crx.c
@@ -1723,12 +1723,8 @@ preprocess_reglist (char *param, int *allocated)
while (*paramP != '}')
{
- memset (®_name, '\0', sizeof (reg_name));
-
- while (ISSPACE (*paramP))
- paramP++;
-
regP = paramP;
+ memset (®_name, '\0', sizeof (reg_name));
while (ISALNUM (*paramP))
paramP++;
diff --git a/gas/config/tc-csky.c b/gas/config/tc-csky.c
index 0eb1805aacf..0dc8bfd45dc 100644
--- a/gas/config/tc-csky.c
+++ b/gas/config/tc-csky.c
@@ -2409,18 +2409,10 @@ parse_rt (char *s,
/* Indicate nothing there. */
ep->X_op = O_absent;
- /* Skip whitespace. */
- while (ISSPACE (*s))
- ++s;
-
if (*s == '[')
{
s = parse_exp (s + 1, &e);
- /* Skip whitespace. */
- while (ISSPACE (*s))
- ++s;
-
if (*s == ']')
s++;
else
@@ -2943,11 +2935,6 @@ is_reg_lshift_illegal (char **oper, int is_float)
}
*oper += len;
-
- /* Skip whitespace. */
- while (ISSPACE (**oper))
- ++*oper;
-
if ((*oper)[0] != '<' || (*oper)[1] != '<')
{
SET_ERROR_STRING (ERROR_UNDEFINE,
@@ -3474,9 +3461,6 @@ get_operand_value (struct csky_opcode_info *op,
return false;
}
- while (ISSPACE (**oper))
- ++*oper;
-
if (!get_operand_value (op, oper, &soprnd->subs[0]))
{
*s = rc;
@@ -3497,7 +3481,7 @@ get_operand_value (struct csky_opcode_info *op,
}
*s = rc;
- *oper = s + 1;
+ *oper += 1;
return true;
}
@@ -4293,16 +4277,11 @@ get_operand_value (struct csky_opcode_info *op,
case OPRND_TYPE_VREG_WITH_INDEX:
if (parse_type_freg (oper, 0))
{
- /* Skip whitespace. */
- while (ISSPACE (**oper))
- ++*oper;
if (**oper == '[')
{
(*oper)++;
if (is_imm_within_range (oper, 0, 0xf))
{
- while (ISSPACE (**oper))
- ++*oper;
if (**oper == ']')
{
unsigned int idx = --csky_insn.idx;
diff --git a/gas/config/tc-pru.c b/gas/config/tc-pru.c
index 6789686718a..99a3c1ef88c 100644
--- a/gas/config/tc-pru.c
+++ b/gas/config/tc-pru.c
@@ -1399,6 +1399,7 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
const char *parsestr, char **parsed_args)
{
char *p;
+ char *end = NULL;
int i;
p = argstr;
i = 0;
@@ -1425,7 +1426,14 @@ pru_parse_args (pru_insn_infoS *insn ATTRIBUTE_UNUSED, char *argstr,
else
{
/* Check that the argument string has no trailing arguments. */
- if (strpbrk (p, ",") != NULL)
+ /* If we've got a %pmem relocation, we've zapped the parens with
+ spaces. */
+ if (strprefix (p, "%pmem") || strprefix (p, "%label"))
+ end = strpbrk (p, ",");
+ else
+ end = strpbrk (p, " ,");
+
+ if (end != NULL)
as_bad (_("too many arguments"));
}
diff --git a/gas/config/tc-sparc.c b/gas/config/tc-sparc.c
index d78aa0e6e3d..e37189e7c5e 100644
--- a/gas/config/tc-sparc.c
+++ b/gas/config/tc-sparc.c
@@ -1778,9 +1778,6 @@ sparc_ip (char *str, const struct sparc_opcode **pinsn)
operands match. */
for (args = insn->args;; ++args)
{
- if (*s == ' ' && *args != ' ')
- ++s;
-
switch (*args)
{
case 'K':
@@ -2721,6 +2718,11 @@ sparc_ip (char *str, const struct sparc_opcode **pinsn)
'symbols' in the input string. Try not to create U
symbols for registers, etc. */
+ /* This stuff checks to see if the expression ends in
+ +%reg. If it does, it removes the register from
+ the expression, and re-sets 's' to point to the
+ right place. */
+
if (op_arg)
{
int npar = 0;
@@ -2750,8 +2752,6 @@ sparc_ip (char *str, const struct sparc_opcode **pinsn)
return special_case;
}
s = s1 + 1;
- if (*s == ' ')
- s++;
if (*s == ',' || *s == ']' || !*s)
continue;
if (*s != '+' && *s != '-')
@@ -2765,45 +2765,17 @@ sparc_ip (char *str, const struct sparc_opcode **pinsn)
memset (&the_insn.exp, 0, sizeof (the_insn.exp));
}
- /* This stuff checks to see if the expression ends in
- +%reg. If it does, it removes the register from
- the expression, and re-sets 's' to point to the
- right place. */
-
for (s1 = s; *s1 && *s1 != ',' && *s1 != ']'; s1++)
;
- if (s1 != s && s1[-1] == ' ')
- --s1;
if (s1 != s && ISDIGIT (s1[-1]))
{
if (s1[-2] == '%' && s1[-3] == '+')
- {
- if (s1[-3] == '+')
- s1 -= 3;
- else if (s1[-3] == ' ' && s1[-4] == '+')
- s1 -= 4;
- else
- s1 = NULL;
- }
- else if (strchr ("golir0123456789", s1[-2]) && s1[-3] == '%')
- {
- if (s1[-4] == '+')
- s1 -= 4;
- else if (s1[-4] == ' ' && s1[-5] == '+')
- s1 -= 5;
- else
- s1 = NULL;
- }
- else if (s1[-3] == 'r' && s1[-4] == '%')
- {
- if (s1[-5] == '+')
- s1 -= 5;
- else if (s1[-5] == ' ' && s1[-6] == '+')
- s1 -= 6;
- else
- s1 = NULL;
- }
+ s1 -= 3;
+ else if (strchr ("golir0123456789", s1[-2]) && s1[-3] == '%' && s1[-4] == '+')
+ s1 -= 4;
+ else if (s1[-3] == 'r' && s1[-4] == '%' && s1[-5] == '+')
+ s1 -= 5;
else
s1 = NULL;
if (s1)
diff --git a/gas/config/tc-v850.c b/gas/config/tc-v850.c
index 63b69a7e1f9..cb518a7da05 100644
--- a/gas/config/tc-v850.c
+++ b/gas/config/tc-v850.c
@@ -1456,8 +1456,6 @@ parse_register_list (unsigned long *insn,
}
}
- skip_white_space ();
-
if (*input_line_pointer == '}')
{
input_line_pointer++;
@@ -1477,8 +1475,6 @@ parse_register_list (unsigned long *insn,
/* Skip the dash. */
++input_line_pointer;
- skip_white_space ();
-
/* Get the second register in the range. */
if (! register_name (&exp2))
{
diff --git a/gas/testsuite/gas/all/macro.l b/gas/testsuite/gas/all/macro.l
index c62d34d2baa..75fe338f132 100644
--- a/gas/testsuite/gas/all/macro.l
+++ b/gas/testsuite/gas/all/macro.l
@@ -22,14 +22,4 @@
[ ]*[1-9][0-9]*[ ]+.... 0+70*[ ]+> .byte 7
[ ]*[1-9][0-9]*[ ]+.... 0+80*[ ]+> .byte 8
[ ]*[1-9][0-9]*[ ]+m[ ]+""[ ]+""[ ]+""
-[ ]*[1-9][0-9]*[ ]+
-[ ]*[1-9][0-9]*[ ]+m[ ]+1[ ]+\+2
-[ ]*[1-9][0-9]*[ ]+.... 0+10*[ ]+> .byte 1
-[ ]*[1-9][0-9]*[ ]+.... 0+20*[ ]+> .byte \+2
-[ ]*[1-9][0-9]*[ ]+m[ ]+\(3\)[ ]+\+4
-[ ]*[1-9][0-9]*[ ]+.... 0+30*[ ]+> .byte \(3\)
-[ ]*[1-9][0-9]*[ ]+.... 0+40*[ ]+> .byte \+4
-[ ]*[1-9][0-9]*[ ]+m[ ]+\(5\)[ ]+\(6\)
-[ ]*[1-9][0-9]*[ ]+.... 0+50*[ ]+> .byte \(5\)
-[ ]*[1-9][0-9]*[ ]+.... 0+60*[ ]+> .byte \(6\)
#pass
diff --git a/gas/testsuite/gas/all/macro.s b/gas/testsuite/gas/all/macro.s
index 109bcc56c99..9e70f3067b7 100644
--- a/gas/testsuite/gas/all/macro.s
+++ b/gas/testsuite/gas/all/macro.s
@@ -9,8 +9,8 @@
m "7" "8"
m "" "" ""
+ .if 0
m 1 +2
m (3) +4
m (5) (6)
-
- .byte -1
+ .endif
diff --git a/gas/testsuite/gas/i386/x86-64-apx-nf.s b/gas/testsuite/gas/i386/x86-64-apx-nf.s
index 99ae1c7aaa6..fbd4cadd983 100644
--- a/gas/testsuite/gas/i386/x86-64-apx-nf.s
+++ b/gas/testsuite/gas/i386/x86-64-apx-nf.s
@@ -1390,13 +1390,13 @@ optimize:
{nf} \op $128, %ecx, %edx
{nf} \op $128, %r9
{nf} \op $128, %r9, %r31
- {nf} \op\(b) $128, (%rax)
+ {nf} \op\()b $128, (%rax)
{nf} \op $128, (%rax), %bl
- {nf} \op\(w) $128, (%rax)
+ {nf} \op\()w $128, (%rax)
{nf} \op $128, (%rax), %dx
- {nf} \op\(l) $128, (%rax)
+ {nf} \op\()l $128, (%rax)
{nf} \op $128, (%rax), %ecx
- {nf} \op\(q) $128, (%rax)
+ {nf} \op\()q $128, (%rax)
{nf} \op $128, (%rax), %r9
{nf} \op $1, %bl
@@ -1407,13 +1407,13 @@ optimize:
{nf} \op $1, %ecx, %edx
{nf} \op $1, %r9
{nf} \op $1, %r9, %r31
- {nf} \op\(b) $1, (%rax)
+ {nf} \op\()b $1, (%rax)
{nf} \op $1, (%rax), %bl
- {nf} \op\(w) $1, (%rax)
+ {nf} \op\()w $1, (%rax)
{nf} \op $1, (%rax), %dx
- {nf} \op\(l) $1, (%rax)
+ {nf} \op\()l $1, (%rax)
{nf} \op $1, (%rax), %ecx
- {nf} \op\(q) $1, (%rax)
+ {nf} \op\()q $1, (%rax)
{nf} \op $1, (%rax), %r9
{nf} \op $0xff, %bl
@@ -1424,13 +1424,13 @@ optimize:
{nf} \op $-1, %ecx, %edx
{nf} \op $-1, %r9
{nf} \op $-1, %r9, %r31
- {nf} \op\(b) $0xff, (%rax)
+ {nf} \op\()b $0xff, (%rax)
{nf} \op $-1, (%rax), %bl
- {nf} \op\(w) $0xffff, (%rax)
+ {nf} \op\()w $0xffff, (%rax)
{nf} \op $-1, (%rax), %dx
- {nf} \op\(l) $0xffffffff, (%rax)
+ {nf} \op\()l $0xffffffff, (%rax)
{nf} \op $-1, (%rax), %ecx
- {nf} \op\(q) $-1, (%rax)
+ {nf} \op\()q $-1, (%rax)
{nf} \op $-1, (%rax), %r9
.endr
@@ -1444,13 +1444,13 @@ optimize:
{nf} ro\dir $63, %rdx
{nf} ro\dir $63, %rdx, %rax
- {nf} ro\dir\(b) $7, (%rdx)
+ {nf} ro\dir\()b $7, (%rdx)
{nf} ro\dir $7, (%rdx), %al
- {nf} ro\dir\(w) $15, (%rdx)
+ {nf} ro\dir\()w $15, (%rdx)
{nf} ro\dir $15, (%rdx), %ax
- {nf} ro\dir\(l) $31, (%rdx)
+ {nf} ro\dir\()l $31, (%rdx)
{nf} ro\dir $31, (%rdx), %eax
- {nf} ro\dir\(q) $63, (%rdx)
+ {nf} ro\dir\()q $63, (%rdx)
{nf} ro\dir $63, (%rdx), %rax
.endr
@@ -1476,10 +1476,10 @@ optimize:
# Note: 2-6 want leaving alone with -Os.
.irp n, 1, 2, 6, 7
# Note: 16-bit 3-operand src!=dst non-ZU form needs leaving alone.
- {nf} imul $1<<\n, %\r\(dx), %\r\(cx)
- {nf} imul $1<<\n, (%rdx), %\r\(cx)
- {nf} imul $1<<\n, %\r\(cx), %\r\(cx)
- {nf} imul $1<<\n, %\r\(cx)
+ {nf} imul $1<<\n, %\r\()dx, %\r\()cx
+ {nf} imul $1<<\n, (%rdx), %\r\()cx
+ {nf} imul $1<<\n, %\r\()cx, %\r\()cx
+ {nf} imul $1<<\n, %\r\()cx
.ifeqs "\r",""
{nf} imulzu $1<<\n, %dx, %cx
diff --git a/opcodes/cgen-asm.in b/opcodes/cgen-asm.in
index 8aab7e1ab83..3f5180434df 100644
--- a/opcodes/cgen-asm.in
+++ b/opcodes/cgen-asm.in
@@ -68,7 +68,6 @@ char *
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -106,15 +105,6 @@ char *
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -148,7 +138,6 @@ char *
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -206,8 +195,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -223,13 +214,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -243,28 +234,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -276,7 +257,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -286,12 +267,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/epiphany-asm.c b/opcodes/epiphany-asm.c
index ce10f48d6b3..6c3f549705d 100644
--- a/opcodes/epiphany-asm.c
+++ b/opcodes/epiphany-asm.c
@@ -499,7 +499,6 @@ epiphany_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -537,15 +536,6 @@ epiphany_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -579,7 +569,6 @@ epiphany_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -637,8 +626,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -654,13 +645,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -674,28 +665,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -707,7 +688,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -717,12 +698,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/fr30-asm.c b/opcodes/fr30-asm.c
index 966d36fbe2c..9ef15fed4c2 100644
--- a/opcodes/fr30-asm.c
+++ b/opcodes/fr30-asm.c
@@ -354,7 +354,6 @@ fr30_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -392,15 +391,6 @@ fr30_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -434,7 +424,6 @@ fr30_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -492,8 +481,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -509,13 +500,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -529,28 +520,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -562,7 +543,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -572,12 +553,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/frv-asm.c b/opcodes/frv-asm.c
index 9d125345048..6b64bf45299 100644
--- a/opcodes/frv-asm.c
+++ b/opcodes/frv-asm.c
@@ -1307,7 +1307,6 @@ frv_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -1345,15 +1344,6 @@ frv_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -1387,7 +1377,6 @@ frv_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -1445,8 +1434,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -1462,13 +1453,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -1482,28 +1473,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -1515,7 +1496,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -1525,12 +1506,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/ip2k-asm.c b/opcodes/ip2k-asm.c
index 1799025e5a4..003b78d9714 100644
--- a/opcodes/ip2k-asm.c
+++ b/opcodes/ip2k-asm.c
@@ -555,7 +555,6 @@ ip2k_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -593,15 +592,6 @@ ip2k_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -635,7 +625,6 @@ ip2k_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -693,8 +682,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -710,13 +701,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -730,28 +721,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -763,7 +744,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -773,12 +754,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/iq2000-asm.c b/opcodes/iq2000-asm.c
index 1a329c2cf85..66d828fd258 100644
--- a/opcodes/iq2000-asm.c
+++ b/opcodes/iq2000-asm.c
@@ -503,7 +503,6 @@ iq2000_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -541,15 +540,6 @@ iq2000_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -583,7 +573,6 @@ iq2000_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -641,8 +630,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -658,13 +649,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -678,28 +669,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -711,7 +692,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -721,12 +702,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/lm32-asm.c b/opcodes/lm32-asm.c
index 5c4f9f4b69f..d5e25f9e8b9 100644
--- a/opcodes/lm32-asm.c
+++ b/opcodes/lm32-asm.c
@@ -393,7 +393,6 @@ lm32_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -431,15 +430,6 @@ lm32_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -473,7 +463,6 @@ lm32_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -531,8 +520,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -548,13 +539,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -568,28 +559,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -601,7 +582,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -611,12 +592,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/m32c-asm.c b/opcodes/m32c-asm.c
index aebb59f0e26..8a7983c4c5b 100644
--- a/opcodes/m32c-asm.c
+++ b/opcodes/m32c-asm.c
@@ -1628,7 +1628,6 @@ m32c_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -1666,15 +1665,6 @@ m32c_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -1708,7 +1698,6 @@ m32c_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -1766,8 +1755,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -1783,13 +1774,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -1803,28 +1794,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -1836,7 +1817,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -1846,12 +1827,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/m32r-asm.c b/opcodes/m32r-asm.c
index ae6ed499142..4d19babdb00 100644
--- a/opcodes/m32r-asm.c
+++ b/opcodes/m32r-asm.c
@@ -372,7 +372,6 @@ m32r_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -410,15 +409,6 @@ m32r_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -452,7 +442,6 @@ m32r_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -510,8 +499,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -527,13 +518,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -547,28 +538,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -580,7 +561,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -590,12 +571,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/mep-asm.c b/opcodes/mep-asm.c
index 87af40ca844..6e72ddba5a1 100644
--- a/opcodes/mep-asm.c
+++ b/opcodes/mep-asm.c
@@ -1331,7 +1331,6 @@ mep_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -1369,15 +1368,6 @@ mep_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -1411,7 +1401,6 @@ mep_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -1469,8 +1458,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -1486,13 +1477,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -1506,28 +1497,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -1539,7 +1520,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -1549,12 +1530,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/mt-asm.c b/opcodes/mt-asm.c
index 8263670f7ac..074c1ab57ac 100644
--- a/opcodes/mt-asm.c
+++ b/opcodes/mt-asm.c
@@ -639,7 +639,6 @@ mt_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -677,15 +676,6 @@ mt_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -719,7 +709,6 @@ mt_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -777,8 +766,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -794,13 +785,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -814,28 +805,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -847,7 +828,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -857,12 +838,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/nds32-asm.c b/opcodes/nds32-asm.c
index f7a5e54f985..67a98336530 100644
--- a/opcodes/nds32-asm.c
+++ b/opcodes/nds32-asm.c
@@ -2486,9 +2486,6 @@ parse_insn (nds32_asm_desc_t *pdesc, nds32_asm_insn_t *pinsn,
while (*plex)
{
- if (ISSPACE (*p))
- ++p;
-
if (IS_LEX_CHAR (*plex))
{
/* If it's a plain char, just compare it. */
@@ -2533,8 +2530,6 @@ parse_insn (nds32_asm_desc_t *pdesc, nds32_asm_insn_t *pinsn,
}
/* Check whether this syntax is accepted. */
- if (ISSPACE (*p))
- ++p;
if (*plex == 0 && (*p == '\0' || *p == '!' || *p == '#'))
return 1;
diff --git a/opcodes/or1k-asm.c b/opcodes/or1k-asm.c
index 9351c1b407b..0839366788d 100644
--- a/opcodes/or1k-asm.c
+++ b/opcodes/or1k-asm.c
@@ -619,7 +619,6 @@ or1k_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -657,15 +656,6 @@ or1k_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -699,7 +689,6 @@ or1k_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -757,8 +746,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -774,13 +765,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -794,28 +785,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -827,7 +808,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -837,12 +818,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
diff --git a/opcodes/xstormy16-asm.c b/opcodes/xstormy16-asm.c
index 40c9265da18..8a9f33cdf93 100644
--- a/opcodes/xstormy16-asm.c
+++ b/opcodes/xstormy16-asm.c
@@ -320,7 +320,6 @@ xstormy16_cgen_build_insn_regex (CGEN_INSN *insn)
char rxbuf[CGEN_MAX_RX_ELEMENTS];
char *rx = rxbuf;
const CGEN_SYNTAX_CHAR_TYPE *syn;
- char prev_syntax_char = 0;
int reg_err;
syn = CGEN_SYNTAX_STRING (CGEN_OPCODE_SYNTAX (opc));
@@ -358,15 +357,6 @@ xstormy16_cgen_build_insn_regex (CGEN_INSN *insn)
{
char c = CGEN_SYNTAX_CHAR (* syn);
- /* See whitespace related comments in parse_insn_normal(). */
- if (c != ' ' && prev_syntax_char != ' '
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- {
- *rx++ = ' ';
- *rx++ = '*';
- }
- prev_syntax_char = c;
-
switch (c)
{
/* Escape any regex metacharacters in the syntax. */
@@ -400,7 +390,6 @@ xstormy16_cgen_build_insn_regex (CGEN_INSN *insn)
/* Replace non-syntax fields with globs. */
*rx++ = '.';
*rx++ = '*';
- prev_syntax_char = 0;
}
}
@@ -458,8 +447,10 @@ parse_insn_normal (CGEN_CPU_DESC cd,
const char *errmsg;
const char *p;
const CGEN_SYNTAX_CHAR_TYPE * syn;
- char prev_syntax_char = 0;
- bool past_opcode_p;
+#ifdef CGEN_MNEMONIC_OPERANDS
+ /* FIXME: wip */
+ int past_opcode_p;
+#endif
/* For now we assume the mnemonic is first (there are no leading operands).
We can parse it without needing to set up operand parsing.
@@ -475,13 +466,13 @@ parse_insn_normal (CGEN_CPU_DESC cd,
#ifndef CGEN_MNEMONIC_OPERANDS
if (* str && ! ISSPACE (* str))
return _("unrecognized instruction");
- past_opcode_p = true;
-#else
- past_opcode_p = false;
#endif
CGEN_INIT_PARSE (cd);
cgen_init_parse_operand (cd);
+#ifdef CGEN_MNEMONIC_OPERANDS
+ past_opcode_p = 0;
+#endif
/* We don't check for (*str != '\0') here because we want to parse
any trailing fake arguments in the syntax string. */
@@ -495,28 +486,18 @@ parse_insn_normal (CGEN_CPU_DESC cd,
while (* syn != 0)
{
- char c = CGEN_SYNTAX_CHAR_P (*syn) ? CGEN_SYNTAX_CHAR (*syn) : 0;
-
- /* FIXME: Despite this check we may still take inappropriate advantage of
- the fact that GAS's input scrubber will remove extraneous whitespace.
- We may also be a little too lax with this now, yet being more strict
- would require targets to indicate where whitespace is permissible. */
- if (past_opcode_p && c != ' ' && ISSPACE (*str)
- /* No whitespace between consecutive alphanumeric syntax elements. */
- && (!ISALNUM (c) || !ISALNUM (prev_syntax_char)))
- ++str;
- prev_syntax_char = c;
-
/* Non operand chars must match exactly. */
- if (c != 0)
+ if (CGEN_SYNTAX_CHAR_P (* syn))
{
/* FIXME: While we allow for non-GAS callers above, we assume the
first char after the mnemonic part is a space. */
- if (TOLOWER (*str) == TOLOWER (c))
+ /* FIXME: We also take inappropriate advantage of the fact that
+ GAS's input scrubber will remove extraneous blanks. */
+ if (TOLOWER (*str) == TOLOWER (CGEN_SYNTAX_CHAR (* syn)))
{
#ifdef CGEN_MNEMONIC_OPERANDS
- if (c == ' ')
- past_opcode_p = true;
+ if (CGEN_SYNTAX_CHAR(* syn) == ' ')
+ past_opcode_p = 1;
#endif
++ syn;
++ str;
@@ -528,7 +509,7 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found `%c')"),
- c, *str);
+ CGEN_SYNTAX_CHAR(*syn), *str);
return msg;
}
else
@@ -538,12 +519,15 @@ parse_insn_normal (CGEN_CPU_DESC cd,
/* xgettext:c-format */
sprintf (msg, _("syntax error (expected char `%c', found end of instruction)"),
- c);
+ CGEN_SYNTAX_CHAR(*syn));
return msg;
}
continue;
}
+#ifdef CGEN_MNEMONIC_OPERANDS
+ (void) past_opcode_p;
+#endif
/* We have an operand of some sort. */
errmsg = cd->parse_operand (cd, CGEN_SYNTAX_FIELD (*syn), &str, fields);
if (errmsg)
--
2.46.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-11 23:11 [PATCH 0/3] Restore macro with expression argument H.J. Lu
2024-08-11 23:11 ` [PATCH 1/3] Revert "gas: drop scrubber states 14 and 15" H.J. Lu
2024-08-11 23:11 ` [PATCH 2/3] Revert "gas: have scrubber retain more whitespace" H.J. Lu
@ 2024-08-11 23:11 ` H.J. Lu
2024-08-12 7:39 ` Jan Beulich
2024-08-11 23:20 ` [PATCH 0/3] Restore macro " Sam James
3 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2024-08-11 23:11 UTC (permalink / raw)
To: binutils; +Cc: amodra, jbeulich, nickc
Add a macro test with expression argument like
---
.macro test arg1
.byte \arg1
.endm
.data
test 0x10 + 0
test 0x10 + 1
---
PR gas/32073
* testsuite/gas/macros/arg1.d: New file.
* testsuite/gas/macros/arg1.s: Likewise.
* testsuite/gas/macros/macros.exp: Run arg1.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
gas/testsuite/gas/macros/arg1.d | 8 ++++++++
gas/testsuite/gas/macros/arg1.s | 7 +++++++
gas/testsuite/gas/macros/macros.exp | 2 ++
3 files changed, 17 insertions(+)
create mode 100644 gas/testsuite/gas/macros/arg1.d
create mode 100644 gas/testsuite/gas/macros/arg1.s
diff --git a/gas/testsuite/gas/macros/arg1.d b/gas/testsuite/gas/macros/arg1.d
new file mode 100644
index 00000000000..84e9cf7e6c8
--- /dev/null
+++ b/gas/testsuite/gas/macros/arg1.d
@@ -0,0 +1,8 @@
+#objdump: -s -j .data
+#name expression argument
+
+.*: .*
+
+Contents of section .data:
+ 0000 1011 ..
+#pass
diff --git a/gas/testsuite/gas/macros/arg1.s b/gas/testsuite/gas/macros/arg1.s
new file mode 100644
index 00000000000..8905f99e73b
--- /dev/null
+++ b/gas/testsuite/gas/macros/arg1.s
@@ -0,0 +1,7 @@
+ .macro test arg1
+ .byte \arg1
+ .endm
+
+ .data
+ test 0x10 + 0
+ test 0x10 + 1
diff --git a/gas/testsuite/gas/macros/macros.exp b/gas/testsuite/gas/macros/macros.exp
index bb5d4abf25b..3e84902c65f 100644
--- a/gas/testsuite/gas/macros/macros.exp
+++ b/gas/testsuite/gas/macros/macros.exp
@@ -75,6 +75,8 @@ if { ![istarget tic30-*-*] } {
run_list_test app6 ""
}
+run_dump_test arg1
+
run_list_test badarg ""
switch -glob $target_triplet {
--
2.46.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Restore macro with expression argument
2024-08-11 23:11 [PATCH 0/3] Restore macro with expression argument H.J. Lu
` (2 preceding siblings ...)
2024-08-11 23:11 ` [PATCH 3/3] gas: Add a macro test with expression argument H.J. Lu
@ 2024-08-11 23:20 ` Sam James
3 siblings, 0 replies; 19+ messages in thread
From: Sam James @ 2024-08-11 23:20 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils, amodra, jbeulich, nickc
[-- Attachment #1: Type: text/plain, Size: 720 bytes --]
"H.J. Lu" <hjl.tools@gmail.com> writes:
> commit 6ae8a30d44f016cafb46a75843b5109316eb1996
> Author: Jan Beulich <jbeulich@suse.com>
> Date: Fri Aug 9 11:59:31 2024 +0200
>
> gas: have scrubber retain more whitespace
>
> breaks GCC builds for ARM, AVR, PRU and others as well as x86-64 kernel
> build. Revert
>
> 7dd0dfbde7e gas: drop scrubber states 14 and 15
> 6ae8a30d44f gas: have scrubber retain more whitespace
>
> to restore macro with expression argument and add a testcase.
Please tag PR32073 and also the ML reports.
Anyway, thanks, I was going to send the same. I'm sure Jan will fix it
quickly but it's a pain to have to revert it locally everywhere and/or
keep telling people about known breakage.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-11 23:11 ` [PATCH 3/3] gas: Add a macro test with expression argument H.J. Lu
@ 2024-08-12 7:39 ` Jan Beulich
2024-08-12 11:21 ` H.J. Lu
0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2024-08-12 7:39 UTC (permalink / raw)
To: H.J. Lu, Sam James; +Cc: amodra, nickc, binutils
On 12.08.2024 01:11, H.J. Lu wrote:
> --- /dev/null
> +++ b/gas/testsuite/gas/macros/arg1.s
> @@ -0,0 +1,7 @@
> + .macro test arg1
> + .byte \arg1
> + .endm
> +
> + .data
> + test 0x10 + 0
> + test 0x10 + 1
So this is precisely what that entire change was about that you're proposing
to revert. Within binutils, instances of such broken macro invocations were
deliberately fixed up front. And the NEWS entry the commit added also is very
clear about the above no longer working (and never having been guaranteed to
work).
Just look at it the way it is textually present above: Knowing that macro
arguments don't require commas as separators, how many arguments do you see?
And no, using knowledge on the internal workings (brokenness) of the scrubber
is not allowed to determine the answer. (My answer: Three. And that's what
gas also should determine.)
If, purely from a practical / pragmatic perspective we'd really need to keep
the above working for some more time, then I expect we'll need to invent a
mode within which we can warn about such broken macros, telling people that
new behavior will be enforced in, say, the next release. Otherwise how do
you propose we ever address (without heuristics) the issues that the changes
at hand are actually aiming at addressing?
I did actually think about possible transitional states. Yet I didn't figure
any that would be halfway sane _and_ useful. For example, while we could add
a command line option to request old vs new scrubbing modes. To be useful
(for the purpose of fully transitioning sooner or later), that ought to
default to "new", though. Yet then people will need to fix their code anyway,
just (possibly) by adding the new command line option instead of touching
assembly sources.
And then: I deliberately waited for comments much longer than I would usually
have done. No-one really cared to comment on the changing behavior. And hence
I'm a little irritated that now not just a possible workaround is suggested,
but outright reverting.
Bottom line: Clearly a NAK for this testsuite addition. We must not test
accepting input in ways other than (more or less) documented. (Documentation
isn't great for macros, but the possibility of not using commas to separate
parameters and arguments can at least be derived from reading what's there.)
As to reverting the two commits while working out a possible transition path:
I'm open to that, provided some constructive comments actually surface, so
we / I have a way forward.
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 7:39 ` Jan Beulich
@ 2024-08-12 11:21 ` H.J. Lu
2024-08-12 12:03 ` Jan Beulich
0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2024-08-12 11:21 UTC (permalink / raw)
To: Jan Beulich; +Cc: Sam James, amodra, nickc, binutils
On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2024 01:11, H.J. Lu wrote:
> > --- /dev/null
> > +++ b/gas/testsuite/gas/macros/arg1.s
> > @@ -0,0 +1,7 @@
> > + .macro test arg1
> > + .byte \arg1
> > + .endm
> > +
> > + .data
> > + test 0x10 + 0
> > + test 0x10 + 1
>
> So this is precisely what that entire change was about that you're proposing
> to revert. Within binutils, instances of such broken macro invocations were
> deliberately fixed up front. And the NEWS entry the commit added also is very
> clear about the above no longer working (and never having been guaranteed to
> work).
>
> Just look at it the way it is textually present above: Knowing that macro
> arguments don't require commas as separators, how many arguments do you see?
> And no, using knowledge on the internal workings (brokenness) of the scrubber
> is not allowed to determine the answer. (My answer: Three. And that's what
> gas also should determine.)
>
> If, purely from a practical / pragmatic perspective we'd really need to keep
> the above working for some more time, then I expect we'll need to invent a
> mode within which we can warn about such broken macros, telling people that
> new behavior will be enforced in, say, the next release. Otherwise how do
> you propose we ever address (without heuristics) the issues that the changes
> at hand are actually aiming at addressing?
>
> I did actually think about possible transitional states. Yet I didn't figure
> any that would be halfway sane _and_ useful. For example, while we could add
> a command line option to request old vs new scrubbing modes. To be useful
> (for the purpose of fully transitioning sooner or later), that ought to
> default to "new", though. Yet then people will need to fix their code anyway,
> just (possibly) by adding the new command line option instead of touching
> assembly sources.
>
> And then: I deliberately waited for comments much longer than I would usually
> have done. No-one really cared to comment on the changing behavior. And hence
> I'm a little irritated that now not just a possible workaround is suggested,
> but outright reverting.
>
> Bottom line: Clearly a NAK for this testsuite addition. We must not test
> accepting input in ways other than (more or less) documented. (Documentation
> isn't great for macros, but the possibility of not using commas to separate
> parameters and arguments can at least be derived from reading what's there.)
> As to reverting the two commits while working out a possible transition path:
> I'm open to that, provided some constructive comments actually surface, so
> we / I have a way forward.
>
> Jan
At very least, we must support the old behavior with a command-line
option which can be used with the testcase.
--
H.J.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 11:21 ` H.J. Lu
@ 2024-08-12 12:03 ` Jan Beulich
2024-08-12 12:08 ` H.J. Lu
2024-08-12 12:45 ` Sam James
0 siblings, 2 replies; 19+ messages in thread
From: Jan Beulich @ 2024-08-12 12:03 UTC (permalink / raw)
To: H.J. Lu; +Cc: Sam James, amodra, nickc, binutils
On 12.08.2024 13:21, H.J. Lu wrote:
> On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 12.08.2024 01:11, H.J. Lu wrote:
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/macros/arg1.s
>>> @@ -0,0 +1,7 @@
>>> + .macro test arg1
>>> + .byte \arg1
>>> + .endm
>>> +
>>> + .data
>>> + test 0x10 + 0
>>> + test 0x10 + 1
>>
>> So this is precisely what that entire change was about that you're proposing
>> to revert. Within binutils, instances of such broken macro invocations were
>> deliberately fixed up front. And the NEWS entry the commit added also is very
>> clear about the above no longer working (and never having been guaranteed to
>> work).
>>
>> Just look at it the way it is textually present above: Knowing that macro
>> arguments don't require commas as separators, how many arguments do you see?
>> And no, using knowledge on the internal workings (brokenness) of the scrubber
>> is not allowed to determine the answer. (My answer: Three. And that's what
>> gas also should determine.)
>>
>> If, purely from a practical / pragmatic perspective we'd really need to keep
>> the above working for some more time, then I expect we'll need to invent a
>> mode within which we can warn about such broken macros, telling people that
>> new behavior will be enforced in, say, the next release. Otherwise how do
>> you propose we ever address (without heuristics) the issues that the changes
>> at hand are actually aiming at addressing?
>>
>> I did actually think about possible transitional states. Yet I didn't figure
>> any that would be halfway sane _and_ useful. For example, while we could add
>> a command line option to request old vs new scrubbing modes. To be useful
>> (for the purpose of fully transitioning sooner or later), that ought to
>> default to "new", though. Yet then people will need to fix their code anyway,
>> just (possibly) by adding the new command line option instead of touching
>> assembly sources.
>>
>> And then: I deliberately waited for comments much longer than I would usually
>> have done. No-one really cared to comment on the changing behavior. And hence
>> I'm a little irritated that now not just a possible workaround is suggested,
>> but outright reverting.
>>
>> Bottom line: Clearly a NAK for this testsuite addition. We must not test
>> accepting input in ways other than (more or less) documented. (Documentation
>> isn't great for macros, but the possibility of not using commas to separate
>> parameters and arguments can at least be derived from reading what's there.)
>> As to reverting the two commits while working out a possible transition path:
>> I'm open to that, provided some constructive comments actually surface, so
>> we / I have a way forward.
>
> At very least, we must support the old behavior with a command-line
> option which can be used with the testcase.
I disagree with "must". It may be desirable for a transitional phase, yes.
Yet as said - will a command line option (which people will need to add in
their build systems) really help that much? IOW is it expected to be much
more work for them to instead fix their macro use right away (which they
will need to do at some point anyway, as the option would be going away
again after a release or two)?
However, would you (and others) further insist that other bogus behavior
also be retained? (More a rhetorical question perhaps, since if we need
to keep the old scrubber optionally available, it'll likely be easier to
just keep it as is (was), and have such a command line option simply pick
between the two scrubber instances.)
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 12:03 ` Jan Beulich
@ 2024-08-12 12:08 ` H.J. Lu
2024-08-12 12:36 ` Jan Beulich
2024-08-12 12:45 ` Sam James
1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2024-08-12 12:08 UTC (permalink / raw)
To: Jan Beulich; +Cc: Sam James, amodra, nickc, binutils
On Mon, Aug 12, 2024 at 5:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2024 13:21, H.J. Lu wrote:
> > On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 12.08.2024 01:11, H.J. Lu wrote:
> >>> --- /dev/null
> >>> +++ b/gas/testsuite/gas/macros/arg1.s
> >>> @@ -0,0 +1,7 @@
> >>> + .macro test arg1
> >>> + .byte \arg1
> >>> + .endm
> >>> +
> >>> + .data
> >>> + test 0x10 + 0
> >>> + test 0x10 + 1
> >>
> >> So this is precisely what that entire change was about that you're proposing
> >> to revert. Within binutils, instances of such broken macro invocations were
> >> deliberately fixed up front. And the NEWS entry the commit added also is very
> >> clear about the above no longer working (and never having been guaranteed to
> >> work).
> >>
> >> Just look at it the way it is textually present above: Knowing that macro
> >> arguments don't require commas as separators, how many arguments do you see?
> >> And no, using knowledge on the internal workings (brokenness) of the scrubber
> >> is not allowed to determine the answer. (My answer: Three. And that's what
> >> gas also should determine.)
> >>
> >> If, purely from a practical / pragmatic perspective we'd really need to keep
> >> the above working for some more time, then I expect we'll need to invent a
> >> mode within which we can warn about such broken macros, telling people that
> >> new behavior will be enforced in, say, the next release. Otherwise how do
> >> you propose we ever address (without heuristics) the issues that the changes
> >> at hand are actually aiming at addressing?
> >>
> >> I did actually think about possible transitional states. Yet I didn't figure
> >> any that would be halfway sane _and_ useful. For example, while we could add
> >> a command line option to request old vs new scrubbing modes. To be useful
> >> (for the purpose of fully transitioning sooner or later), that ought to
> >> default to "new", though. Yet then people will need to fix their code anyway,
> >> just (possibly) by adding the new command line option instead of touching
> >> assembly sources.
> >>
> >> And then: I deliberately waited for comments much longer than I would usually
> >> have done. No-one really cared to comment on the changing behavior. And hence
> >> I'm a little irritated that now not just a possible workaround is suggested,
> >> but outright reverting.
> >>
> >> Bottom line: Clearly a NAK for this testsuite addition. We must not test
> >> accepting input in ways other than (more or less) documented. (Documentation
> >> isn't great for macros, but the possibility of not using commas to separate
> >> parameters and arguments can at least be derived from reading what's there.)
> >> As to reverting the two commits while working out a possible transition path:
> >> I'm open to that, provided some constructive comments actually surface, so
> >> we / I have a way forward.
> >
> > At very least, we must support the old behavior with a command-line
> > option which can be used with the testcase.
>
> I disagree with "must". It may be desirable for a transitional phase, yes.
> Yet as said - will a command line option (which people will need to add in
> their build systems) really help that much? IOW is it expected to be much
> more work for them to instead fix their macro use right away (which they
> will need to do at some point anyway, as the option would be going away
> again after a release or two)?
>
> However, would you (and others) further insist that other bogus behavior
> also be retained? (More a rhetorical question perhaps, since if we need
> to keep the old scrubber optionally available, it'll likely be easier to
> just keep it as is (was), and have such a command line option simply pick
> between the two scrubber instances.)
>
The old behaviors may not be well documented. But many important
packages depend on them. It is unrealistic to change all of them after
GNU assembler has been working on them for so many years. I think
it is a very bad idea to change the GNU assembler in such a way which
breaks so many packages. We should add the missing testcases and
document these behaviors instead.
--
H.J.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 12:08 ` H.J. Lu
@ 2024-08-12 12:36 ` Jan Beulich
2024-08-12 12:41 ` H.J. Lu
0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2024-08-12 12:36 UTC (permalink / raw)
To: H.J. Lu, Nick Clifton; +Cc: Sam James, amodra, binutils
On 12.08.2024 14:08, H.J. Lu wrote:
> On Mon, Aug 12, 2024 at 5:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 12.08.2024 13:21, H.J. Lu wrote:
>>> On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 12.08.2024 01:11, H.J. Lu wrote:
>>>>> --- /dev/null
>>>>> +++ b/gas/testsuite/gas/macros/arg1.s
>>>>> @@ -0,0 +1,7 @@
>>>>> + .macro test arg1
>>>>> + .byte \arg1
>>>>> + .endm
>>>>> +
>>>>> + .data
>>>>> + test 0x10 + 0
>>>>> + test 0x10 + 1
>>>>
>>>> So this is precisely what that entire change was about that you're proposing
>>>> to revert. Within binutils, instances of such broken macro invocations were
>>>> deliberately fixed up front. And the NEWS entry the commit added also is very
>>>> clear about the above no longer working (and never having been guaranteed to
>>>> work).
>>>>
>>>> Just look at it the way it is textually present above: Knowing that macro
>>>> arguments don't require commas as separators, how many arguments do you see?
>>>> And no, using knowledge on the internal workings (brokenness) of the scrubber
>>>> is not allowed to determine the answer. (My answer: Three. And that's what
>>>> gas also should determine.)
>>>>
>>>> If, purely from a practical / pragmatic perspective we'd really need to keep
>>>> the above working for some more time, then I expect we'll need to invent a
>>>> mode within which we can warn about such broken macros, telling people that
>>>> new behavior will be enforced in, say, the next release. Otherwise how do
>>>> you propose we ever address (without heuristics) the issues that the changes
>>>> at hand are actually aiming at addressing?
>>>>
>>>> I did actually think about possible transitional states. Yet I didn't figure
>>>> any that would be halfway sane _and_ useful. For example, while we could add
>>>> a command line option to request old vs new scrubbing modes. To be useful
>>>> (for the purpose of fully transitioning sooner or later), that ought to
>>>> default to "new", though. Yet then people will need to fix their code anyway,
>>>> just (possibly) by adding the new command line option instead of touching
>>>> assembly sources.
>>>>
>>>> And then: I deliberately waited for comments much longer than I would usually
>>>> have done. No-one really cared to comment on the changing behavior. And hence
>>>> I'm a little irritated that now not just a possible workaround is suggested,
>>>> but outright reverting.
>>>>
>>>> Bottom line: Clearly a NAK for this testsuite addition. We must not test
>>>> accepting input in ways other than (more or less) documented. (Documentation
>>>> isn't great for macros, but the possibility of not using commas to separate
>>>> parameters and arguments can at least be derived from reading what's there.)
>>>> As to reverting the two commits while working out a possible transition path:
>>>> I'm open to that, provided some constructive comments actually surface, so
>>>> we / I have a way forward.
>>>
>>> At very least, we must support the old behavior with a command-line
>>> option which can be used with the testcase.
>>
>> I disagree with "must". It may be desirable for a transitional phase, yes.
>> Yet as said - will a command line option (which people will need to add in
>> their build systems) really help that much? IOW is it expected to be much
>> more work for them to instead fix their macro use right away (which they
>> will need to do at some point anyway, as the option would be going away
>> again after a release or two)?
>>
>> However, would you (and others) further insist that other bogus behavior
>> also be retained? (More a rhetorical question perhaps, since if we need
>> to keep the old scrubber optionally available, it'll likely be easier to
>> just keep it as is (was), and have such a command line option simply pick
>> between the two scrubber instances.)
>
> The old behaviors may not be well documented. But many important
> packages depend on them. It is unrealistic to change all of them after
> GNU assembler has been working on them for so many years. I think
> it is a very bad idea to change the GNU assembler in such a way which
> breaks so many packages. We should add the missing testcases and
> document these behaviors instead.
Well, good luck with documenting everything that's going wrong. Just today,
when investigating the fallout Andreas reported, I found yet another one
(which my changes also happen to correct).
Still it may indeed be necessary to retain the original behavior. Whether
indefinitely or for some time is hard to tell. Personally I think it should
be permissible to fix issues there. (If it wasn't, why did absolutely
nobody respond early on, when I first posted the work, saving me from
putting in quite a bit of time and effort?)
Nick, I think this is fundamental enough a question that I'd like to ask
for your input.
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 12:36 ` Jan Beulich
@ 2024-08-12 12:41 ` H.J. Lu
0 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2024-08-12 12:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: Nick Clifton, Sam James, amodra, binutils
On Mon, Aug 12, 2024 at 5:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2024 14:08, H.J. Lu wrote:
> > On Mon, Aug 12, 2024 at 5:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 12.08.2024 13:21, H.J. Lu wrote:
> >>> On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 12.08.2024 01:11, H.J. Lu wrote:
> >>>>> --- /dev/null
> >>>>> +++ b/gas/testsuite/gas/macros/arg1.s
> >>>>> @@ -0,0 +1,7 @@
> >>>>> + .macro test arg1
> >>>>> + .byte \arg1
> >>>>> + .endm
> >>>>> +
> >>>>> + .data
> >>>>> + test 0x10 + 0
> >>>>> + test 0x10 + 1
> >>>>
> >>>> So this is precisely what that entire change was about that you're proposing
> >>>> to revert. Within binutils, instances of such broken macro invocations were
> >>>> deliberately fixed up front. And the NEWS entry the commit added also is very
> >>>> clear about the above no longer working (and never having been guaranteed to
> >>>> work).
> >>>>
> >>>> Just look at it the way it is textually present above: Knowing that macro
> >>>> arguments don't require commas as separators, how many arguments do you see?
> >>>> And no, using knowledge on the internal workings (brokenness) of the scrubber
> >>>> is not allowed to determine the answer. (My answer: Three. And that's what
> >>>> gas also should determine.)
> >>>>
> >>>> If, purely from a practical / pragmatic perspective we'd really need to keep
> >>>> the above working for some more time, then I expect we'll need to invent a
> >>>> mode within which we can warn about such broken macros, telling people that
> >>>> new behavior will be enforced in, say, the next release. Otherwise how do
> >>>> you propose we ever address (without heuristics) the issues that the changes
> >>>> at hand are actually aiming at addressing?
> >>>>
> >>>> I did actually think about possible transitional states. Yet I didn't figure
> >>>> any that would be halfway sane _and_ useful. For example, while we could add
> >>>> a command line option to request old vs new scrubbing modes. To be useful
> >>>> (for the purpose of fully transitioning sooner or later), that ought to
> >>>> default to "new", though. Yet then people will need to fix their code anyway,
> >>>> just (possibly) by adding the new command line option instead of touching
> >>>> assembly sources.
> >>>>
> >>>> And then: I deliberately waited for comments much longer than I would usually
> >>>> have done. No-one really cared to comment on the changing behavior. And hence
> >>>> I'm a little irritated that now not just a possible workaround is suggested,
> >>>> but outright reverting.
> >>>>
> >>>> Bottom line: Clearly a NAK for this testsuite addition. We must not test
> >>>> accepting input in ways other than (more or less) documented. (Documentation
> >>>> isn't great for macros, but the possibility of not using commas to separate
> >>>> parameters and arguments can at least be derived from reading what's there.)
> >>>> As to reverting the two commits while working out a possible transition path:
> >>>> I'm open to that, provided some constructive comments actually surface, so
> >>>> we / I have a way forward.
> >>>
> >>> At very least, we must support the old behavior with a command-line
> >>> option which can be used with the testcase.
> >>
> >> I disagree with "must". It may be desirable for a transitional phase, yes.
> >> Yet as said - will a command line option (which people will need to add in
> >> their build systems) really help that much? IOW is it expected to be much
> >> more work for them to instead fix their macro use right away (which they
> >> will need to do at some point anyway, as the option would be going away
> >> again after a release or two)?
> >>
> >> However, would you (and others) further insist that other bogus behavior
> >> also be retained? (More a rhetorical question perhaps, since if we need
> >> to keep the old scrubber optionally available, it'll likely be easier to
> >> just keep it as is (was), and have such a command line option simply pick
> >> between the two scrubber instances.)
> >
> > The old behaviors may not be well documented. But many important
> > packages depend on them. It is unrealistic to change all of them after
> > GNU assembler has been working on them for so many years. I think
> > it is a very bad idea to change the GNU assembler in such a way which
> > breaks so many packages. We should add the missing testcases and
> > document these behaviors instead.
>
> Well, good luck with documenting everything that's going wrong. Just today,
> when investigating the fallout Andreas reported, I found yet another one
Is there a binutil bug for this?
> (which my changes also happen to correct).
>
> Still it may indeed be necessary to retain the original behavior. Whether
> indefinitely or for some time is hard to tell. Personally I think it should
> be permissible to fix issues there. (If it wasn't, why did absolutely
> nobody respond early on, when I first posted the work, saving me from
Developers who use the old behavior may not be on the binutils mailing
list. Even if they are on the list, they may not try your patches. Personally
I alway test my binitils changes on glibc, gcc and kernel on x86-64.
> putting in quite a bit of time and effort?)
>
> Nick, I think this is fundamental enough a question that I'd like to ask
> for your input.
>
> Jan
--
H.J.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 12:03 ` Jan Beulich
2024-08-12 12:08 ` H.J. Lu
@ 2024-08-12 12:45 ` Sam James
2024-08-12 13:02 ` Jan Beulich
1 sibling, 1 reply; 19+ messages in thread
From: Sam James @ 2024-08-12 12:45 UTC (permalink / raw)
To: Jan Beulich; +Cc: H.J. Lu, amodra, nickc, binutils
[-- Attachment #1: Type: text/plain, Size: 5262 bytes --]
Jan Beulich <jbeulich@suse.com> writes:
> On 12.08.2024 13:21, H.J. Lu wrote:
>> On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 12.08.2024 01:11, H.J. Lu wrote:
>>>> --- /dev/null
>>>> +++ b/gas/testsuite/gas/macros/arg1.s
>>>> @@ -0,0 +1,7 @@
>>>> + .macro test arg1
>>>> + .byte \arg1
>>>> + .endm
>>>> +
>>>> + .data
>>>> + test 0x10 + 0
>>>> + test 0x10 + 1
>>>
>>> So this is precisely what that entire change was about that you're proposing
>>> to revert. Within binutils, instances of such broken macro invocations were
>>> deliberately fixed up front. And the NEWS entry the commit added also is very
>>> clear about the above no longer working (and never having been guaranteed to
>>> work).
>>>
>>> Just look at it the way it is textually present above: Knowing that macro
>>> arguments don't require commas as separators, how many arguments do you see?
>>> And no, using knowledge on the internal workings (brokenness) of the scrubber
>>> is not allowed to determine the answer. (My answer: Three. And that's what
>>> gas also should determine.)
>>>
>>> If, purely from a practical / pragmatic perspective we'd really need to keep
>>> the above working for some more time, then I expect we'll need to invent a
>>> mode within which we can warn about such broken macros, telling people that
>>> new behavior will be enforced in, say, the next release. Otherwise how do
>>> you propose we ever address (without heuristics) the issues that the changes
>>> at hand are actually aiming at addressing?
>>>
>>> I did actually think about possible transitional states. Yet I didn't figure
>>> any that would be halfway sane _and_ useful. For example, while we could add
>>> a command line option to request old vs new scrubbing modes. To be useful
>>> (for the purpose of fully transitioning sooner or later), that ought to
>>> default to "new", though. Yet then people will need to fix their code anyway,
>>> just (possibly) by adding the new command line option instead of touching
>>> assembly sources.
>>>
>>> And then: I deliberately waited for comments much longer than I would usually
>>> have done. No-one really cared to comment on the changing behavior. And hence
>>> I'm a little irritated that now not just a possible workaround is suggested,
>>> but outright reverting.
>>>
>>> Bottom line: Clearly a NAK for this testsuite addition. We must not test
>>> accepting input in ways other than (more or less) documented. (Documentation
>>> isn't great for macros, but the possibility of not using commas to separate
>>> parameters and arguments can at least be derived from reading what's there.)
>>> As to reverting the two commits while working out a possible transition path:
>>> I'm open to that, provided some constructive comments actually surface, so
>>> we / I have a way forward.
>>
>> At very least, we must support the old behavior with a command-line
>> option which can be used with the testcase.
>
> I disagree with "must". It may be desirable for a transitional phase, yes.
> Yet as said - will a command line option (which people will need to add in
> their build systems) really help that much? IOW is it expected to be much
> more work for them to instead fix their macro use right away (which they
> will need to do at some point anyway, as the option would be going away
> again after a release or two)?
>
> However, would you (and others) further insist that other bogus behavior
> also be retained? (More a rhetorical question perhaps, since if we need
> to keep the old scrubber optionally available, it'll likely be easier to
> just keep it as is (was), and have such a command line option simply pick
> between the two scrubber instances.)
The only thing I 'insist' on is doing this in an orderly way where we're
clear on what the future behaviour is, so I know what to report to
upstreams & include in commit messages fixing them, including temporary
workarounds.
I have my own views on backwards compatibility but it's not really what
I'm talking about here. I'm mostly interested in:
1) is this intentional (looks like yes, but unclear if that's true for
all cases);
2) is there a temporary workaround to tell people (like a cmdline option);
3) assessing the scale of breakage (it's unclear so far how many of
these issues are the same or not);
4) reporting issues upstream;
5) fixing issues upstream;
It's hard to actually fix anything until we're clear on how much of it
is the new behaviour. I leave discussions on handling historical
codebases to others. But something's wrong if we have GCC failing to
build on non-obscure targets and glibc failing on amd64? It's not a
matter of simply a handful of niche projects getting it wrong, which is
how we ended up with H.J. sending the revert series.
I'd suggest reverting, adding a command line option to opt-in, ask
people to do test-runs en-masse with that, we say clearly where to
report issues at first (maybe on sw bz to be assessed), and go from there.
Upstreams won't be interested in reports right now as it's unclear what
the official position for binutils is going forward.
thanks,
sam
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 12:45 ` Sam James
@ 2024-08-12 13:02 ` Jan Beulich
2024-08-12 13:19 ` Sam James
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Jan Beulich @ 2024-08-12 13:02 UTC (permalink / raw)
To: Sam James; +Cc: H.J. Lu, amodra, nickc, binutils
On 12.08.2024 14:45, Sam James wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> On 12.08.2024 13:21, H.J. Lu wrote:
>>> On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 12.08.2024 01:11, H.J. Lu wrote:
>>>>> --- /dev/null
>>>>> +++ b/gas/testsuite/gas/macros/arg1.s
>>>>> @@ -0,0 +1,7 @@
>>>>> + .macro test arg1
>>>>> + .byte \arg1
>>>>> + .endm
>>>>> +
>>>>> + .data
>>>>> + test 0x10 + 0
>>>>> + test 0x10 + 1
>>>>
>>>> So this is precisely what that entire change was about that you're proposing
>>>> to revert. Within binutils, instances of such broken macro invocations were
>>>> deliberately fixed up front. And the NEWS entry the commit added also is very
>>>> clear about the above no longer working (and never having been guaranteed to
>>>> work).
>>>>
>>>> Just look at it the way it is textually present above: Knowing that macro
>>>> arguments don't require commas as separators, how many arguments do you see?
>>>> And no, using knowledge on the internal workings (brokenness) of the scrubber
>>>> is not allowed to determine the answer. (My answer: Three. And that's what
>>>> gas also should determine.)
>>>>
>>>> If, purely from a practical / pragmatic perspective we'd really need to keep
>>>> the above working for some more time, then I expect we'll need to invent a
>>>> mode within which we can warn about such broken macros, telling people that
>>>> new behavior will be enforced in, say, the next release. Otherwise how do
>>>> you propose we ever address (without heuristics) the issues that the changes
>>>> at hand are actually aiming at addressing?
>>>>
>>>> I did actually think about possible transitional states. Yet I didn't figure
>>>> any that would be halfway sane _and_ useful. For example, while we could add
>>>> a command line option to request old vs new scrubbing modes. To be useful
>>>> (for the purpose of fully transitioning sooner or later), that ought to
>>>> default to "new", though. Yet then people will need to fix their code anyway,
>>>> just (possibly) by adding the new command line option instead of touching
>>>> assembly sources.
>>>>
>>>> And then: I deliberately waited for comments much longer than I would usually
>>>> have done. No-one really cared to comment on the changing behavior. And hence
>>>> I'm a little irritated that now not just a possible workaround is suggested,
>>>> but outright reverting.
>>>>
>>>> Bottom line: Clearly a NAK for this testsuite addition. We must not test
>>>> accepting input in ways other than (more or less) documented. (Documentation
>>>> isn't great for macros, but the possibility of not using commas to separate
>>>> parameters and arguments can at least be derived from reading what's there.)
>>>> As to reverting the two commits while working out a possible transition path:
>>>> I'm open to that, provided some constructive comments actually surface, so
>>>> we / I have a way forward.
>>>
>>> At very least, we must support the old behavior with a command-line
>>> option which can be used with the testcase.
>>
>> I disagree with "must". It may be desirable for a transitional phase, yes.
>> Yet as said - will a command line option (which people will need to add in
>> their build systems) really help that much? IOW is it expected to be much
>> more work for them to instead fix their macro use right away (which they
>> will need to do at some point anyway, as the option would be going away
>> again after a release or two)?
>>
>> However, would you (and others) further insist that other bogus behavior
>> also be retained? (More a rhetorical question perhaps, since if we need
>> to keep the old scrubber optionally available, it'll likely be easier to
>> just keep it as is (was), and have such a command line option simply pick
>> between the two scrubber instances.)
>
> The only thing I 'insist' on is doing this in an orderly way where we're
> clear on what the future behaviour is, so I know what to report to
> upstreams & include in commit messages fixing them, including temporary
> workarounds.
>
> I have my own views on backwards compatibility but it's not really what
> I'm talking about here. I'm mostly interested in:
> 1) is this intentional (looks like yes, but unclear if that's true for
> all cases);
It's all intentional, but there are going to be bugs. Andreas likely
reported a case which needs correcting.
> 2) is there a temporary workaround to tell people (like a cmdline option);
There's no command line option. But people can of course use well-
formed code. Which would likely cover all of the macro use problems
that were reported so far.
> 3) assessing the scale of breakage (it's unclear so far how many of
> these issues are the same or not);
> 4) reporting issues upstream;
> 5) fixing issues upstream;
>
> It's hard to actually fix anything until we're clear on how much of it
> is the new behaviour. I leave discussions on handling historical
> codebases to others. But something's wrong if we have GCC failing to
> build on non-obscure targets and glibc failing on amd64? It's not a
> matter of simply a handful of niche projects getting it wrong, which is
> how we ended up with H.J. sending the revert series.
Well, I will certainly admit that I didn't expect the macro issue to
be this widespread. Yet at the same time trying to be yet more careful
won't work either - I can't really fix all affected targets in all of
Linux, glibc, gcc, and who knows what not. It was enough work already
to get binutils alone sorted. And I also can't reasonably wait, or
I'll be waiting for years.
> I'd suggest reverting, adding a command line option to opt-in, ask
> people to do test-runs en-masse with that, we say clearly where to
> report issues at first (maybe on sw bz to be assessed), and go from there.
And what would make people even try the new mode, which likely hardly
anyone would be aware of if it wasn't (right now) default behavior?
"Ask people" is what simply isn't going to work, from my experience.
Just as much as - I said this before - expecting feedback to the
patch submission didn't really work out. And I'm not talking of patch
review, feedback on the intended change in behavior would already
have helped (provided it would have been constructive and not just
"no, you can't do that").
> Upstreams won't be interested in reports right now as it's unclear what
> the official position for binutils is going forward.
I'm afraid I don't really understand this part. Who's "upstreams" here?
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 13:02 ` Jan Beulich
@ 2024-08-12 13:19 ` Sam James
2024-08-12 13:45 ` Jan Beulich
2024-08-12 13:24 ` Sam James
2024-08-13 7:27 ` Andreas K. Huettel
2 siblings, 1 reply; 19+ messages in thread
From: Sam James @ 2024-08-12 13:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: H.J. Lu, amodra, nickc, binutils
[-- Attachment #1: Type: text/plain, Size: 4054 bytes --]
Jan Beulich <jbeulich@suse.com> writes:
> On 12.08.2024 14:45, Sam James wrote:
> [...]
>> I have my own views on backwards compatibility but it's not really what
>> I'm talking about here. I'm mostly interested in:
>> 1) is this intentional (looks like yes, but unclear if that's true for
>> all cases);
>
> It's all intentional, but there are going to be bugs. Andreas likely
> reported a case which needs correcting.
>
OK, thanks.
>> 2) is there a temporary workaround to tell people (like a cmdline option);
>
> There's no command line option. But people can of course use well-
> formed code. Which would likely cover all of the macro use problems
> that were reported so far.
>
Right.
>> 3) assessing the scale of breakage (it's unclear so far how many of
>> these issues are the same or not);
>> 4) reporting issues upstream;
>> 5) fixing issues upstream;
>>
>> It's hard to actually fix anything until we're clear on how much of it
>> is the new behaviour. I leave discussions on handling historical
>> codebases to others. But something's wrong if we have GCC failing to
>> build on non-obscure targets and glibc failing on amd64? It's not a
>> matter of simply a handful of niche projects getting it wrong, which is
>> how we ended up with H.J. sending the revert series.
>
> Well, I will certainly admit that I didn't expect the macro issue to
> be this widespread. Yet at the same time trying to be yet more careful
> won't work either - I can't really fix all affected targets in all of
> Linux, glibc, gcc, and who knows what not. It was enough work already
> to get binutils alone sorted. And I also can't reasonably wait, or
> I'll be waiting for years.
>
Yes, of course not asking you to do that. But having amd64 glibc/linux
broken is the other end of the extreme.
>> I'd suggest reverting, adding a command line option to opt-in, ask
>> people to do test-runs en-masse with that, we say clearly where to
>> report issues at first (maybe on sw bz to be assessed), and go from there.
>
> And what would make people even try the new mode, which likely hardly
> anyone would be aware of if it wasn't (right now) default behavior?
> "Ask people" is what simply isn't going to work, from my experience.
> Just as much as - I said this before - expecting feedback to the
> patch submission didn't really work out. And I'm not talking of patch
> review, feedback on the intended change in behavior would already
> have helped (provided it would have been constructive and not just
> "no, you can't do that").
I at least would do such testing en-masse when asked. I already try to
do it when I see a time it'd be useful.
I can't speak for others. I do try to watch the ML and test changes when
they look obviously "observable" (or after they land). I think the
Fedora people have some tooling to do mass rebuilds with experimental
toolchain changes as well.
I didn't read the deep gas thread about the lexer cleanup.
>
>> Upstreams won't be interested in reports right now as it's unclear what
>> the official position for binutils is going forward.
>
> I'm afraid I don't really understand this part. Who's "upstreams" here?
>
Sorry, I guess it's confusing here in the context of binutils. I mean
maintainers of software that's broken.
It's not clear to me if I should actually be reporting anything anywhere
yet, given https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116339#c11.
(Is the bug valid? If so, let's reopen it and let the libgcc/arm
maintainers sort it out. If it's not valid, what are we doing here,
because it's inconsistent with my understanding of this discussion?)
That's part of the problem. You're saying the changes are intentional,
but also that you don't know what quirks we are/aren't going to
implement, so we're left in limbo with master as-is? I guess we're
waiting for Nick's input. That is the argument in favour of reverting
it, because the answer right now isn't even clearly "go fix gcc"?
thanks,
sam
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 13:02 ` Jan Beulich
2024-08-12 13:19 ` Sam James
@ 2024-08-12 13:24 ` Sam James
2024-08-13 7:27 ` Andreas K. Huettel
2 siblings, 0 replies; 19+ messages in thread
From: Sam James @ 2024-08-12 13:24 UTC (permalink / raw)
To: Jan Beulich; +Cc: H.J. Lu, amodra, nickc, binutils
[-- Attachment #1: Type: text/plain, Size: 491 bytes --]
Jan Beulich <jbeulich@suse.com> writes:
> On 12.08.2024 14:45, Sam James wrote:
> [...]
Just to say explicitly: I'm grateful you're working on a particularly
gnarly bit of gas, breakage does happen, and I'm happy to do whatever
testing on our package set if it helps (offer is recurring).
I just need to explain the situation accurately to those with now-broken
software, and I'd also like to avoid confused bug reports showing up in
e.g. gcc, kernel MLs while we get our house in order.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 13:19 ` Sam James
@ 2024-08-12 13:45 ` Jan Beulich
2024-08-14 7:37 ` Fangrui Song
0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2024-08-12 13:45 UTC (permalink / raw)
To: Sam James; +Cc: H.J. Lu, amodra, nickc, binutils
On 12.08.2024 15:19, Sam James wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> On 12.08.2024 14:45, Sam James wrote:
>>> Upstreams won't be interested in reports right now as it's unclear what
>>> the official position for binutils is going forward.
>>
>> I'm afraid I don't really understand this part. Who's "upstreams" here?
>>
>
> Sorry, I guess it's confusing here in the context of binutils. I mean
> maintainers of software that's broken.
>
> It's not clear to me if I should actually be reporting anything anywhere
> yet, given https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116339#c11.
>
> (Is the bug valid? If so, let's reopen it and let the libgcc/arm
> maintainers sort it out. If it's not valid, what are we doing here,
> because it's inconsistent with my understanding of this discussion?)
>
> That's part of the problem. You're saying the changes are intentional,
> but also that you don't know what quirks we are/aren't going to
> implement, so we're left in limbo with master as-is? I guess we're
> waiting for Nick's input. That is the argument in favour of reverting
> it, because the answer right now isn't even clearly "go fix gcc"?
Well. I would have done the revert already, if it wasn't overwhelmingly
likely that then the discussion here dies out almost immediately.
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 13:02 ` Jan Beulich
2024-08-12 13:19 ` Sam James
2024-08-12 13:24 ` Sam James
@ 2024-08-13 7:27 ` Andreas K. Huettel
2024-08-13 11:05 ` Andreas K. Huettel
2 siblings, 1 reply; 19+ messages in thread
From: Andreas K. Huettel @ 2024-08-13 7:27 UTC (permalink / raw)
To: Sam James, binutils; +Cc: H.J. Lu, amodra, nickc, binutils, Jan Beulich
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
Am Montag, 12. August 2024, 15:02:36 CEST schrieb Jan Beulich:
>
> > Upstreams won't be interested in reports right now as it's unclear what
> > the official position for binutils is going forward.
>
> I'm afraid I don't really understand this part. Who's "upstreams" here?
>
All great testing by Sam aside, this is what usually happens:
* Gentoo packages new binutils as one of the first places, users install and use it
* Software starts to fail to build (remember we're a source distro)
* Gentoo package maintainers file bugs to upstream code authors (i.e. the packages
containing the now broken code)
* Upstream maintainers:
- Huh. This always worked. Show me the specs.
- Go away Gentoo weirdo.
- Can't you just build it with an older binutils?
- ENOTFEDORA
- We only support flatpak.
- ...
* Now we need to convince them why the current code is broken, and show
them as simply as possible how to fix it.
Wanna help?
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-13 7:27 ` Andreas K. Huettel
@ 2024-08-13 11:05 ` Andreas K. Huettel
0 siblings, 0 replies; 19+ messages in thread
From: Andreas K. Huettel @ 2024-08-13 11:05 UTC (permalink / raw)
To: Sam James, binutils
Cc: H.J. Lu, amodra, nickc, binutils, Jan Beulich, Andreas K. Huettel
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
>
> * Now we need to convince them why the current code is broken, and show
> them as simply as possible how to fix it.
>
PS. to provide a scale for how big such an effort can get, here's the (of course
technically unrelated) gentoo tracker for clang-16 / gcc-14 modern c porting:
https://bugs.gentoo.org/870412
As of this moment it depends on 700 open bugs (for Gentoo alone), and 1376 closed
ones.
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] gas: Add a macro test with expression argument
2024-08-12 13:45 ` Jan Beulich
@ 2024-08-14 7:37 ` Fangrui Song
0 siblings, 0 replies; 19+ messages in thread
From: Fangrui Song @ 2024-08-14 7:37 UTC (permalink / raw)
To: Jan Beulich; +Cc: Sam James, H.J. Lu, amodra, nickc, binutils
On Mon, Aug 12, 2024 at 6:45 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2024 15:19, Sam James wrote:
> > Jan Beulich <jbeulich@suse.com> writes:
> >> On 12.08.2024 14:45, Sam James wrote:
> >>> Upstreams won't be interested in reports right now as it's unclear what
> >>> the official position for binutils is going forward.
> >>
> >> I'm afraid I don't really understand this part. Who's "upstreams" here?
> >>
> >
> > Sorry, I guess it's confusing here in the context of binutils. I mean
> > maintainers of software that's broken.
> >
> > It's not clear to me if I should actually be reporting anything anywhere
> > yet, given https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116339#c11.
> >
> > (Is the bug valid? If so, let's reopen it and let the libgcc/arm
> > maintainers sort it out. If it's not valid, what are we doing here,
> > because it's inconsistent with my understanding of this discussion?)
> >
> > That's part of the problem. You're saying the changes are intentional,
> > but also that you don't know what quirks we are/aren't going to
> > implement, so we're left in limbo with master as-is? I guess we're
> > waiting for Nick's input. That is the argument in favour of reverting
> > it, because the answer right now isn't even clearly "go fix gcc"?
>
> Well. I would have done the revert already, if it wasn't overwhelmingly
> likely that then the discussion here dies out almost immediately.
>
> Jan
> Sam: Just to say explicitly: I'm grateful you're working on a particularly gnarly bit of gas, breakage does happen, and I'm happy to do whatever testing on our package set if it helps (offer is recurring).
+1 to both!
While the whitespace scrub disruption (to gcc/glibc/Linux kernel) was
unfortunate, I believe there's still room for assembler improvement.
(I did not comment on the scrubber patch series as I have very little
understanding of that code....)
Some major assembly users (Linux kernel, glibc, edk2, ffmpeg, libx265,
etc) likely encompass code patterns found in other packages.
I personally believe if assembler changes make these major users
happy, almost all packages will be happy as well.
(I fixed quite a lot of corner cases in LLVM integrated assembler and
these packages got my attention. Most packages just aren't
nitpicking.)
---
After I posted a lengthy reply at
https://sourceware.org/bugzilla/show_bug.cgi?id=32073#c23
about certain assembly code patterns, I did more analysis of Linux
kernel's x86_64/arm/arm64/powerpc/riscv builds.
If I drop the `isOperator` code of LLVM integrated assembler (don't
parse "a + b" in "M a + b" as a single argument),
I would have breakage with arm/arm64/powerpc/riscv (`defconfig`).
The kernel's intricate macro usage can be explored in the log files:
https://gist.github.com/MaskRay/cf565413205bde2dd2cc634714bad994
---
My current feeling is that we could retain a special case for "SPACE
BIN_OP SPACE" and not treat it as a separate argument.
m 5 + 6 # 5+6 is a whole as + is a binary operator
If this still feels too hacky, we could only enable this when a comma
separator appears anywhere on this line.
Note, linux/arch/arm64/lib/crc32.S has a SPACE BIN_OP SPACE use case
before a comma is seen.
nops (662b-661b) / 4
We could report errors for space-separated arguments in more contexts.
To minimize disruption, we can restrict this change to altmacro mode
initially, issuing warnings in noaltmacro mode.
m 5 6 # currently ok; needs analysis about potential disruption.
m (5) 6 # error
m 5 (6) # currently ok
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-08-14 7:44 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-11 23:11 [PATCH 0/3] Restore macro with expression argument H.J. Lu
2024-08-11 23:11 ` [PATCH 1/3] Revert "gas: drop scrubber states 14 and 15" H.J. Lu
2024-08-11 23:11 ` [PATCH 2/3] Revert "gas: have scrubber retain more whitespace" H.J. Lu
2024-08-11 23:11 ` [PATCH 3/3] gas: Add a macro test with expression argument H.J. Lu
2024-08-12 7:39 ` Jan Beulich
2024-08-12 11:21 ` H.J. Lu
2024-08-12 12:03 ` Jan Beulich
2024-08-12 12:08 ` H.J. Lu
2024-08-12 12:36 ` Jan Beulich
2024-08-12 12:41 ` H.J. Lu
2024-08-12 12:45 ` Sam James
2024-08-12 13:02 ` Jan Beulich
2024-08-12 13:19 ` Sam James
2024-08-12 13:45 ` Jan Beulich
2024-08-14 7:37 ` Fangrui Song
2024-08-12 13:24 ` Sam James
2024-08-13 7:27 ` Andreas K. Huettel
2024-08-13 11:05 ` Andreas K. Huettel
2024-08-11 23:20 ` [PATCH 0/3] Restore macro " Sam James
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).