Skip to content

Delete the zero-offset field sequence map #71455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jakobbotsch merged 13 commits into from
Jul 8, 2022
Merged

Delete the zero-offset field sequence map #71455

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
85a0ab4
Delete the unused GTF_ICON_FIELD_OFF
SingleAccretion May 9, 2022
86f0e3a
Do not try to find "IND(struct ARR_ADDR)" handle
SingleAccretion Jun 14, 2022
4822795
Delete the zero-offset field sequence map
SingleAccretion May 9, 2022
2e9e7b7
Make all field sequences singletons
SingleAccretion May 9, 2022
3a9095e
Print struct field offsets in the sequence
SingleAccretion May 9, 2022
5c87956
Improve address mode marking to handle COMMAs
SingleAccretion Jun 14, 2022
f2c866e
Delete NotAField
SingleAccretion Jun 29, 2022
2159def
Refactor the field sequence store
SingleAccretion Jun 29, 2022
f5341c2
FieldSeqNode -> FieldSeq
SingleAccretion Jun 29, 2022
c760d47
Turn asserts into "noway_assert"s
SingleAccretion Jun 29, 2022
319febe
Fix formatting
SingleAccretion Jun 30, 2022
bdbaf25
Take down SPMI
SingleAccretion Jul 2, 2022
0cad61a
Resurrect SPMI to work around a CG2 bug
SingleAccretion Jul 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Improve address mode marking to handle COMMAs 
This avoids regressing cases where we now have new CSE
opportunities for statics with static init helper calls.
  • Loading branch information
@SingleAccretion
SingleAccretion committed Jul 2, 2022
commit 5c87956ad5e61e32ef854379b458eabf3fba8f51
105 changes: 64 additions & 41 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3903,6 +3903,9 @@ bool Compiler::gtCanSwapOrder(GenTree* firstNode, GenTree* secondNode)
//
bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_types type)
{
GenTree* addrComma = addr;
addr = addr->gtEffectiveVal(/* commaOnly */ true);

// These are "out" parameters on the call to genCreateAddrMode():
bool rev; // This will be true if the operands will need to be reversed. At this point we
// don't care about this because we're not yet instantiating this addressing mode.
Expand All @@ -3929,8 +3932,13 @@ bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_typ

// 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.

addr->gtFlags |= GTF_ADDRMODE_NO_CSE;

int originalAddrCostEx = addr->GetCostEx();
int originalAddrCostSz = addr->GetCostSz();
int addrModeCostEx = 0;
int addrModeCostSz = 0;

#ifdef TARGET_XARCH
// addrmodeCount is the count of items that we used to form
// an addressing mode. The maximum value is 4 when we have
Expand All @@ -3939,27 +3947,27 @@ bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_typ
unsigned addrmodeCount = 0;
if (base)
{
*pCostEx += base->GetCostEx();
*pCostSz += base->GetCostSz();
addrModeCostEx += base->GetCostEx();
addrModeCostSz += base->GetCostSz();
addrmodeCount++;
}

if (idx)
{
*pCostEx += idx->GetCostEx();
*pCostSz += idx->GetCostSz();
addrModeCostEx += idx->GetCostEx();
addrModeCostSz += idx->GetCostSz();
addrmodeCount++;
}

if (cns)
{
if (((signed char)cns) == ((int)cns))
{
*pCostSz += 1;
addrModeCostSz += 1;
}
else
{
*pCostSz += 4;
addrModeCostSz += 4;
}
addrmodeCount++;
}
Expand Down Expand Up @@ -4026,21 +4034,21 @@ bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_typ
#elif defined TARGET_ARM
if (base)
{
*pCostEx += base->GetCostEx();
*pCostSz += base->GetCostSz();
addrModeCostEx += base->GetCostEx();
addrModeCostSz += base->GetCostSz();
if ((base->gtOper == GT_LCL_VAR) && ((idx == NULL) || (cns == 0)))
{
*pCostSz -= 1;
addrModeCostSz -= 1;
}
}

if (idx)
{
*pCostEx += idx->GetCostEx();
*pCostSz += idx->GetCostSz();
addrModeCostEx += idx->GetCostEx();
addrModeCostSz += idx->GetCostSz();
if (mul > 0)
{
*pCostSz += 2;
addrModeCostSz += 2;
}
}

Expand All @@ -4052,56 +4060,56 @@ bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_typ
{
if (!varTypeIsFloating(type))
{
*pCostSz += 2;
addrModeCostSz += 2;
}
}
else
{
*pCostEx += 2; // Very large offsets require movw/movt instructions
*pCostSz += 8;
addrModeCostEx += 2; // Very large offsets require movw/movt instructions
addrModeCostSz += 8;
}
}
}
#elif defined TARGET_ARM64
if (base)
{
*pCostEx += base->GetCostEx();
*pCostSz += base->GetCostSz();
addrModeCostEx += base->GetCostEx();
addrModeCostSz += base->GetCostSz();
}

