On Wed, 12 Feb 2014, Richard Biener wrote:
> On Wed, 12 Feb 2014, Jakub Jelinek wrote:
>
> > On Wed, Feb 12, 2014 at 10:30:01AM +0100, Richard Biener wrote:
> > > Bah. I am testing the following.
> >
> > But then there is no guarantee that ptr is aligned after the call.
> > char buf[32] __attribute__((aligned (32)));
> > int
> > foo (void)
> > {
> > void *ptr = buf + 1;
> > posix_memalign (&ptr, 32, -1);
> > /* Assume posix_memalign has failed. */
> > return ((__UINTPTR_TYPE__)ptr) & 31;
> > }
> >
> > This should return 1, but supposedly doesn't with the optimization.
> > So, either we need to revert the lowering, or perhaps do it only if
> > the original posix_memalign has a lhs and do it like:
> > void *tmp;
> > int res = posix_memalign (&tmp, align, size);
> > if (!res)
> > ptr = __builtin_assume_aligned (tmp, align);
> > or so (no need to initialize tmp and copy it back for the failure case,
> > but perhaps it would result in better code).
>
> Yeah. It seems to work modulo alias-31.c (checking what happens here,
> alias info looks good). Prelimiary patch below.
Ok, I've analyzed what happens here and it's a pass ordering issue
NEXT_PASS (pass_build_ealias);
NEXT_PASS (pass_sra_early);
NEXT_PASS (pass_fre);
PTA can figure out all points-to sets properly but when SRA
scalarizes the struct with the two posix_memaligned pointers
the replacements get written into SSA form and PHI nodes are
inserted for them (as assignment is now conditional). That
process cannot reliably (and does never) set points-to info
for those vars. FRE then fails to disambiguate.
Now, there is no reason why pass_build_ealias should be before
pass_sra_early - in fact that's a useless pessimization.
But as the testcase was supposed to test field-sensitive points-to
I chose to disable SRA for the testcase (and queue pass interchange
for 4.10 - unless you think it's ok now - I think it's harmless
and should only improve early FRE results when SRA happens).
Patch below in testing now.
Thanks,
Richard.
2014-02-12 Richard Biener <rguent...@suse.de>
PR middle-end/60092
* gimple-low.c (lower_builtin_posix_memalign): Lower conditional
of posix_memalign being successful.
(lower_stmt): Restrict lowering of posix_memalign to when
-ftree-bit-ccp is enabled.
* gcc.dg/torture/pr60092.c: New testcase.
* gcc.dg/tree-ssa/alias-31.c: Disable SRA.
Index: gcc/gimple-low.c
===================================================================
*** gcc/gimple-low.c (revision 207714)
--- gcc/gimple-low.c (working copy)
*************** lower_stmt (gimple_stmt_iterator *gsi, s
*** 336,342 ****
data->cannot_fallthru = false;
return;
}
! else if (DECL_FUNCTION_CODE (decl) == BUILT_IN_POSIX_MEMALIGN)
{
lower_builtin_posix_memalign (gsi);
return;
--- 336,343 ----
data->cannot_fallthru = false;
return;
}
! else if (DECL_FUNCTION_CODE (decl) == BUILT_IN_POSIX_MEMALIGN
! && flag_tree_bit_ccp)
{
lower_builtin_posix_memalign (gsi);
return;
*************** lower_builtin_setjmp (gimple_stmt_iterat
*** 781,817 ****
}
/* Lower calls to posix_memalign to
! posix_memalign (ptr, align, size);
! tem = *ptr;
! tem = __builtin_assume_aligned (tem, align);
! *ptr = tem;
or to
void *tem;
! posix_memalign (&tem, align, size);
! ttem = tem;
! ttem = __builtin_assume_aligned (ttem, align);
! ptr = tem;
in case the first argument was &ptr. That way we can get at the
alignment of the heap pointer in CCP. */
static void
lower_builtin_posix_memalign (gimple_stmt_iterator *gsi)
{
! gimple stmt = gsi_stmt (*gsi);
! tree pptr = gimple_call_arg (stmt, 0);
! tree align = gimple_call_arg (stmt, 1);
tree ptr = create_tmp_reg (ptr_type_node, NULL);
if (TREE_CODE (pptr) == ADDR_EXPR)
{
tree tem = create_tmp_var (ptr_type_node, NULL);
TREE_ADDRESSABLE (tem) = 1;
! gimple_call_set_arg (stmt, 0, build_fold_addr_expr (tem));
stmt = gimple_build_assign (ptr, tem);
}
else
stmt = gimple_build_assign (ptr,
fold_build2 (MEM_REF, ptr_type_node, pptr,
build_int_cst (ptr_type_node, 0)));
gsi_insert_after (gsi, stmt, GSI_NEW_STMT);
stmt = gimple_build_call (builtin_decl_implicit (BUILT_IN_ASSUME_ALIGNED),
2, ptr, align);
--- 782,828 ----
}
/* Lower calls to posix_memalign to
! res = posix_memalign (ptr, align, size);
! if (res == 0)
! *ptr = __builtin_assume_aligned (*ptr, align);
or to
void *tem;
! res = posix_memalign (&tem, align, size);
! if (res == 0)
! ptr = __builtin_assume_aligned (tem, align);
in case the first argument was &ptr. That way we can get at the
alignment of the heap pointer in CCP. */
static void
lower_builtin_posix_memalign (gimple_stmt_iterator *gsi)
{
! gimple stmt, call = gsi_stmt (*gsi);
! tree pptr = gimple_call_arg (call, 0);
! tree align = gimple_call_arg (call, 1);
! tree res = gimple_call_lhs (call);
tree ptr = create_tmp_reg (ptr_type_node, NULL);
if (TREE_CODE (pptr) == ADDR_EXPR)
{
tree tem = create_tmp_var (ptr_type_node, NULL);
TREE_ADDRESSABLE (tem) = 1;
! gimple_call_set_arg (call, 0, build_fold_addr_expr (tem));
stmt = gimple_build_assign (ptr, tem);
}
else
stmt = gimple_build_assign (ptr,
fold_build2 (MEM_REF, ptr_type_node, pptr,
build_int_cst (ptr_type_node, 0)));
+ if (res == NULL_TREE)
+ {
+ res = create_tmp_reg (integer_type_node, NULL);
+ gimple_call_set_lhs (call, res);
+ }
+ tree align_label = create_artificial_label (UNKNOWN_LOCATION);
+ tree noalign_label = create_artificial_label (UNKNOWN_LOCATION);
+ gimple cond = gimple_build_cond (EQ_EXPR, res, integer_zero_node,
+ align_label, noalign_label);
+ gsi_insert_after (gsi, cond, GSI_NEW_STMT);
+ gsi_insert_after (gsi, gimple_build_label (align_label), GSI_NEW_STMT);
gsi_insert_after (gsi, stmt, GSI_NEW_STMT);
stmt = gimple_build_call (builtin_decl_implicit (BUILT_IN_ASSUME_ALIGNED),
2, ptr, align);
*************** lower_builtin_posix_memalign (gimple_stm
*** 821,826 ****
--- 832,838 ----
build_int_cst (ptr_type_node, 0)),
ptr);
gsi_insert_after (gsi, stmt, GSI_NEW_STMT);
+ gsi_insert_after (gsi, gimple_build_label (noalign_label), GSI_NEW_STMT);
}
Index: gcc/testsuite/gcc.dg/torture/pr60092.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr60092.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr60092.c (working copy)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do run } */
+ /* { dg-require-weak "" } */
+
+ typedef __SIZE_TYPE__ size_t;
+ extern int posix_memalign(void **memptr, size_t alignment, size_t size)
__attribute__((weak));
+ extern void abort(void);
+ int
+ main (void)
+ {
+ void *p;
+ int ret;
+
+ if (!posix_memalign)
+ return 0;
+
+ p = (void *)&ret;
+ ret = posix_memalign (&p, sizeof (void *), -1);
+ if (p != (void *)&ret)
+ abort ();
+ return 0;
+ }
Index: gcc/testsuite/gcc.dg/tree-ssa/alias-31.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/alias-31.c (revision 207714)
--- gcc/testsuite/gcc.dg/tree-ssa/alias-31.c (working copy)
***************
*** 1,5 ****
/* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-cddce1" } */
extern int posix_memalign(void **memptr,
__SIZE_TYPE__ alignment, __SIZE_TYPE__ size);
--- 1,5 ----
/* { dg-do compile } */
! /* { dg-options "-O2 -fno-tree-sra -fdump-tree-cddce1" } */
extern int posix_memalign(void **memptr,
__SIZE_TYPE__ alignment, __SIZE_TYPE__ size);
