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

Reply via email to