[dart2js] Remove obsolete code from DCE

Remove an obsolete optimization: an instruction that has no users and
has the only effect of checking the the receiver is non-null, can be
removed when followed by another instruction with the same
null-checking effect.

With sound null safety, this rarely occurs. Most cases where this
would have fired are now explicit null checks that are not dead
because the second instruction is data-dependent of the check
instruction. There is similar code in instruction selection to omit
null checks, so we can rely on that.

The remaining cases are with back-to-back JS-fragments. This can be managed by adding a null check. Added a test case for an added null check.

Issue: #60327
Change-Id: Iace58bd944839166bc677d85bba2fe8d5c75da4c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446483
Reviewed-by: Mayank Patke <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>

Author: rakudrama

pkg/compiler/lib/src/ssa/optimize.dart

@@ -3213,56 +3213,6 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
     closedWorld,
   );
 
-  /// Determines whether we can delete [instruction] because the only thing it
-  /// does is throw the same exception as the next instruction that throws or
-  /// has an effect.
-  bool canFoldIntoFollowingInstruction(HInstruction instruction) {
-    assert(instruction.usedBy.isEmpty);
-    assert(instruction.canThrow(_abstractValueDomain));
-
-    if (!instruction.onlyThrowsNSM()) return false;
-
-    final receiver = instruction.getDartReceiver();
-    HInstruction? current = instruction.next;
-    do {
-      if ((current!.getDartReceiver() == receiver) &&
-          current.canThrow(_abstractValueDomain)) {
-        return true;
-      }
-      if (current is HForeignCode && current.isNullGuardFor(receiver)) {
-        return true;
-      }
-      if (current.canThrow(_abstractValueDomain) ||
-          current.sideEffects.hasSideEffects()) {
-        return false;
-      }
-
-      HInstruction? next = current.next;
-      if (next == null) {
-        // We do not merge blocks in our SSA graph, so if this block just jumps
-        // to a single successor, visit the successor, avoiding back-edges.
-        HBasicBlock? successor;
-        if (current is HGoto) {
-          successor = current.block!.successors.single;
-        } else if (current is HIf) {
-          // We also leave HIf nodes in place when one branch is dead.
-          HInstruction condition = current.inputs.first;
-          if (condition is HConstant) {
-            successor = condition.constant is TrueConstantValue
-                ? current.thenBlock
-                : current.elseBlock;
-            assert(successor.isLive);
-          }
-        }
-        if (successor != null && successor.id > current.block!.id) {
-          next = successor.first;
-        }
-      }
-      current = next;
-    } while (current != null);
-    return false;
-  }
-
   bool isTrivialDeadStoreReceiver(HInstruction instruction) {
     // For an allocation, if all the loads are dead (awaiting removal after
     // SsaLoadElimination) and the only other uses are stores, then the
@@ -3297,14 +3247,7 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
     if (isTrivialDeadStore(instruction)) return true;
     if (instruction.allowDCE) return true;
     if (instruction.sideEffects.hasSideEffects()) return false;
-    if (instruction.canThrow(_abstractValueDomain)) {
-      if (canFoldIntoFollowingInstruction(instruction)) {
-        // TODO(35996): If we remove [instruction], the source location of the
-        // 'equivalent' instruction should be updated.
-        return true;
-      }
-      return false;
-    }
+    if (instruction.canThrow(_abstractValueDomain)) return false;
     if (instruction is HParameterValue) return false;
     if (instruction is HLocalSet) return false;
     return true;

pkg/compiler/test/js/js_spec_optimization_test.dart

@@ -53,8 +53,8 @@ const String TEST_3 = r"""
         '#.toUpperCase()', s);
     print(s2);
 
-    // absent: 'toLowerCase' - removed since s.toUpperCase() generates the same
-    // noSuchMethod.
+    // present: 'toLowerCase'
+    // present: 'toUpperCase'
   }
 """;
 
@@ -85,6 +85,23 @@ const String TEST_5 = r"""
   }
 """;
 
+const String TEST_6 = r"""
+  import 'dart:_foreign_helper';
+  main() {
+    var s = JS('String|Null', '"Hello"');
+    s!;
+    var s1 = JS('returns:String;depends:none;effects:none;throws:null(1)',
+        '#.toLowerCase()', s);
+    var s2 = JS('returns:String;depends:none;effects:none;throws:null(1)',
+        '#.toUpperCase()', s);
+    print(s2);
+    // absent: 'toLowerCase' - removed since has no effect after null check.
+    // present: 'toUpperCase' - retained since used.
+    // present: '"Hello".toUpperCase' - null check folded into call.
+    // absent: '.toString;' - no null check as folded into 's.toUpperCase()'
+  }
+""";
+
 main() {
   runTests() async {
     check(String test) async {
@@ -112,6 +129,7 @@ main() {
     await check(TEST_3);
     await check(TEST_4);
     await check(TEST_5);
+    await check(TEST_6);
   }
 
   asyncTest(() async {