From fd71558d0d32dd60d19373afbc8f9713bc78f23d Mon Sep 17 00:00:00 2001 From: Douglas Katzman Date: Tue, 15 Mar 2022 10:32:31 -0400 Subject: [PATCH] mips: fix failing ash-amount-unsigned-comparison test Also: - Put a comment and assertion back the way I meant it (Messed it up due to merge conflict) - Skip/ignore some unimportant tests --- src/compiler/mips/arith.lisp | 4 ++-- src/runtime/gencgc.c | 21 ++++++++++++++------- tests/allocator.pure.lisp | 3 +-- tests/backtrace.impure.lisp | 2 +- tests/callback.impure.lisp | 7 +++++-- tests/random.pure.lisp | 2 +- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/compiler/mips/arith.lisp b/src/compiler/mips/arith.lisp index d99ee80c1..8b8b0badd 100644 --- a/src/compiler/mips/arith.lisp +++ b/src/compiler/mips/arith.lisp @@ -191,7 +191,7 @@ (:generator 3 (inst bgez amount positive) (inst subu ndesc zero-tn amount) - (inst slt temp ndesc 32) + (inst slt temp ndesc 32) ; FIXME: should be sltu here ? (inst bne temp done) (inst srl result number ndesc) (inst b done) @@ -217,7 +217,7 @@ (:generator 3 (inst bgez amount positive) (inst subu ndesc zero-tn amount) - (inst slt temp ndesc 31) + (inst sltu temp ndesc 31) (inst bne temp done) (inst sra result number ndesc) (inst b done) diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c index 0af612b55..f07f9b8dc 100644 --- a/src/runtime/gencgc.c +++ b/src/runtime/gencgc.c @@ -4776,17 +4776,24 @@ lisp_alloc(int largep, struct alloc_region *region, sword_t nbytes, large_allocation = nbytes; /* maybe we can do this quickly ... */ + /* I'd really like this "quick" case to be more uniform in terms of whether + * it's allowed to occur at all. Some of the inconsistencies are: + * - 32-bit x86 will (or would, not sure any more) choose to use + * out-of-line allocation if lexical policy favors space. + * - PPC at git rev 28aaa39f4e had a subtle "but-not-wrong" bug at the edge + * where it trapped to C if the new free pointer was ':lge' instead of ':lgt' + * the region end, fixed in rev 05047647. + * - other architectures may have similar issues. + * So because of those reasons, even if we satisfy the allocation + * from the TLAB it might be worth a check of whether to refill + * the TLAB now. */ void *new_obj = region->free_pointer; char *new_free_pointer = (char*)new_obj + nbytes; if (new_free_pointer <= (char*)region->end_addr) { region->free_pointer = new_free_pointer; -#ifdef LISP_FEATURE_X86_64 - // Non-code allocations should never get here - it would mean there's - // something wrong in the inline allocator. This assertion pertains - // to any architecture that always uses an inline allocator. - // That's actually most of them, but I haven't tested that they're right. - // e.g. x86 forgoes inline allocation depending on policy, - // and git revision 05047647 tweaked the edge case for PPC. +#if defined LISP_FEATURE_MIPS || defined LISP_FEATURE_PPC || \ + defined LISP_FEATURE_PPC64 || defined LISP_FEATURE_X86_64 + // Most allocations should never get here, but two page types are special. gc_assert(page_type == PAGE_TYPE_CONS || page_type == PAGE_TYPE_CODE); #endif return(new_obj); /* yup */ diff --git a/tests/allocator.pure.lisp b/tests/allocator.pure.lisp index 042d4acd3..3bfc74e28 100644 --- a/tests/allocator.pure.lisp +++ b/tests/allocator.pure.lisp @@ -50,8 +50,7 @@ ;;; The 32-bit architectures that use GENCGC-PAGE-BYTES = 65536 are unaffected ;;; by the change that took the size test out of the allocator fast path. -#+gencgc ; PSEUDO-STATIC-GENERATION etc don't exist for cheneygc -(with-test (:name :pseudostatic-large-objects :fails-on :x86) +(with-test (:name :pseudostatic-large-objects :fails-on (or :mips :x86)) (sb-vm:map-allocated-objects (lambda (obj type size) (declare (ignore type size)) diff --git a/tests/backtrace.impure.lisp b/tests/backtrace.impure.lisp index 84741d133..662690953 100644 --- a/tests/backtrace.impure.lisp +++ b/tests/backtrace.impure.lisp @@ -264,7 +264,7 @@ (defun throw-test () (throw 'no-such-tag t)) (with-test (:name (:backtrace :throw :no-such-tag) - :fails-on (and :sparc :linux)) + :fails-on (or :mips (and :sparc :linux))) (assert-backtrace #'throw-test '((throw-test)))) (funcall (checked-compile diff --git a/tests/callback.impure.lisp b/tests/callback.impure.lisp index 6a0a76290..f6ac36a2a 100644 --- a/tests/callback.impure.lisp +++ b/tests/callback.impure.lisp @@ -14,8 +14,11 @@ (in-package :cl-user) ;;; callbacks only on a few platforms -#-alien-callbacks -(exit :code 104) +;;; (actually, all platforms claim to support them now, +;;; and :alien-callbacks is almost everywhere defined. +;;; However mips doesn't seem to correctly implement them, +;;; making the feature indicator somewhat useless) +#+(or (not alien-callbacks) mips) (exit :code 104) ;;; simple callback for a function diff --git a/tests/random.pure.lisp b/tests/random.pure.lisp index b37b7a84e..8383705a1 100644 --- a/tests/random.pure.lisp +++ b/tests/random.pure.lisp @@ -181,7 +181,7 @@ (assert (not (find 0.0d0 values :test-not #'=)))))) (with-test (:name :float-no-consing - :fails-on :ppc + :fails-on (or :mips :ppc) :skipped-on :interpreter) (let ((fun (checked-compile `(lambda () (declare (optimize speed))