Re: VEC_COND_EXPR optimizations v2
On Fri, Aug 7, 2020 at 2:15 PM Marc Glisse wrote:
>
> On Fri, 7 Aug 2020, Richard Biener wrote:
>
> > On Fri, Aug 7, 2020 at 10:33 AM Marc Glisse wrote:
> >>
> >> On Fri, 7 Aug 2020, Richard Biener wrote:
> >>
> >>> On Thu, Aug 6, 2020 at 8:07 PM Marc Glisse wrote:
>
> On Thu, 6 Aug 2020, Christophe Lyon wrote:
>
> >> Was I on the right track configuring with
> >> --target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
> >> --with-fpu=neon-fp16
> >> then compiling without any special option?
> >
> > Maybe you also need --with-float=hard, I don't remember if it's
> > implied by the 'hf' target suffix
>
> Thanks! That's what I was missing to reproduce the issue. Now I can
> reproduce it with just
>
> typedef unsigned int vec __attribute__((vector_size(16)));
> typedef int vi __attribute__((vector_size(16)));
> vi f(vec a,vec b){
> return a==5 | b==7;
> }
>
> with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2
> -fdisable-tree-forwprop3 -O1
>
> _1 = a_5(D) == { 5, 5, 5, 5 };
> _3 = b_6(D) == { 7, 7, 7, 7 };
> _9 = _1 | _3;
> _7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
> 107);
>
> we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
> false), while with -fdisable-tree-forwprop4 we do manage to expand
>
> _2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0,
> 0 }, 112);
>
> It doesn't make much sense to me that we can expand the more complicated
> form and not the simpler form of the same operation (both compare a to 5
> and produce a vector of -1 or 0 of the same size), especially when the
> target has an instruction (vceq) that does just what we want.
>
> Introducing boolean vectors was fine, but I think they should be real
> types, that we can operate on, not be forced to appear only as the first
> argument of a vcond.
>
> I can think of 2 natural ways to improve things: either implement vector
> comparisons in the ARM backend (possibly by forwarding to their existing
> code for vcond), or in the generic expansion code try using vcond if the
> direct comparison opcode is not provided.
>
> We can temporarily revert my patch, but I would like it to be temporary.
> Since aarch64 seems to handle the same code just fine, maybe someone who
> knows arm could copy the relevant code over?
>
> Does my message make sense, do people have comments?
> >>>
> >>> So what complicates things now (and to some extent pre-existed when you
> >>> used AVX512 which _could_ operate on boolean vectors) is that we
> >>> have split out the condition from VEC_COND_EXPR to separate stmts
> >>> but we do not expect backends to be able to code-generate the separate
> >>> form - instead we rely on the ISEL pass to trasform VEC_COND_EXPRs
> >>> to .VCOND[U] "merging" the compares again. Now that process breaks
> >>> down once we have things like _9 = _1 | _3; - at some point I argued
> >>> that we should handle vector compares [and operations on boolean vectors]
> >>> as well in ISEL but then when it came up again for some reason I
> >>> disregarded that again.
> >>>
> >>> Thus - we don't want to go back to fixing up the generic expansion code
> >>> (which looks at one instruction at a time and is restricted by TER
> >>> single-use
> >>> restrictions).
> >>
> >> Here, it would only be handling comparisons left over by ISEL because they
> >> do not feed a VEC_COND_EXPR, maybe not so bad. Handling it in ISEL would
> >> be more consistent, but then we may have to teach the vector lowering pass
> >> about this.
> >>
> >>> Instead we want to deal with this in ISEL which should
> >>> behave more intelligently. In the above case it might involve turning
> >>> the _1 and _3 defs into .VCOND [with different result type], doing
> >>> _9 in that type and then somehow dealing with _7 ... but this eventually
> >>> means undoing the match simplification that introduced the code?
> >>
> >> For targets that do not have compact boolean vectors,
> >> VEC_COND_EXPR<*,-1,0> does nothing, it is just there as a technicality.
> >> Removing it to allow more simplifications makes sense, reintroducing it
> >> for expansion can make sense as well, I think it is ok if the second one
> >> reverses the first, but very locally, without having to propagate a change
> >> of type to the uses. So on ARM we could turn _1 and _3 into .VCOND
> >> producing bool:32 vectors, and replace _7 with a VIEW_CONVERT_EXPR. Or
> >> does using bool vectors like that seem bad? (maybe they aren't guaranteed
> >> to be equivalent to signed integer vectors with values 0 and -1 and we
> >> need to query the target to know if that is the case, or introduce an
> >> extra .VCOND)
> >>
> >> Also, we only want to replace comparisons with .VCOND if
Re: VEC_COND_EXPR optimizations v2
On Fri, 7 Aug 2020, Richard Biener wrote:
On Fri, Aug 7, 2020 at 10:33 AM Marc Glisse wrote:
On Fri, 7 Aug 2020, Richard Biener wrote:
On Thu, Aug 6, 2020 at 8:07 PM Marc Glisse wrote:
On Thu, 6 Aug 2020, Christophe Lyon wrote:
Was I on the right track configuring with
--target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
--with-fpu=neon-fp16
then compiling without any special option?
Maybe you also need --with-float=hard, I don't remember if it's
implied by the 'hf' target suffix
Thanks! That's what I was missing to reproduce the issue. Now I can
reproduce it with just
typedef unsigned int vec __attribute__((vector_size(16)));
typedef int vi __attribute__((vector_size(16)));
vi f(vec a,vec b){
return a==5 | b==7;
}
with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2 -fdisable-tree-forwprop3
-O1
_1 = a_5(D) == { 5, 5, 5, 5 };
_3 = b_6(D) == { 7, 7, 7, 7 };
_9 = _1 | _3;
_7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }, 107);
we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
false), while with -fdisable-tree-forwprop4 we do manage to expand
_2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
112);
It doesn't make much sense to me that we can expand the more complicated
form and not the simpler form of the same operation (both compare a to 5
and produce a vector of -1 or 0 of the same size), especially when the
target has an instruction (vceq) that does just what we want.
Introducing boolean vectors was fine, but I think they should be real
types, that we can operate on, not be forced to appear only as the first
argument of a vcond.
I can think of 2 natural ways to improve things: either implement vector
comparisons in the ARM backend (possibly by forwarding to their existing
code for vcond), or in the generic expansion code try using vcond if the
direct comparison opcode is not provided.
We can temporarily revert my patch, but I would like it to be temporary.
Since aarch64 seems to handle the same code just fine, maybe someone who
knows arm could copy the relevant code over?
Does my message make sense, do people have comments?
So what complicates things now (and to some extent pre-existed when you
used AVX512 which _could_ operate on boolean vectors) is that we
have split out the condition from VEC_COND_EXPR to separate stmts
but we do not expect backends to be able to code-generate the separate
form - instead we rely on the ISEL pass to trasform VEC_COND_EXPRs
to .VCOND[U] "merging" the compares again. Now that process breaks
down once we have things like _9 = _1 | _3; - at some point I argued
that we should handle vector compares [and operations on boolean vectors]
as well in ISEL but then when it came up again for some reason I
disregarded that again.
Thus - we don't want to go back to fixing up the generic expansion code
(which looks at one instruction at a time and is restricted by TER single-use
restrictions).
Here, it would only be handling comparisons left over by ISEL because they
do not feed a VEC_COND_EXPR, maybe not so bad. Handling it in ISEL would
be more consistent, but then we may have to teach the vector lowering pass
about this.
Instead we want to deal with this in ISEL which should
behave more intelligently. In the above case it might involve turning
the _1 and _3 defs into .VCOND [with different result type], doing
_9 in that type and then somehow dealing with _7 ... but this eventually
means undoing the match simplification that introduced the code?
For targets that do not have compact boolean vectors,
VEC_COND_EXPR<*,-1,0> does nothing, it is just there as a technicality.
Removing it to allow more simplifications makes sense, reintroducing it
for expansion can make sense as well, I think it is ok if the second one
reverses the first, but very locally, without having to propagate a change
of type to the uses. So on ARM we could turn _1 and _3 into .VCOND
producing bool:32 vectors, and replace _7 with a VIEW_CONVERT_EXPR. Or
does using bool vectors like that seem bad? (maybe they aren't guaranteed
to be equivalent to signed integer vectors with values 0 and -1 and we
need to query the target to know if that is the case, or introduce an
extra .VCOND)
Also, we only want to replace comparisons with .VCOND if the target
doesn't handle the comparison directly. For AVX512, we do want to produce
SImode bool vectors (for k* registers) and operate on them directly,
that's the whole point of introducing the bool vectors (if bool vectors
were only used to feed VEC_COND_EXPR and were all turned into .VCOND
before expansion, I don't see the point of specifying different types for
bool vectors for AVX512 vs non-AVX512, as it would make no difference on
what is passed to the backend).
I think targets should provide some number of operations, including for
instance bit_and and bit_ior on bool vectors, and be a bit consistent
about what they provide, it becomes unmanageable
Re: VEC_COND_EXPR optimizations v2
On Fri, Aug 7, 2020 at 10:33 AM Marc Glisse wrote:
>
> On Fri, 7 Aug 2020, Richard Biener wrote:
>
> > On Thu, Aug 6, 2020 at 8:07 PM Marc Glisse wrote:
> >>
> >> On Thu, 6 Aug 2020, Christophe Lyon wrote:
> >>
> Was I on the right track configuring with
> --target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
> --with-fpu=neon-fp16
> then compiling without any special option?
> >>>
> >>> Maybe you also need --with-float=hard, I don't remember if it's
> >>> implied by the 'hf' target suffix
> >>
> >> Thanks! That's what I was missing to reproduce the issue. Now I can
> >> reproduce it with just
> >>
> >> typedef unsigned int vec __attribute__((vector_size(16)));
> >> typedef int vi __attribute__((vector_size(16)));
> >> vi f(vec a,vec b){
> >> return a==5 | b==7;
> >> }
> >>
> >> with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2
> >> -fdisable-tree-forwprop3 -O1
> >>
> >>_1 = a_5(D) == { 5, 5, 5, 5 };
> >>_3 = b_6(D) == { 7, 7, 7, 7 };
> >>_9 = _1 | _3;
> >>_7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
> >> 107);
> >>
> >> we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
> >> false), while with -fdisable-tree-forwprop4 we do manage to expand
> >>
> >>_2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0
> >> }, 112);
> >>
> >> It doesn't make much sense to me that we can expand the more complicated
> >> form and not the simpler form of the same operation (both compare a to 5
> >> and produce a vector of -1 or 0 of the same size), especially when the
> >> target has an instruction (vceq) that does just what we want.
> >>
> >> Introducing boolean vectors was fine, but I think they should be real
> >> types, that we can operate on, not be forced to appear only as the first
> >> argument of a vcond.
> >>
> >> I can think of 2 natural ways to improve things: either implement vector
> >> comparisons in the ARM backend (possibly by forwarding to their existing
> >> code for vcond), or in the generic expansion code try using vcond if the
> >> direct comparison opcode is not provided.
> >>
> >> We can temporarily revert my patch, but I would like it to be temporary.
> >> Since aarch64 seems to handle the same code just fine, maybe someone who
> >> knows arm could copy the relevant code over?
> >>
> >> Does my message make sense, do people have comments?
> >
> > So what complicates things now (and to some extent pre-existed when you
> > used AVX512 which _could_ operate on boolean vectors) is that we
> > have split out the condition from VEC_COND_EXPR to separate stmts
> > but we do not expect backends to be able to code-generate the separate
> > form - instead we rely on the ISEL pass to trasform VEC_COND_EXPRs
> > to .VCOND[U] "merging" the compares again. Now that process breaks
> > down once we have things like _9 = _1 | _3; - at some point I argued
> > that we should handle vector compares [and operations on boolean vectors]
> > as well in ISEL but then when it came up again for some reason I
> > disregarded that again.
> >
> > Thus - we don't want to go back to fixing up the generic expansion code
> > (which looks at one instruction at a time and is restricted by TER
> > single-use
> > restrictions).
>
> Here, it would only be handling comparisons left over by ISEL because they
> do not feed a VEC_COND_EXPR, maybe not so bad. Handling it in ISEL would
> be more consistent, but then we may have to teach the vector lowering pass
> about this.
>
> > Instead we want to deal with this in ISEL which should
> > behave more intelligently. In the above case it might involve turning
> > the _1 and _3 defs into .VCOND [with different result type], doing
> > _9 in that type and then somehow dealing with _7 ... but this eventually
> > means undoing the match simplification that introduced the code?
>
> For targets that do not have compact boolean vectors,
> VEC_COND_EXPR<*,-1,0> does nothing, it is just there as a technicality.
> Removing it to allow more simplifications makes sense, reintroducing it
> for expansion can make sense as well, I think it is ok if the second one
> reverses the first, but very locally, without having to propagate a change
> of type to the uses. So on ARM we could turn _1 and _3 into .VCOND
> producing bool:32 vectors, and replace _7 with a VIEW_CONVERT_EXPR. Or
> does using bool vectors like that seem bad? (maybe they aren't guaranteed
> to be equivalent to signed integer vectors with values 0 and -1 and we
> need to query the target to know if that is the case, or introduce an
> extra .VCOND)
>
> Also, we only want to replace comparisons with .VCOND if the target
> doesn't handle the comparison directly. For AVX512, we do want to produce
> SImode bool vectors (for k* registers) and operate on them directly,
> that's the whole point of introducing the bool vectors (if bool vectors
> were only used to feed VEC_COND_EXPR and were all turned into .VCOND
> before expansion, I
Re: VEC_COND_EXPR optimizations v2
On Fri, 7 Aug 2020, Richard Biener wrote:
On Thu, Aug 6, 2020 at 8:07 PM Marc Glisse wrote:
On Thu, 6 Aug 2020, Christophe Lyon wrote:
Was I on the right track configuring with
--target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
--with-fpu=neon-fp16
then compiling without any special option?
Maybe you also need --with-float=hard, I don't remember if it's
implied by the 'hf' target suffix
Thanks! That's what I was missing to reproduce the issue. Now I can
reproduce it with just
typedef unsigned int vec __attribute__((vector_size(16)));
typedef int vi __attribute__((vector_size(16)));
vi f(vec a,vec b){
return a==5 | b==7;
}
with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2 -fdisable-tree-forwprop3
-O1
_1 = a_5(D) == { 5, 5, 5, 5 };
_3 = b_6(D) == { 7, 7, 7, 7 };
_9 = _1 | _3;
_7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }, 107);
we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
false), while with -fdisable-tree-forwprop4 we do manage to expand
_2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
112);
It doesn't make much sense to me that we can expand the more complicated
form and not the simpler form of the same operation (both compare a to 5
and produce a vector of -1 or 0 of the same size), especially when the
target has an instruction (vceq) that does just what we want.
Introducing boolean vectors was fine, but I think they should be real
types, that we can operate on, not be forced to appear only as the first
argument of a vcond.
I can think of 2 natural ways to improve things: either implement vector
comparisons in the ARM backend (possibly by forwarding to their existing
code for vcond), or in the generic expansion code try using vcond if the
direct comparison opcode is not provided.
We can temporarily revert my patch, but I would like it to be temporary.
Since aarch64 seems to handle the same code just fine, maybe someone who
knows arm could copy the relevant code over?
Does my message make sense, do people have comments?
So what complicates things now (and to some extent pre-existed when you
used AVX512 which _could_ operate on boolean vectors) is that we
have split out the condition from VEC_COND_EXPR to separate stmts
but we do not expect backends to be able to code-generate the separate
form - instead we rely on the ISEL pass to trasform VEC_COND_EXPRs
to .VCOND[U] "merging" the compares again. Now that process breaks
down once we have things like _9 = _1 | _3; - at some point I argued
that we should handle vector compares [and operations on boolean vectors]
as well in ISEL but then when it came up again for some reason I
disregarded that again.
Thus - we don't want to go back to fixing up the generic expansion code
(which looks at one instruction at a time and is restricted by TER single-use
restrictions).
Here, it would only be handling comparisons left over by ISEL because they
do not feed a VEC_COND_EXPR, maybe not so bad. Handling it in ISEL would
be more consistent, but then we may have to teach the vector lowering pass
about this.
Instead we want to deal with this in ISEL which should
behave more intelligently. In the above case it might involve turning
the _1 and _3 defs into .VCOND [with different result type], doing
_9 in that type and then somehow dealing with _7 ... but this eventually
means undoing the match simplification that introduced the code?
For targets that do not have compact boolean vectors,
VEC_COND_EXPR<*,-1,0> does nothing, it is just there as a technicality.
Removing it to allow more simplifications makes sense, reintroducing it
for expansion can make sense as well, I think it is ok if the second one
reverses the first, but very locally, without having to propagate a change
of type to the uses. So on ARM we could turn _1 and _3 into .VCOND
producing bool:32 vectors, and replace _7 with a VIEW_CONVERT_EXPR. Or
does using bool vectors like that seem bad? (maybe they aren't guaranteed
to be equivalent to signed integer vectors with values 0 and -1 and we
need to query the target to know if that is the case, or introduce an
extra .VCOND)
Also, we only want to replace comparisons with .VCOND if the target
doesn't handle the comparison directly. For AVX512, we do want to produce
SImode bool vectors (for k* registers) and operate on them directly,
that's the whole point of introducing the bool vectors (if bool vectors
were only used to feed VEC_COND_EXPR and were all turned into .VCOND
before expansion, I don't see the point of specifying different types for
bool vectors for AVX512 vs non-AVX512, as it would make no difference on
what is passed to the backend).
I think targets should provide some number of operations, including for
instance bit_and and bit_ior on bool vectors, and be a bit consistent
about what they provide, it becomes unmanageable in the middle-end
otherwise...
--
Marc Glisse
Re: VEC_COND_EXPR optimizations v2
On Thu, Aug 6, 2020 at 8:07 PM Marc Glisse wrote:
>
> On Thu, 6 Aug 2020, Christophe Lyon wrote:
>
> >> Was I on the right track configuring with
> >> --target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
> >> --with-fpu=neon-fp16
> >> then compiling without any special option?
> >
> > Maybe you also need --with-float=hard, I don't remember if it's
> > implied by the 'hf' target suffix
>
> Thanks! That's what I was missing to reproduce the issue. Now I can
> reproduce it with just
>
> typedef unsigned int vec __attribute__((vector_size(16)));
> typedef int vi __attribute__((vector_size(16)));
> vi f(vec a,vec b){
> return a==5 | b==7;
> }
>
> with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2
> -fdisable-tree-forwprop3 -O1
>
>_1 = a_5(D) == { 5, 5, 5, 5 };
>_3 = b_6(D) == { 7, 7, 7, 7 };
>_9 = _1 | _3;
>_7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }, 107);
>
> we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
> false), while with -fdisable-tree-forwprop4 we do manage to expand
>
>_2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
> 112);
>
> It doesn't make much sense to me that we can expand the more complicated
> form and not the simpler form of the same operation (both compare a to 5
> and produce a vector of -1 or 0 of the same size), especially when the
> target has an instruction (vceq) that does just what we want.
>
> Introducing boolean vectors was fine, but I think they should be real
> types, that we can operate on, not be forced to appear only as the first
> argument of a vcond.
>
> I can think of 2 natural ways to improve things: either implement vector
> comparisons in the ARM backend (possibly by forwarding to their existing
> code for vcond), or in the generic expansion code try using vcond if the
> direct comparison opcode is not provided.
>
> We can temporarily revert my patch, but I would like it to be temporary.
> Since aarch64 seems to handle the same code just fine, maybe someone who
> knows arm could copy the relevant code over?
>
> Does my message make sense, do people have comments?
So what complicates things now (and to some extent pre-existed when you
used AVX512 which _could_ operate on boolean vectors) is that we
have split out the condition from VEC_COND_EXPR to separate stmts
but we do not expect backends to be able to code-generate the separate
form - instead we rely on the ISEL pass to trasform VEC_COND_EXPRs
to .VCOND[U] "merging" the compares again. Now that process breaks
down once we have things like _9 = _1 | _3; - at some point I argued
that we should handle vector compares [and operations on boolean vectors]
as well in ISEL but then when it came up again for some reason I
disregarded that again.
Thus - we don't want to go back to fixing up the generic expansion code
(which looks at one instruction at a time and is restricted by TER single-use
restrictions). Instead we want to deal with this in ISEL which should
behave more intelligently. In the above case it might involve turning
the _1 and _3 defs into .VCOND [with different result type], doing
_9 in that type and then somehow dealing with _7 ... but this eventually
means undoing the match simplification that introduced the code?
Not sure if that helps though.
Richard.
> --
> Marc Glisse
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020, Christophe Lyon wrote:
Was I on the right track configuring with
--target=arm-none-linux-gnueabihf --with-cpu=cortex-a9
--with-fpu=neon-fp16
then compiling without any special option?
Maybe you also need --with-float=hard, I don't remember if it's
implied by the 'hf' target suffix
Thanks! That's what I was missing to reproduce the issue. Now I can
reproduce it with just
typedef unsigned int vec __attribute__((vector_size(16)));
typedef int vi __attribute__((vector_size(16)));
vi f(vec a,vec b){
return a==5 | b==7;
}
with -fdisable-tree-forwprop1 -fdisable-tree-forwprop2 -fdisable-tree-forwprop3
-O1
_1 = a_5(D) == { 5, 5, 5, 5 };
_3 = b_6(D) == { 7, 7, 7, 7 };
_9 = _1 | _3;
_7 = .VCOND (_9, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }, 107);
we fail to expand the equality comparison (expand_vec_cmp_expr_p returns
false), while with -fdisable-tree-forwprop4 we do manage to expand
_2 = .VCONDU (a_5(D), { 5, 5, 5, 5 }, { -1, -1, -1, -1 }, { 0, 0, 0, 0 },
112);
It doesn't make much sense to me that we can expand the more complicated
form and not the simpler form of the same operation (both compare a to 5
and produce a vector of -1 or 0 of the same size), especially when the
target has an instruction (vceq) that does just what we want.
Introducing boolean vectors was fine, but I think they should be real
types, that we can operate on, not be forced to appear only as the first
argument of a vcond.
I can think of 2 natural ways to improve things: either implement vector
comparisons in the ARM backend (possibly by forwarding to their existing
code for vcond), or in the generic expansion code try using vcond if the
direct comparison opcode is not provided.
We can temporarily revert my patch, but I would like it to be temporary.
Since aarch64 seems to handle the same code just fine, maybe someone who
knows arm could copy the relevant code over?
Does my message make sense, do people have comments?
--
Marc Glisse
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020 at 13:42, Marc Glisse wrote: > > On Thu, 6 Aug 2020, Christophe Lyon wrote: > > > On Thu, 6 Aug 2020 at 11:06, Marc Glisse wrote: > >> > >> On Thu, 6 Aug 2020, Christophe Lyon wrote: > >> > > 2020-08-05 Marc Glisse > > > > PR tree-optimization/95906 > > PR target/70314 > > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > > (op (c ? a : b)): Update to match the new transformations. > > > > * gcc.dg/tree-ssa/andnot-2.c: New file. > > * gcc.dg/tree-ssa/pr95906.c: Likewise. > > * gcc.target/i386/pr70314.c: Likewise. > > > >>> > >>> I think this patch is causing several ICEs on arm-none-linux-gnueabihf > >>> --with-cpu cortex-a9 --with-fpu neon-fp16: > >>> Executed from: gcc.c-torture/compile/compile.exp > >>>gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer > >>> -funroll-loops -fpeel-loops -ftracer -finline-functions (internal > >>> compiler error) > >>>gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) > >>> Executed from: gcc.dg/dg.exp > >>>gcc.dg/pr87746.c (internal compiler error) > >>> Executed from: gcc.dg/tree-ssa/tree-ssa.exp > >>>gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) > >> > >> I tried a cross from x86_64-linux with current master > >> > >> .../configure --target=arm-none-linux-gnueabihf --enable-languages=c,c++ > >> --with-system-zlib --disable-nls --with-cpu=cortex-a9 --with-fpu=neon-fp16 > >> make > >> > >> it stops at some point with an error, but I have xgcc and cc1 in > >> build/gcc. > >> > >> I copied 2 of the testcases and compiled > >> > >> ./xgcc pr87746.c -Ofast -S -B. > >> ./xgcc -O3 -fdump-tree-ifcvt-details-blocks-details ifc-cd.c -S -B. > >> > >> without getting any ICE. > > > > Sorry for the delay, I had to reproduce the problem manually. > >> > >> Is there a machine on the compile farm where this is easy to reproduce? > > I don't think there is any arm machine in the compile farm. > > > >> Or could you attach the .optimized dump that corresponds to the > >> backtrace below? It looks like we end up with a comparison with an > >> unexpected return type. > >> > > > > I've compiled pr87746.c with -fdump-tree-ifcvt-details-blocks-details, > > here is the log. > > Is that what you need? > > Thanks. > The one from -fdump-tree-optimized would be closer to the ICE. Here it is. > Though it would also be convenient to know which stmt is being expanded > when we ICE, etc. I think it's when expanding _96 = _86 | _95; (that the value of "stmt" in expand_gimple_stmt_1 when we enter do_store_flag > Was I on the right track configuring with > --target=arm-none-linux-gnueabihf --with-cpu=cortex-a9 > --with-fpu=neon-fp16 > then compiling without any special option? > Maybe you also need --with-float=hard, I don't remember if it's implied by the 'hf' target suffix (I saw similar problems with arm-none-linux-gnueabi anyway) > > Thanks, > > > > Christophe > > > >>> Executed from: gcc.dg/vect/vect.exp > >>>gcc.dg/vect/pr59591-1.c (internal compiler error) > >>>gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler > >>> error) > >>>gcc.dg/vect/pr86927.c (internal compiler error) > >>>gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) > >>>gcc.dg/vect/slp-cond-5.c (internal compiler error) > >>>gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler > >>> error) > >>>gcc.dg/vect/vect-23.c (internal compiler error) > >>>gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) > >>>gcc.dg/vect/vect-24.c (internal compiler error) > >>>gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) > >>>gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) > >>>gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal > >>> compiler error) > >>> > >>> Backtrace for gcc.c-torture/compile/20160205-1.c -O3 > >>> -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > >>> -finline-functions > >>> during RTL pass: expand > >>> /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal > >>> compiler error: in do_store_flag, at expr.c:12259 > >>> 0x8feb26 do_store_flag > >>>/gcc/expr.c:12259 > >>> 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > >>> expand_modifier) > >>>/gcc/expr.c:9617 > >>> 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > >>> expand_modifier, rtx_def**, bool) > >>>/gcc/expr.c:10159 > >>> 0x91174e expand_expr > >>>/gcc/expr.h:282 > >>> 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > >>> rtx_def**, expand_modifier) > >>>/gcc/expr.c:8065 > >>> 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > >>> expand_modifier) > >>>/gcc/expr.c:9950 > >>> 0x908cd0
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020, Christophe Lyon wrote: On Thu, 6 Aug 2020 at 11:06, Marc Glisse wrote: On Thu, 6 Aug 2020, Christophe Lyon wrote: 2020-08-05 Marc Glisse PR tree-optimization/95906 PR target/70314 * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. (op (c ? a : b)): Update to match the new transformations. * gcc.dg/tree-ssa/andnot-2.c: New file. * gcc.dg/tree-ssa/pr95906.c: Likewise. * gcc.target/i386/pr70314.c: Likewise. I think this patch is causing several ICEs on arm-none-linux-gnueabihf --with-cpu cortex-a9 --with-fpu neon-fp16: Executed from: gcc.c-torture/compile/compile.exp gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) Executed from: gcc.dg/dg.exp gcc.dg/pr87746.c (internal compiler error) Executed from: gcc.dg/tree-ssa/tree-ssa.exp gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) I tried a cross from x86_64-linux with current master .../configure --target=arm-none-linux-gnueabihf --enable-languages=c,c++ --with-system-zlib --disable-nls --with-cpu=cortex-a9 --with-fpu=neon-fp16 make it stops at some point with an error, but I have xgcc and cc1 in build/gcc. I copied 2 of the testcases and compiled ./xgcc pr87746.c -Ofast -S -B. ./xgcc -O3 -fdump-tree-ifcvt-details-blocks-details ifc-cd.c -S -B. without getting any ICE. Sorry for the delay, I had to reproduce the problem manually. Is there a machine on the compile farm where this is easy to reproduce? I don't think there is any arm machine in the compile farm. Or could you attach the .optimized dump that corresponds to the backtrace below? It looks like we end up with a comparison with an unexpected return type. I've compiled pr87746.c with -fdump-tree-ifcvt-details-blocks-details, here is the log. Is that what you need? Thanks. The one from -fdump-tree-optimized would be closer to the ICE. Though it would also be convenient to know which stmt is being expanded when we ICE, etc. Was I on the right track configuring with --target=arm-none-linux-gnueabihf --with-cpu=cortex-a9 --with-fpu=neon-fp16 then compiling without any special option? Thanks, Christophe Executed from: gcc.dg/vect/vect.exp gcc.dg/vect/pr59591-1.c (internal compiler error) gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/pr86927.c (internal compiler error) gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/slp-cond-5.c (internal compiler error) gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-23.c (internal compiler error) gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-24.c (internal compiler error) gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal compiler error) Backtrace for gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions during RTL pass: expand /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal compiler error: in do_store_flag, at expr.c:12259 0x8feb26 do_store_flag /gcc/expr.c:12259 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9617 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020 at 11:06, Marc Glisse wrote: > > On Thu, 6 Aug 2020, Christophe Lyon wrote: > > >>> 2020-08-05 Marc Glisse > >>> > >>> PR tree-optimization/95906 > >>> PR target/70314 > >>> * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > >>> (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > >>> (op (c ? a : b)): Update to match the new transformations. > >>> > >>> * gcc.dg/tree-ssa/andnot-2.c: New file. > >>> * gcc.dg/tree-ssa/pr95906.c: Likewise. > >>> * gcc.target/i386/pr70314.c: Likewise. > >>> > > > > I think this patch is causing several ICEs on arm-none-linux-gnueabihf > > --with-cpu cortex-a9 --with-fpu neon-fp16: > > Executed from: gcc.c-torture/compile/compile.exp > >gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer > > -funroll-loops -fpeel-loops -ftracer -finline-functions (internal > > compiler error) > >gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) > > Executed from: gcc.dg/dg.exp > >gcc.dg/pr87746.c (internal compiler error) > > Executed from: gcc.dg/tree-ssa/tree-ssa.exp > >gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) > > I tried a cross from x86_64-linux with current master > > .../configure --target=arm-none-linux-gnueabihf --enable-languages=c,c++ > --with-system-zlib --disable-nls --with-cpu=cortex-a9 --with-fpu=neon-fp16 > make > > it stops at some point with an error, but I have xgcc and cc1 in > build/gcc. > > I copied 2 of the testcases and compiled > > ./xgcc pr87746.c -Ofast -S -B. > ./xgcc -O3 -fdump-tree-ifcvt-details-blocks-details ifc-cd.c -S -B. > > without getting any ICE. Sorry for the delay, I had to reproduce the problem manually. > > Is there a machine on the compile farm where this is easy to reproduce? I don't think there is any arm machine in the compile farm. > Or could you attach the .optimized dump that corresponds to the > backtrace below? It looks like we end up with a comparison with an > unexpected return type. > I've compiled pr87746.c with -fdump-tree-ifcvt-details-blocks-details, here is the log. Is that what you need? Thanks, Christophe > > Executed from: gcc.dg/vect/vect.exp > >gcc.dg/vect/pr59591-1.c (internal compiler error) > >gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) > >gcc.dg/vect/pr86927.c (internal compiler error) > >gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) > >gcc.dg/vect/slp-cond-5.c (internal compiler error) > >gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler > > error) > >gcc.dg/vect/vect-23.c (internal compiler error) > >gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) > >gcc.dg/vect/vect-24.c (internal compiler error) > >gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) > >gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) > >gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal > > compiler error) > > > > Backtrace for gcc.c-torture/compile/20160205-1.c -O3 > > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > > -finline-functions > > during RTL pass: expand > > /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal > > compiler error: in do_store_flag, at expr.c:12259 > > 0x8feb26 do_store_flag > >/gcc/expr.c:12259 > > 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > > expand_modifier) > >/gcc/expr.c:9617 > > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > > expand_modifier, rtx_def**, bool) > >/gcc/expr.c:10159 > > 0x91174e expand_expr > >/gcc/expr.h:282 > > 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > > rtx_def**, expand_modifier) > >/gcc/expr.c:8065 > > 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > > expand_modifier) > >/gcc/expr.c:9950 > > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > > expand_modifier, rtx_def**, bool) > >/gcc/expr.c:10159 > > 0x91174e expand_expr > >/gcc/expr.h:282 > > 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > > rtx_def**, expand_modifier) > >/gcc/expr.c:8065 > > 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > > expand_modifier) > >/gcc/expr.c:9950 > > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > > expand_modifier, rtx_def**, bool) > >/gcc/expr.c:10159 > > 0x91174e expand_expr > >/gcc/expr.h:282 > > 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > > rtx_def**, expand_modifier) > >/gcc/expr.c:8065 > > 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > > expand_modifier) > >/gcc/expr.c:9950 > > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > > expand_modifier, rtx_def**, bool) > >
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020, Richard Biener wrote: On Thu, Aug 6, 2020 at 10:17 AM Christophe Lyon wrote: Hi, On Wed, 5 Aug 2020 at 16:24, Richard Biener via Gcc-patches wrote: On Wed, Aug 5, 2020 at 3:33 PM Marc Glisse wrote: New version that passed bootstrap+regtest during the night. When vector comparisons were forced to use vec_cond_expr, we lost a number of optimizations (my fault for not adding enough testcases to prevent that). This patch tries to unwrap vec_cond_expr a bit so some optimizations can still happen. I wasn't planning to add all those transformations together, but adding one caused a regression, whose fix introduced a second regression, etc. Restricting to constant folding would not be sufficient, we also need at least things like X|0 or X The transformations are quite conservative with :s and folding only if everything simplifies, we may want to relax this later. And of course we are going to miss things like a?b:c + a?c:b -> b+c. In terms of number of operations, some transformations turning 2 VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not look like a gain... I expect the bit_not disappears in most cases, and VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR. I am a bit confused that with avx512 we get types like "vector(4) " with :2 and not :1 (is it a hack so true is 1 and not -1?), but that doesn't matter for this patch. OK. Thanks, Richard. 2020-08-05 Marc Glisse PR tree-optimization/95906 PR target/70314 * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. (op (c ? a : b)): Update to match the new transformations. * gcc.dg/tree-ssa/andnot-2.c: New file. * gcc.dg/tree-ssa/pr95906.c: Likewise. * gcc.target/i386/pr70314.c: Likewise. I think this patch is causing several ICEs on arm-none-linux-gnueabihf --with-cpu cortex-a9 --with-fpu neon-fp16: Executed from: gcc.c-torture/compile/compile.exp gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) Executed from: gcc.dg/dg.exp gcc.dg/pr87746.c (internal compiler error) Executed from: gcc.dg/tree-ssa/tree-ssa.exp gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) Executed from: gcc.dg/vect/vect.exp gcc.dg/vect/pr59591-1.c (internal compiler error) gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/pr86927.c (internal compiler error) gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/slp-cond-5.c (internal compiler error) gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-23.c (internal compiler error) gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-24.c (internal compiler error) gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal compiler error) Backtrace for gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions during RTL pass: expand /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal compiler error: in do_store_flag, at expr.c:12259 0x8feb26 do_store_flag /gcc/expr.c:12259 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9617 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier,
Re: VEC_COND_EXPR optimizations v2
On Thu, Aug 6, 2020 at 10:17 AM Christophe Lyon wrote: > > Hi, > > > On Wed, 5 Aug 2020 at 16:24, Richard Biener via Gcc-patches > wrote: > > > > On Wed, Aug 5, 2020 at 3:33 PM Marc Glisse wrote: > > > > > > New version that passed bootstrap+regtest during the night. > > > > > > When vector comparisons were forced to use vec_cond_expr, we lost a > > > number of > > > optimizations (my fault for not adding enough testcases to prevent that). > > > This patch tries to unwrap vec_cond_expr a bit so some optimizations can > > > still happen. > > > > > > I wasn't planning to add all those transformations together, but adding > > > one > > > caused a regression, whose fix introduced a second regression, etc. > > > > > > Restricting to constant folding would not be sufficient, we also need at > > > least things like X|0 or X The transformations are quite conservative > > > with :s and folding only if everything simplifies, we may want to relax > > > this later. And of course we are going to miss things like a?b:c + a?c:b > > > -> b+c. > > > > > > In terms of number of operations, some transformations turning 2 > > > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not > > > look > > > like a gain... I expect the bit_not disappears in most cases, and > > > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR. > > > > > > I am a bit confused that with avx512 we get types like "vector(4) > > > " with :2 and not :1 (is it a hack so true is 1 and not > > > -1?), but that doesn't matter for this patch. > > > > OK. > > > > Thanks, > > Richard. > > > > > 2020-08-05 Marc Glisse > > > > > > PR tree-optimization/95906 > > > PR target/70314 > > > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > > > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > > > (op (c ? a : b)): Update to match the new transformations. > > > > > > * gcc.dg/tree-ssa/andnot-2.c: New file. > > > * gcc.dg/tree-ssa/pr95906.c: Likewise. > > > * gcc.target/i386/pr70314.c: Likewise. > > > > > I think this patch is causing several ICEs on arm-none-linux-gnueabihf > --with-cpu cortex-a9 --with-fpu neon-fp16: > Executed from: gcc.c-torture/compile/compile.exp > gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer > -funroll-loops -fpeel-loops -ftracer -finline-functions (internal > compiler error) > gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) > Executed from: gcc.dg/dg.exp > gcc.dg/pr87746.c (internal compiler error) > Executed from: gcc.dg/tree-ssa/tree-ssa.exp > gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) > Executed from: gcc.dg/vect/vect.exp > gcc.dg/vect/pr59591-1.c (internal compiler error) > gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) > gcc.dg/vect/pr86927.c (internal compiler error) > gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) > gcc.dg/vect/slp-cond-5.c (internal compiler error) > gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler error) > gcc.dg/vect/vect-23.c (internal compiler error) > gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) > gcc.dg/vect/vect-24.c (internal compiler error) > gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) > gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) > gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal > compiler error) > > Backtrace for gcc.c-torture/compile/20160205-1.c -O3 > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > -finline-functions > during RTL pass: expand > /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal > compiler error: in do_store_flag, at expr.c:12259 > 0x8feb26 do_store_flag > /gcc/expr.c:12259 > 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > expand_modifier) > /gcc/expr.c:9617 > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > expand_modifier, rtx_def**, bool) > /gcc/expr.c:10159 > 0x91174e expand_expr > /gcc/expr.h:282 > 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > rtx_def**, expand_modifier) > /gcc/expr.c:8065 > 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > expand_modifier) > /gcc/expr.c:9950 > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > expand_modifier, rtx_def**, bool) > /gcc/expr.c:10159 > 0x91174e expand_expr > /gcc/expr.h:282 > 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, > rtx_def**, expand_modifier) > /gcc/expr.c:8065 > 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, > expand_modifier) > /gcc/expr.c:9950 > 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > expand_modifier, rtx_def**, bool) > /gcc/expr.c:10159 > 0x91174e
Re: VEC_COND_EXPR optimizations v2
On Thu, 6 Aug 2020, Christophe Lyon wrote: 2020-08-05 Marc Glisse PR tree-optimization/95906 PR target/70314 * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. (op (c ? a : b)): Update to match the new transformations. * gcc.dg/tree-ssa/andnot-2.c: New file. * gcc.dg/tree-ssa/pr95906.c: Likewise. * gcc.target/i386/pr70314.c: Likewise. I think this patch is causing several ICEs on arm-none-linux-gnueabihf --with-cpu cortex-a9 --with-fpu neon-fp16: Executed from: gcc.c-torture/compile/compile.exp gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) Executed from: gcc.dg/dg.exp gcc.dg/pr87746.c (internal compiler error) Executed from: gcc.dg/tree-ssa/tree-ssa.exp gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) I tried a cross from x86_64-linux with current master .../configure --target=arm-none-linux-gnueabihf --enable-languages=c,c++ --with-system-zlib --disable-nls --with-cpu=cortex-a9 --with-fpu=neon-fp16 make it stops at some point with an error, but I have xgcc and cc1 in build/gcc. I copied 2 of the testcases and compiled ./xgcc pr87746.c -Ofast -S -B. ./xgcc -O3 -fdump-tree-ifcvt-details-blocks-details ifc-cd.c -S -B. without getting any ICE. Is there a machine on the compile farm where this is easy to reproduce? Or could you attach the .optimized dump that corresponds to the backtrace below? It looks like we end up with a comparison with an unexpected return type. Executed from: gcc.dg/vect/vect.exp gcc.dg/vect/pr59591-1.c (internal compiler error) gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/pr86927.c (internal compiler error) gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/slp-cond-5.c (internal compiler error) gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-23.c (internal compiler error) gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-24.c (internal compiler error) gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal compiler error) Backtrace for gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions during RTL pass: expand /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal compiler error: in do_store_flag, at expr.c:12259 0x8feb26 do_store_flag /gcc/expr.c:12259 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9617 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 Christophe -- Marc Glisse
Re: VEC_COND_EXPR optimizations v2
Hi, On Wed, 5 Aug 2020 at 16:24, Richard Biener via Gcc-patches wrote: > > On Wed, Aug 5, 2020 at 3:33 PM Marc Glisse wrote: > > > > New version that passed bootstrap+regtest during the night. > > > > When vector comparisons were forced to use vec_cond_expr, we lost a number > > of > > optimizations (my fault for not adding enough testcases to prevent that). > > This patch tries to unwrap vec_cond_expr a bit so some optimizations can > > still happen. > > > > I wasn't planning to add all those transformations together, but adding one > > caused a regression, whose fix introduced a second regression, etc. > > > > Restricting to constant folding would not be sufficient, we also need at > > least things like X|0 or X The transformations are quite conservative > > with :s and folding only if everything simplifies, we may want to relax > > this later. And of course we are going to miss things like a?b:c + a?c:b > > -> b+c. > > > > In terms of number of operations, some transformations turning 2 > > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not > > look > > like a gain... I expect the bit_not disappears in most cases, and > > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR. > > > > I am a bit confused that with avx512 we get types like "vector(4) > > " with :2 and not :1 (is it a hack so true is 1 and not > > -1?), but that doesn't matter for this patch. > > OK. > > Thanks, > Richard. > > > 2020-08-05 Marc Glisse > > > > PR tree-optimization/95906 > > PR target/70314 > > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > > (op (c ? a : b)): Update to match the new transformations. > > > > * gcc.dg/tree-ssa/andnot-2.c: New file. > > * gcc.dg/tree-ssa/pr95906.c: Likewise. > > * gcc.target/i386/pr70314.c: Likewise. > > I think this patch is causing several ICEs on arm-none-linux-gnueabihf --with-cpu cortex-a9 --with-fpu neon-fp16: Executed from: gcc.c-torture/compile/compile.exp gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/compile/20160205-1.c -O3 -g (internal compiler error) Executed from: gcc.dg/dg.exp gcc.dg/pr87746.c (internal compiler error) Executed from: gcc.dg/tree-ssa/tree-ssa.exp gcc.dg/tree-ssa/ifc-cd.c (internal compiler error) Executed from: gcc.dg/vect/vect.exp gcc.dg/vect/pr59591-1.c (internal compiler error) gcc.dg/vect/pr59591-1.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/pr86927.c (internal compiler error) gcc.dg/vect/pr86927.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/slp-cond-5.c (internal compiler error) gcc.dg/vect/slp-cond-5.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-23.c (internal compiler error) gcc.dg/vect/vect-23.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-24.c (internal compiler error) gcc.dg/vect/vect-24.c -flto -ffat-lto-objects (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c (internal compiler error) gcc.dg/vect/vect-cond-reduc-6.c -flto -ffat-lto-objects (internal compiler error) Backtrace for gcc.c-torture/compile/20160205-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions during RTL pass: expand /gcc/testsuite/gcc.c-torture/compile/20160205-1.c:2:5: internal compiler error: in do_store_flag, at expr.c:12259 0x8feb26 do_store_flag /gcc/expr.c:12259 0x900201 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9617 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) /gcc/expr.c:10159 0x91174e expand_expr /gcc/expr.h:282 0x91174e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier) /gcc/expr.c:8065 0x8ff543 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier) /gcc/expr.c:9950 0x908cd0
Re: VEC_COND_EXPR optimizations v2
On Wed, Aug 5, 2020 at 3:33 PM Marc Glisse wrote: > > New version that passed bootstrap+regtest during the night. > > When vector comparisons were forced to use vec_cond_expr, we lost a number of > optimizations (my fault for not adding enough testcases to prevent that). > This patch tries to unwrap vec_cond_expr a bit so some optimizations can > still happen. > > I wasn't planning to add all those transformations together, but adding one > caused a regression, whose fix introduced a second regression, etc. > > Restricting to constant folding would not be sufficient, we also need at > least things like X|0 or X The transformations are quite conservative > with :s and folding only if everything simplifies, we may want to relax > this later. And of course we are going to miss things like a?b:c + a?c:b > -> b+c. > > In terms of number of operations, some transformations turning 2 > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not look > like a gain... I expect the bit_not disappears in most cases, and > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR. > > I am a bit confused that with avx512 we get types like "vector(4) > " with :2 and not :1 (is it a hack so true is 1 and not > -1?), but that doesn't matter for this patch. OK. Thanks, Richard. > 2020-08-05 Marc Glisse > > PR tree-optimization/95906 > PR target/70314 > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > (op (c ? a : b)): Update to match the new transformations. > > * gcc.dg/tree-ssa/andnot-2.c: New file. > * gcc.dg/tree-ssa/pr95906.c: Likewise. > * gcc.target/i386/pr70314.c: Likewise. > > -- > Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, 31 Jul 2020, Marc Glisse wrote:
On Fri, 31 Jul 2020, Richard Sandiford wrote:
Marc Glisse writes:
On Fri, 31 Jul 2020, Richard Sandiford wrote:
Marc Glisse writes:
+/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
+ (simplify
+ (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
+ (with
+ {
+ tree rhs1, rhs2 = NULL;
+ rhs1 = fold_binary (op, type, @1, @3);
+ if (rhs1 && is_gimple_val (rhs1))
+ rhs2 = fold_binary (op, type, @2, @4);
+ }
+ (if (rhs2 && is_gimple_val (rhs2))
+(vec_cond @0 { rhs1; } { rhs2; })
+#endif
This one looks dangerous for potentially-trapping ops.
I would expect the operation not to be folded if it can trap. Is that too
optimistic?
Not sure TBH. I was thinking of “trapping” in the sense of raising
an IEEE exception, rather than in the could-throw/must-end-bb sense.
That's what I understood from your message :-)
I thought match.pd applied to things like FP addition as normal and
it was up to individual patterns to check the appropriate properties.
Yes, and in this case I am delegating that to fold_binary, which already
performs this check.
I tried with this C++ program
typedef double vecf __attribute__((vector_size(16)));
typedef long long veci __attribute__((vector_size(16)));
vecf f(veci c){
return (c?1.:2.)/(c?3.:7.);
}
the folding happens by default, but not with -frounding-math, which seems
like exactly what we want.
That was for rounding. -ftrapping-math does not disable folding of
typedef double vecf __attribute__((vector_size(16)));
typedef long long veci __attribute__((vector_size(16)));
vecf f(veci c){
vecf z={};
return (z+1)/(z+3);
}
despite a possible inexact flag, so it doesn't disable vec_cond_expr
folding either.
(not sure we want to fix this unless -fno-trapping-math becomes the
default)
--
Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, 31 Jul 2020, Richard Sandiford wrote:
Marc Glisse writes:
On Fri, 31 Jul 2020, Richard Sandiford wrote:
Marc Glisse writes:
+/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
+ (simplify
+ (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
+ (with
+ {
+ tree rhs1, rhs2 = NULL;
+ rhs1 = fold_binary (op, type, @1, @3);
+ if (rhs1 && is_gimple_val (rhs1))
+ rhs2 = fold_binary (op, type, @2, @4);
+ }
+ (if (rhs2 && is_gimple_val (rhs2))
+(vec_cond @0 { rhs1; } { rhs2; })
+#endif
This one looks dangerous for potentially-trapping ops.
I would expect the operation not to be folded if it can trap. Is that too
optimistic?
Not sure TBH. I was thinking of “trapping” in the sense of raising
an IEEE exception, rather than in the could-throw/must-end-bb sense.
That's what I understood from your message :-)
I thought match.pd applied to things like FP addition as normal and
it was up to individual patterns to check the appropriate properties.
Yes, and in this case I am delegating that to fold_binary, which already
performs this check.
I tried with this C++ program
typedef double vecf __attribute__((vector_size(16)));
typedef long long veci __attribute__((vector_size(16)));
vecf f(veci c){
return (c?1.:2.)/(c?3.:7.);
}
the folding happens by default, but not with -frounding-math, which seems
like exactly what we want.
--
Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, Jul 31, 2020 at 2:50 PM Richard Sandiford
wrote:
>
> Marc Glisse writes:
> > On Fri, 31 Jul 2020, Richard Sandiford wrote:
> >
> >> Marc Glisse writes:
> >>> +/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
> >>> + (simplify
> >>> + (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
> >>> + (with
> >>> + {
> >>> + tree rhs1, rhs2 = NULL;
> >>> + rhs1 = fold_binary (op, type, @1, @3);
> >>> + if (rhs1 && is_gimple_val (rhs1))
> >>> + rhs2 = fold_binary (op, type, @2, @4);
> >>> + }
> >>> + (if (rhs2 && is_gimple_val (rhs2))
> >>> +(vec_cond @0 { rhs1; } { rhs2; })
> >>> +#endif
> >>
> >> This one looks dangerous for potentially-trapping ops.
> >
> > I would expect the operation not to be folded if it can trap. Is that too
> > optimistic?
>
> Not sure TBH. I was thinking of “trapping” in the sense of raising
> an IEEE exception, rather than in the could-throw/must-end-bb sense.
> I thought match.pd applied to things like FP addition as normal and
> it was up to individual patterns to check the appropriate properties.
I think it can be indeed defered to the simplification of (op @0 @2)
because that would be invalid if it removes a IEEE exception.
The VEC_COND_EXPR itself cannot trap.
Richard.
> Thanks,
> Richard
Re: VEC_COND_EXPR optimizations
Marc Glisse writes:
> On Fri, 31 Jul 2020, Richard Sandiford wrote:
>
>> Marc Glisse writes:
>>> +/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
>>> + (simplify
>>> + (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
>>> + (with
>>> + {
>>> + tree rhs1, rhs2 = NULL;
>>> + rhs1 = fold_binary (op, type, @1, @3);
>>> + if (rhs1 && is_gimple_val (rhs1))
>>> + rhs2 = fold_binary (op, type, @2, @4);
>>> + }
>>> + (if (rhs2 && is_gimple_val (rhs2))
>>> +(vec_cond @0 { rhs1; } { rhs2; })
>>> +#endif
>>
>> This one looks dangerous for potentially-trapping ops.
>
> I would expect the operation not to be folded if it can trap. Is that too
> optimistic?
Not sure TBH. I was thinking of “trapping” in the sense of raising
an IEEE exception, rather than in the could-throw/must-end-bb sense.
I thought match.pd applied to things like FP addition as normal and
it was up to individual patterns to check the appropriate properties.
Thanks,
Richard
Re: VEC_COND_EXPR optimizations
On Fri, 31 Jul 2020, Richard Biener wrote:
On Fri, Jul 31, 2020 at 1:39 PM Richard Biener
wrote:
On Fri, Jul 31, 2020 at 1:35 PM Richard Biener
wrote:
On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse wrote:
When vector comparisons were forced to use vec_cond_expr, we lost a number
of optimizations (my fault for not adding enough testcases to prevent
that). This patch tries to unwrap vec_cond_expr a bit so some
optimizations can still happen.
I wasn't planning to add all those transformations together, but adding
one caused a regression, whose fix introduced a second regression, etc.
Using a simple fold_binary internally looks like an ok compromise to me.
It remains cheap enough (not recursive, and vector instructions are not
that frequent), while still allowing more than const_binop (X|0 or X for
instance). The transformations are quite conservative with :s and folding
only if everything simplifies, we may want to relax this later. And of
course we are going to miss things like a?b:c + a?c:b -> b+c.
In terms of number of operations, some transformations turning 2
VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
look like a gain... I expect the bit_not disappears in most cases, and
VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.
I am a bit confused that with avx512 we get types like "vector(4)
" with :2 and not :1 (is it a hack so true is 1 and not
-1?), but that doesn't matter for this patch.
Regtest+bootstrap on x86_64-pc-linux-gnu
+ (with
+ {
+ tree rhs1, rhs2 = NULL;
+ rhs1 = fold_binary (op, type, @1, @3);
+ if (rhs1 && is_gimple_val (rhs1))
+ rhs2 = fold_binary (op, type, @2, @3);
ICK. I guess a more match-and-simplify way would be
I was focused on avoiding recursion, but indeed that's independent, I
could have set a trivial valueize function for that.
(with
{
tree rhs1, rhs2;
gimple_match_op op (gimple_match_cond::UNCOND, op,
type, @1, @3);
if (op.resimplify (NULL, valueize)
Oh, so you would be ok with something that recurses without limit? I
thought we were avoiding this because it may allow some compile time
explosion with pathological examples.
&& gimple_simplified_result_is_gimple_val (op))
{
rhs1 = op.ops[0];
... other operand ...
}
now in theory we could invent some new syntax for this, like
(simplify
(op (vec_cond:s @0 @1 @2) @3)
(vec_cond @0 (op:x @1 @3) (op:x @2 @3)))
and pick something better instead of :x (:s is taken,
would be 'simplified', :c is taken would be 'constexpr', ...).
_Maybe_ just
(simplify
(op (vec_cond:s @0 @1 @2) @3)
(vec_cond:x @0 (op @1 @3) (op @2 @3)))
which would have the same practical meaning as passing
NULL for the seq argument to simplification - do not allow
any intermediate stmt to be generated.
Note I specifically do not like those if (it-simplifies) checks
because we already would code-generate those anyway. For
(simplify
(plus (vec_cond:s @0 @1 @2) @3)
(vec_cond @0 (plus @1 @3) (plus @2 @3)))
we get
res_op->set_op (VEC_COND_EXPR, type, 3);
res_op->ops[0] = captures[1];
res_op->ops[0] = unshare_expr (res_op->ops[0]);
{
tree _o1[2], _r1;
_o1[0] = captures[2];
_o1[1] = captures[4];
gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
tem_op.resimplify (lseq, valueize);
_r1 = maybe_push_res_to_seq (_op, lseq); ()
if (!_r1) return false;
res_op->ops[1] = _r1;
}
{
tree _o1[2], _r1;
_o1[0] = captures[3];
_o1[1] = captures[4];
gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
tem_op.resimplify (lseq, valueize);
_r1 = maybe_push_res_to_seq (_op, lseq); (***)
if (!_r1) return false;
res_op->ops[2] = _r1;
}
res_op->resimplify (lseq, valueize);
return true;
and the only change required would be to pass NULL to maybe_push_res_to_seq
here instead of lseq at the (***) marked points.
(simplify
(plus (vec_cond:s @0 @1 @2) @3)
(vec_cond:l @0 (plus @1 @3) (plus @2 @3)))
'l' for 'force leaf'. I'll see if I can quickly cme up with a patch.
Does it have a clear meaning for GENERIC? I guess that's probably not such
a big problem.
There are a number of transformations that we would like to perform "if
simplifies", but I don't know if it will always have exactly
this form of "if this part of the output pattern doesn't simplify, give
Re: VEC_COND_EXPR optimizations
On Fri, Jul 31, 2020 at 1:47 PM Richard Biener
wrote:
>
> On Fri, Jul 31, 2020 at 1:39 PM Richard Biener
> wrote:
> >
> > On Fri, Jul 31, 2020 at 1:35 PM Richard Biener
> > wrote:
> > >
> > > On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse wrote:
> > > >
> > > > When vector comparisons were forced to use vec_cond_expr, we lost a
> > > > number
> > > > of optimizations (my fault for not adding enough testcases to prevent
> > > > that). This patch tries to unwrap vec_cond_expr a bit so some
> > > > optimizations can still happen.
> > > >
> > > > I wasn't planning to add all those transformations together, but adding
> > > > one caused a regression, whose fix introduced a second regression, etc.
> > > >
> > > > Using a simple fold_binary internally looks like an ok compromise to me.
> > > > It remains cheap enough (not recursive, and vector instructions are not
> > > > that frequent), while still allowing more than const_binop (X|0 or X
> > > > for
> > > > instance). The transformations are quite conservative with :s and
> > > > folding
> > > > only if everything simplifies, we may want to relax this later. And of
> > > > course we are going to miss things like a?b:c + a?c:b -> b+c.
> > > >
> > > > In terms of number of operations, some transformations turning 2
> > > > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
> > > > look like a gain... I expect the bit_not disappears in most cases, and
> > > > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.
> > > >
> > > > I am a bit confused that with avx512 we get types like "vector(4)
> > > > " with :2 and not :1 (is it a hack so true is 1 and
> > > > not
> > > > -1?), but that doesn't matter for this patch.
> > > >
> > > > Regtest+bootstrap on x86_64-pc-linux-gnu
> > >
> > > + (with
> > > + {
> > > + tree rhs1, rhs2 = NULL;
> > > + rhs1 = fold_binary (op, type, @1, @3);
> > > + if (rhs1 && is_gimple_val (rhs1))
> > > + rhs2 = fold_binary (op, type, @2, @3);
> > >
> > > ICK. I guess a more match-and-simplify way would be
> > >
> > >(with
> > > {
> > > tree rhs1, rhs2;
> > > gimple_match_op op (gimple_match_cond::UNCOND, op,
> > > type, @1, @3);
> > > if (op.resimplify (NULL, valueize)
> > > && gimple_simplified_result_is_gimple_val (op))
> > >{
> > > rhs1 = op.ops[0];
> > > ... other operand ...
> > >}
> > >
> > > now in theory we could invent some new syntax for this, like
> > >
> > > (simplify
> > > (op (vec_cond:s @0 @1 @2) @3)
> > > (vec_cond @0 (op:x @1 @3) (op:x @2 @3)))
> > >
> > > and pick something better instead of :x (:s is taken,
> > > would be 'simplified', :c is taken would be 'constexpr', ...).
> > >
> > > _Maybe_ just
> > >
> > > (simplify
> > > (op (vec_cond:s @0 @1 @2) @3)
> > > (vec_cond:x @0 (op @1 @3) (op @2 @3)))
> > >
> > > which would have the same practical meaning as passing
> > > NULL for the seq argument to simplification - do not allow
> > > any intermediate stmt to be generated.
> >
> > Note I specifically do not like those if (it-simplifies) checks
> > because we already would code-generate those anyway. For
> >
> > (simplify
> > (plus (vec_cond:s @0 @1 @2) @3)
> > (vec_cond @0 (plus @1 @3) (plus @2 @3)))
> >
> > we get
> >
> > res_op->set_op (VEC_COND_EXPR, type, 3);
> > res_op->ops[0] = captures[1];
> > res_op->ops[0] = unshare_expr (res_op->ops[0]);
> > {
> > tree _o1[2], _r1;
> > _o1[0] = captures[2];
> > _o1[1] = captures[4];
> > gimple_match_op tem_op (res_op->cond.any_else
> > (), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
> > tem_op.resimplify (lseq, valueize);
> > _r1 = maybe_push_res_to_seq (_op, lseq); ()
> > if (!_r1) return false;
> > res_op->ops[1] = _r1;
> > }
> > {
> > tree _o1[2], _r1;
> > _o1[0] = captures[3];
> > _o1[1] = captures[4];
> > gimple_match_op tem_op (res_op->cond.any_else
> > (), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
> > tem_op.resimplify (lseq, valueize);
> > _r1 = maybe_push_res_to_seq (_op, lseq); (***)
> > if (!_r1) return false;
> > res_op->ops[2] = _r1;
> > }
> > res_op->resimplify (lseq, valueize);
> > return true;
> >
> > and the only change required would be to pass NULL to maybe_push_res_to_seq
> > here instead of lseq at the (***) marked points.
>
> (simplify
> (plus (vec_cond:s @0 @1 @2) @3)
> (vec_cond:l @0 (plus @1 @3) (plus @2 @3)))
>
> 'l' for 'force leaf'.
Re: VEC_COND_EXPR optimizations
On Fri, 31 Jul 2020, Richard Biener wrote: +/* (v ? w : 0) ? a : b is just (v & w) ? a : b */ +(simplify + (vec_cond (vec_cond:s @0 @3 integer_zerop) @1 @2) + (vec_cond (bit_and @0 @3) @1 @2)) Does something check automatically that @0 and @3 have compatible types? @0 should always have a vector boolean type and thus will not be generally compatible with @3. But OTOH then when you see (vec_cond (vec_cond ... then @3 must be vector boolean as well... But in theory with AVX512 the inner vec_cond could have a SImode condition @0 producing a regular V4SImode vector mask for an outer AVX512 SSE-style vec-cond and you then would get a mismatch. Ah, I thought the SSE-style vec_cond was impossible in AVX512 mode, at least I couldn't generate one in a few tests, but I didn't try very hard. So indeed better add a type compatibility check. Ok, it can't hurt. -- Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, Jul 31, 2020 at 1:39 PM Richard Biener
wrote:
>
> On Fri, Jul 31, 2020 at 1:35 PM Richard Biener
> wrote:
> >
> > On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse wrote:
> > >
> > > When vector comparisons were forced to use vec_cond_expr, we lost a number
> > > of optimizations (my fault for not adding enough testcases to prevent
> > > that). This patch tries to unwrap vec_cond_expr a bit so some
> > > optimizations can still happen.
> > >
> > > I wasn't planning to add all those transformations together, but adding
> > > one caused a regression, whose fix introduced a second regression, etc.
> > >
> > > Using a simple fold_binary internally looks like an ok compromise to me.
> > > It remains cheap enough (not recursive, and vector instructions are not
> > > that frequent), while still allowing more than const_binop (X|0 or X for
> > > instance). The transformations are quite conservative with :s and folding
> > > only if everything simplifies, we may want to relax this later. And of
> > > course we are going to miss things like a?b:c + a?c:b -> b+c.
> > >
> > > In terms of number of operations, some transformations turning 2
> > > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
> > > look like a gain... I expect the bit_not disappears in most cases, and
> > > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.
> > >
> > > I am a bit confused that with avx512 we get types like "vector(4)
> > > " with :2 and not :1 (is it a hack so true is 1 and not
> > > -1?), but that doesn't matter for this patch.
> > >
> > > Regtest+bootstrap on x86_64-pc-linux-gnu
> >
> > + (with
> > + {
> > + tree rhs1, rhs2 = NULL;
> > + rhs1 = fold_binary (op, type, @1, @3);
> > + if (rhs1 && is_gimple_val (rhs1))
> > + rhs2 = fold_binary (op, type, @2, @3);
> >
> > ICK. I guess a more match-and-simplify way would be
> >
> >(with
> > {
> > tree rhs1, rhs2;
> > gimple_match_op op (gimple_match_cond::UNCOND, op,
> > type, @1, @3);
> > if (op.resimplify (NULL, valueize)
> > && gimple_simplified_result_is_gimple_val (op))
> >{
> > rhs1 = op.ops[0];
> > ... other operand ...
> >}
> >
> > now in theory we could invent some new syntax for this, like
> >
> > (simplify
> > (op (vec_cond:s @0 @1 @2) @3)
> > (vec_cond @0 (op:x @1 @3) (op:x @2 @3)))
> >
> > and pick something better instead of :x (:s is taken,
> > would be 'simplified', :c is taken would be 'constexpr', ...).
> >
> > _Maybe_ just
> >
> > (simplify
> > (op (vec_cond:s @0 @1 @2) @3)
> > (vec_cond:x @0 (op @1 @3) (op @2 @3)))
> >
> > which would have the same practical meaning as passing
> > NULL for the seq argument to simplification - do not allow
> > any intermediate stmt to be generated.
>
> Note I specifically do not like those if (it-simplifies) checks
> because we already would code-generate those anyway. For
>
> (simplify
> (plus (vec_cond:s @0 @1 @2) @3)
> (vec_cond @0 (plus @1 @3) (plus @2 @3)))
>
> we get
>
> res_op->set_op (VEC_COND_EXPR, type, 3);
> res_op->ops[0] = captures[1];
> res_op->ops[0] = unshare_expr (res_op->ops[0]);
> {
> tree _o1[2], _r1;
> _o1[0] = captures[2];
> _o1[1] = captures[4];
> gimple_match_op tem_op (res_op->cond.any_else
> (), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
> tem_op.resimplify (lseq, valueize);
> _r1 = maybe_push_res_to_seq (_op, lseq); ()
> if (!_r1) return false;
> res_op->ops[1] = _r1;
> }
> {
> tree _o1[2], _r1;
> _o1[0] = captures[3];
> _o1[1] = captures[4];
> gimple_match_op tem_op (res_op->cond.any_else
> (), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
> tem_op.resimplify (lseq, valueize);
> _r1 = maybe_push_res_to_seq (_op, lseq); (***)
> if (!_r1) return false;
> res_op->ops[2] = _r1;
> }
> res_op->resimplify (lseq, valueize);
> return true;
>
> and the only change required would be to pass NULL to maybe_push_res_to_seq
> here instead of lseq at the (***) marked points.
(simplify
(plus (vec_cond:s @0 @1 @2) @3)
(vec_cond:l @0 (plus @1 @3) (plus @2 @3)))
'l' for 'force leaf'. I'll see if I can quickly cme up with a patch.
Richard.
> Richard.
>
> > The other "simple" patterns look good, you can commit
> > them separately if you like.
> >
> > Richard.
> >
> > > 2020-07-30 Marc Glisse
> > >
> > > PR tree-optimization/95906
> > > PR target/70314
> > > * match.pd ((c ? a
Re: VEC_COND_EXPR optimizations
On Fri, Jul 31, 2020 at 1:39 PM Marc Glisse wrote:
>
> On Fri, 31 Jul 2020, Richard Sandiford wrote:
>
> > Marc Glisse writes:
> >> +/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
> >> + (simplify
> >> + (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
> >> + (with
> >> + {
> >> + tree rhs1, rhs2 = NULL;
> >> + rhs1 = fold_binary (op, type, @1, @3);
> >> + if (rhs1 && is_gimple_val (rhs1))
> >> + rhs2 = fold_binary (op, type, @2, @4);
> >> + }
> >> + (if (rhs2 && is_gimple_val (rhs2))
> >> +(vec_cond @0 { rhs1; } { rhs2; })
> >> +#endif
> >
> > This one looks dangerous for potentially-trapping ops.
>
> I would expect the operation not to be folded if it can trap. Is that too
> optimistic?
>
> >> +/* (v ? w : 0) ? a : b is just (v & w) ? a : b */
> >> +(simplify
> >> + (vec_cond (vec_cond:s @0 @3 integer_zerop) @1 @2)
> >> + (vec_cond (bit_and @0 @3) @1 @2))
> >
> > Does something check automatically that @0 and @3 have compatible types?
@0 should always have a vector boolean type and thus will not be generally
compatible with @3. But OTOH then when you see (vec_cond (vec_cond ...
then @3 must be vector boolean as well...
But in theory with AVX512 the inner vec_cond could have a SImode
condition @0 producing a regular V4SImode vector mask for an outer
AVX512 SSE-style vec-cond and you then would get a mismatch.
So indeed better add a type compatibility check.
> My memory of this dates from before the avx512-related changes, but at
> least at the time, the type of the condition in vec_cond_expr had to have
> the same size and number of elements as the result, and have signed
> integral elements. Now the size constraint has changed, but it still looks
> like for a given number of elements and size (this last one ignored for
> avx512), there is essentially a single type that can appear as condition.
> Is this wrong for x86? For SVE?
>
> I could add some types_match conditions if that seems too unsafe...
>
> --
> Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, Jul 31, 2020 at 1:35 PM Richard Biener
wrote:
>
> On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse wrote:
> >
> > When vector comparisons were forced to use vec_cond_expr, we lost a number
> > of optimizations (my fault for not adding enough testcases to prevent
> > that). This patch tries to unwrap vec_cond_expr a bit so some
> > optimizations can still happen.
> >
> > I wasn't planning to add all those transformations together, but adding
> > one caused a regression, whose fix introduced a second regression, etc.
> >
> > Using a simple fold_binary internally looks like an ok compromise to me.
> > It remains cheap enough (not recursive, and vector instructions are not
> > that frequent), while still allowing more than const_binop (X|0 or X for
> > instance). The transformations are quite conservative with :s and folding
> > only if everything simplifies, we may want to relax this later. And of
> > course we are going to miss things like a?b:c + a?c:b -> b+c.
> >
> > In terms of number of operations, some transformations turning 2
> > VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
> > look like a gain... I expect the bit_not disappears in most cases, and
> > VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.
> >
> > I am a bit confused that with avx512 we get types like "vector(4)
> > " with :2 and not :1 (is it a hack so true is 1 and not
> > -1?), but that doesn't matter for this patch.
> >
> > Regtest+bootstrap on x86_64-pc-linux-gnu
>
> + (with
> + {
> + tree rhs1, rhs2 = NULL;
> + rhs1 = fold_binary (op, type, @1, @3);
> + if (rhs1 && is_gimple_val (rhs1))
> + rhs2 = fold_binary (op, type, @2, @3);
>
> ICK. I guess a more match-and-simplify way would be
>
>(with
> {
> tree rhs1, rhs2;
> gimple_match_op op (gimple_match_cond::UNCOND, op,
> type, @1, @3);
> if (op.resimplify (NULL, valueize)
> && gimple_simplified_result_is_gimple_val (op))
>{
> rhs1 = op.ops[0];
> ... other operand ...
>}
>
> now in theory we could invent some new syntax for this, like
>
> (simplify
> (op (vec_cond:s @0 @1 @2) @3)
> (vec_cond @0 (op:x @1 @3) (op:x @2 @3)))
>
> and pick something better instead of :x (:s is taken,
> would be 'simplified', :c is taken would be 'constexpr', ...).
>
> _Maybe_ just
>
> (simplify
> (op (vec_cond:s @0 @1 @2) @3)
> (vec_cond:x @0 (op @1 @3) (op @2 @3)))
>
> which would have the same practical meaning as passing
> NULL for the seq argument to simplification - do not allow
> any intermediate stmt to be generated.
Note I specifically do not like those if (it-simplifies) checks
because we already would code-generate those anyway. For
(simplify
(plus (vec_cond:s @0 @1 @2) @3)
(vec_cond @0 (plus @1 @3) (plus @2 @3)))
we get
res_op->set_op (VEC_COND_EXPR, type, 3);
res_op->ops[0] = captures[1];
res_op->ops[0] = unshare_expr (res_op->ops[0]);
{
tree _o1[2], _r1;
_o1[0] = captures[2];
_o1[1] = captures[4];
gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
tem_op.resimplify (lseq, valueize);
_r1 = maybe_push_res_to_seq (_op, lseq); ()
if (!_r1) return false;
res_op->ops[1] = _r1;
}
{
tree _o1[2], _r1;
_o1[0] = captures[3];
_o1[1] = captures[4];
gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
tem_op.resimplify (lseq, valueize);
_r1 = maybe_push_res_to_seq (_op, lseq); (***)
if (!_r1) return false;
res_op->ops[2] = _r1;
}
res_op->resimplify (lseq, valueize);
return true;
and the only change required would be to pass NULL to maybe_push_res_to_seq
here instead of lseq at the (***) marked points.
Richard.
> The other "simple" patterns look good, you can commit
> them separately if you like.
>
> Richard.
>
> > 2020-07-30 Marc Glisse
> >
> > PR tree-optimization/95906
> > PR target/70314
> > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e),
> > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations.
> >
> > * gcc.dg/tree-ssa/andnot-2.c: New file.
> > * gcc.dg/tree-ssa/pr95906.c: Likewise.
> > * gcc.target/i386/pr70314.c: Likewise.
> >
> > --
> > Marc Glisse
Re: VEC_COND_EXPR optimizations
On Fri, 31 Jul 2020, Richard Sandiford wrote:
Marc Glisse writes:
+/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
+ (simplify
+ (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
+ (with
+ {
+ tree rhs1, rhs2 = NULL;
+ rhs1 = fold_binary (op, type, @1, @3);
+ if (rhs1 && is_gimple_val (rhs1))
+ rhs2 = fold_binary (op, type, @2, @4);
+ }
+ (if (rhs2 && is_gimple_val (rhs2))
+(vec_cond @0 { rhs1; } { rhs2; })
+#endif
This one looks dangerous for potentially-trapping ops.
I would expect the operation not to be folded if it can trap. Is that too
optimistic?
+/* (v ? w : 0) ? a : b is just (v & w) ? a : b */
+(simplify
+ (vec_cond (vec_cond:s @0 @3 integer_zerop) @1 @2)
+ (vec_cond (bit_and @0 @3) @1 @2))
Does something check automatically that @0 and @3 have compatible types?
My memory of this dates from before the avx512-related changes, but at
least at the time, the type of the condition in vec_cond_expr had to have
the same size and number of elements as the result, and have signed
integral elements. Now the size constraint has changed, but it still looks
like for a given number of elements and size (this last one ignored for
avx512), there is essentially a single type that can appear as condition.
Is this wrong for x86? For SVE?
I could add some types_match conditions if that seems too unsafe...
--
Marc Glisse
Re: VEC_COND_EXPR optimizations
On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse wrote:
>
> When vector comparisons were forced to use vec_cond_expr, we lost a number
> of optimizations (my fault for not adding enough testcases to prevent
> that). This patch tries to unwrap vec_cond_expr a bit so some
> optimizations can still happen.
>
> I wasn't planning to add all those transformations together, but adding
> one caused a regression, whose fix introduced a second regression, etc.
>
> Using a simple fold_binary internally looks like an ok compromise to me.
> It remains cheap enough (not recursive, and vector instructions are not
> that frequent), while still allowing more than const_binop (X|0 or X for
> instance). The transformations are quite conservative with :s and folding
> only if everything simplifies, we may want to relax this later. And of
> course we are going to miss things like a?b:c + a?c:b -> b+c.
>
> In terms of number of operations, some transformations turning 2
> VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
> look like a gain... I expect the bit_not disappears in most cases, and
> VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.
>
> I am a bit confused that with avx512 we get types like "vector(4)
> " with :2 and not :1 (is it a hack so true is 1 and not
> -1?), but that doesn't matter for this patch.
>
> Regtest+bootstrap on x86_64-pc-linux-gnu
+ (with
+ {
+ tree rhs1, rhs2 = NULL;
+ rhs1 = fold_binary (op, type, @1, @3);
+ if (rhs1 && is_gimple_val (rhs1))
+ rhs2 = fold_binary (op, type, @2, @3);
ICK. I guess a more match-and-simplify way would be
(with
{
tree rhs1, rhs2;
gimple_match_op op (gimple_match_cond::UNCOND, op,
type, @1, @3);
if (op.resimplify (NULL, valueize)
&& gimple_simplified_result_is_gimple_val (op))
{
rhs1 = op.ops[0];
... other operand ...
}
now in theory we could invent some new syntax for this, like
(simplify
(op (vec_cond:s @0 @1 @2) @3)
(vec_cond @0 (op:x @1 @3) (op:x @2 @3)))
and pick something better instead of :x (:s is taken,
would be 'simplified', :c is taken would be 'constexpr', ...).
_Maybe_ just
(simplify
(op (vec_cond:s @0 @1 @2) @3)
(vec_cond:x @0 (op @1 @3) (op @2 @3)))
which would have the same practical meaning as passing
NULL for the seq argument to simplification - do not allow
any intermediate stmt to be generated.
The other "simple" patterns look good, you can commit
them separately if you like.
Richard.
> 2020-07-30 Marc Glisse
>
> PR tree-optimization/95906
> PR target/70314
> * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e),
> (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations.
>
> * gcc.dg/tree-ssa/andnot-2.c: New file.
> * gcc.dg/tree-ssa/pr95906.c: Likewise.
> * gcc.target/i386/pr70314.c: Likewise.
>
> --
> Marc Glisse
Re: VEC_COND_EXPR optimizations
Marc Glisse writes:
> +/* (c ? a : b) op (c ? d : e) --> c ? (a op d) : (b op e) */
> + (simplify
> + (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
> + (with
> + {
> + tree rhs1, rhs2 = NULL;
> + rhs1 = fold_binary (op, type, @1, @3);
> + if (rhs1 && is_gimple_val (rhs1))
> + rhs2 = fold_binary (op, type, @2, @4);
> + }
> + (if (rhs2 && is_gimple_val (rhs2))
> +(vec_cond @0 { rhs1; } { rhs2; })
> +#endif
This one looks dangerous for potentially-trapping ops.
> +/* (v ? w : 0) ? a : b is just (v & w) ? a : b */
> +(simplify
> + (vec_cond (vec_cond:s @0 @3 integer_zerop) @1 @2)
> + (vec_cond (bit_and @0 @3) @1 @2))
Does something check automatically that @0 and @3 have compatible types?
Same question for the later folds.
Thanks,
Richard
