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

Updating lclmorph to support bitcast #71839

Closed
tannergooding opened this issue Jul 8, 2022 · 6 comments · Fixed by #84144
Closed

Updating lclmorph to support bitcast #71839

tannergooding opened this issue Jul 8, 2022 · 6 comments · Fixed by #84144
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@tannergooding
Copy link
Member

tannergooding commented Jul 8, 2022

With #71567, the front end now has minimal support for GT_BITCAST nodes including value numbering and constant folding.

It should now be feasible to address some of the TODO-ADDR comments in LocalAddressVisitor::SelectLocalIndirTransform and LocalAddressVisitor::MorphLocalAddress by recognizing certain indirections as being representable as a GT_BITCAST rather than keeping it as a GT_IND.

To start we should likely only handle float<->int/uint and double<->long/ulong. There may also be some benefit in handling conversions surrounding simd8.

In the future this could likely be extended to struct types and potentially other primitives as well.

category:cq
theme:morph

@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 Jul 8, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 8, 2022
@ghost
Copy link

ghost commented Jul 8, 2022

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

Issue Details

With #71567, the front end now has minimal support for GT_BITCAST nodes including value numbering and constant folding.

It should now be feasible to address some of the TODO-ADDR comments in LocalAddressVisitor::SelectLocalIndirTransform and LocalAddressVisitor::MorphLocalAddress by recognizing certain indirections as being representable as a GT_BITCAST rather than keeping it as a GT_IND.

To start we should likely only handle float<->int/uint and double<->long/ulong. There may also be some benefit in handling conversions surrounding simd8.

In the future this could likely be extended to struct types and potentially other primitives as well.

Author: tannergooding
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@JulieLeeMSFT
Copy link
Member

cc @dotnet/jit-contrib.

@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Jul 11, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 11, 2022
@AndyAyersMS
Copy link
Member

@tannergooding is this something you hope to see in .net 8?

@SingleAccretion
Copy link
Contributor

This is now effectively a one-line change:

// Turn this into a bitcast if we can.
if ((genTypeSize(indir) == genTypeSize(varDsc)) && (varTypeIsFloating(indir) || varTypeIsFloating(varDsc)))
{
// TODO-ADDR: enable this optimization for all users and all targets.
if (user->OperIs(GT_RETURN) && (genTypeSize(indir) <= TARGET_POINTER_SIZE))
{
return IndirTransform::BitCast;
}
}

@SingleAccretion SingleAccretion self-assigned this Nov 6, 2022
@TIHan
Copy link
Contributor

TIHan commented Feb 2, 2023

@SingleAccretion you still working on this one?

@SingleAccretion
Copy link
Contributor

Yes, if not "right now". If someone wants to do the (small) amount of work to close this issue though, they should feel free to do so.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 30, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 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 a pull request may close this issue.

5 participants