Skip to content

Commit

Permalink
JIT: Fix various HW intrinsic lowerings for unused values (#91213)
Browse files Browse the repository at this point in the history
In the test case we disable CSE but then hoisting hoists out a HWINTRINSIC node
that ends up not CSE'd. This results in a top level unused HWINTRINSIC node that
lowering didn't handle in a transformation. There's a bunch of transformations
that do not handle this correctly, so fix it in all of them.

I couldn't find a test case that does not require disabling CSE (and I didn't
include the existing one since it never finishes with the bug fixed), but I
wouldn't bet on that one doesn't exist, so I think we should backport this
anyway.

Fix #91173
  • Loading branch information
jakobbotsch authored Aug 29, 2023
1 parent dff23ec commit 88860b7
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1900,6 +1900,10 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(tmp2);
}
else
{
tmp2->SetUnusedValue();
}

BlockRange().Remove(node);
return tmp2->gtNext;
Expand Down
45 changes: 43 additions & 2 deletions src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3868,6 +3868,10 @@ GenTree* Lowering::LowerHWIntrinsicGetElement(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(newIndir);
}
else
{
newIndir->SetUnusedValue();
}

BlockRange().Remove(op1);
BlockRange().Remove(node);
Expand Down Expand Up @@ -3916,6 +3920,10 @@ GenTree* Lowering::LowerHWIntrinsicGetElement(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(lclFld);
}
else
{
lclFld->SetUnusedValue();
}

BlockRange().Remove(op1);
BlockRange().Remove(op2);
Expand Down Expand Up @@ -4158,6 +4166,11 @@ GenTree* Lowering::LowerHWIntrinsicGetElement(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(cast);
}
else
{
node->ClearUnusedValue();
cast->SetUnusedValue();
}
next = LowerNode(cast);
}

Expand Down Expand Up @@ -4737,6 +4750,10 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(tmp1);
}
else
{
tmp1->SetUnusedValue();
}

BlockRange().Remove(node);
return LowerNode(tmp1);
Expand Down Expand Up @@ -5267,6 +5284,10 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(tmp1);
}
else
{
tmp1->SetUnusedValue();
}

BlockRange().Remove(node);
return tmp1->gtNext;
Expand Down Expand Up @@ -5314,6 +5335,10 @@ GenTree* Lowering::LowerHWIntrinsicToScalar(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(newIndir);
}
else
{
newIndir->SetUnusedValue();
}

BlockRange().Remove(op1);
BlockRange().Remove(node);
Expand Down Expand Up @@ -5342,6 +5367,10 @@ GenTree* Lowering::LowerHWIntrinsicToScalar(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(lclFld);
}
else
{
lclFld->SetUnusedValue();
}

BlockRange().Remove(op1);
BlockRange().Remove(node);
Expand Down Expand Up @@ -5426,6 +5455,11 @@ GenTree* Lowering::LowerHWIntrinsicToScalar(GenTreeHWIntrinsic* node)
{
use.ReplaceWith(cast);
}
else
{
node->ClearUnusedValue();
cast->SetUnusedValue();
}
next = LowerNode(cast);
}

Expand Down Expand Up @@ -8332,8 +8366,15 @@ void Lowering::TryFoldCnsVecForEmbeddedBroadcast(GenTreeHWIntrinsic* parentNode,
BlockRange().InsertBefore(broadcastNode, createScalar);
BlockRange().InsertBefore(createScalar, constScalar);
LIR::Use use;
BlockRange().TryGetUse(childNode, &use);
use.ReplaceWith(broadcastNode);
if (BlockRange().TryGetUse(childNode, &use))
{
use.ReplaceWith(broadcastNode);
}
else
{
broadcastNode->SetUnusedValue();
}

BlockRange().Remove(childNode);
LowerNode(createScalar);
LowerNode(broadcastNode);
Expand Down

0 comments on commit 88860b7

Please sign in to comment.