Closed Bug 728517 Opened 12 years ago Closed 12 years ago

IonMonkey: testStringTruthy clobbers the tag register on arm

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mjrosenb, Unassigned)

Details

Attachments

(1 file)

I'm also moving testStringTruthy from the code generator to the macro assembler so it can be with its test*Truthy brethren.

This was introduced when I accidentally misread a comment stating that the tag may be stored in the scratch register on x64 as saying the tag register is dead.
Attachment #598482 - Flags: review?(Jacob.Bramley)
Comment on attachment 598482 [details] [diff] [review]
/home/mrosenberg/patches/fixTestStringTruthy-r0.patch

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

It looks good, except that I don't think you actually removed the implementation from CodeGenerator-arm.cpp.

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +2017,5 @@
> +
> +    size_t mask = (0xFFFFFFFF << JSString::LENGTH_SHIFT);
> +    ma_dtr(IsLoad, string, Imm32(JSString::offsetOfLengthAndFlags()), ScratchRegister);
> +    // bit clear into the scratch register.
> +    ma_bic(Imm32(~mask), ScratchRegister, SetCond);

I think you've done this for optimization reasons, since ~mask is more likely to be encodable than mask. Is that correct? Otherwise, it look strange that you're writing to ScratchRegister here. Perhaps a comment would be useful, clarifying this.
Attachment #598482 - Flags: review?(Jacob.Bramley) → review+
landed: http://hg.mozilla.org/projects/ionmonkey/rev/76b94b38b92d
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.