Re: [PATCH] Add support for OpenMP fortran user defined reductions

[Jakub Jelinek]

On Mon, Jun 02, 2014 at 09:11:20PM +0200, Paul Richard Thomas wrote:
> First I should say many thanks for this support for gfortran. This is
> the one area for which we have professional support and I for one am
> truly grateful.
> 
> I have scanned through the patch and can see nothing to object to. So
> I would say that this is good for trunk, especially at this stage of
> the cycle.

As discussed with Tobias on IRC yesterday, the fact that I'd like to
eventually backport the Fortran OpenMP 4.0 support to 4.9 branch
poses a problem with the module.c changes (though, not just this, but
already the earlier !$omp declare simd change to module.c).
While the simd change was partly compatible (older compilers could read
newer *.mod file as long as there is no !$omp declare simd, newer 
compilers could read older *.mod file always), the !$omp declare reduction
module.c change is now incompatible.  There are ways to do this compatibly,
or partially compatibly, e.g. instead of adding unconditional ( list )
into the middle of *.mod file add 'OMP UDR' ( list ) conditionally
(if the list is not empty, otherwise nothing), or bump MODULE_VERSION to
"13" but handle both "12" and "13" in the reader, or bump MODULE_VERSION
to "13" only if there is any !$omp declare {simd,reduction} to be emitted.

Jakub

[PATCH, Pointer Bounds Checker 23/x] Function split

[Ilya Enkovich]

Hi,

This patch does not allow splitting in case bounds are returned until retutrned 
bounds are supported.  It also propagates instrumentation marks for generated 
call and function.

Bootstrapped and tested on linux-x86_64.

Thanks,
Ilya
--
gcc/

2014-06-03  Ilya Enkovich  

* ipa-split.c: Include tree-chkp.h.
(consider_split): Do not split when return bounds.
(split_function): Propagate Pointer Bounds Checker
instrumentation marks.


diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 38bd883..edf322f 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -110,6 +110,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-pretty-print.h"
 #include "ipa-inline.h"
 #include "cfgloop.h"
+#include "tree-chkp.h"
 
 /* Per basic block info.  */
 
@@ -496,6 +497,19 @@ consider_split (struct split_point *current, bitmap 
non_ssa_vars,
   if (!VOID_TYPE_P (TREE_TYPE (current_function_decl)))
 call_overhead += estimate_move_cost (TREE_TYPE (current_function_decl));
 
+  /* Currently returned value is processed but returned bounds
+ are not processed.  It results in bounds in return statement
+ with no definition.  Forbid split until returned bounds are
+ supported.  */
+  if (chkp_function_instrumented_p (current_function_decl)
+  && chkp_type_has_pointer (TREE_TYPE (TREE_TYPE (current_function_decl
+{
+  if (dump_file && (dump_flags & TDF_DETAILS))
+   fprintf (dump_file,
+"  Refused: need to return bounds\n");
+  return;
+}
+
   if (current->split_size <= call_overhead)
 {
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -1096,6 +1110,7 @@ split_function (struct split_point *split_point)
   edge_iterator ei;
   tree retval = NULL, real_retval = NULL;
   bool split_part_return_p = false;
+  bool with_bounds = chkp_function_instrumented_p (current_function_decl);
   gimple last_stmt = NULL;
   unsigned int i;
   tree arg, ddef;
@@ -1248,6 +1263,12 @@ split_function (struct split_point *split_point)
   DECL_BUILT_IN_CLASS (node->decl) = NOT_BUILT_IN;
   DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0;
 }
+
+  /* If the original function is instrumented then it's
+ part is also instrumented.  */
+  if (with_bounds)
+chkp_function_mark_instrumented (node->decl);
+
   /* If the original function is declared inline, there is no point in issuing
  a warning for the non-inlinable part.  */
   DECL_NO_INLINE_WARNING_P (node->decl) = 1;
@@ -1282,6 +1303,7 @@ split_function (struct split_point *split_point)
args_to_pass[i] = arg;
   }
   call = gimple_build_call_vec (node->decl, args_to_pass);
+  gimple_call_set_with_bounds (call, with_bounds);
   gimple_set_block (call, DECL_INITIAL (current_function_decl));
   args_to_pass.release ();
 

[PATCH, Pointer Bounds Checker 24/x] PRE

[Ilya Enkovich]

Hi,

This patch preserves CALL_WITH_BOUNDS flag for calls during PRE.

Bootstrapped and tested on linux-x86_64.

Thanks,
Ilya
--
gcc/

2014-06-03  Ilya Enkovich  

* tree-ssa-pre.c (create_component_ref_by_pieces_1): Store
CALL_WITH_BOUNDS_P for calls.
(copy_reference_ops_from_call): Restore CALL_WITH_BOUNDS_P
flag.


diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index 1e55356..d5b9f3b 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -2579,6 +2579,8 @@ create_component_ref_by_pieces_1 (basic_block block, 
vn_reference_t ref,
   (TREE_CODE (fn) == FUNCTION_DECL
? build_fold_addr_expr (fn) : fn),
   nargs, args);
+   if (currop->op2 == integer_one_node)
+ CALL_WITH_BOUNDS_P (folded) = true;
free (args);
if (sc)
  CALL_EXPR_STATIC_CHAIN (folded) = sc;
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index f7ec8b6..e83d9dc 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1124,6 +1124,8 @@ copy_reference_ops_from_call (gimple call,
   temp.opcode = CALL_EXPR;
   temp.op0 = gimple_call_fn (call);
   temp.op1 = gimple_call_chain (call);
+  if (gimple_call_with_bounds_p (call))
+temp.op2 = integer_one_node;
   temp.off = -1;
   result->safe_push (temp);
 

[PATCH] Fix PR61328: fully initialize symbolic number before using it

[Thomas Preud'homme]

When a bitwise OR gimple statement has for operands SSA_NAME initialized 
directly from memory source (no cast or other unary statement intervening), a 
symbolic number will be used only partly initialized. This was discovered by 
valgrind and reported as PR61328. This patch fixes that by moving the 
initialization code in a separate function that can be called from the two 
places that need it. There was a problem of a field of a structure that was set 
in a function and the value of this field was read before checking the result 
of the function call. This would lead to missed optimization.

ChangeLog is as follows:

2014-05-29  Thomas Preud'homme  

PR tree-optimization/61328
* tree-ssa-math-opts.c (init_symbolic_number): Extract symbolic number
initialization from find_bswap_or_nop_1.
(find_bswap_or_nop_1): Test return value of find_bswap_or_nop_1 stored
in source_expr2 before using the size value the function sets. Also
make use of init_symbolic_number () in both the old place and
find_bswap_or_nop_load () to avoid reading uninitialized memory when
doing recursion in the GIMPLE_BINARY_RHS case.

Ok for trunk?

diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index d9afccf..6c26d6d 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1701,6 +1701,30 @@ verify_symbolic_number_p (struct symbolic_number *n, 
gimple stmt)
   return true;
 }
 
+/* Initialize the symbolic number N for the bswap pass from the base element
+   SRC manipulated by the bitwise OR expression.  */
+
+static bool
+init_symbolic_number (struct symbolic_number *n, tree src)
+{
+  n->base_addr = n->offset = n->alias_set = n->vuse = NULL_TREE;
+
+  /* Set up the symbolic number N by setting each byte to a value between 1 and
+ the byte size of rhs1.  The highest order byte is set to n->size and the
+ lowest order byte to 1.  */
+  n->size = TYPE_PRECISION (TREE_TYPE (src));
+  if (n->size % BITS_PER_UNIT != 0)
+return false;
+  n->size /= BITS_PER_UNIT;
+  n->range = n->size;
+  n->n = CMPNOP;
+
+  if (n->size < (int)sizeof (int64_t))
+n->n &= ((uint64_t)1 << (n->size * BITS_PER_UNIT)) - 1;
+
+  return true;
+}
+
 /* Check if STMT might be a byte swap or a nop from a memory source and returns
the answer. If so, REF is that memory source and the base of the memory area
accessed and the offset of the access from that base are recorded in N.  */
@@ -1713,26 +1737,27 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct 
symbolic_number *n)
   HOST_WIDE_INT bitsize, bitpos;
   enum machine_mode mode;
   int unsignedp, volatilep;
+  tree offset, base_addr;
 
   if (!gimple_assign_load_p (stmt) || gimple_has_volatile_ops (stmt))
 return false;
 
-  n->base_addr = get_inner_reference (ref, &bitsize, &bitpos, &n->offset,
- &mode, &unsignedp, &volatilep, false);
+  base_addr = get_inner_reference (ref, &bitsize, &bitpos, &offset, &mode,
+  &unsignedp, &volatilep, false);
 
-  if (TREE_CODE (n->base_addr) == MEM_REF)
+  if (TREE_CODE (base_addr) == MEM_REF)
 {
   offset_int bit_offset = 0;
-  tree off = TREE_OPERAND (n->base_addr, 1);
+  tree off = TREE_OPERAND (base_addr, 1);
 
   if (!integer_zerop (off))
{
- offset_int boff, coff = mem_ref_offset (n->base_addr);
+ offset_int boff, coff = mem_ref_offset (base_addr);
  boff = wi::lshift (coff, LOG2_BITS_PER_UNIT);
  bit_offset += boff;
}
 
-  n->base_addr = TREE_OPERAND (n->base_addr, 0);
+  base_addr = TREE_OPERAND (base_addr, 0);
 
   /* Avoid returning a negative bitpos as this may wreak havoc later.  */
   if (wi::neg_p (bit_offset))
@@ -1743,11 +1768,11 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct 
symbolic_number *n)
 Subtract it to BIT_OFFSET and add it (scaled) to OFFSET.  */
  bit_offset -= tem;
  tem = wi::arshift (tem, LOG2_BITS_PER_UNIT);
- if (n->offset)
-   n->offset = size_binop (PLUS_EXPR, n->offset,
+ if (offset)
+   offset = size_binop (PLUS_EXPR, offset,
wide_int_to_tree (sizetype, tem));
  else
-   n->offset = wide_int_to_tree (sizetype, tem);
+   offset = wide_int_to_tree (sizetype, tem);
}
 
   bitpos += bit_offset.to_shwi ();
@@ -1758,6 +1783,9 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct 
symbolic_number *n)
   if (bitsize % BITS_PER_UNIT)
 return false;
 
+  init_symbolic_number (n, ref);
+  n->base_addr = base_addr;
+  n->offset = offset;
   n->bytepos = bitpos / BITS_PER_UNIT;
   n->alias_set = reference_alias_ptr_type (ref);
   n->vuse = gimple_vuse (stmt);
@@ -1816,28 +1844,12 @@ find_bswap_or_nop_1 (gimple stmt, struct 
symbolic_number *n, int limit)
 
   /* If find_bswap_or_nop_1 returned NULL, STMT is a leaf node and

Re: RFA: Remove "m" column from backends.html

[Richard Sandiford]

Segher Boessenkool  writes:
> On Mon, Jun 02, 2014 at 08:14:49PM +0100, Richard Sandiford wrote:
>> As per the patch I just posted, all ports have moved over to the new
>> way of defining constraints.  This patch removes the associated "m"
>> column from backends.html.  (Note that the ports still listed with "m"
>> have in fact been converted.)
>
>> -m   Port does not use define_constants.
>
> define_constant and define_constraint are different beasts; or was
> this a typo originally?

Sorry, was clearly reading what I wanted to read.

Richard

Re: [RFC][AArch64] Remove CORE_REGS form reg_class

[Marcus Shawcroft]

On 27 May 2014 23:27, Kugan  wrote:

> Due to the cost changes in IRA, now part of the arguments(v0.d[1]) for
> multf3 ends up in stack. Reason for this us, in IRA, assign_hard_reg,
> while iterating for the cost for assigning register to reg:TI 99,
> allocates register 32 (FP register). Which I think is wrong. After which
> LRA makes it worse. There could be a latent bug here in LRA side but I
> think still we need to look at cost model as well. Increasing the cost
> model like below helps here.


If I understand correctly this is real regression, in which case
adjusting the costs like this is simply papering over the cracks.  Can
you figure out what the real issue is?

Cheers
/Marcus

[PATCH, Pointer Bounds Checker 25/x] DCE

[Ilya Enkovich]

Hi,

This patch adjusts alloc-free removal algorithm in DCE to take into account 
BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.

Bootstrapped and tested on linux-x86_64.

Thanks,
Ilya
--
gcc/

2014-06-03  Ilya Enkovich  

* tree-ssa-dce.c: Include target.h.
(propagate_necessity): For free call fed by alloc check
bounds are also provided by the same alloc.
(eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
used by free calls.

diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 13a71ce..59a0b71 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "flags.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
+#include "target.h"
 
 static struct stmt_stats
 {
@@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
  && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
  && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
  || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
-   continue;
+   {
+ tree retfndecl
+   = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
+ gimple bounds_def_stmt;
+ tree bounds;
+
+ /* For instrumented calls we should also check used
+bounds are returned by the same allocation call.  */
+ if (!gimple_call_with_bounds_p (stmt)
+ || ((bounds = gimple_call_arg (stmt, 1))
+ && TREE_CODE (bounds) == SSA_NAME
+ && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
+ && is_gimple_call (bounds_def_stmt)
+ && gimple_call_fndecl (bounds_def_stmt) == retfndecl
+ && gimple_call_arg (bounds_def_stmt, 0) == ptr))
+   continue;
+   }
}
 
  FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
@@ -1204,6 +1221,23 @@ eliminate_unnecessary_stmts (void)
  && !gimple_plf (def_stmt, STMT_NECESSARY))
gimple_set_plf (stmt, STMT_NECESSARY, false);
}
+ /* We did not propagate necessity for free calls fed
+by allocation function to allow unnecessary
+alloc-free sequence elimination.  For instrumented
+calls it also means we did not mark bounds producer
+as necessary and it is time to do it in case free
+call is not removed.  */
+ if (gimple_call_with_bounds_p (stmt))
+   {
+ gimple bounds_def_stmt;
+ tree bounds = gimple_call_arg (stmt, 1);
+ gcc_assert (TREE_CODE (bounds) == SSA_NAME);
+ bounds_def_stmt = SSA_NAME_DEF_STMT (bounds);
+ if (bounds_def_stmt
+ && !gimple_plf (bounds_def_stmt, STMT_NECESSARY))
+   gimple_set_plf (bounds_def_stmt, STMT_NECESSARY,
+   gimple_plf (stmt, STMT_NECESSARY));
+   }
}
 
  /* If GSI is not necessary then remove it.  */
@@ -1216,6 +1250,7 @@ eliminate_unnecessary_stmts (void)
  else if (is_gimple_call (stmt))
{
  tree name = gimple_call_lhs (stmt);
+ tree retfn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
 
  notice_special_calls (stmt);
 
@@ -1233,7 +1268,9 @@ eliminate_unnecessary_stmts (void)
  && DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC
  && DECL_FUNCTION_CODE (call) != BUILT_IN_ALLOCA
  && (DECL_FUNCTION_CODE (call)
- != BUILT_IN_ALLOCA_WITH_ALIGN
+ != BUILT_IN_ALLOCA_WITH_ALIGN)))
+ /* Avoid doing so for bndret calls for the same reason.  */
+ && (!retfn || gimple_call_fndecl (stmt) != retfn))
{
  something_changed = true;
  if (dump_file && (dump_flags & TDF_DETAILS))

Re: detecting "container overflow" bugs in std::vector

[Konstantin Serebryany]

On Thu, May 29, 2014 at 6:29 PM, Jonathan Wakely  wrote:
> On 26/05/14 19:19 +0400, Konstantin Serebryany wrote:
>>>
>>> It does look useful but I'm concerned about a proliferation of
>>> container checks, we already have the libstdc++ Debug Mode
>>> and I'd
>>> like to see some of the lightweight checks from the Google branch
>>> added to trunk too.
>>
>> Me too, but these checks are mostly orthogonal to the proposed
>> annotations.
>
>
> Thanks for clarifying that (and to Paul).
>
>
>>> Aren't they still much cheaper than asan
>>> instrumentation?
>>
>>
>> Of course, they are much cheaper than asan. But they do not cover the case
>> that motivated the container overflow annotations (when the contents
>> of vector are accessed via
>> vector::data())
>
>
> Yes, I don't think I've ever seen that error in code I work with, but
These errors are indeed much less frequent that others.
But in our code base we've cleaned most of those popular ones and now
chasing the long tail of infrequent types of bugs.
Our current variant of libstdc++ patch (+asan) found quite a few of them.

> if Asan can be made to detect it then I'm in favour of the changes.

Thanks, I'll work on the updated libstdc++ path then.


--kcc


>
> Thanks.
>

[PATCH, Pointer Bounds Checker 26/x] CCP

[Ilya Enkovich]

Hi,

This patch allows BUILT_IN_CHKP_BNDRET as a consumer of a result of 
BUILT_IN_STACK_SAVE call.

Bootstrapped and tested on linux-x86_64.

Thanks,
Ilya
--
gcc/

2014-06-03  Ilya Enkovich  

* tree-ssa-ccp.c (insert_clobber_before_stack_restore): Handle
BUILT_IN_CHKP_BNDRET calls.


diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index eeefeaf..c138337 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -1848,7 +1848,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree 
var,
 gimple_htab *visited)
 {
   gimple stmt, clobber_stmt;
-  tree clobber;
+  tree clobber, fndecl;
   imm_use_iterator iter;
   gimple_stmt_iterator i;
   gimple *slot;
@@ -1880,6 +1880,10 @@ insert_clobber_before_stack_restore (tree saved_val, 
tree var,
 else if (gimple_assign_ssa_name_copy_p (stmt))
   insert_clobber_before_stack_restore (gimple_assign_lhs (stmt), var,
   visited);
+else if (gimple_code (stmt) == GIMPLE_CALL
+&& (fndecl = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET))
+&& gimple_call_fndecl (stmt) == fndecl)
+  continue;
 else
   gcc_assert (is_gimple_debug (stmt));
 }

Re: [PATCH] Add support for OpenMP fortran user defined reductions

[FX]

> As discussed with Tobias on IRC yesterday, the fact that I'd like to
> eventually backport the Fortran OpenMP 4.0 support to 4.9 branch
> poses a problem with the module.c changes

But this is by design, because we’re not supposed to add new features 
(especially API-changing or module-changing ones) in a release branch. The 
compatibility fixes you propose will increase the complexity of the module 
reader code, and creates some precedent.

I don’t think there’s much pressure from the “general public” for Fortran 
OpenMP 4.0, so having it in 4.10 only rather than 4.9 is probably not such a 
big deal.

FX

Re: [PATCH] Fix PR61328: fully initialize symbolic number before using it

[Richard Biener]

On June 3, 2014 9:18:29 AM CEST, Thomas Preud'homme  
wrote:
>When a bitwise OR gimple statement has for operands SSA_NAME
>initialized directly from memory source (no cast or other unary
>statement intervening), a symbolic number will be used only partly
>initialized. This was discovered by valgrind and reported as PR61328.
>This patch fixes that by moving the initialization code in a separate
>function that can be called from the two places that need it. There was
>a problem of a field of a structure that was set in a function and the
>value of this field was read before checking the result of the function
>call. This would lead to missed optimization.
>
>ChangeLog is as follows:
>
>2014-05-29  Thomas Preud'homme  
>
>   PR tree-optimization/61328
>   * tree-ssa-math-opts.c (init_symbolic_number): Extract symbolic number
>   initialization from find_bswap_or_nop_1.
>   (find_bswap_or_nop_1): Test return value of find_bswap_or_nop_1 stored
>   in source_expr2 before using the size value the function sets. Also
>   make use of init_symbolic_number () in both the old place and
>   find_bswap_or_nop_load () to avoid reading uninitialized memory when
>   doing recursion in the GIMPLE_BINARY_RHS case.
>
>Ok for trunk?

OK.

Thanks,
Richard.

>diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
>index d9afccf..6c26d6d 100644
>--- a/gcc/tree-ssa-math-opts.c
>+++ b/gcc/tree-ssa-math-opts.c
>@@ -1701,6 +1701,30 @@ verify_symbolic_number_p (struct symbolic_number
>*n, gimple stmt)
>   return true;
> }
> 
>+/* Initialize the symbolic number N for the bswap pass from the base
>element
>+   SRC manipulated by the bitwise OR expression.  */
>+
>+static bool
>+init_symbolic_number (struct symbolic_number *n, tree src)
>+{
>+  n->base_addr = n->offset = n->alias_set = n->vuse = NULL_TREE;
>+
>+  /* Set up the symbolic number N by setting each byte to a value
>between 1 and
>+ the byte size of rhs1.  The highest order byte is set to n->size
>and the
>+ lowest order byte to 1.  */
>+  n->size = TYPE_PRECISION (TREE_TYPE (src));
>+  if (n->size % BITS_PER_UNIT != 0)
>+return false;
>+  n->size /= BITS_PER_UNIT;
>+  n->range = n->size;
>+  n->n = CMPNOP;
>+
>+  if (n->size < (int)sizeof (int64_t))
>+n->n &= ((uint64_t)1 << (n->size * BITS_PER_UNIT)) - 1;
>+
>+  return true;
>+}
>+
>/* Check if STMT might be a byte swap or a nop from a memory source and
>returns
>the answer. If so, REF is that memory source and the base of the memory
>area
>accessed and the offset of the access from that base are recorded in N.
> */
>@@ -1713,26 +1737,27 @@ find_bswap_or_nop_load (gimple stmt, tree ref,
>struct symbolic_number *n)
>   HOST_WIDE_INT bitsize, bitpos;
>   enum machine_mode mode;
>   int unsignedp, volatilep;
>+  tree offset, base_addr;
> 
>   if (!gimple_assign_load_p (stmt) || gimple_has_volatile_ops (stmt))
> return false;
> 
>-  n->base_addr = get_inner_reference (ref, &bitsize, &bitpos,
>&n->offset,
>-&mode, &unsignedp, &volatilep, false);
>+  base_addr = get_inner_reference (ref, &bitsize, &bitpos, &offset,
>&mode,
>+ &unsignedp, &volatilep, false);
> 
>-  if (TREE_CODE (n->base_addr) == MEM_REF)
>+  if (TREE_CODE (base_addr) == MEM_REF)
> {
>   offset_int bit_offset = 0;
>-  tree off = TREE_OPERAND (n->base_addr, 1);
>+  tree off = TREE_OPERAND (base_addr, 1);
> 
>   if (!integer_zerop (off))
>   {
>-offset_int boff, coff = mem_ref_offset (n->base_addr);
>+offset_int boff, coff = mem_ref_offset (base_addr);
> boff = wi::lshift (coff, LOG2_BITS_PER_UNIT);
> bit_offset += boff;
>   }
> 
>-  n->base_addr = TREE_OPERAND (n->base_addr, 0);
>+  base_addr = TREE_OPERAND (base_addr, 0);
> 
>/* Avoid returning a negative bitpos as this may wreak havoc later.  */
>   if (wi::neg_p (bit_offset))
>@@ -1743,11 +1768,11 @@ find_bswap_or_nop_load (gimple stmt, tree ref,
>struct symbolic_number *n)
>Subtract it to BIT_OFFSET and add it (scaled) to OFFSET.  */
> bit_offset -= tem;
> tem = wi::arshift (tem, LOG2_BITS_PER_UNIT);
>-if (n->offset)
>-  n->offset = size_binop (PLUS_EXPR, n->offset,
>+if (offset)
>+  offset = size_binop (PLUS_EXPR, offset,
>   wide_int_to_tree (sizetype, tem));
> else
>-  n->offset = wide_int_to_tree (sizetype, tem);
>+  offset = wide_int_to_tree (sizetype, tem);
>   }
> 
>   bitpos += bit_offset.to_shwi ();
>@@ -1758,6 +1783,9 @@ find_bswap_or_nop_load (gimple stmt, tree ref,
>struct symbolic_number *n)
>   if (bitsize % BITS_PER_UNIT)
> return false;
> 
>+  init_symbolic_number (n, ref);
>+  n->base_addr = base_addr;
>+  n->offset = offset;
>   n->bytepos = bitpos / BITS_PER_UNIT;
>   n->alias_set = reference_alias_ptr_type (ref);
>   n->vuse = gimple_vuse (stmt);
>@@ -1816,28 +1844,1

[PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

Hi all,

This is a second try on outline Asan instrumentation (original patch: 
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg02194.html).


This patch adds support for replacing Asan inline checks with function 
calls. This feature has been recently added to LLVM to
* reduce long compiler runtimes on large functions with huge (over 10K) 
number of memory accesses (http://llvm.org/bugs/show_bug.cgi?id=12653)

* a step to full Kasan support in GCC
* reduce code size overhead

Bootstrapped/regtested on x64.

-Y
2014-06-03  Yury Gribov  

	New asan-instrumentation-with-call-threshold parameter.

	gcc/
	* asan.c (check_func): New function.
	(build_check_stmt_sized): Likewise.
	(build_check_stmt): Add support for new parameter.
	(instrument_mem_region_access): Likewise.
	(asan_instrument): Likewise.
	* cfgcleanup.c (old_insns_match_p): Add support for new
	functions.
	* doc/invoke.texi: Describe new parameter.
	* params.def: Define new parameter.
	* params.h: Likewise.
	* sanitizer.def: Describe new builtins.

	gcc/testsuite/
	* c-c++-common/asan/instrument-with-calls-1.c: New test.
	* c-c++-common/asan/instrument-with-calls-2.c: Likewise.
	* c-c++-common/asan/instrument-with-calls-3.c: Likewise.

diff --git a/gcc/asan.c b/gcc/asan.c
index 3397655..1677c51 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -257,6 +257,8 @@ struct asan_mem_ref
 
 static alloc_pool asan_mem_ref_alloc_pool;
 
+static int asan_num_accesses;
+
 /* This creates the alloc pool used to store the instances of
asan_mem_ref that are stored in the hash table asan_mem_ref_ht.  */
 
@@ -1335,6 +1337,26 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
   return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
 }
 
+/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
+   IS_STORE is either 1 (for a store) or 0 (for a load).  */
+
+static tree
+check_func (bool is_store, int size_in_bytes, int slow_p)
+{
+  static enum built_in_function check[2][6]
+= { { BUILT_IN_ASAN_CHECK_LOAD1, BUILT_IN_ASAN_CHECK_LOAD2,
+  BUILT_IN_ASAN_CHECK_LOAD4, BUILT_IN_ASAN_CHECK_LOAD8,
+  BUILT_IN_ASAN_CHECK_LOAD16, BUILT_IN_ASAN_CHECK_LOAD_N },
+	{ BUILT_IN_ASAN_CHECK_STORE1, BUILT_IN_ASAN_CHECK_STORE2,
+  BUILT_IN_ASAN_CHECK_STORE4, BUILT_IN_ASAN_CHECK_STORE8,
+  BUILT_IN_ASAN_CHECK_STORE16, BUILT_IN_ASAN_CHECK_STORE_N } };
+  if ((size_in_bytes & (size_in_bytes - 1)) != 0
+  || size_in_bytes > 16
+  || slow_p)
+return builtin_decl_implicit (check[is_store][5]);
+  return builtin_decl_implicit (check[is_store][exact_log2 (size_in_bytes)]);
+}
+
 /* Split the current basic block and create a condition statement
insertion point right before or after the statement pointed to by
ITER.  Return an iterator to the point at which the caller might
@@ -1520,9 +1542,11 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
   tree shadow_type = TREE_TYPE (shadow_ptr_type);
   tree uintptr_type
 = build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
-  tree base_ssa = base;
+  tree base_ssa;
   HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
   tree sz_arg = NULL_TREE;
+  bool use_calls =
+asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
 
   if (size_in_bytes == 1)
 slow_p = false;
@@ -1534,15 +1558,27 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
   slow_p = true;
 }
 
-  /* Get an iterator on the point where we can add the condition
- statement for the instrumentation.  */
-  gsi = create_cond_insert_point (iter, before_p,
-  /*then_more_likely_p=*/false,
-  /*create_then_fallthru_edge=*/false,
-  &then_bb,
-  &else_bb);
+  base_ssa = base = unshare_expr (base);
 
-  base = unshare_expr (base);
+  if (use_calls)
+{
+  gsi = *iter;
+  g = gimple_build_nop ();
+  if (before_p)
+	gsi_insert_before (&gsi, g, GSI_NEW_STMT);
+  else
+	gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+}
+  else
+{
+  /* Get an iterator on the point where we can add the condition
+ statement for the instrumentation.  */
+  gsi = create_cond_insert_point (iter, before_p,
+	/*then_more_likely_p=*/false,
+	/*create_then_fallthru_edge=*/false,
+	&then_bb,
+	&else_bb);
+}
 
   /* BASE can already be an SSA_NAME; in that case, do not create a
  new SSA_NAME for it.  */
@@ -1563,6 +1599,22 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
   gsi_insert_after (&gsi, g, GSI_NEW_STMT);
   base_addr = gimple_assign_lhs (g);
 
+  if (use_calls)
+{
+  tree fun = check_func (is_store, size_in_bytes, slow_p);
+  g = slow_p
+	? gimple_build_call (fun, 1, base_addr)
+	: gimple_build_call (fun, 2, base_addr,
+			 build_int_cst (pointer_sized_int_node, size_in_bytes));
+  gimple_set_location (g, location);
+  gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+
+  if (

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Maxim Kuvyrkov]

On Jun 2, 2014, at 7:54 PM, Andreas Schwab  wrote:

> Regtested on ia64-suse-linux and installed as obvious.
> 
> Andreas.
> 
>   * config/ia64/ia64.c
>   (ia64_first_cycle_multipass_dfa_lookahead_guard): Check
>   pending_data_specs first.
> 
> diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
> index 118e5bf..4c5390b 100644
> --- a/gcc/config/ia64/ia64.c
> +++ b/gcc/config/ia64/ia64.c
> @@ -7536,7 +7536,7 @@ ia64_first_cycle_multipass_dfa_lookahead_guard (rtx 
> insn, int ready_index)
> 
>   /* Size of ALAT is 32.  As far as we perform conservative
>  data speculation, we keep ALAT half-empty.  */
> -  if ((TODO_SPEC (insn) & BEGIN_DATA) && pending_data_specs >= 16)
> +  if (pending_data_specs >= 16 && (TODO_SPEC (insn) & BEGIN_DATA))
> return ready_index == 0 ? -1 : 1;
> 
>   if (ready_index == 0)

Thanks for fixing this.  To make it a bit more robust I would suggest using 
same condition as in ia64_variable_issue():
===
  if (sched_deps_info->generate_spec_deps && !sel_sched_p ())
/* Modulo scheduling does not extend h_i_d when emitting
   new instructions.  Don't use h_i_d, if we don't have to.  */
{
  if (DONE_SPEC (insn) & BEGIN_DATA)
pending_data_specs++;
  if (CHECK_SPEC (insn) & BEGIN_DATA)
pending_data_specs--;
}
===

The underlaying problem is that TODO_SPEC(insn) should not be accessed for 
sel-sched.  Because pending_data_specs turns out to always be null during 
sel-sched, "pending_data_specs >= 16" can be used as a shorthand for
 "sched_deps_info->generate_spec_deps && !sel_sched_p () && pending_data_specs 
>= 16"

--
Maxim Kuvyrkov
www.linaro.org

Re: [RFC] optimize x - y cmp 0 with undefined overflow

[Eric Botcazou]

> Looks mostly ok.  Any reason why you are not re-creating
> MINUS_EXPR in build_symbolic_expr?  That is, build
> inv - t (for non-pointers, of course)?

It's more uniform and compare_values expects an INTEGER_CST on the RHS, 
although the patch was lacking a small tweak to the function:

@@ -1205,19 +1292,23 @@ compare_values_warnv (tree val1, tree va
   STRIP_USELESS_TYPE_CONVERSION (val2);
 
   if ((TREE_CODE (val1) == SSA_NAME
+   || (TREE_CODE (val1) == NEGATE_EXPR
+  && TREE_CODE (TREE_OPERAND (val1, 0)) == SSA_NAME)
|| TREE_CODE (val1) == PLUS_EXPR
|| TREE_CODE (val1) == MINUS_EXPR)
   && (TREE_CODE (val2) == SSA_NAME
+ || (TREE_CODE (val2) == NEGATE_EXPR
+ && TREE_CODE (TREE_OPERAND (val2, 0)) == SSA_NAME)
  || TREE_CODE (val2) == PLUS_EXPR
  || TREE_CODE (val2) == MINUS_EXPR))
 {
   tree n1, c1, n2, c2;
   enum tree_code code1, code2;
 
-  /* If VAL1 and VAL2 are of the form 'NAME [+-] CST' or 'NAME',
+  /* If VAL1 and VAL2 are of the form '[-]NAME [+-] CST' or 'NAME',
 return -1 or +1 accordingly.  If VAL1 and VAL2 don't use the
 same name, return -2.  */
-  if (TREE_CODE (val1) == SSA_NAME)
+  if (TREE_CODE (val1) == SSA_NAME || TREE_CODE (val1) == NEGATE_EXPR)
{
  code1 = SSA_NAME;
  n1 = val1;
@@ -1239,7 +1330,7 @@ compare_values_warnv (tree val1, tree va
}
}
 
-  if (TREE_CODE (val2) == SSA_NAME)
+  if (TREE_CODE (val2) == SSA_NAME || TREE_CODE (val2) == NEGATE_EXPR)
{
  code2 = SSA_NAME;
  n2 = val2;
@@ -1262,6 +1353,11 @@ compare_values_warnv (tree val1, tree va
}
 
   /* Both values must use the same name.  */
+  if (TREE_CODE (n1) == NEGATE_EXPR && TREE_CODE (n2) == NEGATE_EXPR)
+   {
+ n1 = TREE_OPERAND (n1, 0);
+ n2 = TREE_OPERAND (n2, 0);
+   }
   if (n1 != n2)
return -2;
 
> Otherwise if a range becomes -t + inv that will no longer match
> get_single_symbol for further propagation?

Yes, it will, NEGATE_EXPR is handled by get_single_symbol.

> Then I'm not sure if
> 
> +  /* Try with VR0 and [-INF, OP1].  */
> +  set_value_range (&new_vr1, VR_RANGE, vrp_val_min (expr_type), op1,
> NULL); +  extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
> &new_vr1); +  if (vr->type != VR_VARYING)
> +   return;
> +
> +  /* Try with VR0 and [OP1, +INF].  */
> +  set_value_range (&new_vr1, VR_RANGE, op1, vrp_val_max (expr_type),
> NULL); +  extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
> &new_vr1); +  if (vr->type != VR_VARYING)
> +   return;
> 
> is a safe thing to do.  If it does make a difference to try [-INF, OP1],
> [OP1, +INF] instead of just [OP1, OP1] then at least it's very suspicious ;)
> (or an "easy" missed optimization).

Yes, it makes a difference and is required to eliminate half-symbolic ranges 
(the ones deduced from x >= y).  Otherwise extract_range_from_binary_expr_1 
will reject the resulting range because it cannot compare the bounds.

As discussed, extract_range_from_binary_expr_1 doesn't do any fiddling with 
the input ranges, it just computes the resulting range and see whether it can 
interpret it.  Half-symbolic ranges cannot be interpreted and probably should 
not, in the general case at least, because I think it can be very delicate, so 
extract_range_from_binary_expr will just try all the possible combinations for 
PLUS and MINUS.

The testcases were attached to the first message in the thread, but I presume 
that decent mail programs are a thing of the past. :-)  Attached again.

-- 
Eric Botcazou-- { dg-do compile }
-- { dg-options "-O2 -fdump-tree-optimized" }

function Opt38 (X : Integer; Y : Integer ) return Positive is
   Z : Integer;
begin
   if X >= Y then
  return 1;
   end if;
   Z := Y - X;
   return Z;
end;

-- { dg-final { scan-tree-dump-not "gnat_rcheck" "optimized" } }
-- { dg-final { cleanup-tree-dump "optimized" } }/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

extern void abort (void);

int foo1 (int x, int y)
{
  int z;

  if (x >= y)
return 1;

  z = y - x;
  if (z <= 0)
abort ();

  return z;
}

unsigned int foo2 (unsigned int x, unsigned int y)
{
  unsigned int z;

  if (x >= y)
return 1;

  z = y - x;
  if (z == 0)
abort ();

  return z;
}

/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */

Re: [PATCH][match-and-simplify]

[Richard Biener]

On Mon, 2 Jun 2014, Marc Glisse wrote:

> 
> >(plus (bit_not @0) @0)
> >if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> >{ build_int_cst (TREE_TYPE (@0), -1); })
> > +(match_and_simplify
> > +  (plus @0 (bit_not @0))
> > +  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> > +  { build_int_cst (TREE_TYPE (@0), -1); })
> 
> Why not just:
> 
> (match_and_simplify
>   (plus @0 (bit_not @0))
>   { build_all_ones_cst (TREE_TYPE (@0)); })
> 
> ? Works for vector/complex, I don't know what type a bit_not_expr can have
> where the simplification wouldn't be true.

Sure.

Richard.

2014-06-03  Richard Biener  

* match.pd: Generalize ~A + A -> -1 simplification to all
types.

Index: gcc/match.pd
===
--- gcc/match.pd(revision 211135)
+++ gcc/match.pd(working copy)
@@ -185,12 +185,10 @@ to (minus @1 @0)
 /* ~A + A -> -1 */
 (match_and_simplify
   (plus (bit_not @0) @0)
-  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
-  { build_int_cst (TREE_TYPE (@0), -1); })
+  { build_all_ones_cst (type); })
 (match_and_simplify
   (plus @0 (bit_not @0))
-  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
-  { build_int_cst (TREE_TYPE (@0), -1); })
+  { build_all_ones_cst (type); })
 
 /* ~A + 1 -> -A */
 (match_and_simplify

Re: [PATCH/AARCH64v2 1/2] Factor out IF_THEN_ELSE case from aarch64_rtx_costs

[Marcus Shawcroft]

On 3 June 2014 02:05, Andrew Pinski  wrote:
> This factors out the IF_THEN_ELSE from aarch64_rtx_costs as that function was 
> getting too large.
>
> OK?  Build and tested for aarch64-elf with no regressions.

OK, Thanks /Marcus

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Jakub Jelinek]

