-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Allow more operators on bool vectors #24131
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
Allow more operators on bool vectors #24131
Conversation
@@ -806,7 +806,7 @@ fn expr(gz: *GenZir, scope: *Scope, ri: ResultInfo, node: Ast.Node.Index) InnerE | |||
.bool_and => return boolBinOp(gz, scope, ri, node, .bool_br_and), | |||
.bool_or => return boolBinOp(gz, scope, ri, node, .bool_br_or), | |||
|
|||
.bool_not => return simpleUnOp(gz, scope, ri, node, coerced_bool_ri, tree.nodeData(node).node, .bool_not), | |||
.bool_not => return simpleUnOp(gz, scope, ri, node, .{ .rl = .none }, tree.nodeData(node).node, .bool_not), |
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 could make it work without this update, but from my understanding we should not use coerced_bool_ri
here? Please correct me if I am wrong, and I can change it back.
src/Sema.zig
Outdated
return if (val.isUndef(zcu)) .undef_bool else if (val.toBool()) .bool_false else .bool_true; | ||
const operand_type_tag = operand_type.zigTypeTag(zcu); | ||
if (val.isUndef(zcu)) { | ||
return if (operand_type_tag == .bool) .undef_bool else pt.undefRef(operand_type); |
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 was unsure if I should keep .undef_bool
here, or just always return pt.undefRef(operand_type)
?
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.
Overall this looks quite close to ready.
For the vector case of zirBoolNot
, you should be able to share the implementation with the vector case of zirBinNot
.
cc39452
to
8551b7f
Compare
I have cleaned up the PR as suggested now, but I am not sure if you wanted to keep the explicit |
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.
It's not my preferred way of writing it - rather than for example:
zirBitNot(operation_is_bool) -> if (operation_is_bool) x() else y()
I like instead:
zirBitNot() -> ... if (type_is_bool) x() else y()
zirBoolNot() -> ... x()
Because the latter means less branching, which I find easier to read, and optimizes better in general. That also answers your question about the explicit undef_bool
, bool_false
, and bool_true
- optimizations which are naturally written with this style.
However, this is perfectly adequate. Thanks!
decided that it bothered me after all so I opened #24162 |
Allow binary not, binary and, binary or, binary xor, and boolean not operators on vectors of bool and add corresponding tests.
Also update the lang ref to clarify use of operators on vectors.
This closes #24093.