Skip to content

Conversation

@shawnl
Copy link
Contributor

@shawnl shawnl commented Apr 10, 2019

breaking: make @clz() @ctz() @popCount() @bSwap() @bitReverse() all take types

Closes: #2119

breaking: @bitreverse -> @bitReverse, @bswap -> @bSwap

Closes: #2120

Comes with tests

@shawnl shawnl force-pushed the builtins branch 3 times, most recently from 08b0f10 to 7986208 Compare April 10, 2019 22:43
@andrewrk andrewrk added the work in progress This pull request is not ready for review yet. label Apr 29, 2019
src/analyze.cpp Outdated
}

//Equivilent to math.Log2Int. ceil log2int
ZigType *get_shift_type(CodeGen *g, uint64_t x) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the existing functions get_smallest_unsigned_int_type and bits_needed_for_unsigned. See ir_analyze_bit_shift which has to do the same thing as @clz.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't. There are differn't. I added a comment explaining why.


{#header_open|@bswap#}
<pre>{#syntax#}@bswap(comptime T: type, value: T) T{#endsyntax#}</pre>
{#header_open|@bSwap#}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we go straight from bswap (which matches gcc builtin) to byteSwap (more verbose, zig-style name)?


if (!is_test) {
// only create these aliases when not testing
// Why don't we need these when testing?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember, to be honest. You can delete my unhelpful comment (apologies for that). I don't think changing it to a question helps anything though.

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 figured it out. These are aliases, not the canonical names...I will update this comment

src/ir.cpp Outdated
IrInstruction *result = ir_build_clz(&ira->new_irb,
clz_instruction->base.scope, clz_instruction->base.source_node, value);
result->value.type = return_type;
if (op->value.type->id == ZigTypeIdComptimeInt) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your proposal #2121 was rejected. This pull request implements #2121 as if it were accepted.

What are you expecting to accomplish by doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it does implicitly cast, but yes this check should be removed too. (ever occurrence)

@shawnl
Copy link
Contributor Author

shawnl commented May 5, 2019

1 sec, bugs

@shawnl
Copy link
Contributor Author

shawnl commented May 5, 2019

needs some more tests.

@andrewrk
Copy link
Member

still working on this?

@shawnl shawnl changed the title make @clz() @ctz() @popCount() @bSwap() @bitReverse() all take types make @clz() @ctz() @popCount() @byteSwap() @bitReverse() all take types May 15, 2019
The Zig error when trying to do things with a function pointer is not very good....
@andrewrk andrewrk removed the work in progress This pull request is not ready for review yet. label May 15, 2019
@andrewrk andrewrk dismissed their stale review May 16, 2019 16:43

changes

@andrewrk
Copy link
Member

Merged in e09c05f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants