Re: [PATCH #2a/2]
On Wed, Dec 13, 2023 at 4:05 AM Alexandre Oliva wrote:
>
> On Dec 12, 2023, Richard Biener wrote:
>
> > On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva wrote:
>
> >> DECL_NOT_GIMPLE_REG_P (arg) = 0;
>
> > I wonder why you clear this at all?
>
> That code seems to be inherited from expand_thunk.
> ISTR that flag was not negated when I started the strub implementation,
> back in gcc-10.
>
> >> +convert in separate statements. ??? Should
> >> +we drop volatile from the wrapper
> >> +instead? */
>
> > volatile on function parameters are indeed odd beasts. You could
> > also force volatile arguments to be passed indirectly.
>
> Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of
> #1/2, now a cleanup that IMHO would still be desirable.
>
>
> strub: indirect volatile parms in wrappers
>
> Arrange for strub internal wrappers to pass volatile arguments by
> reference to the wrapped bodies.
OK.
>
> for gcc/ChangeLog
>
> PR middle-end/112938
> * ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args
> by reference to internal strub wrapped bodies.
>
> for gcc/testsuite/ChangeLog
>
> PR middle-end/112938
> * gcc.dg/strub-internal-volatile.c: Check indirection of
> volatile args.
> ---
> gcc/ipa-strub.cc | 19 +--
> gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 +
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 45294b0b46bcb..943bb60996fc1 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *)
>parm = DECL_CHAIN (parm),
>nparm = DECL_CHAIN (nparm),
>nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE)
> - if (!(0 /* DECL_BY_REFERENCE (narg) */
> - || is_gimple_reg_type (TREE_TYPE (nparm))
> - || VECTOR_TYPE_P (TREE_TYPE (nparm))
> - || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
> - || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
> - && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
> - <= 4 * UNITS_PER_WORD
> + if (TREE_THIS_VOLATILE (parm)
> + || !(0 /* DECL_BY_REFERENCE (narg) */
> + || is_gimple_reg_type (TREE_TYPE (nparm))
> + || VECTOR_TYPE_P (TREE_TYPE (nparm))
> + || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
> + || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
> + && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
> + <= 4 * UNITS_PER_WORD
> {
> /* No point in indirecting pointer types. Presumably they
> won't ever pass the size-based test above, but check the
> @@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *)
> {
> tree tmp = arg;
> /* If ARG is e.g. volatile, we must copy and
> -convert in separate statements. ??? Should
> -we drop volatile from the wrapper
> -instead? */
> +convert in separate statements. */
> if (!is_gimple_val (arg))
> {
> tmp = create_tmp_reg (TYPE_MAIN_VARIANT
> diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> index cdfca67616bc8..227406af245cc 100644
> --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-options "-fdump-ipa-strub" } */
> /* { dg-require-effective-target strub } */
>
> void __attribute__ ((strub("internal")))
> @@ -8,3 +9,7 @@ f(volatile short) {
> void g(void) {
>f(0);
> }
> +
> +/* We make volatile parms indirect in the wrapped f. */
> +/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */
> +/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */
>
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2a/2] strub: indirect volatile parms in wrappers
[sorry that the previous, unfinished post got through]
On Dec 12, 2023, Richard Biener wrote:
> On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva wrote:
>> DECL_NOT_GIMPLE_REG_P (arg) = 0;
> I wonder why you clear this at all?
That code seems to be inherited from expand_thunk.
ISTR that flag was not negated when I started the strub implementation,
back in gcc-10.
>> +convert in separate statements. ??? Should
>> +we drop volatile from the wrapper
>> +instead? */
> volatile on function parameters are indeed odd beasts. You could
> also force volatile arguments to be passed indirectly.
Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of
#1/2, now a cleanup that IMHO would still be desirable.
Arrange for strub internal wrappers to pass volatile arguments by
reference to the wrapped bodies.
for gcc/ChangeLog
PR middle-end/112938
* ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args
by reference to internal strub wrapped bodies.
for gcc/testsuite/ChangeLog
PR middle-end/112938
* gcc.dg/strub-internal-volatile.c: Check indirection of
volatile args.
---
gcc/ipa-strub.cc | 19 +--
gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 +
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
index 45294b0b46bcb..943bb60996fc1 100644
--- a/gcc/ipa-strub.cc
+++ b/gcc/ipa-strub.cc
@@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *)
parm = DECL_CHAIN (parm),
nparm = DECL_CHAIN (nparm),
nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE)
- if (!(0 /* DECL_BY_REFERENCE (narg) */
- || is_gimple_reg_type (TREE_TYPE (nparm))
- || VECTOR_TYPE_P (TREE_TYPE (nparm))
- || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
- || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
- && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
- <= 4 * UNITS_PER_WORD
+ if (TREE_THIS_VOLATILE (parm)
+ || !(0 /* DECL_BY_REFERENCE (narg) */
+ || is_gimple_reg_type (TREE_TYPE (nparm))
+ || VECTOR_TYPE_P (TREE_TYPE (nparm))
+ || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
+ || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
+ && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
+ <= 4 * UNITS_PER_WORD
{
/* No point in indirecting pointer types. Presumably they
won't ever pass the size-based test above, but check the
@@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *)
{
tree tmp = arg;
/* If ARG is e.g. volatile, we must copy and
-convert in separate statements. ??? Should
-we drop volatile from the wrapper
-instead? */
+convert in separate statements. */
if (!is_gimple_val (arg))
{
tmp = create_tmp_reg (TYPE_MAIN_VARIANT
diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
index cdfca67616bc8..227406af245cc 100644
--- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
+++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-options "-fdump-ipa-strub" } */
/* { dg-require-effective-target strub } */
void __attribute__ ((strub("internal")))
@@ -8,3 +9,7 @@ f(volatile short) {
void g(void) {
f(0);
}
+
+/* We make volatile parms indirect in the wrapped f. */
+/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */
+/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2a/2]
On Dec 12, 2023, Richard Biener wrote:
> On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva wrote:
>> DECL_NOT_GIMPLE_REG_P (arg) = 0;
> I wonder why you clear this at all?
That code seems to be inherited from expand_thunk.
ISTR that flag was not negated when I started the strub implementation,
back in gcc-10.
>> +convert in separate statements. ??? Should
>> +we drop volatile from the wrapper
>> +instead? */
> volatile on function parameters are indeed odd beasts. You could
> also force volatile arguments to be passed indirectly.
Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of
#1/2, now a cleanup that IMHO would still be desirable.
strub: indirect volatile parms in wrappers
Arrange for strub internal wrappers to pass volatile arguments by
reference to the wrapped bodies.
for gcc/ChangeLog
PR middle-end/112938
* ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args
by reference to internal strub wrapped bodies.
for gcc/testsuite/ChangeLog
PR middle-end/112938
* gcc.dg/strub-internal-volatile.c: Check indirection of
volatile args.
---
gcc/ipa-strub.cc | 19 +--
gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 +
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
index 45294b0b46bcb..943bb60996fc1 100644
--- a/gcc/ipa-strub.cc
+++ b/gcc/ipa-strub.cc
@@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *)
parm = DECL_CHAIN (parm),
nparm = DECL_CHAIN (nparm),
nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE)
- if (!(0 /* DECL_BY_REFERENCE (narg) */
- || is_gimple_reg_type (TREE_TYPE (nparm))
- || VECTOR_TYPE_P (TREE_TYPE (nparm))
- || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
- || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
- && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
- <= 4 * UNITS_PER_WORD
+ if (TREE_THIS_VOLATILE (parm)
+ || !(0 /* DECL_BY_REFERENCE (narg) */
+ || is_gimple_reg_type (TREE_TYPE (nparm))
+ || VECTOR_TYPE_P (TREE_TYPE (nparm))
+ || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE
+ || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
+ && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
+ <= 4 * UNITS_PER_WORD
{
/* No point in indirecting pointer types. Presumably they
won't ever pass the size-based test above, but check the
@@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *)
{
tree tmp = arg;
/* If ARG is e.g. volatile, we must copy and
-convert in separate statements. ??? Should
-we drop volatile from the wrapper
-instead? */
+convert in separate statements. */
if (!is_gimple_val (arg))
{
tmp = create_tmp_reg (TYPE_MAIN_VARIANT
diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
index cdfca67616bc8..227406af245cc 100644
--- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c
+++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-options "-fdump-ipa-strub" } */
/* { dg-require-effective-target strub } */
void __attribute__ ((strub("internal")))
@@ -8,3 +9,7 @@ f(volatile short) {
void g(void) {
f(0);
}
+
+/* We make volatile parms indirect in the wrapped f. */
+/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */
+/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH][2a/2] Remove referenced vars
This is part one of the patch (and thus single commit) that will
remove referenced vars once we got rid of var annotations.
It removes referenced_var_lookup and all callers - which shows
you where dumping will be affected.
Bootstrap & regtest pending on x86_64-unknown-linux-gnu.
Richard.
2012-08-01 Richard Guenther
* tree-dfa.c (referenced_var_lookup): Remove.
* tree-flow.h (referenced_var_lookup): Likewise.
* cfgexpand.c (update_alias_info_with_stack_vars): Remove assert.
* gimple-pretty-print.c (pp_points_to_solution): Dump UIDs
unconditionally.
* tree-into-ssa.c (dump_decl_set): Likewise.
* tree-ssa.c (target_for_debug_bind): Virtual operands are
not suitable, but all register type vars are.
Index: trunk/gcc/cfgexpand.c
===
*** trunk.orig/gcc/cfgexpand.c 2012-07-20 12:11:05.0 +0200
--- trunk/gcc/cfgexpand.c 2012-08-01 11:46:40.447070164 +0200
*** update_alias_info_with_stack_vars (void)
*** 620,632
{
tree decl = stack_vars[j].decl;
unsigned int uid = DECL_PT_UID (decl);
- /* We should never end up partitioning SSA names (though they
-may end up on the stack). Neither should we allocate stack
-space to something that is unused and thus unreferenced, except
-for -O0 where we are preserving even unreferenced variables. */
- gcc_assert (DECL_P (decl)
- && (!optimize
- || referenced_var_lookup (cfun, DECL_UID (decl;
bitmap_set_bit (part, uid);
*((bitmap *) pointer_map_insert (decls_to_partitions,
(void *)(size_t) uid)) = part;
--- 620,625
Index: trunk/gcc/gimple-pretty-print.c
===
*** trunk.orig/gcc/gimple-pretty-print.c2012-07-26 10:46:42.0
+0200
--- trunk/gcc/gimple-pretty-print.c 2012-08-01 11:49:41.513063937 +0200
*** pp_points_to_solution (pretty_printer *b
*** 597,617
pp_string (buffer, "{ ");
EXECUTE_IF_SET_IN_BITMAP (pt->vars, 0, i, bi)
{
! tree var = referenced_var_lookup (cfun, i);
! if (var)
! {
! dump_generic_node (buffer, var, 0, dump_flags, false);
! if (DECL_PT_UID (var) != DECL_UID (var))
! {
! pp_string (buffer, "ptD.");
! pp_decimal_int (buffer, DECL_PT_UID (var));
! }
! }
! else
! {
! pp_string (buffer, "D.");
! pp_decimal_int (buffer, i);
! }
pp_character (buffer, ' ');
}
pp_character (buffer, '}');
--- 597,604
pp_string (buffer, "{ ");
EXECUTE_IF_SET_IN_BITMAP (pt->vars, 0, i, bi)
{
! pp_string (buffer, "D.");
! pp_decimal_int (buffer, i);
pp_character (buffer, ' ');
}
pp_character (buffer, '}');
Index: trunk/gcc/tree-dfa.c
===
*** trunk.orig/gcc/tree-dfa.c 2012-08-01 11:16:45.0 +0200
--- trunk/gcc/tree-dfa.c2012-08-01 11:49:57.773063325 +0200
*** find_referenced_vars_in (gimple stmt)
*** 430,448
}
- /* Lookup UID in the referenced_vars hashtable and return the associated
-variable. */
-
- tree
- referenced_var_lookup (struct function *fn, unsigned int uid)
- {
- tree h;
- struct tree_decl_minimal in;
- in.uid = uid;
- h = (tree) htab_find_with_hash (gimple_referenced_vars (fn), &in, uid);
- return h;
- }
-
/* Check if TO is in the referenced_vars hash table and insert it if not.
Return true if it required insertion. */
--- 430,435
Index: trunk/gcc/tree-flow.h
===
*** trunk.orig/gcc/tree-flow.h 2012-08-01 11:16:45.0 +0200
--- trunk/gcc/tree-flow.h 2012-08-01 12:02:27.703037366 +0200
*** typedef struct
*** 323,329
!end_referenced_vars_p (&(ITER)); \
(VAR) = next_referenced_var (&(ITER)))
- extern tree referenced_var_lookup (struct function *, unsigned int);
#define num_referenced_vars htab_elements (gimple_referenced_vars (cfun))
#define num_ssa_names (VEC_length (tree, cfun->gimple_df->ssa_names))
--- 323,328
Index: trunk/gcc/tree-into-ssa.c
===
*** trunk.orig/gcc/tree-into-ssa.c 2012-08-01 11:16:45.0 +0200
--- trunk/gcc/tree-into-ssa.c 2012-08-01 11:50:17.248062660 +0200
*** dump_decl_set (FILE *file, bitmap set)
*** 1554,1564
EXECUTE_IF_SET_IN_BITMAP (set, 0, i, bi)
{
! tree var = referenced_var_lookup (cfun, i);
! if (var)
!
