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

JIT: Remove JTRUE(relop) invariant in the backend #82766

Merged
merged 9 commits into from
Mar 7, 2023
1 change: 1 addition & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void genCodeForJcc(GenTreeCC* tree);
void genCodeForSetcc(GenTreeCC* setcc);
void genCodeForJTrue(GenTreeOp* jtrue);
#endif // !TARGET_LOONGARCH64
};

Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/codegenarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,22 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
}
}

//------------------------------------------------------------------------
// genCodeForJTrue: Produce code for a GT_JTRUE node.
//
// Arguments:
// jtrue - the node
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
inst_RV_RV(INS_tst, reg, reg, genActualType(op));
inst_JMP(EJ_ne, compiler->compCurBB->bbJumpDest);
}

//------------------------------------------------------------------------
// genCodeForReturnTrap: Produce code for a GT_RETURNTRAP node.
//
Expand Down
15 changes: 15 additions & 0 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4564,6 +4564,21 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
}
}

//------------------------------------------------------------------------
// genCodeForJTrue: Produce code for a GT_JTRUE node.
//
// Arguments:
// jtrue - the node
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
GetEmitter()->emitIns_J_R(INS_cbnz, emitActualTypeSize(op), compiler->compCurBB->bbJumpDest, reg);
}

//------------------------------------------------------------------------
// genCodeForConditionalCompare: Produce code for a compare that's dependent on a previous compare.
//
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
break;
#endif // TARGET_ARM64

case GT_JTRUE:
genCodeForJTrue(treeNode->AsOp());
break;

case GT_JCC:
genCodeForJcc(treeNode->AsCC());
break;
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,22 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
}
}

//------------------------------------------------------------------------
// genCodeForJTrue: Produce code for a GT_JTRUE node.
//
// Arguments:
// jtrue - the node
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
inst_RV_RV(INS_test, reg, reg, genActualType(op));
inst_JMP(EJ_jne, compiler->compCurBB->bbJumpDest);
}

//------------------------------------------------------------------------
// JumpKindToCmov:
// Convert an emitJumpKind to the corresponding cmov instruction.
Expand Down Expand Up @@ -1836,6 +1852,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
genCodeForCompare(treeNode->AsOp());
break;

case GT_JTRUE:
genCodeForJTrue(treeNode->AsOp());
break;

case GT_JCC:
genCodeForJcc(treeNode->AsCC());
break;
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12740,8 +12740,9 @@ void Compiler::gtDispRange(LIR::ReadOnlyRange const& range)
//
void Compiler::gtDispTreeRange(LIR::Range& containingRange, GenTree* tree)
{
bool unused;
gtDispRange(containingRange.GetTreeRange(tree, &unused));
bool closed;
unsigned sideEffects;
gtDispRange(containingRange.GetTreeRangeWithFlags(tree, &closed, &sideEffects));
}

//------------------------------------------------------------------------
Expand Down
71 changes: 64 additions & 7 deletions src/coreclr/jit/lir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1131,9 +1131,9 @@ bool LIR::Range::TryGetUse(GenTree* node, Use* use)
}

//------------------------------------------------------------------------
// LIR::Range::GetTreeRange: Computes the subrange that includes all nodes
// in the dataflow trees rooted at a particular
// set of nodes.
// LIR::Range::GetMarkedRange:
// Computes the subrange that includes all nodes in the dataflow trees rooted
// at a particular set of nodes.
//
// This method logically uses the following algorithm to compute the
// range:
Expand Down Expand Up @@ -1164,14 +1164,26 @@ bool LIR::Range::TryGetUse(GenTree* node, Use* use)
// occurring before uses).
//
// Arguments:
// root - The root of the dataflow tree.
// isClosed - An output parameter that is set to true if the returned
// range contains only nodes in the dataflow tree and false
// otherwise.
// markFlagsOperands - Whether the dataflow algorithm should also try to
// mark operands that are defining flags. For example,
// GT_JCC implicitly consumes the flags defined by the
// previous node in linear order. If this argument is
// true, then the algorithm will also include the flags
// definition (which is e.g. a CMP node) in the returned
// range.
// This works on a best-effort basis; the user should not
// rely on all flags defs to be found.
Comment on lines +1176 to +1184
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this down to "Template arguments"?

