Skip to content

fix blis dynamic lib name, avoid bli_init output at runtime #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bung87 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix blis dynamic lib name, avoid bli_init output at runtime #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/arraymancer/tensor/backend/blis.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
when defined(blis):
static: echo "--USING BLIS--"
include ./blis_api
echo "Blis initialization status: " & $bli_init()
discard bli_init()
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

By discarding the return value of bli_init(), you are ignoring potential initialization errors. This could lead to silent failures at runtime if BLIS fails to initialize correctly. It's better to check the return code and raise an exception on failure to ensure the library is properly initialized before use.

let err = bli_init()
doAssert err == BLIS_SUCCESS, "BLIS initialization failed with error: " & $err

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialization error silently discarded in all build modes

bli_init() returns a BlisError that can signal BLIS_FAILURE (-2). The existing quit_blis proc correctly logs the result under defined(debug) and only discards in release mode. The new discard bli_init() deviates from this pattern and drops the return value unconditionally — so a failed initialization will produce no warning even in debug builds, leading to confusing downstream errors.

Consider mirroring the same debug/release pattern already used for bli_finalize():

Suggested change
discard bli_init()
when defined(debug):
echo "Blis initialization status: " & $bli_init()
else:
discard bli_init()


proc quit_blis() {.noconv.}=
when defined(debug):
Expand Down
11 changes: 9 additions & 2 deletions src/arraymancer/tensor/backend/blis_api.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,19 @@ type

#################################################

when defined(windows):
const blisPrefix = ""
else:
const blisPrefix = "lib"

when defined(windows):
const blisSuffix = ".dll"
elif defined(macosx):
const blisSuffix = ".dylib"
else:
const blisSuffix = ".so" #MacOS & Linux
const blisSuffix = ".so"
Comment on lines +124 to +134
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These two when blocks can be combined into a single, more concise expression using a tuple assignment. This improves readability and reduces code duplication.

const (blisPrefix, blisSuffix) = when defined(windows):
  ("", ".dll")
elif defined(macosx):
  ("lib", ".dylib")
else: # Linux and others
  ("lib", ".so")


const libblis = "libblis" & blisSuffix
const libblis = blisPrefix & "blis" & blisSuffix

proc bli_init(): BlisError {.importc: "bli_init", dynlib: libblis.}
proc bli_finalize(): BlisError {.importc: "bli_finalize", dynlib: libblis.}
Expand Down
Loading