cosmos72 / stmx

High performance Transactional Memory for Common Lisp
http://stmx.org/
241 stars 14 forks source link

VOP failing with SBCL 2.3.5 #27

Closed glv2 closed 8 months ago

glv2 commented 1 year ago

Hi. When trying to compile STMX with SBCL 2.3.5 on GNU/Linux x86-64, I get the following error:

; compiling file "/gnu/store/ij4dkw97l4mmx873a9n7jv276x92z2hp-sbcl-stmx-2.0.5-2.f71e742/share/common-lisp/sbcl/stmx/asm/transaction.lisp" (written 01 JAN 1970 12:00:00 AM):
Unhandled SIMPLE-ERROR in thread #<SB-THREAD:THREAD tid=28 "main thread" RUNNING
                                    {10013D8003}>:
  #<SB-C:TN t1 :NORMAL> is not valid as the first result to VOP:
  %XBEGIN
Primitive type: T
SC restrictions:
  (SB-VM::UNSIGNED-REG)
The primitive type disallows these loadable SCs:
  (SB-VM::UNSIGNED-REG)

Backtrace for: #<SB-THREAD:THREAD tid=28 "main thread" RUNNING {10013D8003}>
[...]
11: (SB-C::COMPILE-COMPONENT #<SB-C:COMPONENT :NAME "DEFUN TRANSACTION-BEGIN" {1007DBDC83}>)
[...]
glv2 commented 1 year ago

With the following patch I can compile STMX and the tests pass:

diff --git a/asm/transaction.lisp b/asm/transaction.lisp
index 10099ac..94c36a4 100644
--- a/asm/transaction.lisp
+++ b/asm/transaction.lisp
@@ -16,7 +16,7 @@
 (in-package :stmx.asm)

-(declaim (ftype (function () fixnum)        transaction-begin)
+(declaim (ftype (function () (unsigned-byte 32)) transaction-begin)
          (ftype (function () (integer 0 0)) transaction-end)
          (ftype (function () (integer 0 0)) transaction-abort)
          (ftype (function () boolean)       transaction-running-p)
@@ -37,7 +37,7 @@ otherwise return code of the error that caused the transaction to abort.

 Invoking TRANSACTION-BEGIN while there is already a running hardware
 memory transaction has implementation-dependent effects."
-  (the (values fixnum  &optional)
+  (the (unsigned-byte 32)
     #-(and) (%transaction-begin)
     (sb-c::%primitive %xbegin)))

But I'm not sure if changing (the (values some-type &optional) ...) to (the some-type ...) is completely valid...

cosmos72 commented 1 year ago

Hi @glv2, thanks for spotting this! and even more thanks for the patch :)

Replacing fixnum -> (unsigned-byte 32) makes sense, since the underlying xbegin CPU instruction fills the 32-bit register eax. I'll have to double-check the replacement (values TYPE &optional) -> TYPE, because I am not 100% sure about the implications

cosmos72 commented 1 year ago

I have tested your patch on sbcl 2.1.5 and merged it. Thanks again :)

Trying to test it on sbcl 2.3.5, I met the following error while quicklisp was loading the "log4cl" dependency:

* (ql:quickload "stmx")
To load "stmx":
  Load 1 ASDF system:
    stmx
; Loading "stmx"
.;
; caught ERROR:
;   READ error during COMPILE-FILE:
;
;     Lock on package SB-C violated when interning LAMBDA-PARENT while in package
;     LOG4CL-IMPL.
;   See also:
;     The SBCL Manual, Node "Package Locks"
;
;     (in form starting at line: 99, column: 0, position: 3779)

debugger invoked on a UIOP/LISP-BUILD:COMPILE-FILE-ERROR in thread
#<THREAD tid=5820 "main thread" RUNNING {10044C81B3}>:
  COMPILE-FILE-ERROR while
  compiling #<CL-SOURCE-FILE "log4cl" "src" "naming-sbcl">

Did you solve it? A workaround would be really useful to test the patch also on sbcl 2.3.5

glv2 commented 1 year ago

I didn't try with Quicklisp, I used Guix, which has a more recent version of log4cl which has the patch for this issue (see https://github.com/sharplispers/log4cl/commit/01242ffbc43f5cf17f612878666341b0a3430c46).