[PATCH] D30416: [InstCombine] Redo reduceLoadOpStoreWidth in instcombine for bitfield store optimization.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> What kind of optimizations are you worried about?

Primarily, I'm worried this could end up blocking GVN and DSE for a series of bitfield operations; it's much easier to reason about loads and stores with the same size/address, as opposed to overlapping operations.



================
Comment at: test/Transforms/InstCombine/bitfield-store.ll:89
+; a4.f1 = n;
+; The bitfield store cannot be shrinked because the field is not 8/16/32 bits.
+; CHECK-LABEL: @test4(
----------------
wmi wrote:
> efriedma wrote:
> > I'm not following; I'm pretty sure we could shrink the store if we wanted to.  Or do you mean it isn't profitable to shrink it?
> There is a correctness issue. If we shrink the store and generate store like below:
> 
>   %t0 = trunc i32 %n to i13
>   store i13 %t0, i13* bitcast (%class.A4* @a4 to i13*), align 8
>   ret void
> 
> llvm will generate code:
>         andl    $8191, %edi             # imm = 0x1FFF
>         movw    %di, a4(%rip)
>         retq
> 
> and a4.f2 will be overwritten.
You could slightly tweak your optimization so it doesn't run into that problem: instead of truncating and storing MaskedVal, just truncate+store Val.


Repository:
  rL LLVM

https://reviews.llvm.org/D30416