815249 - IonMonkey: add masm.printf for debugging

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

I find it handy to be able to print data during IM execution. Therefore I've added the function masm.printf that will call printf during executing. It is only for debugging, but I've also used it on --disable-debug builds. Therefore I put no #ifdef DEBUG around it.

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: add printf

Comment 2

[Hannes Verschore [:h4writer]]

Attachment: add printf

Has a printf version that makes it possible to print information from registers

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 685826 [details] [diff] [review]
add printf

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

Nice idea, seems useful. r=me with comments below addressed.

Can you make sure jit-tests pass if you add masm.printf(..) calls to some common LIR instructions?

::: js/src/ion/IonMacroAssembler.cpp
@@ +626,5 @@
>          handleException();
>      }
>  }
>  
> +void printf0_(char *test) {

Nit: "s/char */const char *" here and everywhere below.

@@ +638,5 @@
> +            ~Registers::CallMask);
> +    JS_ASSERT(!regs.empty());
> +    Register temp = regs.getAny();
> +    push(temp);
> +    push(ReturnReg);

We have to save all volatile registers since the printf call may overwrite them:

RegisterSet regs = RegisterSet::Volatile();
PushRegsInMask(regs);

Register temp = regs.getAny();

...CALL...

PopRegsInMask(regs);

@@ +659,5 @@
> +{
> +    GeneralRegisterSet regs(Registers::TempMask & ~Registers::JSCallMask &
> +            ~Registers::CallMask);
> +    if (regs.has(value))
> +        regs.take(value);

Same here. We have to make sure though that if we take registers from the set, the PushRegsInMask and PopRegsInMask still push/pop the same registers, so I'd just do something like:

RegisterSet regs = RegisterSet::Volatile();
PushRegsInMask(regs);

if (regs.has(value))
    regs.take(value);

Register temp = regs.getAny();

...CALL...

PopRegsInMask(RegisterSet::Volatile());

@@ +661,5 @@
> +            ~Registers::CallMask);
> +    if (regs.has(value))
> +        regs.take(value);
> +
> +    JS_ASSERT(!regs.empty());

Nit: don't need this assert, getAny also asserts !empty.

Comment 4

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/72c8d4dc1032

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/72c8d4dc1032