Skip to content

Commit cc8def7

Browse files
committed
Preserve input mode in t() and diag()
R's t() and diag() preserve their input's type, but the handlers unconditionally cast to double: t(m) and diag(m) on an integer matrix returned doubles, and the constructor forms diag(x) / diag(x, nrow, ncol) lost typeof(x) too. Only the identity forms (diag(n), diag(nrow = n)) are double in R, which is what quickr already emits for them. Drop the maybe_cast_double() calls and carry the input mode through the result Variable, the hoisted temporaries, and the zero fill in diag_matrix(). can_use_output() gains a `mode` argument (default "double"; all other callers unchanged) so an in-place destination is only used when its declared mode matches, and infer_dest_diag() reports the input's mode instead of hard-coding double -- a double-inferred dest would have mislabeled an integer result's declaration. The transpose casts in unwrap_transpose_arg() stay, now with a comment saying why: that path only feeds matrix products, which always return double in R.
1 parent ef58aaf commit cc8def7

6 files changed

Lines changed: 182 additions & 16 deletions

File tree

R/r2f-matrix-blas.R

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -261,12 +261,13 @@ can_use_output <- function(
261261
input_names = character(),
262262
expected_dims = NULL,
263263
context,
264-
allow_alias = character()
264+
allow_alias = character(),
265+
mode = "double"
265266
) {
266267
if (is.null(dest)) {
267268
return(FALSE)
268269
}
269-
if (!identical(dest@mode, "double")) {
270+
if (!identical(dest@mode, mode)) {
270271
return(FALSE)
271272
}
272273
assert_dest_dims_compatible(dest, expected_dims, context)
@@ -1190,7 +1191,8 @@ lapack_chol2inv <- function(
11901191
diag_extract <- function(x, scope, hoist, dest = NULL, context = "diag") {
11911192
assert_hoist_env(hoist)
11921193

1193-
x <- maybe_cast_double(x)
1194+
# R's diag(<matrix>) preserves the input mode; the copy loop is
1195+
# mode-agnostic.
11941196
assert_rank2_matrix(x, paste0(context, " expects a matrix input"))
11951197

11961198
x_dims <- matrix_dims(x)
@@ -1204,14 +1206,15 @@ diag_extract <- function(x, scope, hoist, dest = NULL, context = "diag") {
12041206
dest,
12051207
input_names = x_name,
12061208
expected_dims = list(diag_len),
1207-
context = context
1209+
context = context,
1210+
mode = x@value@mode
12081211
)
12091212
) {
12101213
out_var <- dest
12111214
out_name <- dest@name
12121215
writes_to_dest <- TRUE
12131216
} else {
1214-
out_var <- hoist$declare_tmp(mode = "double", dims = list(diag_len))
1217+
out_var <- hoist$declare_tmp(mode = x@value@mode, dims = list(diag_len))
12151218
out_name <- out_var@name
12161219
}
12171220

@@ -1241,9 +1244,20 @@ diag_matrix <- function(
12411244
) {
12421245
assert_hoist_env(hoist)
12431246

1244-
x <- maybe_cast_double(x)
1247+
# R's diag(x, ...) preserves typeof(x). The identity-matrix callers pass
1248+
# a synthesized 1.0_c_double, which keeps diag(n) double, as in R.
12451249
assert_rank_leq1(x, paste0(context, " expects a vector or scalar input"))
12461250

1251+
mode <- x@value@mode
1252+
zero <- switch(
1253+
mode,
1254+
double = "0.0_c_double",
1255+
integer = "0_c_int",
1256+
logical = ".false.",
1257+
complex = "(0.0_c_double, 0.0_c_double)",
1258+
stop(context, " does not support mode ", mode, call. = FALSE)
1259+
)
1260+
12471261
diag_len <- diag_length_expr(nrow, ncol, context)
12481262
x_scalar <- passes_as_scalar(x@value)
12491263
x_len <- if (x_scalar) 1L else dim_or_one(x, 1L)
@@ -1256,18 +1270,19 @@ diag_matrix <- function(
12561270
dest,
12571271
input_names = x_name,
12581272
expected_dims = list(nrow, ncol),
1259-
context = context
1273+
context = context,
1274+
mode = mode
12601275
)
12611276
) {
12621277
out_var <- dest
12631278
out_name <- dest@name
12641279
writes_to_dest <- TRUE
12651280
} else {
1266-
out_var <- hoist$declare_tmp(mode = "double", dims = list(nrow, ncol))
1281+
out_var <- hoist$declare_tmp(mode = mode, dims = list(nrow, ncol))
12671282
out_name <- out_var@name
12681283
}
12691284

1270-
hoist$emit(glue("{out_name} = 0.0_c_double"))
1285+
hoist$emit(glue("{out_name} = {zero}"))
12711286

12721287
idx_i <- hoist$declare_tmp(mode = "integer", dims = NULL)
12731288
value_expr <- if (x_scalar) {

R/r2f-matrix-infer.R

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,14 @@ infer_dest_diag <- function(args, scope) {
311311

312312
# Case: x is a matrix -> extract diagonal (returns vector)
313313
if (!is.null(x) && x@rank == 2L) {
314+
if (is.null(x@mode)) {
315+
return(NULL)
316+
}
314317
x_dims <- matrix_dims_var(x)
315318
diag_len <- diag_length_expr(x_dims$rows, x_dims$cols, "diag")
316-
return(Variable("double", list(diag_len)))
319+
# diag_extract() preserves x's mode; a double-inferred dest would
320+
# mislabel an integer diagonal.
321+
return(Variable(x@mode, list(diag_len)))
317322
}
318323

319324
# Case: x is a scalar literal (identity matrix of that size)
@@ -330,7 +335,8 @@ infer_dest_diag <- function(args, scope) {
330335
}
331336

332337
# Case: x is a vector or scalar, construct diagonal matrix
333-
if (!is.null(x) && x@rank <= 1L) {
338+
# (diag_matrix() preserves x's mode, matching R)
339+
if (!is.null(x) && x@rank <= 1L && !is.null(x@mode)) {
334340
if (has_nrow || has_ncol) {
335341
nrow <- if (has_nrow) infer_size(nrow_arg, scope) else NULL
336342
ncol <- if (has_ncol) infer_size(ncol_arg, scope) else NULL
@@ -343,12 +349,12 @@ infer_dest_diag <- function(args, scope) {
343349
if (is.null(ncol)) {
344350
ncol <- nrow
345351
}
346-
return(Variable("double", list(nrow, ncol)))
352+
return(Variable(x@mode, list(nrow, ncol)))
347353
}
348354
# No nrow/ncol: square matrix from vector length
349355
if (x@rank == 1L) {
350356
len <- var_dim_or_one(x, 1L)
351-
return(Variable("double", list(len, len)))
357+
return(Variable(x@mode, list(len, len)))
352358
}
353359
}
354360

R/r2f-matrix-parse.R

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# Matrix parsing helpers
22

33
# Unwrap t() calls to infer transpose flags and normalize scalars/vectors.
4+
# The double casts here are correct and intentional: this path only feeds
5+
# matrix-multiplication handlers (%*%, crossprod, ...), and R's matrix
6+
# products always return double. The standalone t() handler preserves mode.
47
unwrap_transpose_arg <- function(arg, scope, ..., hoist) {
58
arg_unwrapped <- unwrap_parens(arg)
69
if (is_call(arg_unwrapped, quote(t)) && length(arg_unwrapped) == 2L) {

R/r2f-matrix.R

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,14 @@ register_r2f_handler(
137137
r2f_handlers[["t"]] <- function(args, scope, ..., hoist = NULL) {
138138
stopifnot(length(args) == 1L)
139139
x <- r2f(args[[1L]], scope, ..., hoist = hoist)
140-
x <- maybe_cast_double(x)
140+
# R's t() preserves the input mode. (The transposes feeding matrix
141+
# multiplication go through unwrap_transpose_arg(), not this handler.)
141142
if (x@value@rank == 2) {
142-
val <- Variable("double", list(x@value@dims[[2]], x@value@dims[[1]]))
143+
val <- Variable(x@value@mode, list(x@value@dims[[2]], x@value@dims[[1]]))
143144
return(Fortran(glue("transpose({x})"), val))
144145
} else if (x@value@rank == 1) {
145146
len <- x@value@dims[[1]]
146-
val <- Variable("double", list(1L, len))
147+
val <- Variable(x@value@mode, list(1L, len))
147148
return(Fortran(glue("reshape({x}, [1, int({len})])"), val))
148149
} else if (x@value@rank == 0) {
149150
return(x)

tests/testthat/_snaps/matrix.md

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,79 @@
231231
return out;
232232
}
233233

234+
# t() and diag() preserve integer mode
235+
236+
Code
237+
fn
238+
Output
239+
function(m) {
240+
declare(type(m = integer(3, 3)))
241+
out <- diag(m)
242+
out
243+
}
244+
<environment: 0x0>
245+
Code
246+
cat(fsub)
247+
Output
248+
subroutine fn(m, out) bind(c)
249+
use iso_c_binding, only: c_int
250+
implicit none
251+
252+
! manifest start
253+
! args
254+
integer(c_int), intent(in) :: m(3, 3)
255+
integer(c_int), intent(out) :: out(3)
256+
! manifest end
257+
258+
259+
block
260+
integer(c_int) :: btmp1_
261+
262+
do btmp1_ = 1_c_int, int(3, kind=c_int)
263+
out(btmp1_) = m(btmp1_, btmp1_)
264+
end do
265+
end block
266+
end subroutine
267+
Code
268+
cat(cwrapper)
269+
Output
270+
#define R_NO_REMAP
271+
#include <R.h>
272+
#include <Rinternals.h>
273+
274+
275+
extern void fn(const int* const m__, int* const out__);
276+
277+
SEXP fn_(SEXP _args) {
278+
// m
279+
_args = CDR(_args);
280+
SEXP m = CAR(_args);
281+
if (TYPEOF(m) != INTSXP) {
282+
Rf_error("typeof(m) must be 'integer', not '%s'", Rf_type2char(TYPEOF(m)));
283+
}
284+
const int* const m__ = INTEGER(m);
285+
const int* const m__dim_ = ({
286+
SEXP dim_ = Rf_getAttrib(m, R_DimSymbol);
287+
if (Rf_length(dim_) != 2) Rf_error(
288+
"m must be a 2D-array, but length(dim(m)) is %i",
289+
(int) Rf_length(dim_));
290+
INTEGER(dim_);});
291+
const int m__dim_1_ = m__dim_[0];
292+
const int m__dim_2_ = m__dim_[1];
293+
294+
if (m__dim_1_ != 3)
295+
Rf_error("dim(m)[1] must be 3, not %0.f",
296+
(double)m__dim_1_);
297+
if (m__dim_2_ != 3)
298+
Rf_error("dim(m)[2] must be 3, not %0.f",
299+
(double)m__dim_2_);
300+
const R_xlen_t out__len_ = 3;
301+
SEXP out = PROTECT(Rf_allocVector(INTSXP, out__len_));
302+
int* out__ = INTEGER(out);
303+
304+
fn(m__, out__);
305+
306+
UNPROTECT(1);
307+
return out;
308+
}
309+

tests/testthat/test-matrix.R

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,68 @@ test_that("indexing function like transposed expressions hoists temporaries that
298298
x <- matrix(runif(25), 5, 5)
299299
expect_quick_identical(fn, list(x = x))
300300
})
301+
302+
test_that("t() and diag() preserve integer mode", {
303+
tfn <- function(m) {
304+
declare(type(m = integer(2, 3)))
305+
t(m)
306+
}
307+
expect_quick_equal(tfn, list(matrix(1:6, 2, 3))) # typeof integer
308+
309+
tvec <- function(x) {
310+
declare(type(x = integer(3)))
311+
t(x)
312+
}
313+
expect_quick_equal(tvec, list(1:3)) # R: 1 x 3 integer matrix
314+
315+
dfn <- function(m) {
316+
declare(type(m = integer(3, 3)))
317+
diag(m)
318+
}
319+
expect_quick_equal(dfn, list(matrix(1:9, 3, 3))) # R: integer vector
320+
321+
# same, through the inferred-destination path (out <- diag(m))
322+
dfn2 <- function(m) {
323+
declare(type(m = integer(3, 3)))
324+
out <- diag(m)
325+
out
326+
}
327+
expect_translation_snapshots(dfn2)
328+
expect_quick_equal(dfn2, list(matrix(1:9, 3, 3)))
329+
330+
dvec <- function(x) {
331+
declare(type(x = integer(3)))
332+
diag(x)
333+
}
334+
expect_quick_equal(dvec, list(1:3)) # R: integer matrix
335+
336+
# x recycled along the diagonal of a non-square result
337+
drect <- function(x) {
338+
declare(type(x = integer(2)))
339+
diag(x, 3L, 4L)
340+
}
341+
expect_quick_equal(drect, list(1:2))
342+
343+
# identity forms stay double, as in R
344+
dident <- function() {
345+
out <- diag(3L)
346+
out
347+
}
348+
expect_quick_equal(dident, list())
349+
})
350+
351+
test_that("t() and diag() preserve logical mode", {
352+
m <- matrix(c(TRUE, FALSE, TRUE, TRUE), 2, 2)
353+
354+
tfn <- function(m) {
355+
declare(type(m = logical(2, 2)))
356+
t(m)
357+
}
358+
expect_quick_equal(tfn, list(m))
359+
360+
dfn <- function(m) {
361+
declare(type(m = logical(2, 2)))
362+
diag(m)
363+
}
364+
expect_quick_equal(dfn, list(m))
365+
})

0 commit comments

Comments
 (0)