This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch ObjC/Darwin 1/3] Fix wrong code gen for m64 Objective-C
Hi Nicola,
On 16 Feb 2011, at 13:25, Nicola Pero wrote:
thanks a lot for spending time on this.
likewise.
Is there any particular reason why this header file exists at all ?
I can coalesce the files if that's the consensus.
I would do it. I'm not sure that means "it's the consensus".
Someone else
would need to agree with me to make it the "consensus" ;-)
But I guess it's low priority. You can ignore it, and we'll (maybe)
change it in 4.7.
OK, I'll do it .. better to have more time for testing...
.. than to wait for other comment...
it's a stylistic point really
-- keep the declarative stuff separate and that means it's ready if
you want to separate the source into multiple files.
+#define
OBJCMETA(DECL,VERS,KIND)
[...]
This is new, and it's for the GNU runtime.
NeXT uses different sections for objc metadata (depending on the
entity). GNU does not.
At present, there is no way in LTO code to tell which front end
generated the metadata.
This results in incorrect placement of the GNU metadata into NeXT
sections for LTO.
Does the problem only occur with Objective-C code compiled for the
GNU runtime on Apple systems, and further compiled/linked using LTO ?
If so, maybe this very unlikely case could have been ignored for
stage 4 ?
Yes - this is true - unfortunately one of the 'very unlikely cases' is
the test-suite :-)
and Darwin tests both GNU and NeXT runtimes.
The tag is an ObjC private attribute - it should have no effect
unless
the target back-end acts upon it (which Darwin does for both GNU &
NeXT runtime code).
Ok, I guess we'll have to leave it in for 4.6. At this stage it
sounds
it would be more dangerous to rewrite the code to remove it than to
just
leave it in.
Hm - it's not about danger so much as there not being a suitable
alternative solution.
If you wanted to make super-safe for non - Darwin targets you could
include conditional code like
#ifdef NEXT_OBJC_RUNTIME
if (!objc_meta)
objc_meta = get_identifier ("OBJC1METG");
#else
objc_meta = NULL_TREE;
#endif
which would cause the attributes to be omitted ...
.. this puts a tm.h dependency there (but the header is already used
at 4.6 .. so nothing new)
Can we just revert the GNU runtime naming scheme to whatever it was ?
Just everywhere ?
All or none .. some are in common code ...
(I really don't think it's worth duplicating the string code just to
do this)
Then (as Mike suggested) I can literally compare the compiler output
for the
GNU runtime before and after your changes, see that they are
identical and
I'd feel much more confident about the changes. :-)
for the code-gen, fine....
.. I don't expect the _order_ of the metadata to be identical, I
expect its _content_ to be so.
(if that is not a harmless change, then the compiler would be broken
w.r.t. optimization which already moves it around).
Why moving the headers around ?
this one I will check and reply on separately.
Ok - let's ignore it. It's hardly a priority and it shouldn't break
anything.
I moved them when checking the minimum set of headers required for
the separated files.
(reverted).
bool
objc_init (void)
{
generate_struct_value_by_array does not return and does not use any
of
the other infrastructure of the compiler.
... there is no point in initializing anything - it won't be used.
Yes, I agree. On the other hand, what does this change have to do
with the
new Apple ABI ? :-)
there was a need to move init code around -
(when I did this last autumn) it seemed logical to tidy it up too.
Can you at least document it in the ChangeLog ?
of course.
And, you moved the flag_gen_declarations implementation from
objc_init () to objc_write_global_declarations ().
You also skip finish_objc() (and the flag_gen_declarations) if
flag_syntax_only or pch_file.
There is no point in opening / holding this open - unless metadata
generation is done, since it is not used otherwise.
Ok, but this has not much to do with the new Apple ABI, and as we
don't
have any testcases for the -gen-decls option, I wouldn't make changes
to it. Otherwise, you may at least want to add a testcase to confirm
it still works ?
OK, will think how best to do this.
With respect to skipping finish_objc() if flag_syntax_only, that
sounds
good, but, as you explain above, there are some language checks that
are
currently done in finish_objc(). So you may want to use -fsyntax-only
but still get the warnings generated by theses checks ?
I believe I've caught them .. but I will take a fresh look.
Anyhow, I guess to move forward quickly, you could document the
changes
in the ChangeLog and then someone would need to test that the various
combinations of -fsyntax-only, PCH etc. still work fine.
yes, as you know from my logging of bugs -
I have been doing this (to find that PCH is broken and untestable on
trunk objc++ anyway)
-static tree
+tree
get_objc_string_decl (tree ident, enum string_section section)
{
[...]
- gcc_unreachable ();
+ /* We didn't find the entry. */
return NULL_TREE;
}
I guess the gcc_unreachable is no longer unreachable ? In what
condition would it be reached ?
when the function is used in EH to test whether a class name is
already present (and therefore does not need to be added/emitted).
Hmmm. Ok, I had not read objc_eh_runtime_type() yet. I see you
changed
things there. It used to just emit the class name, it now tries to
look
it up in the class names that have already been emitted, presumably to
unique class names emitted in the object file.
I hope that doesn't break anything.
ummm...
exceptions don't work for GNU Objective-C++
(the code didn't even link until I did this - which does actually help
reduce test-suite noise).
perhaps we should 'sorry' GNU ObjC++ exceptions for 4.6?
otherwise, there are no test-suite regressions for GNU Objective-C on
any of the platforms I've tried.
cheers
Iain