Skip to content

Crossgen2-related CoreCLR test infra fixes - #49867

Merged
trylek merged 1 commit into
dotnet:mainfrom
trylek:CrossgenTestFixes
Mar 19, 2021
Merged

trylek merged 1 commit into
dotnet:mainfrom
trylek:CrossgenTestFixes

Conversation

@trylek

@trylek trylek commented Mar 19, 2021

Copy link
Copy Markdown
Member

(*) When compiling tests with Crossgen2, use optimizations by default
like Crossgen1 does.

(*) Don't use large version bubble when Crossgen2-compiling
the framework. While at some point this had value as preparatory
testing for composite mode, it's not a shipping scenario right now
and it was hiding the codegen bug causing the issue

#49826

in System.Text.Json.

Thanks

Tomas

/cc @dotnet/crossgen-contrib

(*) When compiling tests with Crossgen2, use optimizations by default
like Crossgen1 does.

(*) Don't use large version bubble when Crossgen2-compiling
the framework. While at some point this had value as preparatory
testing for composite mode, it's not a shipping scenario right now
and it was hiding the codegen bug causing the issue

dotnet#49826

in System.Text.Json.

Thanks

Tomas
@trylek trylek added this to the 6.0.0 milestone Mar 19, 2021
@trylek
trylek requested a review from davidwrighton March 19, 2021 13:34

@davidwrighton davidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like to see us respect the debuggable attribute of the assembly instead of using the -O parameter, but this is good for now.

@mangod9

mangod9 commented Mar 19, 2021

Copy link
Copy Markdown
Member

If optimizations would have been enabled would it have caught the json issue?

@trylek

trylek commented Mar 19, 2021

Copy link
Copy Markdown
Member Author

The optimizations are mostly a matter of better matching "end customer" scenario (the SDK also uses -O when invoking Crossgen2). This bug requires optimizations to be turned on in framework compilation and that happens in both CoreCLR testing and in installer build. The problematic bit that was hiding the bug was the "--large-bubble" specifier (specific to CoreCLR testing, the installer build doesn't have that problem, that's why I was able to repro the bug with a downloaded runtime but not with my locally built CORE_ROOT); that made us compile the entire framework as one version bubble (albeit file by file) and that enabled more aggressive inlining that removed the problematic calls emitting the invalid signatures.

@trylek
trylek merged commit c505249 into dotnet:main Mar 19, 2021
@trylek
trylek deleted the CrossgenTestFixes branch March 19, 2021 18:31
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants