492299 - xptcall for e500(booke)

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Hideo Saito]

I couldn't search a dependent code for e500 that is based on a PowerPC architecture but doesn't have FPU. This may be useful if there is not yet the code.

Comment 1

[Hideo Saito]

Attachment: patch

Comment 2

[Sebastian A. Siewior]

I made a similar patch before I found this one. After a review I noticed that this patch contains the same bug for double handling as current code for long longs as I reported in #520367.

I attach my patch :)

Comment 3

[Sebastian A. Siewior]

Attachment: soft float addon

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

jst/sicking: can we help Sebastian find a reviewer?

Comment 5

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

Josh thought that bsmedberg or dbaron might be able to review ppc xptcall code.

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think Benjamin's better. :-)

Which patch needs review?  (How do they differ?)

Comment 7

[Sebastian A. Siewior]

I would say the second one https://bugzilla.mozilla.org/attachment.cgi?id=404421 which obsoletes the first one.

The difference is that, the first one has the same bug for double as described in #520367 for long long. Both types are 64bit, have to be split in two 32-bit regs / stack slots and padded properly.

Comment 8

[Benjamin Smedberg]

Is this patch covered by the existing xptcall tests?

Comment 9

[Sebastian A. Siewior]

Which one would be that?
I tested it with xpcom/reflect/xptcall/tests/TestXPTCInvoke.cpp. Is there something else?

Comment 10

[Benjamin Smedberg]

Yes, that one. Does that test the functionality you've changed here?

Comment 11

[Sebastian A. Siewior]

Yes it does. I did the test on a softloat machine. I didn't test it on a machine with hardfloat but it should not change anything due to the ifdefs.
Do you want me to test it on a box with hardfloat?

Comment 12

[Sebastian A. Siewior]

I've built the files in question on a powerpc without softloat with and without the patch. The result after striping the resulting binary (to lose debug info is):

xpcom/reflect/xptcall/src/md/unix/no-patch$ sha1sum * ../patch/* | sort
32f9955e8267b4bbaf68b44ab364338d2dcb702e  ../patch/xptcinvoke_ppc_linux.o
32f9955e8267b4bbaf68b44ab364338d2dcb702e  xptcinvoke_ppc_linux.o
86b26e652731ed942fe059c4f85b25e85055f7d3  ../patch/xptcstubs_asm_ppc_linux.o
86b26e652731ed942fe059c4f85b25e85055f7d3  xptcstubs_asm_ppc_linux.o
91f581143a5c2286c0f26682d8c5365082e8ac32  ../patch/xptcstubs_ppc_linux.o
91f581143a5c2286c0f26682d8c5365082e8ac32  xptcstubs_ppc_linux.o
d8e183b0288d1f6d214eb1e38031df92339a020d  ../patch/xptcinvoke_asm_ppc_linux.o
d8e183b0288d1f6d214eb1e38031df92339a020d  xptcinvoke_asm_ppc_linux.o

So the patch does not change the behavior for the non-softfloat case.

Comment 13

[Sebastian A. Siewior]

This patch depends on the patch in #520367.

Comment 14

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/38b06fd30c16