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

[Ppc64le] bug fixes #74131

Merged
merged 8 commits into from
Aug 19, 2022

Conversation

alhad-deshpande
Copy link
Contributor

@alhad-deshpande alhad-deshpande commented Aug 18, 2022

This PR fixes below things:

  1. g_assert mentioned in github issue 71080
  2. Test case failures occurring on CICD pipeling due to incorrect instruction length of tailcall.
  3. Corrections in instruction lengths for all opcodes in which new memory patch thunking has been impemented.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 18, 2022
@carlossanlop
Copy link
Member

@alhad-deshpande RC1 changes need a fix in main first. Was this done already? If not, please close this PR, send a PR to main first, then you can backport it with the help of the bot (ping me and I can help you).

@carlossanlop carlossanlop added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) needs-author-action An issue or pull request that requires more info or actions from the author. and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) needs-author-action An issue or pull request that requires more info or actions from the author. labels Aug 18, 2022
@carlossanlop
Copy link
Member

carlossanlop commented Aug 18, 2022

Oh I see the main PR got merged. But this backport did not get automatically created by the maestro bot.

@akoeplinger @lewing can you please manually review this PR, since it's not a direct backport?

@carlossanlop
Copy link
Member

There were failures in runtime-community. Please help confirm if they are related to this change.

@akoeplinger
Copy link
Member

The failures are unrelated.

@carlossanlop
Copy link
Member

Thanks for the sign-off @akoeplinger. Now I just need approval from @lewing.

I am pausing merges right now, in favor of getting the 7-rc1->7 PR merged without any more changes (the bot autocommits anything merged to rc1, and restarts the CI).

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

Lgtm

@carlossanlop
Copy link
Member

The 7.0-rc1 -> 7.0 PR was merged, so now we're unblocked to merge this.
Approved and signed-off.
CI failures are unrelated.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit f908dbc into dotnet:release/7.0-rc1 Aug 19, 2022
@alhad-deshpande
Copy link
Contributor Author

alhad-deshpande commented Aug 19, 2022

@carlossanlop @lewing @akoeplinger
Thanks for merging this PR into RC1 branch. But I dont see similar PR 74058 raised against main yet merged. So I think RC1 has these changes which are not yet in main.

The PR 73616 was for memory thunking and those changes are already there in RC1 branch.

Can you please merge PR 74058 into main so that RC1 branch is not ahead of main?

@akoeplinger
Copy link
Member

@alhad-deshpande I merged #74058, thanks!

@alhad-deshpande
Copy link
Contributor Author

@rzikm @akoeplinger
Did not understood why issue PR 74484 got tagged. Is this got tagged mistakenly?

@akoeplinger
Copy link
Member

@alhad-deshpande yeah it was inadvertently mentioned in the PR description of that PR, no worries.

@alhad-deshpande
Copy link
Contributor Author

@akoeplinger
Thanks for the confirmation.

@rzikm
Copy link
Member

rzikm commented Aug 25, 2022

@rzikm @akoeplinger Did not understood why issue PR 74484 got tagged. Is this got tagged mistakenly?

Yes, I mistakenly created the PR against different base and it tagged lots of people for review and lots of PRs 🤦

@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants