[PATCH] D22840: Split the store of an int value merged from a pair of smaller values into multiple stores.

[Chandler Carruth via llvm-commits]

On Fri, Sep 16, 2016 at 10:54 AM Wei Mi via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hi Chandler,
>
> I found the patch still missed something here. I saw sometimes the
> float merged to int-fp pair was in a different basic block then
> dagcombiner cannot see it. Even if we relax
> TLI.isMultiStoresCheaperThanBitsMerge on x86 and do split for int-int
> pair, the float to int bitcast in another bb cannot be merged with the
> splitted store and it is not optimal.
>
> bb1:
>   %6 = bitcast float %5 to i32
>    ...
> bb2:
>    ...
>   %retval.sroa.2.0.insert.ext.i75.i = zext i32 %6 to i64
>   %retval.sroa.2.0.insert.shift.i76.i = shl nuw i64
> %retval.sroa.2.0.insert.ext.i75.i, 32
>   %tmp13 = trunc i64 %25 to i32
>   %retval.sroa.0.0.insert.ext.i77.i = zext i32 %tmp13 to i64
>   %retval.sroa.0.0.insert.insert.i78.i = or i64
> %retval.sroa.2.0.insert.shift.i76.i, %retval.sroa.0.0.insert.ext.i77.i
>   store i64 %retval.sroa.0.0.insert.insert.i78.i, i64* %ref.tmp7.i, align 8
>
> I can try to sink %6 to its single use in codegenprep, but I am not
> sure how strictly the pattern I should specify there.  If I try to
> find the exact pattern there, why not just do the whole thing in
> codegenprep? If I don't, we may unnecessarily do some sinking for
> bitcast float to i32 and I am not sure its impact. I don't know how to
> choose so I want to hear your thought about it.
>

I agree that sinking seems really nasty.

This is what GlobalISel is going to hopefully fix.

If this is still causing measurable performance problems and the code to do
splitting in CodeGenPrep is reasonably simple, I agree with splitting in
CodeGenPrep.

(We should probably sadly still have the DAG combine so that we can catch
cases that show up after DAG combining runs...)

-Chandler


>
> Thanks,
> Wei.
>
> On Fri, Sep 2, 2016 at 10:25 AM, Wei Mi <wmi at google.com> wrote:
> > This revision was automatically updated to reflect the committed changes.
> > Closed by commit rL280505: Split the store of a wide value merged from
> an int-fp pair into multiple stores. (authored by wmi).
> >
> > Changed prior to commit:
> >   https://reviews.llvm.org/D22840?vs=69494&id=70186#toc
> >
> > Repository:
> >   rL LLVM
> >
> > https://reviews.llvm.org/D22840
> >
> > Files:
> >   llvm/trunk/include/llvm/Target/TargetLowering.h
> >   llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> >   llvm/trunk/lib/Target/X86/X86ISelLowering.h
> >   llvm/trunk/test/Transforms/InstCombine/split-store.ll
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160916/b2068180/attachment.html>