Bugzilla – Bug 316467
[PPC PATCH] OP_ARGLIST unsupported
Last modified: 2007-09-15 21:24:46 UTC
---- Reported by grompf@sublimeintervention.com 2004-11-15 23:59:01 MST ---- The attached patch adds __arglist support to the PPC jit. -kangaroo ---- Additional Comments From grompf@sublimeintervention.com 2004-11-15 23:59:36 MST ---- Created an attachment (id=167057) proposed patch ---- Additional Comments From grompf@sublimeintervention.com 2004-11-16 02:02:09 MST ---- Please hold off reviewing this patch; upon further review it only 1/2 fixes the problem; it doesn't account for locals moving the stack size around. -kangaroo ---- Additional Comments From grompf@sublimeintervention.com 2004-11-16 12:22:44 MST ---- Created an attachment (id=167058) updated patch ---- Additional Comments From grompf@sublimeintervention.com 2004-11-16 12:24:10 MST ---- This latest patch handles static and inst methods properly; but fails on methods returning a struct; I'll continue to work on that time permitting. -kangaroo ---- Additional Comments From grompf@sublimeintervention.com 2004-11-16 16:22:48 MST ---- Created an attachment (id=167059) final patch ---- Additional Comments From grompf@sublimeintervention.com 2004-11-16 16:23:16 MST ---- This final patch now supports struct returning as well; and passes mcs/tests/test-269 properly -kangaroo ---- Additional Comments From lupus@ximian.com 2004-11-23 12:29:05 MST ---- The patch doesn't look completely correct. If the slot for the cookie is allocated with add_general(), why do you increase its offset for instance methods and methods that return structs? Also, you can't just use cinfo->sig_cookie.offset, since on Linux the coockie may not get assigned its stack offset (on Linux there is no room in the param area for arguments passed in registers). You could move the gr = PPC_LAST_ARG_REG + 1; code before add_general() to quickly fix this though with a small perf hit. You should also not change cpu-g4.md: the instruction can be 12 bytes long once you take into account the fact that the offset (cfg->sig_cookie + cfg->stack_usage) doesn't fit in the immediate field of the instruction: then you need to add the check and optionally build the offset in a register and add that. Last, in OP_ARGLIST you can't use ppc_sp, since that may not always be the frame register: use cfg->frame_reg. Thanks. ---- Additional Comments From grompf@sublimeintervention.com 2004-11-24 15:52:44 MST ---- Created an attachment (id=167060) updated patch ---- Additional Comments From grompf@sublimeintervention.com 2004-11-24 15:53:31 MST ---- lupus, I believe this patch is working; however it wont currently work on lin/ppc because the MonoTypedRef is a struct passed on the stack which as we talked about earlier isn't currently working on the sysv abi. -kangaroo ---- Additional Comments From grompf@sublimeintervention.com 2004-11-24 16:45:35 MST ---- lupus, btw; I also tested: result = AddAThirdBunchOfInts (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, __arglist ( 2, 3, 4 )); with this new patch (which was failing before); this is now working. -kangaroo ---- Additional Comments From grompf@sublimeintervention.com 2004-12-02 17:24:08 MST ---- Created an attachment (id=167061) updated patch ---- Additional Comments From grompf@sublimeintervention.com 2004-12-02 17:24:37 MST ---- Patch updated again to get rid of some bit-rot in the earlier one (it conflicted with some patches that have been commited since) -kangaroo ---- Additional Comments From lupus@ximian.com 2004-12-07 05:32:27 MST ---- Thanks for the patch, committed. Could you please also commit your extended tests? Thanks! Imported an attachment (id=167057) Imported an attachment (id=167058) Imported an attachment (id=167059) Imported an attachment (id=167060) Imported an attachment (id=167061) Unknown bug field "cf_op_sys_details" encountered while moving bug <cf_op_sys_details>OSX 10.3.4</cf_op_sys_details> Unknown operating system unknown. Setting to default OS "Other".