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.

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

Jump to bottom

[LoongArch64] Fix code generation for type conversion from float/double to uint/ulong. #105783

Merged
merged 2 commits into from
Aug 7, 2024
Merged

[LoongArch64] Fix code generation for type conversion from float/double to uint/ulong. #105783

merged 2 commits into from
Aug 7, 2024

Conversation

sunlijun-610
Copy link
Contributor

The previous conversion from floating-point numbers to unsigned integers was incorrect in handling negative numbers and NaN.The previous instructions are as follows:
lu52i.d $r21, $zero, 1086
movgr2fr.d $ft3, $r21
fcmp.clt.d $fcc2, $fs0, $ft3
fcmp.ceq.d $fcc3, $fs0, $ft3
li.w $r21, 0x0
bcnez $fcc2, 16
fsub.d $ft3, $fs0, $ft3
li.w $r21, 0x1
slli.d $r21, $r21, 0x3f
fsel $ft3, $ft3, $fs0, $fcc2
ftintrz.l.d $ft3, $ft3
movfr2gr.d $a0, $ft3
bcnez $fcc3, 8
beqz $a0, 8
or $a0, $r21, $a0

After modification, negative numbers and NaN are handled first.The modified instructions are as follows:
li.w $a0, 0x0
movgr2fr.d $ft3, $zero
fcmp.cult.d $fcc2, $fa0, $ft3
bcnez $fcc2, 52
lu52i.d $r21, $zero, 1086
movgr2fr.d $ft3, $r21
fcmp.clt.d $fcc2, $fa0, $ft3
li.w $r21, 0x0
bcnez $fcc2, 16
fsub.d $ft3, $fa0, $ft3
li.w $r21, 0x1
slli.d $r21, $r21, 0x3f
fsel $ft3, $ft3, $fa0, $fcc2
ftintrz.l.d $ft3, $ft3
movfr2gr.d $a0, $ft3
or $a0, $r21, $a0

@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 1, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 1, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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

@sunlijun-610
Copy link
Contributor Author

@shushanhf Could you please review this change？
Thanks！

@huoyaoyuan
Copy link
Member

If LA64 doesn't have native conversion instruction for fp<->unsigned, are the JIT_Dbl2UInt JIT_Dbl2ULng helpers applicable?

@shushanhf
Copy link
Contributor

If LA64 doesn't have native conversion instruction for fp<->unsigned, are the JIT_Dbl2UInt JIT_Dbl2ULng helpers applicable?

We just use some instructions embedded by the codegen to avoid calling helpers.

@huoyaoyuan
Copy link
Member

Is there any JIT tests covering this? I can find src/tests/JIT/Methodical/NaN/r4NaNconv for NaN.

@sunlijun-610
Copy link
Contributor Author

Is there any JIT tests covering this? I can find src/tests/JIT/Methodical/NaN/r4NaNconv for NaN.

src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions for negative numbers and NaN.

@build-analysis build-analysis bot mentioned this pull request Aug 1, 2024
Open
3 tasks
shushanhf
shushanhf approved these changes Aug 1, 2024
View reviewed changes
Copy link
Contributor

@shushanhf shushanhf left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks

@jakobbotsch could you please review this PR?

@sunlijun-610
Copy link
Contributor Author

@jakobbotsch could you please review this PR?
Thanks!

@jakobbotsch
Copy link
Member

/azp run runtime

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Aug 6, 2024
Closed
Open
@jakobbotsch
Copy link
Member

/azp run runtime-dev-innerloop

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch jakobbotsch merged commit fa6f62b into dotnet:main Aug 7, 2024
99 of 106 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shushanhf shushanhf shushanhf approved these changes

@jakobbotsch jakobbotsch jakobbotsch approved these changes

Assignees
No one assigned
Labels
arch-loongarch64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sunlijun-610 @huoyaoyuan @shushanhf @jakobbotsch