[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66812 Eric Gallager changed: What|Removed |Added Keywords||patch CC||egallager at gcc dot gnu.org --- Comment #11 from Eric Gallager --- (In reply to David Malcolm from comment #10) > Candidate patch posted as: > https://gcc.gnu.org/ml/jit/2015-q3/msg00060.html adding "patch" keyword
[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work
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
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=35945action=edit Minimal reproducer
[Bug jit/66812] jit code-generation example that unexpectedly required -fno-strict-aliasing to work
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
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=35946action=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
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; 5181TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF; 5182u = 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
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:
2262return 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
5160alias_set_type
5161c_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))
5183if (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
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; 5181TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF; 5182u = 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
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
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
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
