On November 24, 2014 7:12:01 PM CET, Martin Jambor <mjam...@suse.cz> wrote: >On Sat, Nov 22, 2014 at 07:36:59PM +0100, Richard Biener wrote: >> On November 22, 2014 12:45:58 PM CET, Jakub Jelinek ><ja...@redhat.com> wrote: >> >On Sat, Nov 22, 2014 at 12:09:46PM +0100, Martin Jambor wrote: >> >> 2014-11-21 Martin Jambor <mjam...@suse.cz> >> >> >> >> PR ipa/63551 >> >> * ipa-inline-analysis.c (evaluate_conditions_for_known_args): >> >Convert >> >> value of the argument to the type of the value in the condition. >> >> >> >> testsuite/ >> >> * gcc.dg/ipa/pr63551.c: New test. >> >> >> >> >> >> Index: src/gcc/ipa-inline-analysis.c >> >> >=================================================================== >> >> --- src.orig/gcc/ipa-inline-analysis.c >> >> +++ src/gcc/ipa-inline-analysis.c >> >> @@ -880,7 +880,10 @@ evaluate_conditions_for_known_args (stru >> >> } >> >> if (c->code == IS_NOT_CONSTANT || c->code == CHANGED) >> >> continue; >> >> - res = fold_binary_to_constant (c->code, boolean_type_node, >> >val, c->val); >> >> + val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (c->val), >val); >> > >> >VCE should only be used if the sizes of the types are the same. >> >Is that always the case here? >> >> I hope so. But I also think it will simply not fold otherwise? >> > >Unfortunately, neither is really the case. I have modified the >testcase so that the union view_converts an unsigned long to a >structure of two signed shorts and sure enough, the code ended up >folding a VIEW_CONVERT_EXPR of (unsigned long) -1 to signed short and >did that successfully, getting the value of signed short -1. > >Should I add an extra check to make sure the type sizes match?
I would rather say you need to figure out how you end up not rejecting this during propagation. I suppose only the low part will be handled correctly (thus it will fail with a less uniform value either on big or on little-endian). I don't know the IPA code good enough to tell whether you need a size check or whether that would be enough. Sure it Is safer than no size check and I suppose fixing this even more can be done as follow-up. Thus the VIEW_CONVERT patch is still OK. Richard. > >Thanks, > >Martin
