Stream: git-wasmtime

Topic: wasmtime / PR #3314 Implement bit operations for Cranelif...

Wasmtime GitHub notifications bot (Sep 08 2021 at 10:08):

dheaton-arm opened PR #3314 from implement-bitops to main:

Implemented for the Cranelift interpreter:

• Bitrev to reverse the order of the bits in an integer.

• Cls to count the leading bits which are the same as the sign bit in
  an integer, yielding one less than the size of the integer for 0 and -1.

• Clz to count the number of leading zeros in the bitwise representation of the
  integer.

• Ctz to count the number of trailing zeros in the bitwise representation of the
  integer.

• Popcnt to count the number of ones in the bitwise representation of the
  integer.

Copyright (c) 2021, Arm Limited

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:02):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:02):

afonso360 created PR review comment:

                let lanes = extractlanes(&arg(0)?, ctrl_ty.lane_type())?
                    .into_iter()
                    .map(|lane| lane.count_ones()?)
                    .collect::<ValueResult<SimdVec<V>>>()?;

                vectorizelanes(&new_vec, ctrl_ty)?

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:02):

afonso360 created PR review comment:

Would it make sense for us to instead do a .convert(ValueConversionKind::Exact(ctrl_ty)), instead of casting to and from int?

        Opcode::Clz => assign(
            arg(0)?
                .leading_zeroes()?
                .convert(ValueConversionKind::Exact(ctrl_ty))?
         )?,

I think Exact would be able to both extend and reduce types? Although I'm not sure about converting from unsigned to signed types.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:02):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:03):

afonso360 edited PR review comment.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:05):

afonso360 edited PR review comment.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:07):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:07):

afonso360 created PR review comment:

Or we could cast it to the original type in the macro that leading_zeroes uses, so that it would always return the original type? I think that would also solve a lot of issues and make the rest of the interface cleaner.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:09):

afonso360 edited PR review comment.

Wasmtime GitHub notifications bot (Sep 08 2021 at 11:32):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 08 2021 at 17:47):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 08 2021 at 17:47):

cfallin created PR review comment:

+1, iterator methods are clearer (to me at least!) and more idiomatic than the imperative loop style.

Wasmtime GitHub notifications bot (Sep 10 2021 at 08:38):

dheaton-arm updated PR #3314 from implement-bitops to main.

Wasmtime GitHub notifications bot (Sep 10 2021 at 11:23):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 10 2021 at 11:23):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 10 2021 at 11:23):

afonso360 created PR review comment:

A really nice thing you can do with iterators is collecting from an iterator of Result<T, _> into Result<Vec<T>, _>. This fails on the first Result that isn't Ok, which is exactly what we want! I think the rust book has a better explanation if you want to look more into it.

                    .map(|lane| lane.count_ones())
                    .collect::<ValueResult<SimdVec<V>>>()?;

Wasmtime GitHub notifications bot (Sep 13 2021 at 15:29):

dheaton-arm updated PR #3314 from implement-bitops to main.

Wasmtime GitHub notifications bot (Sep 13 2021 at 16:29):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 13 2021 at 16:29):

cfallin merged PR #3314.

Last updated: Dec 23 2024 at 12:05 UTC