Re: [PATCH] Fix result for conditional reductions matching at index 0

[Alan Hayward <Alan.Hayward@arm.com>]

> On 22 Nov 2017, at 09:14, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, Nov 21, 2017 at 5:43 PM, Kilian Verhetsel
> <kilian.verhetsel@uclouvain.be> wrote:
>> 
>>> This is PR81179 I think, please mention that in the changelog.
>> 
>> Correct, my bad for missing that.
>> 
>>> This unconditionally pessimizes code even if there is no valid index
>>> zero, right?
>> 
>> Almost, since for a loop such as:
>> 
>>  #define OFFSET 1
>>  unsigned int find(const unsigned int *a, unsigned int v) {
>>    unsigned int result = 120;
>>    for (unsigned int i = OFFSET; i < 32+OFFSET; i++) {
>>      if (a[i-OFFSET] == v) result = i;
>>    }
>>    return result;
>>  }
>> 
>> the index i will match the contents of the index vector used here ---
>> but this does indeed pessimize the code generated for, say, OFFSET
>> = 2. It is probably more sensible to use the existing code path in those
>> situations.
>> 
>>> The issue with the COND_REDUCITION index vector is overflow IIRC.
>> 
>> Does that mean such overflows can already manifest themselves for
>> regular COND_REDUCTION? I had assumed sufficient checks were already in
>> place because of the presence of the is_nonwrapping_integer_induction
>> test.
> 
> But only if we need the index vector?  The patch looked like you're changing
> how other modes are handled (in my look I didn't make myself familiar with
> the various modes again...).  Anyway, Alan hopefully remembers what he
> coded so I'll defer to him here.
> 
> If Alan is happy with the patch consider it approved.
> 

Richard’s right with his question.

The optimisation needs to fail if the number of interactions of the loop + 1 doesn’t
fit into the data type used for the result.

I took the test pr65947-14.c
First I set N to 0xffffffff-1. This compiled and vectorised. That’s ok.
Now if I set N to 0xffffffff it still vectorises, but this should fail.

Compare to pr65947-14.c where we set  last = a[I]; inside the if.
When set N to 0xffffffff-1, it compiled and vectorised. That’s ok.
When set N to 0xffffffff it fails to vectorise with the message
"loop size is greater than data size”.

Looks like you might just need to add the one check.

Also see pr65947-9.c which uses the slightly more useful char indexes.


Alan.