Re: [PATCH] Updated automated patch (was Re: [PATCH 3/6] Automated part of conversion of gimple types to use C++ inheritance)

[Andrew MacLeod <amacleod@redhat.com>]

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.

Andrew