On Tue, Nov 14, 2017 at 10:31 AM, Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> wrote: > On 3 November 2017 at 15:38, Richard Biener <richard.guent...@gmail.com> > wrote: >> On Fri, Nov 3, 2017 at 6:15 AM, Prathamesh Kulkarni >> <prathamesh.kulka...@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 ?
I don't think it is good to do that. If we can't get the correct type then we have to only support known-good operations. There's the ipa_node_params->descriptors array where the type should be attached to, no? So use TREE_TYPE (ipa_get_param (info, idx))? Other than that Martin should have a look as I'm not really familiar with this IPA API. Richard. > Patch is bootstrapped+tested on x86_64-unknown-linux-gnu. > > Thanks, > Prathamesh >> >> Thanks, >> Richard. >> >>> Thanks, >>> Prathamesh
