Skip to content

Commit d689928

Browse files
committed
Relax quickr gfortran vectorizer cost model
Default gfortran builds in quickr were picking up GCC's conservative -O2 vectorizer cost model, which left generated array kernels like the README convolve example on a slow scalar path on Linux. Detect gfortran from R's FC setting and write a temporary Makevars file with: PKG_FFLAGS += -fvect-cost-model=cheap This keeps the change scoped to gfortran-backed quickr builds without changing flang behavior or generated Fortran source. Add compiler tests covering the new Makevars path and update existing compiler-env tests to keep non-gfortran cases explicit.
1 parent 30f99b1 commit d689928

3 files changed

Lines changed: 90 additions & 6 deletions

File tree

R/compiler.R

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,36 @@ quickr_prefer_flang <- function(
171171
FALSE
172172
}
173173

174+
quickr_default_fortran_makevars_lines <- function(
175+
config_value = quickr_r_cmd_config_value
176+
) {
177+
fc <- trimws(config_value("FC"))
178+
if (!nzchar(fc)) {
179+
return(character())
180+
}
181+
182+
compiler <- strsplit(fc, "\\s+")[[1L]][[1L]]
183+
compiler <- basename(compiler)
184+
if (!grepl("^gfortran(?:-[0-9]+)?(?:\\.exe)?$", compiler)) {
185+
return(character())
186+
}
187+
188+
# GCC 12+ uses a very-cheap vectorizer cost model at -O2. That keeps the
189+
# default build conservative for loops like quickr's generated array kernels.
190+
# Relaxing the cost model restores the vectorized code path without changing
191+
# the compiler or generated Fortran source.
192+
"PKG_FFLAGS += -fvect-cost-model=cheap"
193+
}
194+
174195
quickr_fcompiler_env <- function(
175196
build_dir,
176197
which = Sys.which,
177198
system2 = base::system2,
178199
write_lines = writeLines,
179200
sysname = Sys.info()[["sysname"]],
180201
use_openmp = FALSE,
181-
link_flags = character()
202+
link_flags = character(),
203+
config_value = quickr_r_cmd_config_value
182204
) {
183205
stopifnot(is.character(build_dir), length(build_dir) == 1L, nzchar(build_dir))
184206

@@ -234,7 +256,13 @@ quickr_fcompiler_env <- function(
234256
}
235257
}
236258

237-
if (!use_flang && !use_openmp) {
259+
default_makevars_lines <- if (use_flang) {
260+
character()
261+
} else {
262+
quickr_default_fortran_makevars_lines(config_value = config_value)
263+
}
264+
265+
if (!use_flang && !use_openmp && !length(default_makevars_lines)) {
238266
return(character())
239267
}
240268

@@ -249,6 +277,7 @@ quickr_fcompiler_env <- function(
249277
}
250278
)
251279
},
280+
default_makevars_lines,
252281
if (use_openmp) openmp_makevars_lines(),
253282
if (length(link_flags)) {
254283
paste("PKG_LIBS +=", paste(link_flags, collapse = " "))

tests/testthat/test-compiler.R

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,35 @@ test_that("quickr_prefer_flang respects quickr.fortran_compiler", {
8181
expect_false(quickr:::quickr_prefer_flang(sysname = "Darwin"))
8282
})
8383

84+
test_that("quickr_default_fortran_makevars_lines relaxes gfortran cost model", {
85+
expect_equal(
86+
quickr:::quickr_default_fortran_makevars_lines(
87+
config_value = function(name) {
88+
if (identical(name, "FC")) "gfortran -m64" else ""
89+
}
90+
),
91+
"PKG_FFLAGS += -fvect-cost-model=cheap"
92+
)
93+
94+
expect_equal(
95+
quickr:::quickr_default_fortran_makevars_lines(
96+
config_value = function(name) {
97+
if (identical(name, "FC")) "/usr/bin/gfortran-13" else ""
98+
}
99+
),
100+
"PKG_FFLAGS += -fvect-cost-model=cheap"
101+
)
102+
103+
expect_identical(
104+
quickr:::quickr_default_fortran_makevars_lines(
105+
config_value = function(name) {
106+
if (identical(name, "FC")) "flang-new" else ""
107+
}
108+
),
109+
character()
110+
)
111+
})
112+
84113
test_that("quickr_fortran_compiler_option validates values", {
85114
withr::local_options(quickr.fortran_compiler = "auto")
86115
expect_null(quickr:::quickr_fortran_compiler_option())
@@ -130,6 +159,28 @@ test_that("quickr_fcompiler_env writes Makevars when flang is usable", {
130159
expect_true(file.exists(sub("R_MAKEVARS_USER=", "", env, fixed = TRUE)))
131160
})
132161

162+
test_that("quickr_fcompiler_env writes Makevars for default gfortran flags", {
163+
build_dir <- withr::local_tempdir()
164+
165+
withr::local_options(quickr.fortran_compiler = "gfortran")
166+
env <- quickr:::quickr_fcompiler_env(
167+
build_dir = build_dir,
168+
which = function(cmd) "",
169+
system2 = function(...) "",
170+
sysname = "Linux",
171+
config_value = function(name) {
172+
if (identical(name, "FC")) "gfortran -m64" else ""
173+
}
174+
)
175+
176+
expect_true(startsWith(env, "R_MAKEVARS_USER="))
177+
makevars_path <- sub("^R_MAKEVARS_USER=", "", env)
178+
expect_equal(
179+
readLines(makevars_path),
180+
"PKG_FFLAGS += -fvect-cost-model=cheap"
181+
)
182+
})
183+
133184
test_that("quickr_fcompiler_env errors when flang is explicitly requested but unavailable", {
134185
build_dir <- withr::local_tempdir()
135186

@@ -286,7 +337,8 @@ test_that("quickr_fcompiler_env handles flang unavailable for non-explicit reque
286337
build_dir = build_dir,
287338
which = function(x) "",
288339
system2 = function(...) "",
289-
sysname = "Darwin"
340+
sysname = "Darwin",
341+
config_value = function(name) if (identical(name, "FC")) "clang" else ""
290342
)
291343
expect_identical(result, character())
292344
})
@@ -341,7 +393,8 @@ test_that("quickr_fcompiler_env falls back when flang runtime not found non-expl
341393
build_dir = build_dir,
342394
which = function(x) if (x == "flang-new") flang else "",
343395
system2 = function(...) "",
344-
sysname = "Darwin"
396+
sysname = "Darwin",
397+
config_value = function(name) if (identical(name, "FC")) "clang" else ""
345398
)
346399
# Should fall back to character() since runtime not found and not explicit
347400
expect_identical(result, character())

tests/testthat/test-flang-preference.R

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ test_that("quickr_fcompiler_env returns empty when disabled or unavailable", {
6363
expect_equal(
6464
quickr:::quickr_fcompiler_env(
6565
build_dir,
66-
which = which
66+
which = which,
67+
config_value = function(name) if (identical(name, "FC")) "clang" else ""
6768
),
6869
character()
6970
)
@@ -72,7 +73,8 @@ test_that("quickr_fcompiler_env returns empty when disabled or unavailable", {
7273
expect_equal(
7374
quickr:::quickr_fcompiler_env(
7475
build_dir,
75-
which = which
76+
which = which,
77+
config_value = function(name) if (identical(name, "FC")) "clang" else ""
7678
),
7779
character()
7880
)

0 commit comments

Comments
 (0)