From dc87572a2d24f7dd42bec3af3005f09bc2c26af3 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Wed, 7 Aug 2024 11:34:25 -0700
Subject: GTO: Remove minor optimization of avoiding ChildLocalizer sometimes
 (#6818)

The optimization is to only use ChildLocalizer, which moves children to
locals, if we actually have a reason to use it. It is simple enough to see if
we are removing fields with side effects here, and only call ChildLocalizer
if we are not. However, this will become much more complicated in a
subsequent PR which will reorder fields, which allows removing yet more
of them (without reordering, we can only remove fields at the end, if any
subtype needs the field).

This is a pretty minor optimization, as it avoids adding a few locals in the rare
case of struct.new operands having side effects. We run --gto at the
start of the pipeline, so later opts will clean that up anyhow. (Though, this
might make us a little less efficient, but the following PR will justify this
regression.)
---
 src/passes/GlobalTypeOptimization.cpp | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

(limited to 'src')

diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index e5cd4e410..536213561 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -22,7 +22,6 @@
 //  * Fields that are never read from can be removed entirely.
 //
 
-#include "ir/effects.h"
 #include "ir/localize.h"
 #include "ir/ordering.h"
 #include "ir/struct-utils.h"
@@ -348,26 +347,11 @@ struct GlobalTypeOptimization : public Pass {
         auto& operands = curr->operands;
         assert(indexesAfterRemoval.size() == operands.size());
 
-        // Check for side effects in removed fields. If there are any, we must
-        // use locals to save the values (while keeping them in order).
-        bool useLocals = false;
-        for (Index i = 0; i < operands.size(); i++) {
-          auto newIndex = indexesAfterRemoval[i];
-          if (newIndex == RemovedField &&
-              EffectAnalyzer(getPassOptions(), *getModule(), operands[i])
-                .hasUnremovableSideEffects()) {
-            useLocals = true;
-            break;
-          }
-        }
-        if (useLocals) {
-          auto* func = getFunction();
-          if (!func) {
-            Fatal() << "TODO: side effects in removed fields in globals\n";
-          }
-          ChildLocalizer localizer(curr, func, *getModule(), getPassOptions());
-          replaceCurrent(localizer.getReplacement());
-        }
+        // Localize things so that we can simply remove the operands we no
+        // longer need.
+        ChildLocalizer localizer(
+          curr, getFunction(), *getModule(), getPassOptions());
+        replaceCurrent(localizer.getReplacement());
 
         // Remove the unneeded operands.
         Index removed = 0;
@@ -381,6 +365,8 @@ struct GlobalTypeOptimization : public Pass {
           }
         }
         operands.resize(operands.size() - removed);
+        // We should only get here if we did actual work.
+        assert(removed > 0);
       }
 
       void visitStructSet(StructSet* curr) {
-- 
cgit v1.2.3