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

Revert "Handle casts done via helpers and fold overflow operations value numbering (#50450)" #51401

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Apr 16, 2021

This is causing too many failures for me to investigate in time (#51381, #51380), so reverting the change wholesale.

I am planning to revisit this with the approach of not folding UB casts.

Note that UB means Undefined Behavior

cc @BruceForstall, @tannergooding

@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 Apr 16, 2021
@tannergooding
Copy link
Member

tannergooding commented Apr 16, 2021

This LGTM. However, is there a smaller change we can make to just block the new functionality enabled by the PR rather than reverting the entire thing?

If not, that's fine.

@SingleAccretion
Copy link
Contributor Author

I think so, yes, there is a small change that I could make here (block folding for all UB casts, not just checked ones, in gtFoldExprConst, and the same in VN), but it does require me to make sure all the failing tests pass, and I am afraid I will not be able to make sure that is the case in time.

Note on the formatting failure:

ERROR: failed to download clang-tidy.

this is the second time I am seeing this...

Copy link
Contributor

@briansull briansull left a comment

Choose a reason for hiding this comment

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

I approve this revert

@briansull
Copy link
Contributor

The jit-format on windows x64 failed due to an infrastructure glitch:

2021-04-16T16:55:44.9456599Z Invoke-WebRequest : Unable to read data from the transport connection: An existing connection was forcibly closed by 
2021-04-16T16:55:44.9463469Z the remote host.
2021-04-16T16:55:44.9464753Z At line:1 char:1
2021-04-16T16:55:44.9466317Z + Invoke-WebRequest -Uri https://clrjit.blob.core.windows.net/clang-too ...
2021-04-16T16:55:44.9468097Z + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-04-16T16:55:44.9470134Z     + CategoryInfo          : NotSpecified: (:) [Invoke-WebRequest], IOException
2021-04-16T16:55:44.9471879Z     + FullyQualifiedErrorId : System.IO.IOException,Microsoft.PowerShell.Commands.InvokeWebRequestCommand
2021-04-16T16:55:44.9473964Z  
2021-04-16T16:55:44.9810889Z ERROR: failed to download clang-tidy.

@briansull
Copy link
Contributor

I asked that the Formatting x64 task be re-run, and it acknowledges it but I didn't see it start the re-run.

@briansull
Copy link
Contributor

briansull commented Apr 16, 2021

The Linux release failures issue this message:

2021-04-16T18:34:59.7566163Z XUnit : error : Tests failed: /root/runtime/artifacts/TestResults/Release/Microsoft.NET.HostModel.AppHost.Tests_net6.0_x64.html [net6.0|x64] [/root/runtime/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/Microsoft.NET.HostModel.AppHost.Tests.csproj]

This is a known failure:" #51372

@briansull briansull merged commit 9d7ee19 into dotnet:main Apr 16, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 16, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@SingleAccretion SingleAccretion deleted the Revert-50450 branch June 8, 2021 04:11
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
4 participants