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.

Sign up for GitHub

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

Jump to bottom

[CP] Disable graph intrinsics for implicit getters with unboxed record return values #52467

Closed
alexmarkov opened this issue May 22, 2023 · 3 comments
Closed

[CP] Disable graph intrinsics for implicit getters with unboxed record return values #52467

alexmarkov opened this issue May 22, 2023 · 3 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable

Comments

@alexmarkov
Copy link
Contributor

alexmarkov commented May 22, 2023 •
edited
Loading

Commit(s) to merge

86c1825

Target

stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/304760

Issue Description

AOT compiler crashed when generating an intrinsic for implicit getter returning an unboxed record. The implicit getter is needed only if it is polymorphic and not inlined.

What is the fix

Generation of intrinsics for implicit getters are disabled when the return value is an unboxed record.

Why cherry-pick

There are no easy workarounds for users.

Risk

low

Issue link(s)

#52449

Extra Info

No response
@alexmarkov alexmarkov added the cherry-pick-review Issue that need cherry pick triage to approve label May 22, 2023
@alexmarkov
Copy link
Contributor Author

@mraleph @a-siva

@mraleph
Copy link
Member

mraleph commented May 22, 2023

CP looks good: the fix is trivial. I support.

@a-siva
Copy link
Contributor

a-siva commented May 23, 2023

lgtm

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request merge-to-stable labels May 23, 2023
copybara-service bot pushed a commit that referenced this issue May 24, 2023
@alexmarkov
[stable] [vm/aot] Disable graph intrinsics for implicit getters with …
4e7afbb 
…unboxed record return values

Unboxing of records involves a more complex code, and such intrinsics
would be similar to the normal code of the implicit getters.

TEST=runtime/tests/vm/dart/regress_52449_test.dart

Bug: #52449
Change-Id: Ic6b4ca21609921f0215275be6578f6bbc1fe3eeb
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/304282
Fixes: #52467
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304760
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
@itsjustkevin itsjustkevin added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@whesse whesse

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mraleph @vsmenon @whesse @a-siva @itsjustkevin @mit-mit @athomas @alexmarkov