bogus type-error from array access optimization using negative index and positive offset

[Nikodemus Siivola]

x86 and x86-64 only, I think.

Test case:

(defun oops ()
  #+nil
  (declare (optimize (safety 0)))
  (let ((table (make-array 7 :element-type 'fixnum :initial-contents '(0 1 2 3 4 5 6))))
    (loop for n from -3 upto 3
          do (print (aref table (+ 3 n))))))

signals an error unless SAFETY 0:

  The value -3 is not of type (MOD 1152921504606846973).

With SAFETY 0 prints out numbers 0-6, demonstrating that the computed offsets are correct.

The deftransform for data-vector-ref-with-offset calls FOLD-INDEX-ADDRESSING, which realizes that +3 means a constant offset, and the call is transformed into one using two offsets with the +3 as one.

However, somewhere we end up generating a type-check for the other offset, asserting that it is a regular array index -- whereas due to the +3 the lower end of the range has been extended to -3. I suspect that that typecheck is wholly pointless, actually.

[Nikodemus Siivola (nikodemus)]

This is what the fix looks like:

diff --git a/src/compiler/fndb.lisp b/src/compiler/fndb.lisp
index dfb0dcd..5991492 100644
--- a/src/compiler/fndb.lisp
+++ b/src/compiler/fndb.lisp
@@ -1448,7 +1448,7 @@
 (defknown %check-bound (array index fixnum) index (movable foldable flushable))
 (defknown data-vector-ref (simple-array index) t
   (foldable explicit-check always-translatable))
-(defknown data-vector-ref-with-offset (simple-array index fixnum) t
+(defknown data-vector-ref-with-offset (simple-array fixnum fixnum) t
   (foldable explicit-check always-translatable))
 (defknown data-vector-set (array index t) t
   (unsafe explicit-check always-translatable))

Merging after freeze -- though I have good confidence in this fix, the problem has been there for a long time and no-one has yet to report it.

[Nikodemus Siivola (nikodemus)]

Actually, it is still fairly simple to construct cases that fail. Consider:

 (aref v (- x #.(1+ most-positive-fixnum)))

[Nikodemus Siivola (nikodemus)]

Ignore that last comment -- that doesn't end up on the DATA-VECTOR-REF-WITH-OFFSET path at all.

[Nikodemus Siivola (nikodemus)]

Fixed in SBCL 1.0.42.2.

[Nikodemus Siivola (nikodemus)]

Feh, backed out from CVS as of 1.0.42.5:

* The VOPs need to accept negative indexes as well, of course -- which requires not only changing the type from positive-fixnum to tagged-num, but auditing them for sign handling.

* Same issue applies to DATA-VECTOR-SET-WITH-OFFSET too, of course.

[Paul Khuong (pvk)]

Fixed in 1.1.9 (a3b10e4, Handle (aref v (+ i k)), with i negative).