On Wed, 18 Jul 2012, Siddhesh Poyarekar wrote:
> Hi,
>
> Resending. I did not get any responses the last two times and I too
> forgot about it. Can someone please review this?
This is *not* an approver-review.
> An assembler directive with an operand is filtered through
> output_asm_insn (or asm_fprintf for gcc internal asm() directives) to
> expand the operand values in the assembler as well as to choose
> dialects if present. This patch is concerned primarily with the
> dialects, since their syntax prevent inclusion of assembler strings
> with curly braces, causing them to be interpreted as dialects.
>
> The attached patch allows printing of curly braces in assembler by
> preceding them with a \\.
(the single character \ to wit. The \\ is the sequence you have
to write in C.)
Sounds like a good idea, but I think you shouldn't limit that to
"{}" but rather introduce \ to escape and cause any next
character to be emitted as-is, in particular "|".
> So to print the following code into assembler:
>
> .pushsection ".foo"; .asciz "div { width : 50%% | height=10px }"; .long
> 42; .popsection
>
> The following code needs to be used with this patch:
>
> void f()
> {
> asm ( ".pushsection \".foo\"; .asciz \"div \\{ width : 50%% |
> height=10px \\} \"; .long %c0; .popsection" : : "i"(42) ); }
>
> The other option to \\ (since it doesn't look as clean) was to use %
> as an escape character, but I was not sure if that is a better looking
> option or a worse looking one. I don't mind resubmitting the patch to
> use %{ and %} to print curly braces if that is acceptable.
I'm on the fence regarding this, but leaning towards maybe
that's a better way forward, as it can't collide with any
current use of \\ (seeing as "%{" "%}" and "%|" currently fall
down into a gcc_unreachable). If you change your patch to this,
make sure you include "%|" to emit "|".
(Hm, why is there a gcc_unreachable? It should be an error as
"user" asm statements can reach it!)
> It is still possible to print curly braces in asm string literals
> without operands since they do not undergo any transformation.
>
> The patch does not introduce any regressions. I have tested this with
> x86_64 and i686 and it works well with both of them.
That's a target (yes, counting it as one target) that defines
ASSEMBLER_DIALECT wherein "{}|" has the predefined meaning you
try to escape. I suggest testing a non-ASSEMBLER_DIALECT target
as well. And check that no target uses \\ for something else
through ASM_FPRINTF_EXTENSIONS... ...so it seems like %{ etc.
would be safer.
> gcc/ChangeLog:
>
> 2012-03-27 Siddhesh Poyarekar <siddh...@redhat.com>
>
> * final.c (output_asm_insn, asm_fprintf): Print curly braces if
> preceded by an escaped backslash (\\).
>
> testsuite/ChangeLog:
>
> 2012-03-27 Siddhesh Poyarekar <siddh...@redhat.com>
>
> * gcc.dg/asm-braces.c: New test case.
>
Thanks and keep pinging.
brgds, H-P
