This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Shouldn't convert_scalars_to_vector call free_dominance_info?


On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law <law@redhat.com> wrote:
>>> On 03/10/2016 08:00 PM, H.J. Lu wrote:
>>>>
>>>> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>
>>>>> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law <law@redhat.com> wrote:
>>>>>>
>>>>>> On 03/10/2016 01:18 PM, Richard Biener wrote:
>>>>>>>
>>>>>>>
>>>>>>> On March 10, 2016 6:02:58 PM GMT+01:00, "H.J. Lu" <hjl.tools@gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek <jakub@redhat.com>
>>>>>>>>
>>>>>>>>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Thu, Mar 10, 2016 at 05:43:27AM -0800, H.J. Lu wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> free_dominance_info (CDI_DOMINATORS);
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Since convert_scalars_to_vector may add instructions, dominance
>>>>>>>>>>> info is no longer up to date.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Adding instructions doesn't change anything on the dominance info,
>>>>>>>>
>>>>>>>>
>>>>>>>> just
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> cfg manipulations that don't keep the dominators updated.
>>>>>>>>>> You can try to verify the dominance info at the end of the stv pass,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I added
>>>>>>>>>
>>>>>>>>> verify_dominators (CDI_DOMINATORS);
>>>>>>>>> '
>>>>>>>>> It did trigger assert in my 64-bit STV pass in 64-bit libgcc build:
>>>>>>>>>
>>>>>>>>> /export/gnu/import/git/sources/gcc/libgcc/config/libbid/bid128_fma.c:
>>>>>>>>> In function \u2018add_and_round.constprop\u2019:
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> /export/gnu/import/git/sources/gcc/libgcc/config/libbid/bid128_fma.c:629:1:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> error: dominator of 158 should be 107, not 101
>>>>>>>>>
>>>>>>>>> I will investigate.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Here is the problem:
>>>>>>>>
>>>>>>>> 1. I extended the STV pass to 64-bit to convert TI load/store to
>>>>>>>> V1TI load/store to use SSE load/store for 128-bit load/store.
>>>>>>>> 2. The 64-bit STV pass generates settings of CONST0_RTX and
>>>>>>>> CONSTM1_RTX to store 128-bit 0 and -1.
>>>>>>>> 3. I placed the 64-bit STV pass before the CSE pass so that
>>>>>>>> CONST0_RTX and CONSTM1_RTX generated by the STV pass
>>>>>>>> can be CSEed.
>>>>>>>> 4. After settings of CONST0_RTX and CONSTM1_RTX are CSEed,
>>>>>>>> dominance info will be wrong.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Can't see how cse can ever invalidate dominators.
>>>>>>
>>>>>>
>>>>>> cse can simplify jumps which can invalidate dominance information.
>>>>>>
>>>>>> But cse-ing CONST0_RTX and CONSTM1_RTX shouldn't invalidate dominators,
>>>>>> that's just utter nonsense -- ultimately it has to come down to changing
>>>>>> jumps.  ISTM HJ has more digging to do here.
>>>>>
>>>>>
>>>>> Not just CONST0_RTX and CONSTM1_RTX.  The new STV
>>>>> pass changes mode of SET from TImode to V1TImode which
>>>>> exposes more opportunities to CSE.
>>>>>
>>>>
>>>> What I did is equivalent to
>>>>
>>>> diff --git a/gcc/cse.c b/gcc/cse.c
>>>> index 2665d9a..43202a1 100644
>>>> --- a/gcc/cse.c
>>>> +++ b/gcc/cse.c
>>>> @@ -7644,7 +7644,11 @@ public:
>>>>         return optimize > 0 && flag_rerun_cse_after_loop;
>>>>       }
>>>>
>>>> -  virtual unsigned int execute (function *) { return rest_of_handle_cse2
>>>> (); }
>>>> +  virtual unsigned int execute (function *)
>>>> +    {
>>>> +      calculate_dominance_info (CDI_DOMINATORS);
>>>> +      return rest_of_handle_cse2 ();
>>>> +    }
>>>>
>>>>   }; // class pass_cse2
>>>>
>>>>
>>>> which leads to the same ICE:
>>>
>>> But you haven't done the proper analysis to understand why the dominance
>>> relationships have changed.  Nothing of the changes you've outlined in your
>>> messages should invalidate the dominance information.
>>
>> Nothing is changed.  Just calling
>>
>> calculate_dominance_info (CDI_DOMINATORS);
>>
>> before rest_of_handle_cse2 will lead to ICE.
>
> Well, so CSE2 invalidates dominators but fails to free them when necessary.
> Please figure out the CSE transform that invalidates them and free dominators
> there.

I can give it a try.  But I'd like to first ask since CSE2 never calls
calculate_dominance_info (CDI_DOMINATORS), does it need to
keep dominators valid?

free_dominance_info (CDI_DOMINATORS);

at beginning will do the job.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]