https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91468
Martin Jambor <jamborm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |kugan.vivekanandarajah@lina
| |ro.org
Assignee|unassigned at gcc dot gnu.org |jamborm at gcc dot
gnu.org
--- Comment #1 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Feng Xue from comment #0)
> Some might be a bug, and some might be redundant.
>
> ipa-prop.c:
>
> In function ipcp_modif_dom_walker::before_dom_children(),
>
> vce = false;
> t = rhs;
> while (handled_component_p (t))
> {
> /* V_C_E can do things like convert an array of integers to one
> bigger integer and similar things we do not handle below. */
> if (TREE_CODE (rhs) == VIEW_CONVERT_EXPR)
> {
> vce = true;
> break;
> }
> t = TREE_OPERAND (t, 0);
> }
> if (vce)
> continue;
>
> Should "rhs" in "if (TREE_CODE (rhs) == VIEW_CONVERT_EXPR)" be "t"?
>
Yes, of course.
>
> In function update_jump_functions_after_inlining(),
>
> if (dst->type == IPA_JF_ANCESTOR)
> {
> ......
>
> if (src->type == IPA_JF_PASS_THROUGH
> && src->value.pass_through.operation == NOP_EXPR)
> {
> ......
> }
> else if (src->type == IPA_JF_PASS_THROUGH
> && TREE_CODE_CLASS (src->value.pass_through.operation) ==
> tcc_unary)
> {
> dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> dst->value.ancestor.agg_preserved = false;
> }
> ......
> }
>
> If we suppose pass_through operation is "negate_expr" (while it is not a
> reasonable operation on pointer type), the code might be incorrect. It's
> better to specify expected unary operations here.
Kugan, you added this in 2016 and unfortunately I think it is wrong.
Are there any unary operations we could possibly want to handle?
In any event, the information that there was an arithmetic function in
the path of the parameter would be completely lost if the code ever
executed. (Which I don't think it ever does, I think it would take
crazy code that employs LTO to pass an integer to a pointer parameter
to trigger).
So I plan to remove the whole if.
>
>
> In function compute_complex_assign_jump_func(),
>
> case GIMPLE_UNARY_RHS:
> if (is_gimple_assign (stmt)
> && gimple_assign_rhs_class (stmt) == GIMPLE_UNARY_RHS
> && ! CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt)))
> ipa_set_jf_unary_pass_through (jfunc, index,
> gimple_assign_rhs_code (stmt));
>
> The condition "is_gimple_assign (stmt)
> && gimple_assign_rhs_class (stmt) == GIMPLE_UNARY_RHS" seems to
> be redundant, might be omit.
>
Right, this is a leftover from a cleanup.
>
> ipa-cp.c:
>
> In function merge_agg_lats_step(),
>
> if (**aglat && (**aglat)->offset == offset)
> {
> if ((**aglat)->size != val_size
> || ((**aglat)->next
> && (**aglat)->next->offset < offset + val_size))
> {
> set_agg_lats_to_bottom (dest_plats);
> return false;
> }
> gcc_checking_assert (!(**aglat)->next
> || (**aglat)->next->offset >= offset + val_size);
> return true;
> }
>
> The condition "|| ((**aglat)->next && (**aglat)->next->offset < offset +
> val_size))" seems to be always false, because the next item should not be
> overlapped with its prev, this is what merge_agg_lats_step() tries
> to ensure.
True, assuming that (**aglat)->size == val_size. But I will make the
checking assert a normal one just to be sure it stays that way :-)
Thanks for checking the code, I am testing a patch.
Martin
