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

[release/8.0-staging] [Mono AOT] Fix error when returning zero sized struct #107198

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 30, 2024

Backport of #106013 to release/8.0-staging

/cc @jkurdek

Customer Impact

  • Customer reported
  • Found internally

Customer reported #103628

The scenario is to use an Auto or Sequential layout struct with no fields as a return value of a method. Mono AOT compiler will fail in that case (the compiler itself will fail). The only workaround is to change the struct to not be auto/sequential.

This caused different compilation failures on both ArgVTypeAsScalar and ArgVTypeInReg codepaths. Fix for ArgVTypeAsScalar path involved rounding up a struct to one byte when empty. This matches a behaviour of not specifying a layout attribute on the struct. (#103628) The ArgVTypeInReg produces malformed LLVM-IR. The fix was to align types. (#106259)

Regression

  • Yes
  • No

Testing

New CI regression test.

Risk

Low. Any potential issues which this fix could introduce would be for scenarios where the compiler failed previously.

@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 30, 2024
Copy link
Contributor

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

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 8.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Sep 3, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 3, 2024
@rbhanda rbhanda added this to the 8.0.10 milestone Sep 3, 2024
@carlossanlop
Copy link
Member

Friendly reminder that Code Complete for the October Release is September 9. If we want this fix to be included in that release, please merge this PR before that date.

@jkurdek
Copy link
Member

jkurdek commented Sep 8, 2024

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkurdek
Copy link
Member

jkurdek commented Sep 9, 2024

/ba-g the runtime failures seem to be coming from browser-wasm linux Release WasmBuildTests #106663 . The runtime-extra-platforms fails are unrelated.

@jkurdek jkurdek merged commit 423fd78 into release/8.0-staging Sep 9, 2024
193 of 219 checks passed
@jkurdek jkurdek deleted the backport/pr-106013-to-release/8.0-staging branch September 9, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Codegen-AOT-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants