Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Intercept calls to libm - #264

Merged
mergify[bot] merged 3 commits into
mainfrom
intercept-libm
Nov 20, 2020
Merged

mergify[bot] merged 3 commits into
mainfrom
intercept-libm

Conversation

@khyperia

Copy link
Copy Markdown
Contributor

Matching on symbol names is probably not the right thing to do here, but I'm not sure what other options there are (Instance's Display impl?).

This corresponds to a change to our glam PR to use libm instead of MathExt. Currently that's at EmbarkStudios/glam-rs@709ac6d and not merged into the spirv PR branch, I'll do so once this is in.

@khyperia
khyperia requested a review from eddyb November 20, 2020 08:54

@eddyb eddyb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, modulo the matching on mangled symbol names.

@eddyb

eddyb commented Nov 20, 2020 •

Copy link
Copy Markdown
Contributor

(Moved here because GitHub really wanted to show 100 lines of the diff, when it was a diff comment, oops)

Looks like the functions are all鹿 math::$fn::$fn, so it should be possible to grab the function name (via tcx.item_name(def_id), or tcx.def_path(def_id) if you want the entire path at once), and check that the rest of the path conforms.

鹿exceptions:

// math::erf::erfc
// math::erff::erfcf
// math::j0::y0
// math::j0f::y0f
// math::j1::y1
// math::j1f::y1f
// math::jn::yn
// math::jnf::ynf

@eddyb eddyb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r=me with the commented out line removed

really_unsafe_ignore_bitcasts: Symbol,
attributes: HashMap<Symbol, SpirvAttribute>,
execution_modes: HashMap<Symbol, (ExecutionMode, ExecutionModeExtraDim)>,
pub libm_intrinsics: HashMap<Symbol, libm_intrinsics::LibmIntrinsic>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost suggested this (but forgot), nice!

Comment thread crates/rustc_codegen_spirv/src/codegen_cx/declare.rs Outdated
@khyperia
khyperia requested a review from eddyb November 20, 2020 18:17
@mergify
mergify Bot merged commit a92d76a into main Nov 20, 2020
@mergify
mergify Bot deleted the intercept-libm branch November 20, 2020 18:36
@fu5ha

fu5ha commented Nov 22, 2020

Copy link
Copy Markdown
Member

馃帀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants