Skip to content

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

Merged
merged 2 commits into from
Jun 12, 2025

Conversation

Danielkonge
Copy link
Contributor

@Danielkonge Danielkonge commented Jun 9, 2025

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.

@@ -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),
Copy link
Contributor Author

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);
Copy link
Contributor Author

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)?

Copy link
Member

@andrewrk andrewrk left a 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.

@Danielkonge Danielkonge force-pushed the operators-on-bool-vectors branch from cc39452 to 8551b7f Compare June 9, 2025 18:57
@Danielkonge
Copy link
Contributor Author

Danielkonge commented Jun 9, 2025

I have cleaned up the PR as suggested now, but I am not sure if you wanted to keep the explicit .undef_bool, .bool_false and .bool_true in the non-vector case?

Copy link
Member

@andrewrk andrewrk left a 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!

@andrewrk andrewrk merged commit 5e3c0b7 into ziglang:master Jun 12, 2025
10 checks passed
@andrewrk andrewrk added the release notes This PR should be mentioned in the release notes. label Jun 12, 2025
@andrewrk
Copy link
Member

decided that it bothered me after all so I opened #24162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes This PR should be mentioned in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow binary not, binary and, binary or, binary xor, and boolean not operators on vectors of bool
2 participants