Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
> There are four in-tree target architectures that already use %|. I think
> it would be better if you made these new escapes target-specific.
Escaped curly braces cannot be target-specific since
do_assembler_dialects() in final.c ignores any % and considers '{' and
'}' to be alternative delimeters.
> For the logic to find the end of an alternative, you can simply always
> skip over the next char after any percent sign (well, check for end of
> string, of course); there is no need to count percent signs.
That would not be a general approach. %% stands for printing percent
sign, so in assembler string "{%%}" closing curly brace should be
handled as the end of an alternative, though it follows a percent
sign.
--
Maxim Kuznetsov
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
> There are two parts to it: the actual print_operand thing, that I say
> should be target-specific, since some targets already use those characters
> to mean different things; and of course the assembler dialects code needs
> to be fixed to not choke on escaped versions of the characters it is
> looking for. I say you can do this second thing without special-casing
> { | }, making it more generic and cleaner and simpler.
>>> For the logic to find the end of an alternative, you can simply always
>>> skip over the next char after any percent sign (well, check for end of
>>> string, of course); there is no need to count percent signs.
>>
>>
>> That would not be a general approach. %% stands for printing percent
>> sign, so in assembler string "{%%}" closing curly brace should be
>> handled as the end of an alternative, though it follows a percent
>> sign.
>
>
> The second % is skipped over.
Thanks for the explanation, now I understand it. I fixed the patch
according to your remarks. I removed %| support since we don't
actually need it in i386 right now, it was added for the purpose of
possible generalization.
Updated patch is attached.
ChangeLog:
2012-11-14 Maxim Kuznetsov
* final.c (do_assembler_dialects): Don't handle curly braces and
vertical bar escaped by % as dialect delimiters.
* config/i386/i386.c (ix86_print_operand_punct_valid_p): Add '{' and
'}'.
(ix86_print_operand): Handle '{' and '}'.
testsuite/ChangeLog:
2012-11-14 Maxim Kuznetsov
* gcc.target/i386/asm-dialect-2.c: New testcase.
--
Maxim Kuznetsov
curly_braces.patch
Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
Jakub, Richard, thank you for your feedback!
> I wonder if it %{ and %} shouldn't be better handled in final.c
> for all #ifdef ASSEMBLER_DIALECT targets, rather than just for one specific.
I moved %{ and %} cases to output_asm_insn in final.c
> Also:
> *(p + 1)
> should be better written as p[1] (more readable).
Fixed.
I also documented new escapes.
Could you please have a look?
--
Maxim Kuznetsov
curly_braces_20130429.patch
Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
2013/4/29 Jakub Jelinek :
> Also, why are you handling just %{ and %}, and
> not also %| ? I mean, if you want to print say {|} into assembly for both
> dialects, don't you need:
> asm ("{dialect1%{%|%}|%{%|%}dialect2}");
> or similar? If you use just | instead of %|, it would be handled as
> separator of the dialects.
Sure. %| was removed due to concerns over some target architectures
already use it, but now %| is under ASSEMBLER_DIALECT and doesn't seem
to affect them.
ChangeLog:
2013-04-29 Maxim Kuznetsov
* final.c (do_assembler_dialects): Don't handle curly braces and
vertical bar escaped by % as dialect delimiters.
(output_asm_insn): Print curly braces and vertical bar if escaped
by % and ASSEMBLER_DIALECT defined.
* doc/tm.texi (ASSEMBLER_DIALECT): Document new standard escapes.
testsuite/ChangeLog:
2013-04-29 Maxim Kuznetsov
* gcc.target/i386/asm-dialect-2.c: New testcase.
--
Maxim Kuznetsov
curly_braces_20130429-2.patch
Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
> I checked it in for him with a small change in document. It > should be `|' instead of '@|'. Thanks a lot! -- Maxim Kuznetsov
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
> Thanks for the explanation, now I understand it. I fixed the patch > according to your remarks. I removed %| support since we don't > actually need it in i386 right now, it was added for the purpose of > possible generalization. > > Updated patch is attached. Ping -- Maxim Kuznetsov curly_braces.patch Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
Thank you for your feedback!
> For '}' case, can you simply just add
>
> /* Skip over any character after a percent sign. */
> if (*p == '%' && *(p + 1))
> {
> p += 2;
> continue;
> }
>
> without changing the do-while loop to the while loop?
Loop condition (*p++ != '}') must be moved to loop body for it to not
execute after "continue" (we just want to skip over % with following
character without any other increments or checks). Although, loop form
doesn't matter so I changed it back to do-while.
> That's not the same thing though. Maksim's code is correct,
> although it could certainly be written more clearly.
> Maybe something like
>
> if (*p == '%')
> p++;
> if (*p)
> p++;
Fixed.
I also noticed that previous patch broke intel (or any alternative)
syntax. This was because the original loop:
while (*p && *p != '}' && *p++ != '|');
incremented p after '|' is found, but loop in my patch didn't:
while (*p && *p != '}' && *p != '|')
p += (*p == '%' && *(p + 1)) ? 2 : 1;
I fixed it too.
Updated patch is attached. Could you please have a look?
ChangeLog:
2013-04-03 Maxim Kuznetsov
* final.c (do_assembler_dialects): Don't handle curly braces
escaped by % as dialect delimiters.
* config/i386/i386.c (ix86_print_operand_punct_valid_p): Add '{' and
'}'.
(ix86_print_operand): Handle '{' and '}'.
testsuite/ChangeLog:
2013-04-03 Maxim Kuznetsov
* gcc.target/i386/asm-dialect-2.c: New testcase.
--
Maxim Kuznetsov
curly_braces_20130403.patch
Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
Ping
2013/4/3 Maksim Kuznetsov :
> Thank you for your feedback!
>
>> For '}' case, can you simply just add
>>
>> /* Skip over any character after a percent sign. */
>> if (*p == '%' && *(p + 1))
>> {
>> p += 2;
>> continue;
>> }
>>
>> without changing the do-while loop to the while loop?
>
> Loop condition (*p++ != '}') must be moved to loop body for it to not
> execute after "continue" (we just want to skip over % with following
> character without any other increments or checks). Although, loop form
> doesn't matter so I changed it back to do-while.
>
>> That's not the same thing though. Maksim's code is correct,
>> although it could certainly be written more clearly.
>> Maybe something like
>>
>> if (*p == '%')
>> p++;
>> if (*p)
>> p++;
>
> Fixed.
>
>
> I also noticed that previous patch broke intel (or any alternative)
> syntax. This was because the original loop:
>
> while (*p && *p != '}' && *p++ != '|');
>
> incremented p after '|' is found, but loop in my patch didn't:
>
> while (*p && *p != '}' && *p != '|')
> p += (*p == '%' && *(p + 1)) ? 2 : 1;
>
> I fixed it too.
>
> Updated patch is attached. Could you please have a look?
>
> ChangeLog:
>
> 2013-04-03 Maxim Kuznetsov
>
> * final.c (do_assembler_dialects): Don't handle curly braces
> escaped by % as dialect delimiters.
> * config/i386/i386.c (ix86_print_operand_punct_valid_p): Add '{' and
> '}'.
> (ix86_print_operand): Handle '{' and '}'.
>
> testsuite/ChangeLog:
>
> 2013-04-03 Maxim Kuznetsov
>
> * gcc.target/i386/asm-dialect-2.c: New testcase.
>
> --
> Maxim Kuznetsov
--
Maxim Kuznetsov
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
>> For '}' case, can you simply just add
>>
>> /* Skip over any character after a percent sign. */
>> if (*p == '%' && *(p + 1))
>> {
>> p += 2;
>> continue;
>> }
>>
>> without changing the do-while loop to the while loop?
>
> Loop condition (*p++ != '}') must be moved to loop body for it to not
> execute after "continue" (we just want to skip over % with following
> character without any other increments or checks). Although, loop form
> doesn't matter so I changed it back to do-while.
>
>> That's not the same thing though. Maksim's code is correct,
>> although it could certainly be written more clearly.
>> Maybe something like
>>
>> if (*p == '%')
>> p++;
>> if (*p)
>> p++;
>
> Fixed.
>
>
> I also noticed that previous patch broke intel (or any alternative)
> syntax. This was because the original loop:
>
> while (*p && *p != '}' && *p++ != '|');
>
> incremented p after '|' is found, but loop in my patch didn't:
>
> while (*p && *p != '}' && *p != '|')
> p += (*p == '%' && *(p + 1)) ? 2 : 1;
>
> I fixed it too.
Ping.
Richard, Jeff, could you please have a look?
--
Maxim Kuznetsov
curly_braces_20130403.patch
Description: Binary data
Re: [PATCH, generic] Support printing of escaped curly braces and vertical bar in assembler output
Ping. Could someone please have a look? -- Maxim Kuznetsov > Thanks for the explanation, now I understand it. I fixed the patch > according to your remarks. I removed %| support since we don't > actually need it in i386 right now, it was added for the purpose of > possible generalization. > > Updated patch is attached. > > Maxim Kuznetsov
