On 11.11.20 03:25, Jakub Jelinek wrote:
On Wed, Nov 11, 2020 at 03:14:59AM -0800, Erick Ochoa wrote:
Using the Dead Field Analysis, Dead Field Elimination
automatically transforms gimple to eliminate fields that
are never read.
2020-11-04 Erick Ochoa <erick.oc...@theobroma-systems.com>
* gcc/Makefile.in: add file to list of sources
* gcc/ipa-dfe.c: New
* gcc/ipa-dfe.h: Same
* gcc/ipa-type-escape-analysis.h: Export code used in dfe.
* gcc/ipa-type-escape-analysis.c: Call transformation
Just random general nits, not a review.
The gcc/ prefix shouldn't be in the filenames, paths are relative
to the ChangeLog file into which it goes and gcc/ directory has a ChangeLog.
All entries should start with a capital letter, so Add above, and all should
end with a period (missing in all but one place).
---
gcc/Makefile.in | 1 +
gcc/ipa-dfe.c | 1284 ++++++++++++++++++++++++++++++++
gcc/ipa-dfe.h | 247 ++++++
gcc/ipa-type-escape-analysis.c | 22 +-
gcc/ipa-type-escape-analysis.h | 10 +
5 files changed, 1554 insertions(+), 10 deletions(-)
create mode 100644 gcc/ipa-dfe.c
create mode 100644 gcc/ipa-dfe.h
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8b18c9217a2..8ef6047870b 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1416,6 +1416,7 @@ OBJS = \
init-regs.o \
internal-fn.o \
ipa-type-escape-analysis.o \
+ ipa-dfe.o \
ipa-cp.o \
ipa-sra.o \
ipa-devirt.o \
diff --git a/gcc/ipa-dfe.c b/gcc/ipa-dfe.c
new file mode 100644
index 00000000000..5ba68332ad2
--- /dev/null
+++ b/gcc/ipa-dfe.c
@@ -0,0 +1,1284 @@
+/* IPA Type Escape Analysis and Dead Field Elimination
+ Copyright (C) 2019-2020 Free Software Foundation, Inc.
+
+ Contributed by Erick Ochoa <erick.oc...@theobroma-systems.com>
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+/* Interprocedural dead field elimination (IPA-DFE)
+
+ The goal of this transformation is to
+
+ 1) Create new types to replace RECORD_TYPEs which hold dead fields.
+ 2) Substitute instances of old RECORD_TYPEs for new RECORD_TYPEs.
+ 3) Substitute instances of old FIELD_DECLs for new FIELD_DECLs.
+ 4) Fix some instances of pointer arithmetic.
+ 5) Relayout where needed.
+
+ First stage - DFA
+ =================
+
+ Use DFA to compute the set of FIELD_DECLs which can be deleted.
+
+ Second stage - Reconstruct Types
+ ================================
+
+ This stage is done by two family of classes, the SpecificTypeCollector
+ and the TypeReconstructor.
+
+ The SpecificTypeCollector collects all TYPE_P trees which point to
+ RECORD_TYPE trees returned by DFA. The TypeReconstructor will create
+ new RECORD_TYPE trees and new TYPE_P trees replacing the old RECORD_TYPE
+ trees with the new RECORD_TYPE trees.
+
+ Third stage - Substitute Types and Relayout
+ ===========================================
+
+ This stage is handled by ExprRewriter and GimpleRewriter.
+ Some pointer arithmetic is fixed here to take into account those
eliminated
+ FIELD_DECLS.
+ */
+
+#include "config.h"
+
+#include <map>
+#include <set>
+#include <vector>
+#include <stack>
+#include <string>
We really do not want to use STL in GCC sources that much, we have our own
vectors and hash_sets/maps. If something is still needed after that,
the standard way to include STL header is define INCLUDE_ALGORITHM etc.
macros before including system.h.
Ah, I thought that just including them before was enough. Sure, I can
change this, however, my next version of this patch will remove the STL
from this pass.
+
+#include "system.h"
+ TypeStringifier stringifier;
GCC is not a CamelCase shop, so types etc. should use lower-case
only and underscores instead. This is everywhere in the patch.
Understood. Thanks!
+
+ // Here we are just placing the types of interest in a set.
+ for (std::map<tree, field_offsets_t>::const_iterator i
+ = record_field_offset_map.begin (),
+ e = record_field_offset_map.end ();
+ i != e; ++i)
This should just use hash_map.
Ditto.
+ for (std::set<tree>::const_iterator i = non_escaping.begin (),
+ e = non_escaping.end ();
+ i != e; ++i)
+ {
+ tree type = *i;
+ specifier.walk (type);
+ }
+
+ // These are all the types which need modifications.
+ std::set<tree> to_modify = specifier.get_set ();
And hash_set.
Ditto.
Also note that code-generation from hash table iterations should be
generally avoided, because the exact iteration order can change between
different runs (or e.g. between cross-compilers from different hosts etc.).
Ditto.
+ tree o_record = i->first;
+ std::string o_name = stringifier.stringify (o_record);
+ log ("original: %s\n", o_name.c_str ());
+ tree r_record = i->second;
+ std::string r_name
+ = r_record ? stringifier.stringify (r_record) : std::string ("");
+ log ("modified: %s\n", r_name.c_str ());
This should use the various diagnostic routines rather than std::string etc.
Ditto.
+static tree
+get_new_identifier (tree type)
+{
+ const char *identifier = TypeStringifier::get_type_identifier
(type).c_str ();
+ const bool is_new_type = strstr (identifier, "reorg");
reorg substring can appear in any user type name, can't it?
Yes, you are right. I can make this assertion more specific by matching
"\.reorg$". I'll make the change. Thanks!
Jakub