Skip to content
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.

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

Arm64-SVE: Fix up comments in optimizemaskconversions #109955

Merged
merged 3 commits into from
Nov 21, 2024
Merged
Changes from all commits
Commits
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
19 changes: 9 additions & 10 deletions src/coreclr/jit/optimizemaskconversions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class MaskConversionsCheckVisitor final : public GenTreeVisitor<MaskConversionsC
MaskConversionsWeight defaultWeight;
MaskConversionsWeight* weight = weightsTable->LookupPointerOrAdd(lclOp->GetLclNum(), defaultWeight);

JITDUMP("Local %s V%02d at [%06u] ", isLocalStore ? "store" : "var", lclOp->GetLclNum(),
JITDUMP("Local %s V%02d at [%06u] ", isLocalStore ? "store" : "use", lclOp->GetLclNum(),
m_compiler->dspTreeID(lclOp));

// Cannot convert any locals with an exposed address.
Expand All @@ -189,7 +189,7 @@ class MaskConversionsCheckVisitor final : public GenTreeVisitor<MaskConversionsC
// cannot be stored as a mask as data will be lost.
// For all of these, conversions could be done by creating a new store of type mask.
// Then uses as mask could be converted to type mask and pointed to use the new
// definition. Tbe weighting would need updating to take this into account.
// definition. The weighting would need updating to take this into account.
else if (isLocalUse && !hasConversion)
{
JITDUMP("is used as vector. ");
Expand Down Expand Up @@ -310,13 +310,13 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
// Quit if the cost of changing is higher or is invalid.
if (weight.currentCost <= weight.switchCost || weight.invalid)
{
JITDUMP("Local %s V%02d at [%06u] will not be converted. ", isLocalStore ? "store" : "var",
JITDUMP("Local %s V%02d at [%06u] will not be converted. ", isLocalStore ? "store" : "use",
lclOp->GetLclNum(), Compiler::dspTreeID(lclOp));
weight.DumpTotalWeight();
return fgWalkResult::WALK_CONTINUE;
}

JITDUMP("Local %s V%02d at [%06u] will be converted. ", isLocalStore ? "store" : "var", lclOp->GetLclNum(),
JITDUMP("Local %s V%02d at [%06u] will be converted. ", isLocalStore ? "store" : "use", lclOp->GetLclNum(),
Compiler::dspTreeID(lclOp));
weight.DumpTotalWeight();

Expand Down Expand Up @@ -377,7 +377,7 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
m_compiler->gtNewSimdCvtMaskToVectorNode(lclOrigType, lclOp, weight.simdBaseJitType, weight.simdSize);
}

JITDUMP("Updated %s V%02d at [%06u] to mask (%s conversion)\n", isLocalStore ? "store" : "var",
JITDUMP("Updated %s V%02d at [%06u] to mask (%s conversion)\n", isLocalStore ? "store" : "use",
lclOp->GetLclNum(), m_compiler->dspTreeID(lclOp), addConversion ? "added" : "removed");
DISPTREE(*use);

Expand Down Expand Up @@ -420,11 +420,10 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
// To optimize this, the pass searches every local variable definition (GT_STORE_LCL_VAR)
// and use (GT_LCL_VAR). A weighting is calculated and kept in a hash table - one entry
// for each lclvar number. The weighting contains two values. The first value is the count of
// of every convert node for the var, each instance multiplied by the number of instructions
// of every convert node for the var - each instance multiplied by the number of instructions
// in the convert and the weighting of the block it exists in. The second value assumes the
// local var has been switched to store as a mask and performs the same count. The switch
// will count removes every existing convert and add a convert where there isn't currently
// a convert.
// local var has been switched to the mask during the store and performs a similar count
// calculation to see what the cost of loading the "converted mask" values are.
//
// Once every definition and use has been parsed, the parsing runs again. At each step,
// if the weighting for switching that var is lower than the current weighting then switch
Expand All @@ -433,7 +432,7 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
// Limitations:
//
// Local variables that are defined then immediately used just once may not be saved to a
// store. Here a convert to to vector will be used by a convert to mask. These instances will
// store. Here a convert to vector will be used by a convert to mask. These instances will
// be caught in the lowering phase.
//
// This weighting does not account for re-definition. A variable may first be created as a
Expand Down
Loading