bug: EQUAL optimization is too agressive

Bug #722734 reported by Roman Marynchak on 2011-02-21
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

Consider this session in REPL:

[roman@myhost ~]$ sbcl
This is SBCL, 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)))

; (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)

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

  Basic qualities:

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.

Changed in sbcl:
importance: Undecided → High
status: New → Triaged

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

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
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

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
Nikodemus Siivola (nikodemus) wrote :

Thank you. Fix based on your patch committed as 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  Edit
Everyone can see this information.

Other bug subscribers