118823 - better assembler for win32 xptcinvoke

Status: VERIFIED FIXED

(Core :: XPConnect, defect, P3)

Description

[Bernard Alleysson]

modifications: 

* make invoke_copy_to_stack fastcall instead of stdcall
* test edx,edx instead of cmp edx,0
* use call [edx][eax*4] instead of computing address by hand

results:

this saves a few push/pop for arguments and better use addressing capabilities
of x86 CPU

I've tested this and everything works fine (mail, browser)

Comment 1

[Bernard Alleysson]

Attachment: better assembler

Comment 2

[Matthias Versen [:Matti]]

marking new 

Comment 3

[David Bradley]

Did you run TestXPCTCInvoke?

Comment 4

[Bernard Alleysson]

ok, I've just run through it
I'll attach the results

I've put some breakpoints and stepped through the assembly code (tested the case
with 0 params)

Comment 5

[Bernard Alleysson]

Attachment: TestXPTCInvoke output

Comment 6

[David Bradley]

Moving out to 1.3. If this needs to be in before 1.3 please comment.

Comment 7

[David Bradley]

Everything looks good, one question I have is about the last call statement call
   [edx][eax*4]. I'm not real familiar with the call[reg][reg] syntax.
Admittedly my x86 assembler is weak. Is this form new, or has it been around a
while. When was it introduced?

Comment 8

[Bernard Alleysson]

This addressing mode is valid for Intel 80386:
http://sources.redhat.com/binutils/docs-2.12/as.info/i386-Memory.html

And the CALL instruction can take a memory reference of 32 bits or a register
reference (r/m 32). The syntax for a memory reference is "section:[base +
index*scale + disp]".

So I think that this was introduced with the Intel 80386 processor.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

so [edx][eax*4] is the same as [edx+eax*4]?

Comment 10

[Bernard Alleysson]

yes, absolutely

Comment 11

[David Bradley]

Moving to 1.4 Alpha

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is there a reason not to land this patch? It should give some perfimprovement to
a function called quite a lot.

Comment 13

[David Bradley]

Comment on attachment 64010 [details] [diff] [review]
better assembler

I meant to r= this, but forgot to. We need to get an sr=. Requesting shaver.

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 64010 [details] [diff] [review]
better assembler

My first assembler of the new year!  Looks good, thanks for the patch.

Comment 15

[David Bradley]

Patch checked in

Thanks for the quick response Shaver.

Updated milestone to 1.3beta to reflect when patch went in

Comment 16

[Phil Schwartau]

Patch check-in verified -