if (idx)
{
*pCostEx += idx->GetCostEx();
*pCostSz += idx->GetCostSz();
addrModeCostEx += idx->GetCostEx();
addrModeCostSz += idx->GetCostSz();
}

if (cns != 0)
{
if (cns >= (4096 * genTypeSize(type)))
{
*pCostEx += 1;
*pCostSz += 4;
addrModeCostEx += 1;
addrModeCostSz += 4;
}
}
#elif defined(TARGET_LOONGARCH64)
if (base)
{
*pCostEx += base->GetCostEx();
*pCostSz += base->GetCostSz();
addrModeCostEx += base->GetCostEx();
addrModeCostSz += base->GetCostSz();
}

if (idx)
{
*pCostEx += idx->GetCostEx();
*pCostSz += idx->GetCostSz();
addrModeCostEx += idx->GetCostEx();
addrModeCostSz += idx->GetCostSz();
}
if (cns != 0)
{
if (!emitter::isValidSimm12(cns))
{
// TODO-LoongArch64-CQ: tune for LoongArch64.
*pCostEx += 1;
*pCostSz += 4;
addrModeCostEx += 1;
addrModeCostSz += 4;
}
}
#else
Expand Down Expand Up @@ -4243,9 +4251,27 @@ bool Compiler::gtMarkAddrMode(GenTree* addr, int* pCostEx, int* pCostSz, var_typ
// op1 isn't base or idx. Is this possible? Or should there be an assert?
}
}

// Finally, adjust the costs on the parenting COMMAs.
while (addrComma != addr)
{
int addrCostExDelta = originalAddrCostEx - addrModeCostEx;
int addrCostSzDelta = originalAddrCostSz - addrModeCostSz;
addrComma->SetCosts(addrComma->GetCostEx() - addrCostExDelta, addrComma->GetCostSz() - addrCostExDelta);

*pCostEx += addrComma->AsOp()->gtGetOp1()->GetCostEx();
*pCostSz += addrComma->AsOp()->gtGetOp1()->GetCostSz();

addrComma = addrComma->AsOp()->gtGetOp2();
}

*pCostEx += addrModeCostEx;
*pCostSz += addrModeCostSz;

return true;

} // end if (genCreateAddrMode(...))
} // if (genCreateAddrMode(...))

return false;
}

Expand Down Expand Up @@ -4968,7 +4994,7 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)

case GT_BLK:
case GT_IND:

{
/* An indirection should always have a non-zero level.
* Only constant leaf nodes have level 0.
*/
Expand Down Expand Up @@ -4997,35 +5023,31 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
#endif // TARGET_ARM

// Can we form an addressing mode with this indirection?
// TODO-CQ: Consider changing this to op1->gtEffectiveVal() to take into account
// addressing modes hidden under a comma node.
GenTree* addr = op1->gtEffectiveVal();

if (op1->gtOper == GT_ADD)
if (addr->OperIs(GT_ADD))
{
// See if we can form a complex addressing mode.

GenTree* addr = op1->gtEffectiveVal();

bool doAddrMode = true;

// TODO-1stClassStructs: Always do this, but first make sure it's done in Lowering as well.
if (tree->TypeGet() == TYP_STRUCT)
{
doAddrMode = false;
}

#ifdef TARGET_ARM64
if (tree->gtFlags & GTF_IND_VOLATILE)
if (tree->AsIndir()->IsVolatile())
{
// For volatile store/loads when address is contained we always emit `dmb`
// if it's not - we emit one-way barriers i.e. ldar/stlr
doAddrMode = false;
}
#endif // TARGET_ARM64
if (doAddrMode && gtMarkAddrMode(addr, &costEx, &costSz, tree->TypeGet()))
if (doAddrMode && gtMarkAddrMode(op1, &costEx, &costSz, tree->TypeGet()))
{
goto DONE;
}
} // end if (op1->gtOper == GT_ADD)
}
else if (gtIsLikelyRegVar(op1))
{
/* Indirection of an enregister LCL_VAR, don't increase costEx/costSz */
Expand All @@ -5042,7 +5064,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
goto DONE;
}
#endif
break;
}
break;

default:
break;
Expand Down