1248396 - IonMonkey: MIPS64: branchTest32 have to neuter high bits.

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Tooru Fujisawa [:arai]]

Separated from bug 1245112 comment #40

(In reply to Heiher [:hev] from bug 1245112 comment #40)
> (In reply to Nicolas B. Pierron [:nbp] from bug 1245112 comment #36)
> > (In reply to Heiher [:hev] from bug 1245112 comment #35)
> > > (In reply to Nicolas B. Pierron [:nbp] from bug 1245112 comment #30)
> > > > Comment on attachment 8718236 [details] [diff] [review]
> > > > Part 6: Move MacroAssembler::branchTest32 into generic macro assembler.
> > > > 
> > > > Review of attachment 8718236 [details] [diff] [review]:
> > > > -----------------------------------------------------------------
> > > >
> > > > ::: js/src/jit/mips-shared/MacroAssembler-mips-shared-inl.h
> > > > @@ +350,5 @@
> > > > > +MacroAssembler::branchTest32(Condition cond, Register lhs, Register rhs, L label)
> > > > > +{
> > > > > +    MOZ_ASSERT(cond == Zero || cond == NonZero || cond == Signed || cond == NotSigned);
> > > > > +    if (lhs == rhs) {
> > > > > +        ma_b(lhs, rhs, label, cond);
> > > > 
> > > > This sounds busted, as this is the same implementation for branchTest64 on
> > > > mips64.
> > > > 
> > > > => ni? Heiher
> > > 
> > > I think yes, The 32-bit integers are always sign-extend into 64-bit register
> > > on mips64.
> > 
> > So that means that if we are trying to compare the low bits of a 64bits
> > value, with a 32bits immediate, then we might have an issue where the test
> > will not work as expected?
> > 
> > Should we neuter the high bits of the other register then?
> 
> Yes. If one operand of branchTest32 is a real 64bits value, we need neuter
> the high bits of this value.