1251140 - Baldr: Implement CtzI.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Michael Bebenita [:mbx]]

Patch coming up.

Comment 1

[Michael Bebenita [:mbx]]

Attachment: ctz.patch

Comment 2

[Michael Bebenita [:mbx]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8753756fae75

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8723397 [details] [diff] [review]
ctz.patch

Review of attachment 8723397 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

::: js/src/asmjs/WasmIonCompile.cpp
@@ +2781,5 @@
>          return EmitUnary<MTruncateToInt32>(f, ExprType::F64, def);
>        case Expr::I32Clz:
>          return EmitUnary<MClz>(f, ExprType::I32, def);
> +      case Expr::I32Ctz:
> +          return EmitUnary<MCtz>(f, ExprType::I32, def);

Nit: switch case/default labels are indented with 2 spaces and their bodies with 2 more (instead of 4)

::: js/src/jit-test/tests/wasm/basic-integer.js
@@ +17,5 @@
>  }
>  
>  testUnary('i32', 'clz', 40, 26);
> +testUnary('i32', 'ctz', 40, 3);
> +testUnary('i32', 'ctz', 0, 32);

Nit: another case that might be interesting:

testUnary('i32', 'ctz', -2147483648, 31);

::: js/src/jit/MIR.h
@@ +6028,5 @@
>      void collectRangeInfoPreTrunc() override;
>  };
>  
> +class MCtz
> +    : public MUnaryInstruction

Nit: indent with 2 spaces

Comment 4

[Michael Bebenita [:mbx]]

Attachment: ctz.patch

Fixed nits.

Comment 5

[Michael Bebenita [:mbx]]

Attachment: ctz.patch

Rebased patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=23ca49c14962

Comment 6

[Michael Bebenita [:mbx]]

Attachment: ctz.patch

Fixed arm64 build.

Comment 7

[Michael Bebenita [:mbx]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c07e690a6233

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1766ac255d5

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b1766ac255d5