Re: [PATCH] Fix Fortran DO loop fallback

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jul 12, 2016 at 2:31 PM, Martin Liška <mliska@suse.cz> wrote:
> On 07/12/2016 12:14 PM, Richard Biener wrote:
>> On Mon, Jul 11, 2016 at 4:44 PM, Jeff Law <law@redhat.com> wrote:
>>> On 07/08/2016 08:26 AM, Martin Liška wrote:
>>>>
>>>> Hello
>>>>
>>>> Following patch fixes fallout caused by the patch set:
>>>> https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html
>>>>
>>>> Ready after it finished regression tests?
>>>> Thanks,
>>>> Martin
>>>>
>>>>
>>>> 0001-Fix-Fortran-DO-loop-fallback.patch
>>>>
>>>>
>>>> From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001
>>>> From: marxin <mliska@suse.cz>
>>>> Date: Fri, 8 Jul 2016 15:51:54 +0200
>>>> Subject: [PATCH] Fix Fortran DO loop fallback
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2016-07-08  Martin Liska  <mliska@suse.cz>
>>>>
>>>>         * gfortran.dg/ldist-1.f90: Update expected dump scan.
>>>>         * gfortran.dg/pr42108.f90: Likewise.
>>>>         * gfortran.dg/vect/pr62283.f: Likewise.
>>>
>>> Shouldn't ldist-1.f90 be scan-tree-dump-not?  It seems like you change it
>>> from that just last week, but it wasn't mentioned in the ChangeLog.
>>
>> gfortran.dg/pr42108.f90 also looks pointless now?  I suppose there is nothing
>> to hoist after the change?  Do we now have an option to revert back to old
>> behavior?  If so it would be better to use that flag here.
>
> No, there's no option. So, as the test-case now looks pointless, should I mark it
> with xfail now?

The scan for 1 *n_ after FRE looks still useful.  Btw, the testcase
doesn't fail for me,
we _do_ hoist the division in PRE, just not with -m32 anymore.  Can
you confirm this?


>>
>> diff --git a/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> b/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> index 7df3d99..2933f51 100644
>> --- a/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> +++ b/gcc/testsuite/gfortran.dg/vect/pr62283.f
>> @@ -13,4 +13,4 @@ C { dg-additional-options "-fvect-cost-model=dynamic" }
>>        beta=3.141593
>>        y=y+beta*x
>>        end
>> -C { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" {
>> target { vect_hw_misalign } } } }
>> +C { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target {
>> vect_hw_misalign } } } }
>>
>> so why do we no longer vectorize 1 loops in two functions?  The
>> testcase works for me
>> unchanged.
>
> Yeah, it works on -m64, however as we're able to merge the functions with -m32 (-fipa-icf),
> then I recommend to disable ICF for the test-case.
>
> Reason why the pair of functions on x86_64 is that:
>
> test3 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
> {
>   <bb 2>:
>
>   <bb 3>:
>   # S.0_6 = PHI <1(2), S.0_12(4)>
>   if (S.0_6 > 4)
>     goto <bb 5>;
>   else
>     goto <bb 4>;
> ...
>
> test2 (real(kind=4)[4] * restrict x, real(kind=4)[4] * restrict y)
> {
>   integer(kind=4) i;
>
>   <bb 2>:
>
>   <bb 3>:
>   # i_6 = PHI <1(2), i_12(4)>
> ...
>
> On x86_64 types of 'S.0_6' and 'i' are not compatible. As I've just read tree dump files,
>   # S.0_6 = PHI <1(2), S.0_12(4)>
>   if (S.0_6 > 4)
>
> is optimized out by VRP, which runs after IPA ICF.
>
> I'll send patch right after we'll agree on pr42108.f90.
>
> Thanks,
> Martin
>
>>
>> Richard.
>>
>>> OK with that change.
>>>
>>> jeff
>>>
>>>
>