-
Notifications
You must be signed in to change notification settings - Fork 5.4k
IR: Taking function arg address and representing references as pointers #6967
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
…to vaivaswatha/ir_consts_1
…to vaivaswatha/ir_consts_2
…to vaivaswatha/ir_consts_2
This is the third PR to address #6351. It allows safely taking addresses of function arguments.
9bb2b0c
to
e65a8da
Compare
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.
Great job @vaivaswatha! 💪 🚀
To my comments, those regarding replacing asm
code generated for slices, can IMO be addressed in a separate PR.
Looking forward to get this PR merged and to continue the work on references!
Thank you @IGI-111 . This was a tough one !
Yes, I agree with you about the comments, and that it's better done as a subsequent PR. Those ASM blocks were sources of multiple bugs that took me quite a while to get right, each time with me wondering why we have it like that.
Indeed ! I'll work on the other comments that you have and update the PR soon. From a first look, I agree with them all. Thank you. |
Co-authored-by: Igor Rončević <[email protected]>
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.
One more thought, in the verify_ptr_to_int
, I've enabled the check to make sure that we don't have any more semantically invalid PtrToInt
casts, where the pointer argument is actually not the pointer.
Good news - looks like it is so 😄
"Bad" news - there are failing tests, but they all fail in the same case, on casting string slices to int. But this is more a conceptual thing because string<N>
in IR is actually a pointer to the slice.
This is IMO a subject for an upcoming PR that will extend the verifyer.
Here is an example of such issues in tests, they are all essentially the same:
v6 = const string<4> "main"
v7 = ptr_to_int v6 to u64
The failing conversion will in this case be:
Internal compiler error: Verification failed: Pointer cast from non pointer string<4>.
Description
This is the third PR to address #6351. It allows safely taking addresses of function arguments, and represents references (
&
) as pointers in the IR.Note: The entire function
type_correction
(inir_generation.rs
) and this edit incompile.rs
, to handle return values, must be removed once we fix and finalize the syntax of references. Until then these two hacks need to stay. Ideally I wanted to put them both in one place (which is thetype_correction
pass), but the return type fix needs to happen, unfortunately, during IR gen.