public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch][objc] Do not call assemble_external
@ 2012-03-21 21:09 Steven Bosscher
  2012-03-21 21:24 ` Iain Sandoe
  2012-03-21 21:55 ` Mike Stump
  0 siblings, 2 replies; 9+ messages in thread
From: Steven Bosscher @ 2012-03-21 21:09 UTC (permalink / raw)
  To: GCC Patches, mikestump

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

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.

[-- Attachment #2: cleanup_objc_assemble_external.diff --]
[-- Type: text/x-patch, Size: 3622 bytes --]

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");  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 21:09 [patch][objc] Do not call assemble_external Steven Bosscher
@ 2012-03-21 21:24 ` Iain Sandoe
  2012-03-21 21:33   ` Steven Bosscher
  2012-03-21 21:55 ` Mike Stump
  1 sibling, 1 reply; 9+ messages in thread
From: Iain Sandoe @ 2012-03-21 21:24 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, mikestump

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 21:24 ` Iain Sandoe
@ 2012-03-21 21:33   ` Steven Bosscher
  2012-03-21 22:11     ` Mike Stump
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2012-03-21 21:33 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, mikestump

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 21:09 [patch][objc] Do not call assemble_external Steven Bosscher
  2012-03-21 21:24 ` Iain Sandoe
@ 2012-03-21 21:55 ` Mike Stump
  1 sibling, 0 replies; 9+ messages in thread
From: Mike Stump @ 2012-03-21 21:55 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches

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.  :-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 21:33   ` Steven Bosscher
@ 2012-03-21 22:11     ` Mike Stump
  2012-03-21 22:45       ` Steven Bosscher
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Stump @ 2012-03-21 22:11 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: Iain Sandoe, GCC Patches

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 22:11     ` Mike Stump
@ 2012-03-21 22:45       ` Steven Bosscher
  2012-03-21 23:53         ` Iain Sandoe
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2012-03-21 22:45 UTC (permalink / raw)
  To: Mike Stump; +Cc: Iain Sandoe, GCC Patches

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 22:45       ` Steven Bosscher
@ 2012-03-21 23:53         ` Iain Sandoe
  2012-03-22  0:00           ` Steven Bosscher
  0 siblings, 1 reply; 9+ messages in thread
From: Iain Sandoe @ 2012-03-21 23:53 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: Mike Stump, GCC Patches


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.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-21 23:53         ` Iain Sandoe
@ 2012-03-22  0:00           ` Steven Bosscher
  2012-03-22  0:17             ` Iain Sandoe
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2012-03-22  0:00 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Mike Stump, GCC Patches

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][objc] Do not call assemble_external
  2012-03-22  0:00           ` Steven Bosscher
@ 2012-03-22  0:17             ` Iain Sandoe
  0 siblings, 0 replies; 9+ messages in thread
From: Iain Sandoe @ 2012-03-22  0:17 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: Mike Stump, GCC Patches


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-03-22  0:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-21 21:09 [patch][objc] Do not call assemble_external Steven Bosscher
2012-03-21 21:24 ` Iain Sandoe
2012-03-21 21:33   ` Steven Bosscher
2012-03-21 22:11     ` Mike Stump
2012-03-21 22:45       ` Steven Bosscher
2012-03-21 23:53         ` Iain Sandoe
2012-03-22  0:00           ` Steven Bosscher
2012-03-22  0:17             ` Iain Sandoe
2012-03-21 21:55 ` Mike Stump

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).