Skip to content
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.

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

Sub-optimal codegen for types containing float+ZST #38269

Closed
iliekturtles opened this issue Dec 9, 2016 · 4 comments
Closed

Sub-optimal codegen for types containing float+ZST #38269

iliekturtles opened this issue Dec 9, 2016 · 4 comments
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@iliekturtles
Copy link
Contributor

Similar to #32031 a struct P<D> { value: f32, PhantomData<D>, } is mapped to i32 instead of f32 in the resulting llvm-ir output. Based on IRC discussions with @rkruppe the issue is likely caused by the inclusion of any zero-sized types in the struct.

A minimal example is available at https://github.com/iliekturtles/llvm-opt/tree/fa05ddb4fb869e492e04c93e04fa0e939fb2fb49 and includes the offending llvm-ir and asm output.

@nox
Copy link
Contributor

nox commented Apr 2, 2018

@eddyb This seems fixed to me.

#[inline(never)]
pub fn calc(v: P<X>) -> P<X> {
    let a = P::<X>::new(v.get_value() * 1.234_f32);
    let b = P::<X>::new(5.0_f32);

    a / b
}
define float @example::calc(float) unnamed_addr #2 !dbg !22 {
  %1 = fmul float %0, 0x3FF3BE76C0000000, !dbg !24
  %2 = fdiv float %1, 5.000000e+00, !dbg !25
  ret float %2, !dbg !31
}

https://godbolt.org/g/sSMhDj

Cc @rust-lang/wg-codegen

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Apr 2, 2018

Yeah this should be fixed for all ZSTs (edit: and even multiple ZST fields), @eddyb told me we do fully general newtype unwrapping for scalars and vectors now when I implemented repr(transparent) (and indeed the implementation depends on it happening, so if that was wrong then we have bigger problems than codegen quality).

@iliekturtles
Copy link
Contributor Author

Just tested this with rustc 1.25.0-beta.13 (da81b8884 2018-03-24) and got good looking llvm-ir and asm output.

@oli-obk oli-obk closed this as completed Apr 2, 2018
@zesterer
Copy link
Contributor

zesterer commented Aug 7, 2018

The README could do with updating to reflect the fact that this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

7 participants