| Message ID | 55F0C66F.8010205@linaro.org |
|---|---|
| State | New |
| Headers | show |
Michael Collison <michael.collison@linaro.org> writes: > Here is a modified patch that takes your comments into account. Breaking > on depth == 0 with '>' does not work due to the code looking for whitespace. What goes wrong? Just to make sure we're talking about the same thing, I meant that in: (match_operand:FOO> ... the name should be "FOO" and you should get an error on ">" when parsing the text after the name, just like you would for: (match_operand:FOO] ... It's not a big deal though, so... > 2015-08-25 Michael Collison <michael.collison@linaro.org> > > PR other/57195 > * read-md.c (read_name): Allow mode iterators inside angle > brackets in rtl expressions. OK, thanks. Richard
On 09/14/2015 02:34 AM, Richard Sandiford wrote: > Michael Collison <michael.collison@linaro.org> writes: >> Here is a modified patch that takes your comments into account. Breaking >> on depth == 0 with '>' does not work due to the code looking for whitespace. > What goes wrong? Just to make sure we're talking about the same thing, > I meant that in: > > (match_operand:FOO> ... > > the name should be "FOO" and you should get an error on ">" when parsing > the text after the name, just like you would for: > > (match_operand:FOO] ... When I try breaking on '>' with a nesting depth of 0 all examples of <FOO> fail. > > It's not a big deal though, so... > >> 2015-08-25 Michael Collison <michael.collison@linaro.org> >> >> PR other/57195 >> * read-md.c (read_name): Allow mode iterators inside angle >> brackets in rtl expressions. > OK, thanks. Meaning okay to check the patch in? > > Richard >
Michael Collison <michael.collison@linaro.org> writes: > On 09/14/2015 02:34 AM, Richard Sandiford wrote: >> Michael Collison <michael.collison@linaro.org> writes: >>> Here is a modified patch that takes your comments into account. Breaking >>> on depth == 0 with '>' does not work due to the code looking for whitespace. >> What goes wrong? Just to make sure we're talking about the same thing, >> I meant that in: >> >> (match_operand:FOO> ... >> >> the name should be "FOO" and you should get an error on ">" when parsing >> the text after the name, just like you would for: >> >> (match_operand:FOO] ... > > When I try breaking on '>' with a nesting depth of 0 all examples of > <FOO> fail. I meant break when depth == 0 before the decrement that's associated with '>'. I think that problem would only occur if you broke _after_ decrementing the depth. >> It's not a big deal though, so... >> >>> 2015-08-25 Michael Collison <michael.collison@linaro.org> >>> >>> PR other/57195 >>> * read-md.c (read_name): Allow mode iterators inside angle >>> brackets in rtl expressions. >> OK, thanks. > Meaning okay to check the patch in? Yeah, it's OK to check in. Richard
--- a/gcc/read-md.c +++ b/gcc/read-md.c @@ -399,20 +399,31 @@ read_name (struct md_name *name) { int c; size_t i; + int angle_bracket_depth; c = read_skip_spaces (); i = 0; + angle_bracket_depth = 0; while (1) { + if (c == '<') + angle_bracket_depth++; + + if ((c == '>') && (angle_bracket_depth > 0)) + angle_bracket_depth--; + if (c == ' ' || c == '\n' || c == '\t' || c == '\f' || c == '\r' || c == EOF) break; - if (c == ':' || c == ')' || c == ']' || c == '"' || c == '/' - || c == '(' || c == '[') + if (angle_bracket_depth == 0) { - unread_char (c); - break; + if (c == ':' || c == ')' || c == ']' + || c == '"' || c == '/' || c == '(' || c == '[') + { + unread_char (c); + break; + } } if (i == sizeof (name->buffer) - 1)
Richard, Here is a modified patch that takes your comments into account. Breaking on depth == 0 with '>' does not work due to the code looking for whitespace. 2015-08-25 Michael Collison <michael.collison@linaro.org> PR other/57195 * read-md.c (read_name): Allow mode iterators inside angle brackets in rtl expressions. On 09/07/2015 02:46 AM, Richard Sandiford wrote: > Michael Collison <michael.collison@linaro.org> writes: >> This patch allow mode iterators inside angle brackets in machine >> description files. I discovered the issue when attempting to use >> iterators on match_operand's as follows: >> >> match_operand:<VW:V_widen> 0 "s_register_operand" "=w") >> >> The function 'read_name' is nor properly handling ':' inside angle brackets. >> >> Bootstrapped on arm-linux. > Sorry for the slow review. > >> diff --git a/gcc/read-md.c b/gcc/read-md.c >> index 9f158ec..0171fb0 100644 >> --- a/gcc/read-md.c >> +++ b/gcc/read-md.c >> @@ -399,17 +399,25 @@ read_name (struct md_name *name) >> { >> int c; >> size_t i; >> + bool in_angle_bracket; >> >> c = read_skip_spaces (); >> >> i = 0; >> + in_angle_bracket = false; >> while (1) >> { >> + if (c == '<') >> + in_angle_bracket = true; >> + >> + if (c == '>') >> + in_angle_bracket = false; >> + >> if (c == ' ' || c == '\n' || c == '\t' || c == '\f' || c == '\r' >> || c == EOF) >> break; >> - if (c == ':' || c == ')' || c == ']' || c == '"' || c == '/' >> - || c == '(' || c == '[') >> + if (((c == ':') and (!in_angle_bracket)) || c == ')' || c == ']' >> + || c == '"' || c == '/' || c == '(' || c == '[') >> { >> unread_char (c); >> break; > I think we should have a nesting depth rather than a boolean. > It also seems more natural to skip the final "if" statement above when > inside an angle bracket, rather than treating ':' as a special case. > (We'd still break at the end of the line in the case of a missing '>', > so the error reporting shouldn't be too bad.) > > I suppose logically '>' with a nesting depth of 0 should also break > the loop. > > Thanks for fixing this. > > Richard >