Fix the build of record_edge_info()

Message ID 20221001005720.28208-1-palmer@rivosinc.com
State Committed
Commit aa360fbf68b11e54017e8fa5b1bdb87ce7c19188
Headers
Series Fix the build of record_edge_info() |

Commit Message

Palmer Dabbelt Oct. 1, 2022, 12:57 a.m. UTC
  As of 1214196da79 ("More gimple const/copy propagation opportunities"),
I'm getting some build failures during bootstrap

    ../../gcc/tree-ssa-dom.cc: In function ‘void record_edge_info(basic_block)’:
    ../../gcc/tree-ssa-dom.cc:689:27: error: ‘dst’ was not declared in this scope; did you mean ‘dse’?
      689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
          |                           ^~~
          |                           dse
    In file included from ../../gcc/gimple-ssa.h:24,
                     from ../../gcc/ssa.h:27,
                     from ../../gcc/tree-ssa-dom.cc:28:
    ../../gcc/tree-ssa-dom.cc:689:47: error: ‘phi’ was not declared in this scope; did you mean ‘gphi’?
      689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
          |                                               ^~~
    ../../gcc/tree-ssa-operands.h:82:54: note: in definition of macro ‘PHI_ARG_DEF’
       82 | #define PHI_ARG_DEF(PHI, I)     gimple_phi_arg_def ((PHI), (I))
          |

I've never looked at this stuff before so I've sort of just pattern
matched this, it at least fixes the build.  Happy to go try and
understand what's going on here, but I'm in the middle of a few things
so I figured it'd be better to just send it along in case anyone else is
running into the same issue -- it's more of a bug report than a fix,
though.

gcc/ChangeLog

	* tree-ssa-dom.c (record_edge_info): Move the alternative check
	below the phi definition.
---
 gcc/tree-ssa-dom.cc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
  

Comments

Jeff Law Oct. 1, 2022, 1:01 a.m. UTC | #1
On 9/30/22 18:57, Palmer Dabbelt wrote:
> As of 1214196da79 ("More gimple const/copy propagation opportunities"),
> I'm getting some build failures during bootstrap
>
>      ../../gcc/tree-ssa-dom.cc: In function ‘void record_edge_info(basic_block)’:
>      ../../gcc/tree-ssa-dom.cc:689:27: error: ‘dst’ was not declared in this scope; did you mean ‘dse’?
>        689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
>            |                           ^~~
>            |                           dse
>      In file included from ../../gcc/gimple-ssa.h:24,
>                       from ../../gcc/ssa.h:27,
>                       from ../../gcc/tree-ssa-dom.cc:28:
>      ../../gcc/tree-ssa-dom.cc:689:47: error: ‘phi’ was not declared in this scope; did you mean ‘gphi’?
>        689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
>            |                                               ^~~
>      ../../gcc/tree-ssa-operands.h:82:54: note: in definition of macro ‘PHI_ARG_DEF’
>         82 | #define PHI_ARG_DEF(PHI, I)     gimple_phi_arg_def ((PHI), (I))
>            |
>
> I've never looked at this stuff before so I've sort of just pattern
> matched this, it at least fixes the build.  Happy to go try and
> understand what's going on here, but I'm in the middle of a few things
> so I figured it'd be better to just send it along in case anyone else is
> running into the same issue -- it's more of a bug report than a fix,
> though.
>
> gcc/ChangeLog
>
> 	* tree-ssa-dom.c (record_edge_info): Move the alternative check
> 	below the phi definition.
> ---

You got it right, but it's already fixed on the trunk (I pushed the 
wrong version of the patch).


jeff
  
Palmer Dabbelt Oct. 1, 2022, 1:27 a.m. UTC | #2
On Fri, 30 Sep 2022 18:01:00 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
> On 9/30/22 18:57, Palmer Dabbelt wrote:
>> As of 1214196da79 ("More gimple const/copy propagation opportunities"),
>> I'm getting some build failures during bootstrap
>>
>>      ../../gcc/tree-ssa-dom.cc: In function ‘void record_edge_info(basic_block)’:
>>      ../../gcc/tree-ssa-dom.cc:689:27: error: ‘dst’ was not declared in this scope; did you mean ‘dse’?
>>        689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
>>            |                           ^~~
>>            |                           dse
>>      In file included from ../../gcc/gimple-ssa.h:24,
>>                       from ../../gcc/ssa.h:27,
>>                       from ../../gcc/tree-ssa-dom.cc:28:
>>      ../../gcc/tree-ssa-dom.cc:689:47: error: ‘phi’ was not declared in this scope; did you mean ‘gphi’?
>>        689 |                       if (dst == PHI_ARG_DEF (phi, !alternative))
>>            |                                               ^~~
>>      ../../gcc/tree-ssa-operands.h:82:54: note: in definition of macro ‘PHI_ARG_DEF’
>>         82 | #define PHI_ARG_DEF(PHI, I)     gimple_phi_arg_def ((PHI), (I))
>>            |
>>
>> I've never looked at this stuff before so I've sort of just pattern
>> matched this, it at least fixes the build.  Happy to go try and
>> understand what's going on here, but I'm in the middle of a few things
>> so I figured it'd be better to just send it along in case anyone else is
>> running into the same issue -- it's more of a bug report than a fix,
>> though.
>>
>> gcc/ChangeLog
>>
>> 	* tree-ssa-dom.c (record_edge_info): Move the alternative check
>> 	below the phi definition.
>> ---
>
> You got it right, but it's already fixed on the trunk (I pushed the
> wrong version of the patch).

Thanks, I must have just had some unlucky timing ;)
  

Patch

diff --git a/gcc/tree-ssa-dom.cc b/gcc/tree-ssa-dom.cc
index 8d8312ca350..e6b8dace5e9 100644
--- a/gcc/tree-ssa-dom.cc
+++ b/gcc/tree-ssa-dom.cc
@@ -684,11 +684,6 @@  record_edge_info (basic_block bb)
 		       !gsi_end_p (gsi);
 		       gsi_next (&gsi))
 		    {
-		      /* If the other alternative is the same as the result,
-			 then this is a degenerate and can be ignored.  */
-		      if (dst == PHI_ARG_DEF (phi, !alternative))
-			continue;
-
 		      /* Now get the EDGE_INFO class so we can append
 			 it to our list.  We want the successor edge
 			 where the destination is not the source of
@@ -697,6 +692,11 @@  record_edge_info (basic_block bb)
 		      tree src = PHI_ARG_DEF (phi, alternative);
 		      tree dst = PHI_RESULT (phi);
 
+		      /* If the other alternative is the same as the result,
+			 then this is a degenerate and can be ignored.  */
+		      if (dst == PHI_ARG_DEF (phi, !alternative))
+			continue;
+
 		      if (EDGE_SUCC (bb, 0)->dest
 			  != EDGE_PRED (bb, !alternative)->src)
 			edge_info = (class edge_info *)EDGE_SUCC (bb, 0)->aux;