On 10.09.2018 19:53, Jakub Jelinek wrote:
> On Mon, Sep 10, 2018 at 04:05:26PM +0200, Andreas Krebbel wrote:
>> On 20.08.2018 16:30, Jeff Law wrote:
>>> On 08/18/2018 03:20 AM, Eric Botcazou wrote:
>>>>> Eric, didn't your patches explicitely handle this case of a non-constant
>>>>> inbetween?
>>>>
>>>> Only if there is no overlap at all, otherwise you cannot do things simply.
>>>>
>>>>> Can you have a look / review here?
>>>>
>>>> Jakub is probably more qualified to give a definitive opinion, as he wrote
>>>> check_no_overlap and the bug is orthogonal to my patches since it is
>>>> present
>>>> in 8.x; in any case, all transformations are supposed to be covered by the
>>>> testsuite.
>>> FYI. Jakub is on PTO through the end of this week and will probably be
>>> buried when he returns.
>>
>> Jakub, could you please have a look whether that's the right fix?
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00474.html
>
> It is a fix, but not optimal.
> We have essentially:
> MEM[(int *)p_28] = 0;
> MEM[(char *)p_28 + 3B] = 1;
> MEM[(char *)p_28 + 1B] = 2;
> MEM[(char *)p_28 + 2B] = MEM[(char *)p_28 + 6B];
> It is useful to merge the first 3 stores into:
> MEM[(int *)p_28] = 0x01000200; // or 0x00020001; depending on endianity
> MEM[(char *)p_28 + 2B] = MEM[(char *)p_28 + 6B];
> rather than punt, and just ignore (i.e. make sure it isn't merged with
> anything else) the non-INTEGER_CST store). If you don't mind, I'll take this
> PR over and handle it tomorrow.
Please do. Thanks!
Andreas
>
> Slightly tweaked testcase:
> __attribute__((noipa)) void
> foo (int *p)
> {
> *p = 0;
> *((char *)p + 3) = 1;
> *((char *)p + 1) = 2;
> *((char *)p + 2) = *((char *)p + 6);
> }
>
> int
> main ()
> {
> int a[2] = { -1, 0 };
> if (sizeof (int) != 4)
> return 0;
> ((char *)a)[6] = 3;
> foo (a);
> if (((char *)a)[0] != 0 || ((char *)a)[1] != 2
> || ((char *)a)[2] != 3 || ((char *)a)[3] != 1)
> __builtin_abort ();
> }
>
> Jakub
>
