On Fri, 21 May 2021, Martin Jambor wrote: > Hi, > > On Mon, May 17 2021, Eric Botcazou wrote: > >> sorry for breaking Ada over the weekend, however... > > > > No problem. > > > >> None of the non-ACATS tests fail for me without doing a bootstrap, but I > >> did manage to reproduce this one (by the way, there really should be a > >> documentation on how to run ACATS tests manually, I should not need to > >> spend an hour and half figuring it out). > > > > https://gcc.gnu.org/wiki/DebuggingGCC > > I missed that. I'll try to put the string ACATS there so that people > can search for it. > > > > >> The problem is that before (early) SRA, there is a TREE_READONLY decl > >> that is being written to and my patch eliminated those writes. > >> Specifically, I see: > >> > >> <bb 183> : > >> var_ara_5D.5012.FD.4868[1]{lb: 1 sz: 4}[1]{lb: 1 sz: 1} = _877; > >> _880 = report.ident_bool (1); > >> > >> <bb 184> : > >> var_ara_5D.5012.FD.4868[1]{lb: 1 sz: 4}[2]{lb: 1 sz: 1} = _880; > >> _883 = report.ident_bool (1); > >> > >> <bb 185> : > >> var_ara_5D.5012.FD.4868[1]{lb: 1 sz: 4}[3]{lb: 1 sz: 1} = _883; > >> _886 = report.ident_bool (1); > >> > >> <bb 186> : > >> var_ara_5D.5012.FD.4868[1]{lb: 1 sz: 4}[4]{lb: 1 sz: 1} = _886; > >> _889 = report.ident_bool (1); > >> > >> [...and many more.] Note that this is an -fdump-tree-all-uid dump, the > >> DECL that is being assigned to has DECL_UID 5012 and when I dump it: > >> > >> DECL_UID of racc->base is: 5012 > >> print_node (dump_file, "", racc->base, 0): > >> <var_decl 0x7fc249fbc5a0 var_ara_5 > >> type <record_type 0x7fc249faf690 c41325a__p__p__obj_ara_5___PAD > >> sizes-gimplified type_5 TI size <integer_cst 0x7fc24b13dc00 constant 128> > >> unit-size <integer_cst 0x7fc24b13dc18 constant 16> > >> align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type > >> 0x7fc249faf690 fields <field_decl 0x7fc249fb02f8 F type <array_type > >> 0x7fc249faf1f8 c41325a__p__array_5> decl_3 TI c41325a.adb:57:11 > >> size <integer_cst 0x7fc24b13dc00 constant 128> > >> unit-size <integer_cst 0x7fc24b13dc18 constant 16> > >> align:8 warn_if_not_align:0 offset_align 128 > >> offset <integer_cst 0x7fc24b13dbe8 constant 0> > >> bit-offset <integer_cst 0x7fc24b13dc30 constant 0> context > >> <record_type 0x7fc249faf690 c41325a__p__p__obj_ara_5___PAD>> context > >> <function_decl 0x7fc249f89d00 c41325a> Ada size <integer_cst 0x7fc24b13dc00 > >> constant 128> > >> pointer_to_this <pointer_type 0x7fc249d4e7e0> chain <type_decl > >> 0x7fc249fb0390 c41325a__p__p__obj_ara_5___PAD>> --> readonly TI > >> c41325a.adb:70:6 > >> size <integer_cst 0x7fc24b13dc00 type <integer_type 0x7fc24b1520a8 > >> bitsizetype> constant 128> unit-size <integer_cst 0x7fc24b13dc18 type > >> <integer_type 0x7fc24b152000 sizetype> constant 16> align:64 > >> warn_if_not_align:0 context <function_decl 0x7fc249f89d00 c41325a> chain > >> <var_decl 0x7fc249fbc630 var_ara_6>> > >> > >> I can see that base is TREE_READONLY. > >> > >> Am I right that this is a bug happening at some point earlier in the Ada > >> compiler? > > > > Yes, in the gimplifier apparently, so try with: > > > > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > > index e790f08b23f..52cef6f8bff 100644 > > --- a/gcc/gimplify.c > > +++ b/gcc/gimplify.c > > @@ -1822,6 +1822,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p) > > if (!TREE_STATIC (decl)) > > { > > DECL_INITIAL (decl) = NULL_TREE; > > + TREE_READONLY (decl) = 0; > > init = build2 (INIT_EXPR, void_type_node, decl, init); > > gimplify_and_add (init, seq_p); > > ggc_free (init); > > The problem with this patch is that it causes: > > FAIL: gnat.dg/opt94.adb scan-tree-dump-times optimized "worker" 1
But isn't it still a correct fix? That said, I wonder how far we would get with verifying that a TREE_READONLY (auto-)var is never stored to ... > which is exactly the testcase from the commit which caused the bug I am > trying to address. > > I have given up attempting to clean IL we get from Ada testcases from > writes to TREE_READONLY decls, at least for now. I spent a bigger part > of today trying to handle those cases gracefully in SRA but I still > cannot make it work, always some Ada testcase breaks. I'll try again > next week. So if the above doesn't work then I'd try to simply disqualifying TREE_READONLY vars for SRA if they would be ever written to. Richard. > Martin > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
