Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: > On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >> On 03/10/2016 08:00 PM, H.J. Lu wrote: >>> >>> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >> wrote: >>> >>> >>> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek >>> >>> >>> 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. Richard. > -- > H.J.
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: >> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >>> On 03/10/2016 08:00 PM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: > > On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >>> wrote: On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: > > > On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek 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.
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: >>> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: On 03/10/2016 08:00 PM, H.J. Lu wrote: > > On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: >> >> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" wrote: > > > On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: >> >> >> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek > > > 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. Richard. > -- > H.J.
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener >> wrote: >>> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: > On 03/10/2016 08:00 PM, H.J. Lu wrote: >> >> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: >>> >>> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" > > wrote: >> >> >> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: >>> >>> >>> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek >> >> >> 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
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: >>> On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener >>> wrote: On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: > On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >> On 03/10/2016 08:00 PM, H.J. Lu wrote: >>> >>> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >> >> wrote: >>> >>> >>> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu >>> wrote: On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek >>> >>> >>> 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
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener >> wrote: >>> On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: >> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >>> On 03/10/2016 08:00 PM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: > > On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >>> >>> wrote: On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: > > > On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek 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 th
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 5:50 AM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: >>> On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener >>> wrote: On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: >>> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: On 03/10/2016 08:00 PM, H.J. Lu wrote: > > On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: >> >> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" wrote: > > > On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu > wrote: >> >> >> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek > > > 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_DOMINAT
Re: Shouldn't convert_scalars_to_vector call free_dominance_info?
On Fri, Mar 11, 2016 at 3:01 PM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 5:50 AM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener >> wrote: >>> On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener >> wrote: >>> On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: > On 03/10/2016 08:00 PM, H.J. Lu wrote: >> >> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: >>> >>> On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" > > wrote: >> >> >> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu >> wrote: >>> >>> >>> On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek >>> >> >> >> 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.
[PATCH] Free dominance info if any edges are eliminated
On Fri, Mar 11, 2016 at 6:03 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 3:01 PM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 5:50 AM, H.J. Lu wrote: >>> On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener >>> wrote: On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: >>> On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener >>> wrote: On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: > On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >> On 03/10/2016 08:00 PM, H.J. Lu wrote: >>> >>> On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu >>> wrote: On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >> >> wrote: >>> >>> >>> On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu >>> wrote: On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek >>> >>> >>> 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_cse
Re: Validity of SUBREG+AND-imm transformations
On 08/03/16 19:11, Jeff Law wrote: On 03/08/2016 11:49 AM, Richard Henderson wrote: On 03/07/2016 02:49 PM, Jeff Law wrote: On 03/07/2016 03:44 AM, Kyrill Tkachov wrote: The RTL documentation for ASHIFT and friends says that the shift amount must be: "a fixed-point mode or be a constant with mode @code{VOIDmode}; which mode is determined by the mode called for in the machine description entry for the left-shift instruction". For example, on the VAX, the mode of @var{c} is @code{QImode} regardless of @var{m}. Use QImode in the named pattern/expander and use the other modes in an unnamed/anonymous pattern. I thought the same thing you did. But I tried it out on the aarch64 port and it didn't work. Combine kept coming back to the QImode pattern. I didn't want to look into it any farther than that, lest I fall down a rabbit hole, but there's more to it than just tweaking the patterns. Strange. Probably worth some investigating for gcc-7. FYI I've attached an example patch that I had implemented for my original proposal to PR 70119. Kyrill jeff
C++17 is_always_lock_free implementation
Hello GCC / libstdc++ folks, I implemented C++17's is_always_lock_free [0] in clang / libc++ and am wondering if GCC and libstdc++ are interested in following a similar API as the one I propose in the clang patch [1]. What I propose is: * Extend the __GCC_ATOMIC_##TYPE##_LOCK_FREE macros to FLOAT, DOUBLE, LDOUBLE. This doesn't add corresponding C macros. * Add a __LLVM_LOCK_FREE_IS_SIZE_BASED macro with value 1 if all the lock-free-ness is based purely on the type's size. For LLVM this is currently the case for all ISAs. * Add macros __LLVM_ATOMIC_##BYTES##_BYTES_LOCK_FREE for sizes 1, 2, 4, 8, 16. We could go further, but I don't think any ISA has 32-byte and up lock-free (and the C++ library can just #if defined() on wider types). I then use these in libc++ [2]. It may be worth standardizing equivalent macros in C, but we'll need these built-in macros anyways. I also think C macros aren't as useful as the C++ template-based constexpr magic. Thanks, JF [0]: http://jfbastien.github.io/papers/P0152R1.html [1]: http://reviews.llvm.org/D17950 [2]: http://reviews.llvm.org/D17951
Re: [PATCH] Free dominance info if any edges are eliminated
On Fri, Mar 11, 2016 at 6:45 AM, H.J. Lu wrote: > On Fri, Mar 11, 2016 at 6:03 AM, Richard Biener > wrote: >> On Fri, Mar 11, 2016 at 3:01 PM, H.J. Lu wrote: >>> On Fri, Mar 11, 2016 at 5:50 AM, H.J. Lu wrote: On Fri, Mar 11, 2016 at 5:19 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 2:02 PM, H.J. Lu wrote: >> On Fri, Mar 11, 2016 at 4:58 AM, Richard Biener >> wrote: >>> On Fri, Mar 11, 2016 at 1:47 PM, H.J. Lu wrote: On Fri, Mar 11, 2016 at 2:06 AM, Richard Biener wrote: > On Fri, Mar 11, 2016 at 4:10 AM, H.J. Lu wrote: >> On Thu, Mar 10, 2016 at 7:03 PM, Jeff Law wrote: >>> On 03/10/2016 08:00 PM, H.J. Lu wrote: On Thu, Mar 10, 2016 at 1:30 PM, H.J. Lu wrote: > > On Thu, Mar 10, 2016 at 1:24 PM, Jeff Law 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" >>> >>> wrote: On Thu, Mar 10, 2016 at 6:57 AM, H.J. Lu wrote: > > > On Thu, Mar 10, 2016 at 5:49 AM, Jakub Jelinek > 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
PowerPC A2 QPX vector instruction support
Hello, I am rebooting a personal project to add support for the PowerPC A2 QPX (Quad Processing eXtension) vector unit to gcc. [1] I am interested in: beginning the process to have my employer approve a contributor assignment; advice on the best way to add in support for QPX instructions and vectorization to gcc; and suggestions for tools and scripts that might make the development process easier. The base PowerPC A2 cpu is essentially non-existent. Most PowerPC A2 cpus have QPX support and are installed in Blue Gene /Q supercomputers. The first of the Blue Gene /Q systems came into production 2012 with expected operation into 2019 or 2020 at most sites. To keep these systems useful for scientific users, ongoing toolchain development is required. IBM has produced patches for QPX support in binutils, gdb, and glibc as it was required for their own XL C/C++ and Fortran compiler. IBM offered patches for basic gcc functionality on the Blue Gene /Q without QPX support. Hal Finkel and others have contributed QPX support to clang/llvm and continue to offer long-term support. Still, there is no GPL-licensed compiler currently available with QPX support which may limit support options in the long term. Similarly, there's great community interest in gcc QPX support as it provides the most realistic chances for near-term Fortran support. An earlier personal effort focused on reworking the AltiVec support to handle the generation of QPX instructions based on some informal guidance from community members. This approach has proven unusably buggy and almost impossible to maintain concurrent with ongoing gcc development, leading to my desire to do this as formally and correctly as possible. My personal background is not in compiler development, but I'm a relatively fast learner, tenacious, and think I could complete the work by July with some formal guidance. -- William Scullin High Performance Computing Systems Administrator, Senior Leadership Computing Facility Argonne National Laboratory 9700 South Cass Avenue, Bldg. 240 Argonne, IL 60439-4847 office: +1-630-252-6412 mobile/voip: +1-773-257-3571 e-mail: wscul...@alcf.anl.gov [1] https://www.alcf.anl.gov/files/Qpx_3.pdf