| Message ID | 51CC8391.5000900@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 27, 2013 at 02:25:21PM -0400, Andrew MacLeod wrote: > I'm doing some trial file conversions to my proposed wrapper > classes, I'm seeing a couple of places which aren't mapping > properly (they triggered compile errors), and I think its because > there is a couple of bugs regarding the use of IDENTIFIERS in > ssanames.. (I wasn't even aware you could have non-var_decls...) > so this is just a sanity check :-) If ssa_name.var is IDENTIFIER_NODE, it is just like ssa_name.var == NULL, i.e. anonymous SSA_NAME, just that in the dumps it is given some name and not just _NNN. > in tree-ssanames.c:release_ssa_names() : > > if (! SSA_NAME_IN_FREE_LIST (var)) > { > tree saved_ssa_name_var = SSA_NAME_VAR (var); > int saved_ssa_name_version = SSA_NAME_VERSION (var); > use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); > <..> > /* Hopefully this can go away once we have the new incremental > SSA updating code installed. */ > SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); I don't see a big issue with this, sure, you could tree saved_ssa_name_identifier = saved_ssa_name_var ? saved_ssa_name_var : SSA_NAME_IDENTIFIER (var); and use that instead in SET_SSA_NAME_VAR_OR_IDENTIFIER. Jakub
On 06/27/2013 02:39 PM, Jakub Jelinek wrote: > > in tree-ssanames.c:release_ssa_names() : > > if (! SSA_NAME_IN_FREE_LIST (var)) > { > tree saved_ssa_name_var = SSA_NAME_VAR (var); > int saved_ssa_name_version = SSA_NAME_VERSION (var); > use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); > <..> > /* Hopefully this can go away once we have the new incremental > SSA updating code installed. */ > SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); > I don't see a big issue with this, sure, you could > tree saved_ssa_name_identifier = saved_ssa_name_var ? saved_ssa_name_var : SSA_NAME_IDENTIFIER (var); > and use that instead in SET_SSA_NAME_VAR_OR_IDENTIFIER. Yeah I wasn't too concerned about this one, the outof-ssa case looked like more of a possible issue. Maybe neither is, they just popped out as inconsistent uses. Andrew
On Thu, Jun 27, 2013 at 8:52 PM, Andrew MacLeod <amacleod@redhat.com> wrote: > On 06/27/2013 02:39 PM, Jakub Jelinek wrote: >> >> >> in tree-ssanames.c:release_ssa_names() : >> >> if (! SSA_NAME_IN_FREE_LIST (var)) >> { >> tree saved_ssa_name_var = SSA_NAME_VAR (var); >> int saved_ssa_name_version = SSA_NAME_VERSION (var); >> use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); >> <..> >> /* Hopefully this can go away once we have the new incremental >> SSA updating code installed. */ >> SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); >> I don't see a big issue with this, sure, you could >> tree saved_ssa_name_identifier = saved_ssa_name_var ? saved_ssa_name_var : >> SSA_NAME_IDENTIFIER (var); >> and use that instead in SET_SSA_NAME_VAR_OR_IDENTIFIER. > > > Yeah I wasn't too concerned about this one, the outof-ssa case looked like > more of a possible issue. Maybe neither is, they just popped out as > inconsistent uses. Restoring SSA_NAME_VAR_OR_IDENTIFIER is only for debugging. Yes, it probably should save SSA_NAME_VAR_OR_IDENTIFIER instead of SSA_NAME_VAR. Richard. > Andrew >
Index: tree.h =================================================================== --- tree.h (revision 196663) +++ tree.h (working copy) @@ -1917,6 +1918,8 @@ || TREE_CODE ((NODE)->ssa_name.var) == IDENTIFIER_NODE \ ? NULL_TREE : (NODE)->ssa_name.var) +#define SSA_NAME_VAR_OR_IDENTIFIER(NODE) (SSA_NAME_CHECK (NODE)->ssa_name.var) + #define SET_SSA_NAME_VAR_OR_IDENTIFIER(NODE,VAR) \ do { SSA_NAME_CHECK (NODE)->ssa_name.var = (VAR); } while (0) Index: tree-ssanames.c =================================================================== --- tree-ssanames.c (revision 195512) +++ tree-ssanames.c (working copy) @@ -200,7 +200,7 @@ defining statement. */ if (! SSA_NAME_IN_FREE_LIST (var)) { - tree saved_ssa_name_var = SSA_NAME_VAR (var); + tree saved_ssa_name_var = SSA_NAME_VAR_OR_IDENTIFIER (var); int saved_ssa_name_version = SSA_NAME_VERSION (var); use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); Index: tree-outof-ssa.c =================================================================== --- tree-outof-ssa.c (revision 195512) +++ tree-outof-ssa.c (working copy) @@ -1039,7 +1039,8 @@ needed. */ if ((e->flags & EDGE_DFS_BACK) && (TREE_CODE (arg) != SSA_NAME - || SSA_NAME_VAR (arg) != SSA_NAME_VAR (result) + || (SSA_NAME_VAR_OR_IDENTIFIER (arg) + != SSA_NAME_VAR_OR_IDENTIFIER (result)) || trivially_conflicts_p (bb, result, arg))) { tree name;
I'm doing some trial file conversions to my proposed wrapper classes, I'm seeing a couple of places which aren't mapping properly (they triggered compile errors), and I think its because there is a couple of bugs regarding the use of IDENTIFIERS in ssanames.. (I wasn't even aware you could have non-var_decls...) so this is just a sanity check :-) the macros are defined as: /* Returns the IDENTIFIER_NODE giving the SSA name a name or NULL_TREE if there is no name associated with it. */ #define SSA_NAME_IDENTIFIER(NODE) \ (SSA_NAME_CHECK (NODE)->ssa_name.var != NULL_TREE \ ? (TREE_CODE ((NODE)->ssa_name.var) == IDENTIFIER_NODE \ ? (NODE)->ssa_name.var \ : DECL_NAME ((NODE)->ssa_name.var)) \ : NULL_TREE) /* Returns the variable being referenced. This can be NULL_TREE for temporaries not associated with any user variable. Once released, this is the only field that can be relied upon. */ #define SSA_NAME_VAR(NODE) \ (SSA_NAME_CHECK (NODE)->ssa_name.var == NULL_TREE \ || TREE_CODE ((NODE)->ssa_name.var) == IDENTIFIER_NODE \ ? NULL_TREE : (NODE)->ssa_name.var) #define SET_SSA_NAME_VAR_OR_IDENTIFIER(NODE,VAR) \ do { SSA_NAME_CHECK (NODE)->ssa_name.var = (VAR); } while (0) in tree-ssanames.c:release_ssa_names() : if (! SSA_NAME_IN_FREE_LIST (var)) { tree saved_ssa_name_var = SSA_NAME_VAR (var); int saved_ssa_name_version = SSA_NAME_VERSION (var); use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); <..> /* Hopefully this can go away once we have the new incremental SSA updating code installed. */ SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); It would seem to me that if the identifier is set rather than the vardecl, it is not going to be reset properly here since the saved value is going to be NULL rather than the identifier node... Or does it really matter here? Its not really clear to me. in tree-outof-ssa.c:insert_backedge_copies() : for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); edge e = gimple_phi_arg_edge (phi, i); /* If the argument is not an SSA_NAME, then we will need a constant initialization. If the argument is an SSA_NAME with a different underlying variable then a copy statement will be needed. */ if ((e->flags & EDGE_DFS_BACK) && (TREE_CODE (arg) != SSA_NAME || SSA_NAME_VAR (arg) != SSA_NAME_VAR (result) || trivially_conflicts_p (bb, result, arg))) If both SSA_NAMEs have an identifier, then "SSA_NAME_VAR(arg) != SSA_NAME_VAR(result)" is going to compare NULL_TREE != NULL_TREE every time... I would say its really looking to compare that underlying field TREE_CODE ((arg)->ssa_name.var != TREE_CODE ((result)->ssa_name.var They could both be resolved by adding an equivalent #define SSA_NAME_VAR_OR_IDENTIFIER(NODE (SSA_NAME_CHECK (NODE)->ssa_name.var) and using that macro instead of SSA_NAME_VAR.. and probably more efficient. something like that attached, which bootstraps but I haven't done a full testrun yet... Or perhaps even just use SSA_NAME_IDENTIFIER in both cases would work... seems dangerous if 2 different decls have the same identifier tho. It is interesting how these 2 cases just popped up as errors building during the conversion of those files... Early tangible benefit? :-) Andrew * tree.h (SSA_NAME_VAR_OR_IDENTIFIER): New. * tree-ssanames.c: Use instead of SSA_NAME_VAR. * tree-outof-ssa.c: Use instead of SSA_NAME_VAR.