Hi, Richi
On 04/21, Richard Biener wrote:
> On Mon, Apr 20, 2020 at 11:45 PM Giuliano Belinassi
> <giuliano.belina...@usp.br> wrote:
> >
> > Hi. Sorry for the late reply.
> >
> > On 03/02, Richard Biener wrote:
> > > On Thu, Feb 27, 2020 at 6:56 PM Giuliano Belinassi
> > > <giuliano.belina...@usp.br> wrote:
> > > >
> > > > Hi, all.
> > > >
> > > > I am tying to fix an issue with a global variable in the parallel gcc
> > > > project. For this, I am trying to move some global variables from
> > > > tree-ssa-operands to struct function. One of this variable is a
> > > > vec<tree*> type, and gengtype doesn't look so happy with it.
> > >
> > > I think the solution for this is to not move it to struct function
> > > but instead have it local to function scope - the state is per
> > > GIMPLE stmt we process and abstracting what is
> > > cleaned up in cleanup_build_arrays() into a class we can
> > > instantiate in the two callers is most appropriate. In theory
> > > all the functions that access the state could move into the
> > > class as methods as well but you can pass down the state
> > > everywhere needed as well.
> >
> > I implemented this strategy, but the issue remains. Therefore, the
> > cause of it must be something else.
>
> Btw, it would be nice to push those changes as cleanups during
> next stage1.
>
> > Just to contextualize, in [1], I also implemented parallelism in
> > ParallelGcc using a pipeline method for testing, where I split the set
> > of GIMPLE Intra Procedural Optimizations into multiple sets, and assign
> > each set to a thread. Then, the function passes through this pipeline.
> >
> > Now, I am trying to make this version pass the testsuite. There is a test
> > in particular ('gcc.dg/20031102-1.c') that I am having difficulties
> > finding the cause of the issue.
> >
> > if I run:
> >
> > /tmp/gcc10_parallel/usr/local/bin/gcc --param=num-threads=2 -O2 -c
> > 20031102-1.c
> >
> > The crash message is:
> >
> > ```
> > <var_decl 0x7f443363c720 .MEM
> > type <void_type 0x7f44334fb000 void VOID
> > align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> > 0x7f44334fb000
> > pointer_to_this <pointer_type 0x7f44334fb0b0>>
> > used static ignored external VOID <built-in>:0:0
> > align:8 warn_if_not_align:0>
> >
> > In function ‘main’:
> > cc1: error: virtual use of statement not up to date
> > # VUSE <.MEM_1(D)>
> > _2 = FooBar ();
> > during GIMPLE pass: walloca
>
> ^^^
>
> so this pass isn't supposed to change anything which either means you're
> missing some global state in the verify_ssa checker. Notably the actual
> verification error triggering checks the underlying .MEM symbol (a VAR_DECL).
> Since your message above only shows one var_decl build_vuse must be
> NULL somehow.
>
> Now it could be that the pass_local_pure_const (the late one) changes 'FooBar'
> to be const which means it wouldn't get a virtual operand anymore. Looking
> at the testcase that's likely the issue here.
>
> That's a "tough one" and would ask for the const/pure-ness of call stmts
> to be encoded in the call stmt itself (this issue is also one reason for
> the fixup_cfg passes we have). So instead of looking at the decl we'd
> track this via a gimple_call_flags () flag and update that from the decl
> at known points (for example when updating SSA operands (sic!) but
> exactly not when just verifying them).
>
> So for your branch try adding a verifying_p member to the class
> and when verifying instead of
>
> /* If aliases have been computed already, add VDEF or VUSE
> operands for all the symbols that have been found to be
> call-clobbered. */
> if (!(call_flags & ECF_NOVOPS))
> {
> /* A 'pure' or a 'const' function never call-clobbers anything. */
> if (!(call_flags & (ECF_PURE | ECF_CONST)))
> add_virtual_operand (fn, stmt, opf_def);
> else if (!(call_flags & ECF_CONST))
> add_virtual_operand (fn, stmt, opf_use);
> }
>
> rely on existing vuse/vdef like
>
> if (verifying_p)
> {
> /* ??? Track const/pure/novop-ness in gimple call flags. */
> if (gimple_vdef (stmt))
> add_virtual_operand (...);
> else if (gimple_vuse (stmt))
> add_virtual_operand (...);
> return;
> }
>
> and call it a day ;)
That indeed worked! Thank you. This one in particular was really tough!
I will prepare a patch about these changes to trunk ready for stage1.
There are some unused stuff that I found here that is nice to have it
cleaned.
I am just curious about how it was working before these changes, once it
seems not to be a race condition. Or probaly there is a race condition
lost somewhere that was triggering it?
Thank you,
Giuliano.
>
> > cc1: internal compiler error: verify_ssa failed
> > 0xfdb8fe verify_ssa(bool, bool)
> > ../../gcc/gcc/tree-ssa.c:1208
> > 0xcd3d08 execute_function_todo
> > ../../gcc/gcc/passes.c:2017
> > 0xcd49f2 execute_todo
> > ../../gcc/gcc/passes.c:2064
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See <https://gcc.gnu.org/bugs/> for instructions.
> > ```
> >
> > Which is triggered by tree-ssa-operands.c:1066 in this branch, checking
> > if build_vuse != use. Interestingly, this crash does not happens if:
> >
> > 1 - I set the number of threads to 1.
> > 2 - I set the optimization level to O0, O1 or O3.
> > 3 - I disable O2, but enable all flags enabled by O2
> > (gcc -O2 -Q --help=optimizer).
> > 4 - I left the first 115 passes in the same thread with a parameter I
> > implmemented (--param=num-threads=2 --param=break=116). Any value
> > smaller that this causes the issue.
> >
> > The crash is also consistent, which means that it happens 100% of time.
> >
> > Any light concerning this issue is welcome. :)
> >
> > If anyone want to reproduce the issue, you can clone this branch, then
> > compile
> > with --disable-bootstrap --enable-language=c, and install it like trunk's
> > GCC.
> >
> > [1] - https://gitlab.com/flusp/gcc/-/tree/giulianob_parallel_pipeline
> >
> >
> > Thank you,
> > Giuliano.
> >
> > >
> > > Richard.
> > >
> > > > In this context, I am trying to add support to vec<tree*> to gengtype.
> > > > Therefore, I first fixed a problem where gengtype couldn't find the
> > > > tree union by:
> > > >
> > > > diff --git a/gcc/gengtype.c b/gcc/gengtype.c
> > > > index 53317337cf8..6f4c77020ea 100644
> > > > --- a/gcc/gengtype.c
> > > > +++ b/gcc/gengtype.c
> > > > @@ -638,7 +638,10 @@ create_user_defined_type (const char *type_name,
> > > > struct fil
> > > > eloc *pos)
> > > > /* Strip off the first '*' character (and any subsequent
> > > > text). */
> > > > *(field_name + offset_to_star) = '\0';
> > > >
> > > > - arg_type = find_structure (field_name, TYPE_STRUCT);
> > > > + arg_type = resolve_typedef (field_name, pos);
> > > > + if (!arg_type)
> > > > + arg_type = find_structure (field_name, TYPE_STRUCT);
> > > > +
> > > > arg_type = create_pointer (arg_type);
> > > > }
> > > > else
> > > >
> > > > After this patch, gengtype seems to correctly detect vec<tree*> types,
> > > > but then I face linking issues. At first, the compiler could not find
> > > > gt_ggc_mx (vec<T> *v) and gt_pch_mx (vec<T> *v), therefore I implemented
> > > > them both in gcc/vec.h:
> > > >
> > > > diff --git a/gcc/vec.h b/gcc/vec.h
> > > > index 091056b37bc..dfa744b684e 100644
> > > > --- a/gcc/vec.h
> > > > +++ b/gcc/vec.h
> > > > @@ -1306,6 +1306,15 @@ vec<T, A, vl_embed>::quick_grow_cleared
> > > > (unsigned len)
> > > > vec_default_construct (address () + oldlen, growby);
> > > > }
> > > >
> > > > +template<typename T>
> > > > +void
> > > > +gt_ggc_mx (vec<T> *v)
> > > > +{
> > > > + extern void gt_ggc_mx (T &);
> > > > + for (unsigned i = 0; i < v->length (); i++)
> > > > + gt_ggc_mx ((*v)[i]);
> > > > +}
> > > > +
> > > > /* Garbage collection support for vec<T, A, vl_embed>. */
> > > >
> > > > template<typename T>
> > > > @@ -1328,6 +1337,15 @@ gt_ggc_mx (vec<T, va_gc_atomic, vl_embed> *v
> > > > ATTRIBUTE_UNUSED)
> > > >
> > > > /* PCH support for vec<T, A, vl_embed>. */
> > > >
> > > > +template<typename T>
> > > > +void
> > > > +gt_pch_nx (vec<T> *v)
> > > > +{
> > > > + extern void gt_pch_nx (T &);
> > > > + for (unsigned i = 0; i < v->length (); i++)
> > > > + gt_pch_nx ((*v)[i]);
> > > > +}
> > > > +
> > > > template<typename T, typename A>
> > > > void
> > > > gt_pch_nx (vec<T, A, vl_embed> *v)
> > > > @@ -1337,6 +1355,14 @@ gt_pch_nx (vec<T, A, vl_embed> *v)
> > > > gt_pch_nx ((*v)[i]);
> > > > }
> > > >
> > > > +template<typename T>
> > > > +void
> > > > +gt_pch_nx (vec<T *> *v, gt_pointer_operator op, void *cookie)
> > > > +{
> > > > + for (unsigned i = 0; i < v->length (); i++)
> > > > + op (&((*v)[i]), cookie);
> > > > +}
> > > > +
> > > > template<typename T, typename A>
> > > > void
> > > > gt_pch_nx (vec<T *, A, vl_embed> *v, gt_pointer_operator op, void
> > > > *cookie)
> > > > @@ -1354,6 +1380,15 @@ gt_pch_nx (vec<T, A, vl_embed> *v,
> > > > gt_pointer_operator op, void *cookie)
> > > > gt_pch_nx (&((*v)[i]), op, cookie);
> > > > }
> > > >
> > > > +template<typename T>
> > > > +void
> > > > +gt_pch_nx (vec<T> *v, gt_pointer_operator op, void *cookie)
> > > > +{
> > > > + extern void gt_pch_nx (T *, gt_pointer_operator, void *);
> > > > + for (unsigned i = 0; i < v->length (); i++)
> > > > + gt_pch_nx (&((*v)[i]), op, cookie);
> > > > +}
> > > > +
> > > >
> > > > After that, I get linking errors because the linker can not find
> > > > gt_ggc_mx (tree *&x) nor void gt_pch_nx (tree *&x). The thing
> > > > is: it doesn't matter where I implement them, or if I declare
> > > > them inline. I always get a linking error one way or another.
> > > >
> > > > Therefore, what should I do to correctly implement the support
> > > > for vec<tree*> types?
> > > >
> > > > Thank you,
> > > > Giuliano.
