Handle OBJ_TYPE_REF in FRE

On Thu, 3 Dec 2015, Jan Hubicka wrote:

> > >may lead to wrong code.
> > 
> > Can you try generating a testcase?
> >  Because with equal vptr and voffset I can't see how that can happen 
> > unless some pass extracts information from the pointer types without 
> > sanity checking with the pointers and offsets.
> 
> I am not sure I can get a wrong code with current mainline, because for now you
> only substitute for the lookup done for speculative devirt and if we wrongly
> predict the thing to be __builtin_unreachable, we dispatch to usual virtual
> call.  Once you get movement on calls it will be easier to do.
> 
> OBJ_TYPE_REF is a wrapper around OBJ_TYPE_EXPR adding three extra parameters:
>  - OBJ_TYPE_REF_OBJECT
>  - OBJ_TYPE_REF_TOKEN
>  - obj_type_ref_class which is computed from TREE_TYPE (obj_type_ref) itself.
> 
> While two OBJ_TYPE_REFS with equivalent OBJ_TYPE_EXPR are kind of same
> expressions, they are optimized differently (just as if they was in different
> alias set).  For that reason you need to match the type of obj_type_ref_class
> because that one is not matched by usless_type_conversion (it is a pointer to
> method of corresponding class type we are looking up)
> 
> The following testcase:
> struct foo {virtual void bar(void) __attribute__ ((const));};
> struct foobar {virtual void bar(void) __attribute__ ((const));};
> void
> dojob(void *ptr, int t)
> {
>   if (t)
>    ((struct foo*)ptr)->bar();
>   else
>    ((struct foobar*)ptr)->bar();
> }
> 
> produces
> void dojob(void*, int) (void * ptr, int t)
> {
>   int (*__vtbl_ptr_type) () * _5;
>   int (*__vtbl_ptr_type) () _6;
>   int (*__vtbl_ptr_type) () * _8;
>   int (*__vtbl_ptr_type) () _9;
> 
>   <bb 2>:
>   if (t_2(D) != 0)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
> 
>   <bb 3>:
>   _5 = MEM[(struct foo *)ptr_4(D)]._vptr.foo;
>   _6 = *_5;
>   OBJ_TYPE_REF(_6;(struct foo)ptr_4(D)->0) (ptr_4(D));
>   goto <bb 5>;
> 
>   <bb 4>:
>   _8 = MEM[(struct foobar *)ptr_4(D)]._vptr.foobar;
>   _9 = *_8;
>   OBJ_TYPE_REF(_9;(struct foobar)ptr_4(D)->0) (ptr_4(D));
> 
>   <bb 5>:
>   return;
> 
> }
> 
> Now I would need to get some code movement done to get _5 and _6
> moved and unified with _8 and _9 that we currently don't do.  
> Still would feel safer if the equivalence predicate also checked
> that the type is the same.

Indeed we don't do code hoisting yet.  Maybe one could trick PPRE
into doing it.

Note that for OBJ_TYPE_REFs in calls you probably should better use
gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway
(well, fntype will be the method-type, not pointer-to-method-type).

Not sure if you need OBJ_TYPE_REFs type in non-call contexts?

> > >Or do you just substitute the operands of OBJ_TYPE_REF? 
> > 
> > No, I value number them.  But yes, the type issue also crossed my 
> > mind.  Meanwhile testing revealed that I need to adjust 
> > gimple_expr_type to preserve the type of the obj-type-ref, otherwise 
> > the devirt machinery ICEs (receiving void *). That's also a reason we 
> > can't make obj-type-ref a ternary RHS.
> 
> Yep, type of OBJ_TYPE_REF matters...

See above.

> > 
> > >> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > >> 
> > >> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
> > >> with SSA names that have the same value - not sure if that would
> > >> be desired generally (does the devirt machinery cope with that?).
> > >
> > >This should work fine.
> > 
> > OK. So with that substituting the direct call later should work as well.
> Great!

For the above reasons I'm defering all this to stage1.

Below is the patch that actually passed bootstrap & regtest on 
x86_64-unknown-linux-gnu, just in case you want to play with it.
It doesn't do the propagation into calls yet though, the following
does (untested)

Handle OBJ_TYPE_REF in FRE

Commit Message

Comments

Patch