From d5e8d6aeb57928b4e8a9ff5e00382e6ad111080f Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Sat, 15 Oct 2022 07:26:51 -0700
Subject: [PATCH 01/11] Forbid creation of non-faulting null-check nodes.

---
 src/coreclr/jit/compiler.cpp |  1 +
 src/coreclr/jit/gentree.cpp  |  3 +++
 src/coreclr/jit/lower.cpp    | 12 +++++++++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index ce74fbb1d95012..8fa12ae7c8d6ba 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -9893,6 +9893,7 @@ void Compiler::gtChangeOperToNullCheck(GenTree* tree, BasicBlock* block)
     assert(tree->OperIs(GT_FIELD, GT_IND, GT_OBJ, GT_BLK));
     tree->ChangeOper(GT_NULLCHECK);
     tree->ChangeType(gtTypeForNullCheck(tree));
+    assert(fgAddrCouldBeNull(tree->AsUnOp()->gtGetOp1()));
     block->bbFlags |= BBF_HAS_NULLCHECK;
     optMethodFlags |= OMF_HAS_NULLCHECK;
 }
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 2321a624fc74a1..b1479b7f1b71d2 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -15860,10 +15860,13 @@ void Compiler::gtExtractSideEffList(GenTree*     expr,
             {
                 if (m_compiler->gtNodeHasSideEffects(node, m_flags))
                 {
+                    // The node could be a side-effect free but marked as `GTF_MAKE_CSE`, so
+                    // we keep it.
                     PushSideEffects(node);
                     if (node->OperIsBlk() && !node->OperIsStoreBlk())
                     {
                         JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node));
+                        // It would change a non-faulting ind to a non-faulting nullcheck.
                         m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB);
                     }
                     return Compiler::WALK_SKIP_SUBTREES;
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 7fef6d758e4494..f68b5139fb7cd3 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7327,6 +7327,15 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas
     //
     assert(ind->OperIs(GT_NULLCHECK, GT_IND, GT_BLK, GT_OBJ));
 
+    GenTree* const addr = ind->Addr();
+    if (!comp->fgAddrCouldBeNull(addr))
+    {
+        addr->SetUnusedValue();
+        ind->gtBashToNOP();
+        JITDUMP("bash an unused indir [%06u] to NOP.\n", comp->dspTreeID(ind));
+        return;
+    }
+
     ind->ChangeType(comp->gtTypeForNullCheck(ind));
 
 #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64)
@@ -7334,7 +7343,8 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas
 #elif TARGET_ARM
     bool           useNullCheck          = false;
 #else  // TARGET_XARCH
-    bool useNullCheck = !ind->Addr()->isContained();
+    // bool useNullCheck = !ind->Addr()->isContained();
+    bool useNullCheck = !addr->isContained();
     ind->ClearDontExtend();
 #endif // !TARGET_XARCH
 

From 0a17d91cfd3ec7f22faf1f7873ccfe60ae69ccf6 Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Thu, 20 Oct 2022 08:03:59 -0700
Subject: [PATCH 02/11] Skip null check if op1 cannot be null.

---
 src/coreclr/jit/importer.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 42875a5df692a3..d49b0b046e5351 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -8392,7 +8392,14 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             // via an underlying address, just null check the address.
                             if (op1->OperIs(GT_FIELD, GT_IND, GT_OBJ))
                             {
-                                gtChangeOperToNullCheck(op1, block);
+                                if (fgAddrCouldBeNull(op1->AsUnOp()->gtGetOp1()))
+                                {
+                                    gtChangeOperToNullCheck(op1, block);
+                                }
+                                else
+                                {
+                                    op1 = gtNewNothingNode();
+                                }
                             }
                             else
                             {

From 37d8f34fc405c0d1c957225c931eff765da716ec Mon Sep 17 00:00:00 2001
From: Julie Lee <63486087+JulieLeeMSFT@users.noreply.github.com>
Date: Fri, 21 Oct 2022 17:27:46 -0700
Subject: [PATCH 03/11] x86 bug fix for Skip gtChangeOperToNullCheck if it
 cannot be null.

---
 src/coreclr/jit/gentree.cpp | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index b1479b7f1b71d2..ddcfeb51e3c580 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -15865,9 +15865,17 @@ void Compiler::gtExtractSideEffList(GenTree*     expr,
                     PushSideEffects(node);
                     if (node->OperIsBlk() && !node->OperIsStoreBlk())
                     {
-                        JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node));
-                        // It would change a non-faulting ind to a non-faulting nullcheck.
-                        m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB);
+                        if (m_compiler->fgAddrCouldBeNull(node->AsUnOp()->gtGetOp1()))
+                        {
+                            JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node));
+                            // It would change a non-faulting ind to a non-faulting nullcheck.
+                            m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB);
+                        }
+                        else
+                        {
+                            JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NOTHING node\n", dspTreeID(node));
+                            node = m_compiler->gtNewNothingNode();
+                        }
                     }
                     return Compiler::WALK_SKIP_SUBTREES;
                 }

From 6434f201176cc25704ebfeedc080446dae0d4f39 Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Wed, 26 Oct 2022 17:00:20 -0700
Subject: [PATCH 04/11] Fix Arm64 AVE: Do not lower indir when NOP.

---
 src/coreclr/jit/lower.cpp | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index f68b5139fb7cd3..a8003e4ca4d1e3 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7273,13 +7273,21 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
 #if defined(TARGET_ARM64)
         // Verify containment safety before creating an LEA that must be contained.
         //
-        const bool isContainable = IsSafeToContainMem(ind, ind->Addr());
+        const bool isContainable = false;
+        if (ind->Addr() != nullptr)
+        {
+            const bool isContainable = IsSafeToContainMem(ind, ind->Addr());
+        }
 #else
-        const bool isContainable         = true;
+        const bool isContainable     = true;
 #endif
 
-        TryCreateAddrMode(ind->Addr(), isContainable, ind);
-        ContainCheckIndir(ind);
+        if (!ind->OperIs(GT_NOP))
+        {
+            TryCreateAddrMode(ind->Addr(), isContainable, ind);
+            ContainCheckIndir(ind);
+        }
+        
 
 #ifdef TARGET_XARCH
         if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue())

From 4c1fddd93e29b4ca9b185b07b69330669214dedb Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Wed, 26 Oct 2022 17:49:47 -0700
Subject: [PATCH 05/11] formatting fix.

---
 src/coreclr/jit/lower.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index a8003e4ca4d1e3..88018eac3827bc 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7279,7 +7279,7 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
             const bool isContainable = IsSafeToContainMem(ind, ind->Addr());
         }
 #else
-        const bool isContainable     = true;
+        const bool isContainable         = true;
 #endif
 
         if (!ind->OperIs(GT_NOP))
@@ -7287,7 +7287,6 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
             TryCreateAddrMode(ind->Addr(), isContainable, ind);
             ContainCheckIndir(ind);
         }
-        
 
 #ifdef TARGET_XARCH
         if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue())

From 98b6b9030612bbbee9b437eb837b33b7e6cd1a8b Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Thu, 27 Oct 2022 14:51:09 -0700
Subject: [PATCH 06/11] Arm64 regression bug fix that removes const bool for a
 variable.

---
 src/coreclr/jit/lower.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 88018eac3827bc..05ad2053c22f33 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7273,10 +7273,10 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
 #if defined(TARGET_ARM64)
         // Verify containment safety before creating an LEA that must be contained.
         //
-        const bool isContainable = false;
+        bool isContainable = false;
         if (ind->Addr() != nullptr)
         {
-            const bool isContainable = IsSafeToContainMem(ind, ind->Addr());
+            isContainable = IsSafeToContainMem(ind, ind->Addr());
         }
 #else
         const bool isContainable         = true;

From d9bb584bbbd121ddd742d015a60180378ff989ed Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Wed, 28 Dec 2022 19:59:23 -0800
Subject: [PATCH 07/11] Preserve const and shorten expression.

