Cleanup and remove unused parameters from genCreateAddrMode (#18258)

* Cleanup and remove unused parameters from genCreateAddrMode, fixes #18177

diff --git a/src/jit/codegen.h b/src/jit/codegen.h
index d6cd23a4c99c38ad54e4c64f5c93ff62aa308439..57ff75726f2f08dd9afa912f3b90b9335d720db7 100644 (file)
--- a/src/jit/codegen.h
+++ b/src/jit/codegen.h
@@ -36,17 +36,14 @@ public:
     // TODO-Cleanup: Abstract out the part of this that finds the addressing mode, and
     // move it to Lower
     virtual bool genCreateAddrMode(GenTree*  addr,
-                                   int       mode,
                                    bool      fold,
-                                   regMaskTP regMask,
                                    bool*     revPtr,
                                    GenTree** rv1Ptr,
                                    GenTree** rv2Ptr,
 #if SCALED_ADDR_MODES
                                    unsigned* mulPtr,
-#endif
-                                   unsigned* cnsPtr,
-                                   bool      nogen = false);
+#endif // SCALED_ADDR_MODES
+                                   ssize_t* cnsPtr);
 
 private:
 #if defined(_TARGET_XARCH_)

diff --git a/src/jit/codegencommon.cpp b/src/jit/codegencommon.cpp
index 7f43fee24e70a00ba986191687bfb50634558391..a4497562d842fa3101c18b4846fd55bcea754497 100644 (file)
--- a/src/jit/codegencommon.cpp
+++ b/src/jit/codegencommon.cpp
@@ -1248,38 +1248,21 @@ unsigned CodeGenInterface::InferStructOpSizeAlign(GenTree* op, unsigned* alignme
  *  #endif
  *      *cnsPtr     ...     integer constant [optional]
  *
- *  The 'mode' parameter may have one of the following values:
- *
- *  #if LEA_AVAILABLE
- *         +1       ...     we're trying to compute a value via 'LEA'
- *  #endif
- *
- *          0       ...     we're trying to form an address mode
- *
- *         -1       ...     we're generating code for an address mode,
- *                          and thus the address must already form an
- *                          address mode (without any further work)
- *
  *  IMPORTANT NOTE: This routine doesn't generate any code, it merely
  *                  identifies the components that might be used to
  *                  form an address mode later on.
  */
 
 bool CodeGen::genCreateAddrMode(GenTree*  addr,
-                                int       mode,
                                 bool      fold,
-                                regMaskTP regMask,
                                 bool*     revPtr,
                                 GenTree** rv1Ptr,
                                 GenTree** rv2Ptr,
 #if SCALED_ADDR_MODES
                                 unsigned* mulPtr,
-#endif
-                                unsigned* cnsPtr,
-                                bool      nogen)
+#endif // SCALED_ADDR_MODES
+                                ssize_t* cnsPtr)
 {
-    assert(nogen == true);
-
     /*
         The following indirections are valid address modes on x86/x64:
 
@@ -1331,7 +1314,7 @@ bool CodeGen::genCreateAddrMode(GenTree*  addr,
     ssize_t cns;
 #if SCALED_ADDR_MODES
     unsigned mul;
-#endif
+#endif // SCALED_ADDR_MODES
 
     GenTree* tmp;
 
@@ -1367,7 +1350,7 @@ bool CodeGen::genCreateAddrMode(GenTree*  addr,
     cns = 0;
 #if SCALED_ADDR_MODES
     mul = 0;
-#endif
+#endif // SCALED_ADDR_MODES
 
 AGAIN:
     /* We come back to 'AGAIN' if we have an add of a constant, and we are folding that
@@ -1378,7 +1361,7 @@ AGAIN:
 
 #if SCALED_ADDR_MODES
     assert(mul == 0);
-#endif
+#endif // SCALED_ADDR_MODES
 
     /* Special case: keep constants as 'op2' */
 
@@ -1441,7 +1424,7 @@ AGAIN:
                         goto FOUND_AM;
                     }
                     break;
-#endif
+#endif // SCALED_ADDR_MODES && !defined(_TARGET_ARMARCH_)
 
                 default:
                     break;
@@ -1528,21 +1511,11 @@ AGAIN:
 
         case GT_NOP:
 
-            if (!nogen)
-            {
-                break;
-            }
-
             op1 = op1->gtOp.gtOp1;
             goto AGAIN;
 
         case GT_COMMA:
 
-            if (!nogen)
-            {
-                break;
-            }
-
             op1 = op1->gtOp.gtOp2;
             goto AGAIN;
 
@@ -1615,21 +1588,11 @@ AGAIN:
 
         case GT_NOP:
 
-            if (!nogen)
-            {
-                break;
-            }
-
             op2 = op2->gtOp.gtOp1;
             goto AGAIN;
 
         case GT_COMMA:
 
-            if (!nogen)
-            {
-                break;
-            }
-
             op2 = op2->gtOp.gtOp2;
             goto AGAIN;
 
@@ -1737,9 +1700,7 @@ FOUND_AM:
 #if SCALED_ADDR_MODES
     *mulPtr = mul;
 #endif
-    // TODO-Cleanup: The offset is signed and it should be returned as such. See also
-    // GenTreeAddrMode::gtOffset and its associated cleanup note.
-    *cnsPtr = (unsigned)cns;
+    *cnsPtr = cns;
 
     return true;
 }

