Re: fix PR46029: reimplement if conversion of loads and stores

[Richard Biener <richard dot guenther at gmail dot com>]

On June 26, 2015 2:21:22 PM GMT+02:00, Alan Lawrence <alan.lawrence@arm.com> wrote:
>Sebastian Pop wrote:
>> On Thu, Jun 25, 2015 at 4:43 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> when the new scheme triggers vectorization cannot succeed on the
>>> result as we get
>>>
>>>   if (cond)
>>>     *p = val;
>>>
>>> if-converted to
>>>
>>>   tem = cond ? p : &scratch;
>>>   *tem = val;
>> 
>> That's correct.
>> 
>>> and
>>>
>>>    if (cond)
>>>      val = *p;
>>>
>>> if-converted to
>>>
>>>   scatch = val;
>>>   tem = cond ? p : &scratch;
>>>   val = *tem;
>> 
>> The patch does this slightly differently:
>> 
>>    tem = cond ? p : &scratch;
>>    val = cond ? *tem : val;
>> 
>> I think I like your version better as it has only one cond_expr.
>
>Another slight concern...by reusing scratchpad's, are we at risk of
>creating 
>lots of false dependencies here, that restrict instruction scheduling /
>other 
>opts later?

Another possibility is to not share them and make sure to insert clobbers for them when they become dead so later stack slot sharing can merge them again.

Richard.

>>> [...]
>>> and thus the store and loads appear as scather/gather ones to
>>> the vectorizer (if-conversion could directly generate masked
>>> load/stores of course and not use a scratch-pad at all in that
>case).
>
>Thank you Richard for much better expressing what I was thinking here
>too :)
>
>> Abe also suggested to continue optimizing the other way in cases
>> where we know to write or load from the same location on all
>branches:
>> 
>> if (c)
>>   A[i] = ...
>> else
>>   A[i] = ...
>
>The store here really should be commoned, yes.
>
>(Related but different?: BZ 56625.)
>
>
>I might add, I find this code much easier to follow than the old
>(removed) code 
>about data references, memrefs_read_unconditionally, and 
>write_memrefs_written_at_least-once...
>
>Thanks, Alan