-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
initial @expect() implementation #3364
Conversation
87ae657
to
327599c
Compare
@andrewrk Frendly ping. :) |
Provide optimization hint to the compiler. Can be used on {#link|integers|Integers#}, {#syntax#}bools{#endsyntax#}, and {#link|enums|enum#}. | ||
</p> | ||
<p> | ||
Tell the compiler that {#syntax#}value{#endsyntax#} is very likely to be {#syntax#}expected{#endsyntax#}. The compiler may ignore this. This may override the branch predictor rather than just provide its default state, and can result in reduced performance (PowerPC disabled branch prediction override by default due to misuse of such hints). |
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.
(PowerPC disabled branch prediction override by default due to misuse of such hints).
It's a good warning, but please cite source or remove assertion.
if (const_val->special == ConstValSpecialUndef) { | ||
return (uint32_t)0xc0ffee00; | ||
} |
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.
This looks like it might be a problem. Why was this necessary?
uint32_t int_size; | ||
ZigType *type = instruction->base.value.type; | ||
|
||
switch (type->id) { |
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.
This function should work on all types. If LLVM does not support lowering it, that's fine, just ignore it and return the value undisturbed.
case ZigTypeIdPointer: | ||
case ZigTypeIdOptional: | ||
case ZigTypeIdErrorUnion: | ||
zig_panic("TODO. more types for @expect()"); |
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.
@expect
should work on all types. Instead of panicking here, just return value
.
if (type_is_invalid(expect_instr->value.type)) | ||
return ira->codegen->invalid_instruction; | ||
|
||
switch (value_type->id) { |
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.
I don't think this switch is needed at all. Implicit cast the expected value to the correct type, resolve it as comptime, and then either return value
(in the case value
is comptime known) or return an expect instruction.
Implementation is not quite right, and it's a low-priority feature (compared to other features in this release cycle). Closing until a more mergeable implementation is available. |
Closes: #489