---
 src/coreclr/jit/lower.cpp | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 05ad2053c22f33..2df25b1e8a3983 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7273,13 +7273,9 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
 #if defined(TARGET_ARM64)
         // Verify containment safety before creating an LEA that must be contained.
         //
-        bool isContainable = false;
-        if (ind->Addr() != nullptr)
-        {
-            isContainable = IsSafeToContainMem(ind, ind->Addr());
-        }
+        const bool isContainable = (ind->Addr() != nullptr) && IsSafeToContainMem(ind, ind->Addr());
 #else
-        const bool isContainable         = true;
+        const bool isContainable = true;
 #endif
 
         if (!ind->OperIs(GT_NOP))

From 6bb0d2697783c135327503772d5fef3f6361814b Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Wed, 28 Dec 2022 21:02:17 -0800
Subject: [PATCH 08/11] Fix format.

---
 src/coreclr/jit/lower.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 2df25b1e8a3983..3a3294e127c707 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7275,7 +7275,7 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
         //
         const bool isContainable = (ind->Addr() != nullptr) && IsSafeToContainMem(ind, ind->Addr());
 #else
-        const bool isContainable = true;
+        const bool isContainable         = true;
 #endif
 
         if (!ind->OperIs(GT_NOP))

From 7193ca32e33d9ec8cf7f8fd0e972e4cb00a95fff Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Fri, 20 Jan 2023 16:37:20 -0800
Subject: [PATCH 09/11] Addressed code review feedback

---
 src/coreclr/jit/compiler.cpp | 7 ++++---
 src/coreclr/jit/gentree.cpp  | 5 +++--
 src/coreclr/jit/importer.cpp | 2 +-
 src/coreclr/jit/lower.cpp    | 3 +--
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index 476debc9ff96b6..0a914c11d93ff5 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -9940,8 +9940,8 @@ var_types Compiler::gtTypeForNullCheck(GenTree* tree)
 // gtChangeOperToNullCheck: helper to change tree oper to a NULLCHECK.
 //
 // Arguments:
-//    tree       - the node to change;
-//    basicBlock - basic block of the node.
+//    tree  - the node to change;
+//    block - basic block of the node.
 //
 // Notes:
 //    the function should not be called after lowering for platforms that do not support
@@ -9953,7 +9953,8 @@ void Compiler::gtChangeOperToNullCheck(GenTree* tree, BasicBlock* block)
     assert(tree->OperIs(GT_FIELD, GT_IND, GT_OBJ, GT_BLK));
     tree->ChangeOper(GT_NULLCHECK);
     tree->ChangeType(gtTypeForNullCheck(tree));
