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: PR82808


On Tue, Nov 14, 2017 at 10:31 AM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> 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 ?

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


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