Re: PR78599

2016-12-02 Thread Prathamesh Kulkarni
On 3 December 2016 at 00:25, Martin Jambor  wrote:
> Hi,
>
> On Thu, Dec 01, 2016 at 01:43:16PM +0100, Richard Biener wrote:
>> On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni
>>  wrote:
>> > Hi,
>> > As mentioned in PR, the issue seems to be that in
>> > propagate_bits_accross_jump_functions(),
>> > ipa_get_type() returns record_type during WPA and hence we pass
>> > invalid precision to
>> > ipcp_bits_lattice::meet_with (value, mask, precision) which eventually
>> > leads to runtime error.
>> > The attached patch tries to fix that, by bailing out if type of param
>> > is not integral or pointer type.
>> > This happens for the edge from deque_test -> 
>> > _Z4copyIPd1BEvT_S2_T0_.isra.0/9.
>>
>> Feels more like a DECL_BY_REFERENCE mishandling and should be fixed 
>> elsewhere.
>
> That is indeed what is happening. Prathamesh, if you are going to save
> the type of arguments in the jump function, it should help you also
> with this issue.  I know it was me who suggested using the function
> type to get at them and am sorry, I did not realize there potential
> issues with promotions and by_reference passing.
>
> By the way, please be careful not to introduce code style violations,
> especially lines exceeding 80 characters and adding trailing
> whitespace (propagate_bits_accross_jump_function has a few instances
> of both), I'd suggest setting your editor to highlight them.
Oops sorry about that, I will pay more attention to formatting henceforth.
Using editor to highlight stray whitespace is indeed quite useful,
thanks for that suggestion.

Kugan has a patch for adding param type to jump function:
https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02732.html
Once that gets committed, I will send a patch to use jfunc->param_type in
propagate_bits_accross_jump_function().

Thanks,
Prathamesh
>
> Thanks,
>
> Martin
>


Re: PR78599

2016-12-02 Thread Martin Jambor
Hi,

On Thu, Dec 01, 2016 at 01:43:16PM +0100, Richard Biener wrote:
> On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni
>  wrote:
> > Hi,
> > As mentioned in PR, the issue seems to be that in
> > propagate_bits_accross_jump_functions(),
> > ipa_get_type() returns record_type during WPA and hence we pass
> > invalid precision to
> > ipcp_bits_lattice::meet_with (value, mask, precision) which eventually
> > leads to runtime error.
> > The attached patch tries to fix that, by bailing out if type of param
> > is not integral or pointer type.
> > This happens for the edge from deque_test -> 
> > _Z4copyIPd1BEvT_S2_T0_.isra.0/9.
> 
> Feels more like a DECL_BY_REFERENCE mishandling and should be fixed elsewhere.

That is indeed what is happening. Prathamesh, if you are going to save
the type of arguments in the jump function, it should help you also
with this issue.  I know it was me who suggested using the function
type to get at them and am sorry, I did not realize there potential
issues with promotions and by_reference passing.

By the way, please be careful not to introduce code style violations,
especially lines exceeding 80 characters and adding trailing
whitespace (propagate_bits_accross_jump_function has a few instances
of both), I'd suggest setting your editor to highlight them.

Thanks,

Martin



Re: PR78599

2016-12-01 Thread Richard Biener
On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni
 wrote:
> Hi,
> As mentioned in PR, the issue seems to be that in
> propagate_bits_accross_jump_functions(),
> ipa_get_type() returns record_type during WPA and hence we pass
> invalid precision to
> ipcp_bits_lattice::meet_with (value, mask, precision) which eventually
> leads to runtime error.
> The attached patch tries to fix that, by bailing out if type of param
> is not integral or pointer type.
> This happens for the edge from deque_test -> _Z4copyIPd1BEvT_S2_T0_.isra.0/9.

Feels more like a DECL_BY_REFERENCE mishandling and should be fixed elsewhere.

> However I am not sure how ipcp_bits_lattice::meet_with (value, mask,
> precision) gets called for this case. In
> ipa_compute_jump_functions_for_edge(), we set jfunc->bits.known to
> true only
> if parm's type satisfies INTEGRAL_TYPE_P or POINTER_TYPE_P.
> And ipcp_bits_lattice::meet_with (value, mask, precision) is called
> only if jfunc->bits.known
> is set to true. So I suppose it shouldn't really happen that
> ipcp_bits_lattice::meet_with(value, mask, precision) gets called when
> callee parameter's type is record_type, since the corresponding
> argument's type would also need to be record_type and
> jfunc->bits.known would be set to false.
>
> Without -flto, parm_type is reference_type so that satisfies POINTER_TYPE_P,
> but with -flto it's appearing to be record_type. Is this possibly the
> same issue of TYPE_ARG_TYPES returning bogus types during WPA ?
>
> I verified the attached patch fixes the runtime error with ubsan-built gcc.
> Bootstrap+tested on x86_64-unknown-linux-gnu.
> Cross-tested on arm*-*-*, aarch64*-*-*.
> LTO bootstrap on x86_64-unknown-linux-gnu in progress.
> Is it OK to commit if it succeeds ?
>
> Thanks,
> Prathamesh