-    assert(fgAddrCouldBeNull(tree->AsUnOp()->gtGetOp1()));
+    assert(fgAddrCouldBeNull(tree->gtGetOp1()));
+    tree->gtFlags |= GTF_EXCEPT;
     block->bbFlags |= BBF_HAS_NULLCHECK;
     optMethodFlags |= OMF_HAS_NULLCHECK;
 }
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 882124a9d58626..3ce926ee5987b2 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -16050,10 +16050,11 @@ void Compiler::gtExtractSideEffList(GenTree*     expr,
                     Append(node);
                     if (node->OperIsBlk() && !node->OperIsStoreBlk())
                     {
-                        if (m_compiler->fgAddrCouldBeNull(node->AsUnOp()->gtGetOp1()))
+                        // Check for a guaranteed non-faulting IND, and create a NOP node instead of a NULLCHECK in that
+                        // case.
+                        if (m_compiler->fgAddrCouldBeNull(node->AsBlk()->Addr()))
                         {
                             JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node));
-                            // It would change a non-faulting ind to a non-faulting nullcheck.
                             m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB);
                         }
                         else
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 9204a796461b8a..b12b9c9a296689 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -8251,7 +8251,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             // via an underlying address, just null check the address.
                             if (op1->OperIs(GT_FIELD, GT_IND, GT_OBJ))
                             {
-                                if (fgAddrCouldBeNull(op1->AsUnOp()->gtGetOp1()))
+                                if (fgAddrCouldBeNull(op1->gtGetOp1()))
                                 {
                                     gtChangeOperToNullCheck(op1, block);
                                 }
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 4bff95e902840b..93dfa116af9e46 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -7349,10 +7349,9 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas
 
 #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64)
     bool useNullCheck = true;
-#elif TARGET_ARM
+#elif defined(TARGET_ARM)
     bool           useNullCheck          = false;
 #else  // TARGET_XARCH
-    // bool useNullCheck = !ind->Addr()->isContained();
     bool useNullCheck = !addr->isContained();
     ind->ClearDontExtend();
 #endif // !TARGET_XARCH

From c984a910916aae745f1a76a82e6338aab1168c77 Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Wed, 25 Jan 2023 21:46:04 -0800
Subject: [PATCH 10/11] Forbid creation of non-faulting nullcheck node:
 Addressed feedback

---
 src/coreclr/jit/gentree.cpp  |  9 ++++++---
 src/coreclr/jit/importer.cpp | 10 ++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 3ce926ee5987b2..5f4bc68c5a9f78 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -16047,22 +16047,25 @@ void Compiler::gtExtractSideEffList(GenTree*     expr,
             {
                 if (m_compiler->gtNodeHasSideEffects(node, m_flags))
                 {
-                    Append(node);
                     if (node->OperIsBlk() && !node->OperIsStoreBlk())
                     {
                         // Check for a guaranteed non-faulting IND, and create a NOP node instead of a NULLCHECK in that
                         // case.
                         if (m_compiler->fgAddrCouldBeNull(node->AsBlk()->Addr()))
                         {
+                            Append(node);
                             JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NULLCHECK\n", dspTreeID(node));
                             m_compiler->gtChangeOperToNullCheck(node, m_compiler->compCurBB);
                         }
                         else
                         {
-                            JITDUMP("Replace an unused OBJ/BLK node [%06d] with a NOTHING node\n", dspTreeID(node));
-                            node = m_compiler->gtNewNothingNode();
+                            JITDUMP("Dropping non-faulting OBJ/BLK node [%06d]\n", dspTreeID(node));
                         }
                     }
+                    else
+                    {
+                        Append(node);
+                    }
                     return Compiler::WALK_SKIP_SUBTREES;
                 }
 
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index b12b9c9a296689..3f04daaa8f5e10 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -8251,6 +8251,16 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             // via an underlying address, just null check the address.
                             if (op1->OperIs(GT_FIELD, GT_IND, GT_OBJ))
                             {
+                                GenTree* addr = op1->gtGetOp1();
+                                if ((addr != nullptr) && fgAddrCouldBeNull(addr))
+                                {
+                                    gtChangeOperToNullCheck(op1, block);
+                                }
+                                else
+                                {
+                                    op1 = gtNewNothingNode();
+                                }
+
                                 if (fgAddrCouldBeNull(op1->gtGetOp1()))
                                 {
                                     gtChangeOperToNullCheck(op1, block);

From e20a756b6f5bcbee375bdd2eec903a7e17950fdd Mon Sep 17 00:00:00 2001
From: Julie Lee <julie.lee@microsoft.com>
Date: Thu, 26 Jan 2023 09:14:21 -0800
Subject: [PATCH 11/11] Forbid creation of non-faulting null-check nodes:
 Remove duplicate codes that was supposed to be deleted.

---
 src/coreclr/jit/importer.cpp | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 3f04daaa8f5e10..0604828b6a58d0 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -8260,15 +8260,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                                 {
                                     op1 = gtNewNothingNode();
                                 }
-
-                                if (fgAddrCouldBeNull(op1->gtGetOp1()))
-                                {
-                                    gtChangeOperToNullCheck(op1, block);
-                                }
-                                else
-                                {
-                                    op1 = gtNewNothingNode();
-                                }
                             }
                             else
                             {