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.
It shows we miss at least one case in the fortran frontend, though
there's a suspicious amount of COMPONENT_REF creation compared
to little setting of TREE_THIS_VOLATILE. This fixes the FAIL
of gfortran.dg/volatile11.f90 that would otherwise occur.
Visually inspecting fortran/ reveals a bunch of likely to fix
cases but I don't know the constraints of 'volatile' uses in
the fortran language to assess whether some of these are not
necessary. The cases would be fixed with
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0d013defdbb..5d708fe90aa 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -6983,9 +6983,10 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm,
tree desc, tree offset,
case REF_COMPONENT:
field = ref->u.c.component->backend_decl;
gcc_assert (field && TREE_CODE (field) == FIELD_DECL);
- tmp = fold_build3_loc (input_location, COMPONENT_REF,
- TREE_TYPE (field),
- tmp, field, NULL_TREE);
+ tmp = build3_loc (input_location, COMPONENT_REF,
+ TREE_TYPE (field), tmp, field, NULL_TREE);
+ if (TREE_THIS_VOLATILE (field))
+ TREE_THIS_VOLATILE (tmp) = 1;
break;
case REF_SUBSTRING:
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index c4291cce079..e6dc79f8c1e 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -2244,9 +2244,11 @@ gfc_get_tree_for_caf_expr (gfc_expr *expr)
if (POINTER_TYPE_P (TREE_TYPE (caf_decl)))
caf_decl = build_fold_indirect_ref_loc (input_location, caf_decl);
- caf_decl = fold_build3_loc (input_location, COMPONENT_REF,
- TREE_TYPE (comp->backend_decl), caf_decl,
- comp->backend_decl, NULL_TREE);
+ caf_decl = build3_loc (input_location, COMPONENT_REF,
+ TREE_TYPE (comp->backend_decl), caf_decl,
+ comp->backend_decl, NULL_TREE);
+ if (TREE_THIS_VOLATILE (comp->backend_decl))
+ TREE_THIS_VOLATILE (caf_decl) = 1;
if (comp->ts.type == BT_CLASS)
{
caf_decl = gfc_class_data_get (caf_decl);
@@ -2755,8 +2757,10 @@ gfc_conv_component_ref (gfc_se * se, gfc_ref * ref)
else
se->class_vptr = NULL_TREE;
- tmp = fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
- decl, field, NULL_TREE);
+ tmp =build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
+ decl, field, NULL_TREE);
+ if (TREE_THIS_VOLATILE (field))
+ TREE_THIS_VOLATILE (tmp) = 1;
se->expr = tmp;
@@ -8792,8 +8796,10 @@ gfc_trans_structure_assign (tree dest, gfc_expr * expr,
bool init, bool coarray)
}
field = cm->backend_decl;
gcc_assert(field);
- tmp = fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
- dest, field, NULL_TREE);
+ tmp = build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
+ dest, field, NULL_TREE);
+ if (TREE_THIS_VOLATILE (field))
+ TREE_THIS_VOLATILE (tmp) = 1;
if (!c->expr)
{
gfc_expr *e = gfc_get_null_expr (NULL);
but I did not include them as they have no effect on the testsuite.
The question is whether we instead want to amend build3 to
set TREE_THIS_VOLATILE automatically when the FIELD_DECL has
it set. 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
