subject:"\[PATCHv2\] Add a couple of A\?CST1\:CST2 match and simplify optimizations"

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-28 Thread Richard Biener via Gcc-patches

On Fri, May 28, 2021 at 7:21 AM Jeff Law via Gcc-patches
 wrote:
>
>
>
> On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote:
> > On 5/26/21 7:07 AM, Andrew Pinski wrote:
> >> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
> >>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
> >>>  wrote:
>  On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> >  wrote:
> >> From: Andrew Pinski 
> >>
> >> Instead of some of the more manual optimizations inside phi-opt,
> >> it would be good idea to do a lot of the heavy lifting inside match
> >> and simplify instead. In the process, this moves the three simple
> >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> >>
> >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> >>
> >> Differences from V1:
> >> * Use bit_xor 1 instead of bit_not to fix the problem with
> >> boolean types
> >> which are not 1 bit precision.
> > OK.
> >
> > Thanks,
> > Richard.
> >
>  Hmm, sorry, no luck.
> 
>  I think this caused:
> >>> If anything it is a bad interaction with changes between r12-1046 and
> >>> r12-1053; I am suspecting a bug in those changes rather than my
> >>> changes causing the bug.  Debugging it right now.
> >> (gdb) p debug_tree(name)
> >>>>  type  >>  size 
> >>  unit-size 
> >>  align:8 warn_if_not_align:0 symtab:0 alias-set -1
> >> canonical-type 0x76b45b28 precision:1 min  >> 0x76b4a030 0> max >
> >>
> >>  def_stmt _19 = ~_8;
> >>  version:19>
> >>
> >> So what is happening is evrp converted:
> >> ct_12 = ct_5 + -1;
> >> Into
> >> ct_12 = ct_5 == 1 ? 0 : 1;
> >> (this was done before my patch)
> >> And then it gets simplified to:
> >>_8 = ct_5 == 1;
> >>_19 = ~_8;
> >>ct_12 = (int) _19;
> >> (after my match.pd patch)
> Yup.  I've chased this kind of thing down repeatedly through the years.
> It's rare, but some transformations from match.pd create new SSA_NAMEs
> and the various passes need to be prepared to handle that.

You can suppress that at the expense of some missed simplifications.
Using fold_stmt_inplace will, for example, or when passing a NULL seq
arguments to the lower-level functions.

Richard.

> Jeff
>

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-27 Thread Jeff Law via Gcc-patches





On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote:

On 5/26/21 7:07 AM, Andrew Pinski wrote:

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with 
boolean types

which are not 1 bit precision.

OK.

Thanks,
Richard.


Hmm, sorry, no luck.

I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
  
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

 def_stmt _19 = ~_8;
 version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)
And then it gets simplified to:
   _8 = ct_5 == 1;
   _19 = ~_8;
   ct_12 = (int) _19;
(after my match.pd patch)
Yup.  I've chased this kind of thing down repeatedly through the years.  
It's rare, but some transformations from match.pd create new SSA_NAMEs 
and the various passes need to be prepared to handle that.


Jeff

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew MacLeod via Gcc-patches


On 5/26/21 7:07 AM, Andrew Pinski wrote:

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with boolean types
which are not 1 bit precision.

OK.

Thanks,
Richard.


Hmm, sorry, no luck.

I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
  
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

 def_stmt _19 = ~_8;
 version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)
And then it gets simplified to:
   _8 = ct_5 == 1;
   _19 = ~_8;
   ct_12 = (int) _19;
(after my match.pd patch)

Which is correct, but the range code is not expecting new SSA names to
be added .
It looks like the issue was introduced with r12-1048 (Add imports and
strengthen the export definition in range_def and gori_map).
I suspect there are other match.pd patterns which would also hit this
issue where a new ssa name is introduced.

I have no idea how to get this fixed because gimple-range-* is new to
me; I tried looking into it but calling has_def_chain/get_def_chain
inside register_dependency seems wrong, maybe Andrew MacLeod can help
here.
This happens even with a stage 1 gcc so it is not miscompiling.

Also sorry for the breakage, I was not expecting it this time around
as I ran bootstrap like three times and I did not expect an
interaction with other parts of the compiler like this.

Thanks,
Andrew Pinski




Yeah, not your fault, mine.  I'm sure it would have shown up very soon 
elsewhere, you were just the unlucky victim.


Sorry about that.

Andrew

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew MacLeod via Gcc-patches


On 5/26/21 1:07 PM, Bernd Edlinger wrote:

On 5/26/21 7:03 PM, Bernd Edlinger wrote:

On 5/26/21 2:05 PM, Richard Biener wrote:

On Wed, May 26, 2021 at 1:37 PM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 4:28 AM Richard Biener
 wrote:

On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with boolean types
which are not 1 bit precision.

OK.

Thanks,
Richard.


Hmm, sorry, no luck.

I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
  
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

 def_stmt _19 = ~_8;
 version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)

Note this COND_EXPR is supposed to be combined
with its single use in a GIMPLE_COND ...

I Noticed it was not doing it (before my patch) inside evrp either.

I think it is at most done in forwprop, but even then it likely
lacks a fold pattern - we only seem to forward comparisons
into GIMPLE_CONDs explicitely, leaving the rest to
match.pd patterns.


How about this for a quick fix:

commit b71621f51bc2819bb7d202efabc17fec5cc92f8f
Author: Bernd Edlinger 
Date:   Wed May 26 18:45:09 2021 +0200

 Fix gcc-bootstrap issue
 
 ... or at least try to.
 
 2021-05-26  Bernd Edlinger  
 
 * gimple-range-gori.cc (range_def_chain::register_dependency):

 Resize m_def_chain when needed.

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index a4c4bf5..722bf5d 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree dep, 
basic_block bb)
  
unsigned v = SSA_NAME_VERSION (name);

