Re: [patch] Fix PR lto/50492

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Oct 7, 2011 at 12:37 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is another LTO bootstrap failure with Ada enabled. ?There are 2 problems:
>
> ?1. dwarf2out.c thinks that it is processing pure C++ code for gnat1 so it is
> invoking some C++-specific code paths that aren't compatible with Ada. ?Fixed
> by building TRANSLATION_UNIT_DECL nodes in Ada as well.
>
> ?2. the recent change:
>
> ? ? ? ?* gimple.c (canonicalize_cond_expr_cond): Handle cast from
> ? ? ? ?boolean-type.
>
> breaks cross-language inlining. ?More specifically, it removes a necessary cast
> between an Ada boolean type and a C boolean type (hence a GIMPLE verification
> failure later) when __gnat_is_absolute_path is inlined into Is_Absolute_Path:
>
> ? extern int __gnat_is_absolute_path (char *, int);
>
> ? function Is_Absolute_Path (Name : String) return Boolean is
> ? ? ?function Is_Absolute_Path
> ? ? ? ?(Name ? : Address;
> ? ? ? ? Length : Integer) return Integer;
> ? ? ?pragma Import (C, Is_Absolute_Path, "__gnat_is_absolute_path");
> ? begin
> ? ? ?return Is_Absolute_Path (Name'Address, Name'Length) /= 0;
> ? end Is_Absolute_Path;
>
> We start with:
>
> ?Ada_Boolean_Var = (Integer_Var != 0);
>
> and forwprop figures out that (Integer_Var != 0) is equivalent to iftmp.xxx.
> In combine_cond_expr_cond, the constant folder correctly turns this into:
>
> ?Ada_Boolean_Var = (Ada_Boolean_Type) iftmp.xxx;
>
> but the call to canonicalize_cond_expr_cond wrongly removes the cast.
>
> Note that changing Ada's boolean_type_node to a C-compatible type doesn't fix
> the problem as the boolean return type of Is_Absolute_Path has to be an Ada
> boolean type in any case, so a cast will be always necessary here.

Hm, but canonicalize_cond_expr_cond is supposed to produce a
tree that is suitable for a condition in a GIMPLE_COND, or a
COND_EXPR.  So the issue is that it is used for a tcc_comparison
on a assignment RHS?  Wouldn't the "proper" fix then be to verify
that the result from forward_propagate_into_comparison_1 in forwprop
is a suitable
replacement of the assign rhs in forward_propagate_into_comparison?

Thus,

Index: gcc/tree-ssa-forwprop.c
===================================================================
--- gcc/tree-ssa-forwprop.c     (revision 179647)
+++ gcc/tree-ssa-forwprop.c     (working copy)
@@ -474,7 +474,9 @@ forward_propagate_into_comparison (gimpl
                                             TREE_TYPE
                                               (gimple_assign_lhs (stmt)),
                                             rhs1, rhs2);
-  if (tmp)
+  if (tmp
+      && useless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)),
+                                   TREE_TYPE (tmp)))
     {
       gimple_assign_set_rhs_from_tree (gsi, tmp);
       fold_stmt (gsi);

?

> Since reverting the canonicalize_cond_expr_cond change apparently has no effect
> (no regressions detected on x86 and x86-64), the proposed fix is just that.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for mainline?
>
> 2011-10-07 ?Eric Botcazou ?<ebotcazou@adacore.com>
>
> ? ? ? ?PR lto/50492
> ? ? ? ?* gimple.c (canonicalize_cond_expr_cond): Revert 2011-08-02 change.
> ada/
> ? ? ? ?* gcc-interface/gigi.h (gnat_pushdecl): Adjust comment.
> ? ? ? ?* gcc-interface/utils.c (global_context): New variable.
> ? ? ? ?(gnat_pushdecl): Initialize it and set it as the DECL_CONTEXT of DECLs
> ? ? ? ?that are either public external or at top level. ?Use "No" macro.
> ? ? ? ?(end_subprog_body): Call decl_function_context.
> ? ? ? ?(rest_of_subprog_body_compilation): Likewise.
>
>
> --
> Eric Botcazou
>