bug#46540: 28.0.50; Native-comp optimization bug

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Quek Yu Han

The following function gives an incorrect result when compiled natively
with comp-speed >= 2.

;; -*- lexical-binding: t; -*-

(defun native-comp-bug (body)
  (let ((x (car body))
        (y (cadr body)))
    (unless (and
             (natnump x)
             (natnump y)
             (<= x y))
      (error ""))
    x))

(native-comp-bug '(3 4))

;; Expected result: 3
;; Actual result: 0

The car of the argument is ignored and treated as 0.

This appears to be a minimal repro, manually shrunk from the original function
discovered in rx.el (rx--translate-bounded-repetition).

If either lexical binding, natnump/inequality checks, or error form is removed,
the function behaves as expected.

Setting comp-speed to 0 or 1 also causes the function to behave as expected, but
setting it to 2 or 3 produces the error.

In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.2.0, NS appkit-2022.20 Version 11.1 (Build 20C69))
 of 2021-02-03 built on QYH2.local
Repository revision: e56c26d06f72b58acdc59c7e2958be9b691d3e78
Repository branch: master
System Description:  macOS 11.2.1

Configured using:
 'configure --with-ns --with-modules
 '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-xwidgets --with-nativecomp
 --without-rsvg 'CFLAGS=-I/usr/local/opt/gcc/include -O2 -march=native'
 'LDFLAGS=-L/usr/local/opt/gcc/lib/gcc/10
 -L/usr/local/opt/libgccjit/lib/gcc/10
 -L/usr/local/opt/gcc/lib/gcc/10/gcc/x86_64-apple-darwin20/10.2.0
 -I/usr/local/opt/gcc/include -I/usr/local/opt/libgccjit/include''

Configured features:
ACL GIF GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY
KQUEUE NS PDUMPER PNG THREADS TIFF TOOLKIT_SCROLL_BARS XWIDGETS ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LC_CTYPE: UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source eieio eieio-core eieio-loaddefs
password-cache json map text-property-search time-date mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
term/xterm xterm comp comp-cstr warnings subr-x rx cl-seq cl-macs
cl-extra help-mode easymenu seq byte-opt gv cl-loaddefs cl-lib bytecomp
byte-compile cconv iso-transl tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face pcase macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads xwidget-internal kqueue cocoa
ns lcms2 multi-tty make-network-process nativecomp emacs)

Memory information:
((conses 16 79127 4281)
 (symbols 48 8088 1)
 (strings 32 20784 4357)
 (string-bytes 1 680668)
 (vectors 16 13082)
 (vector-slots 8 220094 8657)
 (floats 8 27 13)
 (intervals 56 213 0)
 (buffers 984 11))
Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Emacs - Bugs mailing list
Yu Han Quek <[hidden email]> writes:

> The following function gives an incorrect result when compiled natively
> with comp-speed >= 2.
>
> ;; -*- lexical-binding: t; -*-
>
> (defun native-comp-bug (body)
>   (let ((x (car body))
>         (y (cadr body)))
>     (unless (and
>              (natnump x)
>              (natnump y)
>              (<= x y))
>       (error ""))
>     x))

Cool thanks for the reduced reproducer!

I'll have a in the coming days.

  Andrea



Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Emacs - Bugs mailing list
Andrea Corallo via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <[hidden email]> writes:

> Yu Han Quek <[hidden email]> writes:
>
>> The following function gives an incorrect result when compiled natively
>> with comp-speed >= 2.
>>
>> ;; -*- lexical-binding: t; -*-
>>
>> (defun native-comp-bug (body)
>>   (let ((x (car body))
>>         (y (cadr body)))
>>     (unless (and
>>              (natnump x)
>>              (natnump y)
>>              (<= x y))
>>       (error ""))
>>     x))
>
> Cool thanks for the reduced reproducer!
>
> I'll have a in the coming days.
>
>   Andrea

Ok here a slightly simplified reproducer:

