On 10/16/14 18:41, David Malcolm wrote:
Anyway, just thought it was worth explicitly pointing out that these two
hunks of work, while they're tackling totally different issues may
conflict because of an implementation of the MPX bits.
Presumably my state cleanup patch isn't going to break the MPX bits
though? I haven't been following them in detail.
You won't break MPX, but they might break you due to state being saved
in file-scoped statics. I haven't really looked at the state they've
got and whether or not it needs to be cleared in a JIT world.
I should spell out that I haven't exhaustively hunted down every
file-scoped static in the source tree; this work merely fixes enough
such state to ensure that the testsuite passes:
Noted. We probably need to be more systematic in the future though.
What you've done is probably good enough for now though. Particularly
since this just affects JIT and we can iterate on the file statics.
I've got some ideas for automatically tracking down additional
file-scoped static state by analysis of source or of the .o files. Am I
right in assuming I can regard that as followup bugfixing?
Yes, this would be fine for follow-up fixing.
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 20b5c4e..794403d 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -2042,6 +2043,8 @@ bool resolution_used_from_other_file_p (enum
ld_plugin_symbol_resolution);
extern bool gimple_check_call_matching_types (gimple, tree, bool);
/* In cgraphunit.c */
+void cgraphunit_c_finalize (void);
Combine this with the declaration of init_lowered_empty_function so that
the references to cgraphunit's functions are together.
I believe they are, since it's immediately followed by...
Nevermind, I'm an idiot. Mentally had a "+" in front of the "In
cgraphunit" line and thought you'd added another block of stuff for
cgraphunit rather than just adding stuff to the existing block.
FWIW I've been experimenting with making the debugging structs be
classes, with the hooks becoming virtual functions. Then all of this
debugging state could be turned into member data of the singleton
instance of the appropriate debugging class, created when the
gcc::context is built in toplev initialization.
That would be a much cleaner approach (assuming vfuncs would be
acceptable for the debug hooks) - though I don't yet have a working
patch for this.
So for now I went with the less elegant but equally effective approach.
Understood.
diff --git a/gcc/passes.c b/gcc/passes.c
index 5001c3d..772993d 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1944,6 +1944,7 @@ execute_ipa_summary_passes (ipa_opt_pass_d *ipa_pass)
if (pass->tv_id)
timevar_push (pass->tv_id);
+ current_pass = pass;
ipa_pass->generate_summary ();
Presumably it doesn't make sense to push this down into
ipa_pass->generate_summary?
The issue here is that ipa-pure-const.c has some state that ought to be
member data of the pass_ipa_pure_const instance. The patch makes that
change, the issue is that there's no way to get at the relevant pass
instance just from the IPA pass hooks. They're not vfuncs, they're just
tables of function pointers.
(I skipped this part of the conversion of passes to C++ classes since
there wasn't a good way to handle the equivalent of checking to see if a
function ptr is NULL when turning it into a vfunc).
The easy fix is to simply ensure that "current_pass" is set before
jumping through the IPA hook, so that pass_ipa_pure_const's
implementation can find its instance and get at the data.
Ok. Thanks for the explanation.
jeff
