Patchwork [objc] Do not call assemble_external

login
register
mail settings
Submitter Steven Bosscher
Date March 21, 2012, 9:09 p.m.
Message ID <CABu31nOW0dMs8txBLgq=ZVTO9e4tjF1Z5=8HfkweG1eVm5PbPA@mail.gmail.com>
Download mbox | patch
Permalink /patch/148067/
State New
Headers show

Comments

Steven Bosscher - March 21, 2012, 9:09 p.m.
Hello,

There is no reason for the ObjC front end to call assemble_external on
these symbols, the middle-end handles this just fine via
add_builtin_function.

Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK for trunk?

Ciao!
Steven

objc/
	* objc-act (objc_build_ivar_assignment): Do not call assemble_external.
	(objc_build_global_assignment): Likewise.
	(objc_build_strong_cast_assignment): Likewise.
	* objc-next-runtime-abi-01.c: Cleanup commented-out assemble_external.
	* objc-next-runtime-abi-02.c: Likewise.
	* objc-gnu-runtime-abi-01.c: Likewise.
IainS - March 21, 2012, 9:23 p.m.
Hi Steven,

On 21 Mar 2012, at 21:09, Steven Bosscher wrote:
> There is no reason for the ObjC front end to call assemble_external on
> these symbols, the middle-end handles this just fine via
> add_builtin_function.

Ah, that's the bit I'd yet to figure out ...

> Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK for trunk?
>
> Ciao!
> Steven
>
> objc/
> 	* objc-act (objc_build_ivar_assignment): Do not call  
> assemble_external.
> 	(objc_build_global_assignment): Likewise.
> 	(objc_build_strong_cast_assignment): Likewise.
> 	* objc-next-runtime-abi-01.c: Cleanup commented-out  
> assemble_external.
> 	* objc-next-runtime-abi-02.c: Likewise.
> 	* objc-gnu-runtime-abi-01.c: Likewise.
> <cleanup_objc_assemble_external.diff>

... this would allow us to close PR17982?

... and make progress on PR24777? (I'm not sure where exactly we need  
to go with this one - we have different sets of calls depending on the  
runtime)

Iain
Steven Bosscher - March 21, 2012, 9:32 p.m.
On Wed, Mar 21, 2012 at 10:23 PM, Iain Sandoe
<developer@sandoe-acoustics.co.uk> wrote:
>> objc/
>>        * objc-act (objc_build_ivar_assignment): Do not call
>> assemble_external.
>>        (objc_build_global_assignment): Likewise.
>>        (objc_build_strong_cast_assignment): Likewise.
>>        * objc-next-runtime-abi-01.c: Cleanup commented-out
>> assemble_external.
>>        * objc-next-runtime-abi-02.c: Likewise.
>>        * objc-gnu-runtime-abi-01.c: Likewise.
>> <cleanup_objc_assemble_external.diff>
>
>
> ... this would allow us to close PR17982?

