Re: PR82808

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 3 November 2017 at 15:38, Richard Biener <richard.guenther@gmail.com> wrote:
> On Fri, Nov 3, 2017 at 6:15 AM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>> Hi Martin,
>> As mentioned in PR, the issue here for propagating value of 'm' from
>> f_c1 to foo() is that the jump function operation is FLOAT_EXPR, and
>> the type of input param 'm' is int, so fold_unary() doesn't do the
>> conversion to real_type. The attached patch fixes that by calling
>> fold_convert if operation is FLOAT_EXPR / FIX_TRUNC_EXPR /
>> CONVERT_EXPR and converts it to the type of corresponding parameter in
>> callee.
>>
>> There are still two issues:
>> a) Using NOP_EXPR for early_exit in ipa_get_jf_pass_through_result.
>> I suppose we need to change to some other code to indicate that there
>> is no operation ?
>> b) Patch does not passing param_type from all callers.
>> I suppose we could fix these incrementally ?
>>
>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>> OK for trunk ?
>
> This doesn't look like a well designed fix.  Both fold_unary and fold_binary
> calls get a possibly bogus type and you single out only a few ops.
>
> Either _fully_ list a set of operations that are know to have matching
> input/output types or always require param_type to be non-NULL.
>
> For a) simply remove the special-casing and merge it with CONVERT_EXPR
> handling (however it will end up looking).
>
> Please don't use fold_convert, using fold_unary is fine.
Hi Richard,
Sorry for the delay. In the attached version, parm_type is made non
NULL in ipa_get_jf_pass_through_result().

ipa_get_jf_pass_through_result() is called from two
places - propagate_vals_across_pass_through() and ipa_value_from_jfunc().
However it appears ipa_value_from_jfunc() is called from multiple
functions and it's
hard to detect parm_type in the individual callers. So I have passed
correct parm_type from propagate_vals_across_pass_through(), and kept
the old behavior for ipa_value_from_jfunc().

Would it be OK to fix that incrementally ?
Patch is bootstrapped+tested on x86_64-unknown-linux-gnu.

Thanks,
Prathamesh
>
> Thanks,
> Richard.
>
>> Thanks,
>> Prathamesh

Attachment: pr82808-2.diff
Description: Text document