Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Cleanup and remove unused parameters from genCreateAddrMode (#18258)
Browse files Browse the repository at this point in the history 
* Cleanup and remove unused parameters from genCreateAddrMode, fixes #18177
  • Loading branch information
@Suchiman
Suchiman authored and Sergey Andreenko committed Jun 2, 2018
1 parent 84fcf2b commit ee78787
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 81 deletions.
7 changes: 2 additions & 5 deletions src/jit/codegen.h
View file
Expand Up @@ -36,17 +36,14 @@ class CodeGen : public CodeGenInterface
// 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_)
Expand Down
53 changes: 7 additions & 46 deletions src/jit/codegencommon.cpp
View file
Expand Up @@ -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:

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -1378,7 +1361,7 @@ bool CodeGen::genCreateAddrMode(GenTree* addr,

#if SCALED_ADDR_MODES
assert(mul == 0);
#endif
#endif // SCALED_ADDR_MODES

/* Special case: keep constants as 'op2' */

Expand Down Expand Up @@ -1441,7 +1424,7 @@ bool CodeGen::genCreateAddrMode(GenTree* addr,
goto FOUND_AM;
}
break;
#endif
#endif // SCALED_ADDR_MODES && !defined(_TARGET_ARMARCH_)

default:
break;
Expand Down Expand Up @@ -1528,21 +1511,11 @@ bool CodeGen::genCreateAddrMode(GenTree* addr,

case GT_NOP:

if (!nogen)
{
break;
}

op1 = op1->gtOp.gtOp1;
goto AGAIN;

case GT_COMMA:

if (!nogen)
{
break;
}

op1 = op1->gtOp.gtOp2;
goto AGAIN;

Expand Down Expand Up @@ -1615,21 +1588,11 @@ bool CodeGen::genCreateAddrMode(GenTree* addr,

case GT_NOP:

if (!nogen)
{
break;
}

op2 = op2->gtOp.gtOp1;
goto AGAIN;

case GT_COMMA:

if (!nogen)
{
break;
}

op2 = op2->gtOp.gtOp2;
goto AGAIN;

Expand Down Expand Up @@ -1737,9 +1700,7 @@ bool CodeGen::genCreateAddrMode(GenTree* addr,
#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;
}
Expand Down
7 changes: 2 additions & 5 deletions src/jit/codegeninterface.h
View file
Expand Up @@ -76,17 +76,14 @@ class CodeGenInterface
// 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();

Expand Down
23 changes: 10 additions & 13 deletions src/jit/gentree.cpp
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
9 changes: 2 additions & 7 deletions src/jit/gentree.h
View file
Expand Up @@ -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);
Expand Down
17 changes: 12 additions & 5 deletions src/jit/lower.cpp
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit ee78787

Please sign in to comment.