[Bug jit/66812] New: jit code-generation example that required -fno-strict-aliasing to work

[Bug jit/66812] New: jit code-generation example that required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 66812
           Summary: jit code-generation example that required
                    -fno-strict-aliasing to work
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: jit
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
            Blocks: 66627
  Target Milestone: ---

A user reported a problem on the jit mailing list where incorrect code was
generated by libgccjit at the equivalent -O2 and above unless
-fno-strict-aliasing was supplied:
  https://gcc.gnu.org/ml/jit/2015-q3/msg00015.html


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66627
[Bug 66627] Tracker bug for jit bugs affecting ravi

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Created attachment 35945
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35945&action=edit
Minimal reproducer

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Output from fre1 dump (at e.g. optlevel 3) (with TDF_DETAILS enabled) is:

;; Function test_pr66812 (test_pr66812, funcdef_no=0, decl_uid=56,
cgraph_uid=0, symbol_order=0)

Setting value number of .MEM_4(D) to .MEM_4(D) (changed)
Value numbering .MEM_5 stmt = MEM[(struct value *)arr_2(D) +
16B].union_field.i_field = 0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].union_field.i_field
to 0
Setting value number of .MEM_5 to .MEM_5 (changed)
Value numbering .MEM_8 stmt = MEM[(struct value *)arr_2(D) + 16B].type_code =
0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].type_code to 0
Setting value number of .MEM_8 to .MEM_8 (changed)
Value numbering .MEM_11 stmt = MEM[(struct value *)arr_2(D) +
16B].union_field.ll_field = 10;
RHS 10 simplified to 10
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].union_field.ll_field
to 10
Setting value number of .MEM_11 to .MEM_11 (changed)
Value numbering .MEM_14 stmt = MEM[(struct value *)arr_2(D) + 16B].type_code =
19;
RHS 19 simplified to 19
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].type_code to 19
Setting value number of .MEM_14 to .MEM_14 (changed)
Value numbering .MEM_17 stmt = MEM[(struct value *)arr_2(D) +
16B].union_field.i_field = 0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].union_field.i_field
to 0
Setting value number of .MEM_17 to .MEM_17 (changed)
Value numbering .MEM_20 stmt = MEM[(struct value *)arr_2(D) + 16B].type_code =
0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_2(D) + 16B].type_code to 0
Setting value number of .MEM_20 to .MEM_20 (changed)
Value numbers:
Deleted redundant store MEM[(struct value *)arr_2(D) + 16B].union_field.i_field
= 0;
Removing dead stmt MEM[(struct value *)arr_2(D) + 16B].union_field.i_field = 0;
test_pr66812 (struct value * arr)
{
<L0>:
  MEM[(struct value *)arr_2(D) + 16B].union_field.i_field = 0;
  MEM[(struct value *)arr_2(D) + 16B].type_code = 0;
  MEM[(struct value *)arr_2(D) + 16B].union_field.ll_field = 10;
  MEM[(struct value *)arr_2(D) + 16B].type_code = 19;
  MEM[(struct value *)arr_2(D) + 16B].type_code = 0;
  return;

}

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #2)
> Output from fre1 dump (at e.g. optlevel 3) (with TDF_DETAILS enabled) is:
 (snip)

> Removing dead stmt MEM[(struct value *)arr_2(D) + 16B].union_field.i_field = 0;

I believe that this statement *isn't* dead, and that removing it is the bug.