On Tue, Jun 03, 2014 at 12:03:02PM +0400, Yury Gribov wrote:
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -257,6 +257,8 @@ struct asan_mem_ref
>  
>  static alloc_pool asan_mem_ref_alloc_pool;
>  
> +static int asan_num_accesses;
> +
>  /* This creates the alloc pool used to store the instances of
> asan_mem_ref that are stored in the hash table asan_mem_ref_ht.  */
>  
> @@ -1335,6 +1337,26 @@ report_error_func (bool is_store, HOST_WIDE_INT 
> size_in_bytes, bool slow_p)
>return builtin_decl_implicit (report[is_store][exact_log2 
> (size_in_bytes)]);
>  }
>  
> +/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
> +   IS_STORE is either 1 (for a store) or 0 (for a load).  */
> +
> +static tree
> +check_func (bool is_store, int size_in_bytes, int slow_p)
> +{
> +  static enum built_in_function check[2][6]
> += { { BUILT_IN_ASAN_CHECK_LOAD1, BUILT_IN_ASAN_CHECK_LOAD2,
> +  BUILT_IN_ASAN_CHECK_LOAD4, BUILT_IN_ASAN_CHECK_LOAD8,
> +  BUILT_IN_ASAN_CHECK_LOAD16, BUILT_IN_ASAN_CHECK_LOAD_N },
> + { BUILT_IN_ASAN_CHECK_STORE1, BUILT_IN_ASAN_CHECK_STORE2,
> +  BUILT_IN_ASAN_CHECK_STORE4, BUILT_IN_ASAN_CHECK_STORE8,
> +  BUILT_IN_ASAN_CHECK_STORE16, BUILT_IN_ASAN_CHECK_STORE_N } };

Any reason why the BUILT_IN_* names so differ from the actual function
names?  I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
(no underscore before N, no CHECK_)?

> @@ -1520,9 +1542,11 @@ build_check_stmt (location_t location, tree base, 
> gimple_stmt_iterator *iter,
>tree shadow_type = TREE_TYPE (shadow_ptr_type);
>tree uintptr_type
>  = build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
> -  tree base_ssa = base;
> +  tree base_ssa;
>HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
>tree sz_arg = NULL_TREE;
> +  bool use_calls =
> +asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;

Wouldn't it be better to do
  bool use_calls
= asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
(note, = on next line per GNU Coding Conventions), and then
  if (!use_calls)
asan_num_accesses++;
so that you don't risk signed overflow?  Maybe also
  if (ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD == INT_MAX)
use_calls = false;
before that, so that there is a way to turn the calls off completely.

> @@ -1534,15 +1558,27 @@ build_check_stmt (location_t location, tree base, 
> gimple_stmt_iterator *iter,
>slow_p = true;
>  }
>  
> -  /* Get an iterator on the point where we can add the condition
> - statement for the instrumentation.  */
> -  gsi = create_cond_insert_point (iter, before_p,
> -   /*then_more_likely_p=*/false,
> -   /*create_then_fallthru_edge=*/false,
> -   &then_bb,
> -   &else_bb);
> +  base_ssa = base = unshare_expr (base);
>  
> -  base = unshare_expr (base);
> +  if (use_calls)
> +{
> +  gsi = *iter;
> +  g = gimple_build_nop ();
> +  if (before_p)
> + gsi_insert_before (&gsi, g, GSI_NEW_STMT);
> +  else
> + gsi_insert_after (&gsi, g, GSI_NEW_STMT);

This looks ugly.  I don't like adding GIMPLE_NOP, it is going
to take many passes before it is going to be DCEd.
Just handle the use_calls && before_p case in the two spots
where a new stmt is inserted.

Also note (but, already preexisting issue) is that the
__asan_report* and __asan_{load,store}* calls are declared in
sanitizer.def as having 1st argument PTR type, while in the library
it is uptr (aka PTRMODE).  So, either we should fix up sanitizer.def
to match that (we were passing already base_addr which is integral,
not pointer), or if we keep sanitizer.def as is, we should pass
to the calls base_ssa instead of base_addr.

> @@ -1642,6 +1694,61 @@ build_check_stmt (location_t location, tree base, 
> gimple_stmt_iterator *iter,
>*iter = gsi_start_bb (else_bb);
>  }
>  
> +static void
> +build_check_stmt_sized (location_t location, tree base, tree len,
> +   gimple_stmt_iterator *iter, bool is_store)

Formatting, gimple_stmt_iterator needs to be below location_t.

Furthermore, this looks just like a partial transition, I don't see
why we should emit one __asan_loadN call but two separate
__asan_report_load_1 for the instrumented stringops.

Instead, build_check_stmt should be changed, so that it is possible to pass
it a variable length (checked by the caller for non-zero already),
and in that case force using the slow_p stuff, but instead of adding a
constant size add the variable length - 1.

> @@ -1774,6 +1881,9 @@ instrument_mem_region_access (tree base, tree len,
> gimple_stmt_iterator *iter,
> location_t location, bool is_store)
>  {
> +  bool use_calls =
> +asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
> +

See above.

> @@ -1795,6 +1905,14 @@ instrument_mem_region_access (tree base, tree len,
>

Re: [PATCH/AARCH64v2 2/2] Fix PR 61345: rtx_cost ICEing on simple code

[Marcus Shawcroft]

On 3 June 2014 02:05, Andrew Pinski  wrote:

> - if (GET_CODE (op0) == NE
> - || GET_CODE (op0) == EQ)
> + if (cmpcode == NE
> + || cmpcode == EQ)

Those two can go back on one line ?

> }
> - else if (GET_CODE (op0) == LT
> -  || GET_CODE (op0) == GE)
> + else if (cmpcode == LT
> +  || cmpcode == GE)

Likewise?

OK

/Marcus

Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls

[Richard Biener]

On Mon, Jun 2, 2014 at 4:51 PM, Ilya Enkovich  wrote:
> Hi,
>
> This patch adds support for normal builtin function calls (target ones are 
> not instrumented).  The basic idea of the patch is to make call expr copy 
> with no bounds and expand it instead.  If expr is going to be emitted as a 
> function call then original instrumented expr takes place.  Handle string 
> function separately because they are of high interest for the checker.

It seems to be this should be user-controllable (always expand builtins with
bounds as calls, or not), rather than deciding what is of interest yourself.

From a high-level perspective the expansion path doing inline expansion
should be separated from that expanding as a call (or expanding as a
different call).  I don't like that building of yet another GENERIC call expr
in this code ... it goes very much backwards of the idea that we should
expand from the GIMPLE representation.  You are making such transition
even harder.

Can't you do that frobbing from cfgexpand.c:expand_call_stmt instead (please!)?

Thanks,
Richard.

> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-02  Ilya Enkovich  
>
> * builtins.c: Include rtl-chkp.h, tree-chkp.h.
> (expand_builtin_mempcpy_args): Add orig exp as argument.
> Support BUILT_IN_CHKP_MEMPCPY_NOBND_NOCHK.
> (expand_builtin_mempcpy): Adjust expand_builtin_mempcpy_args call.
> (expand_builtin_stpcpy): Likewise.
> (expand_builtin_memset_args): Support 
> BUILT_IN_CHKP_MEMSET_NOBND_NOCHK.
> (std_expand_builtin_va_start): Initialize bounds for va_list.
> (expand_builtin): Support instrumented calls.
>
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 7357124..c0140e1 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -59,6 +59,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "builtins.h"
>  #include "ubsan.h"
>  #include "cilk.h"
> +#include "tree-chkp.h"
> +#include "rtl-chkp.h"
>
>
>  static tree do_mpc_arg1 (tree, tree, int (*)(mpc_ptr, mpc_srcptr, 
> mpc_rnd_t));
> @@ -124,7 +126,7 @@ static rtx builtin_memcpy_read_str (void *, 
> HOST_WIDE_INT, enum machine_mode);
>  static rtx expand_builtin_memcpy (tree, rtx);
>  static rtx expand_builtin_mempcpy (tree, rtx, enum machine_mode);
>  static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx,
> -   enum machine_mode, int);
> +   enum machine_mode, int, tree);
>  static rtx expand_builtin_strcpy (tree, rtx);
>  static rtx expand_builtin_strcpy_args (tree, tree, rtx);
>  static rtx expand_builtin_stpcpy (tree, rtx, enum machine_mode);
> @@ -3284,7 +3286,8 @@ expand_builtin_mempcpy (tree exp, rtx target, enum 
> machine_mode mode)
>tree src = CALL_EXPR_ARG (exp, 1);
>tree len = CALL_EXPR_ARG (exp, 2);
>return expand_builtin_mempcpy_args (dest, src, len,
> - target, mode, /*endp=*/ 1);
> + target, mode, /*endp=*/ 1,
> + exp);
>  }
>  }
>
> @@ -3296,10 +3299,23 @@ expand_builtin_mempcpy (tree exp, rtx target, enum 
> machine_mode mode)
>
>  static rtx
>  expand_builtin_mempcpy_args (tree dest, tree src, tree len,
> -rtx target, enum machine_mode mode, int endp)
> +rtx target, enum machine_mode mode, int endp,
> +tree orig_exp)
>  {
> +  tree fndecl = get_callee_fndecl (orig_exp);
> +
>  /* If return value is ignored, transform mempcpy into memcpy.  */
> -  if (target == const0_rtx && builtin_decl_implicit_p (BUILT_IN_MEMCPY))
> +  if (target == const0_rtx
> +  && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CHKP_MEMPCPY_NOBND_NOCHK
> +  && builtin_decl_implicit_p (BUILT_IN_CHKP_MEMCPY_NOBND_NOCHK))
> +{
> +  tree fn = builtin_decl_implicit (BUILT_IN_CHKP_MEMCPY_NOBND_NOCHK);
> +  tree result = build_call_nofold_loc (UNKNOWN_LOCATION, fn, 3,
> +  dest, src, len);
> +  return expand_expr (result, target, mode, EXPAND_NORMAL);
> +}
> +  else if (target == const0_rtx
> +  && builtin_decl_implicit_p (BUILT_IN_MEMCPY))
>  {
>tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY);
>tree result = build_call_nofold_loc (UNKNOWN_LOCATION, fn, 3,
> @@ -3484,7 +3500,8 @@ expand_builtin_stpcpy (tree exp, rtx target, enum 
> machine_mode mode)
>
>lenp1 = size_binop_loc (loc, PLUS_EXPR, len, ssize_int (1));
>ret = expand_builtin_mempcpy_args (dst, src, lenp1,
> -target, mode, /*endp=*/2);
> +target, mode, /*endp=*/2,
> +exp);
>
>if (ret)
> return ret;
> @@ -3778,7 +3795,8 @@ expand_builtin_memset_args (tree dest, tree val, tree 
> le

Re: [PATCH, Pointer Bounds Checker 14/x] Pointer Bounds Checker passes

[Richard Biener]

On Mon, Jun 2, 2014 at 5:13 PM, Ilya Enkovich  wrote:
> 2014-06-02 17:37 GMT+04:00 Richard Biener :
>> On Mon, Jun 2, 2014 at 2:44 PM, Ilya Enkovich  wrote:
>>> 2014-06-02 15:35 GMT+04:00 Richard Biener :
 On Fri, May 30, 2014 at 2:25 PM, Ilya Enkovich  
 wrote:
> Hi,
>
> This patch adds Pointer Bounds Checker passes.  Versioning happens before 
> early local passes.  Earply local passes are split into 3 stages to have 
> everything instrumented before any optimization applies.

 That looks artificial to me.  If you need to split up early_local_passes
 then do that - nesting three IPA pass groups inside it looks odd to me.
 Btw - doing this in three "IPA phases" makes things possibly slower
 due to cache effects.  It might be worth pursuing to move the early
 stage completely to the lowering pipeline.
>>>
>>> Early local passes is some special case because these passes are
>>> executed separately for new functions. I did not want to get three
>>> special passes instead and therefore made split inside.
>>
>> Yeah, but all passes are already executed via execute_early_local_passes,
>> so it would be only an implementation detail.
>>
>>> If you prefer split pass itself, I suppose pass_early_local_passes may
>>> be replaced with something like pass_build_ssa_passes +
>>> pass_chkp_instrumentation_passes + pass_ipa_chkp_produce_thunks +
>>> pass_local_optimization_passes. execute_early_local_passes would
>>> execute gimple passes lists of pass_build_ssa_passes,
>>> pass_chkp_instrumentation_passes and pass_local_optimization_passes.
>>>
>>> I think we cannot have the first stage moved into lowering passes
>>> because it should be executed for newly created functions.
>>
>> Well, let's defer that then.
>>

 Btw, fixup_cfg only needs to run once local_pure_const was run
 on a callee, thus it shouldn't be neccessary to run it from the
 first group.
>>>
>>> OK. Will try to remove it from the first group.
>>>

  void
  pass_manager::execute_early_local_passes ()
  {
 -  execute_pass_list (pass_early_local_passes_1->sub);
 +  execute_pass_list (pass_early_local_passes_1->sub->sub);
 +  execute_pass_list (pass_early_local_passes_1->sub->next->sub);
 +  execute_pass_list 
 (pass_early_local_passes_1->sub->next->next->next->sub);
  }

 is gross - it should be enough to execute the early local pass list
 (obsolete comment with the suggestion above).
>>>
>>> This function should call only gimple passes for cfun. Therefore we
>>> cannot call IPA passes here and has to execute each gimple passes list
>>> separately.
>>
>> Ok, given a different split this would then become somewhat more sane
>> anyway.
>
> Sorry, didn't catch it. Should I try a different split or defer it? :)

Please try a different split.  Defer moving the first part to the
lowering stage.

Richard.

> Ilya
>
>>
>> Richard.
>>
>>> Ilya

 Richard.

> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-05-29  Ilya Enkovich  
>
> * tree-chkp.c: New.
> * tree-chkp.h: New.
> * rtl-chkp.c: New.
> * rtl-chkp.h: New.
> * Makefile.in (OBJS): Add tree-chkp.o, rtl-chkp.o.
> (GTFILES): Add tree-chkp.c.
> * c-family/c.opt (fchkp-check-incomplete-type): New.
> (fchkp-zero-input-bounds-for-main): New.
> (fchkp-first-field-has-own-bounds): New.
> (fchkp-narrow-bounds): New.
> (fchkp-narrow-to-innermost-array): New.
> (fchkp-optimize): New.
> (fchkp-use-fast-string-functions): New.
> (fchkp-use-nochk-string-functions): New.
> (fchkp-use-static-bounds): New.
> (fchkp-use-static-const-bounds): New.
> (fchkp-treat-zero-dynamic-size-as-infinite): New.
> (fchkp-check-read): New.
> (fchkp-check-write): New.
> (fchkp-store-bounds): New.
> (fchkp-instrument-calls): New.
> (fchkp-instrument-marked-only): New.
> * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add
> __CHKP__ macro when Pointer Bounds Checker is on.
> * passes.def (pass_ipa_chkp_versioning): New.
> (pass_before_local_optimization_passes): New.
> (pass_chkp_instrumentation_passes): New.
> (pass_ipa_chkp_produce_thunks): New.
> (pass_local_optimization_passes): New.
> (pass_chkp_opt): New.
> * toplev.c: include tree-chkp.h.
> (compile_file): Add chkp_finish_file call.
> * tree-pass.h (make_pass_ipa_chkp_versioning): New.
> (make_pass_ipa_chkp_produce_thunks): New.
> (make_pass_chkp): New.
> (make_pass_chkp_opt): New.
> (make_pass_before_local_optimization_passes): New.
>

Re: [PATCH, Pointer Bounds Checker 20/x] Follow transparent alias chains

[Richard Biener]

On Mon, Jun 2, 2014 at 5:15 PM, Ilya Enkovich  wrote:
> Hi,
>
> In the most case we follow transparent alias chains wne assemble names.  But 
> in some cases it is not performed.  For instrumented functions it is critical 
> and following patch fixes that.  It also adds a visibility inheritance for 
> instrtumented functions.

It feels like this should be handled by the symtab code nowadays ... Honza?

Richard.

> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-02  Ilya Enkovich  
>
> * varasm.c: Include tree-chkp.h.
> (ultimate_transparent_alias_target): Move up.
> (make_decl_rtl): For instrumented function use
> name of the original decl.
> (assemble_start_function): Mark function as global
> in case it is instrumentation clone of the global
> function.
> (do_assemble_alias): Follow transparent alias chain
> for identifier.  Check if original alias is public.
> (maybe_assemble_visibility): Use visibility of the
> original function for instrumented version.
> (default_unique_section): Likewise.
>
>
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index fcae2fa..d473bc7 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "pointer-set.h"
>  #include "asan.h"
>  #include "basic-block.h"
> +#include "tree-chkp.h"
>
>  #ifdef XCOFF_DEBUGGING_INFO
>  #include "xcoffout.h"  /* Needed for external data
> @@ -1200,6 +1201,30 @@ use_blocks_for_decl_p (tree decl)
>return targetm.use_blocks_for_decl_p (decl);
>  }
>
> +/* Follow the IDENTIFIER_TRANSPARENT_ALIAS chain starting at *ALIAS
> +   until we find an identifier that is not itself a transparent alias.
> +   Modify the alias passed to it by reference (and all aliases on the
> +   way to the ultimate target), such that they do not have to be
> +   followed again, and return the ultimate target of the alias
> +   chain.  */
> +
> +static inline tree
> +ultimate_transparent_alias_target (tree *alias)
> +{
> +  tree target = *alias;
> +
> +  if (IDENTIFIER_TRANSPARENT_ALIAS (target))
> +{
> +  gcc_assert (TREE_CHAIN (target));
> +  target = ultimate_transparent_alias_target (&TREE_CHAIN (target));
> +  gcc_assert (! IDENTIFIER_TRANSPARENT_ALIAS (target)
> + && ! TREE_CHAIN (target));
> +  *alias = target;
> +}
> +
> +  return target;
> +}
> +
>  /* Create the DECL_RTL for a VAR_DECL or FUNCTION_DECL.  DECL should
> have static storage duration.  In other words, it should not be an
> automatic variable, including PARM_DECLs.
> @@ -1214,6 +1239,7 @@ make_decl_rtl (tree decl)
>  {
>const char *name = 0;
>int reg_number;
> +  tree id;
>rtx x;
>
>/* Check that we are not being given an automatic variable.  */
> @@ -1271,7 +1297,12 @@ make_decl_rtl (tree decl)
>return;
>  }
>
> -  name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> +  id = DECL_ASSEMBLER_NAME (decl);
> +  if (TREE_CODE (decl) == FUNCTION_DECL
> +  && cgraph_get_node (decl)
> +  && cgraph_get_node (decl)->instrumentation_clone)
> +ultimate_transparent_alias_target (&id);
> +  name = IDENTIFIER_POINTER (id);
>
>if (name[0] != '*' && TREE_CODE (decl) != FUNCTION_DECL
>&& DECL_REGISTER (decl))
> @@ -1699,7 +1730,10 @@ assemble_start_function (tree decl, const char *fnname)
>
>/* Make function name accessible from other files, if appropriate.  */
>
> -  if (TREE_PUBLIC (decl))
> +  if (TREE_PUBLIC (decl)
> +  || (cgraph_get_node (decl)->instrumentation_clone
> + && cgraph_get_node (decl)->instrumented_version
> + && TREE_PUBLIC (cgraph_get_node 
> (decl)->instrumented_version->decl)))
>  {
>notice_global_symbol (decl);
>
> @@ -2386,30 +2420,6 @@ mark_decl_referenced (tree decl)
>  }
>
>
> -/* Follow the IDENTIFIER_TRANSPARENT_ALIAS chain starting at *ALIAS
> -   until we find an identifier that is not itself a transparent alias.
> -   Modify the alias passed to it by reference (and all aliases on the
> -   way to the ultimate target), such that they do not have to be
> -   followed again, and return the ultimate target of the alias
> -   chain.  */
> -
> -static inline tree
> -ultimate_transparent_alias_target (tree *alias)
> -{
> -  tree target = *alias;
> -
> -  if (IDENTIFIER_TRANSPARENT_ALIAS (target))
> -{
> -  gcc_assert (TREE_CHAIN (target));
> -  target = ultimate_transparent_alias_target (&TREE_CHAIN (target));
> -  gcc_assert (! IDENTIFIER_TRANSPARENT_ALIAS (target)
> - && ! TREE_CHAIN (target));
> -  *alias = target;
> -}
> -
> -  return target;
> -}
> -
>  /* Output to FILE (an assembly file) a reference to NAME.  If NAME
> starts with a *, the rest of NAME is output verbatim.  Otherwise
> NAME is transformed in a target-specific way (usually by the
> @@ -5544,6 +5554,9 @@ vec *alias_pairs;
>  v

Re: [PATCH, Pointer Bounds Checker 21/x] Weakrefs output

[Richard Biener]

On Mon, Jun 2, 2014 at 5:22 PM, Ilya Enkovich  wrote:
> Hi,
>
> This patch prevents output of both instrumented and not instrumented weakref 
> variants.

Shouldn't one of them be reclaimed instead at some point?

Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-02  Ilya Enkovich  
>
> * cgraphunit.c (output_weakrefs): If there are both
> instrumented and original versions, output only one
> of them.
>
>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index c5c..ae9e699 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -2111,9 +2111,13 @@ static void
>  output_weakrefs (void)
>  {
>symtab_node *node;
> +  cgraph_node *cnode;
>FOR_EACH_SYMBOL (node)
>  if (node->alias
>  && !TREE_ASM_WRITTEN (node->decl)
> +   && (!(cnode = dyn_cast  (node))
> +   || !cnode->instrumented_version
> +   || !TREE_ASM_WRITTEN (cnode->instrumented_version->decl))
> && node->weakref)
>{
> tree target;

Re: [PATCH, Pointer Bounds Checker 22/x] Inline

[Richard Biener]

On Mon, Jun 2, 2014 at 5:56 PM, Ilya Enkovich  wrote:
> Hi,
>
> This patch adds support for inlining instrumented calls.  Changes are mostly 
> to support returned bounds.  Also generated mem-to-mem assignments are 
> registered to be later instrumented with appropriate bounds copy.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-02  Ilya Enkovich  
>
> * ipa-inline.c (early_inliner): Check edge has summary allocated.
> * tree-inline.c: Include tree-chkp.h.
> (declare_return_variable): Add arg holding
> returned bounds slot.  Create and initialize returned bounds var.
> (remap_gimple_stmt): Handle returned bounds.
> Return sequence of statements instead of a single statement.
> (insert_init_stmt): Add declaration.
> (remap_gimple_seq): Adjust to new remap_gimple_stmt signature.
> (copy_bb): Adjust to changed return type of remap_gimple_stmt.
> (expand_call_inline): Handle returned bounds.  Add bounds copy
> for generated mem to mem assignments.
> * tree-inline.h (copy_body_data): Add fields retbnd and
> assign_stmts.
> * cgraph.c: Include tree-chkp.h.
> (cgraph_redirect_edge_call_stmt_to_callee): Support
> returned bounds.
> * value-prof.c: Include tree-chkp.h.
> (gimple_ic): Support returned bounds.
>
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 1f684c2..4b6996b 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "gimple-pretty-print.h"
>  #include "expr.h"
>  #include "tree-dfa.h"
> +#include "tree-chkp.h"
>
>  /* FIXME: Only for PROP_loops, but cgraph shouldn't have to know about this. 
>  */
>  #include "tree-pass.h"
> @@ -1398,6 +1399,31 @@ cgraph_redirect_edge_call_stmt_to_callee (struct 
> cgraph_edge *e)
>   e->speculative = false;
>   cgraph_set_call_stmt_including_clones (e->caller, e->call_stmt,
>  new_stmt, false);
> +
> + /* Fix edges for BUILT_IN_CHKP_BNDRET calls attached to the
> +processed call stmt.  */
> + if (gimple_call_with_bounds_p (new_stmt)
> + && gimple_call_lhs (new_stmt)
> + && chkp_retbnd_call_by_val (gimple_call_lhs (e2->call_stmt)))
> +   {
> + tree dresult = gimple_call_lhs (new_stmt);
> + tree iresult = gimple_call_lhs (e2->call_stmt);
> + gimple dbndret = chkp_retbnd_call_by_val (dresult);
> + gimple ibndret = chkp_retbnd_call_by_val (iresult);
> + struct cgraph_edge *iedge = cgraph_edge (e2->caller, ibndret);
> + struct cgraph_edge *dedge;
> +
> + if (dbndret)
> +   {
> + dedge = cgraph_create_edge (iedge->caller, iedge->callee,
> + dbndret, e->count,
> + e->frequency);
> + dedge->frequency = compute_call_stmt_bb_frequency
> +   (dedge->caller->decl, gimple_bb (dedge->call_stmt));
> +   }
> + iedge->frequency = compute_call_stmt_bb_frequency
> +   (iedge->caller->decl, gimple_bb (iedge->call_stmt));
> +   }
>   e->frequency = compute_call_stmt_bb_frequency
>(e->caller->decl, gimple_bb (e->call_stmt));
>   e2->frequency = compute_call_stmt_bb_frequency
> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> index 4051819..a6fc853 100644
> --- a/gcc/ipa-inline.c
> +++ b/gcc/ipa-inline.c
> @@ -2301,11 +2301,15 @@ early_inliner (void)
>  info that might be cleared out for newly discovered edges.  */
>   for (edge = node->callees; edge; edge = edge->next_callee)
> {
> - struct inline_edge_summary *es = inline_edge_summary (edge);
> - es->call_stmt_size
> -   = estimate_num_insns (edge->call_stmt, &eni_size_weights);
> - es->call_stmt_time
> -   = estimate_num_insns (edge->call_stmt, &eni_time_weights);
> + /* We have no summary for new bound store calls yet.  */
> + if (inline_edge_summary_vec.length () > (unsigned)edge->uid)
> +   {
> + struct inline_edge_summary *es = inline_edge_summary (edge);
> + es->call_stmt_size
> +   = estimate_num_insns (edge->call_stmt, &eni_size_weights);
> + es->call_stmt_time
> +   = estimate_num_insns (edge->call_stmt, &eni_time_weights);
> +   }
>   if (edge->callee->decl
>   && !gimple_check_call_matching_types (
>   edge->call_stmt, edge->callee->decl, false))
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 23fef90..6557a95 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc

Re: Eliminate write-only variables

[Richard Biener]

On Mon, Jun 2, 2014 at 8:59 PM, Jan Hubicka  wrote:
>>
>> Yeah, I discussed this with martin today on irc.  For aliasing we'd like to 
>> know whether a decl possibly has its address taken. Currently we only trust 
>> TREE_ADDRESSABLE for statics - and lto might change those to hidden 
>> visibility public :(
>>
>> So we want deck_referred_to_by_single_function
>
> OK, I will implement this one in IPA mini-pass.  It is easy.
> This property changes by clonning and inlining. What Martin wants to use it 
> for?
> (I.e. my plan would be to compute it as last IPA pass making it useless for
> IPA analysis&propagation)

He doesn't want to use it but we talked and he said he'd have a look.

Note that it's important the decls are not refered to by global initializers
either.

Why is a new pass necessary - can't the IPA reference maintaining code
update some flag in the varpool magically?

>> and deck_may_have_address_taken.
>
> We currently clear TREE_ADDRESSABLE for statics that have no address taken at
> WPA time and that ought to keep the flag cleared at ltrans (I think I even 
> made
> testcase for this)
>
> What I think we could improve is to not consider string functions as ADDR 
> operations
> for this analysis, i.e. it is common to only memset to 0.
>
> How may_have_address_taken would differ here?

I want tree.h:may_be_aliased to return false if a decl doesn't have its address
taken.  We don't trust TREE_ADDRESSABLE for TREE_PUBLIC/DECL_EXTERNAL
decls because other TUs may take the address.  I want the check to be replaced
with sth more symtab aware, that is, bring in benefits from LTO (and at the same
time be not confused by statics brought public with hidden visibility).

Richard.

> Honza

Re: Eliminate write-only variables

[Martin Jambor]

On Mon, Jun 02, 2014 at 08:59:35PM +0200, Jan Hubicka wrote:
> > 
> > Yeah, I discussed this with martin today on irc.  For aliasing we'd like to 
> > know whether a decl possibly has its address taken. Currently we only trust 
> > TREE_ADDRESSABLE for statics - and lto might change those to hidden 
> > visibility public :(
> > 
> > So we want deck_referred_to_by_single_function 
> 
> OK, I will implement this one in IPA mini-pass.  It is easy.
> This property changes by clonning and inlining. What Martin wants to use it 
> for?
> (I.e. my plan would be to compute it as last IPA pass making it useless for
> IPA analysis&propagation)

That is a misunderstanding, I don't plan to use it for anything.  It
was just a part of a discussion about this thread where I simply
proposed exactly the same thing as you did now.

Martin

> 
> > and deck_may_have_address_taken.
> 
> We currently clear TREE_ADDRESSABLE for statics that have no address taken at
> WPA time and that ought to keep the flag cleared at ltrans (I think I even 
> made
> testcase for this)
> 
> What I think we could improve is to not consider string functions as ADDR 
> operations
> for this analysis, i.e. it is common to only memset to 0.
> 
> How may_have_address_taken would differ here?
> 
> Honza

Re: [PATCH] Fix ICE due to memory corruption in SRA

[Richard Biener]

On Mon, Jun 2, 2014 at 11:21 PM, Teresa Johnson  wrote:
> This patch fixes an ICE due to memory corruption discovered while building a
> large application with FDO and LIPO on the google branch. I don't have a small
> reproducer, but the same code appears on trunk, and I believe it could also
> silently result in incorrect code generation.
>
> The problem occurs if SRA is applied on a recursive call. In this case,
> the redirect_callers vec below contains the recursive edge from node->node.
> When rebuild_cgraph_edges is invoked, it will free the callee edges of node,
> including the one recorded in redirect_callers. In the case I looked at,
> after rebuilding the cgraph edges for node, the address recorded in
> redirect_callers now pointed to a different cgraph edge, and we later
> got an ICE because the (incorrect) callee that we tried to modify had
> the wrong number of arguments.
>
> To fix, I simply moved the collection of caller edges to after the cgraph
> edge rebuilding.
>
> Google ref b/15383777.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Teresa
>
> 2014-06-02  Teresa Johnson  
>
> * tree-sra.c (modify_function): Record caller nodes after rebuild.
>
> Index: tree-sra.c
> ===
> --- tree-sra.c  (revision 211139)
> +++ tree-sra.c  (working copy)
> @@ -4925,12 +4925,15 @@ modify_function (struct cgraph_node *node, ipa_par
>  {
>struct cgraph_node *new_node;
>bool cfg_changed;
> -  vec redirect_callers = collect_callers_of_node (node);
>
>rebuild_cgraph_edges ();
>free_dominance_info (CDI_DOMINATORS);
>pop_cfun ();
>
> +  /* This must be done after rebuilding cgraph edges for node above.
> + Otherwise any recursive calls to node that are recorded in
> + redirect_callers will be corrupted.  */
> +  vec redirect_callers = collect_callers_of_node (node);
>new_node = cgraph_function_versioning (node, redirect_callers,
>  NULL,
>  NULL, false, NULL, NULL, "isra");
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: RFA: PATCH to ctor_for_folding for c++/61020 (ICE with typeid)

[Richard Biener]

On Tue, Jun 3, 2014 at 3:39 AM, Jason Merrill  wrote:
> The C++ front end wants to be able to build up objects of the various
> typeinfo derived classes (such as __si_class_type_info) without needing to
> see the full definition of the class.  As part of this we build a VAR_DECL
> for the vtable, but never define it because we don't actually have
> declarations of the virtual functions, we're only using it for external
> references.  ctor_for_folding was assuming that all vtables are defined,
> which broke on this case.
>
> Tested x86_64-pc-linux-gnu.  OK for trunk?

Ok.

Thanks,
Richard.

Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning

[Richard Biener]

On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich  wrote:
> 2014-06-02 21:27 GMT+04:00 Jeff Law :
>> On 06/02/14 04:48, Ilya Enkovich wrote:

 Hmm, so if I understand things correctly, src_fun has no loop
 structures attached, thus there's nothing to copy.  Presumably at
 some later point we build loop structures for the copy from scratch?
>>>
>>> I suppose it is just a simple bug with absent NULL pointer check.  Here is
>>> original code:
>>>
>>>/* Duplicate the loop tree, if available and wanted.  */
>>>if (loops_for_fn (src_cfun) != NULL
>>>&& current_loops != NULL)
>>>  {
>>>copy_loops (id, entry_block_map->loop_father,
>>>get_loop (src_cfun, 0));
>>>/* Defer to cfgcleanup to update loop-father fields of
>>> basic-blocks.  */
>>>loops_state_set (LOOPS_NEED_FIXUP);
>>>  }
>>>
>>>/* If the loop tree in the source function needed fixup, mark the
>>>   destination loop tree for fixup, too.  */
>>>if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>>>  loops_state_set (LOOPS_NEED_FIXUP);
>>>
>>> As you may see we have check for absent loops structure in the first
>>> if-statement and no check in the second one.  I hit segfault and added the
>>> check.
>>
>> Downthread you indicated you're not in SSA form which might explain the
>> inconsistency here.  If so, then we need to make sure that the loop & df
>> structures do get set up properly later.
>
> That is what init_data_structures pass will do for us as Richard pointed. 
> Right?

loops are set up during the CFG construction and thus are available everywhere.
the df structures are set up in init_data_structures pass which is run before
going into SSA form (I'd like to somehow cleanup that area).

Richard.

> Ilya
>
>>
>> Jeff

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Yury Gribov]

Any reason why the BUILT_IN_* names so differ from the actual function
names?  I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
(no underscore before N, no CHECK_)?


Makes sense.


Wouldn't it be better to do
...
so that you don't risk signed overflow?  Maybe also


Yeah, that looks cleaner.


Also note (but, already preexisting issue) is that the
__asan_report* and __asan_{load,store}* calls are declared in
sanitizer.def as having 1st argument PTR type, while in the library
it is uptr (aka PTRMODE).  So, either we should fix up sanitizer.def
to match that (we were passing already base_addr which is integral,
not pointer), or if we keep sanitizer.def as is, we should pass
to the calls base_ssa instead of base_addr.


Which solution is better for middle-end? Would casting to uptr 
complicate alias analysis or somehow damage optimization capabilities of 
further passes?


Frankly I don't understand why libsanitizer prefers uptr to void*'s and 
size_t's...



Furthermore, this looks just like a partial transition, I don't see
why we should emit one __asan_loadN call but two separate
__asan_report_load_1 for the instrumented stringops.


I was too lazy to bother with strlen. But ok, I'll optimize this piece.


Instead, build_check_stmt should be changed, so that it is possible to pass
it a variable length (checked by the caller for non-zero already),
and in that case force using the slow_p stuff, but instead of adding a
constant size add the variable length - 1.


+1, this will simplify code.

-Y

Re: [PATCH, Pointer Bounds Checker 24/x] PRE

[Richard Biener]

On Tue, Jun 3, 2014 at 9:13 AM, Ilya Enkovich  wrote:
> Hi,
>
> This patch preserves CALL_WITH_BOUNDS flag for calls during PRE.

Ok.

Richard.

> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-03  Ilya Enkovich  
>
> * tree-ssa-pre.c (create_component_ref_by_pieces_1): Store
> CALL_WITH_BOUNDS_P for calls.
> (copy_reference_ops_from_call): Restore CALL_WITH_BOUNDS_P
> flag.
>
>
> diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
> index 1e55356..d5b9f3b 100644
> --- a/gcc/tree-ssa-pre.c
> +++ b/gcc/tree-ssa-pre.c
> @@ -2579,6 +2579,8 @@ create_component_ref_by_pieces_1 (basic_block block, 
> vn_reference_t ref,
>(TREE_CODE (fn) == FUNCTION_DECL
> ? build_fold_addr_expr (fn) : fn),
>nargs, args);
> +   if (currop->op2 == integer_one_node)
> + CALL_WITH_BOUNDS_P (folded) = true;
> free (args);
> if (sc)
>   CALL_EXPR_STATIC_CHAIN (folded) = sc;
> diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
> index f7ec8b6..e83d9dc 100644
> --- a/gcc/tree-ssa-sccvn.c
> +++ b/gcc/tree-ssa-sccvn.c
> @@ -1124,6 +1124,8 @@ copy_reference_ops_from_call (gimple call,
>temp.opcode = CALL_EXPR;
>temp.op0 = gimple_call_fn (call);
>temp.op1 = gimple_call_chain (call);
> +  if (gimple_call_with_bounds_p (call))
> +temp.op2 = integer_one_node;
>temp.off = -1;
>result->safe_push (temp);
>

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Jakub Jelinek]

On Tue, Jun 03, 2014 at 01:33:39PM +0400, Yury Gribov wrote:
> >Also note (but, already preexisting issue) is that the
> >__asan_report* and __asan_{load,store}* calls are declared in
> >sanitizer.def as having 1st argument PTR type, while in the library
> >it is uptr (aka PTRMODE).  So, either we should fix up sanitizer.def
> >to match that (we were passing already base_addr which is integral,
> >not pointer), or if we keep sanitizer.def as is, we should pass
> >to the calls base_ssa instead of base_addr.
> 
> Which solution is better for middle-end? Would casting to uptr
> complicate alias analysis or somehow damage optimization
> capabilities of further passes?

Supposedly at least for use_calls case pretending the argument is
void * instead of uptr would result in smaller IL (no need to cast it
to integral type).

> Frankly I don't understand why libsanitizer prefers uptr to void*'s
> and size_t's...

Supposedly because then you don't have to cast it on the library side
if you want to perform masking/shifting etc. on it.  But that doesn't sound
like a very strong argument.

Jakub

Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)

[Konstantin Serebryany]

On Tue, Jun 3, 2014 at 1:33 PM, Yury Gribov  wrote:
>> Any reason why the BUILT_IN_* names so differ from the actual function
>> names?  I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
>> (no underscore before N, no CHECK_)?
>
>
> Makes sense.
>
>> Wouldn't it be better to do
>> ...
>>
>> so that you don't risk signed overflow?  Maybe also
>
>
> Yeah, that looks cleaner.
>
>
>> Also note (but, already preexisting issue) is that the
>> __asan_report* and __asan_{load,store}* calls are declared in
>> sanitizer.def as having 1st argument PTR type, while in the library
>> it is uptr (aka PTRMODE).  So, either we should fix up sanitizer.def
>> to match that (we were passing already base_addr which is integral,
>> not pointer), or if we keep sanitizer.def as is, we should pass
>> to the calls base_ssa instead of base_addr.
>
>
> Which solution is better for middle-end? Would casting to uptr complicate
> alias analysis or somehow damage optimization capabilities of further
> passes?
>
> Frankly I don't understand why libsanitizer prefers uptr to void*'s and
> size_t's...

We can not use size_t because we do not include system headers.


>
>
>> Furthermore, this looks just like a partial transition, I don't see
>> why we should emit one __asan_loadN call but two separate
>> __asan_report_load_1 for the instrumented stringops.
>
>
> I was too lazy to bother with strlen. But ok, I'll optimize this piece.

Note that in clang we stopped instrumenting stringops in compiler and
fully rely on interceptors
({memset,memcpy,memmove} are replaced with
__asan_{memset,memcpy,memmove} to avoid late inlining)

>
>
>> Instead, build_check_stmt should be changed, so that it is possible to
>> pass
>> it a variable length (checked by the caller for non-zero already),
>> and in that case force using the slow_p stuff, but instead of adding a
>> constant size add the variable length - 1.
>
>
> +1, this will simplify code.
>
> -Y

[COMMITTED] Add myself to MAINTAINERS file

[Andrew Bennett]

Hi,

This patch adds myself to the MAINTAINERS file.  Commmitted as r211167.
The ChangeLog and patch are shown below.

Regards,


Andrew


Andrew Bennett
Software Design Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com


2014-06-03  Andrew Bennett  

* MAINTAINERS (Write After Approval): Add myself.



Index: MAINTAINERS
===
--- MAINTAINERS (revision 211163)
+++ MAINTAINERS (working copy)
@@ -324,6 +324,7 @@
 Tejas Belagod  tejas.bela...@arm.com
 Andrey Belevantsev a...@ispras.ru
 Jon Beniston   j...@beniston.com
+Andrew Bennett andrew.benn...@imgtec.com
 Peter Bergner  berg...@vnet.ibm.com
 Jan Beulichjbeul...@novell.com
 David Billinghurst david.billinghu...@riotinto.com

Re: [PATCH][match-and-simplify]

[Marc Glisse]

On Tue, 3 Jun 2014, Richard Biener wrote:


On Mon, 2 Jun 2014, Marc Glisse wrote:




   (plus (bit_not @0) @0)
   if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
   { build_int_cst (TREE_TYPE (@0), -1); })
+(match_and_simplify
+  (plus @0 (bit_not @0))
+  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
+  { build_int_cst (TREE_TYPE (@0), -1); })


Why not just:

(match_and_simplify
  (plus @0 (bit_not @0))
  { build_all_ones_cst (TREE_TYPE (@0)); })

? Works for vector/complex, I don't know what type a bit_not_expr can have
where the simplification wouldn't be true.


Sure.


Thanks. Sorry for not being clear enough, the same remark applies to 
basically all the bitwise patterns in the file. I could have saved the 
remark for the pre-merge RFC period. I mostly posted it now so the next 
batch of patterns can be directly written in a general way so you 
(Prathamesh or Richard) have fewer to fix later, but it can certainly 
wait.


/* ~~x -> x */
(match_and_simplify
  (bit_not (bit_not @0))
  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
  @0)

We can drop the "if" line.

/* (x | CST1) & CST2 -> (x & CST2) | (CST1 & CST2) */
(match_and_simplify
  (bit_and (bit_ior @0 INTEGER_CST_P@1) INTEGER_CST_P@2)
  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
  (bit_ior (bit_and @0 @2) (bit_and @1 @2)))

Drop the "if" line and replace INTEGER_CST_P with CONSTANT_CLASS_P.

etc.

--
Marc Glisse

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Richard Biener]

On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  wrote:
> Hi,
>
> This patch adjusts alloc-free removal algorithm in DCE to take into account 
> BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-03  Ilya Enkovich  
>
> * tree-ssa-dce.c: Include target.h.
> (propagate_necessity): For free call fed by alloc check
> bounds are also provided by the same alloc.
> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
> used by free calls.
>
> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
> index 13a71ce..59a0b71 100644
> --- a/gcc/tree-ssa-dce.c
> +++ b/gcc/tree-ssa-dce.c
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "flags.h"
>  #include "cfgloop.h"
>  #include "tree-scalar-evolution.h"
> +#include "target.h"
>
>  static struct stmt_stats
>  {
> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
>   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
> -   continue;
> +   {
> + tree retfndecl
> +   = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
> + gimple bounds_def_stmt;
> + tree bounds;
> +
> + /* For instrumented calls we should also check used
> +bounds are returned by the same allocation call.  */
> + if (!gimple_call_with_bounds_p (stmt)
> + || ((bounds = gimple_call_arg (stmt, 1))

Please provide an abstraction for this - I seem to recall seeing multiple
(variants) of this.  Esp. repeated calls to the target hook above look
expensive to me (generally it will not be needed).

I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and I'd
like to see sth similar to gimple_call_builtin_p, for example
gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
target hook invocation and the fndecl check.

> + && TREE_CODE (bounds) == SSA_NAME

What about constant bounds?  That shouldn't make the call necessary either?

> + && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
> + && is_gimple_call (bounds_def_stmt)
> + && gimple_call_fndecl (bounds_def_stmt) == retfndecl
> + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
> +   continue;

So this all becomes

   if (!gimple_call_with_bounds_p (stmt)
   || ((bounds = gimple_call_bndarg (stmt))
   && TREE_CODE (bounds) == SSA_NAME
   && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
   && is_gimple_call (bounds_def_stmt)
   && gimple_call_chkp_p (bounds_def_stmt,
BUILT_IN_CHKP_BNDRET)
...

btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
need a target hook to compare the fndecl?  Why isn't it enough to
just check DECL_FUNCTION_CODE and DECL_BUILT_IN?

Richard.

> +   }
> }
>
>   FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
> @@ -1204,6 +1221,23 @@ eliminate_unnecessary_stmts (void)
>   && !gimple_plf (def_stmt, STMT_NECESSARY))
> gimple_set_plf (stmt, STMT_NECESSARY, false);
> }
> + /* We did not propagate necessity for free calls fed
> +by allocation function to allow unnecessary
> +alloc-free sequence elimination.  For instrumented
> +calls it also means we did not mark bounds producer
> +as necessary and it is time to do it in case free
> +call is not removed.  */
> + if (gimple_call_with_bounds_p (stmt))
> +   {
> + gimple bounds_def_stmt;
> + tree bounds = gimple_call_arg (stmt, 1);
> + gcc_assert (TREE_CODE (bounds) == SSA_NAME);
> + bounds_def_stmt = SSA_NAME_DEF_STMT (bounds);
> + if (bounds_def_stmt
> + && !gimple_plf (bounds_def_stmt, STMT_NECESSARY))
> +   gimple_set_plf (bounds_def_stmt, STMT_NECESSARY,
> +   gimple_plf (stmt, STMT_NECESSARY));
> +   }
> }
>
>   /* If GSI is not necessary then remove it.  */
> @@ -1216,6 +1250,7 @@ eliminate_unnecessary_stmts (void)
>   else if (is_gimple_call (stmt))
> {
>   tree name = gimple_call_lhs (stmt);
> + tree retfn = targetm.builtin_chkp_function 
> (BUILT_IN_CHKP_BNDRET);
>
>   notice_special_calls (stmt);
>
> @@ -1233,7

Re: [PATCH][match-and-simplify]

[Richard Biener]

On Tue, 3 Jun 2014, Marc Glisse wrote:

> On Tue, 3 Jun 2014, Richard Biener wrote:
> 
> > On Mon, 2 Jun 2014, Marc Glisse wrote:
> > 
> > > 
> > > >(plus (bit_not @0) @0)
> > > >if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> > > >{ build_int_cst (TREE_TYPE (@0), -1); })
> > > > +(match_and_simplify
> > > > +  (plus @0 (bit_not @0))
> > > > +  if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
> > > > +  { build_int_cst (TREE_TYPE (@0), -1); })
> > > 
> > > Why not just:
> > > 
> > > (match_and_simplify
> > >   (plus @0 (bit_not @0))
> > >   { build_all_ones_cst (TREE_TYPE (@0)); })
> > > 
> > > ? Works for vector/complex, I don't know what type a bit_not_expr can have
> > > where the simplification wouldn't be true.
> > 
> > Sure.
> 
> Thanks. Sorry for not being clear enough, the same remark applies to basically
> all the bitwise patterns in the file. I could have saved the remark for the
> pre-merge RFC period. I mostly posted it now so the next batch of patterns can
> be directly written in a general way so you (Prathamesh or Richard) have fewer
> to fix later, but it can certainly wait.
> 
> /* ~~x -> x */
> (match_and_simplify
>   (bit_not (bit_not @0))
>   if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
>   @0)
> 
> We can drop the "if" line.
> 
> /* (x | CST1) & CST2 -> (x & CST2) | (CST1 & CST2) */
> (match_and_simplify
>   (bit_and (bit_ior @0 INTEGER_CST_P@1) INTEGER_CST_P@2)
>   if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
>   (bit_ior (bit_and @0 @2) (bit_and @1 @2)))
> 
> Drop the "if" line and replace INTEGER_CST_P with CONSTANT_CLASS_P.
> 
> etc.

Yeah, though we don't restrict the types that the BIT_*_EXPR operations
apply to anywhere and I wonder if this all applies to FP types
for example ... (and if we are not supposed to get FP types (or
struct types!?) here then the checking code and the tree.def documentation
should be amended).

I think the conditions we have now are literally copied from
tree-ssa-forwrpop.c (and the idea was to copy what that does 1:1).

Richard.

Re: [PATCH, Pointer Bounds Checker 26/x] CCP

[Richard Biener]

On Tue, Jun 3, 2014 at 9:38 AM, Ilya Enkovich  wrote:
> Hi,
>
> This patch allows BUILT_IN_CHKP_BNDRET as a consumer of a result of 
> BUILT_IN_STACK_SAVE call.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-03  Ilya Enkovich  
>
> * tree-ssa-ccp.c (insert_clobber_before_stack_restore): Handle
> BUILT_IN_CHKP_BNDRET calls.
>
>
> diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
> index eeefeaf..c138337 100644
> --- a/gcc/tree-ssa-ccp.c
> +++ b/gcc/tree-ssa-ccp.c
> @@ -1848,7 +1848,7 @@ insert_clobber_before_stack_restore (tree saved_val, 
> tree var,
>  gimple_htab *visited)
>  {
>gimple stmt, clobber_stmt;
> -  tree clobber;
> +  tree clobber, fndecl;
>imm_use_iterator iter;
>gimple_stmt_iterator i;
>gimple *slot;
> @@ -1880,6 +1880,10 @@ insert_clobber_before_stack_restore (tree saved_val, 
> tree var,
>  else if (gimple_assign_ssa_name_copy_p (stmt))
>insert_clobber_before_stack_restore (gimple_assign_lhs (stmt), var,
>visited);
> +else if (gimple_code (stmt) == GIMPLE_CALL
> +&& (fndecl = targetm.builtin_chkp_function 
> (BUILT_IN_CHKP_BNDRET))
> +&& gimple_call_fndecl (stmt) == fndecl)
> +  continue;

Please change this to use the proper abstraction once implemented.  It should
be sth like

else if (is_gimple_call (stmt)
   && gimple_call_builtin_p (stmt, BUILT_IN_CHKP_BNDRET))
   continue;

I assume now that the builtins are not target builtins but added to
builtins.def.

Richard.

>  else
>gcc_assert (is_gimple_debug (stmt));
>  }

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Thomas Schwinge]

Hi!

Ping -- OK to commit to trunk?

On Wed, 28 May 2014 23:55:31 +0200, Jan Hubicka  wrote:
> > On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski  wrote:
> > > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres  
> > > wrote:
> > > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., 
> > > > darwin10.
> > > >
> > > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared 
> > > > in this scope
> > 
> > I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
> > GNU extension).
> > 
> > > strnlen should be declared in include/libiberty.h if there is no
> > > declaration as libiberty support is already there.  That should be a
> > > simple fix.
> > 
> > Like this?
> 
> This looks good to me (thoguh strnlen is posix).  I can not approve the patch
> but I would preffer it over just hand implementing strnlen there (that is 
> easy,
> too)

Patch is also considered good by testers in
.

> > --- gcc/config.in
> > +++ gcc/config.in
> > [Regenerate.]
> > --- gcc/configure
> > +++ gcc/configure
> > [Regenerate.]
> > --- gcc/configure.ac
> > +++ gcc/configure.ac
> > @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include 
> > $GMPINC"
> >  saved_CXXFLAGS="$CXXFLAGS"
> >  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
> >  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
> > -   strsignal strstr stpcpy strverscmp \
> > +   stpcpy strnlen strsignal strstr strverscmp \
> > errno snprintf vsnprintf vasprintf malloc realloc calloc \
> > free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
> >  #include "ansidecl.h"
> > diff --git include/libiberty.h include/libiberty.h
> > index 7fd0703..56b8b43 100644
> > --- include/libiberty.h
> > +++ include/libiberty.h
> > @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, 
> > ...) ATTRIBUTE_PRINTF_3;
> >  extern int vsnprintf (char *, size_t, const char *, va_list) 
> > ATTRIBUTE_PRINTF(3,0);
> >  #endif
> >  
> > +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
> > +extern size_t strnlen (const char *, size_t);
> > +#endif
> > +
> >  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
> >  /* Compare version strings.  */
> >  extern int strverscmp (const char *, const char *);


Grüße,
 Thomas


pgpG8JKqsF2Wx.pgp
Description: PGP signature

Re: config-ml.in: Robustify ac_configure_args parsing.

[Thomas Schwinge]

Hi!

Ping.


On Thu, 22 May 2014 12:58:05 +0200, I wrote:
> Ping.
> 
> 
> On Fri, 14 Mar 2014 12:22:29 +0100, I wrote:
> > $ ../configure --enable-foo='--enable-a=1 --enable-b=2 --enable-c=3'
> > [...]
> > $ make configure-zlib
> > config.status: creating Makefile
> > config.status: executing default-1 commands
> > ../../zlib/../config-ml.in: eval: line 142: unexpected EOF while 
> > looking for matching `''
> > ../../zlib/../config-ml.in: eval: line 143: syntax error: unexpected 
> > end of file
> > make: *** [configure-zlib] Error 1
> > 
> >140  case $enableopt in
> >141  enable_shared | enable_static) ;;
> >142  *) eval $enableopt="$optarg" ;;
> >143  esac
> >144  ;;
> > 
> > $ grep ac_configure_args < zlib/config.status
> > ac_configure_args="  '--cache-file=./config.cache' 
> > '--enable-foo=--enable-a=1 --enable-b=2 --enable-c=3' 
> > '--enable-languages=c,c++,fortran,java,lto,objc' 
> > '--program-transform-name=s,y,y,' '--disable-option-checking' 
> > '--build=x86_64-unknown-linux-gnu' '--host=x86_64-unknown-linux-gnu' 
> > '--target=x86_64-unknown-linux-gnu' '--srcdir=../../zlib' 
> > 'build_alias=x86_64-unknown-linux-gnu' 
> > 'host_alias=x86_64-unknown-linux-gnu' 
> > 'target_alias=x86_64-unknown-linux-gnu'"
> > 
> > These are quoted correctly; the error happens because the
> > ac_configure_args parsing logic in config-ml.in will parse this as:
> > 
> >  1. '--enable-foo=--enable-a=1
> >  2. --enable-b=2
> >  3. --enable-c=3'
> > 
> > Below I'm proposing a patch using a shell function and eval to properly
> > handle such configure arguments.  Instead of a shell function, we could
> > also use:
> > 
> > eval set x "${ac_configure_args}" && shift
> > for option
> > do
> >   [...]
> > done
> > 
> > ..., as done in top-level configure.ac for baseargs etc., but as the
> > config-ml.in script is sourced in different contexts, it is not obvious
> > to me that we're permitted to overwrite the shell's positional parameters
> > here.
> > 
> > OK for trunk?  (Will properly indent scan_arguments before commit.)
> > 
> > commit bc6f99e9840994309eaf4e88679c3ba50d5e4918
> > Author: Thomas Schwinge 
> > Date:   Thu Mar 13 19:54:58 2014 +0100
> > 
> > * config-ml.in: Robustify ac_configure_args parsing.
> > 
> > diff --git config-ml.in config-ml.in
> > index 1198346..0cd7db3 100644
> > --- config-ml.in
> > +++ config-ml.in
> > @@ -105,10 +105,13 @@ ml_realsrcdir=${srcdir}
> >  
> >  # Scan all the arguments and set all the ones we need.
> >  
> > +scan_arguments ()
> > +{
> >  ml_verbose=--verbose
> > -for option in ${ac_configure_args}
> > +for option
> >  do
> > -  # strip single quotes surrounding individual options
> > +  # Strip single quotes surrounding individual options, that is, remove one
> > +  # level of shell quoting for these.
> >case $option in
> >\'*\') eval option=$option ;;
> >esac
> > @@ -139,7 +142,7 @@ do
> > # Don't undo its work.
> > case $enableopt in
> > enable_shared | enable_static) ;;
> > -   *) eval $enableopt="$optarg" ;;
> > +   *) eval $enableopt='$optarg' ;;
> > esac
> > ;;
> >--norecursion | --no-recursion)
> > @@ -157,7 +160,7 @@ do
> > *)  optarg=yes ;;
> > esac
> > withopt=`echo ${option} | sed 's:^--::;s:=.*$::;s:-:_:g'`
> > -   eval $withopt="$optarg"
> > +   eval $withopt='$optarg'
> > ;;
> >--without-*)
> > withopt=`echo ${option} | sed 's:^--::;s:out::;s:-:_:g'`
> > @@ -165,6 +168,11 @@ do
> > ;;
> >esac
> >  done
> > +}
> > +# Use eval to properly handle configure arguments such as
> > +# --enable-foo='--enable-a=1 --enable-b=2 --enable-c=3'.
> > +eval scan_arguments "${ac_configure_args}"
> > +unset scan_arguments
> >  
> >  # Only do this if --enable-multilib.
> >  if [ "${enable_multilib}" = yes ]; then
> > @@ -860,7 +868,7 @@ if [ -n "${multidirs}" ] && [ -z "${ml_norecursion}" ]; 
> > then
> >  
> >  if eval ${ml_config_env} ${ml_config_shell} ${ml_recprog} \
> > --with-multisubdir=${ml_dir} --with-multisrctop=${multisrctop} \
> > -   ${ac_configure_args} ${ml_config_env} ${ml_srcdiroption} ; then
> > +   "${ac_configure_args}" ${ml_config_env} ${ml_srcdiroption} ; then
> >true
> >  else
> >exit 1


Grüße,
 Thomas


pgpk78wm2aQEQ.pgp
Description: PGP signature

Re: [AArch64/ARM 2/3] Detect EXT patterns to vec_perm_const, use for EXT intrinsics

[Alan Lawrence]

Ok, this fixes it. We'll output an ext...#0, which is little more than a MOV,
but that seems appropriate in the circumstance.

Regression tested check-gcc and check-g++ on aarch64-none-elf and
aarch64_be-none-elf.

Ok for trunk?

--Alan

Alan Lawrence wrote:

Yes, reproduced. Seems the mid-end doesn't elide no-op masks at -O0 after all...

Fix in progress, think it's almost (tho not quite) simply a bad assertion.

--Alan


Christophe Lyon wrote:

Hi Alan

This causes g++ to ICE on pr59378 test, for aarch64 targets:
http://cbuild.validation.linaro.org/build/cross-validation/gcc/211058/report-build-info.html

Can you check?

Thanks,

Christophe.


On 19 May 2014 14:53, Marcus Shawcroft  wrote:

On 23 April 2014 21:22, Alan Lawrence  wrote:


2014-03-27  Alan Lawrence  
* config/aarch64/aarch64-builtins.c
(aarch64_types_binopv_qualifiers,
TYPES_BINOPV): New static data.
* config/aarch64/aarch64-simd-builtins.def (im_lane_bound): New
builtin.
* config/aarch64/aarch64-simd.md (aarch64_ext,
aarch64_im_lane_boundsi):
New patterns.
* config/aarch64/aarch64.c (aarch64_expand_vec_perm_const_1): Match
patterns for EXT.
(aarch64_evpc_ext): New function.

* config/aarch64/iterators.md (UNSPEC_EXT): New enum element.

* config/aarch64/arm_neon.h (vext_f32, vext_f64, vext_p8, vext_p16,
vext_s8, vext_s16, vext_s32, vext_s64, vext_u8, vext_u16, vext_u32,
vext_u64, vextq_f32, vextq_f64, vextq_p8, vextq_p16, vextq_s8,
vextq_s16, vextq_s32, vextq_s64, vextq_u8, vextq_u16, vextq_u32,
vextq_u64): Replace __asm with __builtin_shuffle and
im_lane_boundsi.

OK /Marcus




diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 898323820201c6a7e52b0224fbeffa2b263b3e39..36173edb3a7cd6818a511ab5bb81557bb65fa287 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8111,9 +8111,6 @@ aarch64_evpc_ext (struct expand_vec_perm_d *d)
 return false;
 }
 
-  /* The mid-end handles masks that just return one of the input vectors.  */
-  gcc_assert (location != 0);
-
   switch (d->vmode)
 {
 case V16QImode: gen = gen_aarch64_extv16qi; break;
@@ -8134,7 +8131,10 @@ aarch64_evpc_ext (struct expand_vec_perm_d *d)
   if (d->testing_p)
 return true;
 
-  if (BYTES_BIG_ENDIAN)
+  /* The case where (location == 0) is a no-op for both big- and little-endian,
+ and is removed by the mid-end at optimization levels -O1 and higher.  */
+
+  if (BYTES_BIG_ENDIAN && (location != 0))
 {
   /* After setup, we want the high elements of the first vector (stored
  at the LSB end of the register), and the low elements of the second

Re: [build, doc, testsuite] Centralise clearing hardware capabilities with Sun ld

[Rainer Orth]

Hi Gerald,

> On Tue, 27 May 2014, Mike Stump wrote:
>> So, I read the doc bits, and they look fine.  I’m not a doc reviewer, 
>> but, the changes are usual and customary for a port, and trivial.
>
> Yes, and I'd like to emphasize this point:  Just because a file
> matches *.texi doesn't mean that ports, middle, front, whatever
> end maintainers cannot and should not just make changes there
> without requiring explicit doc approval.

thanks for the clarification.  In this case, I was rather looking for
another pair of eyes to cross-check the text.

Btw., care to have a look at the docs part of

https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01860.html

This is outside of my maintainerships, so I need approval here.

Thanks.
Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, driver] RFC: Support compressed debug sections

[Rainer Orth]

Rainer Orth  writes:

> "Joseph S. Myers"  writes:
[...]
>> Thanks for the explanation.  The driver changes are OK.
>
> Thanks.  I still need approval for the doc and build parts, as well as
> the Darwin and DJGPP changes.  For the latter two, I've included the
> patch in a x86_64-apple-darwin11.4.2 build, verifying that -gz is
> rejected as expected (no idea if the are any working gas and gold ports
> that would support the option).  For DJGPP, I've tried a
> i386-pc-solaris2.11 x i586-pc-msdosdjgpp cross.  While the specs
> additions look correct, trying to compile even the most trivial program
> SEGVs:
[...]

It's been another week, and I still need approval for the build, doc,
and Darwin changes:

https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01860.html

Thanks.
Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: libgo patch committed: Make libgo C code more portable

[Rainer Orth]

Ian Lance Taylor  writes:

> This patch to libgo, from Peter Collingbourne, changes some of the C
> code to make it easier to compile libgo with a compiler other than GCC.
> Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
> Committed to mainline.
[...]
> diff -r d73f07d002ef libgo/runtime/print.c
> --- a/libgo/runtime/print.c   Tue May 27 15:00:31 2014 -0700
> +++ b/libgo/runtime/print.c   Wed May 28 16:09:25 2014 -0700
> @@ -2,6 +2,8 @@
>  // Use of this source code is governed by a BSD-style
>  // license that can be found in the LICENSE file.
>  
> +#include 
> +#include 
>  #include 
>  #include "runtime.h"
>  #include "array.h"
> @@ -174,13 +176,12 @@
>   gwrite("NaN", 3);
>   return;
>   }
> - i = __builtin_isinf_sign(v);
> - if(i > 0) {
> - gwrite("+Inf", 4);
> - return;
> - }
> - if(i < 0) {
> - gwrite("-Inf", 4);
> + if(isinf(v)) {
> + if(signbit(v)) {
> + gwrite("-Inf", 4);
> + } else {
> + gwrite("+Inf", 4);
> + }
>   return;
>   }
>  

This change broke Solaris Go bootstrap:

/vol/gcc/src/hg/trunk/local/libgo/runtime/print.c: In function 
'__go_print_double':
/vol/gcc/src/hg/trunk/local/libgo/runtime/print.c:200:3: error: dereferencing 
type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   if(signbit(v)) {
   ^
cc1: all warnings being treated as errors
make: *** [print.lo] Error 1

It's an issue in the gcc version of signbit in ,
ultimately:

#undef  signbit
#if defined(__sparc)
#define signbit(x)  __extension__( \
{ __typeof(x) __x_s = (x); \
(int) (*(unsigned *) &__x_s >> 31); })
#elif defined(__i386) || defined(__amd64)
#define signbit(x)  __extension__( \
{ __typeof(x) __x_s = (x); \
(sizeof (__x_s) == sizeof (float) ? \
(int) (*(unsigned *) &__x_s >> 31) : \
sizeof (__x_s) == sizeof (double) ? \
(int) (((unsigned *) &__x_s)[1] >> 31) : \
(int) (((unsigned short *) &__x_s)[4] >> 15)); })
#endif

Solaris lacks __signbit{f,,l} in libm, like some other Unices do, and
cannot use __builtin_signbit since that is missing in gcc 3.4 still
bundled with Solaris 10 and 11.  I'll probably go for handling this in
fixincludes on mainline, though, where the gcc 3 issue is irrelevant.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH, Pointer Bounds Checker 21/x] Weakrefs output

[Ilya Enkovich]

2014-06-03 13:02 GMT+04:00 Richard Biener :
> On Mon, Jun 2, 2014 at 5:22 PM, Ilya Enkovich  wrote:
>> Hi,
>>
>> This patch prevents output of both instrumented and not instrumented weakref 
>> variants.
>
> Shouldn't one of them be reclaimed instead at some point?

Actually both version may be used.  We may have both instrumented and
not instrumented calls to the same function. Also original function
pointer may be used as value somewhere (e.g. in DECL_INIT).

Ilya

>
> Richard.
>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-06-02  Ilya Enkovich  
>>
>> * cgraphunit.c (output_weakrefs): If there are both
>> instrumented and original versions, output only one
>> of them.
>>
>>
>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index c5c..ae9e699 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -2111,9 +2111,13 @@ static void
>>  output_weakrefs (void)
>>  {
>>symtab_node *node;
>> +  cgraph_node *cnode;
>>FOR_EACH_SYMBOL (node)
>>  if (node->alias
>>  && !TREE_ASM_WRITTEN (node->decl)
>> +   && (!(cnode = dyn_cast  (node))
>> +   || !cnode->instrumented_version
>> +   || !TREE_ASM_WRITTEN (cnode->instrumented_version->decl))
>> && node->weakref)
>>{
>> tree target;

Re: [RFC] optimize x - y cmp 0 with undefined overflow

[Richard Biener]

On Tue, Jun 3, 2014 at 10:11 AM, Eric Botcazou  wrote:
>> Looks mostly ok.  Any reason why you are not re-creating
>> MINUS_EXPR in build_symbolic_expr?  That is, build
>> inv - t (for non-pointers, of course)?
>
> It's more uniform and compare_values expects an INTEGER_CST on the RHS,
> although the patch was lacking a small tweak to the function:
>
> @@ -1205,19 +1292,23 @@ compare_values_warnv (tree val1, tree va
>STRIP_USELESS_TYPE_CONVERSION (val2);
>
>if ((TREE_CODE (val1) == SSA_NAME
> +   || (TREE_CODE (val1) == NEGATE_EXPR
> +  && TREE_CODE (TREE_OPERAND (val1, 0)) == SSA_NAME)
> || TREE_CODE (val1) == PLUS_EXPR
> || TREE_CODE (val1) == MINUS_EXPR)
>&& (TREE_CODE (val2) == SSA_NAME
> + || (TREE_CODE (val2) == NEGATE_EXPR
> + && TREE_CODE (TREE_OPERAND (val2, 0)) == SSA_NAME)
>   || TREE_CODE (val2) == PLUS_EXPR
>   || TREE_CODE (val2) == MINUS_EXPR))
>  {
>tree n1, c1, n2, c2;
>enum tree_code code1, code2;
>
> -  /* If VAL1 and VAL2 are of the form 'NAME [+-] CST' or 'NAME',
> +  /* If VAL1 and VAL2 are of the form '[-]NAME [+-] CST' or 'NAME',
>  return -1 or +1 accordingly.  If VAL1 and VAL2 don't use the
>  same name, return -2.  */
> -  if (TREE_CODE (val1) == SSA_NAME)
> +  if (TREE_CODE (val1) == SSA_NAME || TREE_CODE (val1) == NEGATE_EXPR)
> {
>   code1 = SSA_NAME;
>   n1 = val1;
> @@ -1239,7 +1330,7 @@ compare_values_warnv (tree val1, tree va
> }
> }
>
> -  if (TREE_CODE (val2) == SSA_NAME)
> +  if (TREE_CODE (val2) == SSA_NAME || TREE_CODE (val2) == NEGATE_EXPR)
> {
>   code2 = SSA_NAME;
>   n2 = val2;
> @@ -1262,6 +1353,11 @@ compare_values_warnv (tree val1, tree va
> }
>
>/* Both values must use the same name.  */
> +  if (TREE_CODE (n1) == NEGATE_EXPR && TREE_CODE (n2) == NEGATE_EXPR)
> +   {
> + n1 = TREE_OPERAND (n1, 0);
> + n2 = TREE_OPERAND (n2, 0);
> +   }
>if (n1 != n2)
> return -2;

Ah, ok - makes sense.

>> Otherwise if a range becomes -t + inv that will no longer match
>> get_single_symbol for further propagation?
>
> Yes, it will, NEGATE_EXPR is handled by get_single_symbol.

Now spotted.

>> Then I'm not sure if
>>
>> +  /* Try with VR0 and [-INF, OP1].  */
>> +  set_value_range (&new_vr1, VR_RANGE, vrp_val_min (expr_type), op1,
>> NULL); +  extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
>> &new_vr1); +  if (vr->type != VR_VARYING)
>> +   return;
>> +
>> +  /* Try with VR0 and [OP1, +INF].  */
>> +  set_value_range (&new_vr1, VR_RANGE, op1, vrp_val_max (expr_type),
>> NULL); +  extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
>> &new_vr1); +  if (vr->type != VR_VARYING)
>> +   return;
>>
>> is a safe thing to do.  If it does make a difference to try [-INF, OP1],
>> [OP1, +INF] instead of just [OP1, OP1] then at least it's very suspicious ;)
>> (or an "easy" missed optimization).
>
> Yes, it makes a difference and is required to eliminate half-symbolic ranges
> (the ones deduced from x >= y).  Otherwise extract_range_from_binary_expr_1
> will reject the resulting range because it cannot compare the bounds.
>
> As discussed, extract_range_from_binary_expr_1 doesn't do any fiddling with
> the input ranges, it just computes the resulting range and see whether it can
> interpret it.  Half-symbolic ranges cannot be interpreted and probably should
> not, in the general case at least, because I think it can be very delicate, so
> extract_range_from_binary_expr will just try all the possible combinations for
> PLUS and MINUS.

So when computing a range for z in

 z = y - x;

with x = [-INF, y - 1] and y = [x + 1, +INF] (deduced from !(x >= y)) we fail to
do sth sensible with [y, y] - [-INF, y - 1] or [x + 1, +INF] - [x, x] but we do
sth with [x + 1, +INF] - [-INF, x]?  That seems odd to me.

With the patch we compute z to [1, +INF(OVF)]

Going the [x + 1, +INF] - [x,x] path first we obtain

  [1, -x + INF]

which fails the sanity checking

  cmp = compare_values (min, max);
  if (cmp == -2 || cmp == 1)
{
  /* If the new range has its limits swapped around (MIN > MAX),
 then the operation caused one of them to wrap around, mark
 the new range VARYING.  */
  set_value_range_to_varying (vr);
}
  else
set_value_range (vr, type, min, max, NULL);

but clearly the same reasoning you can apply that makes trying
with [-INF, x] valid (it's just enlarging the range) can be applied
here, too, when computing +INF - x for the upper bound.  We can
safely increase that to +INF making the range valid for the above
test.

But I wonder what code path in the routine still relies on that sanity
checking to produce a valid result (so I'd rather try removing it, or
taking uncomparable bounds as a valid range).

Simplest would be to simply do

Re: [PATCH][AARCH64]Support full addressing modes for ldr/str in vectorization scenarios

[Marcus Shawcroft]

On 28 May 2014 08:30, Bin.Cheng  wrote:
> Missing patch.
>
> On Wed, May 28, 2014 at 3:02 PM, bin.cheng  wrote:
>> Hi,
>> I was surprised that GCC didn't support addressing modes like
>> [REG+OFF]/[REG_REG] for instructions ldr/str in vectorization scenarios.
>> The generated assembly is bad since all address expressions have to be
>> computed outside of memory reference.  The root cause is because aarch64
>> effectively rejects reg-indexing (and const-offset) addressing modes in
>> aarch64_classify_address and miscellaneous simd patterns.
>>
>> By fixing this issue, performance of fp benchmarks can be obviously
>> improved.  It can also help vectorized int cases.
>>
>> The patch passes bootstrap and regression test on aarch64/little-endian.  It
>> also passes regression test on aarch64/big-endian except for case
>> "gcc.target/aarch64/vect-mull.c".  I analyzed the failed case and now
>> believe it reveals a latent bug in vectorizer on aarch64/big-endian.  The
>> analysis report is posted at
>> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00182.html.
>>
>> So is it OK?
>>
>> Thanks,
>> bin
>>
>>
>> 2014-05-28  Bin Cheng  
>>
>> * config/aarch64/aarch64.c (aarch64_classify_address)
>> (aarch64_legitimize_reload_address): Support full addressing modes
>> for vector modes.
>> * config/aarch64/aarch64.md (mov, movmisalign)
>> (*aarch64_simd_mov, *aarch64_simd_mov): Relax
>> predicates.

OK Thanks /Marcus

Re: [AArch64/ARM 2/3] Detect EXT patterns to vec_perm_const, use for EXT intrinsics

[Marcus Shawcroft]

On 3 June 2014 11:21, Alan Lawrence  wrote:
> Ok, this fixes it. We'll output an ext...#0, which is little more than a
> MOV,
> but that seems appropriate in the circumstance.
>
> Regression tested check-gcc and check-g++ on aarch64-none-elf and
> aarch64_be-none-elf.
>
> Ok for trunk?


ChangeLog ?

/Marcus

Re: [AArch64/ARM 2/3] Detect EXT patterns to vec_perm_const, use for EXT intrinsics

[Alan Lawrence]

gcc/ChangeLog:

* config/aarch64/aarch64.c (aarch64_evpc_ext): Allow+handle location==0.

?
--Alan

Marcus Shawcroft wrote:

On 3 June 2014 11:21, Alan Lawrence  wrote:

Ok, this fixes it. We'll output an ext...#0, which is little more than a
MOV,
but that seems appropriate in the circumstance.

Regression tested check-gcc and check-g++ on aarch64-none-elf and
aarch64_be-none-elf.

Ok for trunk?



ChangeLog ?

/Marcus

Re: [AArch64/ARM 2/3] Detect EXT patterns to vec_perm_const, use for EXT intrinsics

[Marcus Shawcroft]

On 3 June 2014 12:19, Alan Lawrence  wrote:
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.c (aarch64_evpc_ext): Allow+handle
> location==0.
>
> ?

Allow and handle location == 0.

Otherwise OK
/Marcus

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Ilya Enkovich]

2014-06-03 13:45 GMT+04:00 Richard Biener :
> On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  wrote:
>> Hi,
>>
>> This patch adjusts alloc-free removal algorithm in DCE to take into account 
>> BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>>
>> Bootstrapped and tested on linux-x86_64.
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-06-03  Ilya Enkovich  
>>
>> * tree-ssa-dce.c: Include target.h.
>> (propagate_necessity): For free call fed by alloc check
>> bounds are also provided by the same alloc.
>> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
>> used by free calls.
>>
>> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
>> index 13a71ce..59a0b71 100644
>> --- a/gcc/tree-ssa-dce.c
>> +++ b/gcc/tree-ssa-dce.c
>> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "flags.h"
>>  #include "cfgloop.h"
>>  #include "tree-scalar-evolution.h"
>> +#include "target.h"
>>
>>  static struct stmt_stats
>>  {
>> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
>>   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>>   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>>   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
>> -   continue;
>> +   {
>> + tree retfndecl
>> +   = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
>> + gimple bounds_def_stmt;
>> + tree bounds;
>> +
>> + /* For instrumented calls we should also check used
>> +bounds are returned by the same allocation call.  */
>> + if (!gimple_call_with_bounds_p (stmt)
>> + || ((bounds = gimple_call_arg (stmt, 1))
>
> Please provide an abstraction for this - I seem to recall seeing multiple
> (variants) of this.  Esp. repeated calls to the target hook above look
> expensive to me (generally it will not be needed).
>
> I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and I'd
> like to see sth similar to gimple_call_builtin_p, for example
> gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
> target hook invocation and the fndecl check.

I do not see how to get rid of constant in gimple_call_arg call here.
What semantics does proposed gimple_call_bndarg have? It has to return
the first bounds arg but it's not clear from its name and function
seems to be very specialized.

>
>> + && TREE_CODE (bounds) == SSA_NAME
>
> What about constant bounds?  That shouldn't make the call necessary either?

Yep, it would be useless.

>
>> + && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
>> + && is_gimple_call (bounds_def_stmt)
>> + && gimple_call_fndecl (bounds_def_stmt) == 
>> retfndecl
>> + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
>> +   continue;
>
> So this all becomes
>
>if (!gimple_call_with_bounds_p (stmt)
>|| ((bounds = gimple_call_bndarg (stmt))
>&& TREE_CODE (bounds) == SSA_NAME
>&& (bounds_def_stmt = SSA_NAME_DEF_STMT 
> (bounds))
>&& is_gimple_call (bounds_def_stmt)
>&& gimple_call_chkp_p (bounds_def_stmt,
> BUILT_IN_CHKP_BNDRET)
> ...
>
> btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
> need a target hook to compare the fndecl?  Why isn't it enough to
> just check DECL_FUNCTION_CODE and DECL_BUILT_IN?

Call is required because builtins for Pointer Bounds Checker are
provided by target depending on available features. That is why
gimple_call_builtin_p is not used. I may add new interface function
into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
I do not see how it may speed-up the code. New function would still
need to switch by function code and compare with proper decl which is
basically what we have with target hook.

It is possible to make faster if use the fact that all chkp builtins
codes (normal, not target ones) are consequent. Then we may just check
the range and use code as an index in array of chkp fndecls to compare
decls. Would it be OK to rely on builtin codes numbering?

Ilya

>
> Richard.
>
>> +   }
>> }
>>
>>   FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
>> @@ -1204,6 +1221,23 @@ eliminate_unnecessary_stmts (void)
>>   && !gimple_plf (def_stmt, STMT_NECESSARY))
>> gimple_set_plf (stmt, STMT_NECESSARY, false);
>> }
>> + /* We did not propagate necessity for free calls fed
>> +by allocation function to allow unnecessary
>> +alloc-free sequence elimination.  For instrumented
>> +  

Re: [AArch64/ARM 2/3] Recognize shuffle patterns for REV instructions on AArch64, rewrite intrinsics.

[Alan Lawrence]

I've pushed this as r211174, after merging with the EXT changes (r211058): the 
following instructions are equivalent


ext v.8b, v.8b, v.8b, #4
rev64 v.2s, v.2s

and can both be output for a __builtin_shuffle mask of {1,0}. The latter seems 
more readable and so I've put the call to aarch64_evpc_rev ahead of the call to 
aarch64_evpc_ext in aarch64_expand_vec_perm_const_1. The actual patch I 
committed is attached.


Cheers, Alan

Marcus Shawcroft wrote:

On 15 May 2014 16:52, Alan Lawrence  wrote:


2014-05-15  Alan Lawrence  

* config/aarch64/aarch64-simd.md
(aarch64_rev):
New pattern.
* config/aarch64/aarch64.c (aarch64_evpc_rev): New function.
(aarch64_expand_vec_perm_const_1): Add call to aarch64_evpc_rev.
* config/aarch64/iterators.md (REVERSE): New iterator.
(UNSPEC_REV64, UNSPEC_REV32, UNSPEC_REV16): New enum elements.
(rev_op): New int_attribute.
* config/aarch64/arm_neon.h (vrev16_p8, vrev16_s8, vrev16_u8,
vrev16q_p8, vrev16q_s8, vrev16q_u8, vrev32_p8, vrev32_p16,
vrev32_s8,
vrev32_s16, vrev32_u8, vrev32_u16, vrev32q_p8, vrev32q_p16,
vrev32q_s8,
vrev32q_s16, vrev32q_u8, vrev32q_u16, vrev64_f32, vrev64_p8,
vrev64_p16, vrev64_s8, vrev64_s16, vrev64_s32, vrev64_u8,
vrev64_u16,
vrev64_u32, vrev64q_f32, vrev64q_p8, vrev64q_p16, vrev64q_s8,
vrev64q_s16, vrev64q_s32, vrev64q_u8, vrev64q_u16, vrev64q_u32):
Replace temporary __asm__ with __builtin_shuffle.


OK /Marcus

Index: gcc/config/aarch64/aarch64-simd.md
===
--- gcc/config/aarch64/aarch64-simd.md	(revision 211173)
+++ gcc/config/aarch64/aarch64-simd.md	(revision 211174)
@@ -4196,6 +4196,15 @@
 }
 )
 
+(define_insn "aarch64_rev"
+  [(set (match_operand:VALL 0 "register_operand" "=w")
+	(unspec:VALL [(match_operand:VALL 1 "register_operand" "w")]
+REVERSE))]
+  "TARGET_SIMD"
+  "rev\\t%0., %1."
+  [(set_attr "type" "neon_rev")]
+)
+
 (define_insn "aarch64_st2_dreg"
   [(set (match_operand:TI 0 "aarch64_simd_struct_operand" "=Utv")
 	(unspec:TI [(match_operand:OI 1 "register_operand" "w")
Index: gcc/config/aarch64/arm_neon.h
===
--- gcc/config/aarch64/arm_neon.h	(revision 211173)
+++ gcc/config/aarch64/arm_neon.h	(revision 211174)
@@ -10563,402 +10563,6 @@
   return result;
 }
 
-__extension__ static __inline poly8x8_t __attribute__ ((__always_inline__))
-vrev16_p8 (poly8x8_t a)
-{
-  poly8x8_t result;
-  __asm__ ("rev16 %0.8b,%1.8b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline int8x8_t __attribute__ ((__always_inline__))
-vrev16_s8 (int8x8_t a)
-{
-  int8x8_t result;
-  __asm__ ("rev16 %0.8b,%1.8b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
-vrev16_u8 (uint8x8_t a)
-{
-  uint8x8_t result;
-  __asm__ ("rev16 %0.8b,%1.8b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline poly8x16_t __attribute__ ((__always_inline__))
-vrev16q_p8 (poly8x16_t a)
-{
-  poly8x16_t result;
-  __asm__ ("rev16 %0.16b,%1.16b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline int8x16_t __attribute__ ((__always_inline__))
-vrev16q_s8 (int8x16_t a)
-{
-  int8x16_t result;
-  __asm__ ("rev16 %0.16b,%1.16b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline uint8x16_t __attribute__ ((__always_inline__))
-vrev16q_u8 (uint8x16_t a)
-{
-  uint8x16_t result;
-  __asm__ ("rev16 %0.16b,%1.16b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline poly8x8_t __attribute__ ((__always_inline__))
-vrev32_p8 (poly8x8_t a)
-{
-  poly8x8_t result;
-  __asm__ ("rev32 %0.8b,%1.8b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline poly16x4_t __attribute__ ((__always_inline__))
-vrev32_p16 (poly16x4_t a)
-{
-  poly16x4_t result;
-  __asm__ ("rev32 %0.4h,%1.4h"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline int8x8_t __attribute__ ((__always_inline__))
-vrev32_s8 (int8x8_t a)
-{
-  int8x8_t result;
-  __asm__ ("rev32 %0.8b,%1.8b"
-   : "=w"(result)
-   : "w"(a)
-   : /* No clobbers */);
-  return result;
-}
-
-__extension__ static __inline int16x4_t __attribute__ ((__always_inline__))
-vrev32_s16 (int16x4_t a)
-{
-  int16x4_t result;
-  __asm__ ("rev32 %0.4h,%1.4h"
-   : "=w"

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Richard Biener]

On Tue, Jun 3, 2014 at 1:36 PM, Ilya Enkovich  wrote:
> 2014-06-03 13:45 GMT+04:00 Richard Biener :
>> On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  wrote:
>>> Hi,
>>>
>>> This patch adjusts alloc-free removal algorithm in DCE to take into account 
>>> BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>>>
>>> Bootstrapped and tested on linux-x86_64.
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2014-06-03  Ilya Enkovich  
>>>
>>> * tree-ssa-dce.c: Include target.h.
>>> (propagate_necessity): For free call fed by alloc check
>>> bounds are also provided by the same alloc.
>>> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
>>> used by free calls.
>>>
>>> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
>>> index 13a71ce..59a0b71 100644
>>> --- a/gcc/tree-ssa-dce.c
>>> +++ b/gcc/tree-ssa-dce.c
>>> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "flags.h"
>>>  #include "cfgloop.h"
>>>  #include "tree-scalar-evolution.h"
>>> +#include "target.h"
>>>
>>>  static struct stmt_stats
>>>  {
>>> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
>>>   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>>>   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>>>   || DECL_FUNCTION_CODE (def_callee) == 
>>> BUILT_IN_CALLOC))
>>> -   continue;
>>> +   {
>>> + tree retfndecl
>>> +   = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
>>> + gimple bounds_def_stmt;
>>> + tree bounds;
>>> +
>>> + /* For instrumented calls we should also check used
>>> +bounds are returned by the same allocation call.  */
>>> + if (!gimple_call_with_bounds_p (stmt)
>>> + || ((bounds = gimple_call_arg (stmt, 1))
>>
>> Please provide an abstraction for this - I seem to recall seeing multiple
>> (variants) of this.  Esp. repeated calls to the target hook above look
>> expensive to me (generally it will not be needed).
>>
>> I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and I'd
>> like to see sth similar to gimple_call_builtin_p, for example
>> gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
>> target hook invocation and the fndecl check.
>
> I do not see how to get rid of constant in gimple_call_arg call here.
> What semantics does proposed gimple_call_bndarg have? It has to return
> the first bounds arg but it's not clear from its name and function
> seems to be very specialized.

Ah, ok.  I see now, so the code is ok as-is.

>>
>>> + && TREE_CODE (bounds) == SSA_NAME
>>
>> What about constant bounds?  That shouldn't make the call necessary either?
>
> Yep, it would be useless.

So please fix.

>>
>>> + && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
>>> + && is_gimple_call (bounds_def_stmt)
>>> + && gimple_call_fndecl (bounds_def_stmt) == 
>>> retfndecl
>>> + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
>>> +   continue;
>>
>> So this all becomes
>>
>>if (!gimple_call_with_bounds_p (stmt)
>>|| ((bounds = gimple_call_bndarg (stmt))
>>&& TREE_CODE (bounds) == SSA_NAME
>>&& (bounds_def_stmt = SSA_NAME_DEF_STMT 
>> (bounds))
>>&& is_gimple_call (bounds_def_stmt)
>>&& gimple_call_chkp_p (bounds_def_stmt,
>> BUILT_IN_CHKP_BNDRET)
>> ...
>>
>> btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
>> need a target hook to compare the fndecl?  Why isn't it enough to
>> just check DECL_FUNCTION_CODE and DECL_BUILT_IN?
>
> Call is required because builtins for Pointer Bounds Checker are
> provided by target depending on available features. That is why
> gimple_call_builtin_p is not used. I may add new interface function
> into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
> I do not see how it may speed-up the code. New function would still
> need to switch by function code and compare with proper decl which is
> basically what we have with target hook.

I don't understand - does this mean the builtin calls are in the GIMPLE
IL even though the target doesn't have the feature?  Isn't the presence
of the call enough?

> It is possible to make faster if use the fact that all chkp builtins
> codes (normal, not target ones) are consequent. Then we may just check
> the range and use code as an index in array of chkp fndecls to compare
> decls. Would it be OK to rely on builtin codes numbering?

Yes, but that's not my point here.  In the above code the target hook
is called all the time, even if !gimple_call_with_bounds_p ().  Providing
a suitable

Re: [AArch64/ARM 2/3] Detect EXT patterns to vec_perm_const, use for EXT intrinsics

[Alan Lawrence]

Pushed as r211177.

Thanks, Alan

Marcus Shawcroft wrote:

On 3 June 2014 12:19, Alan Lawrence  wrote:

gcc/ChangeLog:

* config/aarch64/aarch64.c (aarch64_evpc_ext): Allow+handle
location==0.

?


Allow and handle location == 0.

Otherwise OK
/Marcus

Re: [PATCH] Do not build libsanitizer also for powerpc*-*-linux*

[Peter Bergner]

On Tue, 2014-06-03 at 08:41 +0200, Jakub Jelinek wrote:
> On Tue, Jun 03, 2014 at 10:19:48AM +0400, Yury Gribov wrote:
> > >I took that patch and applied it to the gcc sources,
> > >but I still see the error on ppc:
> > >...
> > >[bergner@makalu-lp1 asan]$ 
> > >LD_LIBRARY_PATH=:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs::/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs:
> > > ldd ./asan-interface-1.exe
> > >   linux-vdso32.so.1 =>  (0x0010)
> > >   libm.so.6 => /lib/power8/libm.so.6 (0x0ff0)
> > >   libasan.so.1 => 
> > > /home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs/libasan.so.1
> > >  (0x0f93)
> > 
> > Now check indeed seems to be useful: libasan should be the first
> > library in the list when -fsanitize=address flag is present. Are
> > compiler specs for Power somehow special?
> 
> -fsanitize=address should insert -lasan quite early on the linker command
> line, please try to cut'n'paste the command line from testsuite/g++/g++.log
> and add -v to see what is passed to the linker.
> Perhaps the linker reorders the libraries?
> Or do you have LD_PRELOAD?

No LD_PRELOAD.  It adds -lasan "early", but after the libraries and
object files that are explicitly added to the linker command.
Since -lm is explicitly added to the linker command, the implicitly
added -lasan comes after.  The -v command is below.

Peter


/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/collect2
-plugin 
/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/liblto_plugin.so 
-plugin-opt=/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/lto-wrapper
 -plugin-opt=-fresolution=/tmp/cckyoSrJ.res -plugin-opt=-pass-through=-lgcc 
-plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc 
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s 
--eh-frame-hdr -V -m elf32ppclinux -dynamic-linker /lib/ld.so.1 -o 
./asan-interface-1.exe /lib/../lib/crt1.o /lib/../lib/crti.o 
/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32/crtbegin.o 
-L/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs
 -L/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32 -L/lib/../lib 
-L/usr/lib/../lib -L/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc 
-L/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer
 
-L/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan
 /tmp/ccUTAlke.o -lm -lasan -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc 
--as-needed -lgcc_s --no-as-needed 
/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32/crtend.o 
/lib/../lib/crtn.o

[PATCH][match-and-simplify] Get rid of some stmt expressions

[Richard Biener]

The following arranges for complex C-expressions (multi-stmt ones)
in the transform pattern to be outlined to a separate function.
This avoids the need for using stmt expressions which are not
necessarily supported by all C++ host compilers.

The patch doesn't address the stmt expressions being used by
the matching code generator - that will be rewritten anyways.

Lightly tested, I plan to install this tomorrow.

Note that this also gives way of re-numbering captures before
code generation so their number increases when for example
walking in pre-order.  And it gives an easier possibility for
querying the largest capture number as well.

Richard.

2014-06-03  Richard Biener  

* genmatch.c (c_expr): Record cpp_tokens, the number of
stmts seen and a function identifier.
(c_expr::gen_gimple_transform): Generate textual form
from the token vector or a call to the outlined function.
(write_nary_simplifiers): Adjust.
(outline_c_exprs): New function.
(write_gimple): Call it.
(parse_c_expr): Record a cpp_token vector.

Index: gcc/genmatch.c
===
--- gcc/genmatch.c  (revision 211131)
+++ gcc/genmatch.c  (working copy)
@@ -190,9 +190,13 @@ struct expr : public operand
 
 struct c_expr : public operand
 {
-  c_expr (const char *code_)
-: operand (OP_C_EXPR), code (code_) {}
-  const char *code;
+  c_expr (cpp_reader *r_, vec code_, unsigned nr_stmts_)
+: operand (OP_C_EXPR), r (r_), code (code_),
+  nr_stmts (nr_stmts_), fname (NULL) {}
+  cpp_reader *r;
+  vec code;
+  unsigned nr_stmts;
+  char *fname;
   virtual void gen_gimple_match (FILE *, const char *, const char *) { 
gcc_unreachable (); }
   virtual void gen_gimple_transform (FILE *f, const char *);
 };
@@ -440,7 +444,47 @@ expr::gen_gimple_transform (FILE *f, con
 void
 c_expr::gen_gimple_transform (FILE *f, const char *)
 {
-  fputs (code, f);
+  /* If this expression has an outlined function variant, call it.  */
+  if (fname)
+{
+  fprintf (f, "%s (type, captures)", fname);
+  return;
+}
+
+  /* All multi-stmt expressions should have been outlined.  */
+  gcc_assert (nr_stmts <= 1);
+
+  for (unsigned i = 0; i < code.length (); ++i)
+{
+  const cpp_token *token = &code[i];
+
+  /* Replace captures for code-gen.  */
+  if (token->type == CPP_ATSIGN)
+   {
+ const cpp_token *n = &code[i+1];
+ if (n->type == CPP_NUMBER
+ && !(n->flags & PREV_WHITE))
+   {
+ if (token->flags & PREV_WHITE)
+   fputc (' ', f);
+ fprintf (f, "captures[%s]", n->val.str.text);
+ ++i;
+ continue;
+   }
+   }
+
+  /* Skip a single stmt delimiter.  */
+  if (token->type == CPP_SEMICOLON
+ && nr_stmts == 1)
+   continue;
+
+  if (token->flags & PREV_WHITE)
+   fputc (' ', f);
+
+  /* Output the token as string.  */
+  char *tk = (char *)cpp_token_as_text (r, token);
+  fputs (tk, f);
+}
 }
 
 void
@@ -495,9 +539,9 @@ write_nary_simplifiers (FILE *f, vecifexpr)
{
- fprintf (f, "  if (!");
+ fprintf (f, "  if (!(");
  s->ifexpr->gen_gimple_transform (f, fail_label);
- fprintf (f, ") goto %s;", fail_label);
+ fprintf (f, ")) goto %s;", fail_label);
}
   if (s->result->type == operand::OP_EXPR)
{
@@ -533,11 +577,82 @@ write_nary_simplifiers (FILE *f, vectype == operand::OP_C_EXPR)
+{
+  c_expr *e = static_cast (op);
+  static unsigned fnnr = 1;
+  if (e->nr_stmts > 1
+ && !e->fname)
+   {
+ e->fname = (char *)xmalloc (sizeof ("cexprfn") + 4);
+ sprintf (e->fname, "cexprfn%d", fnnr);
+ fprintf (f, "static tree cexprfn%d (tree type, tree *captures)\n",
+  fnnr);
+ fprintf (f, "{\n");
+ unsigned stmt_nr = 1;
+ for (unsigned i = 0; i < e->code.length (); ++i)
+   {
+ const cpp_token *token = &e->code[i];
+
+ /* Replace captures for code-gen.  */
+ if (token->type == CPP_ATSIGN)
+   {
+ const cpp_token *n = &e->code[i+1];
+ if (n->type == CPP_NUMBER
+ && !(n->flags & PREV_WHITE))
+   {
+ if (token->flags & PREV_WHITE)
+   fputc (' ', f);
+ fprintf (f, "captures[%s]", n->val.str.text);
+ ++i;
+ continue;
+   }
+   }
+
+ if (token->flags & PREV_WHITE)
+   fputc (' ', f);
+
+ /* Output the token as string.  */
+ char *tk = (char *)cpp_token_as_text (e->r, token);
+ fputs (tk, f);
+
+ if (token->type == CPP_SEMICOLON)
+   {
+ stmt_nr++;
+ if (stmt_

Re: [PATCH] Do not build libsanitizer also for powerpc*-*-linux*

[Yury Gribov]

> It adds -lasan "early", but after the libraries and
> object files that are explicitly added to the linker command.
> Since -lm is explicitly added to the linker command, the implicitly
> added -lasan comes after.  The -v command is below.

Hm, -lasan manages to override user-specified -lm on gcc trunk for x64:

$ /tmp/gcc-bootstrap-and-regtest/build-orig/gcc/xgcc 
-B/tmp/gcc-bootstrap-and-regtest/build-orig/gcc/ 
/home/ygribov/src/gcc-master/gcc/testsuite/c-c++-common/asan/asan-interface-1.c 

-B/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/ 

-B/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/asan/ 

-L/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/asan/.libs 
 -fsanitize=address -g 
-I/home/ygribov/src/gcc-master/gcc/testsuite/../../libsanitizer/include 
-fno-diagnostics-show-caret -fdiagnostics-color=never   -O0-lm   -o 
./asan-interface-1.exe -v


...

 /tmp/gcc-bootstrap-and-regtest/build-orig/gcc/collect2 -plugin 
/tmp/gcc-bootstrap-and-regtest/build-orig/gcc/liblto_plugin.so 
-plugin-opt=/tmp/gcc-bootstrap-and-regtest/build-orig/gcc/lto-wrapper 
-plugin-opt=-fresolution=/tmp/ccZD4gPg.res 
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s 
-plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc 
-plugin-opt=-pass-through=-lgcc_s --eh-frame-hdr -m elf_x86_64 
-dynamic-linker /lib64/ld-linux-x86-64.so.2 -o ./asan-interface-1.exe 
/usr/lib/x86_64-linux-gnu/crt1.o /usr/lib/x86_64-linux-gnu/crti.o 
/tmp/gcc-bootstrap-and-regtest/build-orig/gcc/crtbegin.o 
-L/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/asan/.libs 
-L/tmp/gcc-bootstrap-and-regtest/build-orig/gcc 
-L/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer 
-L/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/asan 
-L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu 
/tmp/gcc-bootstrap-and-regtest/build-orig/x86_64-unknown-linux-gnu/./libsanitizer/asan/libasan_preinit.o 
-lasan /tmp/ccgt1Kd9.o -lm -lgcc --as-needed -lgcc_s --no-as-needed -lc 
-lgcc --as-needed -lgcc_s --no-as-needed 
/tmp/gcc-bootstrap-and-regtest/build-orig/gcc/crtend.o 
/usr/lib/x86_64-linux-gnu/crtn.o


-Y

Re: [PATCH] Do not build libsanitizer also for powerpc*-*-linux*

[Jakub Jelinek]

On Tue, Jun 03, 2014 at 08:06:41AM -0500, Peter Bergner wrote:
> On Tue, 2014-06-03 at 08:41 +0200, Jakub Jelinek wrote:
> > On Tue, Jun 03, 2014 at 10:19:48AM +0400, Yury Gribov wrote:
> > > >I took that patch and applied it to the gcc sources,
> > > >but I still see the error on ppc:
> > > >...
> > > >[bergner@makalu-lp1 asan]$ 
> > > >LD_LIBRARY_PATH=:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs::/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/gcc/32:/home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs:
> > > > ldd ./asan-interface-1.exe
> > > > linux-vdso32.so.1 =>  (0x0010)
> > > > libm.so.6 => /lib/power8/libm.so.6 (0x0ff0)
> > > > libasan.so.1 => 
> > > > /home/bergner/gcc/build/gcc-fsf-mainline-asan-debug-3/powerpc64-linux/32/libsanitizer/asan/.libs/libasan.so.1
> > > >  (0x0f93)
> > > 
> > > Now check indeed seems to be useful: libasan should be the first
> > > library in the list when -fsanitize=address flag is present. Are
> > > compiler specs for Power somehow special?
> > 
> > -fsanitize=address should insert -lasan quite early on the linker command
> > line, please try to cut'n'paste the command line from testsuite/g++/g++.log
> > and add -v to see what is passed to the linker.
> > Perhaps the linker reorders the libraries?
> > Or do you have LD_PRELOAD?
> 
> No LD_PRELOAD.  It adds -lasan "early", but after the libraries and
> object files that are explicitly added to the linker command.
> Since -lm is explicitly added to the linker command, the implicitly
> added -lasan comes after.  The -v command is below.

Ah, that is a powerpc*-linux* bug.  All other linux targets include
config/gnu-user.h header, perhaps early and override it, but rs6000*
seems to be the only? exception that does not.

So, either you need to include that header and perhaps tweak afterwards,
or duplicate the asan/tsan related stuff in there and make sure to keep it
up to date.

Jakub

Re: [PING, PATCH2/2, PR52252] Vectorization for load/store groups of size 3.

[Evgeny Stupachenko]

I've added a bug report for the stores group case:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61403


On Wed, May 28, 2014 at 5:18 PM, Evgeny Stupachenko  wrote:
> Ping.
> Test is modified according to the fix in the test for loads.
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr52252-st.c
> b/gcc/testsuite/gcc.dg/vect/pr52252-st.c
> new file mode 100644
> index 000..e7161f7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr52252-st.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-mssse3" { target { i?86-*-* x86_64-*-* } } } */
> +
> +#define byte unsigned char
> +
> +void
> +matrix_mul (byte *in, byte *out, int size)
> +{
> +  int i;
> +  for (i = 0; i < size; i++)
> +{
> +  out[0] = in[0] + in[1] + in[3];
> +  out[1] = in[0] + in[2] + in[4];
> +  out[2] = in[1] + in[2] + in[4];
> +  in += 4;
> +  out += 3;
> +}
> +}
> +
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
> target { i?86-*-* x86_64-*-* } } } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
>
>
> On Tue, May 6, 2014 at 6:39 PM, Evgeny Stupachenko  wrote:
>> 2nd part of patch is on stores group.
>> Bootstrap and make check passed on x86.
>>
>> Is it ok?
>>
>> 2014-05-06  Evgeny Stupachenko  
>>
>> * tree-vect-data-refs.c (vect_grouped_store_supported): New
>> check for storess group of length 3.
>> (vect_permute_store_chain): New permutations for storess group of
>> length 3.
>> * tree-vect-stmts.c (vect_model_store_cost): Change cost
>> of vec_perm_shuffle for the new permutations.
>>
>> ChangeLog for testsuite:
>>
>> 2014-05-06  Evgeny Stupachenko  
>>
>>PR tree-optimization/52252
>>* gcc.dg/vect/pr52252-st.c: Test on stores group of size 3.
>>
>> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>> index ef710cf..fb0e30d 100644
>> --- a/gcc/tree-vect-data-refs.c
>> +++ b/gcc/tree-vect-data-refs.c
>> @@ -4365,13 +4365,14 @@ vect_grouped_store_supported (tree vectype,
>> unsigned HOST_WIDE_INT count)
>>  {
>>enum machine_mode mode = TYPE_MODE (vectype);
>>
>> -  /* vect_permute_store_chain requires the group size to be a power of two. 
>>  */
>> -  if (exact_log2 (count) == -1)
>> +  /* vect_permute_store_chain requires the group size to be equal to 3 or
>> + be a power of two.  */
>> +  if (count != 3 && exact_log2 (count) == -1)
>>  {
>>if (dump_enabled_p ())
>> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> - "the size of the group of accesses"
>> - " is not a power of 2\n");
>> +"the size of the group of accesses"
>> +" is not a power of 2 or not eqaul to 3\n");
>>return false;
>>  }
>>
>> @@ -4380,23 +4381,76 @@ vect_grouped_store_supported (tree vectype,
>> unsigned HOST_WIDE_INT count)
>>  {
>>unsigned int i, nelt = GET_MODE_NUNITS (mode);
>>unsigned char *sel = XALLOCAVEC (unsigned char, nelt);
>> -  for (i = 0; i < nelt / 2; i++)
>> +
>> +  if (count == 3)
>> {
>> - sel[i * 2] = i;
>> - sel[i * 2 + 1] = i + nelt;
>> + unsigned int j0 = 0, j1 = 0, j2 = 0;
>> + unsigned int i, j;
>> +
>> + for (j = 0; j < 3; j++)
>> +   {
>> + int nelt0 = ((3 - j) * nelt) % 3;
>> + int nelt1 = ((3 - j) * nelt + 1) % 3;
>> + int nelt2 = ((3 - j) * nelt + 2) % 3;
>> + for (i = 0; i < nelt; i++)
>> +   {
>> + if (3 * i + nelt0 < nelt)
>> +   sel[3 * i + nelt0] = j0++;
>> + if (3 * i + nelt1 < nelt)
>> +   sel[3 * i + nelt1] = nelt + j1++;
>> + if (3 * i + nelt2 < nelt)
>> +   sel[3 * i + nelt2] = 0;
>> +   }
>> + if (!can_vec_perm_p (mode, false, sel))
>> +   {
>> + if (dump_enabled_p ())
>> +   dump_printf (MSG_MISSED_OPTIMIZATION,
>> +"permutaion op not supported by target.\n");
>> + return false;
>> +   }
>> +
>> + for (i = 0; i < nelt; i++)
>> +   {
>> + if (3 * i + nelt0 < nelt)
>> +   sel[3 * i + nelt0] = 3 * i + nelt0;
>> + if (3 * i + nelt1 < nelt)
>> +   sel[3 * i + nelt1] = 3 * i + nelt1;
>> + if (3 * i + nelt2 < nelt)
>> +   sel[3 * i + nelt2] = nelt + j2++;
>> +   }
>> + if (!can_vec_perm_p (mode, false, sel))
>> +   {
>> + if (dump_enabled_p ())
>> +   dump_printf (MSG_MISSED_OPTIMIZATION,
>> +"permutaion op not supported by target.\n");
>> + return false;
>> +   }
>> +   }
>> + return true;
>> }

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Ilya Enkovich]

2014-06-03 15:56 GMT+04:00 Richard Biener :
> On Tue, Jun 3, 2014 at 1:36 PM, Ilya Enkovich  wrote:
>> 2014-06-03 13:45 GMT+04:00 Richard Biener :
>>> On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  
>>> wrote:
 Hi,

 This patch adjusts alloc-free removal algorithm in DCE to take into 
 account BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.

 Bootstrapped and tested on linux-x86_64.

 Thanks,
 Ilya
 --
 gcc/

 2014-06-03  Ilya Enkovich  

 * tree-ssa-dce.c: Include target.h.
 (propagate_necessity): For free call fed by alloc check
 bounds are also provided by the same alloc.
 (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
 used by free calls.

 diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
 index 13a71ce..59a0b71 100644
 --- a/gcc/tree-ssa-dce.c
 +++ b/gcc/tree-ssa-dce.c
 @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "flags.h"
  #include "cfgloop.h"
  #include "tree-scalar-evolution.h"
 +#include "target.h"

  static struct stmt_stats
  {
 @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
   || DECL_FUNCTION_CODE (def_callee) == 
 BUILT_IN_CALLOC))
 -   continue;
 +   {
 + tree retfndecl
 +   = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
 + gimple bounds_def_stmt;
 + tree bounds;
 +
 + /* For instrumented calls we should also check used
 +bounds are returned by the same allocation call.  */
 + if (!gimple_call_with_bounds_p (stmt)
 + || ((bounds = gimple_call_arg (stmt, 1))
>>>
>>> Please provide an abstraction for this - I seem to recall seeing multiple
>>> (variants) of this.  Esp. repeated calls to the target hook above look
>>> expensive to me (generally it will not be needed).
>>>
>>> I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and 
>>> I'd
>>> like to see sth similar to gimple_call_builtin_p, for example
>>> gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
>>> target hook invocation and the fndecl check.
>>
>> I do not see how to get rid of constant in gimple_call_arg call here.
>> What semantics does proposed gimple_call_bndarg have? It has to return
>> the first bounds arg but it's not clear from its name and function
>> seems to be very specialized.
>
> Ah, ok.  I see now, so the code is ok as-is.
>
>>>
 + && TREE_CODE (bounds) == SSA_NAME
>>>
>>> What about constant bounds?  That shouldn't make the call necessary either?
>>
>> Yep, it would be useless.
>
> So please fix.
>
>>>
 + && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
 + && is_gimple_call (bounds_def_stmt)
 + && gimple_call_fndecl (bounds_def_stmt) == 
 retfndecl
 + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
 +   continue;
>>>
>>> So this all becomes
>>>
>>>if (!gimple_call_with_bounds_p (stmt)
>>>|| ((bounds = gimple_call_bndarg (stmt))
>>>&& TREE_CODE (bounds) == SSA_NAME
>>>&& (bounds_def_stmt = SSA_NAME_DEF_STMT 
>>> (bounds))
>>>&& is_gimple_call (bounds_def_stmt)
>>>&& gimple_call_chkp_p (bounds_def_stmt,
>>> BUILT_IN_CHKP_BNDRET)
>>> ...
>>>
>>> btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
>>> need a target hook to compare the fndecl?  Why isn't it enough to
>>> just check DECL_FUNCTION_CODE and DECL_BUILT_IN?
>>
>> Call is required because builtins for Pointer Bounds Checker are
>> provided by target depending on available features. That is why
>> gimple_call_builtin_p is not used. I may add new interface function
>> into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
>> I do not see how it may speed-up the code. New function would still
>> need to switch by function code and compare with proper decl which is
>> basically what we have with target hook.
>
> I don't understand - does this mean the builtin calls are in the GIMPLE
> IL even though the target doesn't have the feature?  Isn't the presence
> of the call enough?

Call is generated using function decl provided by target. Therefore we
do not know its code and has to compare using fndecl comparison.

>
>> It is possible to make faster if use the fact that all chkp builtins
>> codes (normal, not target ones) are consequent

Re: [PATCH] Do not build libsanitizer also for powerpc*-*-linux*

[Peter Bergner]

On Tue, 2014-06-03 at 15:21 +0200, Jakub Jelinek wrote:
> On Tue, Jun 03, 2014 at 08:06:41AM -0500, Peter Bergner wrote:
> > No LD_PRELOAD.  It adds -lasan "early", but after the libraries and
> > object files that are explicitly added to the linker command.
> > Since -lm is explicitly added to the linker command, the implicitly
> > added -lasan comes after.  The -v command is below.
> 
> Ah, that is a powerpc*-linux* bug.  All other linux targets include
> config/gnu-user.h header, perhaps early and override it, but rs6000*
> seems to be the only? exception that does not.
> 
> So, either you need to include that header and perhaps tweak afterwards,
> or duplicate the asan/tsan related stuff in there and make sure to keep it
> up to date.

I'll try adding the include.  Thanks for pointing that out!

Peter

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Richard Biener]

On Tue, Jun 3, 2014 at 3:27 PM, Ilya Enkovich  wrote:
> 2014-06-03 15:56 GMT+04:00 Richard Biener :
>> On Tue, Jun 3, 2014 at 1:36 PM, Ilya Enkovich  wrote:
>>> 2014-06-03 13:45 GMT+04:00 Richard Biener :
 On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  
 wrote:
> Hi,
>
> This patch adjusts alloc-free removal algorithm in DCE to take into 
> account BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-03  Ilya Enkovich  
>
> * tree-ssa-dce.c: Include target.h.
> (propagate_necessity): For free call fed by alloc check
> bounds are also provided by the same alloc.
> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
> used by free calls.
>
> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
> index 13a71ce..59a0b71 100644
> --- a/gcc/tree-ssa-dce.c
> +++ b/gcc/tree-ssa-dce.c
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "flags.h"
>  #include "cfgloop.h"
>  #include "tree-scalar-evolution.h"
> +#include "target.h"
>
>  static struct stmt_stats
>  {
> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
>   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>   || DECL_FUNCTION_CODE (def_callee) == 
> BUILT_IN_CALLOC))
> -   continue;
> +   {
> + tree retfndecl
> +   = targetm.builtin_chkp_function 
> (BUILT_IN_CHKP_BNDRET);
> + gimple bounds_def_stmt;
> + tree bounds;
> +
> + /* For instrumented calls we should also check used
> +bounds are returned by the same allocation call.  */
> + if (!gimple_call_with_bounds_p (stmt)
> + || ((bounds = gimple_call_arg (stmt, 1))

 Please provide an abstraction for this - I seem to recall seeing multiple
 (variants) of this.  Esp. repeated calls to the target hook above look
 expensive to me (generally it will not be needed).

 I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and 
 I'd
 like to see sth similar to gimple_call_builtin_p, for example
 gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
 target hook invocation and the fndecl check.
>>>
>>> I do not see how to get rid of constant in gimple_call_arg call here.
>>> What semantics does proposed gimple_call_bndarg have? It has to return
>>> the first bounds arg but it's not clear from its name and function
>>> seems to be very specialized.
>>
>> Ah, ok.  I see now, so the code is ok as-is.
>>

> + && TREE_CODE (bounds) == SSA_NAME

 What about constant bounds?  That shouldn't make the call necessary either?
>>>
>>> Yep, it would be useless.
>>
>> So please fix.
>>

> + && (bounds_def_stmt = SSA_NAME_DEF_STMT 
> (bounds))
> + && is_gimple_call (bounds_def_stmt)
> + && gimple_call_fndecl (bounds_def_stmt) == 
> retfndecl
> + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
> +   continue;

 So this all becomes

if (!gimple_call_with_bounds_p (stmt)
|| ((bounds = gimple_call_bndarg (stmt))
&& TREE_CODE (bounds) == SSA_NAME
&& (bounds_def_stmt = SSA_NAME_DEF_STMT 
 (bounds))
&& is_gimple_call (bounds_def_stmt)
&& gimple_call_chkp_p (bounds_def_stmt,
 BUILT_IN_CHKP_BNDRET)
 ...

 btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
 need a target hook to compare the fndecl?  Why isn't it enough to
 just check DECL_FUNCTION_CODE and DECL_BUILT_IN?
>>>
>>> Call is required because builtins for Pointer Bounds Checker are
>>> provided by target depending on available features. That is why
>>> gimple_call_builtin_p is not used. I may add new interface function
>>> into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
>>> I do not see how it may speed-up the code. New function would still
>>> need to switch by function code and compare with proper decl which is
>>> basically what we have with target hook.
>>
>> I don't understand - does this mean the builtin calls are in the GIMPLE
>> IL even though the target doesn't have the feature?  Isn't the presence
>> of the call enough?
>
> Call is generated using function decl provided by target. Therefore we
> 

PR61385: phiopt drops some PHIs

[Marc Glisse]

Hello,

apparently it is possible to have a PHI in the middle basic block of 
value_replacement, so I need to move it as well when I move the statement 
and remove the block.


Bootstrap and testsuite on x86_64-linux-gnu (re-running for various 
reasons but they had completed successfully yesterday).


2014-06-03  Marc Glisse  

PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
removing the basic block.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.

--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+  int *h = &c;
+  for (; c < 1; c++)
+{
+  int *i = &a, *k = &a;
+  f = 0;
+  if (b)
+   return 0;
+  if (*h)
+   {
+ int **j = &i;
+ *j = 0;
+ d = 0;
+   }
+  else
+   g = e = 0;
+  if (*h)
+   {
+ int **l = &k;
+ *l = &g;
+   }
+  d &= *h;
+  assert (k == &a || k);
+  assert (i);
+}
+  return 0;
+}
+
+int
+main ()
+{
+  fn1 (); 
+  return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c   (revision 211178)
+++ gcc/tree-ssa-phiopt.c   (working copy)
@@ -877,20 +877,39 @@ value_replacement (basic_block cond_bb,
   && operand_equal_for_phi_arg_p (rhs2, cond_lhs)
   && neutral_element_p (code_def, cond_rhs, true))
  || (arg1 == rhs2
  && operand_equal_for_phi_arg_p (rhs1, cond_lhs)
  && neutral_element_p (code_def, cond_rhs, false))
  || (operand_equal_for_phi_arg_p (arg1, cond_rhs)
  && (operand_equal_for_phi_arg_p (rhs2, cond_lhs)
  || operand_equal_for_phi_arg_p (rhs1, cond_lhs))
  && absorbing_element_p (code_def, cond_rhs
 {
+  /* Move potential PHI nodes.  */
+  gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+  while (!gsi_end_p (psi))
+   {
+ gimple phi_moving = gsi_stmt (psi);
+ gimple newphi = create_phi_node (gimple_phi_result (phi_moving),
+  cond_bb);
+ int nargs = cond_bb->preds->length();
+ location_t loc = gimple_phi_arg_location (phi_moving, 0);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ for (int i = 0; i < nargs; ++i)
+   {
+ edge e = (*cond_bb->preds)[i];
+ add_phi_arg (newphi, phi_arg, e, loc);
+   }
+ update_stmt (newphi);
+ gsi_remove (&psi, false);
+   }
+
   gsi = gsi_for_stmt (cond);
   gimple_stmt_iterator gsi_from = gsi_for_stmt (assign);
   gsi_move_before (&gsi_from, &gsi);
   replace_phi_edge_with_variable (cond_bb, e1, phi, lhs);
   return 2;
 }
 
   return 0;
 }
 

Re: ipa-visibility TLC 2/n

[David Edelsohn]

Honza,

How can we make further progress with the large regression on AIX?

Thanks, David

On Fri, May 30, 2014 at 1:24 PM, David Edelsohn  wrote:
> Honza,
>
> For example g++.dg/abi/vcall1.C fails at a call in a "localalias"
> function, which jumps to a bad location:
>
>
> (gdb) up
> #1  0x14c0 in B::B() [clone .localalias.2] ()
> (gdb) x/16i $pc-32
>0x14a0 <_ZN1BC2Ev+156>:  add r10,r10,r8
>0x14a4 <_ZN1BC2Ev+160>:  mr  r3,r10
>0x14a8 <_ZN1BC2Ev+164>:  stw r2,20(r1)
>0x14ac <_ZN1BC2Ev+168>:  lwz r10,0(r9)
>0x14b0 <_ZN1BC2Ev+172>:  lwz r11,8(r9)
>0x14b4 <_ZN1BC2Ev+176>:  mtctr   r10
>0x14b8 <_ZN1BC2Ev.localalias.2+180>: lwz r2,4(r9)
>0x14bc <_ZN1BC2Ev.localalias.2+184>: bctrl
> => 0x14c0 <_ZN1BC2Ev.localalias.2+188>: lwz r2,20(r1)
>0x14c4 <_ZN1BC2Ev.localalias.2+192>: addir1,r31,64
>0x14c8 <_ZN1BC2Ev.localalias.2+196>: lwz r0,8(r1)
>0x14cc <_ZN1BC2Ev.localalias.2+200>: mtlrr0
>0x14d0 <_ZN1BC2Ev.localalias.2+204>: lwz r31,-4(r1)
>0x14d4 <_ZN1BC2Ev.localalias.2+208>: blr
>
>
>
> On Fri, May 30, 2014 at 3:20 AM, Richard Sandiford
>  wrote:
>> Jan Hubicka  writes:
 Jan Hubicka  writes:
 >> Richard Sandiford wrote the original section anchors implementation,
 >> so he would be a good person to comment about the interaction between
 >> aliases and section anchors.
 >
 > Thanks! Richard, does this patch seem sane?

 Looks good to me in principle, but with:

 > +  struct symtab_node *snode;
 >decl = SYMBOL_REF_DECL (symbol);
 > +
 > +  snode = symtab_node (decl);
 > +  if (snode->alias)
 > +   {
 > + rtx target = DECL_RTL (symtab_alias_ultimate_target
 > (snode)->decl);
 > + SYMBOL_REF_BLOCK_OFFSET (symbol) = SYMBOL_REF_BLOCK_OFFSET
 > (target);
 > + return;
 > +   }

 is SYMBOL_REF_BLOCK_OFFSET (target) guaranteed to be valid at this point?
 It looked at face value like you'd need a recursive call to 
 place_block_symbol
 on the target before the copy.
>>>
>>> My reading was that SYMBOL_REF_BLOCK_OFFSET is computed at DECL_RTL
>>> calculation time. But you are right - it is done by validize_mem that
>>> is not done by DECL_RTL.  Shall I just call it on target first?
>>
>> Yeah, sounds like calling place_block_symbol would be safer.
>>
>> IIRC, the reason I didn't do it at SET_DECL_RTL time is that some frontends
>> tended to create placeholder decls that for whatever reason ended up with
>> an initial DECL_RTL, then changed the properties of the decl later once
>> more information was known.  (This was all many years ago.)
>>
>> Thanks,
>> Richard

Re: calloc = malloc + memset

[Marc Glisse]

Ping?

On Sat, 17 May 2014, Marc Glisse wrote:


Ping Jakub?
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01104.html

On Wed, 23 Apr 2014, Richard Biener wrote:


On Fri, Apr 18, 2014 at 8:27 PM, Marc Glisse  wrote:

Thanks for the comments!


On Fri, 18 Apr 2014, Jakub Jelinek wrote:


The passes.def change makes me a little bit nervous, but if it works,
perhaps.



Would you prefer running the pass twice? I thought there would be less
resistance to moving the pass than duplicating it.


Indeed.  I think placing it after loops and CSE (thus what you have done)
makes sense.  strlenopt itself shouldn't enable much additional
optimizations.  But well, pass ordering is always tricky.

Didn't look at the rest of the changes, but Jakub is certainly able to
approve the patch so I leave it to him.


--
Marc Glisse

Re: PR61385: phiopt drops some PHIs

[Richard Biener]

On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse  wrote:
> Hello,
>
> apparently it is possible to have a PHI in the middle basic block of
> value_replacement, so I need to move it as well when I move the statement
> and remove the block.
>
> Bootstrap and testsuite on x86_64-linux-gnu (re-running for various reasons
> but they had completed successfully yesterday).
>
> 2014-06-03  Marc Glisse  
>
> PR tree-optimization/61385
> gcc/
> * tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
> removing the basic block.
> gcc/testsuite/
> * gcc.dg/tree-ssa/pr61385.c: New file.
>
> --
> Marc Glisse
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
> @@ -0,0 +1,43 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +#define assert(x) if (!(x)) __builtin_abort ()
> +
> +int a, b, c, d, e, f, g;
> +
> +int
> +fn1 ()
> +{
> +  int *h = &c;
> +  for (; c < 1; c++)
> +{
> +  int *i = &a, *k = &a;
> +  f = 0;
> +  if (b)
> +   return 0;
> +  if (*h)
> +   {
> + int **j = &i;
> + *j = 0;
> + d = 0;
> +   }
> +  else
> +   g = e = 0;
> +  if (*h)
> +   {
> + int **l = &k;
> + *l = &g;
> +   }
> +  d &= *h;
> +  assert (k == &a || k);
> +  assert (i);
> +}
> +  return 0;
> +}
> +
> +int
> +main ()
> +{
> +  fn1 ();
> +  return 0;
> +}
> Index: gcc/tree-ssa-phiopt.c
> ===
> --- gcc/tree-ssa-phiopt.c   (revision 211178)
> +++ gcc/tree-ssa-phiopt.c   (working copy)
> @@ -877,20 +877,39 @@ value_replacement (basic_block cond_bb,
>&& operand_equal_for_phi_arg_p (rhs2, cond_lhs)
>&& neutral_element_p (code_def, cond_rhs, true))
>   || (arg1 == rhs2
>   && operand_equal_for_phi_arg_p (rhs1, cond_lhs)
>   && neutral_element_p (code_def, cond_rhs, false))
>   || (operand_equal_for_phi_arg_p (arg1, cond_rhs)
>   && (operand_equal_for_phi_arg_p (rhs2, cond_lhs)
>   || operand_equal_for_phi_arg_p (rhs1, cond_lhs))
>   && absorbing_element_p (code_def, cond_rhs
>  {
> +  /* Move potential PHI nodes.  */
> +  gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
> +  while (!gsi_end_p (psi))
> +   {
> + gimple phi_moving = gsi_stmt (psi);
> + gimple newphi = create_phi_node (gimple_phi_result (phi_moving),
> +  cond_bb);
> + int nargs = cond_bb->preds->length();
> + location_t loc = gimple_phi_arg_location (phi_moving, 0);
> + tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
> + for (int i = 0; i < nargs; ++i)
> +   {
> + edge e = (*cond_bb->preds)[i];
> + add_phi_arg (newphi, phi_arg, e, loc);

All arguments get the same value (and the PHI in middle-bb is surely
a singleton?), so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block.  If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.

Richard.

> +   }
> + update_stmt (newphi);
> + gsi_remove (&psi, false);
> +   }
> +
>gsi = gsi_for_stmt (cond);
>gimple_stmt_iterator gsi_from = gsi_for_stmt (assign);
>gsi_move_before (&gsi_from, &gsi);
>replace_phi_edge_with_variable (cond_bb, e1, phi, lhs);
>return 2;
>  }
>
>return 0;
>  }
>
>

C++ PATCH for c++/60848 (ICE with user-defined std::initializer_list)

[Jason Merrill]

Here the testcase defines std::initializer_list as a non-template class, 
so looking up its argument fails.  I don't want to get into a lot of 
sanity checking for the definition of initializer_list, but this seems 
simple enough.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit ea126f002069e1f25aeb64b799095aeae8c01482
Author: Jason Merrill 
Date:   Tue Jun 3 09:38:39 2014 -0400

	PR c++/60848
	* call.c (is_std_init_list): Check CLASSTYPE_TEMPLATE_INFO.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 77aa8ca..b60bab7 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -9675,6 +9675,7 @@ is_std_init_list (tree type)
   type = TYPE_MAIN_VARIANT (type);
   return (CLASS_TYPE_P (type)
 	  && CP_TYPE_CONTEXT (type) == std_node
+	  && CLASSTYPE_TEMPLATE_INFO (type)
 	  && strcmp (TYPE_NAME_STRING (type), "initializer_list") == 0);
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist85.C b/gcc/testsuite/g++.dg/cpp0x/initlist85.C
new file mode 100644
index 000..0eb8e26
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist85.C
@@ -0,0 +1,14 @@
+// PR c++/60848
+// { dg-do compile { target c++11 } }
+
+namespace std
+{
+  struct initializer_list {};
+}
+
+void foo(std::initializer_list &);
+
+void f()
+{
+  foo({1, 2});			// { dg-error "" }
+}

Re: [PATCH, Pointer Bounds Checker 25/x] DCE

[Ilya Enkovich]

2014-06-03 17:41 GMT+04:00 Richard Biener :
> On Tue, Jun 3, 2014 at 3:27 PM, Ilya Enkovich  wrote:
>> 2014-06-03 15:56 GMT+04:00 Richard Biener :
>>> On Tue, Jun 3, 2014 at 1:36 PM, Ilya Enkovich  
>>> wrote:
 2014-06-03 13:45 GMT+04:00 Richard Biener :
> On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich  
> wrote:
>> Hi,
>>
>> This patch adjusts alloc-free removal algorithm in DCE to take into 
>> account BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>>
>> Bootstrapped and tested on linux-x86_64.
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-06-03  Ilya Enkovich  
>>
>> * tree-ssa-dce.c: Include target.h.
>> (propagate_necessity): For free call fed by alloc check
>> bounds are also provided by the same alloc.
>> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
>> used by free calls.
>>
>> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
>> index 13a71ce..59a0b71 100644
>> --- a/gcc/tree-ssa-dce.c
>> +++ b/gcc/tree-ssa-dce.c
>> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "flags.h"
>>  #include "cfgloop.h"
>>  #include "tree-scalar-evolution.h"
>> +#include "target.h"
>>
>>  static struct stmt_stats
>>  {
>> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
>>   && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>>   && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>>   || DECL_FUNCTION_CODE (def_callee) == 
>> BUILT_IN_CALLOC))
>> -   continue;
>> +   {
>> + tree retfndecl
>> +   = targetm.builtin_chkp_function 
>> (BUILT_IN_CHKP_BNDRET);
>> + gimple bounds_def_stmt;
>> + tree bounds;
>> +
>> + /* For instrumented calls we should also check used
>> +bounds are returned by the same allocation call.  */
>> + if (!gimple_call_with_bounds_p (stmt)
>> + || ((bounds = gimple_call_arg (stmt, 1))
>
> Please provide an abstraction for this - I seem to recall seeing multiple
> (variants) of this.  Esp. repeated calls to the target hook above look
> expensive to me (generally it will not be needed).
>
> I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and 
> I'd
> like to see sth similar to gimple_call_builtin_p, for example
> gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
> target hook invocation and the fndecl check.

 I do not see how to get rid of constant in gimple_call_arg call here.
 What semantics does proposed gimple_call_bndarg have? It has to return
 the first bounds arg but it's not clear from its name and function
 seems to be very specialized.
>>>
>>> Ah, ok.  I see now, so the code is ok as-is.
>>>
>
>> + && TREE_CODE (bounds) == SSA_NAME
>
> What about constant bounds?  That shouldn't make the call necessary 
> either?

 Yep, it would be useless.
>>>
>>> So please fix.
>>>
>
>> + && (bounds_def_stmt = SSA_NAME_DEF_STMT 
>> (bounds))
>> + && is_gimple_call (bounds_def_stmt)
>> + && gimple_call_fndecl (bounds_def_stmt) == 
>> retfndecl
>> + && gimple_call_arg (bounds_def_stmt, 0) == 
>> ptr))
>> +   continue;
>
> So this all becomes
>
>if (!gimple_call_with_bounds_p (stmt)
>|| ((bounds = gimple_call_bndarg (stmt))
>&& TREE_CODE (bounds) == SSA_NAME
>&& (bounds_def_stmt = SSA_NAME_DEF_STMT 
> (bounds))
>&& is_gimple_call (bounds_def_stmt)
>&& gimple_call_chkp_p (bounds_def_stmt,
> BUILT_IN_CHKP_BNDRET)
> ...
>
> btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
> need a target hook to compare the fndecl?  Why isn't it enough to
> just check DECL_FUNCTION_CODE and DECL_BUILT_IN?

 Call is required because builtins for Pointer Bounds Checker are
 provided by target depending on available features. That is why
 gimple_call_builtin_p is not used. I may add new interface function
 into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
 I do not see how it may speed-up the code. New function would still
 need to switch by function code and compare with proper decl which is
 basically what we have with target hook.
>>>
>>> I don't understand - does this mean the builtin calls are in the G

[patch,avr] atmel avr new devices set-2

[S, Pitchumani]

Hi,

Attached patch adds support for Atmel devices tiny441, tiny828 and tiny841.

Please commit if the patch is OK.

Note: This is continuation of patch attached in 
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00049.html

Regards,
Pitchumani

gcc/ChangeLog
2014-06-03  Vishnu K S 

* config/avr/avr-mcus.def: Add new avr25 devices attiny441, attiny828 
and attiny841.
* config/avr/avr-tables.opt: Regenerate.
* config/avr/t-multilib: Regenerate.
* doc/avr-mmcu.texi: Regenerate.


tiny-devices-support.patch
Description: tiny-devices-support.patch

[C++ Patch] PR 52174 aka DR 1423

[Paolo Carlini]

Hi,

implementing the resolution seems rather straightforward, just check 
LOOKUP_ONLYCONVERTING in standard_conversion. However, while playing 
with some additional tests outside bug & testsuite (similar to 
nullptr32.C), I noticed a latent issue: in case of base initializers we 
were setting anyway LOOKUP_ONLYCONVERTING in add_function_candidate (the 
other cases listed in 8.5/16 are handled elsewhere and seem all fine). 
Thus the rather ad-hoc-ish tweak there: I spent some time on it and 
couldn't find anything reasonably simple & better, appears to work 
anyway, for templates too.


Tested x86_64-linux.

Thanks,
Paolo.

//
gcc/cp
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* call.c (standard_conversion): Convert nullptr to bool only
in case of direct-initialization.
(add_function_candidate): Do not set LOOKUP_ONLYCONVERTING
in lflags when processing a base initializer. 

gcc/testsuite
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* g++.dg/cpp0x/nullptr31.C: New.
* g++.dg/cpp0x/nullptr32.C: Likewise.
* g++.dg/cpp0x/sfinae-nullptr1.C: Likewise.
* g++.dg/cpp0x/nullptr17.C: Update.

libstdc++-v3
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* testsuite/20_util/is_assignable/value.cc: Update.
Index: gcc/cp/call.c
===
--- gcc/cp/call.c   (revision 211162)
+++ gcc/cp/call.c   (working copy)
@@ -1311,15 +1311,15 @@ standard_conversion (tree to, tree from, tree expr
 {
   /* [conv.bool]
 
- An rvalue of arithmetic, unscoped enumeration, pointer, or
- pointer to member type can be converted to an rvalue of type
- bool. ... An rvalue of type std::nullptr_t can be converted
- to an rvalue of type bool;  */
+ A prvalue of arithmetic, unscoped enumeration, pointer, or pointer
+ to member type can be converted to a prvalue of type bool. ...
+ For direct-initialization (8.5 [dcl.init]), a prvalue of type
+ std::nullptr_t can be converted to a prvalue of type bool;  */
   if (ARITHMETIC_TYPE_P (from)
  || UNSCOPED_ENUM_P (from)
  || fcode == POINTER_TYPE
  || TYPE_PTRMEM_P (from)
- || NULLPTR_TYPE_P (from))
+ || (NULLPTR_TYPE_P (from) && !(flags & LOOKUP_ONLYCONVERTING)))
{
  conv = build_conv (ck_std, to, conv);
  if (fcode == POINTER_TYPE
@@ -2056,7 +2056,10 @@ add_function_candidate (struct z_candidate **candi
  && BRACE_ENCLOSED_INITIALIZER_P (arg))
lflags |= LOOKUP_NO_CONVERSION;
}
- else
+ /* Per 8.5/16, certainly the initialization that occurs in
+base initializers is direct-initialization, thus don't
+set the flag in that case.  */
+ else if (!(cfun && cp_function_chain && in_base_initializer))
lflags |= LOOKUP_ONLYCONVERTING;
 
  t = implicit_conversion (parmtype, argtype, arg,
Index: gcc/testsuite/g++.dg/cpp0x/nullptr17.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (revision 211162)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (working copy)
@@ -1,6 +1,7 @@
 // { dg-do compile { target c++11 } }
 
-// Test that bool is a better overload match than int
+// Used to test that bool is a better overload match than int
+// Updated for DR 1423
 
 template  struct tType_equal;
 template  struct tType_equal { typedef void type; };
@@ -16,7 +17,7 @@ bool i( bool );
 void test_i()
 {
   // Overload to bool, not int
-  type_equal(i(nullptr));
+  type_equal(i(nullptr));  // { dg-error "no matching" }
   decltype(nullptr) mynull = 0;
-  type_equal(i(mynull));
+  type_equal(i(mynull));   // { dg-error "no matching" }
 }
Index: gcc/testsuite/g++.dg/cpp0x/nullptr31.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (working copy)
@@ -0,0 +1,11 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+bool b1 = nullptr;  // { dg-error "cannot convert" }
+
+bool b2(nullptr);
+bool b3{nullptr};
+
+int  i1 = nullptr;  // { dg-error "cannot convert" }
+int  i2(nullptr);   // { dg-error "cannot convert" }
+int  i3{nullptr};   // { dg-error "cannot convert" }
Index: gcc/testsuite/g++.dg/cpp0x/nullptr32.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr32.C  (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr32.C  (working copy)
@@ -0,0 +1,30 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+struct Base
+{
+  Base(bool);
+};
+
+struct Derived
+: Base
+{
+  Derived()
+  : Base(nullptr) { }
+};
+
+template
+struct TBase
+{
+  TBase(bool);
+};
+
+template
+struct TD

Re: PR61385: phiopt drops some PHIs

[Marc Glisse]

On Tue, 3 Jun 2014, Richard Biener wrote:


On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse  wrote:

Hello,

apparently it is possible to have a PHI in the middle basic block of
value_replacement, so I need to move it as well when I move the statement
and remove the block.

Bootstrap and testsuite on x86_64-linux-gnu (re-running for various reasons
but they had completed successfully yesterday).

2014-06-03  Marc Glisse  

PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
removing the basic block.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.

--
Marc Glisse
Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+  int *h = &c;
+  for (; c < 1; c++)
+{
+  int *i = &a, *k = &a;
+  f = 0;
+  if (b)
+   return 0;
+  if (*h)
+   {
+ int **j = &i;
+ *j = 0;
+ d = 0;
+   }
+  else
+   g = e = 0;
+  if (*h)
+   {
+ int **l = &k;
+ *l = &g;
+   }
+  d &= *h;
+  assert (k == &a || k);
+  assert (i);
+}
+  return 0;
+}
+
+int
+main ()
+{
+  fn1 ();
+  return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c   (revision 211178)
+++ gcc/tree-ssa-phiopt.c   (working copy)
@@ -877,20 +877,39 @@ value_replacement (basic_block cond_bb,
   && operand_equal_for_phi_arg_p (rhs2, cond_lhs)
   && neutral_element_p (code_def, cond_rhs, true))
  || (arg1 == rhs2
  && operand_equal_for_phi_arg_p (rhs1, cond_lhs)
  && neutral_element_p (code_def, cond_rhs, false))
  || (operand_equal_for_phi_arg_p (arg1, cond_rhs)
  && (operand_equal_for_phi_arg_p (rhs2, cond_lhs)
  || operand_equal_for_phi_arg_p (rhs1, cond_lhs))
  && absorbing_element_p (code_def, cond_rhs
 {
+  /* Move potential PHI nodes.  */
+  gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+  while (!gsi_end_p (psi))
+   {
+ gimple phi_moving = gsi_stmt (psi);
+ gimple newphi = create_phi_node (gimple_phi_result (phi_moving),
+  cond_bb);
+ int nargs = cond_bb->preds->length();
+ location_t loc = gimple_phi_arg_location (phi_moving, 0);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ for (int i = 0; i < nargs; ++i)
+   {
+ edge e = (*cond_bb->preds)[i];
+ add_phi_arg (newphi, phi_arg, e, loc);


All arguments get the same value (and the PHI in middle-bb is surely
a singleton?),


Yes, there is a single incoming edge to middle-bb.


so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block.


I thought there might be a reason why it was a PHI and not an assignment 
so I wasn't sure doing that would be ok. It is indeed much easier, so I'll 
do that...



If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.


... or indeed that, now that I have found a function called 
replace_uses_by which should do exactly what I need.


Thanks,

--
Marc Glisse

Re: [C++ Patch] PR 52174 aka DR 1423

[Paolo Carlini]

.. wondering if I should check DECL_CONSTRUCTOR_P (fn) too.

Paolo.

Re: [C PATCH] Warn if switch has boolean value (PR c/60439)

[Marek Polacek]

On Mon, Jun 02, 2014 at 06:00:04PM -0400, Jason Merrill wrote:
> On 05/24/2014 04:00 AM, Marek Polacek wrote:
> >+  /* Warn if the condition has boolean value.  */
> >+  tree e = cond;
> >+  while (TREE_CODE (e) == COMPOUND_EXPR)
> >+e = TREE_OPERAND (e, 1);
> >+
> >+  if (TREE_CODE (orig_type) == BOOLEAN_TYPE
> >+  || (truth_value_p (TREE_CODE (e))
> >+  && TREE_CODE (orig_type) != INTEGER_TYPE))
> >+warning_at (input_location, OPT_Wswitch_bool,
> >+"switch condition has boolean value");
> 
> For C++ it should be "type bool", not "boolean value".  And it should be

Ok, changed (but not for C), and adjusted the testcase.

> enough to just check for BOOLEAN_TYPE, without looking through
> COMPOUND_EXPR.
 
Nice.  I dropped looking through COMPOUND_EXPR.

> >2) Since the warning is now enabled even for the C++ FE, it's
> >   exercised during bootstrap.  Turned out that gengtype generates
> >   code like
> >   switch (TREE_CODE (...) == INTEGER_TYPE) { ... }
> >   that would mar the bootstrap - so I tweaked it to generate
> >   switch ((int) (TREE_CODE (...) == INTEGER_TYPE) { ... })
> >   instead.  Does that make sense?
> 
> Makes sense to me.

Thanks.  I regtested the following, bootstrap is still in progress, but
I don't expect any issues.

Ok?

2014-06-03  Marek Polacek  

PR c/60439
* doc/invoke.texi: Document -Wswitch-bool.
* function.c (stack_protect_epilogue): Cast controlling expression of
the switch to int.
* gengtype.c (walk_type): Generate switch expression with its
controlling expression cast to int.
c/
* c-parser.c (c_parser_switch_statement): Pass explicit_cast_p to
c_start_case.
* c-tree.h (c_start_case): Update.
* c-typeck.c (c_start_case): Add new boolean parameter.  Warn if
switch condition has boolean value.
cp/
* semantics.c (finish_switch_cond): Warn if switch condition has
boolean value.
c-family/
* c.opt (Wswitch-bool): New option.
testsuite/
* c-c++-common/pr60439.c: New test.
* g++.dg/eh/scope1.C (f4): Add dg-warning.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index c586e65..5d36a80 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -534,6 +534,10 @@ Wswitch-enum
 C ObjC C++ ObjC++ Var(warn_switch_enum) Warning
 Warn about all enumerated switches missing a specific case
 
+Wswitch-bool
+C ObjC C++ ObjC++ Warning Init(1)
+Warn about switches with boolean controlling expression
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format)
 ;
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 1d9780e..abd636c 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -5197,9 +5197,13 @@ c_parser_switch_statement (c_parser *parser)
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_SWITCH));
   c_parser_consume_token (parser);
   block = c_begin_compound_stmt (flag_isoc99);
+  bool explicit_cast_p = false;
   if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
 {
   switch_cond_loc = c_parser_peek_token (parser)->location;
+  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
+ && c_token_starts_typename (c_parser_peek_2nd_token (parser)))
+   explicit_cast_p = true;
   ce = c_parser_expression (parser);
   ce = convert_lvalue_to_rvalue (switch_cond_loc, ce, true, false);
   expr = ce.value;
@@ -5217,7 +5221,7 @@ c_parser_switch_statement (c_parser *parser)
   switch_cond_loc = UNKNOWN_LOCATION;
   expr = error_mark_node;
 }
-  c_start_case (switch_loc, switch_cond_loc, expr);
+  c_start_case (switch_loc, switch_cond_loc, expr, explicit_cast_p);
   save_break = c_break_label;
   c_break_label = NULL_TREE;
   body = c_parser_c99_block_statement (parser);
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index e7dcb35..133930f 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -614,7 +614,7 @@ extern void process_init_element (location_t, struct 
c_expr, bool,
  struct obstack *);
 extern tree build_compound_literal (location_t, tree, tree, bool);
 extern void check_compound_literal_type (location_t, struct c_type_name *);
-extern tree c_start_case (location_t, location_t, tree);
+extern tree c_start_case (location_t, location_t, tree, bool);
 extern void c_finish_case (tree);
 extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
 extern tree build_asm_stmt (tree, tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 6ca584b..a98ce07 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9361,12 +9361,13 @@ struct c_switch *c_switch_stack;
 
 /* Start a C switch statement, testing expression EXP.  Return the new
SWITCH_EXPR.  SWITCH_LOC is the location of the `switch'.
-   SWITCH_COND_LOC is the location of the switch's condition.  */
+   SWITCH_COND_LOC is the location of the switch's condition.
+   EXPLICIT_CAST_P is true if the expression EXP has exp

Re: [C++ Patch] PR 52174 aka DR 1423

[Jason Merrill]

On 06/03/2014 10:30 AM, Paolo Carlini wrote:

implementing the resolution seems rather straightforward, just check
LOOKUP_ONLYCONVERTING in standard_conversion.


Yep.  Though it would be better to return a bad_p conversion than none 
at all.



However, while playing
with some additional tests outside bug & testsuite (similar to
nullptr32.C), I noticed a latent issue: in case of base initializers we
were setting anyway LOOKUP_ONLYCONVERTING in add_function_candidate


Right.  In the case of


+  TDerived()
+  : TBase(nullptr) { }


we have direct-initialization of TBase, but the parameter of the 
TBase constructor is copy-initialized, so nullptr32.C is ill-formed; 
please drop this hunk of the patch.


Jason

Re: [C++ Patch] PR 52174 aka DR 1423

[Paolo Carlini]

Hi,

On 06/03/2014 05:02 PM, Jason Merrill wrote:

Right. In the case of


+  TDerived()
+  : TBase(nullptr) { }


we have direct-initialization of TBase, but the parameter of the 
TBase constructor is copy-initialized, so nullptr32.C is ill-formed; 
please drop this hunk of the patch.

Oops, now I see, thanks. Thus I'm finishing testing the below.

Thanks,
Paolo.


gcc/cp
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* call.c (standard_conversion): Convert nullptr to bool only
in case of direct-initialization.

gcc/testsuite
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* g++.dg/cpp0x/nullptr31.C: New.
* g++.dg/cpp0x/sfinae-nullptr1.C: Likewise.
* g++.dg/cpp0x/nullptr17.C: Update.

libstdc++-v3
2014-06-03  Paolo Carlini  

DR 1423
PR c++/52174
* testsuite/20_util/is_assignable/value.cc: Update.
Index: gcc/cp/call.c
===
--- gcc/cp/call.c   (revision 211162)
+++ gcc/cp/call.c   (working copy)
@@ -1311,10 +1311,10 @@ standard_conversion (tree to, tree from, tree expr
 {
   /* [conv.bool]
 
- An rvalue of arithmetic, unscoped enumeration, pointer, or
- pointer to member type can be converted to an rvalue of type
- bool. ... An rvalue of type std::nullptr_t can be converted
- to an rvalue of type bool;  */
+ A prvalue of arithmetic, unscoped enumeration, pointer, or pointer
+ to member type can be converted to a prvalue of type bool. ...
+ For direct-initialization (8.5 [dcl.init]), a prvalue of type
+ std::nullptr_t can be converted to a prvalue of type bool;  */
   if (ARITHMETIC_TYPE_P (from)
  || UNSCOPED_ENUM_P (from)
  || fcode == POINTER_TYPE
@@ -1328,6 +1328,8 @@ standard_conversion (tree to, tree from, tree expr
  && conv->rank < cr_pbool)
  || NULLPTR_TYPE_P (from))
conv->rank = cr_pbool;
+ if (NULLPTR_TYPE_P (from) && (flags & LOOKUP_ONLYCONVERTING))
+   conv->bad_p = true;
  return conv;
}
 
Index: gcc/testsuite/g++.dg/cpp0x/nullptr17.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (revision 211162)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (working copy)
@@ -1,6 +1,7 @@
 // { dg-do compile { target c++11 } }
 
-// Test that bool is a better overload match than int
+// Used to test that bool is a better overload match than int
+// Updated for DR 1423
 
 template  struct tType_equal;
 template  struct tType_equal { typedef void type; };
@@ -16,7 +17,7 @@ bool i( bool );
 void test_i()
 {
   // Overload to bool, not int
-  type_equal(i(nullptr));
+  type_equal(i(nullptr));  // { dg-error "invalid conversion" }
   decltype(nullptr) mynull = 0;
-  type_equal(i(mynull));
+  type_equal(i(mynull));   // { dg-error "invalid conversion" }
 }
Index: gcc/testsuite/g++.dg/cpp0x/nullptr31.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (working copy)
@@ -0,0 +1,11 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+bool b1 = nullptr;  // { dg-error "invalid conversion" }
+
+bool b2(nullptr);
+bool b3{nullptr};
+
+int  i1 = nullptr;  // { dg-error "cannot convert" }
+int  i2(nullptr);   // { dg-error "cannot convert" }
+int  i3{nullptr};   // { dg-error "cannot convert" }
Index: gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C
===
--- gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C(working copy)
@@ -0,0 +1,18 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+template
+T&& make();
+
+template
+void sink(T);
+
+template(make()))
+>
+auto f(int) -> char(&)[1];
+
+template
+auto f(...) -> char(&)[2];
+
+static_assert(sizeof(f(0)) != 1, "");
Index: libstdc++-v3/testsuite/20_util/is_assignable/value.cc
===
--- libstdc++-v3/testsuite/20_util/is_assignable/value.cc   (revision 
211151)
+++ libstdc++-v3/testsuite/20_util/is_assignable/value.cc   (working copy)
@@ -174,7 +174,7 @@ static_assert(!std::is_assignable::valu
 static_assert(std::is_assignable::value, "Error");
 static_assert(std::is_assignable::value, "Error");
 static_assert(std::is_assignable::value, "Error");
-static_assert(std::is_assignable::value, "Error");
+static_assert(!std::is_assignable::value, "Error");
 
 static_assert(std::is_assignable::value, "Error");

Re: C++ PATCH for c++/60992 (lambda and constant variable)

[Jason Merrill]

A small further tweak: let's try lookup first and when we do create a 
new decl, stash it in local_specializations.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit 92a208c9176551243e305e779c7aaa730cace8f5
Author: Jason Merrill 
Date:   Tue Jun 3 10:09:23 2014 -0400

	PR c++/60992
	* pt.c (tsubst_copy) [VAR_DECL]: Try lookup first.  Add a new
	variable to local_specializations.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index d267a5c..8858908 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12730,14 +12730,19 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	  r = retrieve_local_specialization (t);
 	  if (r == NULL_TREE)
 	{
-	  if (DECL_ANON_UNION_VAR_P (t))
+	  /* First try name lookup to find the instantiation.  */
+	  r = lookup_name (DECL_NAME (t));
+	  if (r)
 		{
-		  /* Just use name lookup to find a member alias for an
-		 anonymous union, but then add it to the hash table.  */
-		  r = lookup_name (DECL_NAME (t));
-		  gcc_assert (DECL_ANON_UNION_VAR_P (r));
-		  register_local_specialization (r, t);
+		  /* Make sure that the one we found is the one we want.  */
+		  tree ctx = tsubst (DECL_CONTEXT (t), args,
+ complain, in_decl);
+		  if (ctx != DECL_CONTEXT (r))
+		r = NULL_TREE;
 		}
+
+	  if (r)
+		/* OK */;
 	  else
 		{
 		  /* This can happen for a variable used in a
@@ -12771,10 +12776,12 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		  else if (decl_constant_var_p (r))
 			/* A use of a local constant decays to its value.
 			   FIXME update for core DR 696.  */
-			return integral_constant_value (r);
+			r = integral_constant_value (r);
 		}
-		  return r;
 		}
+	  /* Remember this for subsequent uses.  */
+	  if (local_specializations)
+		register_local_specialization (r, t);
 	}
 	}
   else

