bug: EQUAL optimization is too agressive

Bug #722734 reported by Roman Marynchak
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
High
Unassigned

Bug Description

Consider this session in REPL:

[roman@myhost ~]$ sbcl
This is SBCL 1.0.46.10, an implementation of ANSI Common Lisp.
More information about SBCL is available at <http://www.sbcl.org/>.

SBCL is free software, provided as is, with absolutely no warranty.
It is mostly in the public domain; some portions are provided under
BSD-style licenses. See the CREDITS and COPYING files in the
distribution for more information.
* (declaim (optimize (debug 3) (speed 0) (safety 3)))

* (defun foo ()
    (equal (list a b c) (make-array 9)))

; in: LAMBDA NIL
; (LIST A B C)
;
; caught WARNING:
; undefined variable: A
;
; caught WARNING:
; undefined variable: B
;
; caught WARNING:
; undefined variable: C
;
; compilation unit finished
; Undefined variables:
; A B C
; caught 3 WARNING conditions

FOO
* (foo)

NIL
* (sb-c::describe-compiler-policy)

  Basic qualities:
COMPILATION-SPEED = 1
DEBUG = 3
SAFETY = 3
SPACE = 1
SPEED = 0

I expect to see the error message about A, B and C being undefined. Instead, the call returns NIL even with the most liberal optimization policy. LW, CLISP and Clozure signal the error for this code.

Tags: compiler-ir1
Changed in sbcl:
importance: Undecided → High
status: New → Triaged
Revision history for this message
Roman Marynchak (roman-marynchak) wrote :

Additional info: EQ and EQL have the same problem, but EQUALP works fine.

Changed in sbcl:
assignee: nobody → Roman Marynchak (roman-marynchak)
status: Triaged → In Progress
Revision history for this message
Roman Marynchak (roman-marynchak) wrote :

I suggest using SAFETY 3 as a barrier for such optimizations (do not guess anything about equality of the things based on their type). Do you think that other optimization policy is better to turn them on/off? Please share your opinion, in case you support the idea, I will add tests and supply a full patch instead of this diff.

Thank you.

tags: added: compiler-ir1 review
removed: compiler
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

The problem isn't really in the equality optimizations. It's fine for the compiler to optimize them away.

What's wrong is that the compiler is also optimizing away references to the global variables -- which can be unbound.

If you look at IR1-CONVERT-VAR, you will see that it is already special casing for this eventuality when the value is unused. But as you discovered, being used is not sufficient as the destination may be deleted later. We should to convert all global variables that are not ALWAYS-BOUND using SYMBOL-VALUE/SYMBOL-GLOBAL-VALUE.

tags: removed: review
Revision history for this message
Roman Marynchak (roman-marynchak) wrote :

Another patch, based on the above advices.

Note that it makes one of stepper tests unhappy with SYMBOL-VALUE, but the workaround is present in the patch itself.

The last question to answer is whether there is performance penalty after the patch is applied.

tags: added: review
Changed in sbcl:
status: In Progress → Confirmed
assignee: Roman Marynchak (roman-marynchak) → nobody
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

Thank you. Fix based on your patch committed as 1.0.48.14: STEP-FORM-P needed adjustment for consistent treatment of global symbol references.

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
status: Confirmed → In Progress
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Fix Committed
tags: removed: review
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.