reactorlabs / rir

GNU General Public License v2.0
62 stars 18 forks source link

Wrong optimization in Extract2_1D for VECSXP #1253

Closed fikovnik closed 11 months ago

fikovnik commented 11 months ago

In lower_function_llvm.cpp, the compilation of the fast path of Extract2_1D is wrong for generic scalar vectors:

A scalar vector:

> a <- list(1)
> str(a)
List of 1
 $ : num 1

accessed by a[[1L]] should return its first element

> str(a[[1L]])
 num 1

But in the current implementation, it returns itself:

                    auto res0 =
                        extract->vec()->type.isScalar()
                            ? vector
                            : accessVector(vector, index, extract->vec()->type);

This happens in either OSR or deopt where the ContinuationContext is created from the actual types of the SEXPs in the current environment.

There are two ways to fix:

  1. Do not consider scalars for VECSXP (in pir/type.cpp)
  2. Fix the fastpath

The latter makes more sense.

fikovnik commented 11 months ago

A reproduction from @JanJecmen :

forceosr <- rir.compile(function(x) x)

fail <- function(x) {
    x <- strsplit(x, "[[:space:]]*=[[:space:]]*")
    x <- forceosr(x)
    x[[1L]]
}

test <- rir.compile(function() {
    fail("a")
    rir.markFunction(forceosr, Reopt = TRUE)
    fail("a")
})

# For a scalar generic vector (i.e., a list), our Extract2_1D returned
# the list itself and not its first element (that only works for normal vectors)
stopifnot(is.character(test()))