> test_pr66812 (struct value * arr)
> {
> <L0>:
>   MEM[(struct value *)arr_2(D) + 16B].union_field.i_field = 0;
>   MEM[(struct value *)arr_2(D) + 16B].type_code = 0;
>   MEM[(struct value *)arr_2(D) + 16B].union_field.ll_field = 10;
>   MEM[(struct value *)arr_2(D) + 16B].type_code = 19;
>   MEM[(struct value *)arr_2(D) + 16B].type_code = 0;
>   return;

Hence ....union_field.ll_field == 10 rather than == 0.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Created attachment 35946
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35946&action=edit
Equivalent C code

(generated using gcc_jit_context_dump_to_file and lightly editing until valid
C)

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #4)
> Created attachment 35946 [details]
> Equivalent C code
> 
> (generated using gcc_jit_context_dump_to_file and lightly editing until
> valid C)

Compiling this with trunk with -O3 *doesn't* show the fre1 issue exhibited by
libgccjit; dump from cc1's fre1 looks like this:

;; Function test_pr66812 (test_pr66812, funcdef_no=0, decl_uid=1762,
cgraph_uid=0, symbol_order=0)

Setting value number of .MEM_3(D) to .MEM_3(D) (changed)
Value numbering .MEM_4 stmt = MEM[(struct value *)arr_1(D) +
16B].union_field.i_field = 0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].union_field.i_field
to 0
Setting value number of .MEM_4 to .MEM_4 (changed)
Value numbering .MEM_6 stmt = MEM[(struct value *)arr_1(D) + 16B].type_code =
0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].type_code to 0
Setting value number of .MEM_6 to .MEM_6 (changed)
Value numbering .MEM_8 stmt = MEM[(struct value *)arr_1(D) +
16B].union_field.ll_field = 10;
RHS 10 simplified to 10
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].union_field.ll_field
to 10
Setting value number of .MEM_8 to .MEM_8 (changed)
Value numbering .MEM_10 stmt = MEM[(struct value *)arr_1(D) + 16B].type_code =
19;
RHS 19 simplified to 19
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].type_code to 19
Setting value number of .MEM_10 to .MEM_10 (changed)
Value numbering .MEM_12 stmt = MEM[(struct value *)arr_1(D) +
16B].union_field.i_field = 0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].union_field.i_field
to 0
Setting value number of .MEM_12 to .MEM_12 (changed)
Value numbering .MEM_14 stmt = MEM[(struct value *)arr_1(D) + 16B].type_code =
0;
RHS 0 simplified to 0
No store match
Value numbering store MEM[(struct value *)arr_1(D) + 16B].type_code to 0
Setting value number of .MEM_14 to .MEM_14 (changed)
Value numbers:
test_pr66812 (struct value * arr)
{
  <bb 2>:
  MEM[(struct value *)arr_1(D) + 16B].union_field.i_field = 0;
  MEM[(struct value *)arr_1(D) + 16B].type_code = 0;
  MEM[(struct value *)arr_1(D) + 16B].union_field.ll_field = 10;
  MEM[(struct value *)arr_1(D) + 16B].type_code = 19;
  MEM[(struct value *)arr_1(D) + 16B].union_field.i_field = 0;
  MEM[(struct value *)arr_1(D) + 16B].type_code = 0;
  return;

}


Note that the final:
  MEM[(struct value *)arr_1(D) + 16B].union_field.i_field = 0;
is still present, and the lack of the:
> Deleted redundant store MEM[(struct value *)arr_2(D) + 16B].union_field.i_field = 0;
from the TDF_DETAILS version of the dump.

So for some reason fre1 is more aggressive on this with libgccjit than with
cc1.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |alias, wrong-code

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This sounds like there is an aliasing set problem.

In the C front-end we have:


5174   /* Permit type-punning when accessing a union, provided the access
5175      is directly through the union.  For example, this code does not
5176      permit taking the address of a union member and then storing
5177      through it.  Even the type-punning allowed here is a GCC
5178      extension, albeit a common and useful one; the C standard says
5179      that such accesses have implementation-defined behavior.  */
5180   for (u = t;
5181        TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
5182        u = TREE_OPERAND (u, 0))
5183     if (TREE_CODE (u) == COMPONENT_REF
5184         && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
5185       return 0;
5186 


In c_common_get_alias_set .  Maybe that is missing on the libjit side.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-07-09
     Ever confirmed|0                           |1

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> This sounds like there is an aliasing set problem.
> 
> In the C front-end we have:
> 
> 
> 5174   /* Permit type-punning when accessing a union, provided the access
> 5175      is directly through the union.  For example, this code does not
> 5176      permit taking the address of a union member and then storing
> 5177      through it.  Even the type-punning allowed here is a GCC
> 5178      extension, albeit a common and useful one; the C standard says
> 5179      that such accesses have implementation-defined behavior.  */
> 5180   for (u = t;
> 5181        TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
> 5182        u = TREE_OPERAND (u, 0))
> 5183     if (TREE_CODE (u) == COMPONENT_REF
> 5184         && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
> 5185       return 0;
> 5186 
> 
> 
> In c_common_get_alias_set .  Maybe that is missing on the libjit side.

Thanks.  Yes; I'd just figured that out, by single-stepping.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Root cause found: it's because of the langhook:
  LANG_HOOKS_GET_ALIAS_SET
which it looks like I need to implement for libgccjit.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #9 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Notes to self:
For the given stmt, fre1's call to:
          val = vn_reference_lookup (gimple_assign_lhs (stmt),
                                     gimple_vuse (stmt), VN_WALK, NULL);
is returning NULL for "val", whereas libgccjit is returning the const_int 0
node,
hence the latter regards stmt as as redundant assignment and eliminates it.

Within the call to vn_reference_lookup:

2232      vr1.set = get_alias_set (op);

For cc1:
  (gdb) p vr1.set
  $24 = 0

  (gdb) p wvnresult
  $26 = (vn_reference_t) 0x0

and it bails out of vn_reference_lookup here:
  2262        return NULL_TREE;

Whereas for libgccjit, the call to vn_reference_lookup does this:

  (gdb) p vr1.set
  $6 = 2

and walk_non_aliased_vuses returns non-NULL:
  (gdb) p wvnresult
  $7 = (vn_reference_t) 0x677dd0


libgccjit:
  (gdb) call get_alias_set (op)
  $11 = 2

cc1:
  (gdb) call get_alias_set(op)
  $27 = 0

and this is because of a langhook, implemented for C/C++/ObjC, but not (yet)
for libgccjit:

837     alias_set_type
838     get_alias_set (tree t)
839     {
(snip)
849     
850       /* We can be passed either an expression or a type.  This and the
851          language-specific routine may make mutually-recursive calls to
each other
852          to figure out what to do.  At each juncture, we see if this is a
tree
853          that the language may need to handle specially.  First handle
things that
854          aren't types.  */
855       if (! TYPE_P (t))
856         {
857           /* Give the language a chance to do something with this tree
858              before we look at it.  */
859           STRIP_NOPS (t);
860           set = lang_hooks.get_alias_set (t);
861           if (set != -1)
862             return set;

c/c-objc-common.h:45:#undef LANG_HOOKS_GET_ALIAS_SET
c/c-objc-common.h:46:#define LANG_HOOKS_GET_ALIAS_SET c_common_get_alias_set

c-family/c-common.c:
5157    /* Return the typed-based alias set for T, which may be an expression
5158       or a type.  Return -1 if we don't do anything special.  */
5159    
5160    alias_set_type
5161    c_common_get_alias_set (tree t)
5162    {
5163      tree u;
(snip)
5173    
5174      /* Permit type-punning when accessing a union, provided the access
5175         is directly through the union.  For example, this code does not
5176         permit taking the address of a union member and then storing
5177         through it.  Even the type-punning allowed here is a GCC
5178         extension, albeit a common and useful one; the C standard says
5179         that such accesses have implementation-defined behavior.  */
5180      for (u = t;
5181           TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
5182           u = TREE_OPERAND (u, 0))
5183        if (TREE_CODE (u) == COMPONENT_REF
5184            && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
5185          return 0;

For C, it's exiting at line 5185 for the given statement's LHS.

[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work

[dmalcolm at gcc dot gnu.org]

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

--- Comment #10 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Candidate patch posted as:
https://gcc.gnu.org/ml/jit/2015-q3/msg00060.html