I believe so, but I have to look into it a but deeper to understand
what the remaining assemble_external calls are for. The ones in
config/* are OK, they are all for writing out multiple-inheritance
thunks. The ones that need checking are:

calls.c:1649:     assemble_external (fndecl);
expr.c:1422:      assemble_external (block_move_fn);
expr.c:2794:      assemble_external (block_clear_fn);
expr.c:7423:          assemble_external (exp);
expr.c:9022:      assemble_external (exp);
final.c:2745:             assemble_external (t);
final.c:3497:   assemble_external (t);
final.c:3568:   assemble_external (SYMBOL_REF_DECL (x));
toplev.c:489:      assemble_external (decl);


> ... and make progress on PR24777? (I'm not sure where exactly we need to go
> with this one - we have different sets of calls depending on the runtime)

The FIXMEs in that PR do not exist anymore. Perhaps this removed them:

2011-10-11  Michael Meissner  <meissner at linux dot vnet dot ibm dot com>

        * objc-next-runtime-abi-01.c (objc_build_exc_ptr): Delete old
        interface with two parallel arrays to hold standard builtin
        declarations, and replace it with a function based interface that
        can support creating builtins on the fly in the future.  Change
        all uses, and poison the old names.  Make sure 0 is not a
        legitimate builtin index.
        * objc-next-runtime-abi-02.c (objc_build_exc_ptr): Ditto.
        * objc-gnu-runtime-abi-01.c (objc_build_exc_ptr): Ditto.

In any case, if there's nothing left to fix for PR24777, I suppose it
can be closed as FIXED.

Ciao!
Steven
Mike Stump - March 21, 2012, 9:55 p.m.
On Mar 21, 2012, at 2:09 PM, Steven Bosscher wrote:
> There is no reason for the ObjC front end to call assemble_external on
> these symbols,

> OK for trunk?

Ok.  Watch for hate mail from Jack, if you guess wrong.  :-)
Mike Stump - March 21, 2012, 10:11 p.m.
On Mar 21, 2012, at 2:32 PM, Steven Bosscher wrote:
> In any case, if there's nothing left to fix for PR24777, I suppose it
> can be closed as FIXED.

I see all sorts of FIXME: in c-decl.c still...  Anyway, someone needs to sort out what is done and remains undone and update the FIXMEs...  I don't know which ones are dead.
Steven Bosscher - March 21, 2012, 10:45 p.m.
On Wed, Mar 21, 2012 at 11:11 PM, Mike Stump <mikestump@comcast.net> wrote:
> On Mar 21, 2012, at 2:32 PM, Steven Bosscher wrote:
>> In any case, if there's nothing left to fix for PR24777, I suppose it
>> can be closed as FIXED.
>
> I see all sorts of FIXME: in c-decl.c still...  Anyway, someone needs to sort out what is done and remains undone and update the FIXMEs...  I don't know which ones are dead.


Ehm, yes. I was looking in the wrong place (objc/*). The "weird
not-really-builtin functions" are the ones added via
add_builtin_function but with builtin type NOT_BUILT_IN. Those
problems still appear to be there:

objc/objc-act.c:    = add_builtin_function (TAG_EXCEPTIONTHROW,
temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-act.c:    = add_builtin_function (TAG_SYNCENTER, temp_type,
0, NOT_BUILT_IN,
objc/objc-act.c:    = add_builtin_function (TAG_SYNCEXIT, temp_type,
0, NOT_BUILT_IN,
objc/objc-act.c:    = add_builtin_function (TAG_ENUMERATION_MUTATION,
type, 0, NOT_BUILT_IN,
objc/objc-gnu-runtime-abi-01.c:    = add_builtin_function
(TAG_GETCLASS, type, 0, NOT_BUILT_IN,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_GETCLASS, type, 0, NOT_BUILT_IN,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_GETMETACLASS, type, 0, NOT_BUILT_IN, NULL, NULL_TREE);
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_SETJMP, temp_type, 0, NOT_BUILT_IN, NULL, NULL_TREE);
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_EXCEPTIONEXTRACT, temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_EXCEPTIONTRYENTER, temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_EXCEPTIONTRYEXIT, temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_EXCEPTIONMATCH, temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-next-runtime-abi-01.c:    = add_builtin_function
(TAG_ASSIGNIVAR, temp_type, 0, NOT_BUILT_IN,
objc/objc-next-runtime-abi-01.c:        = add_builtin_function
(TAG_ASSIGNGLOBAL, temp_type, 0, NOT_BUILT_IN, NULL,
objc/objc-next-runtime-abi-01.c:        = add_builtin_function
(TAG_ASSIGNSTRONGCAST, temp_type, 0, NOT_BUILT_IN, NULL,
(+ some not found by grep because the add_builtin_function call spans
multiple lines)

FWIW, Java does this too:
java/decl.c:    = add_builtin_function ("_Jv_MonitorEnter", t, 0, NOT_BUILT_IN,
java/decl.c:    = add_builtin_function ("_Jv_MonitorExit", t, 0, NOT_BUILT_IN,

Ciao!
Steven
IainS - March 21, 2012, 11:53 p.m.
On 21 Mar 2012, at 22:45, Steven Bosscher wrote:

> On Wed, Mar 21, 2012 at 11:11 PM, Mike Stump <mikestump@comcast.net>  
> wrote:
>> On Mar 21, 2012, at 2:32 PM, Steven Bosscher wrote:
>>> In any case, if there's nothing left to fix for PR24777, I suppose  
>>> it
>>> can be closed as FIXED.
>>
>> I see all sorts of FIXME: in c-decl.c still...  Anyway, someone  
>> needs to sort out what is done and remains undone and update the  
>> FIXMEs...  I don't know which ones are dead.
>
>
> Ehm, yes. I was looking in the wrong place (objc/*). The "weird
> not-really-builtin functions" are the ones added via
> add_builtin_function but with builtin type NOT_BUILT_IN. Those
> problems still appear to be there:
>
> objc/objc-act.c:    = add_builtin_function (TAG_EXCEPTIONTHROW,
> temp_type, 0, NOT_BUILT_IN, NULL,
> objc/objc-act.c:    = add_builtin_function (TAG_SYNCENTER, temp_type,
> 0, NOT_BUILT_IN,
> objc/objc-act.c:    = add_builtin_function (TAG_SYNCEXIT, temp_type,
> 0, NOT_BUILT_IN,
> objc/objc-act.c:    = add_builtin_function (TAG_ENUMERATION_MUTATION,
> type, 0, NOT_BUILT_IN,
> objc/objc-gnu-runtime-abi-01.c:    = add_builtin_function
> (TAG_GETCLASS, type, 0, NOT_BUILT_IN,
> objc/objc-next-runtime-abi-01.c:    = add_builtin_function
> (TAG_GETCLASS, type, 0, NOT_BUILT_IN,

<snip>

> (TAG_ASSIGNGLOBAL, temp_type, 0, NOT_BUILT_IN, NULL,
> objc/objc-next-runtime-abi-01.c:        = add_builtin_function
> (TAG_ASSIGNSTRONGCAST, temp_type, 0, NOT_BUILT_IN, NULL,
> (+ some not found by grep because the add_builtin_function call spans
> multiple lines)

conceptually, the issue is that there are multiple sets of built-ins  
(potentially, one set for each runtime, and the sets are of different  
sizes).  Thus, it's not just a case of turning these into regular  
built-ins, without some mechanism to cater for overloading or presence/ 
absence of particular ones.
Steven Bosscher - March 22, 2012, midnight
On Thu, Mar 22, 2012 at 12:53 AM, Iain Sandoe
<developer@sandoe-acoustics.co.uk> wrote:

> conceptually, the issue is that there are multiple sets of built-ins
> (potentially, one set for each runtime, and the sets are of different
> sizes).  Thus, it's not just a case of turning these into regular built-ins,
> without some mechanism to cater for overloading or presence/absence of
> particular ones.

I don't understand this. We're committed to one runtime per
compilation, right? If so, then we should create only one of the sets
at any time.

Ciao!
Steven
IainS - March 22, 2012, 12:16 a.m.
On 22 Mar 2012, at 00:00, Steven Bosscher wrote:

> On Thu, Mar 22, 2012 at 12:53 AM, Iain Sandoe
> <developer@sandoe-acoustics.co.uk> wrote:
>
>> conceptually, the issue is that there are multiple sets of built-ins
>> (potentially, one set for each runtime, and the sets are of different
>> sizes).  Thus, it's not just a case of turning these into regular  
>> built-ins,
>> without some mechanism to cater for overloading or presence/absence  
>> of
>> particular ones.
>
> I don't understand this. We're committed to one runtime per
> compilation, right? If so, then we should create only one of the sets
> at any time.

Yes, that's true [notwithstanding an erroneous invocation of LTO with  
mixed inputs, which we should detect by alternate means]

but ...
.. don't the indices for built-ins need to be constant?
   (maybe that means that we'd just allocate as many as needed to  
cover all runtimes?)
.. or, otherwise, how does LTO know which set to invoke?

... sorry, I think my observation about pr24777 has pushed this thread  
off course - perhaps we'd be better putting this in the PR thread?

cheers
Iain

Patch

objc/
	* objc-act (objc_build_ivar_assignment): Do not call assemble_external.
	(objc_build_global_assignment): Likewise.
	(objc_build_strong_cast_assignment): Likewise.
	* objc-next-runtime-abi-01.c: Cleanup commented-out assemble_external.
	* objc-next-runtime-abi-02.c: Likewise.
	* objc-gnu-runtime-abi-01.c: Likewise.

Index: objc-act.c
===================================================================
--- objc-act.c	(revision 185603)
+++ objc-act.c	(working copy)
@@ -3553,7 +3553,6 @@  objc_build_ivar_assignment (tree outervar, tree lh
 		tree_cons (NULL_TREE, offs,
 		    NULL_TREE)));
 
-  assemble_external (func);
   return build_function_call (input_location, func, func_params);
 }
 
@@ -3566,7 +3565,6 @@  objc_build_global_assignment (tree lhs, tree rhs)
 		      build_unary_op (input_location, ADDR_EXPR, lhs, 0)),
 		    NULL_TREE));
 
-  assemble_external (objc_assign_global_decl);
   return build_function_call (input_location,
 			      objc_assign_global_decl, func_params);
 }
@@ -3580,7 +3578,6 @@  objc_build_strong_cast_assignment (tree lhs, tree
 		      build_unary_op (input_location, ADDR_EXPR, lhs, 0)),
 		    NULL_TREE));
 
-  assemble_external (objc_assign_strong_cast_decl);
   return build_function_call (input_location,
 			      objc_assign_strong_cast_decl, func_params);
 }
Index: objc-next-runtime-abi-01.c
===================================================================
--- objc-next-runtime-abi-01.c	(revision 185603)
+++ objc-next-runtime-abi-01.c	(working copy)
@@ -977,7 +977,6 @@  next_runtime_abi_01_get_category_super_ref (locati
   /* else do it the slow way.  */
   add_class_reference (super_name);
   super_class = (inst_meth ? objc_get_class_decl : objc_get_meta_class_decl);
