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

[Wei Mi via llvm-commits]

On Fri, Sep 16, 2016 at 11:38 AM, Chandler Carruth <chandlerc at google.com> wrote:
> 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.
>

Yes, it still causes measurable performance problem. I will add
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...)

Ok, then I should change the interface of
TLI.isMultiStoresCheaperThanBitsMerge to make it work for both
CodeGenPrep and DAG combine.

>
> -Chandler
>

Thanks for the suggestion!

Wei.