public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef
@ 2022-01-31 10:09 ebotcazou at gcc dot gnu.org
  2022-01-31 10:12 ` [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref pinskia at gcc dot gnu.org
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-01-31 10:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

            Bug ID: 104303
           Summary: [12 regression] gnatmake is miscompiled by IPA/modef
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ebotcazou at gcc dot gnu.org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

Created attachment 52315
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52315&action=edit
Cocatenated  testcase

The build is done at -O2 but the miscompilation already occurs at -O1 since:

2021-12-14  Jan Hubicka  <hubicka@ucw.cz>

        PR ipa/103585
        * ipa-modref-tree.c (modref_access_node::range_info_useful_p): Handle
        MODREF_GLOBAL_MEMORY_PARM.
        (modref_access_node::dump): Likewise.
        (modref_access_node::get_call_arg): Likewise.
        * ipa-modref-tree.h (enum modref_special_parms): Add
        MODREF_GLOBAL_MEMORY_PARM.
        (modref_access_node::useful_for_kill): Handle
        MODREF_GLOBAL_MEMORY_PARM.
        (modref:tree::merge): Add promote_unknown_to_global.
        * ipa-modref.c (verify_arg):New function.
        (may_access_nonescaping_parm_p): New function.
        (modref_access_analysis::record_global_memory_load): New member
        function.
        (modref_access_analysis::record_global_memory_store): Likewise.
        (modref_access_analysis::process_fnspec): Distingush global and local
        memory.
        (modref_access_analysis::analyze_call): Likewise.
        * tree-ssa-alias.c (ref_may_access_global_memory_p): New function.
        (modref_may_conflict): Use it.

I have attached a testcase for the gnat.dg testsuite:
  - gnatchop concat5.txt
  - gnatmake concat5 -O && ./concat5
  - gcc -c concat5_pkg1.adb -O -fdump-tree-dse2-details yields:

;; Function concat5_pkg1.scan (concat5_pkg1__scan, funcdef_no=3, decl_uid=4922,
cgraph_uid=2, symbol_order=1)

ipa-modref: call stmt concat5_pkg1.make_failed (D.5010);
ipa-modref: call to Concat5_Pkg1.Make_Failed/0 does not use ref:
D.5010.P_BOUNDS alias sets: 2->4
ipa-modref: call stmt concat5_pkg1.make_failed (D.5010);
ipa-modref: call to Concat5_Pkg1.Make_Failed/0 does not use ref: D.5010.P_ARRAY
alias sets: 2->6
ipa-modref: call stmt concat5_pkg1.make_failed (D.5010);
ipa-modref: call to Concat5_Pkg1.Make_Failed/0 does not use ref:
VIEW_CONVERT_EXPR<character[1:23]>((*S9b.6_22)[_12 ...]{lb: 1 sz: 1}) alias
sets: 8->8
  Deleted dead store: VIEW_CONVERT_EXPR<character[1:23]>((*S9b.6_22)[_12
...]{lb: 1 sz: 1}) = " should start with \'--\'";

  Deleted trivially dead stmt: _12 = (sizetype) _11;

  Deleted trivially dead stmt: _11 = iftmp.5_14 + 8;

ipa-modref: call stmt concat5_pkg1.make_failed (D.5010);
ipa-modref: call to Concat5_Pkg1.Make_Failed/0 does not use ref: (*S9b.6_22)[1
...]{lb: 1 sz: 1} alias sets: 8->8
  Deleted dead store: (*S9b.6_22)[1 ...]{lb: 1 sz: 1} = "option ";

but the stores are definitively not dead here.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
@ 2022-01-31 10:12 ` pinskia at gcc dot gnu.org
  2022-01-31 10:21 ` hubicka at gcc dot gnu.org
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-01-31 10:12 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
  2022-01-31 10:12 ` [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref pinskia at gcc dot gnu.org
@ 2022-01-31 10:21 ` hubicka at gcc dot gnu.org
  2022-02-01  8:18 ` rguenth at gcc dot gnu.org
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: hubicka at gcc dot gnu.org @ 2022-01-31 10:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org
   Last reconfirmed|                            |2022-01-31
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Mine.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
  2022-01-31 10:12 ` [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref pinskia at gcc dot gnu.org
  2022-01-31 10:21 ` hubicka at gcc dot gnu.org
@ 2022-02-01  8:18 ` rguenth at gcc dot gnu.org
  2022-03-22 14:52 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-01  8:18 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-02-01  8:18 ` rguenth at gcc dot gnu.org
@ 2022-03-22 14:52 ` jakub at gcc dot gnu.org
  2022-03-31 12:51 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-22 14:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Honza, any progress on this?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-03-22 14:52 ` jakub at gcc dot gnu.org
@ 2022-03-31 12:51 ` rguenth at gcc dot gnu.org
  2022-03-31 13:12 ` hubicka at gcc dot gnu.org
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-03-31 12:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The store

D.5010.P_BOUNDS = &D.5011;

is

 <component_ref 0x7ffff4e951e0
    type <pointer_type 0x7ffff653b150
        type <record_type 0x7ffff653bf18 string___XUB type_4 DI
            size <integer_cst 0x7ffff6517c00 constant 64>
            unit-size <integer_cst 0x7ffff6517c18 constant 8>
            align:32 warn_if_not_align:0 symtab:0 alias-set 9 canonical-type
0x7ffff653bf18 fields <field_decl 0x7ffff6541260 LB0> Ada size <integer_cst
0x7ffff6517c00 64>
            pointer_to_this <pointer_type 0x7ffff653b150> chain <type_decl
0x7ffff6541428 string___XUB>>
        public unsigned DI size <integer_cst 0x7ffff6517c00 64> unit-size
<integer_cst 0x7ffff6517c18 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set 4 canonical-type
0x7ffff653b150>

    arg:0 <var_decl 0x7ffff4e9a090 D.5010
        type <record_type 0x7ffff653b348 string sizes-gimplified visited type_0
TI
            size <integer_cst 0x7ffff6517c48 constant 128>
            unit-size <integer_cst 0x7ffff6517c60 constant 16>
            align:64 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
0x7ffff653b348 fields <field_decl 0x7ffff6538ab0 P_ARRAY> context
<translation_unit_decl 0x7ffff6522168 concat5_pkg1.adb> unconstrained array
<unconstrained_array_type 0x7ffff653b540 string>
            pointer_to_this <pointer_type 0x7ffff4e9b498> chain <type_decl
0x7ffff6538c78 string>>
        used ignored TI concat5_pkg1.adb:15:5 size <integer_cst 0x7ffff6517c48
128> unit-size <integer_cst 0x7ffff6517c60 16>
        align:128 warn_if_not_align:0 context <function_decl 0x7ffff4e83900
concat5_pkg1__scan>
        chain <var_decl 0x7ffff4e9a120 D.5011 type <record_type 0x7ffff653bf18
string___XUB>
            addressable used ignored DI concat5_pkg1.adb:15:5 size <integer_cst
0x7ffff6517c00 64> unit-size <integer_cst 0x7ffff6517c18 8>
            align:32 warn_if_not_align:0 context <function_decl 0x7ffff4e83900
concat5_pkg1__scan> chain <var_decl 0x7ffff4e9a240 saved_stack.10>>>
    arg:1 <field_decl 0x7ffff6538b48 P_BOUNDS type <pointer_type
0x7ffff653b150>
        visited unsigned DI <built-in>:0:0 size <integer_cst 0x7ffff6517c00 64>
unit-size <integer_cst 0x7ffff6517c18 8>
        align:64 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff6517c30 constant 0> bit-offset <integer_cst
0x7ffff6517c00 64> context <record_type 0x7ffff653b348 string>>>

and the issue is that somehow the summary for

concat5_pkg1.make_failed (D.5010);

only has an base/ref/access node for MODREF_GLOBAL_MEMORY_PARM and
nothing else.  In .modref2 this function is just

void concat5_pkg1.make_failed (struct  s)
{
  struct string___XUB * s$P_BOUNDS;

  <bb 2> [local count: 1073741824]:
  concat5_pkg2.compare (s);
  return;

but we do

modref analyzing 'Concat5_Pkg1.Make_Failed/0' (ipa=0)
Past summary:
  loads:
    Every base
  stores:
    Every base
  Side effects
  Nondeterministic
  Global memory read
  Global memory written
 - Analyzing load: s
   - Read-only or local, ignoring.
 - Analyzing call:concat5_pkg2.compare (s);
 - Function availability <= AVAIL_INTERPOSABLE.
 - modref done with result: tracked.
  loads:
      Base 0: alias set 0
        Ref 0: alias set 0
          access: Base in global memory
  stores:
      Base 0: alias set 0
        Ref 0: alias set 0
          access: Base in global memory
  Side effects
  Nondeterministic
  Global memory read
  Global memory written

so it seems we fail to consider by-value escaping parameters.  Something
like the following, but it does not reproduce with that, so there must
be sth special with the Ada testcase.

struct X { int i; };

void foo (struct X);

static void __attribute__((noinline))
bar (struct X x)
{
  foo (x);
}

void baz ()
{
  struct X x;
  x.i = 1;
  bar (x);
}

Btw, if we disable modref2 the testcase works - so somehow dropping that we
load/store all bases wrecks things.

Honza?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-03-31 12:51 ` rguenth at gcc dot gnu.org
@ 2022-03-31 13:12 ` hubicka at gcc dot gnu.org
  2022-04-03 23:49 ` hubicka at gcc dot gnu.org
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: hubicka at gcc dot gnu.org @ 2022-03-31 13:12 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #4 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
What happens to the by-value parameter should get merged from  
concat5_pkg2.compare (s);
I will take a look now - sorry for not handling this bug earlier.

Honza

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-03-31 13:12 ` hubicka at gcc dot gnu.org
@ 2022-04-03 23:49 ` hubicka at gcc dot gnu.org
  2022-04-05 21:46 ` hubicka at gcc dot gnu.org
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: hubicka at gcc dot gnu.org @ 2022-04-03 23:49 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
So what modref see on calle is:
void concat5_pkg1.make_failed (struct  s)                                       
{                                                                               
  struct string___XUB * s$P_BOUNDS;                                             

  <bb 2> [local count: 1073741824]:                                             
  concat5_pkg2.compare (s);                                                     
  return;                                                                       

}                                                                               
and it believes that it is safe to only consider global memory accesses since
the parameter escapes because we have no EAF flags attached to
concat5_pkg1.make_failes nor to concat5_pkg1.compare.

Now alias sees:
ESCAPED, points-to non-local, points-to NULL, points-to vars: { D.5011 D.5040 }
(escaped)
....
  S9b.6_22 = .builtin_alloca_with_align (_5, 8);                                
  (*S9b.6_22)[1 ...]{lb: 1 sz: 1} = "option ";                                  
  _6 = MAX_EXPR <L7b_19, 7>;                                                    
  _7 = (sizetype) _6;                                                           
  _8 = _7 + 18446744073709551609;                                               
  _9 = s.P_ARRAY;                                                               
  _10 = &(*S9b.6_22)[8 ...]{lb: 1 sz: 1};                                       
  .builtin_memcpy (_10, _9, _8);                                                
  _11 = iftmp.5_14 + 8;                                                         
  _12 = (sizetype) _11;                                                         
  VIEW_CONVERT_EXPR<character[1:23]>((*S9b.6_22)[_12 ...]{lb: 1 sz: 1}) = "
should start with \'--\'";
  D.5010.P_ARRAY = S9b.6_22;                                                    
  D.5011.LB0 = 1;                                                               
  D.5011.UB0 = L8b_20;                                                          
  D.5010.P_BOUNDS = &D.5011;                                                    
  concat5_pkg1.make_failed (D.5010);                                            

D.5040 is points-to var of the alloca call. I will have to single-step tomorrow
to figure out why the oracle thinks that it can not be aliased by global memory
when it is seen as escaping...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-04-03 23:49 ` hubicka at gcc dot gnu.org
@ 2022-04-05 21:46 ` hubicka at gcc dot gnu.org
  2022-04-07 11:47 ` rguenth at gcc dot gnu.org
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: hubicka at gcc dot gnu.org @ 2022-04-05 21:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
OK, tree-ssa-alias is using:

if (ref_may_access_global_memory_p (ref))

to determine if the access to S9b.6_22 may be used by the call.  I expected
this to return true because memory pointed to S9b.6_22 is escaping and thus it
is accessible from global memory accesses.

However the implementation does:

/* Return ture if REF may access global memory.  */                             

bool                                                                            
ref_may_access_global_memory_p (ao_ref *ref)                                    
{                                                                               
  if (!ref->ref)                                                                
    return true;                                                                
  tree base = ao_ref_base (ref);                                                
  if (TREE_CODE (base) == MEM_REF                                               
      || TREE_CODE (base) == TARGET_MEM_REF)                                    
    {                                                                           
      if (ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)))                
        return true;                                                            
    }                                                                           
  else                                                                          
    {                                                                           
      if (!auto_var_in_fn_p (base, current_function_decl)                       
          || pt_solution_includes (&cfun->gimple_df->escaped,                   
                                   base))                                       
        return true;                                                            
    }                                                                           
  return false;                                                                 
}                                                                               


so for automatic variables we check "escaped" flag, but since this is heap
alocated we go the ptr_deref_may_alias_global_p path which checks:

/* Return true, if dereferencing PTR may alias with a global variable.  */      

bool                                                                            
ptr_deref_may_alias_global_p (tree ptr)                                         
{                                                                               
  struct ptr_info_def *pi;                                                      

  /* If we end up with a pointer constant here that may point                   
     to global memory.  */                                                      
  if (TREE_CODE (ptr) != SSA_NAME)                                              
    return true;                                                                

  pi = SSA_NAME_PTR_INFO (ptr);                                                 

  /* If we do not have points-to information for this variable,                 
     we have to punt.  */                                                       
  if (!pi)                                                                      
    return true;                                                                

  /* ???  This does not use TBAA to prune globals ptr may not access.  */       
  return pt_solution_includes_global (&pi->pt);                                 
}                                                                               

So this predicate tests something different - it tests if the deref can alias
global variable, not global memory in a sense of anything accessible from
outside world.

So I want to also return true on points to set that contains escaped?

diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index 50bd47b31f3..9e34f76c3cb 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -2578,8 +2578,24 @@ ref_may_access_global_memory_p (ao_ref *ref)
   if (TREE_CODE (base) == MEM_REF
       || TREE_CODE (base) == TARGET_MEM_REF)
     {
-      if (ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)))
+      struct ptr_info_def *pi;
+      tree ptr = TREE_OPERAND (base, 0);
+
+      /* If we end up with a pointer constant here that may point
+        to global memory.  */
+      if (TREE_CODE (ptr) != SSA_NAME)
+       return true;
+
+      pi = SSA_NAME_PTR_INFO (ptr);
+
+      /* If we do not have points-to information for this variable,
+        we have to punt.  */
+      if (!pi)
        return true;
+
+      /* ???  This does not use TBAA to prune globals ptr may not access.  */
+      return pt_solution_includes_global (&pi->pt)
+            || pi->pt.vars_contains_escaped;
     }
   else
     {

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-04-05 21:46 ` hubicka at gcc dot gnu.org
@ 2022-04-07 11:47 ` rguenth at gcc dot gnu.org
  2022-04-07 11:57 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-07 11:47 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #6)
> OK, tree-ssa-alias is using:
> 
> if (ref_may_access_global_memory_p (ref))
> 
> to determine if the access to S9b.6_22 may be used by the call.  I expected
> this to return true because memory pointed to S9b.6_22 is escaping and thus
> it is accessible from global memory accesses.

Where does it exactly do this in breaking the testcase?  It seems this
function is "new" (I don't remember adding it), and it doesn't use
ref_may_alias_global_p for some reason ...

[...]

> So this predicate tests something different - it tests if the deref can
> alias global variable, not global memory in a sense of anything accessible
> from outside world.
>
> So I want to also return true on points to set that contains escaped?
> 
> diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
> index 50bd47b31f3..9e34f76c3cb 100644
> --- a/gcc/tree-ssa-alias.cc
> +++ b/gcc/tree-ssa-alias.cc
> @@ -2578,8 +2578,24 @@ ref_may_access_global_memory_p (ao_ref *ref)
>    if (TREE_CODE (base) == MEM_REF
>        || TREE_CODE (base) == TARGET_MEM_REF)
>      {
> -      if (ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0)))
> +      struct ptr_info_def *pi;
> +      tree ptr = TREE_OPERAND (base, 0);
> +
> +      /* If we end up with a pointer constant here that may point
> +        to global memory.  */
> +      if (TREE_CODE (ptr) != SSA_NAME)
> +       return true;
> +
> +      pi = SSA_NAME_PTR_INFO (ptr);
> +
> +      /* If we do not have points-to information for this variable,
> +        we have to punt.  */
> +      if (!pi)
>         return true;
> +
> +      /* ???  This does not use TBAA to prune globals ptr may not access. 
> */
> +      return pt_solution_includes_global (&pi->pt)
> +            || pi->pt.vars_contains_escaped;
>      }
>    else
>      {

At least it should be consistent with this predicate, so doing sth
special only here doesn't make much sense.

I don't remember why I excluded pt->vars_contains_escaped from
pt_solution_includes_global, the testcase I added also passes with
it checked.

OTOH ref_may_alias_global_p_1 does

static bool
ref_may_alias_global_p_1 (tree base)
{ 
  if (DECL_P (base))
    return is_global_var (base);

so a local escaped variable would not be considered aliasing a global
variable from this.

Maybe for clarity we should add flags (defaulted to true?) to those
predicates indicating whether escaped automatic variables should be
considered "global"?  Or add an explicit ptr_deref_may_alias_escaped ().

For example points_to_local_or_readonly_memory_p () seems to exclude
escaped locals, likewise DSE.

But it seems ref_may_access_global_memory_p would be replaced with
ref_may_alias_global_p (ref, true)?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-04-07 11:47 ` rguenth at gcc dot gnu.org
@ 2022-04-07 11:57 ` rguenth at gcc dot gnu.org
  2022-04-07 12:12 ` rguenth at gcc dot gnu.org
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-07 11:57 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, ref_may_access_global_memory_p already checks

      if (!auto_var_in_fn_p (base, current_function_decl)
          || pt_solution_includes (&cfun->gimple_df->escaped,
                                   base))

so it seems it was aware of some issue.  I'm testing some patch.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2022-04-07 11:57 ` rguenth at gcc dot gnu.org
@ 2022-04-07 12:12 ` rguenth at gcc dot gnu.org
  2022-04-07 13:01 ` hubicka at kam dot mff.cuni.cz
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-07 12:12 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 52768
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52768&action=edit
patch in testing

I'm testing this, adding escaped_local_p parameters throughout the API and
adjusting consumers, throwing away ref_may_access_global_memory_p.

Following passes adjusted from false to true:

- DCE in mark_stmt_if_obviously_necessary but that's just compile-time
  optimization I think
- trans-mem.cc I think got it wrong, so now fixed (but trans-mem is dead)
- the modref query in modref_may_conflict

all others keep passing false.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2022-04-07 12:12 ` rguenth at gcc dot gnu.org
@ 2022-04-07 13:01 ` hubicka at kam dot mff.cuni.cz
  2022-04-07 13:03 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: hubicka at kam dot mff.cuni.cz @ 2022-04-07 13:01 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #10 from hubicka at kam dot mff.cuni.cz ---
Hi,
> 
> I'm testing this, adding escaped_local_p parameters throughout the API and
> adjusting consumers, throwing away ref_may_access_global_memory_p.
> 
> Following passes adjusted from false to true:
> 
> - DCE in mark_stmt_if_obviously_necessary but that's just compile-time
>   optimization I think
> - trans-mem.cc I think got it wrong, so now fixed (but trans-mem is dead)
> - the modref query in modref_may_conflict
> 
> all others keep passing false.

Thanks!  I was adding the function when introducing global memory logic
to modref. This was targeted to handle fatigue regression at -O2 but it
also helps in quite few extra cases.

Honza

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2022-04-07 13:01 ` hubicka at kam dot mff.cuni.cz
@ 2022-04-07 13:03 ` cvs-commit at gcc dot gnu.org
  2022-04-07 13:04 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-07 13:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:8c0ebaf9f586100920a3c0849fb10e9985d7ae58

commit r12-8048-g8c0ebaf9f586100920a3c0849fb10e9985d7ae58
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Apr 7 14:07:54 2022 +0200

    ipa/104303 - miscompilation of gnatmake

    Modref attempts to track memory accesses relative to the base pointers
    which are parameters of functions.
    If it fails, it still makes difference between unknown memory access and
    global memory access.  The second makes it possible to disambiguate with
    memory that is not accessible from outside world (i.e. everything that does
    not escape from the caller function).  This is useful so we do not punt
    when unknown function is called.

    The added ref_may_access_global_memory_p ends up using
    ptr_deref_may_alias_global_p which does not consider escaped automatic
    variables as global.  For modref those are still global since they
    can be accessed from functions called.

    The following adds a flag to the *_global_p APIs indicating whether
    escaped local memory should be considered as global or not and
    removes ref_may_access_global_memory_p in favor of using
    ref_may_alias_global_p with the flag set to true.

    2022-04-07  Richard Biener  <rguenther@suse.de>
                Jan Hubicka  <hubicka@ucw.cz>

            PR ipa/104303
            * tree-ssa-alias.h (ptr_deref_may_alias_global_p,
            ref_may_alias_global_p, ref_may_alias_global_p,
            stmt_may_clobber_global_p, pt_solution_includes_global): Add
            bool parameters indicating whether escaped locals should be
            considered global.
            * tree-ssa-structalias.cc (pt_solution_includes_global):
            When the new escaped_nonlocal_p flag is true also consider
            pt->vars_contains_escaped.
            * tree-ssa-alias.cc (ptr_deref_may_alias_global_p):
            Pass down new escaped_nonlocal_p flag.
            (ref_may_alias_global_p): Likewise.
            (stmt_may_clobber_global_p): Likewise.
            (ref_may_alias_global_p_1): Likewise.  For decls also
            query the escaped solution if true.
            (ref_may_access_global_memory_p): Remove.
            (modref_may_conflict): Use ref_may_alias_global_p with
            escaped locals considered global.
            (ref_maybe_used_by_stmt_p): Adjust.
            * ipa-fnsummary.cc (points_to_local_or_readonly_memory_p):
            Likewise.
            * tree-ssa-dse.cc (dse_classify_store): Likewise.
            * trans-mem.cc (thread_private_new_memory): Likewise, but
            consider escaped locals global.
            * tree-ssa-dce.cc (mark_stmt_if_obviously_necessary): Likewise.

            * gnat.dg/concat5.adb: New.
            * gnat.dg/concat5_pkg1.adb: Likewise.
            * gnat.dg/concat5_pkg1.ads: Likewise.
            * gnat.dg/concat5_pkg2.adb: Likewise.
            * gnat.dg/concat5_pkg2.ads: Likewise.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2022-04-07 13:03 ` cvs-commit at gcc dot gnu.org
@ 2022-04-07 13:04 ` rguenth at gcc dot gnu.org
  2022-04-07 14:16 ` ebotcazou at gcc dot gnu.org
  2022-04-12 15:05 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-07 13:04 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Should be fixed now.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2022-04-07 13:04 ` rguenth at gcc dot gnu.org
@ 2022-04-07 14:16 ` ebotcazou at gcc dot gnu.org
  2022-04-12 15:05 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-04-07 14:16 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #13 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Should be fixed now.

Yes, I confirm that gnatmake now works correctly again, thanks!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref
  2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2022-04-07 14:16 ` ebotcazou at gcc dot gnu.org
@ 2022-04-12 15:05 ` cvs-commit at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-12 15:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104303

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:7c7e78e9c460991349065572e32cac49b20d0432

commit r12-8109-g7c7e78e9c460991349065572e32cac49b20d0432
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 12 16:40:11 2022 +0200

    ipa/104303 - revert overly conservative DCE change

    The following reverts the DCE change back to the original behavior
    which should be handled well during the propagation stage.  That
    should fix the failures Thomas Schwinge is reporting.

    2022-04-12  Richard Biener  <rguenther@suse.de>

            PR ipa/104303
            * tree-ssa-dce.cc (mark_stmt_if_obviously_necessary): Do not
            include local escaped memory as obviously necessary stores.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-04-12 15:05 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-31 10:09 [Bug ipa/104303] New: [12 regression] gnatmake is miscompiled by IPA/modef ebotcazou at gcc dot gnu.org
2022-01-31 10:12 ` [Bug ipa/104303] [12 regression] gnatmake is miscompiled by IPA/modref pinskia at gcc dot gnu.org
2022-01-31 10:21 ` hubicka at gcc dot gnu.org
2022-02-01  8:18 ` rguenth at gcc dot gnu.org
2022-03-22 14:52 ` jakub at gcc dot gnu.org
2022-03-31 12:51 ` rguenth at gcc dot gnu.org
2022-03-31 13:12 ` hubicka at gcc dot gnu.org
2022-04-03 23:49 ` hubicka at gcc dot gnu.org
2022-04-05 21:46 ` hubicka at gcc dot gnu.org
2022-04-07 11:47 ` rguenth at gcc dot gnu.org
2022-04-07 11:57 ` rguenth at gcc dot gnu.org
2022-04-07 12:12 ` rguenth at gcc dot gnu.org
2022-04-07 13:01 ` hubicka at kam dot mff.cuni.cz
2022-04-07 13:03 ` cvs-commit at gcc dot gnu.org
2022-04-07 13:04 ` rguenth at gcc dot gnu.org
2022-04-07 14:16 ` ebotcazou at gcc dot gnu.org
2022-04-12 15:05 ` cvs-commit at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).