This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix devirtualization ICE (PR tree-optimization/59622)


> On Mon, Dec 30, 2013 at 10:14:16PM +0100, Jan Hubicka wrote:
> > this is C version
> > static int (*p) (int a) = (int (*)(int))__builtin_unreachable;
> > main()
> > {
> >   return p(1);
> > }
> 
> Well, that testcase is invalid, as you are calling the function (builtin)
> through incompatible function pointer, so all we care there is not ICEing.
> Not to mention that the above testcase will not link with -O0.
> Plus it is not fold_stmt that folds the above.

Hmm, I would not care much about validity, but indeed the fact that fold_stmt is
not doing this conversion makes me feel better ;)

> where fold_stmt changes the OBJ_TYPE_REF based call into B::foo () which
> is noreturn.  The bad thing on the folding gimple_fold_call did before the
> patch I've posted is mainly that it kept the lhs there even when
> __builtin_unreachable has void return type, and very ugly but not fatal
> is the passing of extra arguments to the builtin that doesn't take any.

Yep, I basically originally added the code for sanity checking (watned to break
programs where GCC fails to identify the correct virtual method) and then
found it is useful in valid testcases because of code specialization.

cgraph_redirect_edge_call_stmt_to_callee can be also used to redirect call
to UNREACHABLE (i.e. via walk_polymorphic_call_targets from cgraphunit.c)
So probably we want to add there a logic updating both return value and
killing the unused arguments (as we may do in gimple fold, I think people
may wire in "abort" call or so into a function pointer taking with incompatible
decl)

Honza
> 
> 	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]