Re: [C++ Patch] PR 52174 aka DR 1423

[Jason Merrill]

On 06/03/2014 11:24 AM, Paolo Carlini wrote:

+ if (NULLPTR_TYPE_P (from) && (flags & LOOKUP_ONLYCONVERTING))
+   conv->bad_p = true;


Thanks.  What kind of error message do we get with this change?  Would 
adding something to convert_like_real provide a more helpful diagnostic?


Jason

Re: [C PATCH] Warn if switch has boolean value (PR c/60439)

[Jason Merrill]

On 06/03/2014 10:57 AM, Marek Polacek wrote:

+ if (TREE_CODE (orig_type) == BOOLEAN_TYPE
+ || (truth_value_p (TREE_CODE (cond))
+ && TREE_CODE (orig_type) != INTEGER_TYPE))


I don't think you need the truth_value_p check, either, just the 
BOOLEAN_TYPE check.


Jason

Re: [C PATCH] Warn if switch has boolean value (PR c/60439)

[Marek Polacek]

On Tue, Jun 03, 2014 at 11:46:35AM -0400, Jason Merrill wrote:
> On 06/03/2014 10:57 AM, Marek Polacek wrote:
> >+  if (TREE_CODE (orig_type) == BOOLEAN_TYPE
> >+  || (truth_value_p (TREE_CODE (cond))
> >+  && TREE_CODE (orig_type) != INTEGER_TYPE))
> 
> I don't think you need the truth_value_p check, either, just the
> BOOLEAN_TYPE check.

