On 12/9/20 4:09 PM, Eric Gallager via Gcc-patches wrote:
On Fri, Dec 4, 2020 at 4:58 AM Erick Ochoa <
erick.oc...@theobroma-systems.com> wrote:
This commit includes the following components:
Type-based escape analysis to determine structs that can be modified at
link-time.
Field access analysis to determine which fields are never read.
The type-based escape analysis provides a list of types, that are not
visible outside of the current linking unit (e.g. parameter types of
external
functions).
The field access analyses non-escaping structs for fields that
are not used in the linking unit and thus can be removed.
2020-11-04 Erick Ochoa <erick.oc...@theobroma-systems.com>
* Makefile.in: Add file to list of new sources.
* common.opt: Add new flags.
* ipa-type-escape-analysis.c: New file.
---
gcc/Makefile.in | 1 +
gcc/common.opt | 8 +
gcc/ipa-type-escape-analysis.c | 3428 ++++++++++++++++++++++++++++++++
gcc/ipa-type-escape-analysis.h | 1152 +++++++++++
gcc/passes.def | 1 +
gcc/timevar.def | 1 +
gcc/tree-pass.h | 2 +
7 files changed, 4593 insertions(+)
create mode 100644 gcc/ipa-type-escape-analysis.c
create mode 100644 gcc/ipa-type-escape-analysis.h
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 978a08f7b04..8b18c9217a2 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1415,6 +1415,7 @@ OBJS = \
incpath.o \
init-regs.o \
internal-fn.o \
+ ipa-type-escape-analysis.o \
ipa-cp.o \
ipa-sra.o \
ipa-devirt.o \
diff --git a/gcc/common.opt b/gcc/common.opt
index d4cbb2f86a5..85351738a29 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3460,4 +3460,12 @@ fipa-ra
Common Report Var(flag_ipa_ra) Optimization
Use caller save register across calls if possible.
+fipa-type-escape-analysis
+Common Report Var(flag_ipa_type_escape_analysis) Optimization
+This flag is only used for debugging the type escape analysis
+
+Wdfa
+Common Var(warn_dfa) Init(1) Warning
+Warn about dead fields at link time.
+
I don't really like the name "-Wdfa" very much; could you maybe come up
with a longer and more descriptive name instead? Say, "-Wunused-field" or
"-Wunused-private-field" depending on the kind of field:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72789
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92801
I second that. Also, invoke.texi needs to document the new option
and if it's only active with LTO it should mention that. I looked
to see how the warning is used so I have some more comments on that
code (I didn't review the rest of the patch).
+// Obtain nonescaping unaccessed fields
+static record_field_offset_map_t
+obtain_nonescaping_unaccessed_fields (tpartitions_t casting,
+ record_field_map_t record_field_map)
+{
+ bool has_fields_that_can_be_deleted = false;
+ record_field_offset_map_t record_field_offset_map;
+ for (std::map<tree, field_access_map_t>::iterator i
+ = record_field_map.begin (),
+ e = record_field_map.end ();
+ i != e; ++i)
+ {
+ tree r_i = i->first;
+ std::vector<tree> equivalence
+ = find_equivalent_trees (r_i, record_field_map, casting);
+ field_offsets_t field_offset;
+ field_access_map_t original_field_map = record_field_map[r_i];
+ keep_only_read_fields_from_field_map (original_field_map,
field_offset);
+ keep_only_read_fields_from_equivalent_field_maps (equivalence,
+ record_field_map,
+ field_offset);
+ // These map holds the following:
+ // RECORD_TYPE -> unsigned (bit_pos_offset which has been read)
+ record_field_offset_map[r_i] = field_offset;
+ }
+
+ // So now that we only have the FIELDS which are read,
+ // we need to compute the complement...
+
+ // Improve: This is tightly coupled, I need to decouple it...
+ std::set<tree> to_erase;
+ std::set<tree> to_keep;
+ mark_escaping_types_to_be_deleted (record_field_offset_map, to_erase,
+ casting);
+ for (std::map<tree, field_offsets_t>::iterator i
+ = record_field_offset_map.begin (),
+ e = record_field_offset_map.end ();
+ i != e; ++i)
+ {
+ tree record = i->first;
+ const bool will_be_erased = to_erase.find (record) !=
to_erase.end ();
+ // No need to compute which fields can be deleted if type is
escaping
+ if (will_be_erased)
+ continue;
+
+ field_offsets_t field_offset = i->second;
+ for (tree field = TYPE_FIELDS (record); field; field = DECL_CHAIN
(field))
+ {
+ unsigned f_offset = bitpos_of_field (field);
+ bool in_set2 = field_offset.find (f_offset) != field_offset.end
();
+ if (in_set2)
+ {
+ field_offset.erase (f_offset);
+ continue;
+ }
+ to_keep.insert (record);
+ field_offset.insert (f_offset);
+ has_fields_that_can_be_deleted = true;
+ // NOTE: With anonymous fields this might be weird to print.
+ log ("%s.%s may be deleted\n",
+ type_stringifier::get_type_identifier (record).c_str (),
+ type_stringifier::get_field_identifier (field).c_str ());
+
+ if (OPT_Wdfa == 0) continue;
OPT_Wdfa is an enumerator so the above will never evaluate to true.
The warning() call checks to make sure a warning option is active
so checks like the above (which would otherwise use the warn_xxx
variable) aren't necessary, or really even reliable because they
don't faitfully reflect the state of warnings changed by #pragma
GCC diagnostic.
+ // Anonymous fields? (Which the record can be!).
+ warning (OPT_Wdfa, "RECORD_TYPE %qE has dead field %qE in
LTO.\n",
+ record, field);
The warning() call should really be warning_at() with the location
of the field (DECL_SOURCE_LOCATION (field). Otherwise I don't think
the location of this warning would be meaningful.
The RECORD_TYPE bit is a GCC-internal detail that shouldn't be
exposed in user-visible diagnostics. If record is a type, %qT
should be used to format it (the directive mentions the class-id
('struct' or 'union' or 'class' in C++).
Warning text should also not end in a period or newline. Most
of these formatting conventions are summarized in
https://www.gnu.org/prep/standards/standards.html#Errors
and
https://gcc.gnu.org/codingconventions.html#Diagnostics
If the warning isn't effective without LTO then specifying
the option without -flto should trigger a warning pointing it
out. Otherwise users who expect it to work might be misled to
believe that their code is warning free.
Martin
