rust-lang/rust

Sub-optimal codegen for types containing float+ZST

iliekturtles opened this issue · 4 comments

iliekturtles commented

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 commented

@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 commented

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 commented

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

zesterer commented

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