Re: [Sbcl-devel] [Sbcl-commits] master: Use :dword operand size in some integer-length vops

[Christophe Rhodes]

Some time ago...

snuglas via Sbcl-commits <sbcl-c...@lists.sourceforge.net> writes:

> @@ -1782,12 +1782,13 @@
> (:arg-types positive-fixnum)
> (:results (res :scs (unsigned-reg)))
> (:result-types unsigned-num)
> + (:arg-refs arg-ref)
> (:generator 24
> - (move res arg)
> - (when (> n-fixnum-tag-bits 1)
> - (inst shr res (1- n-fixnum-tag-bits)))
> - (inst or res 1)
> - (inst bsr res res)))
> + (let ((size (if (csubtypep (tn-ref-type arg-ref) (specifier-type '(unsigned-byte 31)))
> + :dword :qword)))
> + (move res arg size)
> + (inst or size res 1)
> + (inst bsr size res res))))

Doesn't this remove support for n-fixnum-tag-bits not equal to 1?
(Proposed fix attached, but I could be missing something).

Christophe

[Douglas Katzman via Sbcl-devel]

The choice of n-fixnum-tag-bits >1 for x86-64 ceased working in fairly short order after the choice of '1' became the default.

I occasionally used to try to test it for n=2 and n=3 but stopped doing so when it became futile - too many vops don't support it and iirc the regression suite always had problems not that it matters.

So further support for it is a non-goal; keeping the code more readable is the goal.

(The only way to have prevented regressions would have been continuous builds, if actual support were an objective)

[Christophe Rhodes]

Douglas Katzman <do...@google.com> writes:

> (The only way to have prevented regressions would have been continuous
> builds, if actual support were an objective)

Speaking of which: I tried adding a github CI job for the mark-and-sweep
garbage collector; the change is at
<https://github.com/csrhodes/sbcl/tree/mark-region-ci>, and the log from
tests running is at
<https://github.com/csrhodes/sbcl/actions/runs/6361902047/job/17276615614>,
looking very much like some kind of heap corruption or at least heap
misunderstanding.

Firstly, is this failure expected?

Secondly, is anyone using it, or is this (absence of) CI the only
bulwark we have against its bitrot?

Christophe


_______________________________________________
Sbcl-devel mailing list
Sbcl-...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sbcl-devel

[Hayley Patton]

Firstly, is this failure expected?

Test failure is expected; not that one in particular though. Doug fixed it with 9b8f9c878cfb5d43046261b20b8b550b31d951e6 I believe.

Secondly, is anyone using it, or is this (absence of) CI the only
bulwark we have against its bitrot?

I use it infrequently, but I'm currently focusing on implementing immobile-space support, which is completely broken at the moment.

[Christophe Rhodes]

Hayley Patton <hay...@applied-langua.ge> writes:

> Firstly, is this failure expected?
>
> Test failure is expected; not that one in particular though. Doug fixed it with 9b8f9c878cfb5d43046261b20b8b550b31d951e6 I believe.

Thanks. The test logs with that included are at
<https://github.com/csrhodes/sbcl/actions/runs/6370846916/job/17292280228>,
which has a reasonable number of failures in gc-like tests which I could
imagine are expected, and a failure in futex-wait.test.sh which might be
surprising? (There's no explicit dependence on the garbage collector in
futex-wait, but I could imagine an implicit dependence on some feature
of gencgc).

> Secondly, is anyone using it, or is this (absence of) CI the only
> bulwark we have against its bitrot?
>
> I use it infrequently, but I'm currently focusing on implementing immobile-space support, which is completely broken at the moment.

OK. I'd like to get the current state into our automated test
infrastructure, for which I think we need:

1. annotating the existing test failures as :fails-on / :broken-on as
appropriate;
2. where possible, filing bugs to explain to future volunteers what the
issue might be;
3. checking whether the remaining parts of the build (ansi-tests,
crossbuilds) actually work.

It would be good if someone with more knowledge of the mark-region GC
could do the first two, particularly the "filing bugs", so that we don't
lose all context by marking these things as expected to fail.

Thanks,

[Douglas Katzman via Sbcl-devel]

On Sun, Oct 1, 2023 at 9:43 AM Christophe Rhodes <cs...@cantab.net> wrote:

and a failure in futex-wait.test.sh which might be
surprising?  (There's no explicit dependence on the garbage collector in
futex-wait, but I could imagine an implicit dependence on some feature
of gencgc).

the futex test has me baffled. I had seen it on and off ever since working with Hayley on this, and chalked it up to an interaction of condition variables across multiple threads, speed of the machine, number of physical cores.   The test itself is a little bit of a change-detector so not great anyway. But the fact is it got worse with more futex wakeups and should not have.

[Hayley Patton]

On 2/10/23 03:17, Douglas Katzman wrote:

Do you have any idea why it would have actually mattered that gc_gen_of was called wrongly? Granted it was a bug, but it should never fail to find a generation for the line, hence never needed the default.  I added a comment to that effect

For some reason writing the generations is done when closing a TLAB, not when it is allocated, so new objects don't have a generation yet. Try evaluating e.g. (sb-kernel:generation-of (list 1)) - I don't know why I wrote the allocator to do that. My comment in try_allocate_small isn't helping me remember.

[Hayley Patton]

Here is a patch which makes gc_gen_of recognise fresh lines, though I am
not very happy with it. I don't remember why try_allocate_small or such
can't set the generations, but it made the GC misbehave when I tried
that, and I can't investigate further night now.