Skip to content

JIT: Build and consume FMA and Permute node operands in standard order #103100

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 4 commits into from
Jun 6, 2024
Merged

JIT: Build and consume FMA and Permute node operands in standard order #103100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0b1dcd
JIT: Build and consume FMA node operands in standard order
jakobbotsch Jun 5, 2024
f4b6fd9
Handle resultOpNum == 0
jakobbotsch Jun 5, 2024
c36103c
Base it on the old code
jakobbotsch Jun 5, 2024
d6bbc45
Apply same fix to Permute intrinsics
jakobbotsch Jun 6, 2024
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
22 changes: 2 additions & 20 deletions src/coreclr/jit/hwintrinsiccodegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3071,6 +3071,8 @@ void CodeGen::genFMAIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions)

regNumber targetReg = node->GetRegNum();

genConsumeMultiOpOperands(node);

regNumber op1NodeReg = op1->GetRegNum();
regNumber op2NodeReg = op2->GetRegNum();
regNumber op3NodeReg = op3->GetRegNum();
Expand All @@ -3084,11 +3086,6 @@ void CodeGen::genFMAIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions)
// Intrinsics with CopyUpperBits semantics cannot have op1 be contained
assert(!copiesUpperBits || !op1->isContained());

// We need to keep this in sync with lsraxarch.cpp
// Ideally we'd actually swap the operands in lsra and simplify codegen
// but its a bit more complicated to do so for many operands as well
// as being complicated to tell codegen how to pick the right instruction

instruction ins = INS_invalid;

if (op1->isContained() || op1->isUsedFromSpillTemp())
Expand Down Expand Up @@ -3159,21 +3156,6 @@ void CodeGen::genFMAIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions)
}
}

#ifdef DEBUG
// Use nums are assigned in LIR order but this node is special and doesn't
// actually use operands. Fix up the use nums here to avoid asserts.
unsigned useNum1 = op1->gtUseNum;
unsigned useNum2 = op2->gtUseNum;
unsigned useNum3 = op3->gtUseNum;
emitOp1->gtUseNum = useNum1;
emitOp2->gtUseNum = useNum2;
emitOp3->gtUseNum = useNum3;
#endif

genConsumeRegs(emitOp1);
genConsumeRegs(emitOp2);
genConsumeRegs(emitOp3);

assert(ins != INS_invalid);
genHWIntrinsic_R_R_R_RM(ins, attr, targetReg, emitOp1->GetRegNum(), emitOp2->GetRegNum(), emitOp3, instOptions);
genProduceReg(node);
Expand Down
43 changes: 34 additions & 9 deletions src/coreclr/jit/lsraxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,11 +2544,24 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCou
std::swap(emitOp1, emitOp3);
}
}
tgtPrefUse = BuildUse(emitOp1);

srcCount += 1;
srcCount += BuildDelayFreeUses(emitOp2, emitOp1);
srcCount += emitOp3->isContained() ? BuildOperandUses(emitOp3) : BuildDelayFreeUses(emitOp3, emitOp1);
GenTree* ops[] = {op1, op2, op3};
for (GenTree* op : ops)
{
if (op == emitOp1)
{
tgtPrefUse = BuildUse(op);
srcCount++;
}
else if (op == emitOp2)
{
srcCount += BuildDelayFreeUses(op, emitOp1);
}
else if (op == emitOp3)
{
srcCount += op->isContained() ? BuildOperandUses(op) : BuildDelayFreeUses(op, emitOp1);
}
}
Copy link
Member Author

@jakobbotsch jakobbotsch Jun 6, 2024

Choose a reason for hiding this comment

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

This could probably be cleaned up to just build the uses directly without first figuring out emitOp1, emitOp2, emitOp3, but I didn't really understand all the constraints around copiesUpperBits, the preferred target and the contained operand, so I gave up on trying to do it.


if (intrinsicTree->OperIsEmbRoundingEnabled() && !intrinsicTree->Op(4)->IsCnsIntOrI())
{
Expand Down Expand Up @@ -2643,11 +2656,23 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCou
std::swap(emitOp1, emitOp2);
}

tgtPrefUse = BuildUse(emitOp1);

srcCount += 1;
srcCount += BuildDelayFreeUses(emitOp2, emitOp1);
srcCount += op3->isContained() ? BuildOperandUses(emitOp3) : BuildDelayFreeUses(emitOp3, emitOp1);
GenTree* ops[] = {op1, op2, op3};
for (GenTree* op : ops)
{
if (op == emitOp1)
{
tgtPrefUse = BuildUse(op);
srcCount++;
}
else if (op == emitOp2)
{
srcCount += BuildDelayFreeUses(op, emitOp1);
}
else if (op == emitOp3)
{
srcCount += op->isContained() ? BuildOperandUses(op) : BuildDelayFreeUses(op, emitOp1);
}
}

buildUses = false;
break;
Expand Down