On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener
>> <richard.guent...@gmail.com> wrote:
>>> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law <l...@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.to...@gmail.com> wrote:
>>>>>>>
>>>>>>> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law <l...@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.to...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek <ja...@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?
>
> If it doesn't free them then yes.
>
>> free_dominance_info (CDI_DOMINATORS);
>>
>> at beginning will do the job.
>
> Of course. But that may be not always necessary and thus cause extra
> dominance compute for the next user.
Do we need to both CDI_DOMINATORS and CDI_POST_DOMINATORS
valid?
--
H.J.
