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

Reply via email to