On Fri, 1 Apr 2016, Bernd Schmidt wrote: > On 04/01/2016 11:08 AM, Richard Biener wrote: > > { > > ! if (canon_true_dependence (s_info->mem, > > ! GET_MODE (s_info->mem), > > ! s_info->mem_addr, > > ! mem, mem_addr)) > > { > > s_info->rhs = NULL; > > s_info->const_rhs = NULL; > > --- 1609,1617 ---- > > the value of store_info. If it is, set the rhs to NULL to > > keep it from being used to remove a load. */ > > { > > ! if (canon_output_dependence (s_info->mem, true, > > ! mem, GET_MODE (mem), > > ! mem_addr)) > > { > > s_info->rhs = NULL; > > s_info->const_rhs = NULL; > > I think the patch is ok, but there is a comment in that function which > references canon_true_dependence; that should also be fixed up.
Done, though I don't understand it at all ... if alias-set was supposed to be zero for all cases we call canon_true_dependence then the issue wouldn't have happened. Maybe there was times where passing mem_addr == NULL_RTX to canon_true_dependence caused it to bail out? Not sure how to adjust that comment now, maybe it would be valid to simply remove the if (spill_alias_set) case and always use the else case? > Isn't the testcase invalid though? I thought accesses through char * > pointers bypass aliasing rules, but accessing a char array through int * > and long * pointers doesn't? I have installed it as the following with adjusted testcase, if somebody can shed some light on the odd comment I'll happily followup. Richard. 2016-04-04 Richard Biener <rguent...@suse.de> PR rtl-optimization/70484 * rtl.h (canon_output_dependence): Declare. * alias.c (canon_output_dependence): New function. * dse.c (record_store): Use canon_output_dependence rather than canon_true_dependence. * gcc.dg/torture/pr70484.c: New testcase. Index: gcc/rtl.h =================================================================== *** gcc/rtl.h (revision 234663) --- gcc/rtl.h (working copy) *************** extern int anti_dependence (const_rtx, c *** 3652,3657 **** --- 3652,3659 ---- extern int canon_anti_dependence (const_rtx, bool, const_rtx, machine_mode, rtx); extern int output_dependence (const_rtx, const_rtx); + extern int canon_output_dependence (const_rtx, bool, + const_rtx, machine_mode, rtx); extern int may_alias_p (const_rtx, const_rtx); extern void init_alias_target (void); extern void init_alias_analysis (void); Index: gcc/alias.c =================================================================== *** gcc/alias.c (revision 234663) --- gcc/alias.c (working copy) *************** output_dependence (const_rtx mem, const_ *** 3057,3062 **** --- 3057,3076 ---- /*mem_canonicalized=*/false, /*x_canonicalized*/false, /*writep=*/true); } + + /* Likewise, but we already have a canonicalized MEM, and X_ADDR for X. + Also, consider X in X_MODE (which might be from an enclosing + STRICT_LOW_PART / ZERO_EXTRACT). + If MEM_CANONICALIZED is true, MEM is canonicalized. */ + + int + canon_output_dependence (const_rtx mem, bool mem_canonicalized, + const_rtx x, machine_mode x_mode, rtx x_addr) + { + return write_dependence_p (mem, x, x_mode, x_addr, + mem_canonicalized, /*x_canonicalized=*/true, + /*writep=*/true); + } Index: gcc/dse.c =================================================================== *** gcc/dse.c (revision 234663) --- gcc/dse.c (working copy) *************** record_store (rtx body, bb_info_t bb_inf *** 1609,1618 **** the value of store_info. If it is, set the rhs to NULL to keep it from being used to remove a load. */ { ! if (canon_true_dependence (s_info->mem, ! GET_MODE (s_info->mem), ! s_info->mem_addr, ! mem, mem_addr)) { s_info->rhs = NULL; s_info->const_rhs = NULL; --- 1609,1617 ---- the value of store_info. If it is, set the rhs to NULL to keep it from being used to remove a load. */ { ! if (canon_output_dependence (s_info->mem, true, ! mem, GET_MODE (mem), ! mem_addr)) { s_info->rhs = NULL; s_info->const_rhs = NULL; Index: gcc/testsuite/gcc.dg/torture/pr70484.c =================================================================== *** gcc/testsuite/gcc.dg/torture/pr70484.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr70484.c (revision 0) *************** *** 0 **** --- 1,19 ---- + /* { dg-do run } */ + + extern void abort (void); + + int __attribute__((noinline,noclone)) + f(int *pi, long *pl) + { + *pi = 1; + *pl = 0; + return *(char *)pi; + } + + int main() + { + union { long l; int i; } a; + if (f (&a.i, &a.l) != 0) + abort (); + return 0; + }