[Webkit-unassigned] [Bug 112886] [SH4] LLInt sh4 backend implementation

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from Julien Brianceau <jbrianceau at nds.com>  2013-03-31 13:18:31 PST ---
(In reply to comment #23)
> 
> Let me try to distill my issues:
> 
> 1) I don't like how sh4LowerMalformedAddressesDouble is named.  It's not lowering malformed addresses on double accesses.  Instead, it's lowering the access into a form where the address is available in a temporary so that you can emit an instruction that clobbers it.  I would call this phase sh4LowerDoubleAccesses, instead.
Ok

> 2) I'm not sure I like the naming of loadd+ and stored-.  Now that I understand their semantics, I think you're doing it right.  It is acceptable to give instructions long and verbose names, especially when their semantics are so weird.  I would call them loaddReversedAndIncrementAddress and storedReversedAndDecrementAddress.
Ok too

> 3) Instead of emitting a addi in the stored case, you could have just created a new address with a different offset.  I think that would be cleaner.  In fact, this could be super useful in general so I would do it as follows: add a method called withOffset(integerValue) to Address, BaseIndex, and AbsoluteAddress.  Each of these will return a new version of themselves with the integerValue added to their preexisting offset.  Make sure that these methods don't clobber the receiver; offlineasm heavily relies on AST node instances being mostly immutable.
I see what you want, but as I'm not a ruby expert, can you tell me if this is right for Address class for instance ? (or does it need a "yield" or another ruby keyword I don't know)

    def withOffset(extraOffset)
        Address.new(codeOrigin, @base, @offset + extraOffset)
    end

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