| Message ID | CY1PR1201MB10980C732D29B2A280550E8D8F2F0@CY1PR1201MB1098.namprd12.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 30, 2015 at 6:06 PM, Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com> wrote: > Hi Richard, > > I am trying to "if covert the store" in the below test case and later help it to get vectorized under -Ofast -ftree-loop-if-convert-stores -fno-common > > #define LEN 4096 > __attribute__((aligned(32))) float array[LEN]; void test() { for (int i = 0; i < LEN; i++) { > if (array[i] > (float)0.) > array[i] =3 ; > > } > } > > Currently in GCC 5.2 does not vectorize it. > https://goo.gl/9nS6Pd > > However ICC seems to vectorize it > https://goo.gl/y1yGHx > > As discussed with you earlier, to allow "if convert store" I am checking the following: > > (1) We already read the reference "array[i]" unconditionally once . > (2) I am now checking if we are conditionally writing to memory which is defined as read and write and is bound to the definition we are seeing. I don't think this is thread safe .... Thanks, Andrew > > With this change, I get able to if convert and the vectorize the case also. > > /build/gcc/xgcc -B ./build/gcc/ ifconv.c -Ofast -fopt-info-vec -S -ftree-loop-if-convert-stores -fno-common > ifconv.c:2:63: note: loop vectorized > > Patch > ------ > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c > index f201ab5..6475cc0 100644 > --- a/gcc/tree-if-conv.c > +++ b/gcc/tree-if-conv.c > @@ -727,6 +727,34 @@ write_memrefs_written_at_least_once (gimple *stmt, > return true; > } > > +static bool > +write_memrefs_safe_to_access_unconditionally (gimple *stmt, > + vec<data_reference_p> drs) > +{ > + int i; > + data_reference_p a; > + bool found = false; > + > + for (i = 0; drs.iterate (i, &a); i++) > + { > + if (DR_STMT (a) == stmt > + && DR_IS_WRITE (a) > + && (DR_WRITTEN_AT_LEAST_ONCE (a) == 0) > + && (DR_RW_UNCONDITIONALLY (a) == 1)) > + { > + tree base = get_base_address (DR_REF (a)); > + found = false; > + if (DECL_P (base) > + && decl_binds_to_current_def_p (base) > + && !TREE_READONLY (base)) > + { > + found = true; > + } > + } > + } > + return found; > +} > + > /* Return true when the memory references of STMT won't trap in the > if-converted code. There are two things that we have to check for: > > @@ -748,8 +776,20 @@ write_memrefs_written_at_least_once (gimple *stmt, > static bool > ifcvt_memrefs_wont_trap (gimple *stmt, vec<data_reference_p> refs) > { > - return write_memrefs_written_at_least_once (stmt, refs) > - && memrefs_read_or_written_unconditionally (stmt, refs); > + bool memrefs_written_once, memrefs_read_written_unconditionally; > + bool memrefs_safe_to_access; > + > + memrefs_written_once > + = write_memrefs_written_at_least_once (stmt, refs); > + > + memrefs_read_written_unconditionally > + = memrefs_read_or_written_unconditionally (stmt, refs); > + > + memrefs_safe_to_access > + = write_memrefs_safe_to_access_unconditionally (stmt, refs); > + > + return ((memrefs_written_once || memrefs_safe_to_access) > + && memrefs_read_written_unconditionally); > } > > /* Wrapper around gimple_could_trap_p refined for the needs of the > > > do I need this function write_memrefs_written_at_least_once anymore? > Please suggest if there a better way to do this. > > Bootstrapped and regression tested on x86_64. > I am not adding change log and comments now, as I wanted to check approach first. > > Regards, > Venkat. > >
Hi Andrew, > -----Original Message----- > From: Andrew Pinski [mailto:pinskia@gmail.com] > Sent: Friday, October 30, 2015 3:38 PM > To: Kumar, Venkataramanan > Cc: Richard Beiner (richard.guenther@gmail.com); gcc-patches@gcc.gnu.org > Subject: Re: [RFC] [Patch] Relax tree-if-conv.c trap assumptions. > > On Fri, Oct 30, 2015 at 6:06 PM, Kumar, Venkataramanan > <Venkataramanan.Kumar@amd.com> wrote: > > Hi Richard, > > > > I am trying to "if covert the store" in the below test case and later > > help it to get vectorized under -Ofast -ftree-loop-if-convert-stores > > -fno-common > > > > #define LEN 4096 > > __attribute__((aligned(32))) float array[LEN]; void test() { for (int i = 0; i < > LEN; i++) { > > if (array[i] > (float)0.) > > array[i] =3 ; > > > > } > > } > > > > Currently in GCC 5.2 does not vectorize it. > > https://goo.gl/9nS6Pd > > > > However ICC seems to vectorize it > > https://goo.gl/y1yGHx > > > > As discussed with you earlier, to allow "if convert store" I am checking the > following: > > > > (1) We already read the reference "array[i]" unconditionally once . > > (2) I am now checking if we are conditionally writing to memory which is > defined as read and write and is bound to the definition we are seeing. > > > I don't think this is thread safe .... > I thought under -ftree-loop-if-convert-stores it is ok to do this transformation. Regards, Venkat. > Thanks, > Andrew > > > > > With this change, I get able to if convert and the vectorize the case also. > > > > /build/gcc/xgcc -B ./build/gcc/ ifconv.c -Ofast -fopt-info-vec -S > > -ftree-loop-if-convert-stores -fno-common > > ifconv.c:2:63: note: loop vectorized > > > > Patch > > ------ > > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c index > > f201ab5..6475cc0 100644 > > --- a/gcc/tree-if-conv.c > > +++ b/gcc/tree-if-conv.c > > @@ -727,6 +727,34 @@ write_memrefs_written_at_least_once (gimple > *stmt, > > return true; > > } > > > > +static bool > > +write_memrefs_safe_to_access_unconditionally (gimple *stmt, > > + > > +vec<data_reference_p> drs) { > > + int i; > > + data_reference_p a; > > + bool found = false; > > + > > + for (i = 0; drs.iterate (i, &a); i++) > > + { > > + if (DR_STMT (a) == stmt > > + && DR_IS_WRITE (a) > > + && (DR_WRITTEN_AT_LEAST_ONCE (a) == 0) > > + && (DR_RW_UNCONDITIONALLY (a) == 1)) > > + { > > + tree base = get_base_address (DR_REF (a)); > > + found = false; > > + if (DECL_P (base) > > + && decl_binds_to_current_def_p (base) > > + && !TREE_READONLY (base)) > > + { > > + found = true; > > + } > > + } > > + } > > + return found; > > +} > > + > > /* Return true when the memory references of STMT won't trap in the > > if-converted code. There are two things that we have to check for: > > > > @@ -748,8 +776,20 @@ write_memrefs_written_at_least_once (gimple > > *stmt, static bool ifcvt_memrefs_wont_trap (gimple *stmt, > > vec<data_reference_p> refs) { > > - return write_memrefs_written_at_least_once (stmt, refs) > > - && memrefs_read_or_written_unconditionally (stmt, refs); > > + bool memrefs_written_once, memrefs_read_written_unconditionally; > > + bool memrefs_safe_to_access; > > + > > + memrefs_written_once > > + = write_memrefs_written_at_least_once (stmt, refs); > > + > > + memrefs_read_written_unconditionally > > + = memrefs_read_or_written_unconditionally (stmt, refs); > > + > > + memrefs_safe_to_access > > + = write_memrefs_safe_to_access_unconditionally (stmt, > > + refs); > > + > > + return ((memrefs_written_once || memrefs_safe_to_access) > > + && memrefs_read_written_unconditionally); > > } > > > > /* Wrapper around gimple_could_trap_p refined for the needs of the > > > > > > do I need this function write_memrefs_written_at_least_once anymore? > > Please suggest if there a better way to do this. > > > > Bootstrapped and regression tested on x86_64. > > I am not adding change log and comments now, as I wanted to check > approach first. > > > > Regards, > > Venkat. > > > >
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c index f201ab5..6475cc0 100644 --- a/gcc/tree-if-conv.c +++ b/gcc/tree-if-conv.c @@ -727,6 +727,34 @@ write_memrefs_written_at_least_once (gimple *stmt, return true; } +static bool +write_memrefs_safe_to_access_unconditionally (gimple *stmt, + vec<data_reference_p> drs) +{ + int i; + data_reference_p a; + bool found = false; + + for (i = 0; drs.iterate (i, &a); i++) + { + if (DR_STMT (a) == stmt + && DR_IS_WRITE (a) + && (DR_WRITTEN_AT_LEAST_ONCE (a) == 0) + && (DR_RW_UNCONDITIONALLY (a) == 1)) + { + tree base = get_base_address (DR_REF (a)); + found = false; + if (DECL_P (base) + && decl_binds_to_current_def_p (base) + && !TREE_READONLY (base)) + { + found = true; + } + } + } + return found; +} + /* Return true when the memory references of STMT won't trap in the if-converted code. There are two things that we have to check for: @@ -748,8 +776,20 @@ write_memrefs_written_at_least_once (gimple *stmt, static bool ifcvt_memrefs_wont_trap (gimple *stmt, vec<data_reference_p> refs) { - return write_memrefs_written_at_least_once (stmt, refs) - && memrefs_read_or_written_unconditionally (stmt, refs); + bool memrefs_written_once, memrefs_read_written_unconditionally; + bool memrefs_safe_to_access; + + memrefs_written_once + = write_memrefs_written_at_least_once (stmt, refs); + + memrefs_read_written_unconditionally + = memrefs_read_or_written_unconditionally (stmt, refs); + + memrefs_safe_to_access + = write_memrefs_safe_to_access_unconditionally (stmt, refs); + + return ((memrefs_written_once || memrefs_safe_to_access) + && memrefs_read_written_unconditionally); } /* Wrapper around gimple_could_trap_p refined for the needs of the
Hi Richard, I am trying to "if covert the store" in the below test case and later help it to get vectorized under -Ofast -ftree-loop-if-convert-stores -fno-common #define LEN 4096 __attribute__((aligned(32))) float array[LEN]; void test() { for (int i = 0; i < LEN; i++) { if (array[i] > (float)0.) array[i] =3 ; } } Currently in GCC 5.2 does not vectorize it. https://goo.gl/9nS6Pd However ICC seems to vectorize it https://goo.gl/y1yGHx As discussed with you earlier, to allow "if convert store" I am checking the following: (1) We already read the reference "array[i]" unconditionally once . (2) I am now checking if we are conditionally writing to memory which is defined as read and write and is bound to the definition we are seeing. With this change, I get able to if convert and the vectorize the case also. /build/gcc/xgcc -B ./build/gcc/ ifconv.c -Ofast -fopt-info-vec -S -ftree-loop-if-convert-stores -fno-common ifconv.c:2:63: note: loop vectorized Patch ------ do I need this function write_memrefs_written_at_least_once anymore? Please suggest if there a better way to do this. Bootstrapped and regression tested on x86_64. I am not adding change log and comments now, as I wanted to check approach first. Regards, Venkat.