[Webkit-unassigned] [Bug 24986] ARM JIT port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24986





------- Comment #64 from barraclough at apple.com  2009-07-01 17:50 PDT -------
Hi Zoltan, hi Gabor,

Sorry for the delay in getting these reviewed, and yes, things are as busy as
ever!  You'll see I've reviewed the patches, both are r-s, but they're both
99.9% of the way there now, just a few very small issues to clear up.

> I know you are busy since you usually do the coding and reviewing in
> JavaScriptCore alone, so if we can help you in any way just ask us.

I should probably make sure that the rest of the team get the credit due to
them – you may well have seen me committing a bit more to the JIT in ToT over
the last few weeks, but the other guys are still very busy working on the
nitro-extreme branch, and fixing bugs across JSC.

> We also posted the yarr patch, but we feel you may find it too big.
> Unfortunately, we have no idea how to break it into smaller, working parts.

No, you're absolutely right here, this wasn't too big at all.  While clearly
not a small patch, I don't think you could have sensibly broken this down any
further, this patch was absolutely fine as it was.

> Our next step is property caching. Because of the constant pool we had issues
> with fixed instruction offsets. Since the pool can be flushed after any
> instruction, the instruction offsets are not known in advance. We had
> implemented a check before, ...
>
> We can put ensureSpaceFor(getByIdFastCase) calls into the compiler.

Yes, totally!  I was going to suggest something like this too, to make the code
a little less fragile.

I'd suggest you may want to consider having a pre & post method, say,
beginUninterruptedCodeSequence(getByIdFastCaseCodeSize) ,
endUninterruptedCodeSequence(getByIdFastCaseCodeSize), (hmmm, not sure these
are good function names - not meaning to not use your function name here, just
that 'ensureSpaceFor' doesn't read right with pre- & post- prefix added).

This way, on #ifndef NDEBUG builds you could record (to a member variable on
the JIT) a label() of where the block of code begins, and in the 'post' method
you could ASSERT that the difference between a the label() recorded in the
'pre' method, and a label() here, is equal to the argument passed in - just to
help catch cases where someone changes the code, but does not update the
constant (and in non-debug builds the 'post' method would do nothing).

Just a thought, in case it is helpful.

cheers,
G.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.