-
Notifications
You must be signed in to change notification settings - Fork 841
Compat fix for 6258 and fix deterministic flag #6295
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
Conversation
… (64-bit) the default to launch in VS dotnet#6223
|
@dsyme, longer term we will want to shift to the coreclr as the default compiler, and interactive, and keep the desktop around for legacy projects. The coreclr is mostly going to be 64 bit, the 32 bit compiler is often not used or even installed. |
Yes understood |
|
|
|
It's weird, I can't repro this failure in the This shows identical binaries are being produced |
|
@brettfo @KevinRansom Have you ever seen these tests fail? |
|
@dsyme … no. |
|
I think it's that
Surprisingly the output binaries seem to differ slightly when the compiler runs as x86 v. x64. @KevinRansom @brettfo any idea why this may be the case? A vague guess is that this is because the PDB generation library writes slightly different entries |
|
If it's creating desktop pdbs then who knows. If it's portable pdbs, then they should be consistent. |
|
OK, well, the tests pass now, but there must be some small difference. I'll see if I can figure it out |
|
@KevinRansom There was a bug in I will add the fix here |
|
@dsyme , good eyes. |
2 hours looking at hexdumps of binaries.... |
|
@KevinRansom @Pilchie We should consider servicing this in an RC service. I'll queue up a bug. |
|
BTW we could make the desktop fsc.exe and fsi.exe "anycpu, 32-bit preferred". They are currently x86. I don't really see any technical downside to doing that - it means that if they happen to be launched as 64-bit processes then they will launch and run correctly. |
* Make .NET Desktop fsi.exe 32-bit again and make Desktop fsiAnyCpu.exe (64-bit) the default to launch in VS #6223 * make fsc.exe 32-bit for compat * make fsc.exe 32-bit for compat * improve diagnostics * fix tests * fix determinism
|
I don't see a bug yet. |
|
I'll write one up - one-handed 🤕 |
This makes desktop fsc.exe 32-bit again, see #6258. It includes #6223 which is not yet in master.
See #6258 (comment) for the rationale as to why.
The .NET Core fsc.exe remains 64-bit.
(Separately it would be good to know the perf implications either way, mainly out of curiosity as I don't think we'll change either choice. I guess if 64-bit is faster we could add a fscAnyCpu to the build and use it as the default desktop compiler going forward, leaving the other for stack-size/type-provider compat scenarios?)