-
Notifications
You must be signed in to change notification settings - Fork 787
Add a type annotation to return_call_ref #5068
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
The GC spec has been updated to have heap type annotations on call_ref and return_call_ref. To avoid breaking users, we will have a graceful, multi-step upgrade to the annotated version of call_ref, but since return_call_ref has no users yet, update it in a single step.
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
@@ -2014,7 +2014,13 @@ struct PrintExpressionContents | |||
} | |||
void visitCallRef(CallRef* curr) { | |||
if (curr->isReturn) { | |||
printMedium(o, "return_call_ref"); | |||
if (printUnreachableReplacement(curr->target)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return calls are always unreachable aren't they? this would always happen then, unless I'm missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're passing curr->target
, so we only print the replacement if the target is unreachable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % question
;; CHECK-NEXT: (call $tail-caller-call_ref-unreachable) | ||
;; CHECK-NEXT: (drop | ||
;; CHECK-NEXT: (call $tail-caller-call_ref-unreachable) | ||
;; CHECK-NEXT: ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why were drops added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the text parser used ValidatingBuilder::validateAndMakeCallRef
for return calls and that would produce an unreachable block expression rather than an actual call inside tail-caller-call_ref-unreachable
. Now the parser actually produces a tail call CallRef
expression there that prevents DAE from removing the return value, so that return value needs to be dropped here.
The GC spec has been updated to have heap type annotations on call_ref and
return_call_ref. To avoid breaking users, we will have a graceful, multi-step
upgrade to the annotated version of call_ref, but since return_call_ref has no
users yet, update it in a single step.