struct rdc &src = m_def_chain[v];
+  if (v >= m_def_chain.length ())
+m_def_chain.safe_grow_cleared (num_ssa_names + 1);>gimple *def_stmt = 
SSA_NAME_DEF_STMT (dep);
unsigned dep_v = SSA_NAME_VERSION (dep);
bitmap b;


Should I push this?
Or has anyone a better idea?


Aehm, I meant of course:

--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -176,6 +176,8 @@ range_def_chain::register_dependency (tree name, tree dep, b
  return;
  
unsigned v = SSA_NAME_VERSION (name);

+  if (v >= m_def_chain.length ())
+m_def_chain.safe_grow_cleared (num_ssa_names + 1);
struct rdc &src = m_def_chain[v];
gimple *def_stmt = SSA_NAME_DEF_STMT (dep);
unsigned dep_v = SSA_NAME_VERSION (dep);


yeah, I was just about to say this :-)    when everything was 
restructured, it seems there is a path to the new register_dependency 
call from the temporal cache which does not do the  range check...


all the other calls are gated by a call to has_def_chain() which ensures 
the vector is big enough.  This will be redundant in many cases, but 
will do for now until I do a perf test and see whether I should so 
something slightly different.


push is OK.

Andrew

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Bernd Edlinger

On 5/26/21 7:03 PM, Bernd Edlinger wrote:
> On 5/26/21 2:05 PM, Richard Biener wrote:
>> On Wed, May 26, 2021 at 1:37 PM Andrew Pinski  wrote:
>>>
>>> On Wed, May 26, 2021 at 4:28 AM Richard Biener
>>>  wrote:

 On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:
>
> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
>>
>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
>>  wrote:
>>>
>>> On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
 On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
  wrote:
>
> From: Andrew Pinski 
>
> Instead of some of the more manual optimizations inside phi-opt,
> it would be good idea to do a lot of the heavy lifting inside match
> and simplify instead. In the process, this moves the three simple
> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
>
> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
>
> Differences from V1:
> * Use bit_xor 1 instead of bit_not to fix the problem with boolean 
> types
> which are not 1 bit precision.

 OK.

 Thanks,
 Richard.

>>>
>>> Hmm, sorry, no luck.
>>>
>>> I think this caused:
>>
>> If anything it is a bad interaction with changes between r12-1046 and
>> r12-1053; I am suspecting a bug in those changes rather than my
>> changes causing the bug.  Debugging it right now.
>
> (gdb) p debug_tree(name)
>   type  size 
> unit-size 
> align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x76b45b28 precision:1 min  0x76b4a030 0> max >
>
> def_stmt _19 = ~_8;
> version:19>
>
> So what is happening is evrp converted:
> ct_12 = ct_5 + -1;
> Into
> ct_12 = ct_5 == 1 ? 0 : 1;
> (this was done before my patch)

 Note this COND_EXPR is supposed to be combined
 with its single use in a GIMPLE_COND ...
>>>
>>> I Noticed it was not doing it (before my patch) inside evrp either.
>>
>> I think it is at most done in forwprop, but even then it likely
>> lacks a fold pattern - we only seem to forward comparisons
>> into GIMPLE_CONDs explicitely, leaving the rest to
>> match.pd patterns.
>>
> 
> How about this for a quick fix:
> 
> commit b71621f51bc2819bb7d202efabc17fec5cc92f8f
> Author: Bernd Edlinger 
> Date:   Wed May 26 18:45:09 2021 +0200
> 
> Fix gcc-bootstrap issue
> 
> ... or at least try to.
> 
> 2021-05-26  Bernd Edlinger  
> 
> * gimple-range-gori.cc (range_def_chain::register_dependency):
> Resize m_def_chain when needed.
> 
> diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
> index a4c4bf5..722bf5d 100644
> --- a/gcc/gimple-range-gori.cc
> +++ b/gcc/gimple-range-gori.cc
> @@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree 
> dep, basic_block bb)
>  
>unsigned v = SSA_NAME_VERSION (name);
>struct rdc &src = m_def_chain[v];
> +  if (v >= m_def_chain.length ())
> +m_def_chain.safe_grow_cleared (num_ssa_names + 1);>gimple *def_stmt 
> = SSA_NAME_DEF_STMT (dep);
>unsigned dep_v = SSA_NAME_VERSION (dep);
>bitmap b;
> 
> 
> Should I push this?
> Or has anyone a better idea?
> 

Aehm, I meant of course:

