'Re: [Sbcl-devel] [Sbcl-commits] master: Translate <= to < and = on some platforms.'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       sbcl-devel
Subject:    Re: [Sbcl-devel] [Sbcl-commits] master: Translate <= to < and = on some platforms.
From:       Stas Boukarev <stassats () gmail ! com>
Date:       2023-01-20 22:42:47
Message-ID: CAF63=10z-byB9kqC-0HrW2oSnNtGtLHrH9BmnV2HgOWzzNFN9w () mail ! gmail ! com
[Download RAW message or body]

I don't see anything relevant in compute-object-header directly. I'm
somewhat ready to just disable the transform on mips, but it's
probably hiding a real problem.

On Sat, Jan 21, 2023 at 12:55 AM Douglas Katzman <dougk@google.com> wrote:
> 
> No change at latest rev.
> I found a fairly small function that gets miscompiled, \
> sb-vm::compute-object-header. I can make it compile correctly by adding this diff \
> to it (which wouldn't be the worst thing to add anyway, but later after the \
> miscompile is debugged). Naturally cold-init fails for other reasons, but it should \
> be possible to compare a broken version of this against a good version and see \
> where the compiler went wrong. 
> (defun compute-object-header (nwords widetag-or-metadata)
> 
> -  (let* ((widetag (if (typep widetag-or-metadata '(or wrapper \
> defstruct-description)) 
> -                      instance-widetag
> 
> -                      widetag-or-metadata))
> 
> +  (let* ((widetag (the
> 
> +                   (unsigned-byte 8)
> 
> +                   (if (typep widetag-or-metadata '(or wrapper \
> defstruct-description)) 
> +                       instance-widetag
> 
> +                       widetag-or-metadata)))
> 
> (array-header-p
> 
> (or (= widetag simple-array-widetag)
> 
> (>= widetag complex-base-string-widetag))))
> 
> 
> On Fri, Jan 20, 2023 at 2:27 PM Stas Boukarev <stassats@gmail.com> wrote:
> > 
> > I guess try now. The latest fix sounds like something that could be
> > causing this.
> > 
> > On Fri, Jan 20, 2023 at 4:20 AM Stas Boukarev <stassats@gmail.com> wrote:
> > > 
> > > I have no idea how to debug that.
> > > 
> > > On Fri, Jan 20, 2023 at 3:45 AM Douglas Katzman <dougk@google.com> wrote:
> > > > 
> > > > Bisected the 2nd failure to
> > > > 
> > > > 334feec7125f204aa597926a27fa96171ae0b52f is the first bad commit
> > > > 
> > > > commit 334feec7125f204aa597926a27fa96171ae0b52f
> > > > 
> > > > Author: Stas Boukarev <stassats@gmail.com>
> > > > 
> > > > Date:   Wed Jan 18 20:49:14 2023 +0300
> > > > 
> > > > 
> > > > Transform to range< at the end of ir1-phases.
> > > > 
> > > > 
> > > > Avoid poorly duplicating the optimizers for the < functions, no need
> > > > 
> > > > to convert back from range<, no need to worry about a variable not
> > > > 
> > > > triggering reoptimization in another node.
> > > > 
> > > > 
> > > > The backtrace isn't very helpful at all -
> > > > 
> > > > 
> > > > Backtrace for: #<SB-THREAD:THREAD "main thread" RUNNING {50C0AA39}>
> > > > 
> > > > 0: (SB-C:COMPILER-ERROR SB-C::INPUT-ERROR-IN-LOAD :CONDITION #<TYPE-ERROR \
> > > > {52DD13B1}> :POSITION 469 :LINE/COL (1 . -1) :STREAM \
> > > > #<SB-INT:FORM-TRACKING-STREAM for "file \
> > > > /home/snuglas/devel/sbcl/src/cold/warm.lisp" {52DCE0B9}>) 
> > > > 1: (SB-C::%DO-FORMS-FROM-INFO #<FUNCTION (LAMBDA (SB-KERNEL:FORM &KEY \
> > > > :CURRENT-INDEX &ALLOW-OTHER-KEYS) :IN SB-INT:LOAD-AS-SOURCE) {52DCFD95}> \
> > > > #<SB-C::SOURCE-INFO {52DCFD71}> SB-C::INPUT-ERROR-IN-LOAD) 
> > > > 2: (SB-INT:LOAD-AS-SOURCE #<SB-INT:FORM-TRACKING-STREAM for "file \
> > > > /home/snuglas/devel/sbcl/src/cold/warm.lisp" {52DCE0B9}> :VERBOSE NIL :PRINT \
> > > > NIL :CONTEXT "loading") 
> > > > 3: ((LABELS SB-FASL::LOAD-STREAM-1 :IN LOAD) #<SB-INT:FORM-TRACKING-STREAM \
> > > > for "file /home/snuglas/devel/sbcl/src/cold/warm.lisp" {52DCE0B9}> NIL) 
> > > > 4: (SB-FASL::CALL-WITH-LOAD-BINDINGS #<FUNCTION (LABELS \
> > > > SB-FASL::LOAD-STREAM-1 :IN LOAD) {76A8052D}> #<SB-INT:FORM-TRACKING-STREAM \
> > > > for "file /home/snuglas/devel/sbcl/src/cold/warm.lisp" {52DCE0B9}> NIL \
> > > > "src/cold/warm.lisp") 
> > > > 5: (LOAD "src/cold/warm.lisp" :VERBOSE NIL :PRINT NIL :IF-DOES-NOT-EXIST \
> > > > :ERROR :EXTERNAL-FORMAT :DEFAULT) 
> > > > 
> > > > On Thu, Jan 19, 2023 at 5:05 PM Douglas Katzman <dougk@google.com> wrote:
> > > > > 
> > > > > ppc32 works. sparc works, mips gets a type error as shown.  I'll need to \
> > > > > rerun a build at the original culprit change plus just this patch to see if \
> > > > > there was some other breakage later versus the patch being insufficient. 
> > > > > The value
> > > > > -1
> > > > > is not of type
> > > > > (OR NULL (MOD 536870909))
> > > > > when setting slot SB-IMPL::INPUT-CHAR-POS of structure \
> > > > > SB-KERNEL:ANSI-STREAM 
> > > > > On Thu, Jan 19, 2023 at 4:35 PM stassats via Sbcl-commits \
> > > > > <sbcl-commits@lists.sourceforge.net> wrote:
> > > > > > 
> > > > > > The branch "master" has been updated in SBCL:
> > > > > > via  db64046a883bbc1d2cbdf9a713ca39a84738c8ac (commit)
> > > > > > from  f021c6edfcdf2b68b9032396b2602738a05893a6 (commit)
> > > > > > 
> > > > > > - Log -----------------------------------------------------------------
> > > > > > commit db64046a883bbc1d2cbdf9a713ca39a84738c8ac
> > > > > > Author: Stas Boukarev <stassats@gmail.com>
> > > > > > Date:   Fri Jan 20 00:32:35 2023 +0300
> > > > > > 
> > > > > > Translate <= to < and = on some platforms.
> > > > > > 
> > > > > > For some reason they do not survive cold init.
> > > > > > ---
> > > > > > src/compiler/srctran.lisp | 18 ++++++++++++++++++
> > > > > > 1 file changed, 18 insertions(+)
> > > > > > 
> > > > > > diff --git a/src/compiler/srctran.lisp b/src/compiler/srctran.lisp
> > > > > > index 541eb1479..fc4f5e1d3 100644
> > > > > > --- a/src/compiler/srctran.lisp
> > > > > > +++ b/src/compiler/srctran.lisp
> > > > > > @@ -3831,6 +3831,7 @@
> > > > > > (neq *empty-type* (type-intersection (specifier-type 'float)
> > > > > > (lvar-type lvar))))
> > > > > > 
> > > > > > +#+(or arm arm64 x86-64 x86)
> > > > > > (flet ((maybe-invert (op inverted x y)
> > > > > > (cond
> > > > > > ((and (not (vop-existsp :translate >=))
> > > > > > @@ -3849,6 +3850,23 @@
> > > > > > "invert or open code"
> > > > > > (maybe-invert '< '> x y)))
> > > > > > 
> > > > > > +;;; FIXME: for some reason these do not survive cold-init with <=
> > > > > > +#-(or arm arm64 x86-64 x86)
> > > > > > +(flet ((maybe-invert (node op inverted x y)
> > > > > > +         (cond
> > > > > > +           ;; Don't invert if either argument can be a float (NaNs)
> > > > > > +           ((or (maybe-float-lvar-p x) (maybe-float-lvar-p y))
> > > > > > +            (delay-ir1-transform node :constraint)
> > > > > > +            `(or (,op x y) (= x y)))
> > > > > > +           (t
> > > > > > +            `(if (,inverted x y) nil t)))))
> > > > > > +  (deftransform >= ((x y) (number number) * :node node)
> > > > > > +    "invert or open code"
> > > > > > +    (maybe-invert node '> '< x y))
> > > > > > +  (deftransform <= ((x y) (number number) * :node node)
> > > > > > +    "invert or open code"
> > > > > > +    (maybe-invert node '< '> x y)))
> > > > > > +
> > > > > > ;;; See whether we can statically determine (< X Y) using type
> > > > > > ;;; information. If X's high bound is < Y's low, then X < Y.
> > > > > > ;;; Similarly, if X's low is >= to Y's high, the X >= Y (so return
> > > > > > 
> > > > > > -----------------------------------------------------------------------
> > > > > > 
> > > > > > 
> > > > > > hooks/post-receive
> > > > > > --
> > > > > > SBCL
> > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Sbcl-commits mailing list
> > > > > > Sbcl-commits@lists.sourceforge.net
> > > > > > https://lists.sourceforge.net/lists/listinfo/sbcl-commits


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


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic