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 <rguent...@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