Oh yeah, since the type of &&, ||, etc. exprs is in C++ bool...
Done in the following, the testcases still passes.  Otherwise no
changes.

2014-06-03  Marek Polacek  

PR c/60439
* doc/invoke.texi: Document -Wswitch-bool.
* function.c (stack_protect_epilogue): Cast controlling expression of
the switch to int.
* gengtype.c (walk_type): Generate switch expression with its
controlling expression cast to int.
c/
* c-parser.c (c_parser_switch_statement): Pass explicit_cast_p to
c_start_case.
* c-tree.h (c_start_case): Update.
* c-typeck.c (c_start_case): Add new boolean parameter.  Warn if
switch condition has boolean value.
cp/
* semantics.c (finish_switch_cond): Warn if switch condition has
boolean value.
c-family/
* c.opt (Wswitch-bool): New option.
testsuite/
* c-c++-common/pr60439.c: New test.
* g++.dg/eh/scope1.C (f4): Add dg-warning.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index c586e65..5d36a80 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -534,6 +534,10 @@ Wswitch-enum
 C ObjC C++ ObjC++ Var(warn_switch_enum) Warning
 Warn about all enumerated switches missing a specific case
 
+Wswitch-bool
+C ObjC C++ ObjC++ Warning Init(1)
+Warn about switches with boolean controlling expression
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format)
 ;
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 1d9780e..abd636c 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -5197,9 +5197,13 @@ c_parser_switch_statement (c_parser *parser)
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_SWITCH));
   c_parser_consume_token (parser);
   block = c_begin_compound_stmt (flag_isoc99);