;; -*- lexical-binding: t; -*-
(defun native-comp-bug (x y)
  (if (and (natnump x)
           (natnump y)
           (<= x y))
      x
    (error "")))

The following constrain insn clearly resolves in the wrong way:

(assume #(mvar 79140560 0 (integer 0 0)) (<= #(mvar 79118120 0 (integer 0 *)) #(mvar 79118988 3 (integer 0 *))))

Will come-up with a fix.

  Andrea



Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Pip Cet
On Tue, Feb 16, 2021 at 9:44 PM Andrea Corallo wrote:
> The following constrain insn clearly resolves in the wrong way:
>
> (assume #(mvar 79140560 0 (integer 0 0)) (<= #(mvar 79118120 0 (integer 0 *)) #(mvar 79118988 3 (integer 0 *))))
>
> Will come-up with a fix.

My guess is that this code in comp-cstr.el:

(defun comp-cstr-<= (dst old-dst src)
  "Constraint DST being <= than SRC.
SRC can be either a comp-cstr or an integer."
  (with-comp-cstr-accessors
    (let ((ext-range
           (if (integerp src)
               `((- . ,src))
             (when-let* ((range (range src))
                         (low (comp-cstr-smallest-in-range range))
                         (okay (integerp low)))
               `((- . ,low))))))
      (comp-cstr-set-cmp-range dst old-dst ext-range))))

should use comp-cstr-greatest-in-range rather than
comp-cstr-smallest-in-range. Same for comp-cstr-<. Analogous for
comp-cstr->, comp-cstr->=.

(If A's possible values are {3, 4, 5}, and B is <= A, we can only
conclude B <= 5, not that B <= 3, since A = 5, B = 5 is a solution to
the constraint problem).



Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Emacs - Bugs mailing list
Pip Cet <[hidden email]> writes:

> On Tue, Feb 16, 2021 at 9:44 PM Andrea Corallo wrote:
>> The following constrain insn clearly resolves in the wrong way:
>>
>> (assume #(mvar 79140560 0 (integer 0 0)) (<= #(mvar 79118120 0 (integer 0 *)) #(mvar 79118988 3 (integer 0 *))))
>>
>> Will come-up with a fix.
>
> My guess is that this code in comp-cstr.el:
>
> (defun comp-cstr-<= (dst old-dst src)
>   "Constraint DST being <= than SRC.
> SRC can be either a comp-cstr or an integer."
>   (with-comp-cstr-accessors
>     (let ((ext-range
>            (if (integerp src)
>                `((- . ,src))
>              (when-let* ((range (range src))
>                          (low (comp-cstr-smallest-in-range range))
>                          (okay (integerp low)))
>                `((- . ,low))))))
>       (comp-cstr-set-cmp-range dst old-dst ext-range))))
>
> should use comp-cstr-greatest-in-range rather than
> comp-cstr-smallest-in-range. Same for comp-cstr-<. Analogous for
> comp-cstr->, comp-cstr->=.
>
> (If A's possible values are {3, 4, 5}, and B is <= A, we can only
> conclude B <= 5, not that B <= 3, since A = 5, B = 5 is a solution to
> the constraint problem).

Yep, Pip spot on.  Small oversight from me... :D

1fe5994bcb should fix.

Yu Han Quek would you mind to confirm rx now works as excpected for you?

Thanks

  Andrea



Reply | Threaded
Open this post in threaded view
|

bug#46540: 28.0.50; Native-comp optimization bug

Emacs - Bugs mailing list
[re-adding the bug in Cc]

Yu Han Quek <[hidden email]> writes:

[...]

>  Yep, Pip spot on.  Small oversight from me... :D
>
>  1fe5994bcb should fix.
>
>  Yu Han Quek would you mind to confirm rx now works as excpected for you?
>
>  Thanks
>
>    Andrea
>
> Thanks for the quick fix! I can confirm that the rx issue I ran into (rx (repeat 4 5 ?a)) now works as expected :)

Nice, closing then.

Thanks for reporting and testing!

  Andrea