Skip to content

[AArch64] Eliminate Common SUBS by Reassociating Non-Constants #123344

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
merged 2 commits into from
Jan 21, 2025
Merged

[AArch64] Eliminate Common SUBS by Reassociating Non-Constants #123344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e9df16
[AArch64] Add Tests for Reassociation of SUBS with Non-Constant; NFC
mskamp Jan 11, 2025
e302dd6
[AArch64] Eliminate Common SUBS by Reassociating Non-Constants
mskamp Jan 11, 2025
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
58 changes: 42 additions & 16 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24898,16 +24898,31 @@ static SDValue reassociateCSELOperandsForCSE(SDNode *N, SelectionDAG &DAG) {
SDValue SubsNode = N->getOperand(3);
if (SubsNode.getOpcode() != AArch64ISD::SUBS || !SubsNode.hasOneUse())
return SDValue();
auto *CmpOpConst = dyn_cast<ConstantSDNode>(SubsNode.getOperand(1));
if (!CmpOpConst)
return SDValue();

SDValue CmpOpToMatch = SubsNode.getOperand(1);
SDValue CmpOpOther = SubsNode.getOperand(0);
EVT VT = N->getValueType(0);

unsigned ExpectedOpcode;
SDValue ExpectedOp;
SDValue SubsOp;
auto *CmpOpConst = dyn_cast<ConstantSDNode>(CmpOpToMatch);
if (CmpOpConst) {
ExpectedOpcode = ISD::ADD;
ExpectedOp =
DAG.getConstant(-CmpOpConst->getAPIntValue(), SDLoc(CmpOpConst),
CmpOpConst->getValueType(0));
SubsOp = DAG.getConstant(CmpOpConst->getAPIntValue(), SDLoc(CmpOpConst),
CmpOpConst->getValueType(0));
} else {
ExpectedOpcode = ISD::SUB;
ExpectedOp = CmpOpToMatch;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Constants are OK to create if it helps remove the functions pointers, they are simpler than other instructions (and feel like the lesser of two evils) :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good. In this way, we can get rid of at least one std::function.

SubsOp = CmpOpToMatch;
}

// Get the operand that can be reassociated with the SUBS instruction.
auto GetReassociationOp = [&](SDValue Op, APInt ExpectedConst) {
if (Op.getOpcode() != ISD::ADD)
auto GetReassociationOp = [&](SDValue Op, SDValue ExpectedOp) {
if (Op.getOpcode() != ExpectedOpcode)
return SDValue();
if (Op.getOperand(0).getOpcode() != ISD::ADD ||
!Op.getOperand(0).hasOneUse())
Expand All @@ -24918,24 +24933,21 @@ static SDValue reassociateCSELOperandsForCSE(SDNode *N, SelectionDAG &DAG) {
std::swap(X, Y);
if (X != CmpOpOther)
return SDValue();
auto *AddOpConst = dyn_cast<ConstantSDNode>(Op.getOperand(1));
if (!AddOpConst || AddOpConst->getAPIntValue() != ExpectedConst)
if (ExpectedOp != Op.getOperand(1))
return SDValue();
return Y;
};

// Try the reassociation using the given constant and condition code.
auto Fold = [&](APInt NewCmpConst, AArch64CC::CondCode NewCC) {
APInt ExpectedConst = -NewCmpConst;
SDValue TReassocOp = GetReassociationOp(N->getOperand(0), ExpectedConst);
SDValue FReassocOp = GetReassociationOp(N->getOperand(1), ExpectedConst);
auto Fold = [&](AArch64CC::CondCode NewCC, SDValue ExpectedOp,
SDValue SubsOp) {
SDValue TReassocOp = GetReassociationOp(N->getOperand(0), ExpectedOp);
SDValue FReassocOp = GetReassociationOp(N->getOperand(1), ExpectedOp);
if (!TReassocOp && !FReassocOp)
return SDValue();

SDValue NewCmp = DAG.getNode(AArch64ISD::SUBS, SDLoc(SubsNode),
DAG.getVTList(VT, MVT_CC), CmpOpOther,
DAG.getConstant(NewCmpConst, SDLoc(CmpOpConst),
CmpOpConst->getValueType(0)));
DAG.getVTList(VT, MVT_CC), CmpOpOther, SubsOp);

auto Reassociate = [&](SDValue ReassocOp, unsigned OpNum) {
if (!ReassocOp)
Expand All @@ -24957,9 +24969,19 @@ static SDValue reassociateCSELOperandsForCSE(SDNode *N, SelectionDAG &DAG) {

// First, try to eliminate the compare instruction by searching for a
// subtraction with the same constant.
if (SDValue R = Fold(CmpOpConst->getAPIntValue(), CC))
if (SDValue R = Fold(CC, ExpectedOp, SubsOp))
return R;

if (!CmpOpConst) {
// Try again with the operands of the SUBS instruction and the condition
// swapped. Due to canonicalization, this only helps for non-constant
// operands of the SUBS instruction.
std::swap(CmpOpToMatch, CmpOpOther);
if (SDValue R = Fold(getSwappedCondition(CC), CmpOpToMatch, CmpOpToMatch))
return R;
return SDValue();
}

if ((CC == AArch64CC::EQ || CC == AArch64CC::NE) && !CmpOpConst->isZero())
return SDValue();

Expand All @@ -24971,7 +24993,11 @@ static SDValue reassociateCSELOperandsForCSE(SDNode *N, SelectionDAG &DAG) {
// them here but check for them nevertheless to be on the safe side.
auto CheckedFold = [&](bool Check, APInt NewCmpConst,
AArch64CC::CondCode NewCC) {
return Check ? Fold(NewCmpConst, NewCC) : SDValue();
auto ExpectedOp = DAG.getConstant(-NewCmpConst, SDLoc(CmpOpConst),
CmpOpConst->getValueType(0));
auto SubsOp = DAG.getConstant(NewCmpConst, SDLoc(CmpOpConst),
CmpOpConst->getValueType(0));
return Check ? Fold(NewCC, ExpectedOp, SubsOp) : SDValue();
};
switch (CC) {
case AArch64CC::EQ:
Expand Down
Loading
Loading