+  bool explicit_cast_p = false;
   if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
 {
   switch_cond_loc = c_parser_peek_token (parser)->location;
+  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
+ && c_token_starts_typename (c_parser_peek_2nd_token (parser)))
+   explicit_cast_p = true;
   ce = c_parser_expression (parser);
   ce = convert_lvalue_to_rvalue (switch_cond_loc, ce, true, false);
   expr = ce.value;
@@ -5217,7 +5221,7 @@ c_parser_switch_statement (c_parser *parser)
   switch_cond_loc = UNKNOWN_LOCATION;
   expr = error_mark_node;
 }
-  c_start_case (switch_loc, switch_cond_loc, expr);
+  c_start_case (switch_loc, switch_cond_loc, expr, explicit_cast_p);
   save_break = c_break_label;
   c_break_label = NULL_TREE;
   body = c_parser_c99_block_statement (parser);
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index e7dcb35..133930f 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -614,7 +614,7 @@ extern void process_init_element (location_t, struct 
c_expr, bool,
  struct obstack *);
 extern tree build_compound_literal (location_t, tree, tree, bool);
 extern void check_compound_literal_type (location_t, struct c_type_name *);
-extern tree c_start_case (location_t, location_t, tree);
+extern tree c_start_case (location_t, location_t, tree, bool);
 extern void c_finish_case (tree);
 extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
 extern tree build_asm_stmt (tree, tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 6ca584b..a98ce07 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9361,12 +9361,13 @@ struct c_switch *c_switch_stack;
 
 /* Start a C switch statement, testing expression EXP.  Return the new
SWITCH_EXPR.  SWITCH_LOC is the location of the `switch'.
-   SWITCH_COND_LOC is the location of the switch's condition.  */
+   SWITCH_COND_LOC is the location of the switch's condition.
+   EXPLICIT_CAST_P is true if the expression EXP has explicit cast.  */
 
 tree
 c_start_case (location_t switch_loc,
  location_t switch_cond_loc,
- tree exp)
+ tree exp, bool explicit_cast_p)
 {
   tree orig_type = error_mark_node;
   struct c_switch *cs;
@@ -9387,6 +9388,19 @@ c_start_case (location_t switch_loc,
   else
{
  tree type = TYPE_MAIN_VARIANT (orig_type);
+ tree e = exp;
+
+ /* Warn if the condition has boolean value.  */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+   e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (type) == BOOLEAN_TYPE
+  || truth_value_p (TREE_CODE (e)))
+ /* Explicit cast to int suppresses this warning.  */
+ && !(TREE_CODE (type) == INTEGER_TYPE
+  && explicit_cast_p))
+   warning_at (switch

[PATCH, PR 61340] Add default label to two switches on enum ipa_ref_use

[Martin Jambor]

Hi,

in PR 61340 it has been reported that clang warns about unhandeld enum
values in a switch (gcc does not, I guess I'll open a new PR for
that).

Fixed thusly by adding a default label with a gcc_unreachable() in
both.  The potentially unhandled enum value in both cases is
IPA_REF_ALIAS which we cannot encounter because

1) in ipa-pure-const.c we are not processing aliases and

2) in ipa-reference.c we analyze references of a function to a
   variable and that cannot be an alias.