-/* assemble_external (super_class);*/
   super_name = my_build_string_pointer (IDENTIFIER_LENGTH (super_name) + 1,
 					IDENTIFIER_POINTER (super_name));
   /* super_class = objc_get{Meta}Class("CLASS_SUPER_NAME"); */
Index: objc-next-runtime-abi-02.c
===================================================================
--- objc-next-runtime-abi-02.c	(revision 185603)
+++ objc-next-runtime-abi-02.c	(working copy)
@@ -1509,7 +1509,6 @@  next_runtime_abi_02_get_category_super_ref (locati
   /* ??? Do we need to add the class ref anway for zero-link?  */
   /* else do it the slow way.  */
   super_class = (inst_meth ? objc_get_class_decl : objc_get_meta_class_decl);
-  /* assemble_external (super_class); */
   super_name = my_build_string_pointer (IDENTIFIER_LENGTH (super_name) + 1,
 					IDENTIFIER_POINTER (super_name));
   /* super_class = objc_get{Meta}Class("CLASS_SUPER_NAME"); */
Index: objc-gnu-runtime-abi-01.c
===================================================================
--- objc-gnu-runtime-abi-01.c	(revision 185603)
+++ objc-gnu-runtime-abi-01.c	(working copy)
@@ -574,8 +574,6 @@  gnu_runtime_abi_01_get_class_reference (tree ident
 						(IDENTIFIER_LENGTH (ident) + 1,
 						 IDENTIFIER_POINTER (ident)));
 
-  /* FIXME: Do we need this assemble_external() ? */
-  /* assemble_external (objc_get_class_decl);*/
   return build_function_call (input_location, objc_get_class_decl, params);
 }
 
@@ -839,8 +837,6 @@  gnu_runtime_abi_01_get_category_super_ref (locatio
 
   add_class_reference (super_name);
   super_class = (inst_meth ? objc_get_class_decl : objc_get_meta_class_decl);
-  /* FIXME: Do we need this assemble_external() ? */
-  /* assemble_external (super_class);*/
   super_name = my_build_string_pointer (IDENTIFIER_LENGTH (super_name) + 1,
 					IDENTIFIER_POINTER (super_name));
   /* super_class = get_{meta_}class("CLASS_SUPER_NAME");  */