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: Fix illegal IR created by GetElement/ToScalar lowering #91272

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

jakobbotsch
Copy link
Member

These could create TYP_ULONG/TYP_UINT indirs/LCL_FLD nodes, which are illegal.

Fix #91174

These could create TYP_ULONG/TYP_UINT indirs/LCL_FLD nodes, which are
illegal.

Fix dotnet#91174
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 29, 2023
@ghost ghost assigned jakobbotsch Aug 29, 2023
@ghost
Copy link

ghost commented Aug 29, 2023

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

Issue Details

These could create TYP_ULONG/TYP_UINT indirs/LCL_FLD nodes, which are illegal.

Fix #91174

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo

A couple of diffs where containment now kicks in due to matching types.

@jakobbotsch jakobbotsch requested a review from EgorBo August 30, 2023 16:11
@@ -3860,7 +3857,8 @@ GenTree* Lowering::LowerHWIntrinsicGetElement(GenTreeHWIntrinsic* node)
new (comp, GT_LEA) GenTreeAddrMode(addr->TypeGet(), newBase, newIndex, newScale, newOffset);
BlockRange().InsertBefore(node, newAddr);

GenTreeIndir* newIndir = comp->gtNewIndir(newType, newAddr, (indir->gtFlags & GTF_IND_FLAGS));
GenTreeIndir* newIndir =
Copy link
Member

Choose a reason for hiding this comment

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

Would it be good to cache the JITtype2varType(simdBaseJitType) somewhere, rather than duplicating it in 3 places?

Copy link
Member Author

Choose a reason for hiding this comment

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

I preferred the minor duplication here since we don't expect these transformations to kick in that often, so we would usually end up not using the computed value if we did it before.

@jakobbotsch jakobbotsch merged commit d9d47ee into dotnet:main Aug 30, 2023
@jakobbotsch jakobbotsch deleted the fix-91174 branch August 30, 2023 17:24
@jakobbotsch
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6145328559

@ghost ghost locked as resolved and limited conversation to collaborators Oct 11, 2023
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 '!varTypeIsUnsigned(type) || varTypeIsSmall(type)' during 'Generate code'
3 participants