OK for trunk?

Thanks,

Martin


2014-06-03  Martin Jambor  

PR ipa/61340
* ipa-pure-const.c (propagate_pure_const): Add unreachable default
handler for switch on an ipa_ref_use enum.
* ipa-reference.c (analyze_function): Likewise.

diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index 0ebfe5d..b9a3d3e 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1286,6 +1286,8 @@ propagate_pure_const (void)
  break;
case IPA_REF_ADDR:
  break;
+   default:
+ gcc_unreachable ();
}
  better_state (&ref_state, &ref_looping,
w_l->state_previously_known,
diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index ebff60c..bc58cfa 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -483,6 +483,8 @@ analyze_function (struct cgraph_node *fn)
  break;
case IPA_REF_ADDR:
  break;
+   default:
+ gcc_unreachable ();
}
 }
 

[C PATCH] Use inform for "shadowed decl" (PR c/48062)

[Marek Polacek]

For "shadowed declaration" note we were calling warning_at, while we
should use inform.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

2014-06-03  Marek Polacek  

PR c/48062
* c-decl.c (warn_if_shadowing): Call inform instead of warning_at.

* gcc.dg/Wshadow-1.c: Use dg-message for "shadowed declaration".
* gcc.dg/Wshadow-3.c: Likewise.

diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index dc8dbc2..05ab20e 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2635,8 +2635,8 @@ warn_if_shadowing (tree new_decl)
  warning (OPT_Wshadow, "declaration of %q+D shadows a previous local",
   new_decl);
 
