Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
>>> On 05.10.11 at 10:54, Jan Hubicka wrote: >>> we have, like specifying the set of symbols _defined_ by a toplevel >>> asm, right? I might misremember but sth like >>> >>> extern void foo (void); >>> asm("" "foo"); >>> >>> was supposed to do the trick. Or should we treat those as outputs >>> (given you use inputs for symbol uses)? >> >> I don't recall any discussion of how to deal with symbols defined by a >> top level asm - I was just asked to follow the "normal" asm syntax in >> having two colons in the middle instead of one as I had originally (not >> expecting any use for outputs here). >> >>> Honza, do you remember if we decided on anything here? > > To be honest, I don't think we actually dicussed some particular > syntax or my memory is even worse ;). We will need to make one. > In the above example, I don't think quotes about foo is needed. We > actualy provide definition of the declaration, so it should be just > the decl itself, not a string. With there not being a clear direction (and me not having the time to extend what I have currently), is there any chance that what's there could go in then, for someone else to extend? Thanks, Jan > Honza >>> >>> Thanks, >>> Richard. >> >> >>
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
we have, like specifying the set of symbols _defined_ by a toplevel asm, right? I might misremember but sth like extern void foo (void); asm("" "foo"); was supposed to do the trick. Or should we treat those as outputs (given you use inputs for symbol uses)? I don't recall any discussion of how to deal with symbols defined by a top level asm - I was just asked to follow the "normal" asm syntax in having two colons in the middle instead of one as I had originally (not expecting any use for outputs here). Honza, do you remember if we decided on anything here? To be honest, I don't think we actually dicussed some particular syntax or my memory is even worse ;). We will need to make one. In the above example, I don't think quotes about foo is needed. We actualy provide definition of the declaration, so it should be just the decl itself, not a string. Honza Thanks, Richard.
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
On Fri, Sep 30, 2011 at 01:54:16PM +0100, Jan Beulich wrote: > >>> On 30.09.11 at 14:47, Jakub Jelinek wrote: > > On Fri, Sep 30, 2011 at 12:43:54PM +0100, Jan Beulich wrote: > >> This is so that use of symbols referenced in these asm()-s can be > >> properly tracked by the compiler, just like is the case for all other > >> asm()-s. I'm particularly looking forward to use this in the Linux > >> kernel. It is certainly not very useful in PIC code, at least not with > >> some extra care. > > > > Even in PIC code it can be useful to have toplevel asm like > > asm ("..." : : "i" (offsetof (struct S, field)), "i" (some_enum_value), "i" > > (sizeof (struct S))); > > etc. > > But wasn't it you who pointed out that this has limited use in PIC > mode when I first submitted this? Sure, some input operands may be problematic in PIC, but the above ones are just compile time integer constants and those are fine always. Jakub
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
>>> On 30.09.11 at 14:42, Richard Guenther wrote: > On Fri, Sep 30, 2011 at 1:43 PM, Jan Beulich wrote: >> This is so that use of symbols referenced in these asm()-s can be >> properly tracked by the compiler, just like is the case for all other >> asm()-s. I'm particularly looking forward to use this in the Linux >> kernel. It is certainly not very useful in PIC code, at least not with >> some extra care. > > I miss documentation for this. Just like on the first submission - if top level asm-s as they are without this change were documented somewhere, I would be happy to extend that documentation. But honestly, I don't think it's appropriate to ask me to start writing documentation for this from ground up - that should be done by someone more familiar with the base feature, so that eventual caveats can be pointed out, and it can be put at a proper (rather than guessed) location in the documentation. > This does not address the other issue > we have, like specifying the set of symbols _defined_ by a toplevel > asm, right? I might misremember but sth like > > extern void foo (void); > asm("" "foo"); > > was supposed to do the trick. Or should we treat those as outputs > (given you use inputs for symbol uses)? I don't recall any discussion of how to deal with symbols defined by a top level asm - I was just asked to follow the "normal" asm syntax in having two colons in the middle instead of one as I had originally (not expecting any use for outputs here). Jan > Honza, do you remember if we decided on anything here? > > Thanks, > Richard.
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
>>> On 30.09.11 at 14:47, Jakub Jelinek wrote: > On Fri, Sep 30, 2011 at 12:43:54PM +0100, Jan Beulich wrote: >> This is so that use of symbols referenced in these asm()-s can be >> properly tracked by the compiler, just like is the case for all other >> asm()-s. I'm particularly looking forward to use this in the Linux >> kernel. It is certainly not very useful in PIC code, at least not with >> some extra care. > > Even in PIC code it can be useful to have toplevel asm like > asm ("..." : : "i" (offsetof (struct S, field)), "i" (some_enum_value), "i" > (sizeof (struct S))); > etc. But wasn't it you who pointed out that this has limited use in PIC mode when I first submitted this? Jan
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
On Fri, Sep 30, 2011 at 12:43:54PM +0100, Jan Beulich wrote: > This is so that use of symbols referenced in these asm()-s can be > properly tracked by the compiler, just like is the case for all other > asm()-s. I'm particularly looking forward to use this in the Linux > kernel. It is certainly not very useful in PIC code, at least not with > some extra care. Even in PIC code it can be useful to have toplevel asm like asm ("..." : : "i" (offsetof (struct S, field)), "i" (some_enum_value), "i" (sizeof (struct S))); etc. Jakub
Re: [PATCH 2/2] allow certain kinds of inputs to top level asm()-s
On Fri, Sep 30, 2011 at 1:43 PM, Jan Beulich wrote: > This is so that use of symbols referenced in these asm()-s can be > properly tracked by the compiler, just like is the case for all other > asm()-s. I'm particularly looking forward to use this in the Linux > kernel. It is certainly not very useful in PIC code, at least not with > some extra care. I miss documentation for this. This does not address the other issue we have, like specifying the set of symbols _defined_ by a toplevel asm, right? I might misremember but sth like extern void foo (void); asm("" "foo"); was supposed to do the trick. Or should we treat those as outputs (given you use inputs for symbol uses)? Honza, do you remember if we decided on anything here? Thanks, Richard. > gcc/ > 2011-09-30 Jan Beulich > > * c-parser.c (c_parser_simple_asm_expr): Add new second parameter > 'inputsp'. Process inputs if caller indicates they are allowed. Adjust > calls to c_parser_asm_operands(). > (c_parser_asm_operands): Change type of second parameter from 'bool' > to 'int'. Call c_parser_expression() only for non-negative 'mode', and > c_parser_expr_no_commas() otherwise. > (c_parser_declaration_or_fndef): Pass NULL as new second argument to > c_parser_simple_asm_expr(). > (c_parser_asm_definition): New local variables 'loc' and 'inputs'. > Adjust calls to c_parser_simple_asm_expr() and cgraph_add_asm_node(). > (c_parser_simple_asm_expr): > * cgraph.c (cgraph_add_asm_node): Call check_unique_operand_names() to > validate input operands. Store inputs and location. > * cgraph.h (struct cgraph_asm_node): Add 'inputs' and 'loc'. > (cgraph_add_asm_node): New second and third parameters. > * cgraphunit.c (cgraph_output_pending_asms): Pass new second and third > arguments to assemble_asm(). > (process_function_and_variable_attributes): New local variable 'anode'. > Process list starting from 'cgraph_asm_nodes'. > (cgraph_output_in_order): Pass new second and third arguments to > assemble_asm(). > * cp/parser.c (enum required_token): Add RT_COLON_NO_OUTPUT. > (cp_parser_asm_definition): New local variable 'loc'. Correct a > comment. Parse and process input operands if permitted. > (cp_parser_required_error): Handle new case RT_COLON_NO_OUTPUT. > * lto-streamer-in.c (lto_input_toplevel_asms): Pass new second and > third arguments to cgraph_add_asm_node(). > * lto-streamer-out.c (lto_output_toplevel_asms): Also output inputs > and location. > * output.h (assemble_asm): New second and third parameters. > * stmt.c (check_unique_operand_names): Remove static declaration and > make global. > * tree.h (check_unique_operand_names): Declare. > * varasm.c: Include pretty-print.h. > (assemble_asm): New parameters 'inputs' and 'loc'. Process inputs if > provided. > > gcc/testsuite/ > 2011-09-30 Jan Beulich > > * g++.dg/ext/asm-static-1.C: New. > * gcc.dg/asm-static-1.c: New. > * gcc.dg/asm-static-2.c: New. > * gcc.dg/asm-static-3.c: New. > * gcc.dg/asm-static-4.c: New. > > --- 2011-09-29.orig/gcc/c-parser.c 2011-09-28 10:56:01.0 +0200 > +++ 2011-09-29/gcc/c-parser.c 2011-09-29 15:07:29.0 +0200 > @@ -1131,7 +1131,7 @@ static struct c_arg_info *c_parser_parms > static struct c_arg_info *c_parser_parms_list_declarator (c_parser *, tree, > tree); > static struct c_parm *c_parser_parameter_declaration (c_parser *, tree); > -static tree c_parser_simple_asm_expr (c_parser *); > +static tree c_parser_simple_asm_expr (c_parser *, tree *); > static tree c_parser_attributes (c_parser *); > static struct c_type_name *c_parser_type_name (c_parser *); > static struct c_expr c_parser_initializer (c_parser *); > @@ -1150,7 +1150,7 @@ static void c_parser_while_statement (c_ > static void c_parser_do_statement (c_parser *); > static void c_parser_for_statement (c_parser *); > static tree c_parser_asm_statement (c_parser *); > -static tree c_parser_asm_operands (c_parser *, bool); > +static tree c_parser_asm_operands (c_parser *, int); > static tree c_parser_asm_goto_operands (c_parser *); > static tree c_parser_asm_clobbers (c_parser *); > static struct c_expr c_parser_expr_no_commas (c_parser *, struct c_expr *); > @@ -1623,7 +1623,7 @@ c_parser_declaration_or_fndef (c_parser > function definition. */ > fndef_ok = false; > if (c_parser_next_token_is_keyword (parser, RID_ASM)) > - asm_name = c_parser_simple_asm_expr (parser); > + asm_name = c_parser_simple_asm_expr (parser, NULL); > if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE)) > postfix_attrs = c_parser_attributes (parser); > if (c_parser_next_token_is
[PATCH 2/2] allow certain kinds of inputs to top level asm()-s
This is so that use of symbols referenced in these asm()-s can be properly tracked by the compiler, just like is the case for all other asm()-s. I'm particularly looking forward to use this in the Linux kernel. It is certainly not very useful in PIC code, at least not with some extra care. gcc/ 2011-09-30 Jan Beulich * c-parser.c (c_parser_simple_asm_expr): Add new second parameter 'inputsp'. Process inputs if caller indicates they are allowed. Adjust calls to c_parser_asm_operands(). (c_parser_asm_operands): Change type of second parameter from 'bool' to 'int'. Call c_parser_expression() only for non-negative 'mode', and c_parser_expr_no_commas() otherwise. (c_parser_declaration_or_fndef): Pass NULL as new second argument to c_parser_simple_asm_expr(). (c_parser_asm_definition): New local variables 'loc' and 'inputs'. Adjust calls to c_parser_simple_asm_expr() and cgraph_add_asm_node(). (c_parser_simple_asm_expr): * cgraph.c (cgraph_add_asm_node): Call check_unique_operand_names() to validate input operands. Store inputs and location. * cgraph.h (struct cgraph_asm_node): Add 'inputs' and 'loc'. (cgraph_add_asm_node): New second and third parameters. * cgraphunit.c (cgraph_output_pending_asms): Pass new second and third arguments to assemble_asm(). (process_function_and_variable_attributes): New local variable 'anode'. Process list starting from 'cgraph_asm_nodes'. (cgraph_output_in_order): Pass new second and third arguments to assemble_asm(). * cp/parser.c (enum required_token): Add RT_COLON_NO_OUTPUT. (cp_parser_asm_definition): New local variable 'loc'. Correct a comment. Parse and process input operands if permitted. (cp_parser_required_error): Handle new case RT_COLON_NO_OUTPUT. * lto-streamer-in.c (lto_input_toplevel_asms): Pass new second and third arguments to cgraph_add_asm_node(). * lto-streamer-out.c (lto_output_toplevel_asms): Also output inputs and location. * output.h (assemble_asm): New second and third parameters. * stmt.c (check_unique_operand_names): Remove static declaration and make global. * tree.h (check_unique_operand_names): Declare. * varasm.c: Include pretty-print.h. (assemble_asm): New parameters 'inputs' and 'loc'. Process inputs if provided. gcc/testsuite/ 2011-09-30 Jan Beulich * g++.dg/ext/asm-static-1.C: New. * gcc.dg/asm-static-1.c: New. * gcc.dg/asm-static-2.c: New. * gcc.dg/asm-static-3.c: New. * gcc.dg/asm-static-4.c: New. --- 2011-09-29.orig/gcc/c-parser.c 2011-09-28 10:56:01.0 +0200 +++ 2011-09-29/gcc/c-parser.c 2011-09-29 15:07:29.0 +0200 @@ -1131,7 +1131,7 @@ static struct c_arg_info *c_parser_parms static struct c_arg_info *c_parser_parms_list_declarator (c_parser *, tree, tree); static struct c_parm *c_parser_parameter_declaration (c_parser *, tree); -static tree c_parser_simple_asm_expr (c_parser *); +static tree c_parser_simple_asm_expr (c_parser *, tree *); static tree c_parser_attributes (c_parser *); static struct c_type_name *c_parser_type_name (c_parser *); static struct c_expr c_parser_initializer (c_parser *); @@ -1150,7 +1150,7 @@ static void c_parser_while_statement (c_ static void c_parser_do_statement (c_parser *); static void c_parser_for_statement (c_parser *); static tree c_parser_asm_statement (c_parser *); -static tree c_parser_asm_operands (c_parser *, bool); +static tree c_parser_asm_operands (c_parser *, int); static tree c_parser_asm_goto_operands (c_parser *); static tree c_parser_asm_clobbers (c_parser *); static struct c_expr c_parser_expr_no_commas (c_parser *, struct c_expr *); @@ -1623,7 +1623,7 @@ c_parser_declaration_or_fndef (c_parser function definition. */ fndef_ok = false; if (c_parser_next_token_is_keyword (parser, RID_ASM)) - asm_name = c_parser_simple_asm_expr (parser); + asm_name = c_parser_simple_asm_expr (parser, NULL); if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE)) postfix_attrs = c_parser_attributes (parser); if (c_parser_next_token_is (parser, CPP_EQ)) @@ -1782,9 +1782,12 @@ c_parser_declaration_or_fndef (c_parser static void c_parser_asm_definition (c_parser *parser) { - tree asm_str = c_parser_simple_asm_expr (parser); + location_t loc = c_parser_peek_token (parser)->location; + tree inputs = NULL_TREE; + tree asm_str = c_parser_simple_asm_expr (parser, &inputs); + if (asm_str) -cgraph_add_asm_node (asm_str); +cgraph_add_asm_node (asm_str, inputs, loc); c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>"); } @@ -3330,10 +,11 @@ c_parser_asm_string_literal