Skip to content

Conversation

@jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Aug 28, 2023

Fix #91173

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.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 28, 2023
@ghost ghost assigned jakobbotsch Aug 28, 2023
@ghost
Copy link

ghost commented Aug 28, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #91173

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.

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.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Aug 28, 2023

Might makes sense to inspect other TryGetUse+ReplaceUse patterns where we don't handle the else case?

@jakobbotsch
Copy link
Member Author

Might makes sense to inspect other TryGetUse+ReplaceUse patterns where we don't handle the else case?

Good idea, there's definitely more cases, e.g. in LowerHWIntrinsicGetElement.

@jakobbotsch jakobbotsch changed the title JIT: Fix LowerHWIntrinsicToScalar with unused values JIT: Fix various HW intrinsic lowerings for unused values Aug 28, 2023
@jakobbotsch jakobbotsch merged commit 88860b7 into dotnet:main Aug 29, 2023
@jakobbotsch jakobbotsch deleted the fix-91173 branch August 29, 2023 11:09
@jakobbotsch
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion failed 'node->IsUnusedValue() && "found an unmarked unused value"' during 'Lowering nodeinfo'

3 participants