-   warning_at (DECL_SOURCE_LOCATION (old_decl), OPT_Wshadow,
-   "shadowed declaration is here");
+   inform (DECL_SOURCE_LOCATION (old_decl),
+   "shadowed declaration is here");
 
break;
   }
diff --git gcc/testsuite/gcc.dg/Wshadow-1.c gcc/testsuite/gcc.dg/Wshadow-1.c
index 40073f3..6075711 100644
--- gcc/testsuite/gcc.dg/Wshadow-1.c
+++ gcc/testsuite/gcc.dg/Wshadow-1.c
@@ -5,7 +5,7 @@
 
 /* Source: Neil Booth, 5 Dec 2001.  */
 
-int decl1; /* { dg-warning "shadowed declaration" } */
+int decl1; /* { dg-message "shadowed declaration" } */
 void foo (double decl1)/* { dg-warning "shadows a global decl" 
} */
 {  
 }
@@ -16,7 +16,7 @@ void foo1 (int d) /* { dg-message "note: previous 
definition" } */
   /* { dg-error "redeclared as different" "" { target *-*-* } 15 } */
 }
 
-void foo2 (int d)  /* { dg-warning "shadowed declaration" } */
+void foo2 (int d)  /* { dg-message "shadowed declaration" } */
 {
   {
 double d;  /* { dg-warning "shadows a parameter" } */
@@ -25,7 +25,7 @@ void foo2 (int d) /* { dg-warning "shadowed 
declaration" } */
 
 void foo3 ()
 {
-  int local;   /* { dg-warning "shadowed declaration" } */
+  int local;   /* { dg-message "shadowed declaration" } */
   {
 int local; /* { dg-warning "shadows a previous local" } */
   }
diff --git gcc/testsuite/gcc.dg/Wshadow-3.c gcc/testsuite/gcc.dg/Wshadow-3.c
index a7f06a2..ce2879c 100644
--- gcc/testsuite/gcc.dg/Wshadow-3.c
+++ gcc/testsuite/gcc.dg/Wshadow-3.c
@@ -5,7 +5,7 @@
 /* { dg-do compile } */
 /* { dg-options "-std=gnu89 -Wshadow" } */
 
-int v; /* { dg-warning "shadowed declaration" } */
+int v; /* { dg-message "shadowed declaration" } */
 int f1(int v);
 int f2(int v, int x[v]); /* { dg-warning "declaration of 'v' shadows a global 
declaration" } */
 int f3(int v, int y[sizeof(v)]); /* { dg-warning "declaration of 'v' shadows a 
global declaration" } */
@@ -18,4 +18,4 @@ int f9(x) int x; { return 0; }
 int f10(v) { return 0; } /* { dg-warning "declaration of 'v' shadows a global 
declaration" } */
 int f11(int a, int b(int a));
 int f12(int a, int b(int a, int x[a])); /* { dg-warning "declaration of 'a' 
shadows a parameter" } */
-/* { dg-warning "shadowed declaration" "outer parm" { target *-*-* } 20 } */
+/* { dg-message "shadowed declaration" "outer parm" { target *-*-* } 20 } */

Marek

Re: [C PATCH] Use inform for "shadowed decl" (PR c/48062)

[Jakub Jelinek]

On Tue, Jun 03, 2014 at 06:01:57PM +0200, Marek Polacek wrote:
> For "shadowed declaration" note we were calling warning_at, while we
> should use inform.
> 
> Regtested/bootstrapped on x86_64-linux, ok for trunk?

Shouldn't you remember the return value from warning/warning_at calls
which fall through into this spot and guard the inform call with that?

Jakub

Re: Eliminate write-only variables

[Jan Hubicka]

> On Mon, Jun 2, 2014 at 8:59 PM, Jan Hubicka  wrote:
> >>
> >> Yeah, I discussed this with martin today on irc.  For aliasing we'd like 
> >> to know whether a decl possibly has its address taken. Currently we only 
> >> trust TREE_ADDRESSABLE for statics - and lto might change those to hidden 
> >> visibility public :(
> >>
> >> So we want deck_referred_to_by_single_function
> >
> > OK, I will implement this one in IPA mini-pass.  It is easy.
> > This property changes by clonning and inlining. What Martin wants to use it 
> > for?
> > (I.e. my plan would be to compute it as last IPA pass making it useless for
> > IPA analysis&propagation)
> 
> He doesn't want to use it but we talked and he said he'd have a look.
> 
> Note that it's important the decls are not refered to by global initializers
> either.
> 
> Why is a new pass necessary - can't the IPA reference maintaining code
> update some flag in the varpool magically?

If the code to add/remove reference was updating the flag, it would became out 
of
date as we remove callgraph during the late compilation (we do not keep 
references for
functions we already compiled).
I do not want the flags to be computed before end of IPA queue so they won't 
become
out of date as we clone/inline.

We basically need to walk references and check that they are all functions &
either one given function F or a clones inlined to it.  Assuming that function
does not have unbounded number of references to given variale, this is
basically constant time test.
> 
> >> and deck_may_have_address_taken.
> >
> > We currently clear TREE_ADDRESSABLE for statics that have no address taken 
> > at
> > WPA time and that ought to keep the flag cleared at ltrans (I think I even 
> > made
> > testcase for this)
> >
> > What I think we could improve is to not consider string functions as ADDR 
> > operations
> > for this analysis, i.e. it is common to only memset to 0.
> >
> > How may_have_address_taken would differ here?
> 
> I want tree.h:may_be_aliased to return false if a decl doesn't have its 
> address
> taken.  We don't trust TREE_ADDRESSABLE for TREE_PUBLIC/DECL_EXTERNAL
> decls because other TUs may take the address.  I want the check to be replaced
> with sth more symtab aware, that is, bring in benefits from LTO (and at the 
> same
> time be not confused by statics brought public with hidden visibility).


I see, are you sure you need to ignore TREE_ADDRESSABLE for PUBLIC/EXTERNAL?
I belive frontends are resposible to set them for all data that may have address
taken (even externally) and IPA code maintains it - we clear the flagonly for
variables that are static.

Or we can just set the flag to true for externally visible vars ealry in IPA
queue if this is false.

Honza
> 
> Richard.
> 
> > Honza

Re: [build, driver] RFC: Support compressed debug sections

[Mike Stump]

On Jun 3, 2014, at 3:40 AM, Rainer Orth  wrote:
> It's been another week, and I still need approval for the build, doc,
> and Darwin changes:

So, the darwin bits look trivial enough, if the entire scheme is what people 
want to do.  My question would be, why do we want an option for this?  If the 
scheme works, why not just turn it on unconditionally?  If it doesn’t work, why 
add it?  If it isn’t good, why add it?  If it is good, why not do it?

If it is just to reach compatibility with the debugger, then I’d rather either 
just mandate a certain debugger or autoconf for what the current debugger 
supports.  As of late people seem to just break the debugging experience with 
non-updated gdbs and assume that a newer gdb is used.

Re: [patch,avr] atmel avr new devices set-2

[Denis Chertykov]

2014-06-03 18:26 GMT+04:00 S, Pitchumani :
> Hi,
>
> Attached patch adds support for Atmel devices tiny441, tiny828 and tiny841.
>
> Please commit if the patch is OK.
>
> Note: This is continuation of patch attached in
> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00049.html
>
> Regards,
> Pitchumani
>
> gcc/ChangeLog
> 2014-06-03  Vishnu K S 
>
> * config/avr/avr-mcus.def: Add new avr25 devices attiny441, attiny828
> and attiny841.
> * config/avr/avr-tables.opt: Regenerate.
> * config/avr/t-multilib: Regenerate.
> * doc/avr-mmcu.texi: Regenerate.

Committed.

Re: [patch,avr] atmel avr new devices set-1

[Denis Chertykov]

2014-06-02 13:47 GMT+04:00 S, Pitchumani :
> Hi,
>
> Attached patch adds support for Atmel ATA devices (ata6616c,
> ata6617c, ata664251, ata6612c, ata6613c and ata6614q).
>
> Please commit if the patch is OK. I do not have commit access.
>
> Regards,
> Pitchumani
>
> gcc/ChangeLog
> 2014-06-02  Vishnu K S 
> Pitchumani Sivanupandi 
>
> * config/avr/avr-mcus.def (ata6616c): Add new avr25 device.
> (ata6617c, ata664251): Add new avr35 devices.
>   (ata6612c): Add new avr4 device.
>   (ata6613c, ata6614q): Add new avr5 devices.
> * config/avr/avr-tables.opt: Regenerate.
> * config/avr/t-multilib: Regenerate.
> * doc/avr-mmcu.texi: Regenerate.
>

Committed.

Re: [C++ Patch] PR 52174 aka DR 1423

[Paolo Carlini]

Hi,

On 06/03/2014 05:41 PM, Jason Merrill wrote:

On 06/03/2014 11:24 AM, Paolo Carlini wrote:

+  if (NULLPTR_TYPE_P (from) && (flags & LOOKUP_ONLYCONVERTING))
+conv->bad_p = true;


Thanks.  What kind of error message do we get with this change? Would 
adding something to convert_like_real provide a more helpful diagnostic?

We get the default:

invalid conversion from ‘std::nullptr_t’ to ‘bool’ [-fpermissive]

Maybe something like the below then?!?

Thanks,
Paolo.

/
Index: gcc/cp/call.c
===
--- gcc/cp/call.c   (revision 211162)
+++ gcc/cp/call.c   (working copy)
@@ -1311,10 +1311,10 @@ standard_conversion (tree to, tree from, tree expr
 {
   /* [conv.bool]
 
- An rvalue of arithmetic, unscoped enumeration, pointer, or
- pointer to member type can be converted to an rvalue of type
- bool. ... An rvalue of type std::nullptr_t can be converted
- to an rvalue of type bool;  */
+ A prvalue of arithmetic, unscoped enumeration, pointer, or pointer
+ to member type can be converted to a prvalue of type bool. ...
+ For direct-initialization (8.5 [dcl.init]), a prvalue of type
+ std::nullptr_t can be converted to a prvalue of type bool;  */
   if (ARITHMETIC_TYPE_P (from)
  || UNSCOPED_ENUM_P (from)
  || fcode == POINTER_TYPE
@@ -1328,6 +1328,8 @@ standard_conversion (tree to, tree from, tree expr
  && conv->rank < cr_pbool)
  || NULLPTR_TYPE_P (from))
conv->rank = cr_pbool;
+ if (NULLPTR_TYPE_P (from) && (flags & LOOKUP_ONLYCONVERTING))
+   conv->bad_p = true;
  return conv;
}
 
@@ -6062,6 +6064,14 @@ convert_like_real (conversion *convs, tree expr, t
expr = CONSTRUCTOR_ELT (expr, 0)->value;
}
 
+  /* Give a helpful error if this is bad because a conversion to bool
+from std::nullptr_t requires direct-initialization.  */
+  if (NULLPTR_TYPE_P (TREE_TYPE (expr))
+ && TREE_CODE (totype) == BOOLEAN_TYPE)
+   complained = permerror (loc, "converting to %qT from %qT requires "
+   "direct-initialization",
+   totype, TREE_TYPE (expr));
+
   for (; t ; t = next_conversion (t))
{
  if (t->kind == ck_user && t->cand->reason)
Index: gcc/testsuite/g++.dg/cpp0x/nullptr17.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (revision 211162)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr17.C  (working copy)
@@ -1,6 +1,7 @@
 // { dg-do compile { target c++11 } }
 
-// Test that bool is a better overload match than int
+// Used to test that bool is a better overload match than int
+// Updated for DR 1423
 
 template  struct tType_equal;
 template  struct tType_equal { typedef void type; };
@@ -16,7 +17,7 @@ bool i( bool );
 void test_i()
 {
   // Overload to bool, not int
-  type_equal(i(nullptr));
+  type_equal(i(nullptr));  // { dg-error "direct" }
   decltype(nullptr) mynull = 0;
-  type_equal(i(mynull));
+  type_equal(i(mynull));   // { dg-error "direct" }
 }
Index: gcc/testsuite/g++.dg/cpp0x/nullptr31.C
===
--- gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nullptr31.C  (working copy)
@@ -0,0 +1,11 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+bool b1 = nullptr;  // { dg-error "direct" }
+
+bool b2(nullptr);
+bool b3{nullptr};
+
+int  i1 = nullptr;  // { dg-error "cannot convert" }
+int  i2(nullptr);   // { dg-error "cannot convert" }
+int  i3{nullptr};   // { dg-error "cannot convert" }
Index: gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C
===
--- gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/sfinae-nullptr1.C(working copy)
@@ -0,0 +1,18 @@
+// DR 1423, PR c++/52174
+// { dg-do compile { target c++11 } }
+
+template
+T&& make();
+
+template
+void sink(T);
+
+template(make()))
+>
+auto f(int) -> char(&)[1];
+
+template
+auto f(...) -> char(&)[2];
+
+static_assert(sizeof(f(0)) != 1, "");
Index: libstdc++-v3/testsuite/20_util/is_assignable/value.cc
===
--- libstdc++-v3/testsuite/20_util/is_assignable/value.cc   (revision 
211151)
+++ libstdc++-v3/testsuite/20_util/is_assignable/value.cc   (working copy)
@@ -174,7 +174,7 @@ static_assert(!std::is_assignable::valu
 static_assert(std::is_assignable::value, "Error");
 static_assert(std::is_assignable::value, "Error");
 static_assert(std::is_assignable::value, "Error");
-static_assert(std::is_assignable::value, "Error");
+static_assert(!std::is_assignable::value, "Error");
 
 static_assert(std::is_assignable::v

Re: [C PATCH] Use inform for "shadowed decl" (PR c/48062)

[Marek Polacek]

On Tue, Jun 03, 2014 at 06:07:03PM +0200, Jakub Jelinek wrote:
> On Tue, Jun 03, 2014 at 06:01:57PM +0200, Marek Polacek wrote:
> > For "shadowed declaration" note we were calling warning_at, while we
> > should use inform.
> > 
> > Regtested/bootstrapped on x86_64-linux, ok for trunk?
> 
> Shouldn't you remember the return value from warning/warning_at calls
> which fall through into this spot and guard the inform call with that?

Oops, yea, for the #pragma case.  Fixed + testcase attached.

Ok?

2014-06-03  Marek Polacek  

PR c/48062
* c-decl.c (warn_if_shadowing): Call inform instead of warning_at.
Print note only if the warning was printed.

* gcc.dg/Wshadow-1.c: Use dg-message for "shadowed declaration".
* gcc.dg/Wshadow-3.c: Likewise.
* gcc.dg/pr48062.c: New test.

diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index dc8dbc2..8fb3296 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2601,6 +2601,7 @@ warn_if_shadowing (tree new_decl)
 DECL_SOURCE_LOCATION (b->decl
   {
tree old_decl = b->decl;
+   bool warned = false;
 
if (old_decl == error_mark_node)
  {
@@ -2609,8 +2610,9 @@ warn_if_shadowing (tree new_decl)
break;
  }
else if (TREE_CODE (old_decl) == PARM_DECL)
- warning (OPT_Wshadow, "declaration of %q+D shadows a parameter",
-  new_decl);
+ warned = warning (OPT_Wshadow,
+   "declaration of %q+D shadows a parameter",
+   new_decl);
else if (DECL_FILE_SCOPE_P (old_decl))
  {
/* Do not warn if a variable shadows a function, unless
@@ -2620,9 +2622,10 @@ warn_if_shadowing (tree new_decl)
&& !FUNCTION_POINTER_TYPE_P (TREE_TYPE (new_decl)))
continue;
 
-   warning_at (DECL_SOURCE_LOCATION (new_decl), OPT_Wshadow, 
-   "declaration of %qD shadows a global declaration",
-   new_decl);
+   warned = warning_at (DECL_SOURCE_LOCATION (new_decl), OPT_Wshadow,
+"declaration of %qD shadows a global "
+"declaration",
+new_decl);
  }
else if (TREE_CODE (old_decl) == FUNCTION_DECL
 && DECL_BUILT_IN (old_decl))
@@ -2632,11 +2635,12 @@ warn_if_shadowing (tree new_decl)
break;
  }
else
- warning (OPT_Wshadow, "declaration of %q+D shadows a previous local",
-  new_decl);
+ warned = warning (OPT_Wshadow, "declaration of %q+D shadows a "
+   "previous local", new_decl);
 
-   warning_at (DECL_SOURCE_LOCATION (old_decl), OPT_Wshadow,
-   "shadowed declaration is here");
+   if (warned)
+ inform (DECL_SOURCE_LOCATION (old_decl),
+ "shadowed declaration is here");
 
break;
   }
diff --git gcc/testsuite/gcc.dg/Wshadow-1.c gcc/testsuite/gcc.dg/Wshadow-1.c
index 40073f3..6075711 100644
--- gcc/testsuite/gcc.dg/Wshadow-1.c
+++ gcc/testsuite/gcc.dg/Wshadow-1.c
@@ -5,7 +5,7 @@
 
 /* Source: Neil Booth, 5 Dec 2001.  */
 
-int decl1; /* { dg-warning "shadowed declaration" } */
+int decl1; /* { dg-message "shadowed declaration" } */
 void foo (double decl1)/* { dg-warning "shadows a global decl" 
} */
 {  
 }
@@ -16,7 +16,7 @@ void foo1 (int d) /* { dg-message "note: previous 
definition" } */
   /* { dg-error "redeclared as different" "" { target *-*-* } 15 } */
 }
 
-void foo2 (int d)  /* { dg-warning "shadowed declaration" } */
+void foo2 (int d)  /* { dg-message "shadowed declaration" } */
 {
   {
 double d;  /* { dg-warning "shadows a parameter" } */
@@ -25,7 +25,7 @@ void foo2 (int d) /* { dg-warning "shadowed 
declaration" } */
 
 void foo3 ()
 {
-  int local;   /* { dg-warning "shadowed declaration" } */
+  int local;   /* { dg-message "shadowed declaration" } */
   {
 int local; /* { dg-warning "shadows a previous local" } */
   }
diff --git gcc/testsuite/gcc.dg/Wshadow-3.c gcc/testsuite/gcc.dg/Wshadow-3.c
index a7f06a2..ce2879c 100644
--- gcc/testsuite/gcc.dg/Wshadow-3.c
+++ gcc/testsuite/gcc.dg/Wshadow-3.c
@@ -5,7 +5,7 @@
 /* { dg-do compile } */
 /* { dg-options "-std=gnu89 -Wshadow" } */
 
-int v; /* { dg-warning "shadowed declaration" } */
+int v; /* { dg-message "shadowed declaration" } */
 int f1(int v);
 int f2(int v, int x[v]); /* { dg-warning "declaration of 'v' shadows a global 
declaration" } */
 int f3(int v, int y[sizeof(v)]); /* { dg-warning "declaration of 'v' shadows a 
global declaration" } */
@@ -18,4 +18,4 @@ int f9(x) int x; { return 0; }
 int f10(v) { return 0; } /* { dg-warning "declaration 

Re: [PATCH][AARCH64]Support full addressing modes for ldr/str in vectorization scenarios

[Charles Baylis]

On 3 June 2014 12:08, Marcus Shawcroft  wrote:
> On 28 May 2014 08:30, Bin.Cheng  wrote:
>>> So is it OK?
>>>
>>>
>>> 2014-05-28  Bin Cheng  
>>>
>>> * config/aarch64/aarch64.c (aarch64_classify_address)
>>> (aarch64_legitimize_reload_address): Support full addressing modes
>>> for vector modes.
>>> * config/aarch64/aarch64.md (mov, movmisalign)
>>> (*aarch64_simd_mov, *aarch64_simd_mov): Relax
>>> predicates.
>
> OK Thanks /Marcus

Hi Bin,

This resolves an ICE in 4.9 in Neon intrinsics code, so I'd like to
see it backported to the branch too, please.

Thanks
Charles

Re: [C PATCH] Warn if switch has boolean value (PR c/60439)

[Jason Merrill]

OK.

Jason

Re: [C++ Patch] PR 52174 aka DR 1423

[Jason Merrill]

OK, thanks.

Jason

Re: [PATCH][AARCH64]Support full addressing modes for ldr/str in vectorization scenarios

[Marcus Shawcroft]

> On 3 Jun 2014, at 18:08, Charles Baylis  wrote:
> 
>> On 3 June 2014 12:08, Marcus Shawcroft  wrote:
>> On 28 May 2014 08:30, Bin.Cheng  wrote:
 So is it OK?
 
 
 2014-05-28  Bin Cheng  
 
* config/aarch64/aarch64.c (aarch64_classify_address)
(aarch64_legitimize_reload_address): Support full addressing modes
for vector modes.
* config/aarch64/aarch64.md (mov, movmisalign)
(*aarch64_simd_mov, *aarch64_simd_mov): Relax
 predicates.
>> 
>> OK Thanks /Marcus
> 
> Hi Bin,
> 
> This resolves an ICE in 4.9 in Neon intrinsics code, so I'd like to
> see it backported to the branch too, please.
> 
> Thanks
> Charles

Charles,  Have you got a PR/bugzilla no for the ICE in question please?

Re: PR61385: phiopt drops some PHIs

[Jeff Law]

On 06/03/14 08:08, Richard Biener wrote:

On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse  wrote:

Hello,

apparently it is possible to have a PHI in the middle basic block of
value_replacement, so I need to move it as well when I move the statement
and remove the block.

Bootstrap and testsuite on x86_64-linux-gnu (re-running for various reasons
but they had completed successfully yesterday).

2014-06-03  Marc Glisse  

 PR tree-optimization/61385
gcc/
 * tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
 removing the basic block.
gcc/testsuite/
 * gcc.dg/tree-ssa/pr61385.c: New file.

--
Marc Glisse
Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+  int *h = &c;
+  for (; c < 1; c++)
+{
+  int *i = &a, *k = &a;
+  f = 0;
+  if (b)
+   return 0;
+  if (*h)
+   {
+ int **j = &i;
+ *j = 0;
+ d = 0;
+   }
+  else
+   g = e = 0;
+  if (*h)
+   {
+ int **l = &k;
+ *l = &g;
+   }
+  d &= *h;
+  assert (k == &a || k);
+  assert (i);
+}
+  return 0;
+}
+
+int
+main ()
+{
+  fn1 ();
+  return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c   (revision 211178)
+++ gcc/tree-ssa-phiopt.c   (working copy)
@@ -877,20 +877,39 @@ value_replacement (basic_block cond_bb,
&& operand_equal_for_phi_arg_p (rhs2, cond_lhs)
&& neutral_element_p (code_def, cond_rhs, true))
   || (arg1 == rhs2
   && operand_equal_for_phi_arg_p (rhs1, cond_lhs)
   && neutral_element_p (code_def, cond_rhs, false))
   || (operand_equal_for_phi_arg_p (arg1, cond_rhs)
   && (operand_equal_for_phi_arg_p (rhs2, cond_lhs)
   || operand_equal_for_phi_arg_p (rhs1, cond_lhs))
   && absorbing_element_p (code_def, cond_rhs
  {
+  /* Move potential PHI nodes.  */
+  gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+  while (!gsi_end_p (psi))
+   {
+ gimple phi_moving = gsi_stmt (psi);
+ gimple newphi = create_phi_node (gimple_phi_result (phi_moving),
+  cond_bb);
+ int nargs = cond_bb->preds->length();
+ location_t loc = gimple_phi_arg_location (phi_moving, 0);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ for (int i = 0; i < nargs; ++i)
+   {
+ edge e = (*cond_bb->preds)[i];
+ add_phi_arg (newphi, phi_arg, e, loc);


All arguments get the same value (and the PHI in middle-bb is surely
a singleton?), so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block.  If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.

We certainly want to get rid of them :-)

I'd start by finding out which pass left the degenerate, ensure it's not 
marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away.


If it's a systemic problem that a particular pass can leave degenerate 
PHIs, then you might want to schedule the phi-only propagator to run 
after that pass.  It does const/copy propagation seeding from degenerate 
PHIs, so it ought to be very fast.


jeff

[PATCH, testsuite]: Fix g++.dg/ext/mv[14,15].C spurious failure on corei7

[Uros Bizjak]

Hello!

When configured with "--with-arch=core-avx-i --with-cpu=core-avx-i",
g++.dg/ext/mv[14,15].C tests fail on corei7 [1] since the default CPU
is the same as the checked cpu in the test. The patch compiles the
testcase with -march=x86-64 as the generic CPU

2014-06-03  Uros Bizjak  

* g++.dg/ext/mv14.C (dg-options): Add -march=x86-64.
* g++.dg/ext/mv15.C (dg-options): Ditto.

Tested on x86_64-pc-linux-gnu {,-m32} corei7 CPU and committed to mainline SVN.

[1] https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg00243.html

Uros.
Index: g++.dg/ext/mv14.C
===
--- g++.dg/ext/mv14.C   (revision 211188)
+++ g++.dg/ext/mv14.C   (working copy)
@@ -1,7 +1,7 @@
 /* Test case to check if Multiversioning works.  */
 /* { dg-do run { target i?86-*-* x86_64-*-* } } */
 /* { dg-require-ifunc "" }  */
-/* { dg-options "-O2 -fPIC" } */
+/* { dg-options "-O2 -fPIC -march=x86-64" } */
 
 #include 
 
Index: g++.dg/ext/mv15.C
===
--- g++.dg/ext/mv15.C   (revision 211188)
+++ g++.dg/ext/mv15.C   (working copy)
@@ -1,7 +1,7 @@
 /* Test case to check if Multiversioning works.  */
 /* { dg-do run { target i?86-*-* x86_64-*-* } } */
 /* { dg-require-ifunc "" }  */
-/* { dg-options "-O2 -fPIC" } */
+/* { dg-options "-O2 -fPIC -march=x86-64" } */
 
 #include 
 

[patch i386]: Prevent to assume for 64-bit ms-abi that DX_REG is used as function-value

[Kai Tietz]

Hello,

this patch fixes a nit detected in ix86_function_value_regno_p.  For
x64 ms-abi we don't have DX_REG-register as valid function-value
register.

ChangeLog

2014-06-03  Kai Tietz  

* config/i386/i386.c (ix86_function_value_regno_p): Disallow DX_REG for
64-bit ms-abi.

Tested for x86_64-w64-mingw32, and x86_64-unknown-linux-gnu.  If there
are no objections I will commit that patch tomorrow.

Regards,
Kai

Index: i386.c
===
--- i386.c  (Revision 211198)
+++ i386.c  (Arbeitskopie)
@@ -7775,7 +7775,7 @@ ix86_function_value_regno_p (const unsigned int r
 {
 case AX_REG:
 case DX_REG:
-  return true;
+  return (regno != DX_REG || !TARGET_64BIT || ix86_abi != MS_ABI);
 case DI_REG:
 case SI_REG:
   return TARGET_64BIT && ix86_abi != MS_ABI;

Re: [MIPS] Add sbasic supoert ffor MSA (SIMD)

[Richard Sandiford]

Matthew Fortune  writes:
> Mike Stump  writes:
>> On May 28, 2014, at 7:27 AM, Richard Earnshaw  wrote:
>> >
>> > Speed of implementation.  We're gradually replacing these with proper
>> > builtins, but that takes a lot more work.
>> 
>> As an owner of a port with more builtins that yours, I can offer a
>> technological solution to reduce the cost of builtins to:
>> 
>> (define_builtin "my_stop"
>>   [
>> (define_outputs [(void_operand 0)])
>> (define_rtl_pattern "my_stop" [])
>>   ]
>> )
>> 
>> (define_insn "my_stop"
>>   [(unspec_volatile [(const_int 0)]
>> UNSPECV_STOP)]
>>   ""
>>   "stop")
>> 
>> for example.  This creates the builtins, allows overloading, allows
>> input/output parameters, can reorder operands, allows for complex types,
>> allows memory reference parameters, allows pure markings, does vectors,
>> conditional availability, generates documentation, creates test suites and
>> more.  If you wire up a speaker it even sings.
>> 
>> Someone would have have to step forward with a need and some time to port
>> their port over to the new scheme and help with the reason for why the
>> technology should go in.  It is mostly contained in 5600 lines of self
>> contained python code, and is built to solve the problem generally.  It adds
>> about 800 lines to builtins.c.  It has a macro system that is more powerful
>> than the macro system .md files use, so one gets to share and collapse
>> builtins rather nicely.  It is known to work for C and C++.  Other languages
>> may need extending; C for example cost is around 250 lines to support.
>
> Myself and others at IMG would be interested in reviewing/evaluating the
> implementation and assuming it looks useful then we would of course help to
> get it in shape for submission.
>  
>> One promise, you will never have to create an argument list, or a type, for
>> example here is a two output, type input functional instruction with some
>> doc content:
>> 
>> (define_mode_iterator MYTYPE
>> [V8QI V4HI V2SI DI ...])
>> 
>> (define_builtin "my_foo" "my_foo2_"
>>   [
>> (define_desc"Doc string for operation")
>> (define_outputs [(var_operand:T_MYTYPE 0)
>>  (var_operand:T_MYTYPE 1)])
>> (define_inputs  [(var_operand:T_MYTYPE 2)
>>  (var_operand:T_MYTYPE 3)])
>> (define_rtl_pattern "my_foo2_" [0 2 1 3])
>> (attributes [pure])
>>   ]
>> )
>> 
>> I stripped it so you can't know what the instruction was, but you get a
>> flavor of multiple outputs, doc bits, pure, overloading, arguments and
>> argument rearranging.
>
> Can you post the implementation as an RFC? I suspect the python aspect
> will cause the most trouble as GCC builds do not currently require python
> I guess that could change depending on the value added. Otherwise it would
> be a rewrite I guess.
>
> Before digging in too deep though it would be useful to know if RichardS
> would be willing to consider this kind of thing for the MIPS port?

Yeah, it definitely sounds good in principle.

Richard

Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os

[Richard Sandiford]

Sandra Loosemore  writes:
> Catherine included an earlier version of this patch with the microMIPS 
> submission a couple years ago:
>
> https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00972.html
>
> Richard's response was:
>
>> Looks like the wrong place to do this.  Please treat this as a separate
>> patch and get a tree expert to comment.
>
> So, here is the separate patch, finally.  :-)  Can a tree expert 
> comment?  I dug around and didn't see another obvious hook to set 
> function alignment in a target-specific way depending on attributes of 
> the function.
>
> Besides the new test cases, I regression-tested this on mips-sde-elf 
> using Mentor's usual assortment of multilibs, specifically including one 
> for microMIPS.

OK, I asked Richi on IRC today.  To recap, one of the things I was
worried about was a test against the address, like (foo & 2) == 0,
being optimised away before we set the alignment.  Richi pointed
out that my idea downthread about cgraph_create_node would also be
too late to avoid that.  Also, looking at it now, I see that we don't
trust DECL_ALIGN on functions anyway.  From get_object_alignment_2:

  /* Extract alignment information from the innermost object and
 possibly adjust bitpos and offset.  */
  if (TREE_CODE (exp) == FUNCTION_DECL)
{
  /* Function addresses can encode extra information besides their
 alignment.  However, if TARGET_PTRMEMFUNC_VBIT_LOCATION
 allows the low bit to be used as a virtual bit, we know
 that the address itself must be at least 2-byte aligned.  */
  if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn)
align = 2 * BITS_PER_UNIT;
}

And since we use the low bit to encode the ISA mode on MIPS, the upshot
is that we never assume any alignment for functions.  So there's nothing
to worry about after all.

Richi suggested just changing the alignment at output time.  I assume
that would be a case of replacing the DECL_ALIGN in:

  /* Tell assembler to move to target machine's alignment for functions.  */
  align = floor_log2 (DECL_ALIGN (decl) / BITS_PER_UNIT);
  if (align > 0)
{
  ASM_OUTPUT_ALIGN (asm_out_file, align);
}

with a hook.  (Is that right?)

Thanks,
Richard

Re: [PATCH, PR 61340] Add default label to two switches on enum ipa_ref_use

[Jan Hubicka]

> Hi,
> 
> in PR 61340 it has been reported that clang warns about unhandeld enum
> values in a switch (gcc does not, I guess I'll open a new PR for
> that).
> 
> Fixed thusly by adding a default label with a gcc_unreachable() in
> both.  The potentially unhandled enum value in both cases is
> IPA_REF_ALIAS which we cannot encounter because
> 
> 1) in ipa-pure-const.c we are not processing aliases and
> 
> 2) in ipa-reference.c we analyze references of a function to a
>variable and that cannot be an alias.
> 
> OK for trunk?

OK,
thanks!
Honza
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-06-03  Martin Jambor  
> 
>   PR ipa/61340
>   * ipa-pure-const.c (propagate_pure_const): Add unreachable default
>   handler for switch on an ipa_ref_use enum.
>   * ipa-reference.c (analyze_function): Likewise.
> 
> diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
> index 0ebfe5d..b9a3d3e 100644
> --- a/gcc/ipa-pure-const.c
> +++ b/gcc/ipa-pure-const.c
> @@ -1286,6 +1286,8 @@ propagate_pure_const (void)
> break;
>   case IPA_REF_ADDR:
> break;
> + default:
> +   gcc_unreachable ();
>   }
> better_state (&ref_state, &ref_looping,
>   w_l->state_previously_known,
> diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
> index ebff60c..bc58cfa 100644
> --- a/gcc/ipa-reference.c
> +++ b/gcc/ipa-reference.c
> @@ -483,6 +483,8 @@ analyze_function (struct cgraph_node *fn)
> break;
>   case IPA_REF_ADDR:
> break;
> + default:
> +   gcc_unreachable ();
>   }
>  }
>  
> 

Re: Fix a function decl in gfortran

[Bernd Schmidt]

On 05/27/2014 04:01 PM, Tobias Burnus wrote:

Bernd Schmidt wrote:

Compiling Fortran code with the ptx backend I'm working on results in
assembler warnings about mismatch between function calls and function decls.

Bootstrapped and tested on x86_64-linux. Ok?


OK.

The change/bug is due to my fortran-caf -> trunk patch at
https://gcc.gnu.org/ml/fortran/2014-04/msg00091.html,
which seemingly missed the function declaration when updating the calls and
the library.


There's at least one more such problem with gfortran, which I'm having a 
harder time debugging. It occurs in 
gfortran.fortran-torture/compile/mloc.f90 for the library function 
mminloc0_4_r4. We have a decl that shows four arguments:


(gdb) p debug_tree (type)
 >
SI
size  constant 32>
unit size  constant 4>
align 32 symtab 0 alias set -1 canonical type
attributes 
value >>
arg-types 
unsigned DI
size 
unit size 
align 64 symtab 0 alias set -1 canonical type>
chain 
chain 
chain 
chain >>

while the actual library function (and the call to it) only has three 
arguments:

void
mminloc0_4_r4 (gfc_array_i4 * const restrict retarray,
gfc_array_r4 * const restrict array,
gfc_array_l1 * const restrict mask)

The decl is constructed by gfc_get_function_type. It seems to think 
there's one extra parameter named "dim" between "array" and "mask". 
Another clue might be the following printout:


(gdb) p gfc_debug_expr(gfc_expr*) (expr)
_gfortran_mminloc0_4_r4[[((MAIN__:b(FULL)) ((arg not-present)) 
(__convert_l4_l1[[(((< MAIN__:b(FULL) )))]]))]]


which says "arg not-present", presumably for this "dim" argument.

From there I'm unable to determine what's wrong due to lack of Fortran 
knowledge - any ideas?



Bernd

[patch i386]: Fix PR/46219 Generate indirect jump instruction

[Kai Tietz]

Hello,

This patch fixes PR/46219 by introducing special peephole-optimization.  As we 
can't set for new statement in peephole2-define SIBLING_CALL_P easily, I use 
UNSPEC_PEEPSIB to do indentify sibling tail-call-case.

For avoiding modification of ix86_output_call_insn, I set SIBLING_CALL_P 
directly before outputing it.  If it is preferred we can modify here instead 
ix86_output_call_insn to allow forcing to output sibcall on demand.

ChangeLog

2014-06-03  Kai Tietz  

PR target/46219
* config/i386/i386.md (UNSPEC_PEEPSIB): New unspec.
(sibcall_intern): New define-insn to handle UNSPEC_PEEPSIB.
(sibcall_pop_intern): Likewise.
(sibcall_value_intern): Likewise.
(sibcall_value_pop_intern): Likewise.
(define_peephole2): Simple combine for sibling-tail-call.


ChangeLog

2014-06-03  Kai Tietz  

PR target/46219
* gcc.target/i386/sibcall-4.c: Remove xfail.

Tested patch for x86_64-unknown-linux-gnu, and i686-pc-cygwin.  Ok for apply?

Regards,
Kai


Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 211198)
+++ config/i386/i386.md (working copy)
@@ -111,6 +111,7 @@
   UNSPEC_LEA_ADDR
   UNSPEC_XBEGIN_ABORT
   UNSPEC_STOS
+  UNSPEC_PEEPSIB
 
   ;; For SSE/MMX support:
   UNSPEC_FIX_NOTRUNC
@@ -11382,6 +11383,54 @@
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
+(define_insn "*sibcall_intern"
+  [(call (unspec [(mem:QI (match_operand:W 0 "memory_operand"))] 
UNSPEC_PEEPSIB)
+(match_operand 1))]
+  ""
+  "* SIBLING_CALL_P (insn) = 1; return ix86_output_call_insn (insn, 
operands[0]);"
+  [(set_attr "type" "call")])
+
+; TODO
+(define_peephole2
+  [(set (match_operand:DI 0 "register_operand")
+(match_operand:DI 1 "memory_operand"))
+   (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)]
+  "TARGET_64BIT"
+  [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
+   (set (match_dup 0)
+(match_dup 1))])
+
+(define_peephole2
+  [(set (match_operand:SI 0 "register_operand")
+(match_operand:SI 1 "memory_operand"))
+   (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)]
+  "!TARGET_64BIT"
+  [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
+   (set (match_dup 0)
+(match_dup 1))])
+
+(define_peephole2
+  [(set (match_operand:DI 0 "register_operand") 
+(match_operand:DI 1 "memory_operand"))
+   (call (mem:QI (match_operand:DI 2 "register_operand"))
+ (match_operand 3))]
+  "TARGET_64BIT  && REG_P (operands[0])
+&& REG_P (operands[2])
+&& SIBLING_CALL_P (peep2_next_insn (1))
+&& REGNO (operands[0]) == REGNO (operands[2])"
+  [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))])
+
+(define_peephole2
+  [(set (match_operand:SI 0 "register_operand") 
+(match_operand:SI 1 "memory_operand"))
+   (call (mem:QI (match_operand:SI 2 "register_operand"))
+ (match_operand 3))]
+  "!TARGET_64BIT  && REG_P (operands[0])
+&& REG_P (operands[2])
+&& SIBLING_CALL_P (peep2_next_insn (1))
+&& REGNO (operands[0]) == REGNO (operands[2])"
+  [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))])
+
 (define_expand "call_pop"
   [(parallel [(call (match_operand:QI 0)
(match_operand:SI 1))
@@ -11415,6 +11464,16 @@
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
+(define_insn "*sibcall_pop_intern"
+  [(call (unspec [(mem:QI (match_operand:SI 0 "memory_operand"))] 
UNSPEC_PEEPSIB)
+(match_operand 1))
+   (set (reg:SI SP_REG)
+   (plus:SI (reg:SI SP_REG)
+(match_operand:SI 2 "immediate_operand" "i")))]
+  "!TARGET_64BIT"
+  "* SIBLING_CALL_P (insn) = 1; return ix86_output_call_insn (insn, 
operands[0]);"
+  [(set_attr "type" "call")])
+
 ;; Call subroutine, returning value in operand 0
 
 (define_expand "call_value"
@@ -11457,6 +11516,14 @@
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
+(define_insn "*sibcall_value_intern"
+  [(set (match_operand 0)
+   (call (unspec [(mem:QI (match_operand:W 1 "memory_operand"))] 
UNSPEC_PEEPSIB)
+ (match_operand 2)))]
+  ""
+  "* SIBLING_CALL_P (insn) = 1; return ix86_output_call_insn (insn, 
operands[1]);"
+  [(set_attr "type" "callv")])
+
 (define_insn "*call_value_rex64_ms_sysv"
   [(match_parallel 3 "call_rex64_ms_sysv_operation"
 [(set (match_operand 0)
@@ -11503,6 +11570,17 @@
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
+(define_insn "*sibcall_value_pop_intern"
+  [(set (match_operand 0)
+(call (unspec [(mem:QI (match_operand:SI 1 "memory_operand"))] 
UNSPEC_PEEPSIB)
+ (match_operand 2)))
+   (set (reg:SI SP_REG)
+   (plus:SI (reg:SI SP_REG)
+(match_operand:SI 3 "immediate_operand" "i")))]
+  "!TARGET_64BIT"
+  "* SIBLING_CALL_P (insn) = 1; return 

Re: [patch i386]: Prevent to assume for 64-bit ms-abi that DX_REG is used as function-value

[Richard Henderson]

On 06/03/2014 11:21 AM, Kai Tietz wrote:
>  case AX_REG:
>  case DX_REG:
> -  return true;
> +  return (regno != DX_REG || !TARGET_64BIT || ix86_abi != MS_ABI);

You might as well eliminate the first test, and split the case entries:

  case AX_REG:
return true;
  case DX_REG:
return !TARGET_64BIT || ix86_abi != MS_ABI;

Otherwise it looks ok.


r~