Make exp, exp2, log, log2, log10 generic - #160989
Conversation
|
cc @bjorn3 Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr
cc @rust-lang/miri
|
|
I suspect it would be better to submit 295e7a3 to the |
This comment has been minimized.
This comment has been minimized.
d98ca71 to
b3a59f6
Compare
|
cc @rust-lang/clippy |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| CValue::by_val(codegen_f16_f128::fma_f16(fx, args[0], args[1], args[2]), layout) | ||
| } | ||
| let val = match intrinsic { | ||
| sym::fmaf16 | sym::fmuladdf16 => codegen_f16_f128::fma_f16(fx, args[0], args[1], args[2]), |
There was a problem hiding this comment.
This conflicts with rust-lang/rustc_codegen_cranelift#1675. I'm planning to do a sync soon.
There was a problem hiding this comment.
oh then we can hold off from merging this until you've synced, i dont mind
| const _: () = { | ||
| type $generic = $concrete; |
There was a problem hiding this comment.
If these are already in a module, why do you put them additionally into an anonymous const?
There was a problem hiding this comment.
because we need to bind T for that impl, since the generated trait impls look like this:
impl Dispatch<f32> for f32 {
#[inline]
fn dispatch(x: T) -> T { libm::likely_available::expf(x) }
}There was a problem hiding this comment.
Hm... I see, clever. The generic parameter in Dispatch is also part of this trick to keep the T working.
That's definitely worth a comment or three to explain what is going on.
| | | ||
| help: consider importing this function | ||
| | | ||
| LL + use std::intrinsics::log; |
There was a problem hiding this comment.
This seems like a bad side-effect. Do we have an open issue to track the problem that apparently we recommend importing unstable functions?
There was a problem hiding this comment.
not that i could fine, but not sure. agreed this seems pretty bad though, especially for a name as common as log
There was a problem hiding this comment.
Hm, something odd is happening -- I don't get similar suggestions for existing intrinsics
Playground
The comment was not misplaced, it was placed exactly where it should be. rustfmt just doesn't like comments in some places so we have to move the comment into a suboptimal place. :( |
yes sorry, i meant misplaced from rustfmt's perspective ^^ i agree it's not ideal there, it just seemed clearer than inside the match arm:/ |
Rebased and smaller version of #153934
Following
fabs, make theexp,exp2,log,log2andlog10intrinsics generic over the float type, rather than having four variants per float type.The first two commits are purely stylistic:
compiler/rustc_codegen_llvm/src/intrinsic.rsthat causedx fmtto give upThe last commit actually makes them generic! Most code is a bit simpler, and this will also hopefully simplify adding support for these intrinsics for the future
bf16type :)Unfortunately both GCC and Cranelift backend changes are a bit churny. Their code is a bit, opaque, to put it kindly, and I didn't want to refactor those here.
r? @folkertdev
cc @RalfJung
Disclosure: I used an LLM to figure out the macro code.