This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Updated automated patch (was Re: [PATCH 3/6] Automated part of conversion of gimple types to use C++ inheritance)
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Andrew MacLeod <amacleod at redhat dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Jeff Law <law at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 21 Nov 2013 19:32:29 -0500
- Subject: Re: [PATCH] Updated automated patch (was Re: [PATCH 3/6] Automated part of conversion of gimple types to use C++ inheritance)
- Authentication-results: sourceware.org; auth=none
- References: <5271CBF9 dot 2070005 at redhat dot com> <1383236801-13234-1-git-send-email-dmalcolm at redhat dot com> <1383236801-13234-4-git-send-email-dmalcolm at redhat dot com> <5284806A dot 2050607 at redhat dot com> <1384806352 dot 11568 dot 80 dot camel at surprise> <20131121221933 dot GQ892 at tucnak dot redhat dot com> <528E8837 dot 5080300 at redhat dot com> <20131121224257 dot GR892 at tucnak dot redhat dot com> <528E9135 dot 7060408 at redhat dot com>
On Thu, 2013-11-21 at 18:03 -0500, Andrew MacLeod wrote:
> On 11/21/2013 05:42 PM, Jakub Jelinek wrote:
> > On Thu, Nov 21, 2013 at 03:24:55PM -0700, Jeff Law wrote:
> >> On 11/21/13 15:19, Jakub Jelinek wrote:
> >>> On Mon, Nov 18, 2013 at 03:25:52PM -0500, David Malcolm wrote:
> >>>>> So is there some reason the GIMPLE_CHECK was left in here rather than
> >>>>> doing the downcasting? This happens in other places.
> >>> Note that the changes removed tons of checks that IMHO were desirable.
> >>> The as_a that replaced those checks e.g. allows 3 different gimple codes,
> >>> while previously only one was allowed, this is both more expensive for
> >>> --enable-checking=yes, and allows one to use inline wrappers e.g.
> >>> gimple_omp_parallel_something on GIMPLE_OMP_TASK etc.
> >> Can you give a couple examples, please?
> > I mean e.g.
> > gimple_omp_parallel_{,set_}{clauses,child_fn,data_arg}{,_ptr}
> > gimple_omp_taskreg_{,set_}{clauses,child_fn,data_arg}{,_ptr}
> > gimple_omp_target_{,set_}{clauses,child_fn,data_arg}{,_ptr}
> > gimple_omp_teams_{,set_}clauses{,_ptr}
> > gimple_omp_return_{,set_}lhs{,_ptr}
> > gimple_omp_atomic_store_{,set_}val{,_ptr}
> > gimple_resx_{,set_}region
> > gimple_eh_dispatch_{,set_}region
> >
> > Jakub
> Why does is_a_helper <gimple_statement_omp_parallel>::test allow
> anything other than a GIMPLE_OMP_PARALLEL..? That seems wrong to me.
> should just be the one check.
>
> gimple_omp_taskreg and other routines "sharing" that helper should have
> their own helper and only check the one code.. thats is whole point to
> remain at least codegen neutral in these cases and provide correct
> checking. The fact that they may happen to share the same underlying
> structure is irrelevant.
>
> I also think this is wrong.
This was a bug in my script. Sorry. Working on a fix.