--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -176,6 +176,8 @@ range_def_chain::register_dependency (tree name, tree dep, b
 return;
 
   unsigned v = SSA_NAME_VERSION (name);
+  if (v >= m_def_chain.length ())
+m_def_chain.safe_grow_cleared (num_ssa_names + 1);
   struct rdc &src = m_def_chain[v];
   gimple *def_stmt = SSA_NAME_DEF_STMT (dep);
   unsigned dep_v = SSA_NAME_VERSION (dep);


> 
> Thanks
> Bernd.
> 

> And then it gets simplified to:
>   _8 = ct_5 == 1;
>   _19 = ~_8;
>   ct_12 = (int) _19;
> (after my match.pd patch)

 which this one then breaks.  I suppose instead of replacing
 ct_12  adjusting the GIMPLE_COND directly might be
 a better approach ... or not folding the generated COND_EXPR.
>>>
>>> I was going to try to see where COND_EXPR is created but it is late
>>> and there seems to be other issues going on too.
>>> For example, the above really should have been converted to:
>>> _19 = ct_5 != 1;
>>>   ct_12 = (int) _19;
>>>
>>> This might be a gimple-match issue where the conditional part is
>>> always emitted without being simplified with the ~ part; COND_EXPR has
>>> those kind of issues :).
>>
>> No, we always re-simplify things, but there might be no
>> (bit_not (eq @0 integer_onep)) simplifier.
>>
>> Oddly enough
>>
>> _Bool foo (int x)
>> {
>>   _Bool tem = x == 1;
>>   return ~tem;
>> }
>>
>> is simplified to return 1 via matching
>>
>> /* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
>> (for cmp (simple_comparison)

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Bernd Edlinger

On 5/26/21 2:05 PM, Richard Biener wrote:
> On Wed, May 26, 2021 at 1:37 PM Andrew Pinski  wrote:
>>
>> On Wed, May 26, 2021 at 4:28 AM Richard Biener
>>  wrote:
>>>
>>> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:

 On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
>
> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
>  wrote:
>>
>> On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
>>> On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
>>>  wrote:

 From: Andrew Pinski 

 Instead of some of the more manual optimizations inside phi-opt,
 it would be good idea to do a lot of the heavy lifting inside match
 and simplify instead. In the process, this moves the three simple
 A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

 OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

 Differences from V1:
 * Use bit_xor 1 instead of bit_not to fix the problem with boolean 
 types
 which are not 1 bit precision.
>>>
>>> OK.
>>>
>>> Thanks,
>>> Richard.
>>>
>>
>> Hmm, sorry, no luck.
>>
>> I think this caused:
>
> If anything it is a bad interaction with changes between r12-1046 and
> r12-1053; I am suspecting a bug in those changes rather than my
> changes causing the bug.  Debugging it right now.

 (gdb) p debug_tree(name)
  >>> type >>> size 
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
 canonical-type 0x76b45b28 precision:1 min >>> 0x76b4a030 0> max >

 def_stmt _19 = ~_8;
 version:19>

 So what is happening is evrp converted:
 ct_12 = ct_5 + -1;
 Into
 ct_12 = ct_5 == 1 ? 0 : 1;
 (this was done before my patch)
>>>
>>> Note this COND_EXPR is supposed to be combined
>>> with its single use in a GIMPLE_COND ...
>>
>> I Noticed it was not doing it (before my patch) inside evrp either.
> 
> I think it is at most done in forwprop, but even then it likely
> lacks a fold pattern - we only seem to forward comparisons
> into GIMPLE_CONDs explicitely, leaving the rest to
> match.pd patterns.
> 

How about this for a quick fix:

commit b71621f51bc2819bb7d202efabc17fec5cc92f8f
Author: Bernd Edlinger 
Date:   Wed May 26 18:45:09 2021 +0200

Fix gcc-bootstrap issue

... or at least try to.

2021-05-26  Bernd Edlinger  

* gimple-range-gori.cc (range_def_chain::register_dependency):
Resize m_def_chain when needed.

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index a4c4bf5..722bf5d 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -177,6 +177,8 @@ range_def_chain::register_dependency (tree name, tree dep, 
basic_block bb)
 
   unsigned v = SSA_NAME_VERSION (name);
   struct rdc &src = m_def_chain[v];
+  if (v >= m_def_chain.length ())
+m_def_chain.safe_grow_cleared (num_ssa_names + 1);
   gimple *def_stmt = SSA_NAME_DEF_STMT (dep);
   unsigned dep_v = SSA_NAME_VERSION (dep);
   bitmap b;


Should I push this?
Or has anyone a better idea?


Thanks
Bernd.

>>>
 And then it gets simplified to:
   _8 = ct_5 == 1;
   _19 = ~_8;
   ct_12 = (int) _19;
 (after my match.pd patch)
>>>
>>> which this one then breaks.  I suppose instead of replacing
>>> ct_12  adjusting the GIMPLE_COND directly might be
>>> a better approach ... or not folding the generated COND_EXPR.
>>
>> I was going to try to see where COND_EXPR is created but it is late
>> and there seems to be other issues going on too.
>> For example, the above really should have been converted to:
>> _19 = ct_5 != 1;
>>   ct_12 = (int) _19;
>>
>> This might be a gimple-match issue where the conditional part is
>> always emitted without being simplified with the ~ part; COND_EXPR has
>> those kind of issues :).
> 
> No, we always re-simplify things, but there might be no
> (bit_not (eq @0 integer_onep)) simplifier.
> 
> Oddly enough
> 
> _Bool foo (int x)
> {
>   _Bool tem = x == 1;
>   return ~tem;
> }
> 
> is simplified to return 1 via matching
> 
> /* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
> (for cmp (simple_comparison)
>  scmp (swapped_simple_comparison)
>  (simplify
>   (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
>   (if (single_use (@2)
>&& (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
>(scmp @0 (bit_not @1)
> 
> Richard.
> 
>> Thanks,
>> Andrew Pinski
>>
>>
>>>
 Which is correct, but the range code is not expecting new SSA names to
 be added .
 It looks like the issue was introduced with r12-1048 (Add imports and
 strengthen the export definition in range_def and gori_map).
 I suspect there are other match.pd patterns which would also hit this
 issue where a new ssa name is introduced.

 I have no idea ho

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew MacLeod via Gcc-patches


On 5/26/21 8:05 AM, Richard Biener wrote:

On Wed, May 26, 2021 at 1:37 PM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 4:28 AM Richard Biener
 wrote:

On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with boolean types
which are not 1 bit precision.

OK.

Thanks,
Richard.


Hmm, sorry, no luck.

I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
  
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

 def_stmt _19 = ~_8;
 version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)

Note this COND_EXPR is supposed to be combined
with its single use in a GIMPLE_COND ...

I Noticed it was not doing it (before my patch) inside evrp either.

I think it is at most done in forwprop, but even then it likely
lacks a fold pattern - we only seem to forward comparisons
into GIMPLE_CONDs explicitely, leaving the rest to
match.pd patterns.


And then it gets simplified to:
   _8 = ct_5 == 1;
   _19 = ~_8;
   ct_12 = (int) _19;
(after my match.pd patch)

which this one then breaks.  I suppose instead of replacing
ct_12  adjusting the GIMPLE_COND directly might be
a better approach ... or not folding the generated COND_EXPR.

I was going to try to see where COND_EXPR is created but it is late
and there seems to be other issues going on too.
For example, the above really should have been converted to:
_19 = ct_5 != 1;
   ct_12 = (int) _19;

This might be a gimple-match issue where the conditional part is
always emitted without being simplified with the ~ part; COND_EXPR has
those kind of issues :).

No, we always re-simplify things, but there might be no
(bit_not (eq @0 integer_onep)) simplifier.

Oddly enough

_Bool foo (int x)
{
   _Bool tem = x == 1;
   return ~tem;
}

is simplified to return 1 via matching

/* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
(for cmp (simple_comparison)
  scmp (swapped_simple_comparison)
  (simplify
   (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
   (if (single_use (@2)
&& (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
(scmp @0 (bit_not @1)

Richard.


Which actually looks enticingly similar to what I'm seeing in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100774.



IN that code,

Folding statement: _1 = ~b_6;

is not turning ~[1,1] into [0,0] but rather leaving it as varying.  it 
eventually fills the values in


 _1 = 0;
  if (_1 != 0)
    goto ; [INV


but it doesnt simply properly.

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Richard Biener via Gcc-patches

On Wed, May 26, 2021 at 1:37 PM Andrew Pinski  wrote:
>
> On Wed, May 26, 2021 at 4:28 AM Richard Biener
>  wrote:
> >
> > On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:
> > >
> > > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
> > > >
> > > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
> > > >  wrote:
> > > > >
> > > > > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > > > > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> > > > > >  wrote:
> > > > > >>
> > > > > >> From: Andrew Pinski 
> > > > > >>
> > > > > >> Instead of some of the more manual optimizations inside phi-opt,
> > > > > >> it would be good idea to do a lot of the heavy lifting inside match
> > > > > >> and simplify instead. In the process, this moves the three simple
> > > > > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> > > > > >>
> > > > > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> > > > > >>
> > > > > >> Differences from V1:
> > > > > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean 
> > > > > >> types
> > > > > >> which are not 1 bit precision.
> > > > > >
> > > > > > OK.
> > > > > >
> > > > > > Thanks,
> > > > > > Richard.
> > > > > >
> > > > >
> > > > > Hmm, sorry, no luck.
> > > > >
> > > > > I think this caused:
> > > >
> > > > If anything it is a bad interaction with changes between r12-1046 and
> > > > r12-1053; I am suspecting a bug in those changes rather than my
> > > > changes causing the bug.  Debugging it right now.
> > >
> > > (gdb) p debug_tree(name)
> > >   > > type  > > size 
> > > unit-size 
> > > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > > canonical-type 0x76b45b28 precision:1 min  > > 0x76b4a030 0> max >
> > >
> > > def_stmt _19 = ~_8;
> > > version:19>
> > >
> > > So what is happening is evrp converted:
> > > ct_12 = ct_5 + -1;
> > > Into
> > > ct_12 = ct_5 == 1 ? 0 : 1;
> > > (this was done before my patch)
> >
> > Note this COND_EXPR is supposed to be combined
> > with its single use in a GIMPLE_COND ...
>
> I Noticed it was not doing it (before my patch) inside evrp either.

I think it is at most done in forwprop, but even then it likely
lacks a fold pattern - we only seem to forward comparisons
into GIMPLE_CONDs explicitely, leaving the rest to
match.pd patterns.

> >
> > > And then it gets simplified to:
> > >   _8 = ct_5 == 1;
> > >   _19 = ~_8;
> > >   ct_12 = (int) _19;
> > > (after my match.pd patch)
> >
> > which this one then breaks.  I suppose instead of replacing
> > ct_12  adjusting the GIMPLE_COND directly might be
> > a better approach ... or not folding the generated COND_EXPR.
>
> I was going to try to see where COND_EXPR is created but it is late
> and there seems to be other issues going on too.
> For example, the above really should have been converted to:
> _19 = ct_5 != 1;
>   ct_12 = (int) _19;
>
> This might be a gimple-match issue where the conditional part is
> always emitted without being simplified with the ~ part; COND_EXPR has
> those kind of issues :).

No, we always re-simplify things, but there might be no
(bit_not (eq @0 integer_onep)) simplifier.

Oddly enough

_Bool foo (int x)
{
  _Bool tem = x == 1;
  return ~tem;
}

is simplified to return 1 via matching

/* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
(for cmp (simple_comparison)
 scmp (swapped_simple_comparison)
 (simplify
  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
  (if (single_use (@2)
   && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
   (scmp @0 (bit_not @1)

Richard.

> Thanks,
> Andrew Pinski
>
>
> >
> > > Which is correct, but the range code is not expecting new SSA names to
> > > be added .
> > > It looks like the issue was introduced with r12-1048 (Add imports and
> > > strengthen the export definition in range_def and gori_map).
> > > I suspect there are other match.pd patterns which would also hit this
> > > issue where a new ssa name is introduced.
> > >
> > > I have no idea how to get this fixed because gimple-range-* is new to
> > > me; I tried looking into it but calling has_def_chain/get_def_chain
> > > inside register_dependency seems wrong, maybe Andrew MacLeod can help
> > > here.
> > > This happens even with a stage 1 gcc so it is not miscompiling.
> > >
> > > Also sorry for the breakage, I was not expecting it this time around
> > > as I ran bootstrap like three times and I did not expect an
> > > interaction with other parts of the compiler like this.
> > >
> > > Thanks,
> > > Andrew Pinski
> > >
> > >
> > > >
> > > > Thanks,
> > > > Andrew
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
> > > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
> > > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
> > > > > /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
> > > > > /home/ed/gnu/install/

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew Pinski via Gcc-patches

On Wed, May 26, 2021 at 4:28 AM Richard Biener
 wrote:
>
> On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:
> >
> > On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
> > >
> > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
> > >  wrote:
> > > >
> > > > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > > > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> > > > >  wrote:
> > > > >>
> > > > >> From: Andrew Pinski 
> > > > >>
> > > > >> Instead of some of the more manual optimizations inside phi-opt,
> > > > >> it would be good idea to do a lot of the heavy lifting inside match
> > > > >> and simplify instead. In the process, this moves the three simple
> > > > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> > > > >>
> > > > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> > > > >>
> > > > >> Differences from V1:
> > > > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean 
> > > > >> types
> > > > >> which are not 1 bit precision.
> > > > >
> > > > > OK.
> > > > >
> > > > > Thanks,
> > > > > Richard.
> > > > >
> > > >
> > > > Hmm, sorry, no luck.
> > > >
> > > > I think this caused:
> > >
> > > If anything it is a bad interaction with changes between r12-1046 and
> > > r12-1053; I am suspecting a bug in those changes rather than my
> > > changes causing the bug.  Debugging it right now.
> >
> > (gdb) p debug_tree(name)
> >   > type  > size 
> > unit-size 
> > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x76b45b28 precision:1 min  > 0x76b4a030 0> max >
> >
> > def_stmt _19 = ~_8;
> > version:19>
> >
> > So what is happening is evrp converted:
> > ct_12 = ct_5 + -1;
> > Into
> > ct_12 = ct_5 == 1 ? 0 : 1;
> > (this was done before my patch)
>
> Note this COND_EXPR is supposed to be combined
> with its single use in a GIMPLE_COND ...

I Noticed it was not doing it (before my patch) inside evrp either.

>
> > And then it gets simplified to:
> >   _8 = ct_5 == 1;
> >   _19 = ~_8;
> >   ct_12 = (int) _19;
> > (after my match.pd patch)
>
> which this one then breaks.  I suppose instead of replacing
> ct_12  adjusting the GIMPLE_COND directly might be
> a better approach ... or not folding the generated COND_EXPR.

I was going to try to see where COND_EXPR is created but it is late
and there seems to be other issues going on too.
For example, the above really should have been converted to:
_19 = ct_5 != 1;
  ct_12 = (int) _19;

This might be a gimple-match issue where the conditional part is
always emitted without being simplified with the ~ part; COND_EXPR has
those kind of issues :).

Thanks,
Andrew Pinski


>
> > Which is correct, but the range code is not expecting new SSA names to
> > be added .
> > It looks like the issue was introduced with r12-1048 (Add imports and
> > strengthen the export definition in range_def and gori_map).
> > I suspect there are other match.pd patterns which would also hit this
> > issue where a new ssa name is introduced.
> >
> > I have no idea how to get this fixed because gimple-range-* is new to
> > me; I tried looking into it but calling has_def_chain/get_def_chain
> > inside register_dependency seems wrong, maybe Andrew MacLeod can help
> > here.
> > This happens even with a stage 1 gcc so it is not miscompiling.
> >
> > Also sorry for the breakage, I was not expecting it this time around
> > as I ran bootstrap like three times and I did not expect an
> > interaction with other parts of the compiler like this.
> >
> > Thanks,
> > Andrew Pinski
> >
> >
> > >
> > > Thanks,
> > > Andrew
> > >
> > >
> > >
> > >
> > > >
> > > > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
> > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
> > > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
> > > > /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
> > > > /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include   -fchecking=1 -c 
> > > > -g -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes 
> > > > -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute 
> > > > -Wno-overlength-strings -pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. 
> > > > -I../../gcc-trunk/fixincludes -I../include 
> > > > -I../../gcc-trunk/fixincludes/../include 
> > > > ../../gcc-trunk/fixincludes/fixtests.c
> > > > during GIMPLE pass: evrp
> > > > ../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’:
> > > > ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: 
> > > > in operator[], at vec.h:890
> > > >   155 | }
> > > >   | ^
> > > > 0x824622 vec::operator[](unsigned int)
> > > > ../../gcc-trunk/gcc/vec.h:890
> > > > 0x8247f0 vec > > > vl_embed>::operator[](unsigned int)
> > > > ../../gcc-trunk/gcc/tree.h:3366
> > > > 0x8247f0 vec > > > vl_ptr>::operator[](unsigned int)
> > > > ../../gcc-trunk/gcc/vec.h:1461
> > > > 0x8247f0 range_def_chain::register_

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Richard Biener via Gcc-patches

On Wed, May 26, 2021 at 1:07 PM Andrew Pinski  wrote:
>
> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
> >
> > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
> >  wrote:
> > >
> > > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> > > >  wrote:
> > > >>
> > > >> From: Andrew Pinski 
> > > >>
> > > >> Instead of some of the more manual optimizations inside phi-opt,
> > > >> it would be good idea to do a lot of the heavy lifting inside match
> > > >> and simplify instead. In the process, this moves the three simple
> > > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> > > >>
> > > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> > > >>
> > > >> Differences from V1:
> > > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean 
> > > >> types
> > > >> which are not 1 bit precision.
> > > >
> > > > OK.
> > > >
> > > > Thanks,
> > > > Richard.
> > > >
> > >
> > > Hmm, sorry, no luck.
> > >
> > > I think this caused:
> >
> > If anything it is a bad interaction with changes between r12-1046 and
> > r12-1053; I am suspecting a bug in those changes rather than my
> > changes causing the bug.  Debugging it right now.
>
> (gdb) p debug_tree(name)
>   type  size 
> unit-size 
> align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x76b45b28 precision:1 min  0x76b4a030 0> max >
>
> def_stmt _19 = ~_8;
> version:19>
>
> So what is happening is evrp converted:
> ct_12 = ct_5 + -1;
> Into
> ct_12 = ct_5 == 1 ? 0 : 1;
> (this was done before my patch)

Note this COND_EXPR is supposed to be combined
with its single use in a GIMPLE_COND ...

> And then it gets simplified to:
>   _8 = ct_5 == 1;
>   _19 = ~_8;
>   ct_12 = (int) _19;
> (after my match.pd patch)

which this one then breaks.  I suppose instead of replacing
ct_12  adjusting the GIMPLE_COND directly might be
a better approach ... or not folding the generated COND_EXPR.

> Which is correct, but the range code is not expecting new SSA names to
> be added .
> It looks like the issue was introduced with r12-1048 (Add imports and
> strengthen the export definition in range_def and gori_map).
> I suspect there are other match.pd patterns which would also hit this
> issue where a new ssa name is introduced.
>
> I have no idea how to get this fixed because gimple-range-* is new to
> me; I tried looking into it but calling has_def_chain/get_def_chain
> inside register_dependency seems wrong, maybe Andrew MacLeod can help
> here.
> This happens even with a stage 1 gcc so it is not miscompiling.
>
> Also sorry for the breakage, I was not expecting it this time around
> as I ran bootstrap like three times and I did not expect an
> interaction with other parts of the compiler like this.
>
> Thanks,
> Andrew Pinski
>
>
> >
> > Thanks,
> > Andrew
> >
> >
> >
> >
> > >
> > > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
> > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
> > > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
> > > /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
> > > /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include   -fchecking=1 -c -g 
> > > -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
> > > -Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
> > > -pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. 
> > > -I../../gcc-trunk/fixincludes -I../include 
> > > -I../../gcc-trunk/fixincludes/../include 
> > > ../../gcc-trunk/fixincludes/fixtests.c
> > > during GIMPLE pass: evrp
> > > ../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’:
> > > ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: in 
> > > operator[], at vec.h:890
> > >   155 | }
> > >   | ^
> > > 0x824622 vec::operator[](unsigned int)
> > > ../../gcc-trunk/gcc/vec.h:890
> > > 0x8247f0 vec > > vl_embed>::operator[](unsigned int)
> > > ../../gcc-trunk/gcc/tree.h:3366
> > > 0x8247f0 vec::operator[](unsigned 
> > > int)
> > > ../../gcc-trunk/gcc/vec.h:1461
> > > 0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, 
> > > basic_block_def*)
> > > ../../gcc-trunk/gcc/gimple-range-gori.cc:179
> > > 0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, 
> > > fur_source&)
> > > ../../gcc-trunk/gcc/gimple-range.cc:439
> > > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, 
> > > tree_node*)
> > > ../../gcc-trunk/gcc/gimple-range.cc:376
> > > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
> > > ../../gcc-trunk/gcc/gimple-range.cc:1067
> > > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
> > > ../../gcc-trunk/gcc/gimple-range.cc:1097
> > > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
> > > ../../gcc-trunk/

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew Pinski via Gcc-patches

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
>
> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
>  wrote:
> >
> > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> > >  wrote:
> > >>
> > >> From: Andrew Pinski 
> > >>
> > >> Instead of some of the more manual optimizations inside phi-opt,
> > >> it would be good idea to do a lot of the heavy lifting inside match
> > >> and simplify instead. In the process, this moves the three simple
> > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> > >>
> > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> > >>
> > >> Differences from V1:
> > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean types
> > >> which are not 1 bit precision.
> > >
> > > OK.
> > >
> > > Thanks,
> > > Richard.
> > >
> >
> > Hmm, sorry, no luck.
> >
> > I think this caused:
>
> If anything it is a bad interaction with changes between r12-1046 and
> r12-1053; I am suspecting a bug in those changes rather than my
> changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
 
unit-size 
align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

def_stmt _19 = ~_8;
version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)
And then it gets simplified to:
  _8 = ct_5 == 1;
  _19 = ~_8;
  ct_12 = (int) _19;
(after my match.pd patch)

Which is correct, but the range code is not expecting new SSA names to
be added .
It looks like the issue was introduced with r12-1048 (Add imports and
strengthen the export definition in range_def and gori_map).
I suspect there are other match.pd patterns which would also hit this
issue where a new ssa name is introduced.

I have no idea how to get this fixed because gimple-range-* is new to
me; I tried looking into it but calling has_def_chain/get_def_chain
inside register_dependency seems wrong, maybe Andrew MacLeod can help
here.
This happens even with a stage 1 gcc so it is not miscompiling.

Also sorry for the breakage, I was not expecting it this time around
as I ran bootstrap like three times and I did not expect an
interaction with other parts of the compiler like this.

Thanks,
Andrew Pinski


>
> Thanks,
> Andrew
>
>
>
>
> >
> > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
> > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
> > -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
> > /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
> > /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include   -fchecking=1 -c -g 
> > -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
> > -Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
> > -pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. 
> > -I../../gcc-trunk/fixincludes -I../include 
> > -I../../gcc-trunk/fixincludes/../include 
> > ../../gcc-trunk/fixincludes/fixtests.c
> > during GIMPLE pass: evrp
> > ../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’:
> > ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: in 
> > operator[], at vec.h:890
> >   155 | }
> >   | ^
> > 0x824622 vec::operator[](unsigned int)
> > ../../gcc-trunk/gcc/vec.h:890
> > 0x8247f0 vec::operator[](unsigned 
> > int)
> > ../../gcc-trunk/gcc/tree.h:3366
> > 0x8247f0 vec::operator[](unsigned 
> > int)
> > ../../gcc-trunk/gcc/vec.h:1461
> > 0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, 
> > basic_block_def*)
> > ../../gcc-trunk/gcc/gimple-range-gori.cc:179
> > 0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
> > ../../gcc-trunk/gcc/gimple-range.cc:439
> > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, 
> > tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:376
> > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:1067
> > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:1097
> > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
> > ../../gcc-trunk/gcc/gimple-range.cc:980
> > 0x18637c7 fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
> > ../../gcc-trunk/gcc/gimple-range.cc:431
> > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, 
> > tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:376
> > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:1067
> > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
> > ../../gcc-trunk/gcc/gimple-range.cc:1097
> > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gim

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Andrew Pinski via Gcc-patches

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:
>
> On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> >  wrote:
> >>
> >> From: Andrew Pinski 
> >>
> >> Instead of some of the more manual optimizations inside phi-opt,
> >> it would be good idea to do a lot of the heavy lifting inside match
> >> and simplify instead. In the process, this moves the three simple
> >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> >>
> >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> >>
> >> Differences from V1:
> >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean types
> >> which are not 1 bit precision.
> >
> > OK.
> >
> > Thanks,
> > Richard.
> >
>
> Hmm, sorry, no luck.
>
> I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

Thanks,
Andrew




>
> home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
> -B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
> -B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
> /home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
> /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include   -fchecking=1 -c -g -O2 
> -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
> -Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
> -pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. -I../../gcc-trunk/fixincludes 
> -I../include -I../../gcc-trunk/fixincludes/../include 
> ../../gcc-trunk/fixincludes/fixtests.c
> during GIMPLE pass: evrp
> ../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’:
> ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: in 
> operator[], at vec.h:890
>   155 | }
>   | ^
> 0x824622 vec::operator[](unsigned int)
> ../../gcc-trunk/gcc/vec.h:890
> 0x8247f0 vec::operator[](unsigned 
> int)
> ../../gcc-trunk/gcc/tree.h:3366
> 0x8247f0 vec::operator[](unsigned int)
> ../../gcc-trunk/gcc/vec.h:1461
> 0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, 
> basic_block_def*)
> ../../gcc-trunk/gcc/gimple-range-gori.cc:179
> 0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
> ../../gcc-trunk/gcc/gimple-range.cc:439
> 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, 
> tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:376
> 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:1067
> 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:1097
> 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
> ../../gcc-trunk/gcc/gimple-range.cc:980
> 0x18637c7 fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
> ../../gcc-trunk/gcc/gimple-range.cc:431
> 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, 
> tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:376
> 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:1067
> 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
> ../../gcc-trunk/gcc/gimple-range.cc:1097
> 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
> ../../gcc-trunk/gcc/gimple-range.cc:980
> 0x1149961 range_query::value_of_expr(tree_node*, gimple*)
> ../../gcc-trunk/gcc/value-query.cc:86
> 0x1871e51 hybrid_folder::value_of_expr(tree_node*, gimple*)
> ../../gcc-trunk/gcc/gimple-ssa-evrp.c:235
> 0xff9573 substitute_and_fold_engine::replace_uses_in(gimple*)
> ../../gcc-trunk/gcc/tree-ssa-propagate.c:575
> 0xff988c substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
> ../../gcc-trunk/gcc/tree-ssa-propagate.c:845
> 0x183921f dom_walker::walk(basic_block_def*)
> ../../gcc-trunk/gcc/domwalk.c:309
> 0xff8d15 substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
> ../../gcc-trunk/gcc/tree-ssa-propagate.c:987
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> make[2]: *** [Makefile:76: fixtests.o] Error 1
> make[2]: Leaving directory '/home/ed/gnu/gcc-build/fixincludes'
> make[1]: *** [Makefile:3827: all-fixincludes] Error 2
> make[1]: Leaving directory '/home/ed/gnu/gcc-build'
> make: *** [Makefile:1011: all] Error 2
>
>
> Bernd.
>
>
> >> Thanks,
> >> Andrew Pinski
> >>
> >> gcc:
> >> * match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, A?+-1:0,
> >> A?POW2:0 and A?0:POW2.
> >> ---
> >>  gcc/match.pd | 41 +
> >>  1 file changed,

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-26 Thread Bernd Edlinger

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
>  wrote:
>>
>> From: Andrew Pinski 
>>
>> Instead of some of the more manual optimizations inside phi-opt,
>> it would be good idea to do a lot of the heavy lifting inside match
>> and simplify instead. In the process, this moves the three simple
>> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
>>
>> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
>>
>> Differences from V1:
>> * Use bit_xor 1 instead of bit_not to fix the problem with boolean types
>> which are not 1 bit precision.
> 
> OK.
> 
> Thanks,
> Richard.
> 

Hmm, sorry, no luck.

I think this caused:

home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ 
-B/home/ed/gnu/install/x86_64-pc-linux-gnu/bin/ 
-B/home/ed/gnu/install/x86_64-pc-linux-gnu/lib/ -isystem 
/home/ed/gnu/install/x86_64-pc-linux-gnu/include -isystem 
/home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include   -fchecking=1 -c -g -O2 
-W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
-pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. -I../../gcc-trunk/fixincludes 
-I../include -I../../gcc-trunk/fixincludes/../include 
../../gcc-trunk/fixincludes/fixtests.c
during GIMPLE pass: evrp
../../gcc-trunk/fixincludes/fixtests.c: In function ‘run_test’:
../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: in 
operator[], at vec.h:890
  155 | }
  | ^
0x824622 vec::operator[](unsigned int)
../../gcc-trunk/gcc/vec.h:890
0x8247f0 vec::operator[](unsigned int)
../../gcc-trunk/gcc/tree.h:3366
0x8247f0 vec::operator[](unsigned int)
../../gcc-trunk/gcc/vec.h:1461
0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, 
basic_block_def*)
../../gcc-trunk/gcc/gimple-range-gori.cc:179
0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
../../gcc-trunk/gcc/gimple-range.cc:439
0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:376
0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:1067
0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:1097
0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
../../gcc-trunk/gcc/gimple-range.cc:980
0x18637c7 fold_using_range::range_of_range_op(irange&, gimple*, fur_source&)
../../gcc-trunk/gcc/gimple-range.cc:431
0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:376
0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:1067
0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*)
../../gcc-trunk/gcc/gimple-range.cc:1097
0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*)
../../gcc-trunk/gcc/gimple-range.cc:980
0x1149961 range_query::value_of_expr(tree_node*, gimple*)
../../gcc-trunk/gcc/value-query.cc:86
0x1871e51 hybrid_folder::value_of_expr(tree_node*, gimple*)
../../gcc-trunk/gcc/gimple-ssa-evrp.c:235
0xff9573 substitute_and_fold_engine::replace_uses_in(gimple*)
../../gcc-trunk/gcc/tree-ssa-propagate.c:575
0xff988c substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
../../gcc-trunk/gcc/tree-ssa-propagate.c:845
0x183921f dom_walker::walk(basic_block_def*)
../../gcc-trunk/gcc/domwalk.c:309
0xff8d15 substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
../../gcc-trunk/gcc/tree-ssa-propagate.c:987
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
make[2]: *** [Makefile:76: fixtests.o] Error 1
make[2]: Leaving directory '/home/ed/gnu/gcc-build/fixincludes'
make[1]: *** [Makefile:3827: all-fixincludes] Error 2
make[1]: Leaving directory '/home/ed/gnu/gcc-build'
make: *** [Makefile:1011: all] Error 2


Bernd.


>> Thanks,
>> Andrew Pinski
>>
>> gcc:
>> * match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, A?+-1:0,
>> A?POW2:0 and A?0:POW2.
>> ---
>>  gcc/match.pd | 41 +
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 1fc6b7b1557..ad6b057c56d 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -3711,6 +3711,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> (if (integer_all_onesp (@1) && integer_zerop (@2))
>>  @0
>>
>> +/* A few simplifications of "a ? CST1 : CST2". */
>> +/* NOTE: Only do this on gimple as the if-chain-to-switch
>> +   optimization depends on the gimple to have if statements in it.

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-25 Thread Richard Biener via Gcc-patches

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:
>
> From: Andrew Pinski 
>
> Instead of some of the more manual optimizations inside phi-opt,
> it would be good idea to do a lot of the heavy lifting inside match
> and simplify instead. In the process, this moves the three simple
> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
>
> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
>
> Differences from V1:
> * Use bit_xor 1 instead of bit_not to fix the problem with boolean types
> which are not 1 bit precision.

OK.

Thanks,
Richard.

> Thanks,
> Andrew Pinski
>
> gcc:
> * match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, A?+-1:0,
> A?POW2:0 and A?0:POW2.
> ---
>  gcc/match.pd | 41 +
>  1 file changed, 41 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 1fc6b7b1557..ad6b057c56d 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3711,6 +3711,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (if (integer_all_onesp (@1) && integer_zerop (@2))
>  @0
>
> +/* A few simplifications of "a ? CST1 : CST2". */
> +/* NOTE: Only do this on gimple as the if-chain-to-switch
> +   optimization depends on the gimple to have if statements in it. */
> +#if GIMPLE
> +(simplify
> + (cond @0 INTEGER_CST@1 INTEGER_CST@2)
> + (switch
> +  (if (integer_zerop (@2))
> +   (switch
> +/* a ? 1 : 0 -> a if 0 and 1 are integral types. */
> +(if (integer_onep (@1))
> + (convert (convert:boolean_type_node @0)))
> +/* a ? -1 : 0 -> -a. */
> +(if (integer_all_onesp (@1))
> + (negate (convert (convert:boolean_type_node @0
> +/* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
> +(if (!POINTER_TYPE_P (type) && integer_pow2p (@1))
> + (with {
> +   tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
> +  }
> +  (lshift (convert (convert:boolean_type_node @0)) { shift; })
> +  (if (integer_zerop (@1))
> +   (with {
> +  tree booltrue = constant_boolean_node (true, boolean_type_node);
> +}
> +(switch
> + /* a ? 0 : 1 -> !a. */
> + (if (integer_onep (@2))
> +  (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
> + /* a ? -1 : 0 -> -(!a). */
> + (if (integer_all_onesp (@2))
> +  (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } 
> 
> + /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> + (if (!POINTER_TYPE_P (type) && integer_pow2p (@2))
> +  (with {
> +   tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
> +   }
> +   (lshift (convert (bit_xor (convert:boolean_type_node @0) { booltrue; 
> } ))
> +{ shift; }
> +#endif
> +
>  /* Simplification moved from fold_cond_expr_with_comparison.  It may also
> be extended.  */
>  /* This pattern implements two kinds simplification:
> --
> 2.17.1
>

[PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-23 Thread apinski--- via Gcc-patches

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with boolean types
which are not 1 bit precision.

Thanks,
Andrew Pinski

gcc:
* match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, A?+-1:0,
A?POW2:0 and A?0:POW2.
---
 gcc/match.pd | 41 +
 1 file changed, 41 insertions(+)

diff --git a/gcc/match.pd b/gcc/match.pd
index 1fc6b7b1557..ad6b057c56d 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3711,6 +3711,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (integer_all_onesp (@1) && integer_zerop (@2))
 @0
 
+/* A few simplifications of "a ? CST1 : CST2". */
+/* NOTE: Only do this on gimple as the if-chain-to-switch
+   optimization depends on the gimple to have if statements in it. */
+#if GIMPLE
+(simplify
+ (cond @0 INTEGER_CST@1 INTEGER_CST@2)
+ (switch
+  (if (integer_zerop (@2))
+   (switch
+/* a ? 1 : 0 -> a if 0 and 1 are integral types. */
+(if (integer_onep (@1))
+ (convert (convert:boolean_type_node @0)))
+/* a ? -1 : 0 -> -a. */
+(if (integer_all_onesp (@1))
+ (negate (convert (convert:boolean_type_node @0
+/* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
+(if (!POINTER_TYPE_P (type) && integer_pow2p (@1))
+ (with {
+   tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
+  }
+  (lshift (convert (convert:boolean_type_node @0)) { shift; })
+  (if (integer_zerop (@1))
+   (with {
+  tree booltrue = constant_boolean_node (true, boolean_type_node);
+}
+(switch
+ /* a ? 0 : 1 -> !a. */
+ (if (integer_onep (@2))
+  (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
+ /* a ? -1 : 0 -> -(!a). */
+ (if (integer_all_onesp (@2))
+  (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } 

+ /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
+ (if (!POINTER_TYPE_P (type) && integer_pow2p (@2))
+  (with {
+   tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
+   }
+   (lshift (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } 
))
+{ shift; }
+#endif
+
 /* Simplification moved from fold_cond_expr_with_comparison.  It may also
be extended.  */
 /* This pattern implements two kinds simplification:
-- 
2.17.1

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

[PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

15 matches

Site Navigation

Mail list logo

Footer information