Skip to content

Stage2 misc fixes #11716

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

Merged
merged 4 commits into from
May 26, 2022
Merged

Stage2 misc fixes #11716

merged 4 commits into from
May 26, 2022

Conversation

Vexu
Copy link
Member

@Vexu Vexu commented May 25, 2022

I'm not 100% sure about the second commit but it does make sense based on my understanding of the current intended semantics of packed structs. And for the generic poison check, the second best alternative I could come up with was to add the check to resolveInst which would have been a much more intrusive change.

Closes #11709
Closes #11680
Closes #11550
Closes #11504

@andrewrk
Copy link
Member

andrewrk commented May 25, 2022

the second best alternative I could come up with was to add the check to resolveInst which would have been a much more intrusive change.

Hm, why would that be more intrusive? There is typically only a handful of calls to resolveInst at the top of each ZIR instruction handler, and only one per operand. On the other hand, typeOf is needed in utility functions that operate on Air.Inst.Ref parameters, and often typeOf is called multiple times per Air.Inst.Ref. Furthermore, most of the backends already have a fallible resolveInst including the LLVM backend and the x86_64 backend.

@Vexu
Copy link
Member Author

Vexu commented May 25, 2022

Hm, why would that be more intrusive? There is typically only a handful of calls to resolveInst at the top of each ZIR instruction handler, and only one per operand. On the other hand, typeOf is needed in utility functions that operate on Air.Inst.Ref parameters, and often typeOf is called multiple times per Air.Inst.Ref. Furthermore, most of the backends already have a fallible resolveInst including the LLVM backend and the x86_64 backend.

Hmm, I might have estimated wrong, I'll give it a try.

@andrewrk andrewrk merged commit 1dd7109 into ziglang:master May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants