On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> 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?
>
> CDI_POST_DOMINATORS is required to be freed by passes.
>
This works for me. Should I submit a patch?
H.J.
---
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 62b0596..1307e22 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -228,7 +228,11 @@ delete_insn_and_edges (rtx_insn *insn)
purge = true;
delete_insn (insn);
if (purge)
- purge_dead_edges (BLOCK_FOR_INSN (insn));
+ {
+ purge_dead_edges (BLOCK_FOR_INSN (insn));
+ if (dom_info_available_p (CDI_DOMINATORS))
+ free_dominance_info (CDI_DOMINATORS);
+ }
}
/* Unlink a chain of insns between START and FINISH, leaving notes
