* [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
@ 2023-05-10 7:17 pan2.li
2023-05-10 7:56 ` Richard Biener
` (6 more replies)
0 siblings, 7 replies; 21+ messages in thread
From: pan2.li @ 2023-05-10 7:17 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicition dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
gcc/ChangeLog:
* var-tracking.cc (DECL_OR_VALUE_OR_DEFAULT): New macro for
clean code.
(dv_is_decl_p): Adjust type changes to pointer_mux.
(dv_as_decl): Likewise.
(dv_as_value): Likewise.
(dv_as_opaque): Likewise.
(variable_hasher::equal): Likewise.
(dv_from_decl): Likewise.
(dv_from_value): Likewise.
(shared_hash_find_slot_unshare_1): Likewise.
(shared_hash_find_slot_1): Likewise.
(shared_hash_find_slot_noinsert_1): Likewise.
(shared_hash_find_1): Likewise.
(unshare_variable): Likewise.
(vars_copy): Likewise.
(find_loc_in_1pdv): Likewise.
(find_mem_expr_in_1pdv): Likewise.
(dataflow_set_different): Likewise.
(variable_from_dropped): Likewise.
(variable_was_changed): Likewise.
(loc_exp_insert_dep): Likewise.
(notify_dependents_of_resolved_value): Likewise.
(vt_expand_loc_callback): Likewise.
(remove_value_from_changed_variables): Likewise.
(notify_dependents_of_changed_value): Likewise.
(emit_notes_for_differences_1): Likewise.
(emit_notes_for_differences_2): Likewise.
---
gcc/var-tracking.cc | 119 +++++++++++++++++++++++++++-----------------
1 file changed, 74 insertions(+), 45 deletions(-)
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..9bc9d21e5ba 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,9 +116,17 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
+/* A declaration of a variable, or an RTL value being handled like a
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
+
+#define DECL_OR_VALUE_OR_DEFAULT(ptr) \
+ ((ptr) ? decl_or_value (ptr) : decl_or_value ())
+
/* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
Currently the value is the same as IDENTIFIER_NODE, which has such
@@ -196,15 +204,21 @@ struct micro_operation
};
-/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
-
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ bool is_decl = !dv;
+
+ if (dv)
+ {
+ if (dv.is_first ())
+ is_decl = (int) TREE_CODE (dv.known_first ()) != (int) VALUE;
+ else if (!dv.is_first () && !dv.is_second ())
+ is_decl = true;
+ }
+
+ return is_decl;
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +233,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.is_first () ? dv.known_first () : NULL_TREE;
}
/* Return the value in the decl_or_value. */
@@ -227,14 +241,20 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
+ return dv.is_second () ? dv.known_second () : NULL_RTX;;
}
/* Return the opaque pointer in the decl_or_value. */
static inline void *
dv_as_opaque (decl_or_value dv)
{
- return dv;
+ if (dv.is_first ())
+ return dv.known_first ();
+
+ if (dv.is_second ())
+ return dv.known_second ();
+
+ return NULL;
}
@@ -503,9 +523,7 @@ variable_hasher::hash (const variable *v)
inline bool
variable_hasher::equal (const variable *v, const void *y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return dv_as_opaque (v->dv) == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1414,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = DECL_OR_VALUE_OR_DEFAULT (decl);
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1423,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = DECL_OR_VALUE_OR_DEFAULT (value);
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1661,7 +1677,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars, decl_or_value dv,
{
if (shared_hash_shared (*pvars))
*pvars = shared_hash_unshare (*pvars);
- return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
+ return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash, ins);
}
static inline variable **
@@ -1678,7 +1695,8 @@ shared_hash_find_slot_unshare (shared_hash **pvars, decl_or_value dv,
static inline variable **
shared_hash_find_slot_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash,
+ return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash,
shared_hash_shared (vars)
? NO_INSERT : INSERT);
}
@@ -1695,7 +1713,8 @@ static inline variable **
shared_hash_find_slot_noinsert_1 (shared_hash *vars, decl_or_value dv,
hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash, NO_INSERT);
+ return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash, NO_INSERT);
}
static inline variable **
@@ -1710,7 +1729,7 @@ shared_hash_find_slot_noinsert (shared_hash *vars, decl_or_value dv)
static inline variable *
shared_hash_find_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_with_hash (dv, dvhash);
+ return shared_hash_htab (vars)->find_with_hash (dv_as_opaque (dv), dvhash);
}
static inline variable *
@@ -1807,7 +1826,7 @@ unshare_variable (dataflow_set *set, variable **slot, variable *var,
if (var->in_changed_variables)
{
variable **cslot
- = changed_variables->find_slot_with_hash (var->dv,
+ = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
dv_htab_hash (var->dv),
NO_INSERT);
gcc_assert (*cslot == (void *) var);
@@ -1831,8 +1850,8 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (dv_as_opaque (var->dv),
+ dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -3286,7 +3305,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
gcc_checking_assert (!node->next);
dv = dv_from_value (node->loc);
- rvar = vars->find_with_hash (dv, dv_htab_hash (dv));
+ rvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
return find_loc_in_1pdv (loc, rvar, vars);
}
@@ -4664,7 +4683,7 @@ find_mem_expr_in_1pdv (tree expr, rtx val, variable_table_type *vars)
&& !VALUE_RECURSED_INTO (val));
dv = dv_from_value (val);
- var = vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
return NULL;
@@ -5103,7 +5122,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
var1, variable, hi)
{
variable_table_type *htab = shared_hash_htab (new_set->vars);
- variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
+ variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
+ dv_htab_hash (var1->dv));
if (!var2)
{
@@ -5140,7 +5160,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
var1, variable, hi)
{
variable_table_type *htab = shared_hash_htab (old_set->vars);
- variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
+ variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
+ dv_htab_hash (var1->dv));
if (!var2)
{
if (details)
@@ -7422,7 +7443,8 @@ variable_from_dropped (decl_or_value dv, enum insert_option insert)
variable *empty_var;
onepart_enum onepart;
- slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv), insert);
+ slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv), insert);
if (!slot)
return NULL;
@@ -7493,7 +7515,8 @@ variable_was_changed (variable *var, dataflow_set *set)
/* Remember this decl or VALUE has been added to changed_variables. */
set_dv_changed (var->dv, true);
- slot = changed_variables->find_slot_with_hash (var->dv, hash, INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
+ hash, INSERT);
if (*slot)
{
@@ -7520,9 +7543,8 @@ variable_was_changed (variable *var, dataflow_set *set)
if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
{
- dslot = dropped_values->find_slot_with_hash (var->dv,
- dv_htab_hash (var->dv),
- INSERT);
+ dslot = dropped_values->find_slot_with_hash (
+ dv_as_opaque (var->dv), dv_htab_hash (var->dv), INSERT);
empty_var = *dslot;
if (empty_var)
@@ -8199,7 +8221,7 @@ loc_exp_insert_dep (variable *var, rtx x, variable_table_type *vars)
/* ??? Build a vector of variables parallel to EXPANDING, to avoid
an additional look up? */
- xvar = vars->find_with_hash (dv, dv_htab_hash (dv));
+ xvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!xvar)
{
@@ -8211,7 +8233,8 @@ loc_exp_insert_dep (variable *var, rtx x, variable_table_type *vars)
arise if say the same value appears in two complex expressions in
the same loc_list, or even more than once in a single
expression. */
- if (VAR_LOC_DEP_LST (xvar) && VAR_LOC_DEP_LST (xvar)->dv == var->dv)
+ if (VAR_LOC_DEP_LST (xvar)
+ && dv_as_opaque (VAR_LOC_DEP_LST (xvar)->dv) == dv_as_opaque (var->dv))
return;
if (var->onepart == NOT_ONEPART)
@@ -8307,7 +8330,7 @@ notify_dependents_of_resolved_value (variable *ivar, variable_table_type *vars)
continue;
}
- var = vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
var = variable_from_dropped (dv, NO_INSERT);
@@ -8552,7 +8575,7 @@ vt_expand_loc_callback (rtx x, bitmap regs,
return NULL;
}
- var = elcd->vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = elcd->vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
{
@@ -8959,8 +8982,9 @@ remove_value_from_changed_variables (rtx val)
variable **slot;
variable *var;
- slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
+ NO_INSERT);
var = *slot;
var->in_changed_variables = false;
changed_variables->clear_slot (slot);
@@ -8980,12 +9004,15 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
loc_exp_dep *led;
decl_or_value dv = dv_from_rtx (val);
- slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
+ NO_INSERT);
if (!slot)
- slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
+ slot = htab->find_slot_with_hash (dv_as_opaque (dv), dv_htab_hash (dv),
+ NO_INSERT);
if (!slot)
- slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv),
+ slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
NO_INSERT);
var = *slot;
@@ -9017,14 +9044,14 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
break;
case ONEPART_VDECL:
- ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
+ ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
gcc_checking_assert (!VAR_LOC_DEP_LST (ivar));
variable_was_changed (ivar, NULL);
break;
case NOT_ONEPART:
delete led;
- ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
+ ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
if (ivar)
{
int i = ivar->n_var_parts;
@@ -9119,7 +9146,8 @@ emit_notes_for_differences_1 (variable **slot, variable_table_type *new_vars)
variable *old_var, *new_var;
old_var = *slot;
- new_var = new_vars->find_with_hash (old_var->dv, dv_htab_hash (old_var->dv));
+ new_var = new_vars->find_with_hash (dv_as_opaque (old_var->dv),
+ dv_htab_hash (old_var->dv));
if (!new_var)
{
@@ -9191,7 +9219,8 @@ emit_notes_for_differences_2 (variable **slot, variable_table_type *old_vars)
variable *old_var, *new_var;
new_var = *slot;
- old_var = old_vars->find_with_hash (new_var->dv, dv_htab_hash (new_var->dv));
+ old_var = old_vars->find_with_hash (dv_as_opaque (new_var->dv),
+ dv_htab_hash (new_var->dv));
if (!old_var)
{
int i;
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
@ 2023-05-10 7:56 ` Richard Biener
2023-05-10 8:02 ` Richard Sandiford
2023-05-10 8:13 ` Jakub Jelinek
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Richard Biener @ 2023-05-10 7:56 UTC (permalink / raw)
To: pan2.li
Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw,
richard.sandiford
On Wed, 10 May 2023, pan2.li@intel.com wrote:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to tree_node
> or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the location and bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicition dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
>
> gcc/ChangeLog:
>
> * var-tracking.cc (DECL_OR_VALUE_OR_DEFAULT): New macro for
> clean code.
> (dv_is_decl_p): Adjust type changes to pointer_mux.
> (dv_as_decl): Likewise.
> (dv_as_value): Likewise.
> (dv_as_opaque): Likewise.
> (variable_hasher::equal): Likewise.
> (dv_from_decl): Likewise.
> (dv_from_value): Likewise.
> (shared_hash_find_slot_unshare_1): Likewise.
> (shared_hash_find_slot_1): Likewise.
> (shared_hash_find_slot_noinsert_1): Likewise.
> (shared_hash_find_1): Likewise.
> (unshare_variable): Likewise.
> (vars_copy): Likewise.
> (find_loc_in_1pdv): Likewise.
> (find_mem_expr_in_1pdv): Likewise.
> (dataflow_set_different): Likewise.
> (variable_from_dropped): Likewise.
> (variable_was_changed): Likewise.
> (loc_exp_insert_dep): Likewise.
> (notify_dependents_of_resolved_value): Likewise.
> (vt_expand_loc_callback): Likewise.
> (remove_value_from_changed_variables): Likewise.
> (notify_dependents_of_changed_value): Likewise.
> (emit_notes_for_differences_1): Likewise.
> (emit_notes_for_differences_2): Likewise.
> ---
> gcc/var-tracking.cc | 119 +++++++++++++++++++++++++++-----------------
> 1 file changed, 74 insertions(+), 45 deletions(-)
>
> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
> index fae0c73e02f..9bc9d21e5ba 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,9 +116,17 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> +/* A declaration of a variable, or an RTL value being handled like a
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
> +
> +#define DECL_OR_VALUE_OR_DEFAULT(ptr) \
> + ((ptr) ? decl_or_value (ptr) : decl_or_value ())
> +
> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
> Currently the value is the same as IDENTIFIER_NODE, which has such
> @@ -196,15 +204,21 @@ struct micro_operation
> };
>
>
> -/* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> -
> /* Return true if a decl_or_value DV is a DECL or NULL. */
> static inline bool
> dv_is_decl_p (decl_or_value dv)
> {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + bool is_decl = !dv;
> +
> + if (dv)
> + {
> + if (dv.is_first ())
> + is_decl = (int) TREE_CODE (dv.known_first ()) != (int) VALUE;
> + else if (!dv.is_first () && !dv.is_second ())
> + is_decl = true;
> + }
> +
> + return is_decl;
This all looks very confused, shouldn't it just be
return dv.is_first ();
? All the keying on VALUE should no longer be necessary.
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */
> @@ -219,7 +233,7 @@ static inline tree
> dv_as_decl (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.is_first () ? dv.known_first () : NULL_TREE;
and this should be
return dv.known_first ();
? (knowing that ptr-mux will not mutate 'first' and thus preserves
a nullptr there)
> }
>
> /* Return the value in the decl_or_value. */
> @@ -227,14 +241,20 @@ static inline rtx
> dv_as_value (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> + return dv.is_second () ? dv.known_second () : NULL_RTX;;
return dv.known_second (); (the assert makes sure it isn't nullptr)
> }
>
> /* Return the opaque pointer in the decl_or_value. */
> static inline void *
> dv_as_opaque (decl_or_value dv)
> {
> - return dv;
> + if (dv.is_first ())
> + return dv.known_first ();
> +
> + if (dv.is_second ())
> + return dv.known_second ();
> +
> + return NULL;
> }
I think this function should go away - for equality compares
ptr-mux should probably get an operator== overload (or alternatively
an access to the raw pointer value). For cases like
gcc_checking_assert (loc != dv_as_opaque (var->dv));
one could also use var->dv.second_or_null (). But as said
most uses are doing sth like
if (dv_as_opaque (list->dv) == dv_as_opaque (cdv)
which should just become
if (list->dv == cdv)
Richard - any preference here? Go for operator==/!= or go
for an accessor to m_ptr (maybe as uintptr_t)?
>
> @@ -503,9 +523,7 @@ variable_hasher::hash (const variable *v)
> inline bool
> variable_hasher::equal (const variable *v, const void *y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return dv_as_opaque (v->dv) == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
> @@ -1396,8 +1414,7 @@ onepart_pool_allocate (onepart_enum onepart)
> static inline decl_or_value
> dv_from_decl (tree decl)
> {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = DECL_OR_VALUE_OR_DEFAULT (decl);
it should work to do
devl_or_value dv (decl);
no? Alternatively
devl_or_value dv = decl_or_value::first (decl);
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1423,7 @@ dv_from_decl (tree decl)
> static inline decl_or_value
> dv_from_value (rtx value)
> {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = DECL_OR_VALUE_OR_DEFAULT (value);
> gcc_checking_assert (dv_is_value_p (dv));
the later assert here asserts 'value' wasn't nullptr
> return dv;
> }
> @@ -1661,7 +1677,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars, decl_or_value dv,
> {
> if (shared_hash_shared (*pvars))
> *pvars = shared_hash_unshare (*pvars);
> - return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
> + return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
> + dvhash, ins);
OK, it's probably safes to keep dv_as_opaque for these uses.
Thanks a lot for doing this cleanup.
Richard.
> }
>
> static inline variable **
> @@ -1678,7 +1695,8 @@ shared_hash_find_slot_unshare (shared_hash **pvars, decl_or_value dv,
> static inline variable **
> shared_hash_find_slot_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
> {
> - return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash,
> + return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
> + dvhash,
> shared_hash_shared (vars)
> ? NO_INSERT : INSERT);
> }
> @@ -1695,7 +1713,8 @@ static inline variable **
> shared_hash_find_slot_noinsert_1 (shared_hash *vars, decl_or_value dv,
> hashval_t dvhash)
> {
> - return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash, NO_INSERT);
> + return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
> + dvhash, NO_INSERT);
> }
>
> static inline variable **
> @@ -1710,7 +1729,7 @@ shared_hash_find_slot_noinsert (shared_hash *vars, decl_or_value dv)
> static inline variable *
> shared_hash_find_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
> {
> - return shared_hash_htab (vars)->find_with_hash (dv, dvhash);
> + return shared_hash_htab (vars)->find_with_hash (dv_as_opaque (dv), dvhash);
> }
>
> static inline variable *
> @@ -1807,7 +1826,7 @@ unshare_variable (dataflow_set *set, variable **slot, variable *var,
> if (var->in_changed_variables)
> {
> variable **cslot
> - = changed_variables->find_slot_with_hash (var->dv,
> + = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
> dv_htab_hash (var->dv),
> NO_INSERT);
> gcc_assert (*cslot == (void *) var);
> @@ -1831,8 +1850,8 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (dv_as_opaque (var->dv),
> + dv_htab_hash (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -3286,7 +3305,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> gcc_checking_assert (!node->next);
>
> dv = dv_from_value (node->loc);
> - rvar = vars->find_with_hash (dv, dv_htab_hash (dv));
> + rvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
> return find_loc_in_1pdv (loc, rvar, vars);
> }
>
> @@ -4664,7 +4683,7 @@ find_mem_expr_in_1pdv (tree expr, rtx val, variable_table_type *vars)
> && !VALUE_RECURSED_INTO (val));
>
> dv = dv_from_value (val);
> - var = vars->find_with_hash (dv, dv_htab_hash (dv));
> + var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
>
> if (!var)
> return NULL;
> @@ -5103,7 +5122,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
> var1, variable, hi)
> {
> variable_table_type *htab = shared_hash_htab (new_set->vars);
> - variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
> + variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
> + dv_htab_hash (var1->dv));
>
> if (!var2)
> {
> @@ -5140,7 +5160,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
> var1, variable, hi)
> {
> variable_table_type *htab = shared_hash_htab (old_set->vars);
> - variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
> + variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
> + dv_htab_hash (var1->dv));
> if (!var2)
> {
> if (details)
> @@ -7422,7 +7443,8 @@ variable_from_dropped (decl_or_value dv, enum insert_option insert)
> variable *empty_var;
> onepart_enum onepart;
>
> - slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv), insert);
> + slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
> + dv_htab_hash (dv), insert);
>
> if (!slot)
> return NULL;
> @@ -7493,7 +7515,8 @@ variable_was_changed (variable *var, dataflow_set *set)
> /* Remember this decl or VALUE has been added to changed_variables. */
> set_dv_changed (var->dv, true);
>
> - slot = changed_variables->find_slot_with_hash (var->dv, hash, INSERT);
> + slot = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
> + hash, INSERT);
>
> if (*slot)
> {
> @@ -7520,9 +7543,8 @@ variable_was_changed (variable *var, dataflow_set *set)
>
> if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
> {
> - dslot = dropped_values->find_slot_with_hash (var->dv,
> - dv_htab_hash (var->dv),
> - INSERT);
> + dslot = dropped_values->find_slot_with_hash (
> + dv_as_opaque (var->dv), dv_htab_hash (var->dv), INSERT);
> empty_var = *dslot;
>
> if (empty_var)
> @@ -8199,7 +8221,7 @@ loc_exp_insert_dep (variable *var, rtx x, variable_table_type *vars)
>
> /* ??? Build a vector of variables parallel to EXPANDING, to avoid
> an additional look up? */
> - xvar = vars->find_with_hash (dv, dv_htab_hash (dv));
> + xvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
>
> if (!xvar)
> {
> @@ -8211,7 +8233,8 @@ loc_exp_insert_dep (variable *var, rtx x, variable_table_type *vars)
> arise if say the same value appears in two complex expressions in
> the same loc_list, or even more than once in a single
> expression. */
> - if (VAR_LOC_DEP_LST (xvar) && VAR_LOC_DEP_LST (xvar)->dv == var->dv)
> + if (VAR_LOC_DEP_LST (xvar)
> + && dv_as_opaque (VAR_LOC_DEP_LST (xvar)->dv) == dv_as_opaque (var->dv))
> return;
>
> if (var->onepart == NOT_ONEPART)
> @@ -8307,7 +8330,7 @@ notify_dependents_of_resolved_value (variable *ivar, variable_table_type *vars)
> continue;
> }
>
> - var = vars->find_with_hash (dv, dv_htab_hash (dv));
> + var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
>
> if (!var)
> var = variable_from_dropped (dv, NO_INSERT);
> @@ -8552,7 +8575,7 @@ vt_expand_loc_callback (rtx x, bitmap regs,
> return NULL;
> }
>
> - var = elcd->vars->find_with_hash (dv, dv_htab_hash (dv));
> + var = elcd->vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
>
> if (!var)
> {
> @@ -8959,8 +8982,9 @@ remove_value_from_changed_variables (rtx val)
> variable **slot;
> variable *var;
>
> - slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
> + dv_htab_hash (dv),
> + NO_INSERT);
> var = *slot;
> var->in_changed_variables = false;
> changed_variables->clear_slot (slot);
> @@ -8980,12 +9004,15 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
> loc_exp_dep *led;
> decl_or_value dv = dv_from_rtx (val);
>
> - slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
> + dv_htab_hash (dv),
> + NO_INSERT);
> if (!slot)
> - slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
> + slot = htab->find_slot_with_hash (dv_as_opaque (dv), dv_htab_hash (dv),
> + NO_INSERT);
> if (!slot)
> - slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv),
> + slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
> + dv_htab_hash (dv),
> NO_INSERT);
> var = *slot;
>
> @@ -9017,14 +9044,14 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
> break;
>
> case ONEPART_VDECL:
> - ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
> + ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
> gcc_checking_assert (!VAR_LOC_DEP_LST (ivar));
> variable_was_changed (ivar, NULL);
> break;
>
> case NOT_ONEPART:
> delete led;
> - ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
> + ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
> if (ivar)
> {
> int i = ivar->n_var_parts;
> @@ -9119,7 +9146,8 @@ emit_notes_for_differences_1 (variable **slot, variable_table_type *new_vars)
> variable *old_var, *new_var;
>
> old_var = *slot;
> - new_var = new_vars->find_with_hash (old_var->dv, dv_htab_hash (old_var->dv));
> + new_var = new_vars->find_with_hash (dv_as_opaque (old_var->dv),
> + dv_htab_hash (old_var->dv));
>
> if (!new_var)
> {
> @@ -9191,7 +9219,8 @@ emit_notes_for_differences_2 (variable **slot, variable_table_type *old_vars)
> variable *old_var, *new_var;
>
> new_var = *slot;
> - old_var = old_vars->find_with_hash (new_var->dv, dv_htab_hash (new_var->dv));
> + old_var = old_vars->find_with_hash (dv_as_opaque (new_var->dv),
> + dv_htab_hash (new_var->dv));
> if (!old_var)
> {
> int i;
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
2023-05-10 7:56 ` Richard Biener
@ 2023-05-10 8:02 ` Richard Sandiford
0 siblings, 0 replies; 21+ messages in thread
From: Richard Sandiford @ 2023-05-10 8:02 UTC (permalink / raw)
To: Richard Biener
Cc: pan2.li, gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang,
jeffreyalaw
Richard Biener <rguenther@suse.de> writes:
> On Wed, 10 May 2023, pan2.li@intel.com wrote:
>
>> From: Pan Li <pan2.li@intel.com>
>>
>> The decl_or_value is defined as void * before this PATCH. It will take
>> care of both the tree_node and rtx_def. Unfortunately, given a void
>> pointer cannot tell the input is tree_node or rtx_def.
>>
>> Then we have some implicit structure layout requirement similar as
>> below. Or we will touch unreasonable bits when cast void * to tree_node
>> or rtx_def.
>>
>> +--------+-----------+----------+
>> | offset | tree_node | rtx_def |
>> +--------+-----------+----------+
>> | 0 | code: 16 | code: 16 | <- require the location and bitssize
>> +--------+-----------+----------+
>> | 16 | ... | mode: 8 |
>> +--------+-----------+----------+
>> | ... |
>> +--------+-----------+----------+
>> | 24 | ... | ... |
>> +--------+-----------+----------+
>>
>> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
>> 16 bits for running out of machine mode. This PATCH introduced the
>> pointer_mux to tell the input is tree_node or rtx_def, and decouple
>> the above implicition dependency.
>>
>> Signed-off-by: Pan Li <pan2.li@intel.com>
>> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
>> Co-Authored-By: Richard Biener <rguenther@suse.de>
>>
>> gcc/ChangeLog:
>>
>> * var-tracking.cc (DECL_OR_VALUE_OR_DEFAULT): New macro for
>> clean code.
>> (dv_is_decl_p): Adjust type changes to pointer_mux.
>> (dv_as_decl): Likewise.
>> (dv_as_value): Likewise.
>> (dv_as_opaque): Likewise.
>> (variable_hasher::equal): Likewise.
>> (dv_from_decl): Likewise.
>> (dv_from_value): Likewise.
>> (shared_hash_find_slot_unshare_1): Likewise.
>> (shared_hash_find_slot_1): Likewise.
>> (shared_hash_find_slot_noinsert_1): Likewise.
>> (shared_hash_find_1): Likewise.
>> (unshare_variable): Likewise.
>> (vars_copy): Likewise.
>> (find_loc_in_1pdv): Likewise.
>> (find_mem_expr_in_1pdv): Likewise.
>> (dataflow_set_different): Likewise.
>> (variable_from_dropped): Likewise.
>> (variable_was_changed): Likewise.
>> (loc_exp_insert_dep): Likewise.
>> (notify_dependents_of_resolved_value): Likewise.
>> (vt_expand_loc_callback): Likewise.
>> (remove_value_from_changed_variables): Likewise.
>> (notify_dependents_of_changed_value): Likewise.
>> (emit_notes_for_differences_1): Likewise.
>> (emit_notes_for_differences_2): Likewise.
>> ---
>> gcc/var-tracking.cc | 119 +++++++++++++++++++++++++++-----------------
>> 1 file changed, 74 insertions(+), 45 deletions(-)
>>
>> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
>> index fae0c73e02f..9bc9d21e5ba 100644
>> --- a/gcc/var-tracking.cc
>> +++ b/gcc/var-tracking.cc
>> @@ -116,9 +116,17 @@
>> #include "fibonacci_heap.h"
>> #include "print-rtl.h"
>> #include "function-abi.h"
>> +#include "mux-utils.h"
>>
>> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>>
>> +/* A declaration of a variable, or an RTL value being handled like a
>> + declaration by pointer_mux. */
>> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>> +
>> +#define DECL_OR_VALUE_OR_DEFAULT(ptr) \
>> + ((ptr) ? decl_or_value (ptr) : decl_or_value ())
>> +
>> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
>> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
>> Currently the value is the same as IDENTIFIER_NODE, which has such
>> @@ -196,15 +204,21 @@ struct micro_operation
>> };
>>
>>
>> -/* A declaration of a variable, or an RTL value being handled like a
>> - declaration. */
>> -typedef void *decl_or_value;
>> -
>> /* Return true if a decl_or_value DV is a DECL or NULL. */
>> static inline bool
>> dv_is_decl_p (decl_or_value dv)
>> {
>> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
>> + bool is_decl = !dv;
>> +
>> + if (dv)
>> + {
>> + if (dv.is_first ())
>> + is_decl = (int) TREE_CODE (dv.known_first ()) != (int) VALUE;
>> + else if (!dv.is_first () && !dv.is_second ())
>> + is_decl = true;
>> + }
>> +
>> + return is_decl;
>
> This all looks very confused, shouldn't it just be
>
> return dv.is_first ();
>
> ? All the keying on VALUE should no longer be necessary.
>
>> }
>>
>> /* Return true if a decl_or_value is a VALUE rtl. */
>> @@ -219,7 +233,7 @@ static inline tree
>> dv_as_decl (decl_or_value dv)
>> {
>> gcc_checking_assert (dv_is_decl_p (dv));
>> - return (tree) dv;
>> + return dv.is_first () ? dv.known_first () : NULL_TREE;
>
> and this should be
>
> return dv.known_first ();
>
> ? (knowing that ptr-mux will not mutate 'first' and thus preserves
> a nullptr there)
>
>> }
>>
>> /* Return the value in the decl_or_value. */
>> @@ -227,14 +241,20 @@ static inline rtx
>> dv_as_value (decl_or_value dv)
>> {
>> gcc_checking_assert (dv_is_value_p (dv));
>> - return (rtx)dv;
>> + return dv.is_second () ? dv.known_second () : NULL_RTX;;
>
> return dv.known_second (); (the assert makes sure it isn't nullptr)
>
>> }
>>
>> /* Return the opaque pointer in the decl_or_value. */
>> static inline void *
>> dv_as_opaque (decl_or_value dv)
>> {
>> - return dv;
>> + if (dv.is_first ())
>> + return dv.known_first ();
>> +
>> + if (dv.is_second ())
>> + return dv.known_second ();
>> +
>> + return NULL;
>> }
>
> I think this function should go away
Was just about to say the same thing :)
Admittedly, I think I might be missing some of the intended abstraction
here, but at least for variable_table_type, it seems that the void *
compare_type in:
struct variable_hasher : pointer_hash <variable>
{
typedef void *compare_type;
static inline hashval_t hash (const variable *);
static inline bool equal (const variable *, const void *);
static inline void remove (variable *);
};
could/should really be decl_or_value, even under the current scheme.
Perhaps the current code predates strongly-typed hash containers.
> - for equality compares
> ptr-mux should probably get an operator== overload (or alternatively
> an access to the raw pointer value). For cases like
>
> gcc_checking_assert (loc != dv_as_opaque (var->dv));
>
> one could also use var->dv.second_or_null (). But as said
> most uses are doing sth like
>
> if (dv_as_opaque (list->dv) == dv_as_opaque (cdv)
>
> which should just become
>
> if (list->dv == cdv)
>
> Richard - any preference here? Go for operator==/!= or go
> for an accessor to m_ptr (maybe as uintptr_t)?
Would prefer ==/!= for now. Kind-of wary of providing access to the
underlying representation.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
2023-05-10 7:56 ` Richard Biener
@ 2023-05-10 8:13 ` Jakub Jelinek
2023-05-10 11:59 ` Li, Pan2
2023-05-10 11:57 ` [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value pan2.li
` (4 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2023-05-10 8:13 UTC (permalink / raw)
To: pan2.li
Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw,
rguenther, richard.sandiford
On Wed, May 10, 2023 at 03:17:58PM +0800, Pan Li via Gcc-patches wrote:
> gcc/ChangeLog:
>
> * var-tracking.cc (DECL_OR_VALUE_OR_DEFAULT): New macro for
> clean code.
ChangeLog formatting shouldn't have spaces after the initial tab.
Furthermore, the entry doesn't describe what changes you've made.
It should start with:
* var-tracking.cc: Include mux-utils.h.
(decl_or_value): Changed from void * to
pointer_mux<tree_node, rtx_def>.
(DECL_OR_VALUE_OR_DEFAULT): Define.
etc.
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,9 +116,17 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> +/* A declaration of a variable, or an RTL value being handled like a
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
> +
> +#define DECL_OR_VALUE_OR_DEFAULT(ptr) \
> + ((ptr) ? decl_or_value (ptr) : decl_or_value ())
> +
> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
> Currently the value is the same as IDENTIFIER_NODE, which has such
> @@ -196,15 +204,21 @@ struct micro_operation
> };
>
>
> -/* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> -
> /* Return true if a decl_or_value DV is a DECL or NULL. */
> static inline bool
> dv_is_decl_p (decl_or_value dv)
> {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + bool is_decl = !dv;
> +
> + if (dv)
> + {
> + if (dv.is_first ())
> + is_decl = (int) TREE_CODE (dv.known_first ()) != (int) VALUE;
> + else if (!dv.is_first () && !dv.is_second ())
> + is_decl = true;
> + }
> +
> + return is_decl;
I really don't understand why it needs to be so complicated.
decl_or_value is dv_is_decl_p if it is NULL or if it is a tree,
and is false if it is rtx VALUE, no other rtxes are expected.
pointer_mux<tree_node, rtx_def> should accept nullptr as being the first
one, so i'd expect just
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
static inline bool
dv_is_value_p (decl_or_value dv)
{
return dv.is_second ();
}
/* Return the decl in the decl_or_value. */
static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
return dv.known_first ();
}
/* Return the value in the decl_or_value. */
static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
return dv.known_second ();
}
/* Return the opaque pointer in the decl_or_value. */
static inline void *
dv_as_opaque (decl_or_value dv)
{
return dv.is_first () ? (void *) dv.known_first ()
: (void *) dv.known_second ();
}
// Ideally dv_as_opaque would just return m_ptr but that
// is unfortunately private.
And define a hasher for decl_or_value now that it is a class
(that would hash/compare the m_ptr value or separately
dv.is_first () bool and dv_as_opaque pointer).
And then I'd hope you don't need to do any further changes
in the file.
Jakub
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
2023-05-10 7:56 ` Richard Biener
2023-05-10 8:13 ` Jakub Jelinek
@ 2023-05-10 11:57 ` pan2.li
2023-05-10 12:22 ` Jakub Jelinek
2023-05-10 15:23 ` [PATCH v3] " pan2.li
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: pan2.li @ 2023-05-10 11:57 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
jakub, rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicition dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
gcc/ChangeLog:
* mux-utils.h: Add overload operator == and != for pointer_mux..
* var-tracking.cc: Included mux-utils.h for pointer_tmux.
(decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
(dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
(dv_as_decl): Likewise.
(dv_as_value): Likewise.
(dv_as_opaque): Likewise.
(variable_hasher::equal): Likewise.
(dv_from_decl): Likewise.
(dv_from_value): Likewise.
(attrs_list_member): Likewise.
(shared_hash_find_slot_unshare_1): Likewise.
(shared_hash_find_slot_1): Likewise.
(shared_hash_find_slot_noinsert_1): Likewise.
(shared_hash_find_1): Likewise.
(unshare_variable): Likewise.
(vars_copy): Likewise.
(var_reg_decl_set): Likewise.
(var_reg_delete_and_set): Likewise.
(find_loc_in_1pdv): Likewise.
(canonicalize_values_star): Likewise.
(variable_post_merge_new_vals): Likewise.
(find_mem_expr_in_1pdv): Likewise.
(dump_onepart_variable_differences): Likewise.
(variable_different_p): Likewise.
(dataflow_set_different): Likewise.
(variable_from_dropped): Likewise.
(variable_was_changed): Likewise.
(set_slot_part): Likewise.
(clobber_slot_part): Likewise.
(loc_exp_insert_dep): Likewise.
(notify_dependents_of_resolved_value): Likewise.
(vt_expand_loc_callback): Likewise.
(remove_value_from_changed_variables): Likewise
(notify_dependents_of_changed_value): Likewise.
(emit_notes_for_differences_1): Likewise.
(emit_notes_for_differences_2): Likewise.
---
gcc/mux-utils.h | 12 ++++
gcc/var-tracking.cc | 145 +++++++++++++++++++++++---------------------
2 files changed, 87 insertions(+), 70 deletions(-)
diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
index a2b6a316899..3eec4edc833 100644
--- a/gcc/mux-utils.h
+++ b/gcc/mux-utils.h
@@ -72,6 +72,18 @@ public:
// Return true unless the pointer is a null A pointer.
explicit operator bool () const { return m_ptr; }
+ // Return true if class has the same m_ptr, or false.
+ bool operator == (const pointer_mux &other)
+ {
+ return this->m_ptr == other.m_ptr;
+ }
+
+ // Return true if class has the different m_ptr, or false.
+ bool operator != (const pointer_mux &other)
+ {
+ return this->m_ptr != other.m_ptr;
+ }
+
// Assign A and B pointers respectively.
void set_first (T1 *ptr) { *this = first (ptr); }
void set_second (T2 *ptr) { *this = second (ptr); }
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..48a585423d9 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,9 +116,14 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
+/* A declaration of a variable, or an RTL value being handled like a
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
+
/* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
Currently the value is the same as IDENTIFIER_NODE, which has such
@@ -196,15 +201,11 @@ struct micro_operation
};
-/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
-
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +220,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.known_first ();
}
/* Return the value in the decl_or_value. */
@@ -227,14 +228,15 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
+ return dv.known_second ();
}
/* Return the opaque pointer in the decl_or_value. */
static inline void *
dv_as_opaque (decl_or_value dv)
{
- return dv;
+ return dv.is_first () ? (void *) dv.known_first ()
+ : (void *) dv.known_second ();
}
@@ -503,9 +505,7 @@ variable_hasher::hash (const variable *v)
inline bool
variable_hasher::equal (const variable *v, const void *y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return dv_as_opaque (v->dv) == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1396,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = decl_or_value::first (decl);
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1405,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = decl_or_value::second (value);
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1519,7 +1517,7 @@ static attrs *
attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
{
for (; list; list = list->next)
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
+ if (list->dv == dv && list->offset == offset)
return list;
return NULL;
}
@@ -1661,7 +1659,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars, decl_or_value dv,
{
if (shared_hash_shared (*pvars))
*pvars = shared_hash_unshare (*pvars);
- return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
+ return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash, ins);
}
static inline variable **
@@ -1678,7 +1677,8 @@ shared_hash_find_slot_unshare (shared_hash **pvars, decl_or_value dv,
static inline variable **
shared_hash_find_slot_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash,
+ return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash,
shared_hash_shared (vars)
? NO_INSERT : INSERT);
}
@@ -1695,7 +1695,8 @@ static inline variable **
shared_hash_find_slot_noinsert_1 (shared_hash *vars, decl_or_value dv,
hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_slot_with_hash (dv, dvhash, NO_INSERT);
+ return shared_hash_htab (vars)->find_slot_with_hash (dv_as_opaque (dv),
+ dvhash, NO_INSERT);
}
static inline variable **
@@ -1710,7 +1711,7 @@ shared_hash_find_slot_noinsert (shared_hash *vars, decl_or_value dv)
static inline variable *
shared_hash_find_1 (shared_hash *vars, decl_or_value dv, hashval_t dvhash)
{
- return shared_hash_htab (vars)->find_with_hash (dv, dvhash);
+ return shared_hash_htab (vars)->find_with_hash (dv_as_opaque (dv), dvhash);
}
static inline variable *
@@ -1807,7 +1808,7 @@ unshare_variable (dataflow_set *set, variable **slot, variable *var,
if (var->in_changed_variables)
{
variable **cslot
- = changed_variables->find_slot_with_hash (var->dv,
+ = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
dv_htab_hash (var->dv),
NO_INSERT);
gcc_assert (*cslot == (void *) var);
@@ -1831,8 +1832,8 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (dv_as_opaque (var->dv),
+ dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -1866,8 +1867,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
for (node = set->regs[REGNO (loc)]; node; node = node->next)
- if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
- && node->offset == offset)
+ if (node->dv == dv && node->offset == offset)
break;
if (!node)
attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
@@ -1966,7 +1966,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
for (node = *nextp; node; node = next)
{
next = node->next;
- if (dv_as_opaque (node->dv) != decl || node->offset != offset)
+ if (node->dv.first_or_null () != decl || node->offset != offset)
{
delete_variable_part (set, node->loc, node->dv, node->offset);
delete node;
@@ -3242,7 +3242,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
if (!var->n_var_parts)
return NULL;
- gcc_checking_assert (loc != dv_as_opaque (var->dv));
+ gcc_checking_assert (loc != var->dv.second_or_null ());
loc_code = GET_CODE (loc);
for (node = var->var_part[0].loc_chain; node; node = node->next)
@@ -3286,7 +3286,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
gcc_checking_assert (!node->next);
dv = dv_from_value (node->loc);
- rvar = vars->find_with_hash (dv, dv_htab_hash (dv));
+ rvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
return find_loc_in_1pdv (loc, rvar, vars);
}
@@ -3832,16 +3832,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
break;
list = list->next;
}
gcc_assert (list);
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
list->dv = cdv;
for (listp = &list->next; (list = *listp); listp = &list->next)
@@ -3849,7 +3847,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ if (list->dv == cdv)
{
*listp = list->next;
delete list;
@@ -3857,17 +3855,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
+ gcc_assert (list->dv != dv);
}
}
- else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ else if (list->dv == cdv)
{
for (listp = &list->next; (list = *listp); listp = &list->next)
{
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
*listp = list->next;
delete list;
@@ -3875,7 +3873,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
+ gcc_assert (list->dv != cdv);
}
}
else
@@ -3884,9 +3882,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (flag_checking)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
gcc_unreachable ();
list = list->next;
@@ -4475,7 +4471,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
check_dupes = true;
break;
}
- else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
+ else if (att->dv == var->dv)
curp = attp;
}
@@ -4485,7 +4481,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
while (*curp)
if ((*curp)->offset == 0
&& GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
- && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
+ && (*curp)->dv == var->dv)
break;
else
curp = &(*curp)->next;
@@ -4664,7 +4660,7 @@ find_mem_expr_in_1pdv (tree expr, rtx val, variable_table_type *vars)
&& !VALUE_RECURSED_INTO (val));
dv = dv_from_value (val);
- var = vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
return NULL;
@@ -4989,7 +4985,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
gcc_assert (var1 != var2);
gcc_assert (dump_file);
- gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
+ gcc_assert (var1->dv == var2->dv);
gcc_assert (var1->n_var_parts == 1
&& var2->n_var_parts == 1);
@@ -5054,8 +5050,7 @@ variable_different_p (variable *var1, variable *var2)
if (var1->onepart && var1->n_var_parts)
{
- gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
- && var1->n_var_parts == 1);
+ gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
/* One-part values have locations in a canonical order. */
return onepart_variable_different_p (var1, var2);
}
@@ -5103,7 +5098,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
var1, variable, hi)
{
variable_table_type *htab = shared_hash_htab (new_set->vars);
- variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
+ variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
+ dv_htab_hash (var1->dv));
if (!var2)
{
@@ -5140,7 +5136,8 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set)
var1, variable, hi)
{
variable_table_type *htab = shared_hash_htab (old_set->vars);
- variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv));
+ variable *var2 = htab->find_with_hash (dv_as_opaque (var1->dv),
+ dv_htab_hash (var1->dv));
if (!var2)
{
if (details)
@@ -7422,7 +7419,8 @@ variable_from_dropped (decl_or_value dv, enum insert_option insert)
variable *empty_var;
onepart_enum onepart;
- slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv), insert);
+ slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv), insert);
if (!slot)
return NULL;
@@ -7493,7 +7491,8 @@ variable_was_changed (variable *var, dataflow_set *set)
/* Remember this decl or VALUE has been added to changed_variables. */
set_dv_changed (var->dv, true);
- slot = changed_variables->find_slot_with_hash (var->dv, hash, INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (var->dv),
+ hash, INSERT);
if (*slot)
{
@@ -7520,9 +7519,10 @@ variable_was_changed (variable *var, dataflow_set *set)
if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
{
- dslot = dropped_values->find_slot_with_hash (var->dv,
- dv_htab_hash (var->dv),
- INSERT);
+ dslot =
+ dropped_values->find_slot_with_hash (dv_as_opaque (var->dv),
+ dv_htab_hash (var->dv),
+ INSERT);
empty_var = *dslot;
if (empty_var)
@@ -7656,7 +7656,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
onepart = dv_onepart_p (dv);
gcc_checking_assert (offset == 0 || !onepart);
- gcc_checking_assert (loc != dv_as_opaque (dv));
+ gcc_checking_assert (loc != dv.second_or_null ());
if (! flag_var_tracking_uninit)
initialized = VAR_INIT_STATUS_INITIALIZED;
@@ -7684,7 +7684,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
{
int r = -1, c = 0;
- gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
+ gcc_assert (var->dv == dv);
pos = 0;
@@ -7950,8 +7950,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
for (anode = *anextp; anode; anode = anext)
{
anext = anode->next;
- if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
- && anode->offset == offset)
+ if (anode->dv == var->dv && anode->offset == offset)
{
delete anode;
*anextp = anext;
@@ -8199,7 +8198,7 @@ loc_exp_insert_dep (variable *var, rtx x, variable_table_type *vars)
/* ??? Build a vector of variables parallel to EXPANDING, to avoid
an additional look up? */
- xvar = vars->find_with_hash (dv, dv_htab_hash (dv));
+ xvar = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!xvar)
{
@@ -8307,7 +8306,7 @@ notify_dependents_of_resolved_value (variable *ivar, variable_table_type *vars)
continue;
}
- var = vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
var = variable_from_dropped (dv, NO_INSERT);
@@ -8552,7 +8551,7 @@ vt_expand_loc_callback (rtx x, bitmap regs,
return NULL;
}
- var = elcd->vars->find_with_hash (dv, dv_htab_hash (dv));
+ var = elcd->vars->find_with_hash (dv_as_opaque (dv), dv_htab_hash (dv));
if (!var)
{
@@ -8959,8 +8958,9 @@ remove_value_from_changed_variables (rtx val)
variable **slot;
variable *var;
- slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
+ NO_INSERT);
var = *slot;
var->in_changed_variables = false;
changed_variables->clear_slot (slot);
@@ -8980,12 +8980,15 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
loc_exp_dep *led;
decl_or_value dv = dv_from_rtx (val);
- slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ slot = changed_variables->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
+ NO_INSERT);
if (!slot)
- slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
+ slot = htab->find_slot_with_hash (dv_as_opaque (dv), dv_htab_hash (dv),
+ NO_INSERT);
if (!slot)
- slot = dropped_values->find_slot_with_hash (dv, dv_htab_hash (dv),
+ slot = dropped_values->find_slot_with_hash (dv_as_opaque (dv),
+ dv_htab_hash (dv),
NO_INSERT);
var = *slot;
@@ -9017,14 +9020,14 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
break;
case ONEPART_VDECL:
- ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
+ ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
gcc_checking_assert (!VAR_LOC_DEP_LST (ivar));
variable_was_changed (ivar, NULL);
break;
case NOT_ONEPART:
delete led;
- ivar = htab->find_with_hash (ldv, dv_htab_hash (ldv));
+ ivar = htab->find_with_hash (dv_as_opaque (ldv), dv_htab_hash (ldv));
if (ivar)
{
int i = ivar->n_var_parts;
@@ -9119,7 +9122,8 @@ emit_notes_for_differences_1 (variable **slot, variable_table_type *new_vars)
variable *old_var, *new_var;
old_var = *slot;
- new_var = new_vars->find_with_hash (old_var->dv, dv_htab_hash (old_var->dv));
+ new_var = new_vars->find_with_hash (dv_as_opaque (old_var->dv),
+ dv_htab_hash (old_var->dv));
if (!new_var)
{
@@ -9191,7 +9195,8 @@ emit_notes_for_differences_2 (variable **slot, variable_table_type *old_vars)
variable *old_var, *new_var;
new_var = *slot;
- old_var = old_vars->find_with_hash (new_var->dv, dv_htab_hash (new_var->dv));
+ old_var = old_vars->find_with_hash (dv_as_opaque (new_var->dv),
+ dv_htab_hash (new_var->dv));
if (!old_var)
{
int i;
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
2023-05-10 8:13 ` Jakub Jelinek
@ 2023-05-10 11:59 ` Li, Pan2
0 siblings, 0 replies; 21+ messages in thread
From: Li, Pan2 @ 2023-05-10 11:59 UTC (permalink / raw)
To: Jakub Jelinek
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, rguenther, richard.sandiford
Thanks all for comments.
Looks like pay too much attention for the NULL check but it is covered by pointer_mux already. Update PATCH v2 as below, please help to review continuously.
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618007.html
Pan
-----Original Message-----
From: Jakub Jelinek <jakub@redhat.com>
Sent: Wednesday, May 10, 2023 4:14 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; rguenther@suse.de; richard.sandiford@arm.com
Subject: Re: [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value
On Wed, May 10, 2023 at 03:17:58PM +0800, Pan Li via Gcc-patches wrote:
> gcc/ChangeLog:
>
> * var-tracking.cc (DECL_OR_VALUE_OR_DEFAULT): New macro for
> clean code.
ChangeLog formatting shouldn't have spaces after the initial tab.
Furthermore, the entry doesn't describe what changes you've made.
It should start with:
* var-tracking.cc: Include mux-utils.h.
(decl_or_value): Changed from void * to
pointer_mux<tree_node, rtx_def>.
(DECL_OR_VALUE_OR_DEFAULT): Define.
etc.
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,9 +116,17 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> +/* A declaration of a variable, or an RTL value being handled like a
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
> +
> +#define DECL_OR_VALUE_OR_DEFAULT(ptr) \
> + ((ptr) ? decl_or_value (ptr) : decl_or_value ())
> +
> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
> Currently the value is the same as IDENTIFIER_NODE, which has such
> @@ -196,15 +204,21 @@ struct micro_operation };
>
>
> -/* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> -
> /* Return true if a decl_or_value DV is a DECL or NULL. */ static
> inline bool dv_is_decl_p (decl_or_value dv) {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + bool is_decl = !dv;
> +
> + if (dv)
> + {
> + if (dv.is_first ())
> + is_decl = (int) TREE_CODE (dv.known_first ()) != (int) VALUE;
> + else if (!dv.is_first () && !dv.is_second ())
> + is_decl = true;
> + }
> +
> + return is_decl;
I really don't understand why it needs to be so complicated.
decl_or_value is dv_is_decl_p if it is NULL or if it is a tree, and is false if it is rtx VALUE, no other rtxes are expected.
pointer_mux<tree_node, rtx_def> should accept nullptr as being the first one, so i'd expect just
/* Return true if a decl_or_value DV is a DECL or NULL. */ static inline bool dv_is_decl_p (decl_or_value dv) {
return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */ static inline bool dv_is_value_p (decl_or_value dv) {
return dv.is_second ();
}
/* Return the decl in the decl_or_value. */ static inline tree dv_as_decl (decl_or_value dv) {
gcc_checking_assert (dv_is_decl_p (dv));
return dv.known_first ();
}
/* Return the value in the decl_or_value. */ static inline rtx dv_as_value (decl_or_value dv) {
gcc_checking_assert (dv_is_value_p (dv));
return dv.known_second ();
}
/* Return the opaque pointer in the decl_or_value. */ static inline void * dv_as_opaque (decl_or_value dv) {
return dv.is_first () ? (void *) dv.known_first ()
: (void *) dv.known_second ();
}
// Ideally dv_as_opaque would just return m_ptr but that // is unfortunately private.
And define a hasher for decl_or_value now that it is a class (that would hash/compare the m_ptr value or separately dv.is_first () bool and dv_as_opaque pointer).
And then I'd hope you don't need to do any further changes in the file.
Jakub
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 11:57 ` [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value pan2.li
@ 2023-05-10 12:22 ` Jakub Jelinek
2023-05-10 12:53 ` Richard Sandiford
0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2023-05-10 12:22 UTC (permalink / raw)
To: pan2.li
Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw,
rguenther, richard.sandiford
On Wed, May 10, 2023 at 07:57:05PM +0800, pan2.li@intel.com wrote:
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,9 +116,14 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> +/* A declaration of a variable, or an RTL value being handled like a
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
> +
> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
> Currently the value is the same as IDENTIFIER_NODE, which has such
> @@ -196,15 +201,11 @@ struct micro_operation
> };
>
>
> -/* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
Why do you move the typedef?
> @@ -503,9 +505,7 @@ variable_hasher::hash (const variable *v)
> inline bool
> variable_hasher::equal (const variable *v, const void *y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return dv_as_opaque (v->dv) == y;
> }
I'm not convinced this is correct. I think all the find_slot_with_hash
etc. pass in a decl_or_value, so I'd expect y to have decl_or_value
type or something similar.
> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
> @@ -1396,8 +1396,7 @@ onepart_pool_allocate (onepart_enum onepart)
> static inline decl_or_value
> dv_from_decl (tree decl)
> {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl_or_value::first (decl);
Can't you just decl_or_value dv = decl; ? I think pointer_mux has ctors
from pointers to the template parameter types.
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1405,7 @@ dv_from_decl (tree decl)
> static inline decl_or_value
> dv_from_value (rtx value)
> {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = decl_or_value::second (value);
Ditto.
> @@ -1661,7 +1659,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars, decl_or_value dv,
> {
> if (shared_hash_shared (*pvars))
> *pvars = shared_hash_unshare (*pvars);
> - return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
> + return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
> + dvhash, ins);
Then you wouldn't need to change all these.
Jakub
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 12:22 ` Jakub Jelinek
@ 2023-05-10 12:53 ` Richard Sandiford
2023-05-10 13:57 ` Li, Pan2
0 siblings, 1 reply; 21+ messages in thread
From: Richard Sandiford @ 2023-05-10 12:53 UTC (permalink / raw)
To: Jakub Jelinek
Cc: pan2.li, gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang,
jeffreyalaw, rguenther
Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, May 10, 2023 at 07:57:05PM +0800, pan2.li@intel.com wrote:
>> --- a/gcc/var-tracking.cc
>> +++ b/gcc/var-tracking.cc
>> @@ -116,9 +116,14 @@
>> #include "fibonacci_heap.h"
>> #include "print-rtl.h"
>> #include "function-abi.h"
>> +#include "mux-utils.h"
>>
>> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>>
>> +/* A declaration of a variable, or an RTL value being handled like a
>> + declaration by pointer_mux. */
>> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>> +
>> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
>> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
>> Currently the value is the same as IDENTIFIER_NODE, which has such
>> @@ -196,15 +201,11 @@ struct micro_operation
>> };
>>
>>
>> -/* A declaration of a variable, or an RTL value being handled like a
>> - declaration. */
>> -typedef void *decl_or_value;
>
> Why do you move the typedef?
>
>> @@ -503,9 +505,7 @@ variable_hasher::hash (const variable *v)
>> inline bool
>> variable_hasher::equal (const variable *v, const void *y)
>> {
>> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
>> -
>> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
>> + return dv_as_opaque (v->dv) == y;
>> }
>
> I'm not convinced this is correct. I think all the find_slot_with_hash
> etc. pass in a decl_or_value, so I'd expect y to have decl_or_value
> type or something similar.
>
>> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
>> @@ -1396,8 +1396,7 @@ onepart_pool_allocate (onepart_enum onepart)
>> static inline decl_or_value
>> dv_from_decl (tree decl)
>> {
>> - decl_or_value dv;
>> - dv = decl;
>> + decl_or_value dv = decl_or_value::first (decl);
>
> Can't you just decl_or_value dv = decl; ? I think pointer_mux has ctors
> from pointers to the template parameter types.
>
>> gcc_checking_assert (dv_is_decl_p (dv));
>> return dv;
>> }
>> @@ -1406,8 +1405,7 @@ dv_from_decl (tree decl)
>> static inline decl_or_value
>> dv_from_value (rtx value)
>> {
>> - decl_or_value dv;
>> - dv = value;
>> + decl_or_value dv = decl_or_value::second (value);
>
> Ditto.
>
>> @@ -1661,7 +1659,8 @@ shared_hash_find_slot_unshare_1 (shared_hash **pvars, decl_or_value dv,
>> {
>> if (shared_hash_shared (*pvars))
>> *pvars = shared_hash_unshare (*pvars);
>> - return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash, ins);
>> + return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
>> + dvhash, ins);
>
> Then you wouldn't need to change all these.
Also, please do try changing variable_hasher::compare_type to
decl_or_value, and changing the type of the second parameter to
variable_hasher::equal accordingly. I still feel that we should
be able to get rid of dv_as_opaque entirely.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 12:53 ` Richard Sandiford
@ 2023-05-10 13:57 ` Li, Pan2
0 siblings, 0 replies; 21+ messages in thread
From: Li, Pan2 @ 2023-05-10 13:57 UTC (permalink / raw)
To: Richard Sandiford, Jakub Jelinek
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, rguenther
I see, will try to get rid of dv_as_opaque everywhere. Thank you all!
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Wednesday, May 10, 2023 8:53 PM
To: Jakub Jelinek <jakub@redhat.com>
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; rguenther@suse.de
Subject: Re: [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, May 10, 2023 at 07:57:05PM +0800, pan2.li@intel.com wrote:
>> --- a/gcc/var-tracking.cc
>> +++ b/gcc/var-tracking.cc
>> @@ -116,9 +116,14 @@
>> #include "fibonacci_heap.h"
>> #include "print-rtl.h"
>> #include "function-abi.h"
>> +#include "mux-utils.h"
>>
>> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>>
>> +/* A declaration of a variable, or an RTL value being handled like a
>> + declaration by pointer_mux. */
>> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>> +
>> /* var-tracking.cc assumes that tree code with the same value as VALUE rtx code
>> has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
>> Currently the value is the same as IDENTIFIER_NODE, which has
>> such @@ -196,15 +201,11 @@ struct micro_operation };
>>
>>
>> -/* A declaration of a variable, or an RTL value being handled like a
>> - declaration. */
>> -typedef void *decl_or_value;
>
> Why do you move the typedef?
>
>> @@ -503,9 +505,7 @@ variable_hasher::hash (const variable *v) inline
>> bool variable_hasher::equal (const variable *v, const void *y) {
>> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
>> -
>> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
>> + return dv_as_opaque (v->dv) == y;
>> }
>
> I'm not convinced this is correct. I think all the
> find_slot_with_hash etc. pass in a decl_or_value, so I'd expect y to
> have decl_or_value type or something similar.
>
>> /* Free the element of VARIABLE_HTAB (its type is struct
>> variable_def). */ @@ -1396,8 +1396,7 @@ onepart_pool_allocate
>> (onepart_enum onepart) static inline decl_or_value dv_from_decl
>> (tree decl) {
>> - decl_or_value dv;
>> - dv = decl;
>> + decl_or_value dv = decl_or_value::first (decl);
>
> Can't you just decl_or_value dv = decl; ? I think pointer_mux has
> ctors from pointers to the template parameter types.
>
>> gcc_checking_assert (dv_is_decl_p (dv));
>> return dv;
>> }
>> @@ -1406,8 +1405,7 @@ dv_from_decl (tree decl) static inline
>> decl_or_value dv_from_value (rtx value) {
>> - decl_or_value dv;
>> - dv = value;
>> + decl_or_value dv = decl_or_value::second (value);
>
> Ditto.
>
>> @@ -1661,7 +1659,8 @@ shared_hash_find_slot_unshare_1 (shared_hash
>> **pvars, decl_or_value dv, {
>> if (shared_hash_shared (*pvars))
>> *pvars = shared_hash_unshare (*pvars);
>> - return shared_hash_htab (*pvars)->find_slot_with_hash (dv, dvhash,
>> ins);
>> + return shared_hash_htab (*pvars)->find_slot_with_hash (dv_as_opaque (dv),
>> + dvhash, ins);
>
> Then you wouldn't need to change all these.
Also, please do try changing variable_hasher::compare_type to decl_or_value, and changing the type of the second parameter to variable_hasher::equal accordingly. I still feel that we should be able to get rid of dv_as_opaque entirely.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
` (2 preceding siblings ...)
2023-05-10 11:57 ` [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value pan2.li
@ 2023-05-10 15:23 ` pan2.li
2023-05-10 15:55 ` Richard Sandiford
2023-05-11 2:28 ` [PATCH v4] " pan2.li
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: pan2.li @ 2023-05-10 15:23 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
jakub, rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicition dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
gcc/ChangeLog:
* mux-utils.h: Add overload operator == and != for pointer_mux.
* var-tracking.cc: Included mux-utils.h for pointer_tmux.
(decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
(dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
(dv_as_decl): Ditto.
(dv_as_opaque): Removed due to unnecessary.
(struct variable_hasher): Take decl_or_value as compare_type.
(variable_hasher::equal): Diito.
(dv_from_decl): Reconciled to the new type, aka pointer_mux.
(dv_from_value): Ditto.
(attrs_list_member): Ditto.
(vars_copy): Ditto.
(var_reg_decl_set): Ditto.
(var_reg_delete_and_set): Ditto.
(find_loc_in_1pdv): Ditto.
(canonicalize_values_star): Ditto.
(variable_post_merge_new_vals): Ditto.
(dump_onepart_variable_differences): Ditto.
(variable_different_p): Ditto.
(variable_was_changed): Ditto.
(set_slot_part): Ditto.
(clobber_slot_part): Ditto.
(clobber_variable_part): Ditto.
(remove_value_from_changed_variables): Ditto.
(notify_dependents_of_changed_value): Ditto.
---
gcc/mux-utils.h | 12 ++++++
gcc/var-tracking.cc | 96 ++++++++++++++++++---------------------------
2 files changed, 51 insertions(+), 57 deletions(-)
diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
index a2b6a316899..adf3d3b722b 100644
--- a/gcc/mux-utils.h
+++ b/gcc/mux-utils.h
@@ -72,6 +72,18 @@ public:
// Return true unless the pointer is a null A pointer.
explicit operator bool () const { return m_ptr; }
+ // Return true if class has the same m_ptr, or false.
+ bool operator == (const pointer_mux &other) const
+ {
+ return this->m_ptr == other.m_ptr;
+ }
+
+ // Return true if class has the different m_ptr, or false.
+ bool operator != (const pointer_mux &other) const
+ {
+ return this->m_ptr != other.m_ptr;
+ }
+
// Assign A and B pointers respectively.
void set_first (T1 *ptr) { *this = first (ptr); }
void set_second (T2 *ptr) { *this = second (ptr); }
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..7a35f49020a 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,6 +116,7 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
@@ -197,14 +198,14 @@ struct micro_operation
/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +220,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.known_first ();
}
/* Return the value in the decl_or_value. */
@@ -227,14 +228,7 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
-}
-
-/* Return the opaque pointer in the decl_or_value. */
-static inline void *
-dv_as_opaque (decl_or_value dv)
-{
- return dv;
+ return dv.known_second ();
}
@@ -483,9 +477,9 @@ static void variable_htab_free (void *);
struct variable_hasher : pointer_hash <variable>
{
- typedef void *compare_type;
+ typedef decl_or_value compare_type;
static inline hashval_t hash (const variable *);
- static inline bool equal (const variable *, const void *);
+ static inline bool equal (const variable *, const decl_or_value);
static inline void remove (variable *);
};
@@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
/* Compare the declaration of variable X with declaration Y. */
inline bool
-variable_hasher::equal (const variable *v, const void *y)
+variable_hasher::equal (const variable *v, const decl_or_value y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return v->dv == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = decl;
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = value;
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1519,7 +1509,7 @@ static attrs *
attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
{
for (; list; list = list->next)
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
+ if (list->dv == dv && list->offset == offset)
return list;
return NULL;
}
@@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
for (node = set->regs[REGNO (loc)]; node; node = node->next)
- if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
- && node->offset == offset)
+ if (node->dv == dv && node->offset == offset)
break;
if (!node)
attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
@@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
for (node = *nextp; node; node = next)
{
next = node->next;
- if (dv_as_opaque (node->dv) != decl || node->offset != offset)
+ if (node->dv.first_or_null () != decl || node->offset != offset)
{
delete_variable_part (set, node->loc, node->dv, node->offset);
delete node;
@@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
if (!var->n_var_parts)
return NULL;
- gcc_checking_assert (loc != dv_as_opaque (var->dv));
+ gcc_checking_assert (loc != var->dv.second_or_null ());
loc_code = GET_CODE (loc);
for (node = var->var_part[0].loc_chain; node; node = node->next)
@@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
break;
list = list->next;
}
gcc_assert (list);
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
list->dv = cdv;
for (listp = &list->next; (list = *listp); listp = &list->next)
@@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ if (list->dv == cdv)
{
*listp = list->next;
delete list;
@@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
+ gcc_assert (list->dv != dv);
}
}
- else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ else if (list->dv == cdv)
{
for (listp = &list->next; (list = *listp); listp = &list->next)
{
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
*listp = list->next;
delete list;
@@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
+ gcc_assert (list->dv != cdv);
}
}
else
@@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (flag_checking)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
gcc_unreachable ();
list = list->next;
@@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
check_dupes = true;
break;
}
- else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
+ else if (att->dv == var->dv)
curp = attp;
}
@@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
while (*curp)
if ((*curp)->offset == 0
&& GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
- && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
+ && (*curp)->dv == var->dv)
break;
else
curp = &(*curp)->next;
@@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
gcc_assert (var1 != var2);
gcc_assert (dump_file);
- gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
+ gcc_assert (var1->dv == var2->dv);
gcc_assert (var1->n_var_parts == 1
&& var2->n_var_parts == 1);
@@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
if (var1->onepart && var1->n_var_parts)
{
- gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
- && var1->n_var_parts == 1);
+ gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
/* One-part values have locations in a canonical order. */
return onepart_variable_different_p (var1, var2);
}
@@ -7520,9 +7503,10 @@ variable_was_changed (variable *var, dataflow_set *set)
if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
{
- dslot = dropped_values->find_slot_with_hash (var->dv,
- dv_htab_hash (var->dv),
- INSERT);
+ dslot
+ = dropped_values->find_slot_with_hash (var->dv,
+ dv_htab_hash (var->dv),
+ INSERT);
empty_var = *dslot;
if (empty_var)
@@ -7656,7 +7640,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
onepart = dv_onepart_p (dv);
gcc_checking_assert (offset == 0 || !onepart);
- gcc_checking_assert (loc != dv_as_opaque (dv));
+ gcc_checking_assert (loc != dv.second_or_null ());
if (! flag_var_tracking_uninit)
initialized = VAR_INIT_STATUS_INITIALIZED;
@@ -7684,7 +7668,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
{
int r = -1, c = 0;
- gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
+ gcc_assert (var->dv == dv);
pos = 0;
@@ -7950,8 +7934,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
for (anode = *anextp; anode; anode = anext)
{
anext = anode->next;
- if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
- && anode->offset == offset)
+ if (anode->dv == var->dv && anode->offset == offset)
{
delete anode;
*anextp = anext;
@@ -7980,8 +7963,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
{
variable **slot;
- if (!dv_as_opaque (dv)
- || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
+ if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
return;
slot = shared_hash_find_slot_noinsert (set->vars, dv);
@@ -8960,7 +8942,7 @@ remove_value_from_changed_variables (rtx val)
variable *var;
slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ NO_INSERT);
var = *slot;
var->in_changed_variables = false;
changed_variables->clear_slot (slot);
@@ -8981,7 +8963,7 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
decl_or_value dv = dv_from_rtx (val);
slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
- NO_INSERT);
+ NO_INSERT);
if (!slot)
slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
if (!slot)
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 15:23 ` [PATCH v3] " pan2.li
@ 2023-05-10 15:55 ` Richard Sandiford
2023-05-11 2:30 ` Li, Pan2
0 siblings, 1 reply; 21+ messages in thread
From: Richard Sandiford @ 2023-05-10 15:55 UTC (permalink / raw)
To: pan2.li
Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw,
jakub, rguenther
Thanks, mostly looks good to me. Some minor comments below.
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to tree_node
> or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the location and bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicition dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
> Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
> * mux-utils.h: Add overload operator == and != for pointer_mux.
> * var-tracking.cc: Included mux-utils.h for pointer_tmux.
> (decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
> (dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
> (dv_as_decl): Ditto.
> (dv_as_opaque): Removed due to unnecessary.
> (struct variable_hasher): Take decl_or_value as compare_type.
> (variable_hasher::equal): Diito.
> (dv_from_decl): Reconciled to the new type, aka pointer_mux.
> (dv_from_value): Ditto.
> (attrs_list_member): Ditto.
> (vars_copy): Ditto.
> (var_reg_decl_set): Ditto.
> (var_reg_delete_and_set): Ditto.
> (find_loc_in_1pdv): Ditto.
> (canonicalize_values_star): Ditto.
> (variable_post_merge_new_vals): Ditto.
> (dump_onepart_variable_differences): Ditto.
> (variable_different_p): Ditto.
> (variable_was_changed): Ditto.
> (set_slot_part): Ditto.
> (clobber_slot_part): Ditto.
> (clobber_variable_part): Ditto.
> (remove_value_from_changed_variables): Ditto.
> (notify_dependents_of_changed_value): Ditto.
> ---
> gcc/mux-utils.h | 12 ++++++
> gcc/var-tracking.cc | 96 ++++++++++++++++++---------------------------
> 2 files changed, 51 insertions(+), 57 deletions(-)
>
> diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
> index a2b6a316899..adf3d3b722b 100644
> --- a/gcc/mux-utils.h
> +++ b/gcc/mux-utils.h
> @@ -72,6 +72,18 @@ public:
> // Return true unless the pointer is a null A pointer.
> explicit operator bool () const { return m_ptr; }
>
> + // Return true if class has the same m_ptr, or false.
> + bool operator == (const pointer_mux &other) const
> + {
> + return this->m_ptr == other.m_ptr;
> + }
> +
> + // Return true if class has the different m_ptr, or false.
> + bool operator != (const pointer_mux &other) const
> + {
> + return this->m_ptr != other.m_ptr;
> + }
> +
The current code tries to follow the coding standard rule that functions
should be defined outside the class if the whole thing doesn't fit on
one line. Admittedly that's not widely followed, but we might as well
continue to stick to it here.
The comment shouldn't talk about m_ptr, since that's an internal
implementation detail rather than a user-facing thing. I think it's
OK to leave the functions uncommented, since it's obvious what ==
and != do.
> // Assign A and B pointers respectively.
> void set_first (T1 *ptr) { *this = first (ptr); }
> void set_second (T2 *ptr) { *this = second (ptr); }
> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
> index fae0c73e02f..7a35f49020a 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,6 +116,7 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> @@ -197,14 +198,14 @@ struct micro_operation
>
>
> /* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>
> /* Return true if a decl_or_value DV is a DECL or NULL. */
> static inline bool
> dv_is_decl_p (decl_or_value dv)
> {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + return dv.is_first ();
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */
> @@ -219,7 +220,7 @@ static inline tree
> dv_as_decl (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.known_first ();
> }
>
> /* Return the value in the decl_or_value. */
> @@ -227,14 +228,7 @@ static inline rtx
> dv_as_value (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> -}
> -
> -/* Return the opaque pointer in the decl_or_value. */
> -static inline void *
> -dv_as_opaque (decl_or_value dv)
> -{
> - return dv;
> + return dv.known_second ();
> }
>
>
> @@ -483,9 +477,9 @@ static void variable_htab_free (void *);
>
> struct variable_hasher : pointer_hash <variable>
> {
> - typedef void *compare_type;
> + typedef decl_or_value compare_type;
> static inline hashval_t hash (const variable *);
> - static inline bool equal (const variable *, const void *);
> + static inline bool equal (const variable *, const decl_or_value);
> static inline void remove (variable *);
> };
>
> @@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
> /* Compare the declaration of variable X with declaration Y. */
>
> inline bool
> -variable_hasher::equal (const variable *v, const void *y)
> +variable_hasher::equal (const variable *v, const decl_or_value y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return v->dv == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
> @@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
> static inline decl_or_value
> dv_from_decl (tree decl)
> {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl;
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
> static inline decl_or_value
> dv_from_value (rtx value)
> {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = value;
> gcc_checking_assert (dv_is_value_p (dv));
> return dv;
> }
> @@ -1519,7 +1509,7 @@ static attrs *
> attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
> {
> for (; list; list = list->next)
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
> + if (list->dv == dv && list->offset == offset)
> return list;
> return NULL;
> }
> @@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
> dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
>
> for (node = set->regs[REGNO (loc)]; node; node = node->next)
> - if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
> - && node->offset == offset)
> + if (node->dv == dv && node->offset == offset)
> break;
> if (!node)
> attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
> @@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
> for (node = *nextp; node; node = next)
> {
> next = node->next;
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv.first_or_null () != decl || node->offset != offset)
Genuine question, but: is the first_or_null really needed? I would have
expected node->dv != decl to work, with an implicit conversion on the
argument.
> {
> delete_variable_part (set, node->loc, node->dv, node->offset);
> delete node;
> @@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> if (!var->n_var_parts)
> return NULL;
>
> - gcc_checking_assert (loc != dv_as_opaque (var->dv));
> + gcc_checking_assert (loc != var->dv.second_or_null ());
Similarly here, I would expect var->dv != loc to work.
> loc_code = GET_CODE (loc);
> for (node = var->var_part[0].loc_chain; node; node = node->next)
> @@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
>
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> break;
>
> list = list->next;
> }
>
> gcc_assert (list);
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> list->dv = cdv;
> for (listp = &list->next; (list = *listp); listp = &list->next)
> @@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + if (list->dv == cdv)
> {
> *listp = list->next;
> delete list;
> @@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
> + gcc_assert (list->dv != dv);
> }
> }
> - else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + else if (list->dv == cdv)
> {
> for (listp = &list->next; (list = *listp); listp = &list->next)
> {
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> *listp = list->next;
> delete list;
> @@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
> + gcc_assert (list->dv != cdv);
> }
> }
> else
> @@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (flag_checking)
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> gcc_unreachable ();
>
> list = list->next;
> @@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> check_dupes = true;
> break;
> }
> - else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
> + else if (att->dv == var->dv)
> curp = attp;
> }
>
> @@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> while (*curp)
> if ((*curp)->offset == 0
> && GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
> - && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
> + && (*curp)->dv == var->dv)
> break;
> else
> curp = &(*curp)->next;
> @@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
>
> gcc_assert (var1 != var2);
> gcc_assert (dump_file);
> - gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
> + gcc_assert (var1->dv == var2->dv);
> gcc_assert (var1->n_var_parts == 1
> && var2->n_var_parts == 1);
>
> @@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
>
> if (var1->onepart && var1->n_var_parts)
> {
> - gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
> - && var1->n_var_parts == 1);
> + gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
> /* One-part values have locations in a canonical order. */
> return onepart_variable_different_p (var1, var2);
> }
> @@ -7520,9 +7503,10 @@ variable_was_changed (variable *var, dataflow_set *set)
>
> if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
> {
> - dslot = dropped_values->find_slot_with_hash (var->dv,
> - dv_htab_hash (var->dv),
> - INSERT);
> + dslot
> + = dropped_values->find_slot_with_hash (var->dv,
> + dv_htab_hash (var->dv),
> + INSERT);
This is now a no-op change, so probably best to leave it out.
> empty_var = *dslot;
>
> if (empty_var)
> @@ -7656,7 +7640,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> onepart = dv_onepart_p (dv);
>
> gcc_checking_assert (offset == 0 || !onepart);
> - gcc_checking_assert (loc != dv_as_opaque (dv));
> + gcc_checking_assert (loc != dv.second_or_null ());
dv != loc here too.
>
> if (! flag_var_tracking_uninit)
> initialized = VAR_INIT_STATUS_INITIALIZED;
> @@ -7684,7 +7668,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> {
> int r = -1, c = 0;
>
> - gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
> + gcc_assert (var->dv == dv);
>
> pos = 0;
>
> @@ -7950,8 +7934,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
> for (anode = *anextp; anode; anode = anext)
> {
> anext = anode->next;
> - if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
> - && anode->offset == offset)
> + if (anode->dv == var->dv && anode->offset == offset)
> {
> delete anode;
> *anextp = anext;
> @@ -7980,8 +7963,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
> {
> variable **slot;
>
> - if (!dv_as_opaque (dv)
> - || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> + if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> return;
>
> slot = shared_hash_find_slot_noinsert (set->vars, dv);
> @@ -8960,7 +8942,7 @@ remove_value_from_changed_variables (rtx val)
> variable *var;
>
> slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + NO_INSERT);
> var = *slot;
> var->in_changed_variables = false;
> changed_variables->clear_slot (slot);
> @@ -8981,7 +8963,7 @@ notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
> decl_or_value dv = dv_from_rtx (val);
>
> slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + NO_INSERT);
> if (!slot)
> slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
> if (!slot)
The last two changes are no-ops.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
` (3 preceding siblings ...)
2023-05-10 15:23 ` [PATCH v3] " pan2.li
@ 2023-05-11 2:28 ` pan2.li
2023-05-11 6:21 ` [PATCH v5] " pan2.li
2023-05-11 12:54 ` [PATCH v6] " pan2.li
6 siblings, 0 replies; 21+ messages in thread
From: pan2.li @ 2023-05-11 2:28 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
jakub, rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicit dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
gcc/ChangeLog:
* mux-utils.h: Add overload operator == and != for pointer_mux.
* var-tracking.cc: Included mux-utils.h for pointer_tmux.
(decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
(dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
(dv_as_decl): Ditto.
(dv_as_opaque): Removed due to unnecessary.
(struct variable_hasher): Take decl_or_value as compare_type.
(variable_hasher::equal): Diito.
(dv_from_decl): Reconciled to the new type, aka pointer_mux.
(dv_from_value): Ditto.
(attrs_list_member): Ditto.
(vars_copy): Ditto.
(var_reg_decl_set): Ditto.
(var_reg_delete_and_set): Ditto.
(find_loc_in_1pdv): Ditto.
(canonicalize_values_star): Ditto.
(variable_post_merge_new_vals): Ditto.
(dump_onepart_variable_differences): Ditto.
(variable_different_p): Ditto.
(set_slot_part): Ditto.
(clobber_slot_part): Ditto.
(clobber_variable_part): Ditto.
---
gcc/mux-utils.h | 4 +++
gcc/var-tracking.cc | 85 ++++++++++++++++++---------------------------
2 files changed, 37 insertions(+), 52 deletions(-)
diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
index a2b6a316899..486d80915b1 100644
--- a/gcc/mux-utils.h
+++ b/gcc/mux-utils.h
@@ -117,6 +117,10 @@ public:
// ...use ptr.known_second ()...
T2 *second_or_null () const;
+ bool operator == (const pointer_mux &pm) const { return m_ptr == pm.m_ptr; }
+
+ bool operator != (const pointer_mux &pm) const { return m_ptr != pm.m_ptr; }
+
// Return true if the pointer is a T.
//
// This is only valid if T1 and T2 are distinct and if T can be
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..1f7ec0e34ca 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,6 +116,7 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
@@ -197,14 +198,14 @@ struct micro_operation
/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +220,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.known_first ();
}
/* Return the value in the decl_or_value. */
@@ -227,14 +228,7 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
-}
-
-/* Return the opaque pointer in the decl_or_value. */
-static inline void *
-dv_as_opaque (decl_or_value dv)
-{
- return dv;
+ return dv.known_second ();
}
@@ -483,9 +477,9 @@ static void variable_htab_free (void *);
struct variable_hasher : pointer_hash <variable>
{
- typedef void *compare_type;
+ typedef decl_or_value compare_type;
static inline hashval_t hash (const variable *);
- static inline bool equal (const variable *, const void *);
+ static inline bool equal (const variable *, const decl_or_value);
static inline void remove (variable *);
};
@@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
/* Compare the declaration of variable X with declaration Y. */
inline bool
-variable_hasher::equal (const variable *v, const void *y)
+variable_hasher::equal (const variable *v, const decl_or_value y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return v->dv == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = decl;
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = value;
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1519,7 +1509,7 @@ static attrs *
attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
{
for (; list; list = list->next)
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
+ if (list->dv == dv && list->offset == offset)
return list;
return NULL;
}
@@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
for (node = set->regs[REGNO (loc)]; node; node = node->next)
- if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
- && node->offset == offset)
+ if (node->dv == dv && node->offset == offset)
break;
if (!node)
attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
@@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
for (node = *nextp; node; node = next)
{
next = node->next;
- if (dv_as_opaque (node->dv) != decl || node->offset != offset)
+ if (node->dv != decl || node->offset != offset)
{
delete_variable_part (set, node->loc, node->dv, node->offset);
delete node;
@@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
if (!var->n_var_parts)
return NULL;
- gcc_checking_assert (loc != dv_as_opaque (var->dv));
+ gcc_checking_assert ((decl_or_value) loc != var->dv);
loc_code = GET_CODE (loc);
for (node = var->var_part[0].loc_chain; node; node = node->next)
@@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
break;
list = list->next;
}
gcc_assert (list);
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
list->dv = cdv;
for (listp = &list->next; (list = *listp); listp = &list->next)
@@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ if (list->dv == cdv)
{
*listp = list->next;
delete list;
@@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
+ gcc_assert (list->dv != dv);
}
}
- else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ else if (list->dv == cdv)
{
for (listp = &list->next; (list = *listp); listp = &list->next)
{
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
*listp = list->next;
delete list;
@@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
+ gcc_assert (list->dv != cdv);
}
}
else
@@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (flag_checking)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
gcc_unreachable ();
list = list->next;
@@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
check_dupes = true;
break;
}
- else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
+ else if (att->dv == var->dv)
curp = attp;
}
@@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
while (*curp)
if ((*curp)->offset == 0
&& GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
- && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
+ && (*curp)->dv == var->dv)
break;
else
curp = &(*curp)->next;
@@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
gcc_assert (var1 != var2);
gcc_assert (dump_file);
- gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
+ gcc_assert (var1->dv == var2->dv);
gcc_assert (var1->n_var_parts == 1
&& var2->n_var_parts == 1);
@@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
if (var1->onepart && var1->n_var_parts)
{
- gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
- && var1->n_var_parts == 1);
+ gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
/* One-part values have locations in a canonical order. */
return onepart_variable_different_p (var1, var2);
}
@@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
onepart = dv_onepart_p (dv);
gcc_checking_assert (offset == 0 || !onepart);
- gcc_checking_assert (loc != dv_as_opaque (dv));
+ gcc_checking_assert ((decl_or_value) loc != dv);
if (! flag_var_tracking_uninit)
initialized = VAR_INIT_STATUS_INITIALIZED;
@@ -7684,7 +7667,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
{
int r = -1, c = 0;
- gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
+ gcc_assert (var->dv == dv);
pos = 0;
@@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
for (anode = *anextp; anode; anode = anext)
{
anext = anode->next;
- if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
- && anode->offset == offset)
+ if (anode->dv == var->dv && anode->offset == offset)
{
delete anode;
*anextp = anext;
@@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
{
variable **slot;
- if (!dv_as_opaque (dv)
- || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
+ if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
return;
slot = shared_hash_find_slot_noinsert (set->vars, dv);
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 15:55 ` Richard Sandiford
@ 2023-05-11 2:30 ` Li, Pan2
2023-05-11 4:43 ` Richard Sandiford
2023-05-11 6:31 ` Bernhard Reutner-Fischer
0 siblings, 2 replies; 21+ messages in thread
From: Li, Pan2 @ 2023-05-11 2:30 UTC (permalink / raw)
To: Richard Sandiford
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
Thanks Richard Sandiford. Update PATCH v4 here -> https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618099.html.
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv.first_or_null () != decl || node->offset !=
> + offset)
> Genuine question, but: is the first_or_null really needed? I would have expected node->dv != decl to work, with an implicit conversion on the argument.
Directly compare node->dv and decl may requires additional overload operator, or it may complains similar as below. But I am afraid it is unreasonable to add such kind of operator for one specific type RTX in pointer_mux up to a point. Thus I think here we may need node->dv == (decl_or_val) decl here.
../../gcc/var-tracking.cc:3233:28: error: no match for 'operator!=' (operand types are 'rtx' {aka 'rtx_def*'} and 'decl_or_value' {aka 'pointer_mux<tree_node, rtx_def>'}).
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Wednesday, May 10, 2023 11:56 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; jakub@redhat.com; rguenther@suse.de
Subject: Re: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
Thanks, mostly looks good to me. Some minor comments below.
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to
> tree_node or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the location and bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicition dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
> Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
> * mux-utils.h: Add overload operator == and != for pointer_mux.
> * var-tracking.cc: Included mux-utils.h for pointer_tmux.
> (decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
> (dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
> (dv_as_decl): Ditto.
> (dv_as_opaque): Removed due to unnecessary.
> (struct variable_hasher): Take decl_or_value as compare_type.
> (variable_hasher::equal): Diito.
> (dv_from_decl): Reconciled to the new type, aka pointer_mux.
> (dv_from_value): Ditto.
> (attrs_list_member): Ditto.
> (vars_copy): Ditto.
> (var_reg_decl_set): Ditto.
> (var_reg_delete_and_set): Ditto.
> (find_loc_in_1pdv): Ditto.
> (canonicalize_values_star): Ditto.
> (variable_post_merge_new_vals): Ditto.
> (dump_onepart_variable_differences): Ditto.
> (variable_different_p): Ditto.
> (variable_was_changed): Ditto.
> (set_slot_part): Ditto.
> (clobber_slot_part): Ditto.
> (clobber_variable_part): Ditto.
> (remove_value_from_changed_variables): Ditto.
> (notify_dependents_of_changed_value): Ditto.
> ---
> gcc/mux-utils.h | 12 ++++++
> gcc/var-tracking.cc | 96
> ++++++++++++++++++---------------------------
> 2 files changed, 51 insertions(+), 57 deletions(-)
>
> diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h index
> a2b6a316899..adf3d3b722b 100644
> --- a/gcc/mux-utils.h
> +++ b/gcc/mux-utils.h
> @@ -72,6 +72,18 @@ public:
> // Return true unless the pointer is a null A pointer.
> explicit operator bool () const { return m_ptr; }
>
> + // Return true if class has the same m_ptr, or false.
> + bool operator == (const pointer_mux &other) const
> + {
> + return this->m_ptr == other.m_ptr;
> + }
> +
> + // Return true if class has the different m_ptr, or false.
> + bool operator != (const pointer_mux &other) const
> + {
> + return this->m_ptr != other.m_ptr;
> + }
> +
The current code tries to follow the coding standard rule that functions should be defined outside the class if the whole thing doesn't fit on one line. Admittedly that's not widely followed, but we might as well continue to stick to it here.
The comment shouldn't talk about m_ptr, since that's an internal implementation detail rather than a user-facing thing. I think it's OK to leave the functions uncommented, since it's obvious what == and != do.
> // Assign A and B pointers respectively.
> void set_first (T1 *ptr) { *this = first (ptr); }
> void set_second (T2 *ptr) { *this = second (ptr); } diff --git
> a/gcc/var-tracking.cc b/gcc/var-tracking.cc index
> fae0c73e02f..7a35f49020a 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,6 +116,7 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> @@ -197,14 +198,14 @@ struct micro_operation
>
>
> /* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>
> /* Return true if a decl_or_value DV is a DECL or NULL. */ static
> inline bool dv_is_decl_p (decl_or_value dv) {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + return dv.is_first ();
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */ @@ -219,7
> +220,7 @@ static inline tree dv_as_decl (decl_or_value dv) {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.known_first ();
> }
>
> /* Return the value in the decl_or_value. */ @@ -227,14 +228,7 @@
> static inline rtx dv_as_value (decl_or_value dv) {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> -}
> -
> -/* Return the opaque pointer in the decl_or_value. */ -static inline
> void * -dv_as_opaque (decl_or_value dv) -{
> - return dv;
> + return dv.known_second ();
> }
>
>
> @@ -483,9 +477,9 @@ static void variable_htab_free (void *);
>
> struct variable_hasher : pointer_hash <variable> {
> - typedef void *compare_type;
> + typedef decl_or_value compare_type;
> static inline hashval_t hash (const variable *);
> - static inline bool equal (const variable *, const void *);
> + static inline bool equal (const variable *, const decl_or_value);
> static inline void remove (variable *); };
>
> @@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
> /* Compare the declaration of variable X with declaration Y. */
>
> inline bool
> -variable_hasher::equal (const variable *v, const void *y)
> +variable_hasher::equal (const variable *v, const decl_or_value y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return v->dv == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct
> variable_def). */ @@ -1396,8 +1388,7 @@ onepart_pool_allocate
> (onepart_enum onepart) static inline decl_or_value dv_from_decl
> (tree decl) {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl;
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1397,7 @@ dv_from_decl (tree decl) static inline
> decl_or_value dv_from_value (rtx value) {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = value;
> gcc_checking_assert (dv_is_value_p (dv));
> return dv;
> }
> @@ -1519,7 +1509,7 @@ static attrs *
> attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT
> offset) {
> for (; list; list = list->next)
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
> + if (list->dv == dv && list->offset == offset)
> return list;
> return NULL;
> }
> @@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash
> + (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
> dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
>
> for (node = set->regs[REGNO (loc)]; node; node = node->next)
> - if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
> - && node->offset == offset)
> + if (node->dv == dv && node->offset == offset)
> break;
> if (!node)
> attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc); @@
> -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
> for (node = *nextp; node; node = next)
> {
> next = node->next;
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv.first_or_null () != decl || node->offset !=
> + offset)
Genuine question, but: is the first_or_null really needed? I would have expected node->dv != decl to work, with an implicit conversion on the argument.
> {
> delete_variable_part (set, node->loc, node->dv, node->offset);
> delete node;
> @@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> if (!var->n_var_parts)
> return NULL;
>
> - gcc_checking_assert (loc != dv_as_opaque (var->dv));
> + gcc_checking_assert (loc != var->dv.second_or_null ());
Similarly here, I would expect var->dv != loc to work.
> loc_code = GET_CODE (loc);
> for (node = var->var_part[0].loc_chain; node; node = node->next) @@
> -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot,
> dataflow_set *set)
>
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> break;
>
> list = list->next;
> }
>
> gcc_assert (list);
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> list->dv = cdv;
> for (listp = &list->next; (list = *listp); listp = &list->next) @@
> -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + if (list->dv == cdv)
> {
> *listp = list->next;
> delete list;
> @@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
> + gcc_assert (list->dv != dv);
> }
> }
> - else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + else if (list->dv == cdv)
> {
> for (listp = &list->next; (list = *listp); listp = &list->next)
> {
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> *listp = list->next;
> delete list;
> @@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
> + gcc_assert (list->dv != cdv);
> }
> }
> else
> @@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (flag_checking)
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> gcc_unreachable ();
>
> list = list->next;
> @@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> check_dupes = true;
> break;
> }
> - else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
> + else if (att->dv == var->dv)
> curp = attp;
> }
>
> @@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> while (*curp)
> if ((*curp)->offset == 0
> && GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
> - && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
> + && (*curp)->dv == var->dv)
> break;
> else
> curp = &(*curp)->next;
> @@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable
> *var1, variable *var2)
>
> gcc_assert (var1 != var2);
> gcc_assert (dump_file);
> - gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
> + gcc_assert (var1->dv == var2->dv);
> gcc_assert (var1->n_var_parts == 1
> && var2->n_var_parts == 1);
>
> @@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable
> *var2)
>
> if (var1->onepart && var1->n_var_parts)
> {
> - gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
> - && var1->n_var_parts == 1);
> + gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts
> + == 1);
> /* One-part values have locations in a canonical order. */
> return onepart_variable_different_p (var1, var2);
> }
> @@ -7520,9 +7503,10 @@ variable_was_changed (variable *var,
> dataflow_set *set)
>
> if (onepart == ONEPART_VALUE || onepart == ONEPART_DEXPR)
> {
> - dslot = dropped_values->find_slot_with_hash (var->dv,
> - dv_htab_hash (var->dv),
> - INSERT);
> + dslot
> + = dropped_values->find_slot_with_hash (var->dv,
> + dv_htab_hash (var->dv),
> + INSERT);
This is now a no-op change, so probably best to leave it out.
> empty_var = *dslot;
>
> if (empty_var)
> @@ -7656,7 +7640,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> onepart = dv_onepart_p (dv);
>
> gcc_checking_assert (offset == 0 || !onepart);
> - gcc_checking_assert (loc != dv_as_opaque (dv));
> + gcc_checking_assert (loc != dv.second_or_null ());
dv != loc here too.
>
> if (! flag_var_tracking_uninit)
> initialized = VAR_INIT_STATUS_INITIALIZED; @@ -7684,7 +7668,7 @@
> set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> {
> int r = -1, c = 0;
>
> - gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
> + gcc_assert (var->dv == dv);
>
> pos = 0;
>
> @@ -7950,8 +7934,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
> for (anode = *anextp; anode; anode = anext)
> {
> anext = anode->next;
> - if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
> - && anode->offset == offset)
> + if (anode->dv == var->dv && anode->offset == offset)
> {
> delete anode;
> *anextp = anext;
> @@ -7980,8 +7963,7 @@ clobber_variable_part (dataflow_set *set, rtx
> loc, decl_or_value dv, {
> variable **slot;
>
> - if (!dv_as_opaque (dv)
> - || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> + if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> return;
>
> slot = shared_hash_find_slot_noinsert (set->vars, dv); @@ -8960,7
> +8942,7 @@ remove_value_from_changed_variables (rtx val)
> variable *var;
>
> slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + NO_INSERT);
> var = *slot;
> var->in_changed_variables = false;
> changed_variables->clear_slot (slot); @@ -8981,7 +8963,7 @@
> notify_dependents_of_changed_value (rtx val, variable_table_type *htab,
> decl_or_value dv = dv_from_rtx (val);
>
> slot = changed_variables->find_slot_with_hash (dv, dv_htab_hash (dv),
> - NO_INSERT);
> + NO_INSERT);
> if (!slot)
> slot = htab->find_slot_with_hash (dv, dv_htab_hash (dv), NO_INSERT);
> if (!slot)
The last two changes are no-ops.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 2:30 ` Li, Pan2
@ 2023-05-11 4:43 ` Richard Sandiford
2023-05-11 6:30 ` Li, Pan2
2023-05-11 6:31 ` Bernhard Reutner-Fischer
1 sibling, 1 reply; 21+ messages in thread
From: Richard Sandiford @ 2023-05-11 4:43 UTC (permalink / raw)
To: Li, Pan2
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
"Li, Pan2" <pan2.li@intel.com> writes:
> Thanks Richard Sandiford. Update PATCH v4 here -> https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618099.html.
>
>> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
>> + if (node->dv.first_or_null () != decl || node->offset !=
>> + offset)
>
>> Genuine question, but: is the first_or_null really needed? I would have expected node->dv != decl to work, with an implicit conversion on the argument.
>
> Directly compare node->dv and decl may requires additional overload operator, or it may complains similar as below. But I am afraid it is unreasonable to add such kind of operator for one specific type RTX in pointer_mux up to a point. Thus I think here we may need node->dv == (decl_or_val) decl here.
>
> ../../gcc/var-tracking.cc:3233:28: error: no match for 'operator!=' (operand types are 'rtx' {aka 'rtx_def*'} and 'decl_or_value' {aka 'pointer_mux<tree_node, rtx_def>'}).
Yeah, since we're adding operator== and operator!= as member operators,
the decl_or_value has to come first. Please try the conditions in the
order that I'd written them in the review.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
` (4 preceding siblings ...)
2023-05-11 2:28 ` [PATCH v4] " pan2.li
@ 2023-05-11 6:21 ` pan2.li
2023-05-11 7:16 ` Richard Sandiford
2023-05-11 12:54 ` [PATCH v6] " pan2.li
6 siblings, 1 reply; 21+ messages in thread
From: pan2.li @ 2023-05-11 6:21 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
jakub, rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the same location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicit dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
gcc/ChangeLog:
* mux-utils.h: Add overload operator == and != for pointer_mux.
* var-tracking.cc: Included mux-utils.h for pointer_tmux.
(decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
(dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
(dv_as_decl): Ditto.
(dv_as_opaque): Removed due to unnecessary.
(struct variable_hasher): Take decl_or_value as compare_type.
(variable_hasher::equal): Diito.
(dv_from_decl): Reconciled to the new type, aka pointer_mux.
(dv_from_value): Ditto.
(attrs_list_member): Ditto.
(vars_copy): Ditto.
(var_reg_decl_set): Ditto.
(var_reg_delete_and_set): Ditto.
(find_loc_in_1pdv): Ditto.
(canonicalize_values_star): Ditto.
(variable_post_merge_new_vals): Ditto.
(dump_onepart_variable_differences): Ditto.
(variable_different_p): Ditto.
(set_slot_part): Ditto.
(clobber_slot_part): Ditto.
(clobber_variable_part): Ditto.
---
gcc/mux-utils.h | 4 +++
gcc/var-tracking.cc | 85 ++++++++++++++++++---------------------------
2 files changed, 37 insertions(+), 52 deletions(-)
diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
index a2b6a316899..486d80915b1 100644
--- a/gcc/mux-utils.h
+++ b/gcc/mux-utils.h
@@ -117,6 +117,10 @@ public:
// ...use ptr.known_second ()...
T2 *second_or_null () const;
+ bool operator == (const pointer_mux &pm) const { return m_ptr == pm.m_ptr; }
+
+ bool operator != (const pointer_mux &pm) const { return m_ptr != pm.m_ptr; }
+
// Return true if the pointer is a T.
//
// This is only valid if T1 and T2 are distinct and if T can be
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..384084c8b3e 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,6 +116,7 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
@@ -197,14 +198,14 @@ struct micro_operation
/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +220,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.known_first ();
}
/* Return the value in the decl_or_value. */
@@ -227,14 +228,7 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
-}
-
-/* Return the opaque pointer in the decl_or_value. */
-static inline void *
-dv_as_opaque (decl_or_value dv)
-{
- return dv;
+ return dv.known_second ();
}
@@ -483,9 +477,9 @@ static void variable_htab_free (void *);
struct variable_hasher : pointer_hash <variable>
{
- typedef void *compare_type;
+ typedef decl_or_value compare_type;
static inline hashval_t hash (const variable *);
- static inline bool equal (const variable *, const void *);
+ static inline bool equal (const variable *, const decl_or_value);
static inline void remove (variable *);
};
@@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
/* Compare the declaration of variable X with declaration Y. */
inline bool
-variable_hasher::equal (const variable *v, const void *y)
+variable_hasher::equal (const variable *v, const decl_or_value y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return v->dv == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = decl;
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = value;
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1519,7 +1509,7 @@ static attrs *
attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
{
for (; list; list = list->next)
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
+ if (list->dv == dv && list->offset == offset)
return list;
return NULL;
}
@@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
for (node = set->regs[REGNO (loc)]; node; node = node->next)
- if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
- && node->offset == offset)
+ if (node->dv == dv && node->offset == offset)
break;
if (!node)
attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
@@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
for (node = *nextp; node; node = next)
{
next = node->next;
- if (dv_as_opaque (node->dv) != decl || node->offset != offset)
+ if (node->dv != decl || node->offset != offset)
{
delete_variable_part (set, node->loc, node->dv, node->offset);
delete node;
@@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
if (!var->n_var_parts)
return NULL;
- gcc_checking_assert (loc != dv_as_opaque (var->dv));
+ gcc_checking_assert (var->dv == loc);
loc_code = GET_CODE (loc);
for (node = var->var_part[0].loc_chain; node; node = node->next)
@@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
break;
list = list->next;
}
gcc_assert (list);
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
list->dv = cdv;
for (listp = &list->next; (list = *listp); listp = &list->next)
@@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ if (list->dv == cdv)
{
*listp = list->next;
delete list;
@@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
+ gcc_assert (list->dv != dv);
}
}
- else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ else if (list->dv == cdv)
{
for (listp = &list->next; (list = *listp); listp = &list->next)
{
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
*listp = list->next;
delete list;
@@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
+ gcc_assert (list->dv != cdv);
}
}
else
@@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (flag_checking)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
gcc_unreachable ();
list = list->next;
@@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
check_dupes = true;
break;
}
- else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
+ else if (att->dv == var->dv)
curp = attp;
}
@@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
while (*curp)
if ((*curp)->offset == 0
&& GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
- && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
+ && (*curp)->dv == var->dv)
break;
else
curp = &(*curp)->next;
@@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
gcc_assert (var1 != var2);
gcc_assert (dump_file);
- gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
+ gcc_assert (var1->dv == var2->dv);
gcc_assert (var1->n_var_parts == 1
&& var2->n_var_parts == 1);
@@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
if (var1->onepart && var1->n_var_parts)
{
- gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
- && var1->n_var_parts == 1);
+ gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
/* One-part values have locations in a canonical order. */
return onepart_variable_different_p (var1, var2);
}
@@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
onepart = dv_onepart_p (dv);
gcc_checking_assert (offset == 0 || !onepart);
- gcc_checking_assert (loc != dv_as_opaque (dv));
+ gcc_checking_assert (dv != loc);
if (! flag_var_tracking_uninit)
initialized = VAR_INIT_STATUS_INITIALIZED;
@@ -7684,7 +7667,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
{
int r = -1, c = 0;
- gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
+ gcc_assert (var->dv == dv);
pos = 0;
@@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
for (anode = *anextp; anode; anode = anext)
{
anext = anode->next;
- if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
- && anode->offset == offset)
+ if (anode->dv == var->dv && anode->offset == offset)
{
delete anode;
*anextp = anext;
@@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
{
variable **slot;
- if (!dv_as_opaque (dv)
- || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
+ if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
return;
slot = shared_hash_find_slot_noinsert (set->vars, dv);
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 4:43 ` Richard Sandiford
@ 2023-05-11 6:30 ` Li, Pan2
0 siblings, 0 replies; 21+ messages in thread
From: Li, Pan2 @ 2023-05-11 6:30 UTC (permalink / raw)
To: Richard Sandiford
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
Yes, you are right. The decl_or_value take first works well, missed this detail in previous and updated the PATCH v5 for this. Thank you!
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Thursday, May 11, 2023 12:43 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; jakub@redhat.com; rguenther@suse.de
Subject: Re: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
"Li, Pan2" <pan2.li@intel.com> writes:
> Thanks Richard Sandiford. Update PATCH v4 here -> https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618099.html.
>
>> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
>> + if (node->dv.first_or_null () != decl || node->offset !=
>> + offset)
>
>> Genuine question, but: is the first_or_null really needed? I would have expected node->dv != decl to work, with an implicit conversion on the argument.
>
> Directly compare node->dv and decl may requires additional overload operator, or it may complains similar as below. But I am afraid it is unreasonable to add such kind of operator for one specific type RTX in pointer_mux up to a point. Thus I think here we may need node->dv == (decl_or_val) decl here.
>
> ../../gcc/var-tracking.cc:3233:28: error: no match for 'operator!=' (operand types are 'rtx' {aka 'rtx_def*'} and 'decl_or_value' {aka 'pointer_mux<tree_node, rtx_def>'}).
Yeah, since we're adding operator== and operator!= as member operators, the decl_or_value has to come first. Please try the conditions in the order that I'd written them in the review.
Thanks,
Richard
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v3] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 2:30 ` Li, Pan2
2023-05-11 4:43 ` Richard Sandiford
@ 2023-05-11 6:31 ` Bernhard Reutner-Fischer
1 sibling, 0 replies; 21+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-05-11 6:31 UTC (permalink / raw)
To: Li, Pan2, Li, Pan2 via Gcc-patches, Richard Sandiford
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
On 11 May 2023 04:30:16 CEST, "Li, Pan2 via Gcc-patches" <gcc-patches@gcc.gnu.org> wrote:
>../../gcc/var-tracking.cc:3233:28: error: no match for 'operator!=' (operand types are 'rtx' {aka 'rtx_def*'} and 'decl_or_value' {aka 'pointer_mux<tree_node, rtx_def>'}).
Wouldn't you usually declare operator!= by !(left == right) ?
thanks,
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 6:21 ` [PATCH v5] " pan2.li
@ 2023-05-11 7:16 ` Richard Sandiford
2023-05-11 10:55 ` Li, Pan2
0 siblings, 1 reply; 21+ messages in thread
From: Richard Sandiford @ 2023-05-11 7:16 UTC (permalink / raw)
To: pan2.li
Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw,
jakub, rguenther
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to tree_node
> or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the same location and bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicit dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
> Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
> * mux-utils.h: Add overload operator == and != for pointer_mux.
> * var-tracking.cc: Included mux-utils.h for pointer_tmux.
> (decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
> (dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
> (dv_as_decl): Ditto.
> (dv_as_opaque): Removed due to unnecessary.
> (struct variable_hasher): Take decl_or_value as compare_type.
> (variable_hasher::equal): Diito.
> (dv_from_decl): Reconciled to the new type, aka pointer_mux.
> (dv_from_value): Ditto.
> (attrs_list_member): Ditto.
> (vars_copy): Ditto.
> (var_reg_decl_set): Ditto.
> (var_reg_delete_and_set): Ditto.
> (find_loc_in_1pdv): Ditto.
> (canonicalize_values_star): Ditto.
> (variable_post_merge_new_vals): Ditto.
> (dump_onepart_variable_differences): Ditto.
> (variable_different_p): Ditto.
> (set_slot_part): Ditto.
> (clobber_slot_part): Ditto.
> (clobber_variable_part): Ditto.
OK, thanks!
Richard
> ---
> gcc/mux-utils.h | 4 +++
> gcc/var-tracking.cc | 85 ++++++++++++++++++---------------------------
> 2 files changed, 37 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
> index a2b6a316899..486d80915b1 100644
> --- a/gcc/mux-utils.h
> +++ b/gcc/mux-utils.h
> @@ -117,6 +117,10 @@ public:
> // ...use ptr.known_second ()...
> T2 *second_or_null () const;
>
> + bool operator == (const pointer_mux &pm) const { return m_ptr == pm.m_ptr; }
> +
> + bool operator != (const pointer_mux &pm) const { return m_ptr != pm.m_ptr; }
> +
> // Return true if the pointer is a T.
> //
> // This is only valid if T1 and T2 are distinct and if T can be
> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
> index fae0c73e02f..384084c8b3e 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,6 +116,7 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> @@ -197,14 +198,14 @@ struct micro_operation
>
>
> /* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>
> /* Return true if a decl_or_value DV is a DECL or NULL. */
> static inline bool
> dv_is_decl_p (decl_or_value dv)
> {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + return dv.is_first ();
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */
> @@ -219,7 +220,7 @@ static inline tree
> dv_as_decl (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.known_first ();
> }
>
> /* Return the value in the decl_or_value. */
> @@ -227,14 +228,7 @@ static inline rtx
> dv_as_value (decl_or_value dv)
> {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> -}
> -
> -/* Return the opaque pointer in the decl_or_value. */
> -static inline void *
> -dv_as_opaque (decl_or_value dv)
> -{
> - return dv;
> + return dv.known_second ();
> }
>
>
> @@ -483,9 +477,9 @@ static void variable_htab_free (void *);
>
> struct variable_hasher : pointer_hash <variable>
> {
> - typedef void *compare_type;
> + typedef decl_or_value compare_type;
> static inline hashval_t hash (const variable *);
> - static inline bool equal (const variable *, const void *);
> + static inline bool equal (const variable *, const decl_or_value);
> static inline void remove (variable *);
> };
>
> @@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
> /* Compare the declaration of variable X with declaration Y. */
>
> inline bool
> -variable_hasher::equal (const variable *v, const void *y)
> +variable_hasher::equal (const variable *v, const decl_or_value y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return v->dv == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
> @@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
> static inline decl_or_value
> dv_from_decl (tree decl)
> {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl;
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
> static inline decl_or_value
> dv_from_value (rtx value)
> {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = value;
> gcc_checking_assert (dv_is_value_p (dv));
> return dv;
> }
> @@ -1519,7 +1509,7 @@ static attrs *
> attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
> {
> for (; list; list = list->next)
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
> + if (list->dv == dv && list->offset == offset)
> return list;
> return NULL;
> }
> @@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
> dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
>
> for (node = set->regs[REGNO (loc)]; node; node = node->next)
> - if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
> - && node->offset == offset)
> + if (node->dv == dv && node->offset == offset)
> break;
> if (!node)
> attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
> @@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
> for (node = *nextp; node; node = next)
> {
> next = node->next;
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv != decl || node->offset != offset)
> {
> delete_variable_part (set, node->loc, node->dv, node->offset);
> delete node;
> @@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> if (!var->n_var_parts)
> return NULL;
>
> - gcc_checking_assert (loc != dv_as_opaque (var->dv));
> + gcc_checking_assert (var->dv == loc);
>
> loc_code = GET_CODE (loc);
> for (node = var->var_part[0].loc_chain; node; node = node->next)
> @@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
>
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> break;
>
> list = list->next;
> }
>
> gcc_assert (list);
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> list->dv = cdv;
> for (listp = &list->next; (list = *listp); listp = &list->next)
> @@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + if (list->dv == cdv)
> {
> *listp = list->next;
> delete list;
> @@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
> + gcc_assert (list->dv != dv);
> }
> }
> - else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + else if (list->dv == cdv)
> {
> for (listp = &list->next; (list = *listp); listp = &list->next)
> {
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> *listp = list->next;
> delete list;
> @@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
> + gcc_assert (list->dv != cdv);
> }
> }
> else
> @@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (flag_checking)
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> gcc_unreachable ();
>
> list = list->next;
> @@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> check_dupes = true;
> break;
> }
> - else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
> + else if (att->dv == var->dv)
> curp = attp;
> }
>
> @@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> while (*curp)
> if ((*curp)->offset == 0
> && GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
> - && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
> + && (*curp)->dv == var->dv)
> break;
> else
> curp = &(*curp)->next;
> @@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
>
> gcc_assert (var1 != var2);
> gcc_assert (dump_file);
> - gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
> + gcc_assert (var1->dv == var2->dv);
> gcc_assert (var1->n_var_parts == 1
> && var2->n_var_parts == 1);
>
> @@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
>
> if (var1->onepart && var1->n_var_parts)
> {
> - gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
> - && var1->n_var_parts == 1);
> + gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
> /* One-part values have locations in a canonical order. */
> return onepart_variable_different_p (var1, var2);
> }
> @@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> onepart = dv_onepart_p (dv);
>
> gcc_checking_assert (offset == 0 || !onepart);
> - gcc_checking_assert (loc != dv_as_opaque (dv));
> + gcc_checking_assert (dv != loc);
>
> if (! flag_var_tracking_uninit)
> initialized = VAR_INIT_STATUS_INITIALIZED;
> @@ -7684,7 +7667,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> {
> int r = -1, c = 0;
>
> - gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
> + gcc_assert (var->dv == dv);
>
> pos = 0;
>
> @@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
> for (anode = *anextp; anode; anode = anext)
> {
> anext = anode->next;
> - if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
> - && anode->offset == offset)
> + if (anode->dv == var->dv && anode->offset == offset)
> {
> delete anode;
> *anextp = anext;
> @@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
> {
> variable **slot;
>
> - if (!dv_as_opaque (dv)
> - || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> + if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> return;
>
> slot = shared_hash_find_slot_noinsert (set->vars, dv);
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 7:16 ` Richard Sandiford
@ 2023-05-11 10:55 ` Li, Pan2
2023-05-11 12:56 ` Li, Pan2
0 siblings, 1 reply; 21+ messages in thread
From: Li, Pan2 @ 2023-05-11 10:55 UTC (permalink / raw)
To: Richard Sandiford
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
Thanks Richard Sandiford.
There is one interesting thing that the change from v4 to v5 (Aka, remove the case and put dv as first arg) makes some ICE, will have a try for fixing.
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Thursday, May 11, 2023 3:17 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; jakub@redhat.com; rguenther@suse.de
Subject: Re: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to
> tree_node or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the same location and
> | bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicit dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
> Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
> * mux-utils.h: Add overload operator == and != for pointer_mux.
> * var-tracking.cc: Included mux-utils.h for pointer_tmux.
> (decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
> (dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
> (dv_as_decl): Ditto.
> (dv_as_opaque): Removed due to unnecessary.
> (struct variable_hasher): Take decl_or_value as compare_type.
> (variable_hasher::equal): Diito.
> (dv_from_decl): Reconciled to the new type, aka pointer_mux.
> (dv_from_value): Ditto.
> (attrs_list_member): Ditto.
> (vars_copy): Ditto.
> (var_reg_decl_set): Ditto.
> (var_reg_delete_and_set): Ditto.
> (find_loc_in_1pdv): Ditto.
> (canonicalize_values_star): Ditto.
> (variable_post_merge_new_vals): Ditto.
> (dump_onepart_variable_differences): Ditto.
> (variable_different_p): Ditto.
> (set_slot_part): Ditto.
> (clobber_slot_part): Ditto.
> (clobber_variable_part): Ditto.
OK, thanks!
Richard
> ---
> gcc/mux-utils.h | 4 +++
> gcc/var-tracking.cc | 85
> ++++++++++++++++++---------------------------
> 2 files changed, 37 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h index
> a2b6a316899..486d80915b1 100644
> --- a/gcc/mux-utils.h
> +++ b/gcc/mux-utils.h
> @@ -117,6 +117,10 @@ public:
> // ...use ptr.known_second ()...
> T2 *second_or_null () const;
>
> + bool operator == (const pointer_mux &pm) const { return m_ptr ==
> + pm.m_ptr; }
> +
> + bool operator != (const pointer_mux &pm) const { return m_ptr !=
> + pm.m_ptr; }
> +
> // Return true if the pointer is a T.
> //
> // This is only valid if T1 and T2 are distinct and if T can be
> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc index
> fae0c73e02f..384084c8b3e 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,6 +116,7 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> @@ -197,14 +198,14 @@ struct micro_operation
>
>
> /* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>
> /* Return true if a decl_or_value DV is a DECL or NULL. */ static
> inline bool dv_is_decl_p (decl_or_value dv) {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + return dv.is_first ();
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */ @@ -219,7
> +220,7 @@ static inline tree dv_as_decl (decl_or_value dv) {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.known_first ();
> }
>
> /* Return the value in the decl_or_value. */ @@ -227,14 +228,7 @@
> static inline rtx dv_as_value (decl_or_value dv) {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> -}
> -
> -/* Return the opaque pointer in the decl_or_value. */ -static inline
> void * -dv_as_opaque (decl_or_value dv) -{
> - return dv;
> + return dv.known_second ();
> }
>
>
> @@ -483,9 +477,9 @@ static void variable_htab_free (void *);
>
> struct variable_hasher : pointer_hash <variable> {
> - typedef void *compare_type;
> + typedef decl_or_value compare_type;
> static inline hashval_t hash (const variable *);
> - static inline bool equal (const variable *, const void *);
> + static inline bool equal (const variable *, const decl_or_value);
> static inline void remove (variable *); };
>
> @@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
> /* Compare the declaration of variable X with declaration Y. */
>
> inline bool
> -variable_hasher::equal (const variable *v, const void *y)
> +variable_hasher::equal (const variable *v, const decl_or_value y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return v->dv == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct
> variable_def). */ @@ -1396,8 +1388,7 @@ onepart_pool_allocate
> (onepart_enum onepart) static inline decl_or_value dv_from_decl
> (tree decl) {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl;
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1397,7 @@ dv_from_decl (tree decl) static inline
> decl_or_value dv_from_value (rtx value) {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = value;
> gcc_checking_assert (dv_is_value_p (dv));
> return dv;
> }
> @@ -1519,7 +1509,7 @@ static attrs *
> attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT
> offset) {
> for (; list; list = list->next)
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
> + if (list->dv == dv && list->offset == offset)
> return list;
> return NULL;
> }
> @@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash
> + (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
> dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
>
> for (node = set->regs[REGNO (loc)]; node; node = node->next)
> - if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
> - && node->offset == offset)
> + if (node->dv == dv && node->offset == offset)
> break;
> if (!node)
> attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc); @@
> -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
> for (node = *nextp; node; node = next)
> {
> next = node->next;
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv != decl || node->offset != offset)
> {
> delete_variable_part (set, node->loc, node->dv, node->offset);
> delete node;
> @@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> if (!var->n_var_parts)
> return NULL;
>
> - gcc_checking_assert (loc != dv_as_opaque (var->dv));
> + gcc_checking_assert (var->dv == loc);
>
> loc_code = GET_CODE (loc);
> for (node = var->var_part[0].loc_chain; node; node = node->next) @@
> -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot,
> dataflow_set *set)
>
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> break;
>
> list = list->next;
> }
>
> gcc_assert (list);
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> list->dv = cdv;
> for (listp = &list->next; (list = *listp); listp = &list->next) @@
> -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + if (list->dv == cdv)
> {
> *listp = list->next;
> delete list;
> @@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
> + gcc_assert (list->dv != dv);
> }
> }
> - else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + else if (list->dv == cdv)
> {
> for (listp = &list->next; (list = *listp); listp = &list->next)
> {
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> *listp = list->next;
> delete list;
> @@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
> + gcc_assert (list->dv != cdv);
> }
> }
> else
> @@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (flag_checking)
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> gcc_unreachable ();
>
> list = list->next;
> @@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> check_dupes = true;
> break;
> }
> - else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
> + else if (att->dv == var->dv)
> curp = attp;
> }
>
> @@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> while (*curp)
> if ((*curp)->offset == 0
> && GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
> - && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
> + && (*curp)->dv == var->dv)
> break;
> else
> curp = &(*curp)->next;
> @@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable
> *var1, variable *var2)
>
> gcc_assert (var1 != var2);
> gcc_assert (dump_file);
> - gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
> + gcc_assert (var1->dv == var2->dv);
> gcc_assert (var1->n_var_parts == 1
> && var2->n_var_parts == 1);
>
> @@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable
> *var2)
>
> if (var1->onepart && var1->n_var_parts)
> {
> - gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
> - && var1->n_var_parts == 1);
> + gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts
> + == 1);
> /* One-part values have locations in a canonical order. */
> return onepart_variable_different_p (var1, var2);
> }
> @@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> onepart = dv_onepart_p (dv);
>
> gcc_checking_assert (offset == 0 || !onepart);
> - gcc_checking_assert (loc != dv_as_opaque (dv));
> + gcc_checking_assert (dv != loc);
>
> if (! flag_var_tracking_uninit)
> initialized = VAR_INIT_STATUS_INITIALIZED; @@ -7684,7 +7667,7 @@
> set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> {
> int r = -1, c = 0;
>
> - gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
> + gcc_assert (var->dv == dv);
>
> pos = 0;
>
> @@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
> for (anode = *anextp; anode; anode = anext)
> {
> anext = anode->next;
> - if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
> - && anode->offset == offset)
> + if (anode->dv == var->dv && anode->offset == offset)
> {
> delete anode;
> *anextp = anext;
> @@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx
> loc, decl_or_value dv, {
> variable **slot;
>
> - if (!dv_as_opaque (dv)
> - || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> + if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> return;
>
> slot = shared_hash_find_slot_noinsert (set->vars, dv);
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
` (5 preceding siblings ...)
2023-05-11 6:21 ` [PATCH v5] " pan2.li
@ 2023-05-11 12:54 ` pan2.li
6 siblings, 0 replies; 21+ messages in thread
From: pan2.li @ 2023-05-11 12:54 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw,
jakub, rguenther, richard.sandiford
From: Pan Li <pan2.li@intel.com>
The decl_or_value is defined as void * before this PATCH. It will take
care of both the tree_node and rtx_def. Unfortunately, given a void
pointer cannot tell the input is tree_node or rtx_def.
Then we have some implicit structure layout requirement similar as
below. Or we will touch unreasonable bits when cast void * to tree_node
or rtx_def.
+--------+-----------+----------+
| offset | tree_node | rtx_def |
+--------+-----------+----------+
| 0 | code: 16 | code: 16 | <- require the same location and bitssize
+--------+-----------+----------+
| 16 | ... | mode: 8 |
+--------+-----------+----------+
| ... |
+--------+-----------+----------+
| 24 | ... | ... |
+--------+-----------+----------+
This behavior blocks the PATCH that extend the rtx_def mode from 8 to
16 bits for running out of machine mode. This PATCH introduced the
pointer_mux to tell the input is tree_node or rtx_def, and decouple
the above implicit dependency.
Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
Co-Authored-By: Richard Biener <rguenther@suse.de>
Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
gcc/ChangeLog:
* mux-utils.h: Add overload operator == and != for pointer_mux.
* var-tracking.cc: Included mux-utils.h for pointer_tmux.
(decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
(dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
(dv_as_decl): Ditto.
(dv_as_opaque): Removed due to unnecessary.
(struct variable_hasher): Take decl_or_value as compare_type.
(variable_hasher::equal): Diito.
(dv_from_decl): Reconciled to the new type, aka pointer_mux.
(dv_from_value): Ditto.
(attrs_list_member): Ditto.
(vars_copy): Ditto.
(var_reg_decl_set): Ditto.
(var_reg_delete_and_set): Ditto.
(find_loc_in_1pdv): Ditto.
(canonicalize_values_star): Ditto.
(variable_post_merge_new_vals): Ditto.
(dump_onepart_variable_differences): Ditto.
(variable_different_p): Ditto.
(set_slot_part): Ditto.
(clobber_slot_part): Ditto.
(clobber_variable_part): Ditto.
---
gcc/mux-utils.h | 4 +++
gcc/var-tracking.cc | 85 ++++++++++++++++++---------------------------
2 files changed, 37 insertions(+), 52 deletions(-)
diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h
index a2b6a316899..486d80915b1 100644
--- a/gcc/mux-utils.h
+++ b/gcc/mux-utils.h
@@ -117,6 +117,10 @@ public:
// ...use ptr.known_second ()...
T2 *second_or_null () const;
+ bool operator == (const pointer_mux &pm) const { return m_ptr == pm.m_ptr; }
+
+ bool operator != (const pointer_mux &pm) const { return m_ptr != pm.m_ptr; }
+
// Return true if the pointer is a T.
//
// This is only valid if T1 and T2 are distinct and if T can be
diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc
index fae0c73e02f..384084c8b3e 100644
--- a/gcc/var-tracking.cc
+++ b/gcc/var-tracking.cc
@@ -116,6 +116,7 @@
#include "fibonacci_heap.h"
#include "print-rtl.h"
#include "function-abi.h"
+#include "mux-utils.h"
typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
@@ -197,14 +198,14 @@ struct micro_operation
/* A declaration of a variable, or an RTL value being handled like a
- declaration. */
-typedef void *decl_or_value;
+ declaration by pointer_mux. */
+typedef pointer_mux<tree_node, rtx_def> decl_or_value;
/* Return true if a decl_or_value DV is a DECL or NULL. */
static inline bool
dv_is_decl_p (decl_or_value dv)
{
- return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+ return dv.is_first ();
}
/* Return true if a decl_or_value is a VALUE rtl. */
@@ -219,7 +220,7 @@ static inline tree
dv_as_decl (decl_or_value dv)
{
gcc_checking_assert (dv_is_decl_p (dv));
- return (tree) dv;
+ return dv.known_first ();
}
/* Return the value in the decl_or_value. */
@@ -227,14 +228,7 @@ static inline rtx
dv_as_value (decl_or_value dv)
{
gcc_checking_assert (dv_is_value_p (dv));
- return (rtx)dv;
-}
-
-/* Return the opaque pointer in the decl_or_value. */
-static inline void *
-dv_as_opaque (decl_or_value dv)
-{
- return dv;
+ return dv.known_second ();
}
@@ -483,9 +477,9 @@ static void variable_htab_free (void *);
struct variable_hasher : pointer_hash <variable>
{
- typedef void *compare_type;
+ typedef decl_or_value compare_type;
static inline hashval_t hash (const variable *);
- static inline bool equal (const variable *, const void *);
+ static inline bool equal (const variable *, const decl_or_value);
static inline void remove (variable *);
};
@@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
/* Compare the declaration of variable X with declaration Y. */
inline bool
-variable_hasher::equal (const variable *v, const void *y)
+variable_hasher::equal (const variable *v, const decl_or_value y)
{
- decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
-
- return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
+ return v->dv == y;
}
/* Free the element of VARIABLE_HTAB (its type is struct variable_def). */
@@ -1396,8 +1388,7 @@ onepart_pool_allocate (onepart_enum onepart)
static inline decl_or_value
dv_from_decl (tree decl)
{
- decl_or_value dv;
- dv = decl;
+ decl_or_value dv = decl;
gcc_checking_assert (dv_is_decl_p (dv));
return dv;
}
@@ -1406,8 +1397,7 @@ dv_from_decl (tree decl)
static inline decl_or_value
dv_from_value (rtx value)
{
- decl_or_value dv;
- dv = value;
+ decl_or_value dv = value;
gcc_checking_assert (dv_is_value_p (dv));
return dv;
}
@@ -1519,7 +1509,7 @@ static attrs *
attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT offset)
{
for (; list; list = list->next)
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
+ if (list->dv == dv && list->offset == offset)
return list;
return NULL;
}
@@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
{
variable **dstp;
var->refcount++;
- dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
- INSERT);
+ dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv), INSERT);
*dstp = var;
}
}
@@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
for (node = set->regs[REGNO (loc)]; node; node = node->next)
- if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
- && node->offset == offset)
+ if (node->dv == dv && node->offset == offset)
break;
if (!node)
attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc);
@@ -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
for (node = *nextp; node; node = next)
{
next = node->next;
- if (dv_as_opaque (node->dv) != decl || node->offset != offset)
+ if (node->dv != decl || node->offset != offset)
{
delete_variable_part (set, node->loc, node->dv, node->offset);
delete node;
@@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
if (!var->n_var_parts)
return NULL;
- gcc_checking_assert (loc != dv_as_opaque (var->dv));
+ gcc_checking_assert (var->dv != loc);
loc_code = GET_CODE (loc);
for (node = var->var_part[0].loc_chain; node; node = node->next)
@@ -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
break;
list = list->next;
}
gcc_assert (list);
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
list->dv = cdv;
for (listp = &list->next; (list = *listp); listp = &list->next)
@@ -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ if (list->dv == cdv)
{
*listp = list->next;
delete list;
@@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
+ gcc_assert (list->dv != dv);
}
}
- else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
+ else if (list->dv == cdv)
{
for (listp = &list->next; (list = *listp); listp = &list->next)
{
if (list->offset)
continue;
- if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
+ if (list->dv == dv)
{
*listp = list->next;
delete list;
@@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
break;
}
- gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
+ gcc_assert (list->dv != cdv);
}
}
else
@@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
if (flag_checking)
while (list)
{
- if (list->offset == 0
- && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
- || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
+ if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
gcc_unreachable ();
list = list->next;
@@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
check_dupes = true;
break;
}
- else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
+ else if (att->dv == var->dv)
curp = attp;
}
@@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
while (*curp)
if ((*curp)->offset == 0
&& GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
- && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
+ && (*curp)->dv == var->dv)
break;
else
curp = &(*curp)->next;
@@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable *var1, variable *var2)
gcc_assert (var1 != var2);
gcc_assert (dump_file);
- gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
+ gcc_assert (var1->dv == var2->dv);
gcc_assert (var1->n_var_parts == 1
&& var2->n_var_parts == 1);
@@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable *var2)
if (var1->onepart && var1->n_var_parts)
{
- gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
- && var1->n_var_parts == 1);
+ gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts == 1);
/* One-part values have locations in a canonical order. */
return onepart_variable_different_p (var1, var2);
}
@@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
onepart = dv_onepart_p (dv);
gcc_checking_assert (offset == 0 || !onepart);
- gcc_checking_assert (loc != dv_as_opaque (dv));
+ gcc_checking_assert (dv != loc);
if (! flag_var_tracking_uninit)
initialized = VAR_INIT_STATUS_INITIALIZED;
@@ -7684,7 +7667,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
{
int r = -1, c = 0;
- gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
+ gcc_assert (var->dv == dv);
pos = 0;
@@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
for (anode = *anextp; anode; anode = anext)
{
anext = anode->next;
- if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
- && anode->offset == offset)
+ if (anode->dv == var->dv && anode->offset == offset)
{
delete anode;
*anextp = anext;
@@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx loc, decl_or_value dv,
{
variable **slot;
- if (!dv_as_opaque (dv)
- || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
+ if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
return;
slot = shared_hash_find_slot_noinsert (set->vars, dv);
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
2023-05-11 10:55 ` Li, Pan2
@ 2023-05-11 12:56 ` Li, Pan2
0 siblings, 0 replies; 21+ messages in thread
From: Li, Pan2 @ 2023-05-11 12:56 UTC (permalink / raw)
To: Richard Sandiford
Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang,
jeffreyalaw, jakub, rguenther
Sorry for disturbing, fixed my silly mistake in PATCH v6 and passed x86 regression test. If no more concern, will commit after pass the x86 regression test.
Pan
-----Original Message-----
From: Li, Pan2
Sent: Thursday, May 11, 2023 6:56 PM
To: Richard Sandiford <richard.sandiford@arm.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; jakub@redhat.com; rguenther@suse.de
Subject: RE: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
Thanks Richard Sandiford.
There is one interesting thing that the change from v4 to v5 (Aka, remove the case and put dv as first arg) makes some ICE, will have a try for fixing.
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Thursday, May 11, 2023 3:17 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com; jakub@redhat.com; rguenther@suse.de
Subject: Re: [PATCH v5] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> The decl_or_value is defined as void * before this PATCH. It will take
> care of both the tree_node and rtx_def. Unfortunately, given a void
> pointer cannot tell the input is tree_node or rtx_def.
>
> Then we have some implicit structure layout requirement similar as
> below. Or we will touch unreasonable bits when cast void * to
> tree_node or rtx_def.
>
> +--------+-----------+----------+
> | offset | tree_node | rtx_def |
> +--------+-----------+----------+
> | 0 | code: 16 | code: 16 | <- require the same location and
> | bitssize
> +--------+-----------+----------+
> | 16 | ... | mode: 8 |
> +--------+-----------+----------+
> | ... |
> +--------+-----------+----------+
> | 24 | ... | ... |
> +--------+-----------+----------+
>
> This behavior blocks the PATCH that extend the rtx_def mode from 8 to
> 16 bits for running out of machine mode. This PATCH introduced the
> pointer_mux to tell the input is tree_node or rtx_def, and decouple
> the above implicit dependency.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Richard Sandiford <richard.sandiford@arm.com>
> Co-Authored-By: Richard Biener <rguenther@suse.de>
> Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
> * mux-utils.h: Add overload operator == and != for pointer_mux.
> * var-tracking.cc: Included mux-utils.h for pointer_tmux.
> (decl_or_value): Changed from void * to pointer_mux<tree_node, rtx_def>.
> (dv_is_decl_p): Reconciled to the new type, aka pointer_mux.
> (dv_as_decl): Ditto.
> (dv_as_opaque): Removed due to unnecessary.
> (struct variable_hasher): Take decl_or_value as compare_type.
> (variable_hasher::equal): Diito.
> (dv_from_decl): Reconciled to the new type, aka pointer_mux.
> (dv_from_value): Ditto.
> (attrs_list_member): Ditto.
> (vars_copy): Ditto.
> (var_reg_decl_set): Ditto.
> (var_reg_delete_and_set): Ditto.
> (find_loc_in_1pdv): Ditto.
> (canonicalize_values_star): Ditto.
> (variable_post_merge_new_vals): Ditto.
> (dump_onepart_variable_differences): Ditto.
> (variable_different_p): Ditto.
> (set_slot_part): Ditto.
> (clobber_slot_part): Ditto.
> (clobber_variable_part): Ditto.
OK, thanks!
Richard
> ---
> gcc/mux-utils.h | 4 +++
> gcc/var-tracking.cc | 85
> ++++++++++++++++++---------------------------
> 2 files changed, 37 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/mux-utils.h b/gcc/mux-utils.h index
> a2b6a316899..486d80915b1 100644
> --- a/gcc/mux-utils.h
> +++ b/gcc/mux-utils.h
> @@ -117,6 +117,10 @@ public:
> // ...use ptr.known_second ()...
> T2 *second_or_null () const;
>
> + bool operator == (const pointer_mux &pm) const { return m_ptr ==
> + pm.m_ptr; }
> +
> + bool operator != (const pointer_mux &pm) const { return m_ptr !=
> + pm.m_ptr; }
> +
> // Return true if the pointer is a T.
> //
> // This is only valid if T1 and T2 are distinct and if T can be
> diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc index
> fae0c73e02f..384084c8b3e 100644
> --- a/gcc/var-tracking.cc
> +++ b/gcc/var-tracking.cc
> @@ -116,6 +116,7 @@
> #include "fibonacci_heap.h"
> #include "print-rtl.h"
> #include "function-abi.h"
> +#include "mux-utils.h"
>
> typedef fibonacci_heap <long, basic_block_def> bb_heap_t;
>
> @@ -197,14 +198,14 @@ struct micro_operation
>
>
> /* A declaration of a variable, or an RTL value being handled like a
> - declaration. */
> -typedef void *decl_or_value;
> + declaration by pointer_mux. */
> +typedef pointer_mux<tree_node, rtx_def> decl_or_value;
>
> /* Return true if a decl_or_value DV is a DECL or NULL. */ static
> inline bool dv_is_decl_p (decl_or_value dv) {
> - return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> + return dv.is_first ();
> }
>
> /* Return true if a decl_or_value is a VALUE rtl. */ @@ -219,7
> +220,7 @@ static inline tree dv_as_decl (decl_or_value dv) {
> gcc_checking_assert (dv_is_decl_p (dv));
> - return (tree) dv;
> + return dv.known_first ();
> }
>
> /* Return the value in the decl_or_value. */ @@ -227,14 +228,7 @@
> static inline rtx dv_as_value (decl_or_value dv) {
> gcc_checking_assert (dv_is_value_p (dv));
> - return (rtx)dv;
> -}
> -
> -/* Return the opaque pointer in the decl_or_value. */ -static inline
> void * -dv_as_opaque (decl_or_value dv) -{
> - return dv;
> + return dv.known_second ();
> }
>
>
> @@ -483,9 +477,9 @@ static void variable_htab_free (void *);
>
> struct variable_hasher : pointer_hash <variable> {
> - typedef void *compare_type;
> + typedef decl_or_value compare_type;
> static inline hashval_t hash (const variable *);
> - static inline bool equal (const variable *, const void *);
> + static inline bool equal (const variable *, const decl_or_value);
> static inline void remove (variable *); };
>
> @@ -501,11 +495,9 @@ variable_hasher::hash (const variable *v)
> /* Compare the declaration of variable X with declaration Y. */
>
> inline bool
> -variable_hasher::equal (const variable *v, const void *y)
> +variable_hasher::equal (const variable *v, const decl_or_value y)
> {
> - decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
> -
> - return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> + return v->dv == y;
> }
>
> /* Free the element of VARIABLE_HTAB (its type is struct
> variable_def). */ @@ -1396,8 +1388,7 @@ onepart_pool_allocate
> (onepart_enum onepart) static inline decl_or_value dv_from_decl
> (tree decl) {
> - decl_or_value dv;
> - dv = decl;
> + decl_or_value dv = decl;
> gcc_checking_assert (dv_is_decl_p (dv));
> return dv;
> }
> @@ -1406,8 +1397,7 @@ dv_from_decl (tree decl) static inline
> decl_or_value dv_from_value (rtx value) {
> - decl_or_value dv;
> - dv = value;
> + decl_or_value dv = value;
> gcc_checking_assert (dv_is_value_p (dv));
> return dv;
> }
> @@ -1519,7 +1509,7 @@ static attrs *
> attrs_list_member (attrs *list, decl_or_value dv, HOST_WIDE_INT
> offset) {
> for (; list; list = list->next)
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv) && list->offset == offset)
> + if (list->dv == dv && list->offset == offset)
> return list;
> return NULL;
> }
> @@ -1831,8 +1821,7 @@ vars_copy (variable_table_type *dst, variable_table_type *src)
> {
> variable **dstp;
> var->refcount++;
> - dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash (var->dv),
> - INSERT);
> + dstp = dst->find_slot_with_hash (var->dv, dv_htab_hash
> + (var->dv), INSERT);
> *dstp = var;
> }
> }
> @@ -1866,8 +1855,7 @@ var_reg_decl_set (dataflow_set *set, rtx loc, enum var_init_status initialized,
> dv = dv_from_decl (var_debug_decl (dv_as_decl (dv)));
>
> for (node = set->regs[REGNO (loc)]; node; node = node->next)
> - if (dv_as_opaque (node->dv) == dv_as_opaque (dv)
> - && node->offset == offset)
> + if (node->dv == dv && node->offset == offset)
> break;
> if (!node)
> attrs_list_insert (&set->regs[REGNO (loc)], dv, offset, loc); @@
> -1966,7 +1954,7 @@ var_reg_delete_and_set (dataflow_set *set, rtx loc, bool modify,
> for (node = *nextp; node; node = next)
> {
> next = node->next;
> - if (dv_as_opaque (node->dv) != decl || node->offset != offset)
> + if (node->dv != decl || node->offset != offset)
> {
> delete_variable_part (set, node->loc, node->dv, node->offset);
> delete node;
> @@ -3242,7 +3230,7 @@ find_loc_in_1pdv (rtx loc, variable *var, variable_table_type *vars)
> if (!var->n_var_parts)
> return NULL;
>
> - gcc_checking_assert (loc != dv_as_opaque (var->dv));
> + gcc_checking_assert (var->dv == loc);
>
> loc_code = GET_CODE (loc);
> for (node = var->var_part[0].loc_chain; node; node = node->next) @@
> -3832,16 +3820,14 @@ canonicalize_values_star (variable **slot,
> dataflow_set *set)
>
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> break;
>
> list = list->next;
> }
>
> gcc_assert (list);
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> list->dv = cdv;
> for (listp = &list->next; (list = *listp); listp = &list->next) @@
> -3849,7 +3835,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + if (list->dv == cdv)
> {
> *listp = list->next;
> delete list;
> @@ -3857,17 +3843,17 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (dv));
> + gcc_assert (list->dv != dv);
> }
> }
> - else if (dv_as_opaque (list->dv) == dv_as_opaque (cdv))
> + else if (list->dv == cdv)
> {
> for (listp = &list->next; (list = *listp); listp = &list->next)
> {
> if (list->offset)
> continue;
>
> - if (dv_as_opaque (list->dv) == dv_as_opaque (dv))
> + if (list->dv == dv)
> {
> *listp = list->next;
> delete list;
> @@ -3875,7 +3861,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> break;
> }
>
> - gcc_assert (dv_as_opaque (list->dv) != dv_as_opaque (cdv));
> + gcc_assert (list->dv != cdv);
> }
> }
> else
> @@ -3884,9 +3870,7 @@ canonicalize_values_star (variable **slot, dataflow_set *set)
> if (flag_checking)
> while (list)
> {
> - if (list->offset == 0
> - && (dv_as_opaque (list->dv) == dv_as_opaque (dv)
> - || dv_as_opaque (list->dv) == dv_as_opaque (cdv)))
> + if (list->offset == 0 && (list->dv == dv || list->dv == cdv))
> gcc_unreachable ();
>
> list = list->next;
> @@ -4475,7 +4459,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> check_dupes = true;
> break;
> }
> - else if (dv_as_opaque (att->dv) == dv_as_opaque (var->dv))
> + else if (att->dv == var->dv)
> curp = attp;
> }
>
> @@ -4485,7 +4469,7 @@ variable_post_merge_new_vals (variable **slot, dfset_post_merge *dfpm)
> while (*curp)
> if ((*curp)->offset == 0
> && GET_MODE ((*curp)->loc) == GET_MODE (node->loc)
> - && dv_as_opaque ((*curp)->dv) == dv_as_opaque (var->dv))
> + && (*curp)->dv == var->dv)
> break;
> else
> curp = &(*curp)->next;
> @@ -4989,7 +4973,7 @@ dump_onepart_variable_differences (variable
> *var1, variable *var2)
>
> gcc_assert (var1 != var2);
> gcc_assert (dump_file);
> - gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv));
> + gcc_assert (var1->dv == var2->dv);
> gcc_assert (var1->n_var_parts == 1
> && var2->n_var_parts == 1);
>
> @@ -5054,8 +5038,7 @@ variable_different_p (variable *var1, variable
> *var2)
>
> if (var1->onepart && var1->n_var_parts)
> {
> - gcc_checking_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)
> - && var1->n_var_parts == 1);
> + gcc_checking_assert (var1->dv == var2->dv && var1->n_var_parts
> + == 1);
> /* One-part values have locations in a canonical order. */
> return onepart_variable_different_p (var1, var2);
> }
> @@ -7656,7 +7639,7 @@ set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> onepart = dv_onepart_p (dv);
>
> gcc_checking_assert (offset == 0 || !onepart);
> - gcc_checking_assert (loc != dv_as_opaque (dv));
> + gcc_checking_assert (dv != loc);
>
> if (! flag_var_tracking_uninit)
> initialized = VAR_INIT_STATUS_INITIALIZED; @@ -7684,7 +7667,7 @@
> set_slot_part (dataflow_set *set, rtx loc, variable **slot,
> {
> int r = -1, c = 0;
>
> - gcc_assert (dv_as_opaque (var->dv) == dv_as_opaque (dv));
> + gcc_assert (var->dv == dv);
>
> pos = 0;
>
> @@ -7950,8 +7933,7 @@ clobber_slot_part (dataflow_set *set, rtx loc, variable **slot,
> for (anode = *anextp; anode; anode = anext)
> {
> anext = anode->next;
> - if (dv_as_opaque (anode->dv) == dv_as_opaque (var->dv)
> - && anode->offset == offset)
> + if (anode->dv == var->dv && anode->offset == offset)
> {
> delete anode;
> *anextp = anext;
> @@ -7980,8 +7962,7 @@ clobber_variable_part (dataflow_set *set, rtx
> loc, decl_or_value dv, {
> variable **slot;
>
> - if (!dv_as_opaque (dv)
> - || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> + if (!dv || (!dv_is_value_p (dv) && ! DECL_P (dv_as_decl (dv))))
> return;
>
> slot = shared_hash_find_slot_noinsert (set->vars, dv);
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-05-11 13:26 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 7:17 [PATCH] Var-Tracking: Leverage pointer_mux for decl_or_value pan2.li
2023-05-10 7:56 ` Richard Biener
2023-05-10 8:02 ` Richard Sandiford
2023-05-10 8:13 ` Jakub Jelinek
2023-05-10 11:59 ` Li, Pan2
2023-05-10 11:57 ` [PATCH v2] Var-Tracking: Typedef pointer_mux<tree_node, rtx_def> as decl_or_value pan2.li
2023-05-10 12:22 ` Jakub Jelinek
2023-05-10 12:53 ` Richard Sandiford
2023-05-10 13:57 ` Li, Pan2
2023-05-10 15:23 ` [PATCH v3] " pan2.li
2023-05-10 15:55 ` Richard Sandiford
2023-05-11 2:30 ` Li, Pan2
2023-05-11 4:43 ` Richard Sandiford
2023-05-11 6:30 ` Li, Pan2
2023-05-11 6:31 ` Bernhard Reutner-Fischer
2023-05-11 2:28 ` [PATCH v4] " pan2.li
2023-05-11 6:21 ` [PATCH v5] " pan2.li
2023-05-11 7:16 ` Richard Sandiford
2023-05-11 10:55 ` Li, Pan2
2023-05-11 12:56 ` Li, Pan2
2023-05-11 12:54 ` [PATCH v6] " pan2.li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).