Add gtGetOp2IfPresent()

It was noticed that many uses of gtGetOp2() fully expect op2 to
exist and be non-nullptr, and the use of gtGetOp2() was simply
for style/readability purposes. However, it has a cost, as it
checks whether the tree is binary first, and returns nullptr if
not. For most cases, this is unecessary and expensive.

Introduce a new gtGetOp2IfPresent() function that captures the
previous behavior: checking if the tree node is binary before
returning op2, or returning nullptr otherwise. gtGetOp2() is
changed to simply return gtOp2 directly, without any checking
in non-DEBUG builds. It can be used if you know the op2 exists.

Most uses of gtGetOp2() were left alone (and hence get the new
behavior). The ones that need the old behavior were renamed
gtGetOp2IfPresent().

Mostly newer code is affected, as older code would generally
access gtOp2 directly, whereas newer code started using gtGetOp2()
for style and symmetry, especially after gtGetOp1() was introduced.

1 files changed, 20 insertions, 2 deletions

diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index da61debf27..cbaa5afc6b 100644
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -1603,8 +1603,14 @@ public:
 
     inline GenTreePtr gtGetOp1();
 
+    // Directly return op2. Asserts the node is binary. Might return nullptr if the binary node allows
+    // a nullptr op2, such as GT_LIST. This is more efficient than gtGetOp2IfPresent() if you know what
+    // node type you have.
     inline GenTreePtr gtGetOp2();
 
+    // The returned pointer might be nullptr if the node is not binary, or if non-null op2 is not required.
+    inline GenTreePtr gtGetOp2IfPresent();
+
     // Given a tree node, if this is a child of that node, return the pointer to the child node so that it
     // can be modified; otherwise, return null.
     GenTreePtr* gtGetChildPointer(GenTreePtr parent);
@@ -5010,7 +5016,7 @@ inline bool GenTree::IsIntegralConstVector(ssize_t constVal)
     if ((gtOper == GT_SIMD) && (gtSIMD.gtSIMDIntrinsicID == SIMDIntrinsicInit) && gtGetOp1()->IsIntegralConst(constVal))
     {
         assert(varTypeIsIntegral(gtSIMD.gtSIMDBaseType));
-        assert(gtGetOp2() == nullptr);
+        assert(gtGetOp2IfPresent() == nullptr);
         return true;
     }
 #endif
@@ -5191,12 +5197,24 @@ inline bool GenTree::RequiresNonNullOp2(genTreeOps oper)
 
 inline GenTreePtr GenTree::gtGetOp2()
 {
+    assert(OperIsBinary());
+
+    GenTreePtr op2 = gtOp.gtOp2;
+
+    // Only allow null op2 if the node type allows it, e.g. GT_LIST.
+    assert((op2 != nullptr) || !RequiresNonNullOp2(gtOper));
+
+    return op2;
+}
+
+inline GenTreePtr GenTree::gtGetOp2IfPresent()
+{
     /* gtOp.gtOp2 is only valid for GTK_BINOP nodes. */
 
     GenTreePtr op2 = OperIsBinary() ? gtOp.gtOp2 : nullptr;
 
     // This documents the genTreeOps for which gtOp.gtOp2 cannot be nullptr.
-    // This helps prefix in its analyis of code which calls gtGetOp2()
+    // This helps prefix in its analysis of code which calls gtGetOp2()
 
     assert((op2 != nullptr) || !RequiresNonNullOp2(gtOper));