Author: Armin Rigo <ar...@tunes.org> Branch: Changeset: r77329:749bf9a13d9c Date: 2015-05-15 10:58 +0200 http://bitbucket.org/pypy/pypy/changeset/749bf9a13d9c/
Log: Kill 'exchange_result_libffi' and tweak jit_libffi. This might actually fix the issue of test_pypy_c/test_ffi failing since a random point in time, which I believe is related to a possible problem that this checkin fixes. diff --git a/pypy/module/_cffi_backend/ccallback.py b/pypy/module/_cffi_backend/ccallback.py --- a/pypy/module/_cffi_backend/ccallback.py +++ b/pypy/module/_cffi_backend/ccallback.py @@ -3,14 +3,14 @@ """ import os -from rpython.rlib import clibffi, rweakref, jit +from rpython.rlib import clibffi, rweakref, jit, jit_libffi from rpython.rlib.objectmodel import compute_unique_id, keepalive_until_here from rpython.rtyper.lltypesystem import lltype, rffi from pypy.interpreter.error import OperationError, oefmt from pypy.module._cffi_backend import cerrno, misc from pypy.module._cffi_backend.cdataobj import W_CData -from pypy.module._cffi_backend.ctypefunc import SIZE_OF_FFI_ARG, BIG_ENDIAN, W_CTypeFunc +from pypy.module._cffi_backend.ctypefunc import SIZE_OF_FFI_ARG, W_CTypeFunc from pypy.module._cffi_backend.ctypeprim import W_CTypePrimitiveSigned from pypy.module._cffi_backend.ctypevoid import W_CTypeVoid @@ -147,7 +147,7 @@ # zero extension: fill the '*result' with zeros, and (on big- # endian machines) correct the 'result' pointer to write to misc._raw_memclear(ll_res, SIZE_OF_FFI_ARG) - if BIG_ENDIAN: + if jit_libffi.BIG_ENDIAN: diff = SIZE_OF_FFI_ARG - fresult.size ll_res = rffi.ptradd(ll_res, diff) # diff --git a/pypy/module/_cffi_backend/ctypefunc.py b/pypy/module/_cffi_backend/ctypefunc.py --- a/pypy/module/_cffi_backend/ctypefunc.py +++ b/pypy/module/_cffi_backend/ctypefunc.py @@ -188,7 +188,6 @@ # ____________________________________________________________ -BIG_ENDIAN = sys.byteorder == 'big' USE_C_LIBFFI_MSVC = getattr(clibffi, 'USE_C_LIBFFI_MSVC', False) @@ -399,16 +398,6 @@ exchange_offset = rffi.sizeof(rffi.CCHARP) * nargs exchange_offset = self.align_arg(exchange_offset) cif_descr.exchange_result = exchange_offset - cif_descr.exchange_result_libffi = exchange_offset - - if BIG_ENDIAN and self.fresult.is_primitive_integer: - # For results of precisely these types, libffi has a - # strange rule that they will be returned as a whole - # 'ffi_arg' if they are smaller. The difference - # only matters on big-endian. - if self.fresult.size < SIZE_OF_FFI_ARG: - diff = SIZE_OF_FFI_ARG - self.fresult.size - cif_descr.exchange_result += diff # then enough room for the result, rounded up to sizeof(ffi_arg) exchange_offset += max(rffi.getintfield(self.rtype, 'c_size'), diff --git a/pypy/module/cppyy/interp_cppyy.py b/pypy/module/cppyy/interp_cppyy.py --- a/pypy/module/cppyy/interp_cppyy.py +++ b/pypy/module/cppyy/interp_cppyy.py @@ -314,13 +314,6 @@ exchange_offset = rffi.sizeof(rffi.CCHARP) * nargs exchange_offset = (exchange_offset + 7) & ~7 # alignment cif_descr.exchange_result = exchange_offset - cif_descr.exchange_result_libffi = exchange_offset - - # TODO: left this out while testing (see ctypefunc.py) - # For results of precisely these types, libffi has a - # strange rule that they will be returned as a whole - # 'ffi_arg' if they are smaller. The difference - # only matters on big-endian. # then enough room for the result, rounded up to sizeof(ffi_arg) exchange_offset += max(rffi.getintfield(cif_descr.rtype, 'c_size'), diff --git a/rpython/jit/codewriter/jtransform.py b/rpython/jit/codewriter/jtransform.py --- a/rpython/jit/codewriter/jtransform.py +++ b/rpython/jit/codewriter/jtransform.py @@ -1953,11 +1953,6 @@ assert False, 'unsupported oopspec: %s' % oopspec_name return self._handle_oopspec_call(op, args, oopspecindex, extraeffect) - def rewrite_op_jit_ffi_save_result(self, op): - kind = op.args[0].value - assert kind in ('int', 'float', 'longlong', 'singlefloat') - return SpaceOperation('libffi_save_result_%s' % kind, op.args[1:], None) - def rewrite_op_jit_force_virtual(self, op): op0 = SpaceOperation('-live-', [], None) op1 = self._do_builtin_call(op) diff --git a/rpython/jit/metainterp/blackhole.py b/rpython/jit/metainterp/blackhole.py --- a/rpython/jit/metainterp/blackhole.py +++ b/rpython/jit/metainterp/blackhole.py @@ -1431,41 +1431,6 @@ def bhimpl_copyunicodecontent(cpu, src, dst, srcstart, dststart, length): cpu.bh_copyunicodecontent(src, dst, srcstart, dststart, length) - def _libffi_save_result(self, cif_description, exchange_buffer, result): - ARRAY = lltype.Ptr(rffi.CArray(lltype.typeOf(result))) - cast_int_to_ptr = self.cpu.cast_int_to_ptr - cif_description = cast_int_to_ptr(cif_description, CIF_DESCRIPTION_P) - exchange_buffer = cast_int_to_ptr(exchange_buffer, rffi.CCHARP) - # - data_out = rffi.ptradd(exchange_buffer, cif_description.exchange_result) - rffi.cast(ARRAY, data_out)[0] = result - _libffi_save_result._annspecialcase_ = 'specialize:argtype(3)' - - @arguments("self", "i", "i", "i") - def bhimpl_libffi_save_result_int(self, cif_description, - exchange_buffer, result): - self._libffi_save_result(cif_description, exchange_buffer, result) - - @arguments("self", "i", "i", "f") - def bhimpl_libffi_save_result_float(self, cif_description, - exchange_buffer, result): - result = longlong.getrealfloat(result) - self._libffi_save_result(cif_description, exchange_buffer, result) - - @arguments("self", "i", "i", "f") - def bhimpl_libffi_save_result_longlong(self, cif_description, - exchange_buffer, result): - # 32-bit only: 'result' is here a LongLong - assert longlong.is_longlong(lltype.typeOf(result)) - self._libffi_save_result(cif_description, exchange_buffer, result) - - @arguments("self", "i", "i", "i") - def bhimpl_libffi_save_result_singlefloat(self, cif_description, - exchange_buffer, result): - result = longlong.int2singlefloat(result) - self._libffi_save_result(cif_description, exchange_buffer, result) - - # ---------- # helpers to resume running in blackhole mode when a guard failed diff --git a/rpython/jit/metainterp/pyjitpl.py b/rpython/jit/metainterp/pyjitpl.py --- a/rpython/jit/metainterp/pyjitpl.py +++ b/rpython/jit/metainterp/pyjitpl.py @@ -1331,34 +1331,6 @@ metainterp.history.record(rop.VIRTUAL_REF_FINISH, [vrefbox, nullbox], None) - @arguments("box", "box", "box") - def _opimpl_libffi_save_result(self, box_cif_description, - box_exchange_buffer, box_result): - from rpython.rtyper.lltypesystem import llmemory - from rpython.rlib.jit_libffi import CIF_DESCRIPTION_P - from rpython.jit.backend.llsupport.ffisupport import get_arg_descr - - cif_description = box_cif_description.getint() - cif_description = llmemory.cast_int_to_adr(cif_description) - cif_description = llmemory.cast_adr_to_ptr(cif_description, - CIF_DESCRIPTION_P) - - kind, descr, itemsize = get_arg_descr(self.metainterp.cpu, cif_description.rtype) - - if kind != 'v': - ofs = cif_description.exchange_result - assert ofs % itemsize == 0 # alignment check (result) - self.metainterp.history.record(rop.SETARRAYITEM_RAW, - [box_exchange_buffer, - ConstInt(ofs // itemsize), - box_result], - None, descr) - - opimpl_libffi_save_result_int = _opimpl_libffi_save_result - opimpl_libffi_save_result_float = _opimpl_libffi_save_result - opimpl_libffi_save_result_longlong = _opimpl_libffi_save_result - opimpl_libffi_save_result_singlefloat = _opimpl_libffi_save_result - # ------------------------------ def setup_call(self, argboxes): @@ -2910,7 +2882,7 @@ self.history.operations.extend(extra_guards) # # note that the result is written back to the exchange_buffer by the - # special op libffi_save_result_{int,float} + # following operation, which should be a raw_store def direct_call_release_gil(self): op = self.history.operations.pop() diff --git a/rpython/jit/metainterp/test/test_fficall.py b/rpython/jit/metainterp/test/test_fficall.py --- a/rpython/jit/metainterp/test/test_fficall.py +++ b/rpython/jit/metainterp/test/test_fficall.py @@ -9,7 +9,7 @@ from rpython.rlib import jit from rpython.rlib import jit_libffi from rpython.rlib.jit_libffi import (types, CIF_DESCRIPTION, FFI_TYPE_PP, - jit_ffi_call, jit_ffi_save_result) + jit_ffi_call) from rpython.rlib.unroll import unrolling_iterable from rpython.rlib.rarithmetic import intmask, r_longlong, r_singlefloat from rpython.rlib.longlong2float import float2longlong @@ -255,7 +255,7 @@ # when n==50, fn() will force the frame, so guard_not_forced # fails and we enter blackholing: this test makes sure that # the result of call_release_gil is kept alive before the - # libffi_save_result, and that the corresponding box is passed + # raw_store, and that the corresponding box is passed # in the fail_args. Before the fix, the result of # call_release_gil was simply lost and when guard_not_forced # failed, and the value of "res" was unpredictable. @@ -291,7 +291,6 @@ cd.atypes = atypes cd.exchange_size = 64 # 64 bytes of exchange data cd.exchange_result = 24 - cd.exchange_result_libffi = 24 cd.exchange_args[0] = 16 def f(): diff --git a/rpython/rlib/jit_libffi.py b/rpython/rlib/jit_libffi.py --- a/rpython/rlib/jit_libffi.py +++ b/rpython/rlib/jit_libffi.py @@ -1,10 +1,10 @@ - -from rpython.rtyper.lltypesystem import lltype, rffi -from rpython.rtyper.extregistry import ExtRegistryEntry +import sys +from rpython.rtyper.lltypesystem import lltype, llmemory, rffi +from rpython.rtyper.lltypesystem.lloperation import llop from rpython.rlib import clibffi, jit from rpython.rlib.rarithmetic import r_longlong, r_singlefloat -from rpython.rlib.nonconst import NonConstant +BIG_ENDIAN = sys.byteorder == 'big' FFI_CIF = clibffi.FFI_CIFP.TO FFI_TYPE = clibffi.FFI_TYPE_P.TO @@ -13,6 +13,8 @@ FFI_ABI = clibffi.FFI_ABI FFI_TYPE_STRUCT = clibffi.FFI_TYPE_STRUCT SIZE_OF_FFI_ARG = rffi.sizeof(clibffi.ffi_arg) +SIZE_OF_SIGNED = rffi.sizeof(lltype.Signed) +FFI_ARG_P = rffi.CArrayPtr(clibffi.ffi_arg) # Usage: for each C function, make one CIF_DESCRIPTION block of raw # memory. Initialize it by filling all its fields apart from 'cif'. @@ -33,11 +35,12 @@ # - 'exchange_result': the offset in that buffer for the result of the call. # (this and the other offsets must be at least NARGS * sizeof(void*).) # -# - 'exchange_result_libffi': the actual offset passed to ffi_call(). -# Differs on big-endian machines if the result is an integer type smaller -# than SIZE_OF_FFI_ARG (blame libffi). +# - 'exchange_args[nargs]': the offset in that buffer for each argument. # -# - 'exchange_args[nargs]': the offset in that buffer for each argument. +# Each argument and the result should have enough room for at least +# SIZE_OF_FFI_ARG bytes, even if they may be smaller. (Unlike ffi_call, +# we don't have any special rule about results that are integers smaller +# than SIZE_OF_FFI_ARG). CIF_DESCRIPTION = lltype.Struct( 'CIF_DESCRIPTION', @@ -48,7 +51,6 @@ ('atypes', FFI_TYPE_PP), # ('exchange_size', lltype.Signed), ('exchange_result', lltype.Signed), - ('exchange_result_libffi', lltype.Signed), ('exchange_args', lltype.Array(lltype.Signed, hints={'nolength': True, 'immutable': True})), hints={'immutable': True}) @@ -93,12 +95,16 @@ ## ## The result is that now the jitcode looks like this: ## -## %i0 = libffi_call_int(...) +## %i0 = direct_call(libffi_call_int, ...) ## -live- -## libffi_save_result_int(..., %i0) +## raw_store(exchange_result, %i0) ## ## the "-live-" is the key, because it make sure that the value is not lost if ## guard_not_forced fails. +## +## The value of %i0 is stored back in the exchange_buffer at the offset +## exchange_result, which is usually where functions like jit_ffi_call_impl_int +## have just read it from when called *in interpreter mode* only. def jit_ffi_call(cif_description, func_addr, exchange_buffer): @@ -129,51 +135,71 @@ def _do_ffi_call_int(cif_description, func_addr, exchange_buffer): result = jit_ffi_call_impl_int(cif_description, func_addr, exchange_buffer) - jit_ffi_save_result('int', cif_description, exchange_buffer, result) + if BIG_ENDIAN: + # Special case: we need to store an integer of 'c_size' bytes + # only. To avoid type-specialization hell, we always store a + # full Signed here, but by shifting it to the left on big-endian + # we get the result that we want. + size = rffi.getintfield(cif_description.rtype, 'c_size') + if size < SIZE_OF_SIGNED: + result <<= (SIZE_OF_SIGNED - size) * 8 + llop.raw_store(lltype.Void, + llmemory.cast_ptr_to_adr(exchange_buffer), + cif_description.exchange_result, + result) def _do_ffi_call_float(cif_description, func_addr, exchange_buffer): # a separate function in case the backend doesn't support floats result = jit_ffi_call_impl_float(cif_description, func_addr, exchange_buffer) - jit_ffi_save_result('float', cif_description, exchange_buffer, result) + llop.raw_store(lltype.Void, + llmemory.cast_ptr_to_adr(exchange_buffer), + cif_description.exchange_result, + result) def _do_ffi_call_longlong(cif_description, func_addr, exchange_buffer): # a separate function in case the backend doesn't support longlongs result = jit_ffi_call_impl_longlong(cif_description, func_addr, exchange_buffer) - jit_ffi_save_result('longlong', cif_description, exchange_buffer, result) + llop.raw_store(lltype.Void, + llmemory.cast_ptr_to_adr(exchange_buffer), + cif_description.exchange_result, + result) def _do_ffi_call_singlefloat(cif_description, func_addr, exchange_buffer): # a separate function in case the backend doesn't support singlefloats result = jit_ffi_call_impl_singlefloat(cif_description, func_addr, exchange_buffer) - jit_ffi_save_result('singlefloat', cif_description, exchange_buffer,result) + llop.raw_store(lltype.Void, + llmemory.cast_ptr_to_adr(exchange_buffer), + cif_description.exchange_result, + result) -# we must return a NonConstant else we get the constant -1 as the result of -# the flowgraph, and the codewriter does not produce a box for the -# result. Note that when not-jitted, the result is unused, but when jitted the -# box of the result contains the actual value returned by the C function. - @jit.oopspec("libffi_call(cif_description,func_addr,exchange_buffer)") def jit_ffi_call_impl_int(cif_description, func_addr, exchange_buffer): jit_ffi_call_impl_any(cif_description, func_addr, exchange_buffer) - return NonConstant(-1) + # read a complete 'ffi_arg' word + resultdata = rffi.ptradd(exchange_buffer, cif_description.exchange_result) + return rffi.cast(lltype.Signed, rffi.cast(FFI_ARG_P, resultdata)[0]) @jit.oopspec("libffi_call(cif_description,func_addr,exchange_buffer)") def jit_ffi_call_impl_float(cif_description, func_addr, exchange_buffer): jit_ffi_call_impl_any(cif_description, func_addr, exchange_buffer) - return NonConstant(-1.0) + resultdata = rffi.ptradd(exchange_buffer, cif_description.exchange_result) + return rffi.cast(rffi.DOUBLEP, resultdata)[0] @jit.oopspec("libffi_call(cif_description,func_addr,exchange_buffer)") def jit_ffi_call_impl_longlong(cif_description, func_addr, exchange_buffer): jit_ffi_call_impl_any(cif_description, func_addr, exchange_buffer) - return r_longlong(-1) + resultdata = rffi.ptradd(exchange_buffer, cif_description.exchange_result) + return rffi.cast(rffi.LONGLONGP, resultdata)[0] @jit.oopspec("libffi_call(cif_description,func_addr,exchange_buffer)") def jit_ffi_call_impl_singlefloat(cif_description, func_addr, exchange_buffer): jit_ffi_call_impl_any(cif_description, func_addr, exchange_buffer) - return r_singlefloat(-1.0) + resultdata = rffi.ptradd(exchange_buffer, cif_description.exchange_result) + return rffi.cast(rffi.FLOATP, resultdata)[0] @jit.oopspec("libffi_call(cif_description,func_addr,exchange_buffer)") def jit_ffi_call_impl_void(cif_description, func_addr, exchange_buffer): @@ -191,36 +217,12 @@ data = rffi.ptradd(exchange_buffer, cif_description.exchange_args[i]) buffer_array[i] = data resultdata = rffi.ptradd(exchange_buffer, - cif_description.exchange_result_libffi) + cif_description.exchange_result) clibffi.c_ffi_call(cif_description.cif, func_addr, rffi.cast(rffi.VOIDP, resultdata), buffer_array) - return -1 - -def jit_ffi_save_result(kind, cif_description, exchange_buffer, result): - """ - This is a no-op during normal execution, but actually fills the buffer - when jitted - """ - pass - -class Entry(ExtRegistryEntry): - _about_ = jit_ffi_save_result - - def compute_result_annotation(self, kind_s, *args_s): - from rpython.annotator import model as annmodel - assert isinstance(kind_s, annmodel.SomeString) - assert kind_s.const in ('int', 'float', 'longlong', 'singlefloat') - - def specialize_call(self, hop): - hop.exception_cannot_occur() - vlist = hop.inputargs(lltype.Void, *hop.args_r[1:]) - return hop.genop('jit_ffi_save_result', vlist, - resulttype=lltype.Void) - - # ____________________________________________________________ class types(object): diff --git a/rpython/rlib/test/test_jit_libffi.py b/rpython/rlib/test/test_jit_libffi.py --- a/rpython/rlib/test/test_jit_libffi.py +++ b/rpython/rlib/test/test_jit_libffi.py @@ -24,7 +24,6 @@ cd.atypes = atypes cd.exchange_size = 64 # 64 bytes of exchange data cd.exchange_result = 24 - cd.exchange_result_libffi = 24 cd.exchange_args[0] = 16 # jit_ffi_prep_cif(cd) _______________________________________________ pypy-commit mailing list pypy-commit@python.org https://mail.python.org/mailman/listinfo/pypy-commit