[PATCH][v2] Adjust volatile handling of the operand scanner

Richard Biener richard.guenther@gmail.com
Tue Aug 10 15:55:35 GMT 2021


On Tue, Aug 10, 2021 at 1:41 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The GIMPLE SSA operand scanner handles COMPONENT_REFs that are
> not marked TREE_THIS_VOLATILE but have a TREE_THIS_VOLATILE
> FIELD_DECL as volatile.  That's inconsistent in how TREE_THIS_VOLATILE
> testing on GENERIC refs works which requires operand zero of
> component references to mirror TREE_THIS_VOLATILE to the ref
> so that testing TREE_THIS_VOLATILE on the outermost reference
> is enough to determine the volatileness.
>
> The following patch thus removes FIELD_DECL scanning from
> the GIMPLE SSA operand scanner, possibly leaving fewer stmts
> marked as gimple_has_volatile_ops.
[...]

> The question is whether we instead want to amend build3 to
> set TREE_THIS_VOLATILE automatically when the FIELD_DECL has
> it set.

A change along that line also passes bootstrap and regtest.

Any preference?

Thanks,
Richard.

>  At least for the Fortran FE cases the gimplifier
> fails to see some volatile references and thus can generate
> wrong code which is a latent issue.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> OK?
>
> Thanks,
> Richard.
>
> 2021-08-09  Richard Biener  <rguenther@suse.de>
>
> gcc/
>         * tree-ssa-operands.c (operands_scanner::get_expr_operands):
>         Do not look at COMPONENT_REF FIELD_DECLs TREE_THIS_VOLATILE
>         to determine has_volatile_ops.
>
> gcc/fortran/
>         * trans-common.c (create_common): Set TREE_THIS_VOLATILE on the
>         COMPONENT_REF if the field is volatile.
> ---
>  gcc/fortran/trans-common.c | 9 +++++----
>  gcc/tree-ssa-operands.c    | 7 +------
>  2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/fortran/trans-common.c b/gcc/fortran/trans-common.c
> index a11cf4c839e..7bcf18dc475 100644
> --- a/gcc/fortran/trans-common.c
> +++ b/gcc/fortran/trans-common.c
> @@ -759,10 +759,11 @@ create_common (gfc_common_head *com, segment_info *head, bool saw_equiv)
>        else
>         gfc_add_decl_to_function (var_decl);
>
> -      SET_DECL_VALUE_EXPR (var_decl,
> -                          fold_build3_loc (input_location, COMPONENT_REF,
> -                                           TREE_TYPE (s->field),
> -                                           decl, s->field, NULL_TREE));
> +      tree comp = build3_loc (input_location, COMPONENT_REF,
> +                             TREE_TYPE (s->field), decl, s->field, NULL_TREE);
> +      if (TREE_THIS_VOLATILE (s->field))
> +       TREE_THIS_VOLATILE (comp) = 1;
> +      SET_DECL_VALUE_EXPR (var_decl, comp);
>        DECL_HAS_VALUE_EXPR_P (var_decl) = 1;
>        GFC_DECL_COMMON_OR_EQUIV (var_decl) = 1;
>
> diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
> index c15575416dd..ebf7eea3b04 100644
> --- a/gcc/tree-ssa-operands.c
> +++ b/gcc/tree-ssa-operands.c
> @@ -834,12 +834,7 @@ operands_scanner::get_expr_operands (tree *expr_p, int flags)
>         get_expr_operands (&TREE_OPERAND (expr, 0), flags);
>
>         if (code == COMPONENT_REF)
> -         {
> -           if (!(flags & opf_no_vops)
> -               && TREE_THIS_VOLATILE (TREE_OPERAND (expr, 1)))
> -             gimple_set_has_volatile_ops (stmt, true);
> -           get_expr_operands (&TREE_OPERAND (expr, 2), uflags);
> -         }
> +         get_expr_operands (&TREE_OPERAND (expr, 2), uflags);
>         else if (code == ARRAY_REF || code == ARRAY_RANGE_REF)
>           {
>             get_expr_operands (&TREE_OPERAND (expr, 1), uflags);
> --
> 2.31.1


More information about the Gcc-patches mailing list