On Tue, Apr 21, 2020 at 5:56 PM Giuliano Belinassi
<giuliano.belina...@usp.br> wrote:
>
> 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?
Well it needs appropriate timing to catch the issue ... for non-threaded
builds the fixup_cfg pass fixes this up.
Richard.
> 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.