diff --git a/src/jit/codegeninterface.h b/src/jit/codegeninterface.h
index 508386cf8c7159d079668c68e4f6c041201d6764..72f78c98b76f3f9a59829cd81901f62c44d62d0b 100644 (file)
--- a/src/jit/codegeninterface.h
+++ b/src/jit/codegeninterface.h
@@ -76,17 +76,14 @@ public:
     // TODO-Cleanup: Abstract out the part of this that finds the addressing mode, and
     // move it to Lower
     virtual bool genCreateAddrMode(GenTree*  addr,
-                                   int       mode,
                                    bool      fold,
-                                   regMaskTP regMask,
                                    bool*     revPtr,
                                    GenTree** rv1Ptr,
                                    GenTree** rv2Ptr,
 #if SCALED_ADDR_MODES
                                    unsigned* mulPtr,
-#endif
-                                   unsigned* cnsPtr,
-                                   bool      nogen = false) = 0;
+#endif // SCALED_ADDR_MODES
+                                   ssize_t* cnsPtr) = 0;
 
     void genCalcFrameSize();
 

diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 47d76e1a760c2ba76479aed91c272129d4a46941..bc3b56eb2cbf98e9e00d78cdf628226f2bc6d37b 100644 (file)
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -3571,8 +3571,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
                         bool rev;
 #if SCALED_ADDR_MODES
                         unsigned mul;
-#endif
-                        unsigned cns;
+#endif // SCALED_ADDR_MODES
+                        ssize_t  cns;
                         GenTree* base;
                         GenTree* idx;
 
@@ -3607,18 +3607,15 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
                             }
                         }
                         if ((doAddrMode) &&
-                            codeGen->genCreateAddrMode(addr,     // address
-                                                       0,        // mode
-                                                       false,    // fold
-                                                       RBM_NONE, // reg mask
-                                                       &rev,     // reverse ops
-                                                       &base,    // base addr
-                                                       &idx,     // index val
+                            codeGen->genCreateAddrMode(addr,  // address
+                                                       false, // fold
+                                                       &rev,  // reverse ops
+                                                       &base, // base addr
+                                                       &idx,  // index val
 #if SCALED_ADDR_MODES
-                                                       &mul, // scaling
-#endif
-                                                       &cns,  // displacement
-                                                       true)) // don't generate code
+                                                       &mul,  // scaling
+#endif                                                        // SCALED_ADDR_MODES
+                                                       &cns)) // displacement
                         {
                             // We can form a complex addressing mode, so mark each of the interior
                             // nodes with GTF_ADDRMODE_NO_CSE and calculate a more accurate cost.

diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index 5723b060f1334a36c1d8b5a50bd174f093703c24..1ee1d5baa450975063db471cd342ef59b750c1b0 100644 (file)
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -4539,15 +4539,10 @@ struct GenTreeAddrMode : public GenTreeOp
     unsigned gtScale; // The scale factor
 
 private:
-    // TODO-Cleanup: gtOffset should be changed to 'int' to match the getter function and avoid accidental
-    // zero extension to 64 bit. However, this is used by legacy code and initialized, via the offset
-    // parameter of the constructor, by Lowering::TryCreateAddrMode & CodeGenInterface::genCreateAddrMode.
-    // The later computes the offset as 'ssize_t' but returns it as 'unsigned'. We should change
-    // genCreateAddrMode to return 'int' or 'ssize_t' and then update this as well.
-    unsigned gtOffset; // The offset to add
+    ssize_t gtOffset; // The offset to add
 
 public:
-    GenTreeAddrMode(var_types type, GenTree* base, GenTree* index, unsigned scale, unsigned offset)
+    GenTreeAddrMode(var_types type, GenTree* base, GenTree* index, unsigned scale, ssize_t offset)
         : GenTreeOp(GT_LEA, type, base, index)
     {
         assert(base != nullptr || index != nullptr);

diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp
index 23015e0256005cba2970b81aaa4046af3730c34c..a14a5f2a6180a7dfaa5c21b6774226e2090cfe53 100644 (file)
--- a/src/jit/lower.cpp
+++ b/src/jit/lower.cpp
@@ -4341,7 +4341,7 @@ GenTree* Lowering::TryCreateAddrMode(LIR::Use&& use, bool isIndir)
     GenTree* base   = nullptr;
     GenTree* index  = nullptr;
     unsigned scale  = 0;
-    unsigned offset = 0;
+    ssize_t  offset = 0;
     bool     rev    = false;
 
     // TODO-1stClassStructs: This logic is here to preserve prior behavior. Note that previously
@@ -4373,8 +4373,15 @@ GenTree* Lowering::TryCreateAddrMode(LIR::Use&& use, bool isIndir)
     }
 
     // Find out if an addressing mode can be constructed
-    bool doAddrMode =
-        comp->codeGen->genCreateAddrMode(addr, -1, true, 0, &rev, &base, &index, &scale, &offset, true /*nogen*/);
+    bool doAddrMode = comp->codeGen->genCreateAddrMode(addr,   // address
+                                                       true,   // fold
+                                                       &rev,   // reverse ops
+                                                       &base,  // base addr
+                                                       &index, // index val
+#if SCALED_ADDR_MODES
+                                                       &scale,   // scaling
+#endif                                                           // SCALED_ADDR_MODES
+                                                       &offset); // displacement
 
     if (scale == 0)
     {
@@ -4411,12 +4418,12 @@ GenTree* Lowering::TryCreateAddrMode(LIR::Use&& use, bool isIndir)
     DISPNODE(base);
     if (index != nullptr)
     {
-        JITDUMP("  + Index * %u + %u\n    ", scale, offset);
+        JITDUMP("  + Index * %u + %d\n    ", scale, offset);
         DISPNODE(index);
     }
     else
     {
-        JITDUMP("  + %u\n", offset);
+        JITDUMP("  + %d\n", offset);
     }
 
     var_types addrModeType = addr->TypeGet();