// root - The root of the dataflow tree.
// isClosed - An output parameter that is set to true if the returned
// range contains only nodes in the dataflow tree and false
// otherwise.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're here, can you fix this? root doesn't match the declaration. It's missing markCount and sideEffects.

//
// Type arguments:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this "Template arguments:" instead?

//
// Returns:
// The computed subrange.
//
template <bool markFlagsOperands>
LIR::ReadOnlyRange LIR::Range::GetMarkedRange(unsigned markCount,
GenTree* start,
bool* isClosed,
Expand All @@ -1182,6 +1194,12 @@ LIR::ReadOnlyRange LIR::Range::GetMarkedRange(unsigned markCount,
assert(isClosed != nullptr);
assert(sideEffects != nullptr);

#ifndef DEBUG
// The treatment of flags definitions is on a best-effort basis; it should
// be used for debug purposes only.
static_assert_no_msg(!markFlagsOperands);
#endif

bool sawUnmarkedNode = false;
unsigned sideEffectsInRange = 0;

Expand All @@ -1203,6 +1221,17 @@ LIR::ReadOnlyRange LIR::Range::GetMarkedRange(unsigned markCount,
return GenTree::VisitResult::Continue;
});

if (markFlagsOperands && firstNode->OperConsumesFlags())
{
GenTree* prev = firstNode->gtPrev;
if ((prev != nullptr) && ((prev->gtFlags & GTF_SET_FLAGS) != 0) &&
((prev->gtLIRFlags & LIR::Flags::Mark) == 0))
{
prev->gtLIRFlags |= LIR::Flags::Mark;
markCount++;
}
}

// Unmark the node and update `firstNode`
firstNode->gtLIRFlags &= ~LIR::Flags::Mark;
markCount--;
Expand Down Expand Up @@ -1282,6 +1311,34 @@ LIR::ReadOnlyRange LIR::Range::GetTreeRange(GenTree* root, bool* isClosed, unsig
return GetMarkedRange(markCount, root, isClosed, sideEffects);
}

#ifdef DEBUG
//------------------------------------------------------------------------
// LIR::Range::GetTreeRangeWithFlags:
// Computes the subrange that includes all nodes in the dataflow tree rooted at
// a particular node, taking into account that the nodes may also implicitly
// consume flags defined by non-operands.
//
// Arguments:
// root - The root of the dataflow tree range of nodes.
// isClosed - An output parameter that is set to true if the returned
// range contains only nodes in the dataflow tree and false
// otherwise.
// sideEffects - An output parameter that summarizes the side effects
// contained in the returned range.
//
// Returns:
// The computed subrange.
LIR::ReadOnlyRange LIR::Range::GetTreeRangeWithFlags(GenTree* root, bool* isClosed, unsigned* sideEffects) const
{
assert(root != nullptr);

unsigned markCount = 1;
root->gtLIRFlags |= LIR::Flags::Mark;

return GetMarkedRange<true>(markCount, root, isClosed, sideEffects);
}
#endif

//------------------------------------------------------------------------
// LIR::Range::GetTreeRange: Computes the subrange that includes all nodes
// in the dataflow trees rooted by the operands
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/lir.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ class LIR final
Range(const Range& other) = delete;
Range& operator=(const Range& other) = delete;

template <bool markFlagsOperands = false>
ReadOnlyRange GetMarkedRange(unsigned markCount, GenTree* start, bool* isClosed, unsigned* sideEffects) const;

void FinishInsertBefore(GenTree* insertionPoint, GenTree* first, GenTree* last);
Expand Down Expand Up @@ -291,6 +292,9 @@ class LIR final

ReadOnlyRange GetTreeRange(GenTree* root, bool* isClosed) const;
ReadOnlyRange GetTreeRange(GenTree* root, bool* isClosed, unsigned* sideEffects) const;
#ifdef DEBUG
ReadOnlyRange GetTreeRangeWithFlags(GenTree* root, bool* isClosed, unsigned* sideEffects) const;
#endif
ReadOnlyRange GetRangeOfOperandTrees(GenTree* root, bool* isClosed, unsigned* sideEffects) const;

#ifdef DEBUG
Expand Down