[PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener

I am testing the following patch which fixes PR69771 where the code
doesn't match the comment before it.  We get to expand a QImode << QImode
shift but the shift amount was of type int and thus it was expanded
as SImode constant.  Then

  /* In case the insn wants input operands in modes different from
 those of the actual operands, convert the operands.  It would
 seem that we don't need to convert CONST_INTs, but we do, so
 that they're properly zero-extended, sign-extended or truncated
 for their mode.  */

has to apply as we need to re-extend the VOIDmode CONST_INT for
QImode.  But then mode1 is computed as 'mode' (QImode) which happens
to match what is expected even though the constant isn't valid.

The fix is IMHO to always call convert_modes for VOIDmode ops
(if the target doesn't expect VOIDmode itself).

Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

Thanks,
Richard.

2016-02-12  Richard Biener  

PR rtl-optimization/69771
* optabs.c (expand_binop_directly): Properly zero-/sign-extend
VOIDmode operands.

* gcc.dg/torture/pr69771.c: New testcase.

Index: gcc/optabs.c
===
--- gcc/optabs.c(revision 233369)
+++ gcc/optabs.c(working copy)
@@ -1013,15 +1013,15 @@ expand_binop_directly (machine_mode mode
  that they're properly zero-extended, sign-extended or truncated
  for their mode.  */
 
-  mode0 = GET_MODE (xop0) != VOIDmode ? GET_MODE (xop0) : mode;
-  if (xmode0 != VOIDmode && xmode0 != mode0)
+  mode0 = GET_MODE (xop0);
+  if (xmode0 != mode0)
 {
   xop0 = convert_modes (xmode0, mode0, xop0, unsignedp);
   mode0 = xmode0;
 }
 
-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
-  if (xmode1 != VOIDmode && xmode1 != mode1)
+  mode1 = GET_MODE (xop1);
+  if (xmode1 != mode1)
 {
   xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
   mode1 = xmode1;
Index: gcc/testsuite/gcc.dg/torture/pr69771.c
===
--- gcc/testsuite/gcc.dg/torture/pr69771.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr69771.c  (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+
+unsigned char a = 5, c;
+unsigned short b = 0;
+unsigned d = 0x76543210;
+void __attribute__((noinline))
+fn1() { c = d >> ~(a || ~b); } /* { dg-warning "shift count is negative" } */
+
+int main()
+{
+  fn1();
+  if (c != 1)
+__builtin_abort ();
+  return 0;
+}


Re: [patch, Fortran] Fix PR 60526, variable name has already been declared as a type

2016-02-12 Thread Andre Vehreschild
Hi all,

it wasn't my intention to start such a big discussion for such a small
thing. Please stop this academic battle and get back to the issue at
hand:

Thomas is examining a symbol, that

- has come through the scanner, i.e., it adheres to the rules of
  gfortran, especially it is known to be smaller than GFC_MAX_SYMBOL_LEN

- is still a valid symbol. When gfortran's internal
  composition/modification of symbols breaks this rule, we are done
  earlier already. 

- its length is not changed in his routine.

I was only trying to prevent an alloc/free for this small use case,
which should fit on the stack quite easily. The proposed alloca() call
has according to the documentation of libc some availability issues on
certain platforms, too. Therefore why not going with the fixed size
stack array and adding a check for possible overflow to it and be done?

Regards,
Andre

On Fri, 12 Feb 2016 09:02:27 +0100
Thomas Koenig  wrote:

> Hi Janne,
> 
> [std::string]
> 
> > As we're supposed to be C++ nowadays, why aren't we
> > using that?  
> 
> My reason is simple: I know C and Fortran. I hardly know C++,
> and I don't want to learn it for the sake of some largely
> irrelevant micro-optimizations.
> 
> We should restrict C++ usage to those cases where there is
> a clear and large benefit.
> 
> Regards
> 
>   Thomas


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 


Re: [patch, Fortran] Fix PR 60526, variable name has already been declared as a type

2016-02-12 Thread Thomas Koenig

Hi Janne,

[std::string]


As we're supposed to be C++ nowadays, why aren't we
using that?


My reason is simple: I know C and Fortran. I hardly know C++,
and I don't want to learn it for the sake of some largely
irrelevant micro-optimizations.

We should restrict C++ usage to those cases where there is
a clear and large benefit.

Regards

Thomas


Re: [PATCH, reload] PRE_INC with invalid hard reg

2016-02-12 Thread Jeff Law

On 02/11/2016 08:27 PM, Alan Modra wrote:

On Thu, Feb 11, 2016 at 03:29:05PM +0100, Bernd Schmidt wrote:

On 02/11/2016 10:45 AM, Alan Modra wrote:


Due to uses elsewhere in vsx instructions, reload chooses to put
psuedo 185 in fr31, which can't be used as a base register in the
following:


What code exactly makes the choice of fr31? I assume this is in
reg_renumber, so it's IRA and not reload that's making that decision?


Yes, sorry, I shouldn't have said reload chooses the reg.


PR target/68973
* reloads.c (find_reloads_address_1): For pre/post-inc/dec
with an invalid hard reg, reload just the reg not the entire
pre/post-inc/dec address expression.


Hmm, you're patching tricky code.


Thanks for being willing to review!  :)

It's so amazing having Bernd helping out with this stuff.




I'm not sure yet whether this is right or
not. More reload dumps might help if you have them; I'll Cc myself on the
PR.


I've attached rtl dumps to the PR.


My gut feeling is that we want to reload the inner reg before entering this
block of code,


Yes, my first quick hack did just that, then I noticed there was
existing code to reload the inner reg..
I think both approaches can sensibly work.  FWIW, there's other ports 
where this could be a problem.  The PA for example often wants to hold 
integer values in FP regs (due to the integer multiply instruction 
running in the FP unit).  And the PA has auto-inc addressing.  There's 
probably others with those properties.


I suspect the same handling is needed in the PRE_MODIFY/POST_MODIFY case 
immediately before the auto-inc cases.  One could argue that handling 
ought to be generalized and factor'd out rather than (largely) 
duplicated for the cases.


I'll let Bernd own the final review iteration(s) on this issue.
jeff


Re: [PATCH] PR driver/69265: improved suggestions for various misspelled options

2016-02-12 Thread Jeff Law

On 02/11/2016 08:43 AM, David Malcolm wrote:

On Thu, 2016-02-11 at 10:16 +0100, Richard Biener wrote:

On Wed, Feb 10, 2016 at 5:25 PM, Bernd Schmidt 
wrote:

On 02/09/2016 09:44 PM, David Malcolm wrote:


This is a bug in a new feature, so it isn't a regression as such,
but
it's fairly visible, and I believe the fix is relatively low-risk
(error-handling of typos of command-line options).

This also now covers PR driver/69453 (and its duplicate PR
driver/69642), so people *are* running into this.



I think the patch looks reasonable (I expect it needs slight
adjustment
after an earlier sanitizer options change). Whether it's OK or not
at this
stage is something I think I'll want to ask a RM. My inclination
would be
yes.


Yes.


Thanks.  Were you approving the idea of fixing this bug in stage 4, or
approving the patch itself?   Note that the patch needed some changes
to apply against trunk; the latest version is at:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00724.html
I think the combination of Bernd's comments and Richi's "Yes" is enough 
to consider this approved for the trunk.


jeff



Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote:
> 
> I am testing the following patch which fixes PR69771 where the code
> doesn't match the comment before it.  We get to expand a QImode << QImode
> shift but the shift amount was of type int and thus it was expanded
> as SImode constant.  Then
> 
>   /* In case the insn wants input operands in modes different from
>  those of the actual operands, convert the operands.  It would
>  seem that we don't need to convert CONST_INTs, but we do, so
>  that they're properly zero-extended, sign-extended or truncated
>  for their mode.  */
> 
> has to apply as we need to re-extend the VOIDmode CONST_INT for
> QImode.  But then mode1 is computed as 'mode' (QImode) which happens
> to match what is expected even though the constant isn't valid.
> 
> The fix is IMHO to always call convert_modes for VOIDmode ops
> (if the target doesn't expect VOIDmode itself).
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

This looks like the PR69764 fix I've sent last night (and updated patch
this morning).
BTW, I wouldn't use a runtime test with clearly undefined behavior,
especially not if it tests what the outcome of that UB is.

> 2016-02-12  Richard Biener  
> 
>   PR rtl-optimization/69771
>   * optabs.c (expand_binop_directly): Properly zero-/sign-extend
>   VOIDmode operands.
> 
>   * gcc.dg/torture/pr69771.c: New testcase.

Jakub


Re: [PR66726] Fixe regression caused by Factor conversion out of COND_EXPR

2016-02-12 Thread Eric Botcazou
> This causes LTO/PGO bootstrap on ppc64le to ICE all over the place:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781

And a comparison failure with -enable-checking=yes,rtl on x86-64/Linux:

Comparing stages 2 and 3
warning: gcc/cc1plus-checksum.o differs
warning: gcc/cc1-checksum.o differs
Bootstrap comparison failure!
gcc/cp/typeck2.o differs
gcc/cp/decl.o differs
gcc/cp/mangle.o differs
gcc/sched-deps.o differs
gcc/cselib.o differs
gcc/cfgexpand.o differs
gcc/ipa-icf-gimple.o differs
gcc/gcse.o differs
gcc/tree-ssa-alias.o differs
gcc/postreload-gcse.o differs
gcc/reginfo.o differs
gcc/sel-sched.o differs
gcc/build/genmatch.o differs
gcc/build/genrecog.o differs
gcc/gimplify.o differs
gcc/tree-inline.o differs
gcc/optabs.o differs
gcc/emit-rtl.o differs
gcc/dwarf2out.o differs
gcc/tree-cfg.o differs
gcc/tree-eh.o differs
gcc/tree-predcom.o differs
gcc/tree-chrec.o differs
gcc/opts-common.o differs
gcc/tree-ssa-phiopt.o differs
gcc/asan.o differs
gcc/var-tracking.o differs
gcc/ipa-prop.o differs
gcc/diagnostic-show-locus.o differs
gcc/predict.o differs
gcc/tree-vectorizer.o differs
gcc/auto-profile.o differs
gcc/libgcov-driver-tool.o differs
gcc/tree-chkp-opt.o differs
gcc/gcc.o differs
gcc/store-motion.o differs
gcc/sched-rgn.o differs
gcc/haifa-sched.o differs
gcc/ada/rtsfind.o differs
gcc/ada/ali.o differs
gcc/ada/adaint.o differs
gcc/ada/fname-uf.o differs
gcc/ada/set_targ.o differs
gcc/ipa-icf.o differs
gcc/builtins.o differs
gcc/fold-const.o differs
gcc/reload.o differs
gcc/tree-ssa-loop-ivopts.o differs
gcc/tree-sra.o differs
gcc/tree-dfa.o differs
gcc/gimple-ssa-strength-reduction.o differs
gcc/tree-ssa-address.o differs
gcc/gimple-ssa-backprop.o differs
gcc/postreload.o differs
gcc/wide-int.o differs
gcc/tree-switch-conversion.o differs
gcc/real.o differs
gcc/tree-ssa-loop-niter.o differs
gcc/gimple-match.o differs
gcc/simplify-rtx.o differs
gcc/tree-vrp.o differs
gcc/tree-if-conv.o differs
gcc/coverage.o differs
gcc/gimple-fold.o differs
gcc/function.o differs
gcc/i386.o differs
gcc/tree-affine.o differs
gcc/tree-vect-loop.o differs
gcc/tree-ssa-structalias.o differs
gcc/c/c-parser.o differs
gcc/cfgrtl.o differs
gcc/tree-ssa-loop-prefetch.o differs
gcc/generic-match.o differs
gcc/rtl-chkp.o differs
gcc/tree-ssa-coalesce.o differs
gcc/mcf.o differs
gcc/optabs-libfuncs.o differs
libbacktrace/dwarf.o differs
libbacktrace/.libs/dwarf.o differs
libcpp/lex.o differs
Makefile:21544: recipe for target 'compare' failed

-- 
Eric Botcazou


Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.

2016-02-12 Thread Richard Biener
On Fri, Feb 12, 2016 at 12:53 AM, Jeff Law  wrote:
>
> So I've never thought much about our Complex support and I don't have a lot
> of confidence in the coverage for our testsuite for these changes. So I'd
> really appreciate someone with more experience thinking about this issue for
> a bit.
>
> I was looking at 33562 (P2), figuring it was DSE, which I wrote ~15 years
> ago, I could probably get up-to-speed and so something about it without
> major surgery (and for Complex I'm pretty sure I can).
>
> But while looking at the gimple code we handed off to DSE, it occurred to me
> that this would be easy to optimize if it were lowered.  Then, of course i
> remembered that we did lower complex stuff.
>
> So this turned into a short debugging session in tree-complex.c.
>
> The key statement in the test looks like
>
>
>
> complex int t = 0
>
> Where x is a complex object *and* has its address taken.  So the IL looks
> something like:
>
> t = __complex__ (0, 0);
>
>
>
> init_dont_simulate_again ignores this statement because the LHS is not an
> SSA_NAME (remember, t has had its address taken elsewhere in the sources).
>
> So complex lowering ends up totally ignoring the function in question.
>
> ISTM that we can and should still lower this code.  We don't want to set
> sim_again_p because the LHS is not in SSA form, so we don't really want/need
> to set up and track a lattice for this object.
>
> So the first step was to figure out the conditions under which we ought to
> detect an assignment to/from an aggregate that is not in SSA_FORM.
>
> I think we can do that with something like this in the GIMPLE_ASSIGN case:
>  /* Simple assignments involving complex types where
>  the RHS or LHS is addressable should be lowered, but
>  do not inherently trigger resimulation.  */
>   if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
>   == COMPLEX_TYPE)
> saw_a_complex_op = true;
>
>
> Essentially anytime we see a simple assignment (which would include
> initialization) we go ahead and let the complex lowering pass run, but we
> don't set sim_again_p.
>
>
> Then we need to actually lower the construct.  expand_complex_move has 99%
> of what we need.   If we take the code from this clause:
>
>else if (rhs && TREE_CODE (rhs) == SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
> [ ... ]
>
> Refactor it into its own function and call it when this condition is true:
>
> +  /* Assignment to/from a complex object that is addressable.  */
> +  else if (TREE_CODE (lhs) == VAR_DECL
> +  && TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE
> +  && (TREE_CODE (rhs) == VAR_DECL
> +  || TREE_CODE (rhs) == COMPLEX_CST
> +  || TREE_CODE (rhs) == COMPLEX_EXPR)
> +  && TREE_CODE (TREE_TYPE (rhs)) == COMPLEX_TYPE)
>
> Then complex-4.c and complex-5.c both work.  A variant of complex-4.c that I
> hacked up were we pass in a complex int, and assign that to a local
> addressable complex int gets lowered (and better optimized) as well.
>
> So what am I missing here?  Is there any kind of aliasing issues I need to
> be aware of?  Any other dragons lurking?

I think what you are missing is that you'll "optimize"

_Complex double x, y;

void foo (void)
{
  x = y;
}

then but dependent on the targets capability DCmode moves might be
cheaper than four DFmode moves and nothing will combine them together
again.

In complex lowering we're careful to only "optimize" when we know how to
extract components in an efficient way (well, mostly).

That the DSE opportunities in complex-[45].c are exposed if you lower
the aggregate inits is obvious but the lowering is only a workaround for
our lack of handling for this case.  I think the specific cases can be
easily made work by enhancing tree DSE in dse_possible_dead_store_p
to pick up partial kills by adjusting *ref (its offset/size) - keeping the
original ref for picking up uses.

But really we simply need a better DSE algorithm.

I think the two testcases are quite artificial and any "workaround" we
implement will cause more regressions elsewhere (code-size, RA, etc.)
if there are no followup optimization opportunities.

Richard.


> jeff
>
>
>
>
>
> diff --git a/gcc/tree-complex.c b/gcc/tree-complex.c
> index d781a8a..64346a5 100644
> --- a/gcc/tree-complex.c
> +++ b/gcc/tree-complex.c
> @@ -240,6 +240,14 @@ init_dont_simulate_again (void)
> op0 = gimple_assign_rhs1 (stmt);
>   if (gimple_num_ops (stmt) > 2)
> op1 = gimple_assign_rhs2 (stmt);
> +
> + /* Simple assignments involving complex types where
> +the RHS or LHS is addressable should be lowered, but
> +do not inherently trigger resimulation.  */
> + if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
> + == COMPLEX_TYPE)
> +   saw_a_complex_op = true;
> +
>   break;
>
> 

Re: [patch, Fortran] Fix PR 60526, variable name has already been declared as a type

2016-02-12 Thread Janne Blomqvist
On Fri, Feb 12, 2016 at 12:16 PM, Andre Vehreschild  wrote:
> Hi all,
>
> it wasn't my intention to start such a big discussion for such a small
> thing. Please stop this academic battle and get back to the issue at
> hand:
>
> Thomas is examining a symbol, that
>
> - has come through the scanner, i.e., it adheres to the rules of
>   gfortran, especially it is known to be smaller than GFC_MAX_SYMBOL_LEN
>
> - is still a valid symbol. When gfortran's internal
>   composition/modification of symbols breaks this rule, we are done
>   earlier already.
>
> - its length is not changed in his routine.
>
> I was only trying to prevent an alloc/free for this small use case,
> which should fit on the stack quite easily. The proposed alloca() call
> has according to the documentation of libc some availability issues on
> certain platforms, too. Therefore why not going with the fixed size
> stack array and adding a check for possible overflow to it and be done?

Yes, I agree. That sounds like the best approach in this case.



-- 
Janne Blomqvist


[PATCH] Fix PR69776, FRE/PRE not honoring middle-end memory model

2016-02-12 Thread Richard Biener

FRE/PREs redundant store removal code doesn't honor the middle-end
memory model which says that all stores change the dynamic type of
the affected memory region.  So it removed the second 'int' store in

  MEM[(int *)p_3] = 1;
  MEM[(double *)p_3] = 0.0;
  MEM[(int *)p_3] = 1;

which is not valid (if not done later DSE removes the first int store
which _is_ valid).

So I am bootstrapping and testing the following fix on 
x86_64-unknown-linux-gnu.

Richard.

2016-02-12  Richard Biener  

PR tree-optimization/69776
* tree-ssa-sccvn.h (vn_reference_lookup): Adjust prototype.
* tree-ssa-sccvn.c (vn_reference_lookup): Add parameter to
indicate whether we can use TBAA to disambiguate against stores.
Use alias-set zero if not.
(visit_reference_op_store): Do not use TBAA when looking up
redundant stores.
* tree-ssa-pre.c (compute_avail): Use TBAA here.
(eliminate_dom_walker::before_dom_children): But not when looking
up redundant stores.

* gcc.dg/torture/pr69776.c: New testcase.

Index: gcc/tree-ssa-sccvn.h
===
*** gcc/tree-ssa-sccvn.h(revision 233369)
--- gcc/tree-ssa-sccvn.h(working copy)
*** bool ao_ref_init_from_vn_reference (ao_r
*** 216,222 
  tree vn_reference_lookup_pieces (tree, alias_set_type, tree,
 vec ,
 vn_reference_t *, vn_lookup_kind);
! tree vn_reference_lookup (tree, tree, vn_lookup_kind, vn_reference_t *);
  void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t);
  vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, tree,
   vec ,
--- 216,222 
  tree vn_reference_lookup_pieces (tree, alias_set_type, tree,
 vec ,
 vn_reference_t *, vn_lookup_kind);
! tree vn_reference_lookup (tree, tree, vn_lookup_kind, vn_reference_t *, bool);
  void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t);
  vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, tree,
   vec ,
Index: gcc/tree-ssa-sccvn.c
===
*** gcc/tree-ssa-sccvn.c(revision 233369)
--- gcc/tree-ssa-sccvn.c(working copy)
*** vn_reference_lookup_pieces (tree vuse, a
*** 2230,2240 
 number if it exists in the hash table.  Return NULL_TREE if it does
 not exist in the hash table or if the result field of the structure
 was NULL..  VNRESULT will be filled in with the vn_reference_t
!stored in the hashtable if one exists.  */
  
  tree
  vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
!vn_reference_t *vnresult)
  {
vec operands;
struct vn_reference_s vr1;
--- 2230,2241 
 number if it exists in the hash table.  Return NULL_TREE if it does
 not exist in the hash table or if the result field of the structure
 was NULL..  VNRESULT will be filled in with the vn_reference_t
!stored in the hashtable if one exists.  When TBAA_P is false assume
!we are looking up a store and treat it as having alias-set zero.  */
  
  tree
  vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
!vn_reference_t *vnresult, bool tbaa_p)
  {
vec operands;
struct vn_reference_s vr1;
*** vn_reference_lookup (tree op, tree vuse,
*** 2264,2269 
--- 2265,2272 
  || !ao_ref_init_from_vn_reference (, vr1.set, vr1.type,
 vr1.operands))
ao_ref_init (, op);
+   if (! tbaa_p)
+   r.ref_alias_set = r.base_alias_set = 0;
vn_walk_kind = kind;
wvnresult =
(vn_reference_t)walk_non_aliased_vuses (, vr1.vuse,
*** visit_reference_op_load (tree lhs, tree
*** 3350,3356 
last_vuse = gimple_vuse (stmt);
last_vuse_ptr = _vuse;
result = vn_reference_lookup (op, gimple_vuse (stmt),
!   default_vn_walk_kind, NULL);
last_vuse_ptr = NULL;
  
/* We handle type-punning through unions by value-numbering based
--- 3353,3359 
last_vuse = gimple_vuse (stmt);
last_vuse_ptr = _vuse;
result = vn_reference_lookup (op, gimple_vuse (stmt),
!   default_vn_walk_kind, NULL, true);
last_vuse_ptr = NULL;
  
/* We handle type-punning through unions by value-numbering based
*** visit_reference_op_store (tree lhs, tree
*** 3472,3478 
   Otherwise, the vdefs for the store are used when inserting into
   the table, since the store generates a new memory state.  */
  
!   result = vn_reference_lookup (lhs, vuse, VN_NOWALK, NULL);
  
if (result)
  {
--- 3475,3481 
   Otherwise, the vdefs for the 

Re: [PING^3][PATCH, 12/16] Handle acc loop directive

2016-02-12 Thread Tom de Vries

On 26/01/16 13:49, Jakub Jelinek wrote:

On Tue, Jan 26, 2016 at 01:38:39PM +0100, Tom de Vries wrote:

Ping^3. ( https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01089.html )


First of all, I wonder if it wouldn't be far easier to handle these during
gimplification rather than during omp expansion or during parsing.  Inside
kernels, do you need to honor any clauses on the acc loop, like
privatization etc., or can you just ignore it altogether (after parsing them
to ensure they are valid)?


The oacc loop clauses are: gang, worker, vector, seq, auto, tile, 
device_type, independent, private, reduction.


AFAIU, there're all safe to ignore. That has largely been the approach 
in the gomp-4_0-branch, and sofar I haven't seen any failures due to 
ignoring a loop clause in a kernels region.


But we do want to be able to honor loop clauses in a kernels region at 
some point. F.i., supporting the independent clause would allow more 
test-cases to be parallelized.


At some point we had an implementation of the independent clause in the 
gomp-4_0-branch, but that had to be reverted ( 
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00696.html ).


Anyway, the implementation of the propagation of the independent 
property was to keep the loop directive with the independent clause 
until omp-expand (where we have cfg), and set a new field 
marked_independent in the corresponding struct loop.


If we want to do the expansion of the loop directive to a normal loop at 
gimplication, I see two issues:

- in general, we don't only check for correctness during parsing,
  there's also checking being done during scan_omp, which happens in
  pass_lower_omp, after gimplification.
- how do we mark the new loop as being independent?


Handling this in expand_omp_for_generic is not really nice, because it will
make already very complicated function even more complex.


An alternative would be to copy expand_omp_for_generic, apply the patch, 
and partially evaluate for the single call introduced in the patch.


Do you prefer this approach?

Thanks,
- Tom


gomp_ordered *ord_stmt;
+
+  /* True if this is nested inside an OpenACC kernels construct.  */
+  bool inside_kernels_p;
  };

is bad placement, there are other bool/unsigned char fields earlier and the
smaller fields should be adjacent for smaller padding of the struct.

Jakub





Re: [PATCH] Fix another ipa-split caused ICE (PR ipa/69241)

2016-02-12 Thread Richard Biener
On Thu, 11 Feb 2016, Jakub Jelinek wrote:

> On Thu, Feb 11, 2016 at 10:22:24AM +0100, Richard Biener wrote:
> > The other option is to simply not split the function in this case.
> 
> Here is a better fix (but it needs the other patch I've sent, so that
> what is return_bb stays the same).
> 
> This patch arranges for the case where there is return_bb, but does not
> return a value, in function that has return type TREE_ADDRESSABLE,
> to just use void return type from the split part (because we would never
> set lhs of the call to *.part.* in that case anyway).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux on top of the PR68672
> patch, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-02-10  Jakub Jelinek  
> 
>   PR ipa/69241
>   * ipa-split.c (split_function): If split part returns TREE_ADDRESSABLE
>   type by reference, force lhs on the call.
> 
>   * g++.dg/ipa/pr69241-4.C: New test.
> 
> --- gcc/ipa-split.c.jj2016-02-11 12:46:15.975777652 +0100
> +++ gcc/ipa-split.c   2016-02-11 13:06:57.715241871 +0100
> @@ -629,7 +629,18 @@ consider_split (struct split_point *curr
> 4) For non-SSA we need to look where the var is computed. */
>retval = find_retval (return_bb);
>if (!retval)
> -current->split_part_set_retval = true;
> +{
> +  /* If there is a return_bb with no return value in function returning
> +  value by reference, also make the split part return void, otherwise
> +  we expansion would try to create a non-POD temporary, which is
> +  invalid.  */
> +  if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun)
> +   && DECL_RESULT (current_function_decl)
> +   && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> + current->split_part_set_retval = false;
> +  else
> + current->split_part_set_retval = true;
> +}
>else if (is_gimple_min_invariant (retval))
>  current->split_part_set_retval = false;
>/* Special case is value returned by reference we record as if it was 
> non-ssa
> --- gcc/testsuite/g++.dg/ipa/pr69241-4.C.jj   2016-02-11 13:00:04.160075417 
> +0100
> +++ gcc/testsuite/g++.dg/ipa/pr69241-4.C  2016-02-11 13:00:04.160075417 
> +0100
> @@ -0,0 +1,55 @@
> +// PR ipa/69241
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -Wno-return-type" }
> +
> +template  class A;
> +struct B {
> +  using pointer = int *;
> +};
> +template > class basic_string {
> +  long _M_string_length;
> +  enum { _S_local_capacity = 15 } _M_local_buf[_S_local_capacity];
> +  B::pointer _M_local_data;
> +
> +public:
> +  ~basic_string();
> +};
> +template 
> +int operator<<(_Traits, basic_string<_CharT, _Alloc>);
> +class C {
> +  basic_string _M_string;
> +};
> +class D {
> +  C _M_stringbuf;
> +};
> +class F {
> +  int stream;
> +  D stream_;
> +};
> +class G {
> +public:
> +  void operator&(int);
> +};
> +class H {
> +public:
> +  H(unsigned);
> +  H(H &&);
> +  bool m_fn1();
> +};
> +class I {
> +  void m_fn2(const int &&);
> +  static H m_fn3(const int &);
> +};
> +template  void Bind(Functor);
> +class J {
> +public:
> +  static basic_string m_fn4();
> +};
> +int a;
> +void I::m_fn2(const int &&) { Bind(m_fn3); }
> +H I::m_fn3(const int &) {
> +  !false ? (void)0 : G() & F() << J::m_fn4();
> +  H b(a);
> +  if (b.m_fn1())
> +F();
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-12 Thread Andre Vieira (lists)

On 12/02/16 07:43, Jeff Law wrote:

On 02/11/2016 06:28 PM, Bernd Schmidt wrote:

This seems fairly straightforward:

(insn 213 455 216 6 (set (reg:SI 266)
 (mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4  S4 A32])) 748
{*thumb1_movsi_insn}
  (expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
 (expr_list:REG_INC (reg/f:SI 267)
 (nil

We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.

Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?


Bernd

equiv-inc.diff


PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single SET,
also take other side effects into account.

Also note that the reporter says gcc-4.9 didn't have this problem, so
there's a reasonable chance this is a latent regression exposed by
codegen changes prior to IRA.


OK for the trunk.

Jeff

I tested it for the particular testcase it was failing for cortex-m0 and 
it fixed it. Ill fire up a regression run next.


Cheers,
Andre


Re: Fix c/69522, memory management issue in c-parser

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 02:26 PM, Marek Polacek wrote:

On Fri, Feb 12, 2016 at 02:17:19PM +0100, Andreas Schwab wrote:

FAIL: gcc.dg/pr69522.c (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:2:8: error: 
struct has no members [-Wpedantic]
/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C 
forbids empty initializer braces [-Wpedantic]


Bernd, ok to fix this with the following?

2016-02-12  Marek Polacek  <pola...@redhat.com>

* gcc.dg/pr69522.c: Add empty dg-options.


What's causing the problem there, -Wpedantic? Maybe add the opposite 
rather than an empty dg-options? Either way's fine, sorry about the 
breakage - I thought I had the testcase applied during the test run, but 
apparently not.



Bernd



Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 01:50:08PM +0100, Richard Biener wrote:
> > -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> > -  if (xmode1 != VOIDmode && xmode1 != mode1)
> > +  mode1 = GET_MODE (xop1);
> > +  if (xmode1 != mode1)
> >  {
> >xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
> >mode1 = xmode1;
> > 
> > so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> > But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> > do convert_modes.
> > 
> > The only thing that can (hopefully not) happen is that xmode1
> > is VOIDmode but mode1 is not (maybe removing the xmode1 != VOIDmode
> > check is a bit too optimistic here).  Not sure if there can be
> > valid patterns requesting a VOIDmode op ... (and not only accept
> > CONST_INTs).
> 
> Oh, bootstrap & testing went fine on x86_64-unknown-linux-gnu.
> 
> If we can agree on the patch then I'll pick up the testcase from
> your patch (and adjust mine).

Another possibility, only do the convert_modes from VOIDmode for
shift_optab_p's xop1, and keep doing what we've done before otherwise.

2016-02-12  Jakub Jelinek  

PR rtl-optimization/69764
PR rtl-optimization/69771
* optabs.c (expand_binop_directly): For shift_optab_p, force
convert_modes with VOIDmode if xop1 has VOIDmode.

* c-c++-common/pr69764.c: New test.
* gcc.dg/torture/pr69771.c: New testcase.

--- gcc/optabs.c.jj 2016-02-12 00:50:30.410240366 +0100
+++ gcc/optabs.c2016-02-12 10:33:32.743492532 +0100
@@ -988,7 +988,7 @@ expand_binop_directly (machine_mode mode
  from_mode, 1);
   machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
   machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
-  machine_mode mode0, mode1, tmp_mode;
+  machine_mode mode0, mode1 = mode, tmp_mode;
   struct expand_operand ops[3];
   bool commutative_p;
   rtx_insn *pat;
@@ -1006,6 +1006,8 @@ expand_binop_directly (machine_mode mode
   xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
   if (!shift_optab_p (binoptab))
 xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  else
+mode1 = VOIDmode;
 
   /* In case the insn wants input operands in modes different from
  those of the actual operands, convert the operands.  It would
@@ -1020,7 +1020,7 @@ expand_binop_directly (machine_mode mode
   mode0 = xmode0;
 }
 
-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1;
   if (xmode1 != VOIDmode && xmode1 != mode1)
 {
   xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
--- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 10:27:34.522587995 
+0100
+++ gcc/testsuite/c-c++-common/pr69764.c2016-02-12 10:27:34.522587995 
+0100
@@ -0,0 +1,38 @@
+/* PR rtl-optimization/69764 */
+/* { dg-do compile { target int32plus } } */
+
+unsigned char
+fn1 (unsigned char a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned short
+fn2 (unsigned short a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned int
+fn3 (unsigned int a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned char
+fn4 (unsigned char a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned short
+fn5 (unsigned short a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned int
+fn6 (unsigned int a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
--- gcc/testsuite/gcc.dg/torture/pr69771.c.jj   2016-02-12 10:27:34.522587995 
+0100
+++ gcc/testsuite/gcc.dg/torture/pr69771.c  2016-02-12 10:27:34.522587995 
+0100
@@ -0,0 +1,12 @@
+/* PR rtl-optimization/69771 */
+/* { dg-do compile } */
+
+unsigned char a = 5, c;
+unsigned short b = 0;
+unsigned d = 0x76543210;
+
+void
+foo (void)
+{
+  c = d >> ~(a || ~b); /* { dg-warning "shift count is negative" } */
+}

Jakub


Re: AW: Wonly-top-basic-asm

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 08:05 AM, David Wohlferd wrote:

Actually, it was my intent that this apply to v6.  It's not like there
is a significant change here.  We're documenting long-time behavior, and
adding a (disabled) warning.


The doc patch (minus mentioning the warning) could go in now, but for 
gcc-6 we're at a stage where we're only accepting regression fixes with 
very few exceptions. If you can convince a RM that this is important 
enough then it could still go in.



2) There is a significant change to this behavior being proposed for
v7.  When this happens, having a way to locate affected statements with
features from a stable release seems desirable.


I'm actually not convinced that we'll want to change much in asm 
behaviour. Clobbering memory, maybe, but I can't see much beyond that - 
there's just no gain and some risk. So I'm a little more relaxed about 
the whole thing.



"Since the C standards does not specify semantics for @code{asm}, it
is a potential source of incompatibilities between compilers. GCC does
not parse the @var{AssemblerInstructions}, which means there is no way
to communicate to the compiler what is happening inside them.  GCC has
no visibility of any symbols referenced in the @code{asm} and may
discard them as unreferenced. It also does not know about side effects
that may occur, such as modifications of memory locations or
registers. GCC assumes that no such side effects occur, which may not
be what the user expected if code was written for other compilers.

Since basic @code{asm} cannot easily be used in a reliable way,
@option{-Wbasic-asm} should be used to warn about the use of basic asm
inside a function. See
@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to
convert from basic asm to extended asm} for information about how to
convert code to use extended @code{asm}."


Hmm.  Yes, that's better.  But there are some things that got lost here
that I think are important.  How about:


@strong{Warning:} The C standards do not specify semantics for
@code{asm}, making it a potential source of incompatibilities between
compilers.  @code{asm} statements that work correctly on other compilers
may not work correctly with GCC (and vice versa), even though both
compile without error.


This is what I mean when I say "too verbose" - the second sentence 
essentially says exactly the same thing as the first. The repetition is 
unnecessary, and I'd drop it.



GCC does not parse basic @code{asm}'s @var{AssemblerInstructions}, which
means there is no way to communicate to the compiler what is happening
inside them.  GCC has no visibility of symbols in the @code{asm} and may
discard them as unreferenced.  It also does not know about side effects
of the assembler code, such as modifications to memory or registers.
Unlike some compilers, GCC assumes that no changes to either memory or
registers occur.  This assumption may change in a future release.

To avoid complications from future changes to the semantics and the
compatibility issues between compilers, use @option{-Wbasic-asm} to warn
about the use of basic asm inside a function.  See
@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert
from basic asm to extended asm} for information about how to convert
code to use extended @code{asm}.


Other than that they look quite similar, and I think your new suggestion 
is good too. Let's let Sandra have the last word.



Bernd


Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-12 Thread Jiong Wang



On 12/02/16 07:43, Jeff Law wrote:

On 02/11/2016 06:28 PM, Bernd Schmidt wrote:

This seems fairly straightforward:

(insn 213 455 216 6 (set (reg:SI 266)
 (mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4  S4 A32])) 748
{*thumb1_movsi_insn}
  (expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
 (expr_list:REG_INC (reg/f:SI 267)
 (nil

We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.

Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?


Bernd

equiv-inc.diff


PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single 
SET,

also take other side effects into account.


Will it be better that we don't remove the insn if it has side-effect 
instead of don't record the equiv?


This can offer more equiv for later rtl optimization?



Also note that the reporter says gcc-4.9 didn't have this problem, so 
there's a reasonable chance this is a latent regression exposed by 
codegen changes prior to IRA.



OK for the trunk.

Jeff





Re: [PATCH] Spelling fixes - behaviour and neighbour

2016-02-12 Thread Arnaud Charlet
> While working on the -Waddress fix I've posted earlier today, I've noticed
> that the C and C++ FE disagree on the spelling - C uses US english spelling,
> while C++ uses UK english spelling.
> This patch switches to US english spelling of these 2 words, in
> diagnostics, documentation and comments as well.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK for the Ada part (where we indeed use US spelling).

Arno


Re: [PATCH] Fix ipa-split handling of clobbers and debug stmts in return_bb (PR ipa/68672)

2016-02-12 Thread Richard Biener
On Thu, 11 Feb 2016, Jakub Jelinek wrote:

> Hi!
> 
> While the cgraph versioning code is able to deal with clobber stmts that
> refer to SSA_NAMEs set by basic blocks not split into the versioned
> function, and similarly with debug stmts (clobbers refererring to those
> are removed and debug stmts reset), on the main part that doesn't happen,
> because ipa-split makes the split blocks unreachable by jumping around them
> and thus if SSA_NAMEs set in the split basic blocks are referenced
> in return_bb in statements we intentionally ignored for the analysis (debug
> stmts and clobber stmts), we either end up with weird stuff (set debug stmts
> to debug temporary that is set in the basic blocks that are removed as
> dead), or for clobbers ICE.
> 
> Richard has tried to deal with this by throwing away the return_bb in
> certain cases in the main part (forcing recreation of it), but that can
> throw valuable statements that we could have kept (debug stmts or e.g. decl
> clobbers or clobbers that don't need SSA_NAMEs from the split blocks),
> and doesn't work e.g. on the 1st and 3rd testcase below anyway.
> 
> So, this patch reverts the main_part_return_p and instead does
> what is needed for those ignored stmts in return_bb:
> 1) SSA_NAME references to retval in both debug stmts and clobber stmts
>are replaced with the lhs of the call to *.part.*
> 2) debug stmts referencing other SSA_NAMEs set in the split bbs are reset
> 3) clobber stmts referencing other SSA_NAMEs set in the split bbs are removed
> 
> The testcase hopefully cover all the interesting cases (return_bb copied
> from main part (where it is kept) to split part too, and with debug stmts
> and clobbers that refer to SSA_NAMEs set in main or split part or retval).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-02-11  Jakub Jelinek  
> 
>   PR ipa/68672
>   * ipa-split.c (split_function): Don't compute/use main_part_return_p.
>   Compute retval and retbnd early in all cases if split_part_return_p
>   and return_bb is not EXIT.  Remove all clobber stmts and reset
>   all debug stmts that refer to SSA_NAMEs defined in split part,
>   except if it is retval, in that case replace the old retval with the
>   lhs of the call to the split part.
> 
>   * g++.dg/ipa/pr68672-1.C: New test.
>   * g++.dg/ipa/pr68672-2.C: New test.
>   * g++.dg/ipa/pr68672-3.C: New test.
> 
> --- gcc/ipa-split.c.jj2016-02-11 10:50:52.888220581 +0100
> +++ gcc/ipa-split.c   2016-02-11 12:46:15.975777652 +0100
> @@ -1244,28 +1244,13 @@ split_function (basic_block return_bb, s
>   args_to_pass.safe_push (arg);
>}
>  
> -  /* See if the split function or the main part will return.  */
> -  bool main_part_return_p = false;
> +  /* See if the split function will return.  */
>bool split_part_return_p = false;
>FOR_EACH_EDGE (e, ei, return_bb->preds)
>  {
>if (bitmap_bit_p (split_point->split_bbs, e->src->index))
>   split_part_return_p = true;
> -  else
> - main_part_return_p = true;
>  }
> -  /* The main part also returns if we split on a fallthru edge
> - and the split part returns.  */
> -  if (split_part_return_p)
> -FOR_EACH_EDGE (e, ei, split_point->entry_bb->preds)
> -  {
> - if (! bitmap_bit_p (split_point->split_bbs, e->src->index)
> - && single_succ_p (e->src))
> -   {
> - main_part_return_p = true;
> - break;
> -   }
> -  }
>  
>/* Add return block to what will become the split function.
>   We do not return; no return block is needed.  */
> @@ -1279,8 +1264,8 @@ split_function (basic_block return_bb, s
>   FIXME: Once we are able to change return type, we should change function
>   to return void instead of just outputting function with undefined return
>   value.  For structures this affects quality of codegen.  */
> -  else if (!split_point->split_part_set_retval
> -   && (retval = find_retval (return_bb)))
> +  else if ((retval = find_retval (return_bb))
> +&& !split_point->split_part_set_retval)
>  {
>bool redirected = true;
>basic_block new_return_bb = create_basic_block (NULL, 0, return_bb);
> @@ -1308,12 +1293,10 @@ split_function (basic_block return_bb, s
>  }
>/* When we pass around the value, use existing return block.  */
>else
> -bitmap_set_bit (split_point->split_bbs, return_bb->index);
> -
> -  /* If the main part doesn't return pretend the return block wasn't
> - found for all of the following.  */
> -  if (! main_part_return_p)
> -return_bb = EXIT_BLOCK_PTR_FOR_FN (cfun);
> +{
> +  bitmap_set_bit (split_point->split_bbs, return_bb->index);
> +  retbnd = find_retbnd (return_bb);
> +}
>  
>/* If RETURN_BB has virtual operand PHIs, they must be removed and the
>   virtual operand marked for renaming as we change the CFG in a way 

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Bernd Schmidt



On 02/12/2016 11:46 AM, Richard Biener wrote:

On Fri, 12 Feb 2016, Jakub Jelinek wrote:


On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote:


I am testing the following patch which fixes PR69771 where the code
doesn't match the comment before it.  We get to expand a QImode << QImode
shift but the shift amount was of type int and thus it was expanded
as SImode constant.  Then

   /* In case the insn wants input operands in modes different from
  those of the actual operands, convert the operands.  It would
  seem that we don't need to convert CONST_INTs, but we do, so
  that they're properly zero-extended, sign-extended or truncated
  for their mode.  */

has to apply as we need to re-extend the VOIDmode CONST_INT for
QImode.  But then mode1 is computed as 'mode' (QImode) which happens
to match what is expected even though the constant isn't valid.

The fix is IMHO to always call convert_modes for VOIDmode ops
(if the target doesn't expect VOIDmode itself).

Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?


This looks like the PR69764 fix I've sent last night (and updated patch
this morning).
BTW, I wouldn't use a runtime test with clearly undefined behavior,
especially not if it tests what the outcome of that UB is.


Ah, indeed looks like a dup.  Let's go with your version which had
feedback from Bernd already.  Might want to add my testcase (w/o the
runtime outcome test).


Actually, Richard I was just looking at Jakub's second patch and I think 
yours is better - on the first round of review I didn't notice that the 
convert_modes code is there and is documented to deal with the CONST_INT 
problem. If it completed testing I think you should commit it.



Bernd


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener
On Fri, 12 Feb 2016, Jakub Jelinek wrote:

> On Fri, Feb 12, 2016 at 01:15:11PM +0100, Bernd Schmidt wrote:
> > >Ah, indeed looks like a dup.  Let's go with your version which had
> > >feedback from Bernd already.  Might want to add my testcase (w/o the
> > >runtime outcome test).
> > 
> > Actually, Richard I was just looking at Jakub's second patch and I think
> > yours is better - on the first round of review I didn't notice that the
> > convert_modes code is there and is documented to deal with the CONST_INT
> > problem. If it completed testing I think you should commit it.
> 
> That patch doesn't look right to me.  The code is there not just for shifts,
> but also for non-shifts, and for non-shifts, we know the arguments are in
> mode, we also know unsignedp, so if needed convert_modes can perform
> zero or sign extension.  IMHO it is just shifts/rotates that are
> problematical, because what the second operand mode is and whether it is 
> unsigned
> or signed is just less well defined, and then various backends have various
> requirements on it.  Also, on some target for shift/rotate xmode1 might be
> already equal to mode, and in that case convert_modes would not be called,
> but still the CONST_INT might be originally from yet another mode and we'd
> still ICE.

But my patch should deal with all this.

-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
-  if (xmode1 != VOIDmode && xmode1 != mode1)
+  mode1 = GET_MODE (xop1);
+  if (xmode1 != mode1)
 {
   xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
   mode1 = xmode1;

so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
do convert_modes.

The only thing that can (hopefully not) happen is that xmode1
is VOIDmode but mode1 is not (maybe removing the xmode1 != VOIDmode
check is a bit too optimistic here).  Not sure if there can be
valid patterns requesting a VOIDmode op ... (and not only accept
CONST_INTs).

Richard.


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener
On Fri, 12 Feb 2016, Richard Biener wrote:

> On Fri, 12 Feb 2016, Jakub Jelinek wrote:
> 
> > On Fri, Feb 12, 2016 at 01:15:11PM +0100, Bernd Schmidt wrote:
> > > >Ah, indeed looks like a dup.  Let's go with your version which had
> > > >feedback from Bernd already.  Might want to add my testcase (w/o the
> > > >runtime outcome test).
> > > 
> > > Actually, Richard I was just looking at Jakub's second patch and I think
> > > yours is better - on the first round of review I didn't notice that the
> > > convert_modes code is there and is documented to deal with the CONST_INT
> > > problem. If it completed testing I think you should commit it.
> > 
> > That patch doesn't look right to me.  The code is there not just for shifts,
> > but also for non-shifts, and for non-shifts, we know the arguments are in
> > mode, we also know unsignedp, so if needed convert_modes can perform
> > zero or sign extension.  IMHO it is just shifts/rotates that are
> > problematical, because what the second operand mode is and whether it is 
> > unsigned
> > or signed is just less well defined, and then various backends have various
> > requirements on it.  Also, on some target for shift/rotate xmode1 might be
> > already equal to mode, and in that case convert_modes would not be called,
> > but still the CONST_INT might be originally from yet another mode and we'd
> > still ICE.
> 
> But my patch should deal with all this.
> 
> -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> -  if (xmode1 != VOIDmode && xmode1 != mode1)
> +  mode1 = GET_MODE (xop1);
> +  if (xmode1 != mode1)
>  {
>xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
>mode1 = xmode1;
> 
> so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> do convert_modes.
> 
> The only thing that can (hopefully not) happen is that xmode1
> is VOIDmode but mode1 is not (maybe removing the xmode1 != VOIDmode
> check is a bit too optimistic here).  Not sure if there can be
> valid patterns requesting a VOIDmode op ... (and not only accept
> CONST_INTs).

Oh, bootstrap & testing went fine on x86_64-unknown-linux-gnu.

If we can agree on the patch then I'll pick up the testcase from
your patch (and adjust mine).

Richard.


[PATCH, CHKP, committed] Fix PR middle-end/69729

2016-02-12 Thread Ilya Enkovich
Hi,

This patch fixes instrumentation thunk recognition condition in lto_output.  
This avoids missing required thunks in ltrans modules.  Bootstrapped and 
regtested on x86_64-pc-linux-gnu.  Committed to trunk.

Thanks,
Ilya
--
gcc/

2016-02-12  Ilya Enkovich  

PR target/69729
* lto-streamer-out.c (lto_output): Use thunk.add_pointer_bounds_args
to correctly determine instrumentation thunks.

gcc/testsuite/

2016-02-12  Ilya Enkovich  

* g++.dg/lto/lto.exp: Include and init mpx.
* g++.dg/lto/pr69729_0.C: New test.


diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 6bb76cc..997a28b 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -2321,7 +2321,7 @@ lto_output (void)
{
  if (lto_symtab_encoder_encode_body_p (encoder, node)
  && !node->alias
- && (!node->thunk.thunk_p || !node->instrumented_version))
+ && (!node->thunk.thunk_p || !node->thunk.add_pointer_bounds_args))
{
  if (flag_checking)
{
diff --git a/gcc/testsuite/g++.dg/lto/lto.exp b/gcc/testsuite/g++.dg/lto/lto.exp
index 8d99418..48f0947 100644
--- a/gcc/testsuite/g++.dg/lto/lto.exp
+++ b/gcc/testsuite/g++.dg/lto/lto.exp
@@ -31,6 +31,7 @@ if $tracelevel then {
 load_lib standard.exp
 load_lib g++.exp
 load_lib target-libpath.exp
+load_lib mpx-dg.exp
 
 # Load the language-independent compabibility support procedures.
 load_lib lto.exp
@@ -42,6 +43,7 @@ if { ![check_effective_target_lto] } {
 
 g++_init
 lto_init no-mathlib
+mpx_init
 
 # Define an identifier for use with this suite to avoid name conflicts
 # with other lto tests running at the same time.
@@ -57,4 +59,5 @@ foreach src [lsort [find $srcdir/$subdir *_0.\[cC\]]] {
 lto-execute $src $sid
 }
 
+mpx_finish
 lto_finish
diff --git a/gcc/testsuite/g++.dg/lto/pr69729_0.C 
b/gcc/testsuite/g++.dg/lto/pr69729_0.C
new file mode 100644
index 000..b736406
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr69729_0.C
@@ -0,0 +1,35 @@
+/* { dg-lto-do link } */
+/* { dg-require-effective-target mpx } */
+/* { dg-lto-options {{-fcheck-pointer-bounds -mmpx -flto -flto-partition=max}} 
} */
+
+class cl1
+{
+ public:
+  virtual ~cl1 () { };
+};
+
+class cl2
+{
+ public:
+  virtual ~cl2 () { };
+};
+
+class cl3 : cl1, cl2
+{
+};
+
+class cl4 : cl3
+{
+ public:
+  ~cl4 ();
+};
+
+cl4::~cl4 ()
+{
+}
+
+int main (int argc, char **argv)
+{
+  cl4 c;
+  return 0;
+}


Re: Fix c/69522, memory management issue in c-parser

2016-02-12 Thread Andreas Schwab
FAIL: gcc.dg/pr69522.c (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:2:8: error: 
struct has no members [-Wpedantic]
/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C 
forbids empty initializer braces [-Wpedantic]

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


Re: Fix c/69522, memory management issue in c-parser

2016-02-12 Thread Marek Polacek
On Fri, Feb 12, 2016 at 02:17:19PM +0100, Andreas Schwab wrote:
> FAIL: gcc.dg/pr69522.c (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:2:8: error: 
> struct has no members [-Wpedantic]
> /daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO 
> C forbids empty initializer braces [-Wpedantic]

Bernd, ok to fix this with the following?

2016-02-12  Marek Polacek  <pola...@redhat.com>

* gcc.dg/pr69522.c: Add empty dg-options.

diff --git gcc/testsuite/gcc.dg/pr69522.c gcc/testsuite/gcc.dg/pr69522.c
index 452a1ae..820168d 100644
--- gcc/testsuite/gcc.dg/pr69522.c
+++ gcc/testsuite/gcc.dg/pr69522.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-options "" } */
 struct str {};
 struct {
   struct str b;

Marek


Re: [PATCH, reload] PRE_INC with invalid hard reg

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 04:27 AM, Alan Modra wrote:

I don't understand this comment.  If we're pushing a reload of the
inner reg, then the SECONDARY_MEMORY_NEEDED code in push_reload will
fire.  Why then should there be any need to do anything special in
find_reloads_address_1 regarding secondary memory?


The idea was to find out whether we needed to special case things. But, 
after looking at this area for a while, I think your original patch 
makes the most sense and the things I was worried about are non-issues. 
Please commit.



Bernd


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 01:15:11PM +0100, Bernd Schmidt wrote:
> >Ah, indeed looks like a dup.  Let's go with your version which had
> >feedback from Bernd already.  Might want to add my testcase (w/o the
> >runtime outcome test).
> 
> Actually, Richard I was just looking at Jakub's second patch and I think
> yours is better - on the first round of review I didn't notice that the
> convert_modes code is there and is documented to deal with the CONST_INT
> problem. If it completed testing I think you should commit it.

That patch doesn't look right to me.  The code is there not just for shifts,
but also for non-shifts, and for non-shifts, we know the arguments are in
mode, we also know unsignedp, so if needed convert_modes can perform
zero or sign extension.  IMHO it is just shifts/rotates that are
problematical, because what the second operand mode is and whether it is 
unsigned
or signed is just less well defined, and then various backends have various
requirements on it.  Also, on some target for shift/rotate xmode1 might be
already equal to mode, and in that case convert_modes would not be called,
but still the CONST_INT might be originally from yet another mode and we'd
still ICE.

Jakub


[PATCH, MPX, committed] Fix warning in MPX effective target test

2016-02-12 Thread Ilya Enkovich
Hi,

This patch fixes a warning in test used for effective MPX target check.  Fix 
allows to use test with g++.  Bootsrapped and tested on x86_64-pc-linux-gnu.  
Applied to trunk.

Thanks,
Ilya
--
gcc/testsuite/

2016-02-11  Ilya Enkovich  

* lib/mpx-dg.exp: Fix warning in check_effective_target_mpx
test.


diff --git a/gcc/testsuite/lib/mpx-dg.exp b/gcc/testsuite/lib/mpx-dg.exp
index fa2faaa..b245c5f 100644
--- a/gcc/testsuite/lib/mpx-dg.exp
+++ b/gcc/testsuite/lib/mpx-dg.exp
@@ -22,7 +22,7 @@ proc check_effective_target_mpx {} {
int *foo (int *arg) { return arg; }
int main (void)
{
-   int *p = __builtin_malloc (sizeof (int));
+   int *p = (int *)__builtin_malloc (sizeof (int));
int res = foo (p) == 0;
__builtin_free (p);
return res;


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener
On Fri, 12 Feb 2016, Jakub Jelinek wrote:

> On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote:
> > 
> > I am testing the following patch which fixes PR69771 where the code
> > doesn't match the comment before it.  We get to expand a QImode << QImode
> > shift but the shift amount was of type int and thus it was expanded
> > as SImode constant.  Then
> > 
> >   /* In case the insn wants input operands in modes different from
> >  those of the actual operands, convert the operands.  It would
> >  seem that we don't need to convert CONST_INTs, but we do, so
> >  that they're properly zero-extended, sign-extended or truncated
> >  for their mode.  */
> > 
> > has to apply as we need to re-extend the VOIDmode CONST_INT for
> > QImode.  But then mode1 is computed as 'mode' (QImode) which happens
> > to match what is expected even though the constant isn't valid.
> > 
> > The fix is IMHO to always call convert_modes for VOIDmode ops
> > (if the target doesn't expect VOIDmode itself).
> > 
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?
> 
> This looks like the PR69764 fix I've sent last night (and updated patch
> this morning).
> BTW, I wouldn't use a runtime test with clearly undefined behavior,
> especially not if it tests what the outcome of that UB is.

Ah, indeed looks like a dup.  Let's go with your version which had
feedback from Bernd already.  Might want to add my testcase (w/o the
runtime outcome test).

Richard.

> > 2016-02-12  Richard Biener  
> > 
> > PR rtl-optimization/69771
> > * optabs.c (expand_binop_directly): Properly zero-/sign-extend
> > VOIDmode operands.
> > 
> > * gcc.dg/torture/pr69771.c: New testcase.
> 
>   Jakub


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 01:48:36PM +0100, Richard Biener wrote:
> But my patch should deal with all this.
> 
> -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> -  if (xmode1 != VOIDmode && xmode1 != mode1)
> +  mode1 = GET_MODE (xop1);
> +  if (xmode1 != mode1)
>  {
>xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
>mode1 = xmode1;
> 
> so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> do convert_modes.

The case I'm worried about is if xop1 is a constant in narrower
mode (let's say QImode), e.g. -15, unsignedp is 1, and xmode1 is
wider.  Then previously we'd zero extend it, thus get
0xf1, but with your patch we'll end up with -15 instead,
because convert_modes will be called e.g. with (SImode, VOIDmode, xop1, 1)
instead of (SImode, QImode, xop1, 1).
Dunno if it is just hypothetical or real.

Jakub


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 02:45:40PM +0100, Richard Biener wrote:
> > The case I'm worried about is if xop1 is a constant in narrower
> > mode (let's say QImode), e.g. -15, unsignedp is 1, and xmode1 is
> > wider.  Then previously we'd zero extend it, thus get
> > 0xf1, but with your patch we'll end up with -15 instead,
> > because convert_modes will be called e.g. with (SImode, VOIDmode, xop1, 1)
> > instead of (SImode, QImode, xop1, 1).
> > Dunno if it is just hypothetical or real.
> 
> But we don't know which mode xop1 was expanded with here.  The only
> way to know would be passing down its original type (or its mode).

Well, most binary ops have the requirement that both modes are the same,
which is why most of the expansion APIs for them pass around just a single
mode, not two modes (or 3, if even the result could have different mode).
And in that case we better have expanded the CONST_INTs with the right mode
already.  Just shifts are special.

> So yes, that case is sth to worry about (for all operations that
> can arrive in expand_binop_directly which can have different mode
> operands).
> 
> Also note that unsignedp doesn't apply to op1 for shifts, only to op0.
> So eventually we shouldn't (ab-)use expand_binop for expanding shifts
> at all...

Perhaps; but please look at the latest patch I've posted, which IMHO does
what your patch does for shift/rorate xop1 only, and keeps doing what it
used to do otherwise.

Jakub


Re: [PATCH ARM] RFC: PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-02-12 Thread Jiong Wang



On 12/02/16 15:02, Jiong Wang wrote:



On 12/02/16 14:56, Charles Baylis wrote:

This is encountered when building an allyesconfig Linux kernel because
the Linux build system generates very large sections by partial
linking a large number of object files. This causes link failures


I have tried latest BFD linker? I suspect the following patch has 
fixed this


  https://sourceware.org/ml/binutils/2016-01/msg00160.html


Ops, this is ARM32, not AArch64..



[PATCH] Add testcase for ICE in assemble_integer

2016-02-12 Thread Marek Polacek
We've got a report that GCC is crashing when building a package on i686;
there was an ICE in assemble_integer.  Turned out this ICE was already fixed
by r233216, so I reduced it and would like to add it to the testsuite.

Tested on x86_64-linux and i686-linux, ok for trunk?

2016-02-12  Marek Polacek  

* g++.dg/torture/init-list1.C: New.

diff --git gcc/testsuite/g++.dg/torture/init-list1.C 
gcc/testsuite/g++.dg/torture/init-list1.C
index e69de29..b8abf6a 100644
--- gcc/testsuite/g++.dg/torture/init-list1.C
+++ gcc/testsuite/g++.dg/torture/init-list1.C
@@ -0,0 +1,6 @@
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+struct S {
+  long long l;
+} s {(long long) };

Marek


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 02:47 PM, Richard Biener wrote:

Another possibility, only do the convert_modes from VOIDmode for
shift_optab_p's xop1, and keep doing what we've done before otherwise.


That looks like a very targeted and safe fix indeed.


You two can obviously go ahead and sort this out, I'll just make one 
more comment. What attracted me to Richard's patch was its clarity and 
lack of potential to confuse readers.



-  machine_mode mode0, mode1, tmp_mode;
+  machine_mode mode0, mode1 = mode, tmp_mode;



if (!shift_optab_p (binoptab))
  xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  else
+mode1 = VOIDmode;



-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1;
if (xmode1 != VOIDmode && xmode1 != mode1)
  {


Here, things aren't so clear, and the fact that the mode1 calculation 
now differs from the mode0 one may be overlooked by someone in the future.


Rather than use codes like "mode variable is VOIDmode", I'd prefer to 
use booleans with descriptive names, like "op1_may_need_conversion".



Bernd



Re: Fix c/69522, memory management issue in c-parser

2016-02-12 Thread Marek Polacek
On Fri, Feb 12, 2016 at 02:28:39PM +0100, Bernd Schmidt wrote:
> On 02/12/2016 02:26 PM, Marek Polacek wrote:
> >On Fri, Feb 12, 2016 at 02:17:19PM +0100, Andreas Schwab wrote:
> >>FAIL: gcc.dg/pr69522.c (test for excess errors)
> >>Excess errors:
> >>/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:2:8: error: 
> >>struct has no members [-Wpedantic]
> >>/daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:9:8: error: 
> >>ISO C forbids empty initializer braces [-Wpedantic]
> >
> >Bernd, ok to fix this with the following?
> >
> >2016-02-12  Marek Polacek  <pola...@redhat.com>
> >
> > * gcc.dg/pr69522.c: Add empty dg-options.
> 
> What's causing the problem there, -Wpedantic? Maybe add the opposite rather
> than an empty dg-options? Either way's fine, sorry about the breakage - I
> thought I had the testcase applied during the test run, but apparently not.

Without dg-options we test with -ansi (aka -std=c89) -pedantic-errors, hence
the error.  With empty dg-options we test without these two options.  Empty
dg-options are quite common in the testsuite.  I'll go ahead with the patch.

Marek


Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener
On Fri, 12 Feb 2016, Jakub Jelinek wrote:

> On Fri, Feb 12, 2016 at 01:48:36PM +0100, Richard Biener wrote:
> > But my patch should deal with all this.
> > 
> > -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> > -  if (xmode1 != VOIDmode && xmode1 != mode1)
> > +  mode1 = GET_MODE (xop1);
> > +  if (xmode1 != mode1)
> >  {
> >xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
> >mode1 = xmode1;
> > 
> > so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> > But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> > do convert_modes.
> 
> The case I'm worried about is if xop1 is a constant in narrower
> mode (let's say QImode), e.g. -15, unsignedp is 1, and xmode1 is
> wider.  Then previously we'd zero extend it, thus get
> 0xf1, but with your patch we'll end up with -15 instead,
> because convert_modes will be called e.g. with (SImode, VOIDmode, xop1, 1)
> instead of (SImode, QImode, xop1, 1).
> Dunno if it is just hypothetical or real.

But we don't know which mode xop1 was expanded with here.  The only
way to know would be passing down its original type (or its mode).

That's the general issue with our modeless CONST_INTs.

So yes, that case is sth to worry about (for all operations that
can arrive in expand_binop_directly which can have different mode
operands).

Also note that unsignedp doesn't apply to op1 for shifts, only to op0.
So eventually we shouldn't (ab-)use expand_binop for expanding shifts
at all...

Richard.


Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-12 Thread Jiong Wang



On 12/02/16 13:33, Bernd Schmidt wrote:

On 02/12/2016 02:18 PM, Jiong Wang wrote:


PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single
SET,
also take other side effects into account.


Will it be better that we don't remove the insn if it has side-effect
instead of don't record the equiv?

This can offer more equiv for later rtl optimization?


It's an option I considered, but for stage4 this seems like the safest 
fix. It's not like this is a very common problem, the effect should be 
negligible.


I see, thanks for the explanation.

From my quick test, this patch actually generate code slightly better.

The interesting thing I found is if we still initialize the equiv, then
a later use in the testcase will be rematerialized, but the equiv
constant -1044200508 is forced into memory, thus the rematerialization
is a load from memory. Instead, if we don't initialize the equiv then
the register contains the reusable value will be kept alive.




Bernd





Re: [wwwdocs] Mention flexible array type and mangling change

2016-02-12 Thread Marek Polacek
On Thu, Feb 11, 2016 at 02:17:43PM -0700, Martin Sebor wrote:
> >>Actually, there is one other thing that might be wort mentioning
> >>about flexible array members.
> >>
> >>The type and mangling of flexible array members has changed.  While
> >>in GCC 5 and prior the type of a flexible array member is an array
> >>of zero elements (a GCC extension), in 6 it is that of an array of
> >>an unspecified bound (i.e., T[] as opposed to T[0]).  This is
> >>a silent ABI change with no -fabi-version/-Wabi option.
> >
> >Aha.  I think you're better-suited to document this than I am.
> 
> Sure.  Here's the added text.  Please let me know if it's good to
> check in.
> 
> Martin
> 
> 
> Index: htdocs/gcc-6/porting_to.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
> retrieving revision 1.12
> diff -u -r1.12 porting_to.html
> --- htdocs/gcc-6/porting_to.html  11 Feb 2016 18:56:15 -  1.12
> +++ htdocs/gcc-6/porting_to.html  11 Feb 2016 21:15:30 -
> @@ -315,6 +315,15 @@
>  };
>  
> 
> +
> +Finally, the type and mangling of flexible array members has changed
> +from previous releases.  While in GCC 5 and prior the type of a flexible
> +array member is an array of zero elements (a GCC extension), in GCC 6 it
> +is that of an array of an unspecified bound (i.e., T[] as opposed
> +to T[0]).  This is a silent ABI change with no corresponding
> +-fabi-version or -Wabi option to disable or warn about.
> +
> +

This seems ok to me, thanks.

Marek


Re: [PR66726] Fixe regression caused by Factor conversion out of COND_EXPR

2016-02-12 Thread H.J. Lu
On Thu, Feb 11, 2016 at 10:18 PM, Markus Trippelsdorf
 wrote:
> On 2016.02.08 at 09:49 -0700, Jeff Law wrote:
>> On 01/18/2016 08:52 PM, Kugan wrote:
>> >
>> >2016-01-19  Kugan Vivekanandarajah  
>> >
>> > PR middle-end/66726
>> > * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt
>> > whose result is used in PHI.
>> > (maybe_optimize_range_tests): Likewise.
>> > (final_range_test_p): Lokweise.
>> >
>> Otherwise this looks OK for the trunk.  It really hasn't changed much since
>> the version from July.  And while the PR is not marked as such, this is a
>> code quality regression fix for targets with a BRANCH_COST > 1.
>
> This causes LTO/PGO bootstrap on ppc64le to ICE all over the place:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781
>

On ia32, it also caused:

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

and

FAIL: libgomp.oacc-fortran/collapse-4.f90 -DACC_DEVICE_TYPE_host=1
-DACC_MEM_SHARED=1  -O1  execution test
FAIL: libgomp.oacc-fortran/collapse-5.f90 -DACC_DEVICE_TYPE_host=1
-DACC_MEM_SHARED=1  -O1  execution test

which were compiled into infinite loop.

-- 
H.J.


Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 02:18 PM, Jiong Wang wrote:


PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single
SET,
also take other side effects into account.


Will it be better that we don't remove the insn if it has side-effect
instead of don't record the equiv?

This can offer more equiv for later rtl optimization?


It's an option I considered, but for stage4 this seems like the safest 
fix. It's not like this is a very common problem, the effect should be 
negligible.



Bernd



Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Richard Biener
On Fri, 12 Feb 2016, Jakub Jelinek wrote:

> On Fri, Feb 12, 2016 at 01:50:08PM +0100, Richard Biener wrote:
> > > -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> > > -  if (xmode1 != VOIDmode && xmode1 != mode1)
> > > +  mode1 = GET_MODE (xop1);
> > > +  if (xmode1 != mode1)
> > >  {
> > >xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
> > >mode1 = xmode1;
> > > 
> > > so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> > > But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> > > do convert_modes.
> > > 
> > > The only thing that can (hopefully not) happen is that xmode1
> > > is VOIDmode but mode1 is not (maybe removing the xmode1 != VOIDmode
> > > check is a bit too optimistic here).  Not sure if there can be
> > > valid patterns requesting a VOIDmode op ... (and not only accept
> > > CONST_INTs).
> > 
> > Oh, bootstrap & testing went fine on x86_64-unknown-linux-gnu.
> > 
> > If we can agree on the patch then I'll pick up the testcase from
> > your patch (and adjust mine).
> 
> Another possibility, only do the convert_modes from VOIDmode for
> shift_optab_p's xop1, and keep doing what we've done before otherwise.

That looks like a very targeted and safe fix indeed.

Richard.

> 2016-02-12  Jakub Jelinek  
> 
>   PR rtl-optimization/69764
>   PR rtl-optimization/69771
>   * optabs.c (expand_binop_directly): For shift_optab_p, force
>   convert_modes with VOIDmode if xop1 has VOIDmode.
> 
>   * c-c++-common/pr69764.c: New test.
>   * gcc.dg/torture/pr69771.c: New testcase.
> 
> --- gcc/optabs.c.jj   2016-02-12 00:50:30.410240366 +0100
> +++ gcc/optabs.c  2016-02-12 10:33:32.743492532 +0100
> @@ -988,7 +988,7 @@ expand_binop_directly (machine_mode mode
> from_mode, 1);
>machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
>machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
> -  machine_mode mode0, mode1, tmp_mode;
> +  machine_mode mode0, mode1 = mode, tmp_mode;
>struct expand_operand ops[3];
>bool commutative_p;
>rtx_insn *pat;
> @@ -1006,6 +1006,8 @@ expand_binop_directly (machine_mode mode
>xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
>if (!shift_optab_p (binoptab))
>  xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
> +  else
> +mode1 = VOIDmode;
>  
>/* In case the insn wants input operands in modes different from
>   those of the actual operands, convert the operands.  It would
> @@ -1020,7 +1020,7 @@ expand_binop_directly (machine_mode mode
>mode0 = xmode0;
>  }
>  
> -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> +  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1;
>if (xmode1 != VOIDmode && xmode1 != mode1)
>  {
>xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
> --- gcc/testsuite/c-c++-common/pr69764.c.jj   2016-02-12 10:27:34.522587995 
> +0100
> +++ gcc/testsuite/c-c++-common/pr69764.c  2016-02-12 10:27:34.522587995 
> +0100
> @@ -0,0 +1,38 @@
> +/* PR rtl-optimization/69764 */
> +/* { dg-do compile { target int32plus } } */
> +
> +unsigned char
> +fn1 (unsigned char a)
> +{
> +  return a >> ~6;/* { dg-warning "right shift count is negative" } */
> +}
> +
> +unsigned short
> +fn2 (unsigned short a)
> +{
> +  return a >> ~6;/* { dg-warning "right shift count is negative" } */
> +}
> +
> +unsigned int
> +fn3 (unsigned int a)
> +{
> +  return a >> ~6;/* { dg-warning "right shift count is negative" } */
> +}
> +
> +unsigned char
> +fn4 (unsigned char a)
> +{
> +  return a >> 0xff03;/* { dg-warning "right shift count >= width of 
> type" } */
> +}
> +
> +unsigned short
> +fn5 (unsigned short a)
> +{
> +  return a >> 0xff03;/* { dg-warning "right shift count >= width of 
> type" } */
> +}
> +
> +unsigned int
> +fn6 (unsigned int a)
> +{
> +  return a >> 0xff03;/* { dg-warning "right shift count >= width of 
> type" } */
> +}
> --- gcc/testsuite/gcc.dg/torture/pr69771.c.jj 2016-02-12 10:27:34.522587995 
> +0100
> +++ gcc/testsuite/gcc.dg/torture/pr69771.c2016-02-12 10:27:34.522587995 
> +0100
> @@ -0,0 +1,12 @@
> +/* PR rtl-optimization/69771 */
> +/* { dg-do compile } */
> +
> +unsigned char a = 5, c;
> +unsigned short b = 0;
> +unsigned d = 0x76543210;
> +
> +void
> +foo (void)
> +{
> +  c = d >> ~(a || ~b);   /* { dg-warning "shift count is negative" } */
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-12 Thread Ulrich Weigand
(Replying to multiple messages at once ...)

Michael Meissner wrote:

> At the present time, we do not allow DImode to go into Altivec
> registers. Mostly it was due to reload complications in the power8 time frame,
> where we didn't have d-form addressing for the Altivec registers and
> interference with the other DImode reload patterns, but also I felt I would
> need to go through the main rs6000.md to look for the various DImode patterns
> to see if they needed to be updated.  At some I hoped to extend this, so I
> added the "wi" and "wj" constraints to be used whenever the mode is DImode.
> "wi" is the constraint for the VSX register class DImode can go in
> (i.e. currently FLOAT=5FREGS), and "wj" is the VSX register class DImode can 
> go
> in if we have 64-bit direct move.

I see.  I hadn't noticed the restriction on DImode, sorry.  I think that in
general, it would be a good idea to allow DImode wherever DFmode is allowed,
since the same instructions should be able to handle both.  But given the
unexpected problems that seem to turn up whenever we want to allow more modes
in more registers, this is probably better for stage 1 than right now ...
 
> The TFmode thing was a hack.  It was the only way I could see getting two
> registers without a lot of hair.  Since TFmode is also restricted to FPR_REGS,
> you could use %L on it.  When I was writing it, I really wished we had a 
> reload
> pattern to get more than one temporary, but we didn't, so I went for TFmode to
> get 2 FPR registers.  Given the replacement pattern no longer uses a TFmode
> temporary, it can go in any appropriate register.

Again, it would probably make sense to allow TFmode (when it is double double)
in any pair of registers where DFmode is allowed, since the type *is* just a
pair of DFmode values.  Again, really more a stage 1 matter ...

> On Thu, Feb 11, 2016 at 07:38:15PM +0100, Ulrich Weigand wrote:
> > It's not a bug, it's deliberate behavior.  The reload registers allocated
> > for secondary reload clobbers may overlap the destination, since in many
> > cases you simply move the input to the reload register, and then the
> > reload register to the destination (where the latter move can be a no-op
> > if it is possible to allocate the reload register and the destination
> > into the same physical register).  If you need two separate intermediate
> > values, you need to allocate a secondary reload register of a larger
> > mode (as is already done in the pattern).
> 
> This is one of the cases I wished the reload support had the ability to
> allocate 2 scratch temporaries instead of 1.  As I said in my other message,
> TFmode was a hack to get two registers to use.

Yes, it would be nice if we could specify multiple scratch registers.  That's
a long-standing FIXME in the reload code:

  /* ??? It would be useful to be able to handle only two, or more than
 three, operands, but for now we can only handle the case of having
 exactly three: output, input and one temp/scratch.  */

Unfortunately, given the structure of reload, that is difficult to fix.

> On Fri, Feb 12, 2016 at 08:54:19AM +1030, Alan Modra wrote:
> > Another concern I had about this, besides using %L in asm output (what
> > forces TFmode to use just fprs?), is what happens when we're using
> > IEEE 128-bit floats?  In that case it looks like we'd get just one reg.
> 
> Good point that it breaks if the default long double (TFmode) type is IEEE
> 128-bit floating point.  We would need to have two patterns, one that uses
> TFmode and one that uses IFmode.  I wrote the power8 direct move stuff before
> going down the road of IEEE 128-bit floating point.

Right.  It's a bit unfortunate that we can't just use IFmode unconditionally,
but it seems rs6000_scalar_mode_supported_p (IFmode) may return false, and
then we probably shouldn't be using it.

Another option might be to use TDmode to allocate a scratch register pair.


David Edelsohn wrote:

> Good question: is p8_mtvsrd really necessary if the movdi_internal64
> is updated to use the correct constraints?
> 
> The patch definitely is going in the right direction.  Can we remove
> more unnecessary bits?

Given Mike's reply above, I don't think it is simply a matter of updating
constraints in the one pattern.  Rather, enabling DImode in all vector
registers would require a change to rs6000_hard_regno_mode_ok and then
reviewing all DImode patterns to make sure they're still compatible.

As I said above, that's probably more of a stage 1 effort.

As long as that is not done, my original suggestion here:

> Maybe instead have p8_mtvsrd use DImode as output (instead of
> DFmode), which shouldn't be any harder to use in the
> reload_vsx_from_gpr splitter, and keep using the
> vsx_xscvspdpn_directmove pattern?

isn't actually possible.  Instead, one other option might be to
simply do the partial moves all in *DFmode* instead of DImode.
That way, we can make use of the fact that DFmode is already

[PATCH ARM] RFC: PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-02-12 Thread Charles Baylis
When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
function are not generated as long calls.

This is encountered when building an allyesconfig Linux kernel because
the Linux build system generates very large sections by partial
linking a large number of object files. This causes link failures,
which don't go away with -mlong-calls due to this bug. (However, with
this patch linking still fails due to calls in inline asm)

For example:

extern void g(void);
int f() { g(); return 0; }

compiles to:

push{r4, lr}
push{lr}
bl  __gnu_mcount_nc;// not a long call
ldr r3, .L2
blx r3 ;// a long call to g()
mov r0, #0
pop {r4, pc}

The call to __gnu_mcount_nc is generated from ARM_FUNCTION_PROFILER in
config/arm/bpabi.h.

For targets without MOVW/MOVT, the long call sequence requires a load
from the literal pool, and it is too late to set up a literal pool
entry from within ARM_FUNCTION_PROFILER. My approach to fix this is to
modify the prologue generation to load the address of __gnu_mcount_nc
into ip, so that it is ready when the call is generated.

This patch only implements the fix for ARM and Thumb-2. A similar fix
is possible for Thumb-1, but requires more slightly complex changes to
the prologue generation to make sure there is a low register
available.

This feels like a bit of a hack to me, so ideas for a cleaner solution
are welcome, if none, is this acceptable for trunk now, or should it
wait until GCC 7?
From 34993396a43fcfc263db5b02b2d1837c490f52ad Mon Sep 17 00:00:00 2001
From: Charles Baylis 
Date: Thu, 11 Feb 2016 18:07:00 +
Subject: [PATCH] [ARM] PR69770 fix -mlong-calls with -pg

gcc/ChangeLog:

2016-02-12  Charles Baylis  

	* config/arm/arm.c (arm_expand_prologue): Load address of
	__gnu_mcount_nc in r12 if profiling and long calls are enabled.
* config/arm/bpabi.h (ARM_FUNCTION_PROFILER): Emit long call to
	__gnu_mcount_nc long calls are enabled.
	(ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New define.

gcc/testsuite/ChangeLog:

2016-02-12  Charles Baylis  

* gcc.target/arm/pr69770.c: New test.

Change-Id: I4c639a5edf32fa8c67324d37faee1cb4ddd57a5c

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 27aecf7..9ce9a58 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -21739,6 +21739,15 @@ arm_expand_prologue (void)
   arm_load_pic_register (mask);
 }
 
+  if (crtl->profile && TARGET_LONG_CALLS
+  && ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS)
+{
+  rtx tmp = gen_rtx_SET (gen_rtx_REG (SImode, IP_REGNUM),
+			 gen_rtx_SYMBOL_REF (Pmode, "__gnu_mcount_nc"));
+  emit_insn (tmp);
+  emit_insn (gen_rtx_USE (VOIDmode, gen_rtx_REG (SImode, IP_REGNUM)));
+}
+
   /* If we are profiling, make sure no instructions are scheduled before
  the call to mcount.  Similarly if the user has requested no
  scheduling in the prolog.  Similarly if we want non-call exceptions
diff --git a/gcc/config/arm/bpabi.h b/gcc/config/arm/bpabi.h
index 82128ef..b734a24 100644
--- a/gcc/config/arm/bpabi.h
+++ b/gcc/config/arm/bpabi.h
@@ -173,11 +173,21 @@
 
 #undef  NO_PROFILE_COUNTERS
 #define NO_PROFILE_COUNTERS 1
+#undef  ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS
+#define ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS 1
 #undef  ARM_FUNCTION_PROFILER
 #define ARM_FUNCTION_PROFILER(STREAM, LABELNO)  			\
 {	\
-  fprintf (STREAM, "\tpush\t{lr}\n");	\
-  fprintf (STREAM, "\tbl\t__gnu_mcount_nc\n");\
+  if (TARGET_LONG_CALLS && TARGET_32BIT)\
+  { 	\
+fprintf (STREAM, "\tpush\t{lr}\n");	\
+/* arm_expand_prolog() has already set up ip to contain the */	\
+/* address of __gnu_mcount_nc.  */	\
+fprintf (STREAM, "\tblx\tip\n");	\
+  } else {\
+fprintf (STREAM, "\tpush\t{lr}\n");	\
+fprintf (STREAM, "\tbl\t__gnu_mcount_nc\n");			\
+  }	\
 }
 
 #undef SUBTARGET_FRAME_POINTER_REQUIRED
diff --git a/gcc/testsuite/gcc.target/arm/pr69770.c b/gcc/testsuite/gcc.target/arm/pr69770.c
new file mode 100644
index 000..61e5c6d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr69770.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-pg -mlong-calls" } */
+
+extern void g(void);
+
+int f() { g(); return 0; }
+
+/* { dg-final { scan-assembler-not "bl\[ \t\]+__gnu_mcount_nc" } } */
+/* { dg-final { scan-assembler "__gnu_mcount_nc" } } */
-- 
1.9.1



Re: [PATCH ARM] RFC: PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-02-12 Thread Jiong Wang



On 12/02/16 14:56, Charles Baylis wrote:

This is encountered when building an allyesconfig Linux kernel because
the Linux build system generates very large sections by partial
linking a large number of object files. This causes link failures


I have tried latest BFD linker? I suspect the following patch has fixed 
this


  https://sourceware.org/ml/binutils/2016-01/msg00160.html



New Esperanto PO file for 'cpplib' (version 6.1-b20160131)

2016-02-12 Thread Translation Project Robot
Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Esperanto team of translators.  The file is available at:

http://translationproject.org/latest/cpplib/eo.po

(This file, 'cpplib-6.1-b20160131.eo.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

http://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

http://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.




[PATCH] Fix ICE when expanding incorrect shift counts (PR rtl-optimization/69764, take 2)

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 02:08:53AM +0100, Bernd Schmidt wrote:
> Isn't the problem just that shifts are unusual as binops, in that the two
> operands can have different modes? We have the appropriate mode for the
> shift count in xmode1, so I think we should just always rebuild a CONST_INT
> in that mode. As in,
> 
>   else if (CONST_INT_P (xop1))
> /* Shifts can have different modes for the shift count, and the caller
>might not have taken this into account when generating an integer.
> */
> xop1 = gen_int_mode (INTVAL (xop1), xmode1);
> 
> Ok with that change if it passes.

That works, but I need to also ensure that convert_modes is not called on
it, as that would again assume the constant is in mode, while it is now in
xmode1.  So, is this ok for trunk (if it passes bootstrap/regtest)?

2016-02-12  Jakub Jelinek  

PR rtl-optimization/69764
* optabs.c (expand_binop_directly): For shift_optab_p, if xop1
is CONST_INT, recreate it in xmode1 and ensure convert_modes
will not be called on it.

* c-c++-common/pr69764.c: New test.

--- gcc/optabs.c.jj 2016-02-12 00:50:30.410240366 +0100
+++ gcc/optabs.c2016-02-12 10:33:32.743492532 +0100
@@ -988,7 +988,7 @@ expand_binop_directly (machine_mode mode
  from_mode, 1);
   machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
   machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
-  machine_mode mode0, mode1, tmp_mode;
+  machine_mode mode0, mode1 = VOIDmode, tmp_mode;
   struct expand_operand ops[3];
   bool commutative_p;
   rtx_insn *pat;
@@ -1006,6 +1006,13 @@ expand_binop_directly (machine_mode mode
   xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
   if (!shift_optab_p (binoptab))
 xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  else if (CONST_INT_P (xop1))
+{
+  /* Shifts can have different modes for the shift count, and the caller
+might not have taken this into account when generating an integer.  */
+  xop1 = gen_int_mode (INTVAL (xop1), xmode1);
+  mode1 = xmode1;
+}
 
   /* In case the insn wants input operands in modes different from
  those of the actual operands, convert the operands.  It would
@@ -1020,7 +1027,8 @@ expand_binop_directly (machine_mode mode
   mode0 = xmode0;
 }
 
-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  if (mode1 == VOIDmode)
+mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
   if (xmode1 != VOIDmode && xmode1 != mode1)
 {
   xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
--- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 10:27:34.522587995 
+0100
+++ gcc/testsuite/c-c++-common/pr69764.c2016-02-12 10:27:34.522587995 
+0100
@@ -0,0 +1,38 @@
+/* PR rtl-optimization/69764 */
+/* { dg-do compile { target int32plus } } */
+
+unsigned char
+fn1 (unsigned char a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned short
+fn2 (unsigned short a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned int
+fn3 (unsigned int a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned char
+fn4 (unsigned char a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned short
+fn5 (unsigned short a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned int
+fn6 (unsigned int a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}


Jakub


Re: libgcc: On AIX, increase chances to find landing pads for exceptions

2016-02-12 Thread Michael Haubenwallner

On 02/10/2016 02:27 PM, David Edelsohn wrote:
> On Wed, Feb 10, 2016 at 1:52 AM, Michael Haubenwallner
>  wrote:
>>
>> On 02/08/2016 02:59 PM, David Edelsohn wrote:
>>> Runtime linking is disabled by default on AIX, and I disabled it for 
>>> libstdc++.
>>
>> For large applications mainly developed on/for Linux I do prefer/need
>> runtime linking even on AIX. Still I do believe there is no AIX-based
>> reason to leave runtime linking disabled, but build-/linktime issues
>> instead that cause things to fail with runtime linking enabled.
> 
> What do you mean by the term "runtime linking"?  Runtime linking means
> runtime overloading of symbols -- preloading -- not dynamic linking
> and loading.  dlopen does not require runtime linking.  There also are
> issues with searching for shared objects with .a or .so file
> extension, but that can be addressed separately.

We're actually talking about the same "runtime linking" understanding.

> Runtime linking causes every global, exported function call to be
> invoked through indirect glue code.  And each function must be
> inserted into the TOC.  The indirect call overhead is very expensive,
> and potential TOC overflow can cause even more performance
> degradation.

Well, I'm in the lucky situation to not need to care for performance
that much. Don't get me wrong - I don't want to change any default
settings, but keep symbol-overloading at runtime just working if
requested for the final executable (by linking with -brtl).

What I indeed do not know: Is the performance overhead still active
even when the final executable is _not_ linked with -brtl, but the
shared libraries only are linked with -G ?

> Your statement of no AIX-based reason to leave runtime linking
> disabled is fundamentally flawed.

Agreed performance wise. But still I fail to see a functional reason.

>>> There are two remaining issues:
>>>
>>> 1) FDEs with overlapping ranges causing problems with exceptions.  I'm
>>> not sure of the best way to work around this.  Your patch is one
>>> possible solution.
>>
>> This patch is not meant as a final solution, but to improve current
>> situation with broken build systems exporting even _GLOBAL__ symbols.
>> I'm about to prepare another libtool patch to fix that one.
>>
>>> 2) AIX linker garbage collection conflicting with scanning for
>>> symbols.  collect2 scanning needs to better emulate SVR4 linker
>>> semantics for object files and archives.
>>
>> Probably collect2 should filter the symbol list originating in either
>> an explicit -bexport:file or the -bexpall/-bexpfull flags and pass the
>> resulting symbol list as explicit -bexport:file only to the AIX linker?
> 
> -bexpall and -bexpfull cause numerous problem by re-exporting symbols.
> 
> All of the suggestions will produce programs that function, but have
> severe performance impacts and unintended consequences that you seem
> to be ignoring.

Erm... I don't think so - what I mean is: Probably collect2 should _not_
passthrough the -bexpall/-bexpfull/-bexport:file flags to the linker,
but itself create an export file and pass this one _instead_.
However, this feels like a workaround for broken build systems - which
do provide the breaking -bexp* flags to gcc (collect2) already.

Thanks!
/haubi/

> 
> - David
> 
>>
>> /haubi/
>>
>>>
>>> Thanks, David
>>>
>>>
>>> On Mon, Feb 8, 2016 at 7:14 AM, Michael Haubenwallner
>>>  wrote:
 Hi David,

 still experiencing exception-not-caught problems with gcc-4.2.4 on AIX
 leads me to some patch proposed in http://gcc.gnu.org/PR13878 back in
 2004 already, ought to be fixed by some different commit since 3.4.0.

 As long as build systems (even libtool right now) on AIX do export these
 _GLOBAL__* symbols from shared libraries, overlapping frame-base address
 ranges may become registered, even if newer gcc (seen with 4.8) does name
 the FDE symbols more complex to reduce these chances.

 But still, just think of linking some static library into multiple shared
 libraries and/or the main executable. Or sometimes there is just need for
 some hackery to override a shared object's implementation detail and rely
 on runtime linking to do the override at runtime.

 Agreed both is "wrong" to some degree, but the larger an application is,
 the higher is the chance for this to happen.

 Thoughts?

 Thanks!
 /haubi/


Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.

2016-02-12 Thread Jeff Law

On 02/12/2016 02:04 AM, Richard Biener wrote:


So what am I missing here?  Is there any kind of aliasing issues I need to
be aware of?  Any other dragons lurking?


I think what you are missing is that you'll "optimize"

_Complex double x, y;

void foo (void)
{
   x = y;
}

then but dependent on the targets capability DCmode moves might be
cheaper than four DFmode moves and nothing will combine them together
again.
True.  In today's world where we can load/store large objects 
efficiently, that effect probably dwarfs anything we get from handling 
the trivial/artificial cases with more lowering.





In complex lowering we're careful to only "optimize" when we know how to
extract components in an efficient way (well, mostly).
Right.  If I understood the stuff in tree-complex, it's mostly concerned 
with lowering when the complex object is actually a degenerate.




That the DSE opportunities in complex-[45].c are exposed if you lower
the aggregate inits is obvious but the lowering is only a workaround for
our lack of handling for this case.  I think the specific cases can be
easily made work by enhancing tree DSE in dse_possible_dead_store_p
to pick up partial kills by adjusting *ref (its offset/size) - keeping the
original ref for picking up uses.

But really we simply need a better DSE algorithm.
So the way to fix DSE is to keep the existing algorithm and track the 
hunks of Complex/aggregates that have been set a second time.


My first thought was to implement this as an inverted bitmap.  ie, set 
it to 1 for every byte in the complex/aggregate that is set by the first 
store.  Clear bits for each byte in subsequent stores to the pieces. 
If the bitmap reaches an empty state, then the initial store is dead.


Adjusting *ref could work too, but would probably be painful if the 
subsequent stores don't happen in a convenient order.



jeff


Re: bswap PRs 69714, 67781

2016-02-12 Thread Jeff Law

On 02/12/2016 09:28 AM, Bernd Schmidt wrote:

PR69714 is an issue where the bswap pass makes an incorrect
transformation on big-endian targets. The source has a 32-bit bswap, but
PA doesn't have a pattern for that. Still, we recognize that there is a
16-bit bswap involved, and generate code for that - loading the halfword
at offset 2 from the original memory, as per the proper big-endian
correction.

The problem is that we recognized the rotation of the _high_ part, which
is at offset 0 on big-endian. The symbolic number is 0x0304, rather than
0x0102 as it should be. Only the latter form should ever be matched. The
problem is caused by the patch for PR67781, which was intended to solve
a different big-endian problem. Unfortunately, I think it is based on an
incorrect analysis.

The real issue with the PR67781 testcase is in fact the masking loop,
identified by Thomas in comment #7 for 67781.

for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++)
   ;
n->range = rsize;

If we have a value of 0x01020304, but a range of 5, it means that
there's an "invisible" high-order byte that we don't care about. On
little-endian, we can just ignore it. On big-endian, this implies that
the data we're interested in is located at an offset. The code that does
the replacements does not use the offset or bytepos fields, it assumes
that the bytepos always matches that of the load instruction. The only
offset we can introduce is the big-endian correction, but that assumes
we're always dealing with lowparts.

So, I think the correct/conservative fix for both bugs is to revert the
earlier change for PR67781, and then apply the following on top:

--- revert.tree-ssa-math-opts.c2016-02-12 15:22:57.098895058 +0100
+++ tree-ssa-math-opts.c2016-02-12 15:23:08.482228474 +0100
@@ -2473,10 +2473,14 @@ find_bswap_or_nop (gimple *stmt, struct
/* Find real size of result (highest non-zero byte).  */
if (n->base_addr)
  {
-  int rsize;
+  unsigned HOST_WIDE_INT rsize;
uint64_t tmpn;

for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER,
rsize++);
+  if (BYTES_BIG_ENDIAN && n->range != rsize)
+/* This implies an offset, which is currently not handled by
+   bswap_replace.  */
+return NULL;
n->range = rsize;
  }

I'm attaching a full patch. John David Anglin is currently running tests
on PA which may take a while. He's confirmed it fixes his testcase, and
the earlier 67781 testcase seems to be cured as well (only looked at
generated assembly, don't have a full cross environment - Thomas, can
you verify?)
I'm also doing an x86_64 test run. Ok everywhere if it passes? I'm also
attaching a version of the testcase (LGPL, from ffmpeg) which I'd also
apply to gcc.dg/torture if David (or Thomas) can confirm it works on a
big-endian target after the fix.

Yes.  Seems like the safe thing to do here.

Jeff


Re: [Patch X86_64]: Minor changes to znver1 pipe reservations.

2016-02-12 Thread Uros Bizjak
On Fri, Feb 12, 2016 at 5:31 PM, Kumar, Venkataramanan
 wrote:
> Hi Maintainers,
>
> Below patch does some minor changes to pipe reservations in znver1.md.
> GCC bootstrap completed.
>
> GCC regression testing underway. Ok for trunk if the testing passes?
>
>
> ChangeLog
> -
> 2016-02-12  Venkataramanan Kumar  
>
> *  config/i386/znver1.md
> (znver1_pop, znver1_pop_mem,
> znver1_load_imov_double_store,
> znver1_load_imov_direct_store,
> znver1_load_imov_direct_load,
> znver1_load_imov_double_load): Add new.
> (znver1_insn, znver1_insn_load): Add icmov type.
> (znver1_sseavx_fma,
> znver1_sseavx_fma_load,
> znver1_avx256_fma,
> znver1_avx256_fma_load): Fix pipe usage.

OK for mainline.

Thanks,
Uros.

> --- Patch ---
> diff --git a/gcc/config/i386/znver1.md b/gcc/config/i386/znver1.md
> index b7fcf6c..e6babc7 100644
> --- a/gcc/config/i386/znver1.md
> +++ b/gcc/config/i386/znver1.md
> @@ -102,6 +102,18 @@
>(eq_attr "memory" "both")))
>  "znver1-direct,znver1-load,znver1-store")
>
> +(define_insn_reservation "znver1_pop" 4
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "type" "pop")
> +  (eq_attr "memory" "load")))
> +"znver1-direct,znver1-load")
> +
> +(define_insn_reservation "znver1_pop_mem" 4
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "type" "pop")
> +  (eq_attr "memory" "both")))
> +"znver1-direct,znver1-load,znver1-store")
> +
>  ;; Leave
>  (define_insn_reservation "znver1_leave" 1
>  (and (eq_attr "cpu" "znver1")
> @@ -194,26 +206,52 @@
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "znver1_decode" "double")
>(and (eq_attr "type" "imovx")
> -   (eq_attr "memory" "none,load"
> +   (eq_attr "memory" "none"
>  "znver1-double,znver1-ieu")
>
>  (define_insn_reservation "znver1_load_imov_direct" 1
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "type" "imov,imovx")
> -  (eq_attr "memory" "none,load")))
> +  (eq_attr "memory" "none")))
>  "znver1-direct,znver1-ieu")
>
> +(define_insn_reservation "znver1_load_imov_double_store" 2
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "znver1_decode" "double")
> +  (and (eq_attr "type" "imovx")
> +   (eq_attr "memory" "store"
> +"znver1-double,znver1-ieu,znver1-store")
> +
> +(define_insn_reservation "znver1_load_imov_direct_store" 1
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "type" "imov,imovx")
> +  (eq_attr "memory" "store")))
> +  "znver1-direct,znver1-ieu,znver1-store")
> +
> +(define_insn_reservation "znver1_load_imov_double_load" 6
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "znver1_decode" "double")
> +  (and (eq_attr "type" "imovx")
> +   (eq_attr "memory" "load"
> +"znver1-double,znver1-load,znver1-ieu")
> +
> +(define_insn_reservation "znver1_load_imov_direct_load" 5
> +(and (eq_attr "cpu" "znver1")
> + (and (eq_attr "type" "imov,imovx")
> +  (eq_attr "memory" "load")))
> +"znver1-direct,znver1-load,znver1-ieu")
> +
>  ;; INTEGER/GENERAL instructions
>  ;; register/imm operands only: ALU, ICMP, NEG, NOT, ROTATE, ISHIFT, TEST
>  (define_insn_reservation "znver1_insn" 1
>  (and (eq_attr "cpu" "znver1")
> - (and (eq_attr "type" 
> "alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec")
> + (and (eq_attr "type" 
> "alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec,icmov")
>(eq_attr "memory" "none,unknown")))
>  "znver1-direct,znver1-ieu")
>
>  (define_insn_reservation "znver1_insn_load" 5
>  (and (eq_attr "cpu" "znver1")
> - (and (eq_attr "type" 
> "alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec")
> +  

[Patch X86_64]: Minor changes to znver1 pipe reservations.

2016-02-12 Thread Kumar, Venkataramanan
Hi Maintainers,

Below patch does some minor changes to pipe reservations in znver1.md.
GCC bootstrap completed. 

GCC regression testing underway. Ok for trunk if the testing passes?


ChangeLog
-
2016-02-12  Venkataramanan Kumar  

    *  config/i386/znver1.md
    (znver1_pop, znver1_pop_mem,
    znver1_load_imov_double_store,
    znver1_load_imov_direct_store,
znver1_load_imov_direct_load,
    znver1_load_imov_double_load): Add new.
    (znver1_insn, znver1_insn_load): Add icmov type.
    (znver1_sseavx_fma,
    znver1_sseavx_fma_load,
    znver1_avx256_fma,
    znver1_avx256_fma_load): Fix pipe usage.

--- Patch ---
diff --git a/gcc/config/i386/znver1.md b/gcc/config/i386/znver1.md
index b7fcf6c..e6babc7 100644
--- a/gcc/config/i386/znver1.md
+++ b/gcc/config/i386/znver1.md
@@ -102,6 +102,18 @@
   (eq_attr "memory" "both")))
 "znver1-direct,znver1-load,znver1-store")
 
+(define_insn_reservation "znver1_pop" 4
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "type" "pop")
+  (eq_attr "memory" "load")))
+"znver1-direct,znver1-load")
+
+(define_insn_reservation "znver1_pop_mem" 4
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "type" "pop")
+  (eq_attr "memory" "both")))
+"znver1-direct,znver1-load,znver1-store")
+
 ;; Leave
 (define_insn_reservation "znver1_leave" 1
 (and (eq_attr "cpu" "znver1")
@@ -194,26 +206,52 @@
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "znver1_decode" "double")
   (and (eq_attr "type" "imovx")
-   (eq_attr "memory" "none,load"
+   (eq_attr "memory" "none"
 "znver1-double,znver1-ieu")
 
 (define_insn_reservation "znver1_load_imov_direct" 1
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "type" "imov,imovx")
-  (eq_attr "memory" "none,load")))
+  (eq_attr "memory" "none")))
 "znver1-direct,znver1-ieu")
 
+(define_insn_reservation "znver1_load_imov_double_store" 2
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "znver1_decode" "double")
+  (and (eq_attr "type" "imovx")
+   (eq_attr "memory" "store"
+"znver1-double,znver1-ieu,znver1-store")
+
+(define_insn_reservation "znver1_load_imov_direct_store" 1
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "type" "imov,imovx")
+  (eq_attr "memory" "store")))
+  "znver1-direct,znver1-ieu,znver1-store")
+
+(define_insn_reservation "znver1_load_imov_double_load" 6
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "znver1_decode" "double")
+  (and (eq_attr "type" "imovx")
+   (eq_attr "memory" "load"
+"znver1-double,znver1-load,znver1-ieu")
+
+(define_insn_reservation "znver1_load_imov_direct_load" 5
+(and (eq_attr "cpu" "znver1")
+ (and (eq_attr "type" "imov,imovx")
+  (eq_attr "memory" "load")))
+"znver1-direct,znver1-load,znver1-ieu")
+
 ;; INTEGER/GENERAL instructions
 ;; register/imm operands only: ALU, ICMP, NEG, NOT, ROTATE, ISHIFT, TEST
 (define_insn_reservation "znver1_insn" 1
 (and (eq_attr "cpu" "znver1")
- (and (eq_attr "type" 
"alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec")
+ (and (eq_attr "type" 
"alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec,icmov")
   (eq_attr "memory" "none,unknown")))
 "znver1-direct,znver1-ieu")
 
 (define_insn_reservation "znver1_insn_load" 5
 (and (eq_attr "cpu" "znver1")
- (and (eq_attr "type" 
"alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec")
+ (and (eq_attr "type" 
"alu,icmp,negnot,rotate,rotate1,ishift,ishift1,test,setcc,incdec,icmov")
   (eq_attr "memory" "load")))
 "znver1-direct,znver1-load,znver1-ieu")
 
@@ -666,28 +704,28 @@
  (and (eq_attr 

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Bernd Schmidt

So do you prefer e.g. following?  Bootstrapped/regtested on x86_64-linux and
i686-linux.

-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1)
+ ? GET_MODE (xop1) : mode;


Placement of parentheses is wrong for formatting, but otherwise I think 
this patch is good.



Bernd


Re: [PATCH] Add testcase for ICE in assemble_integer

2016-02-12 Thread Bernd Schmidt

On 02/12/2016 03:08 PM, Marek Polacek wrote:

We've got a report that GCC is crashing when building a package on i686;
there was an ICE in assemble_integer.  Turned out this ICE was already fixed
by r233216, so I reduced it and would like to add it to the testsuite.

Tested on x86_64-linux and i686-linux, ok for trunk?

2016-02-12  Marek Polacek  

* g++.dg/torture/init-list1.C: New.


Sure.


Bernd


bswap PRs 69714, 67781

2016-02-12 Thread Bernd Schmidt
PR69714 is an issue where the bswap pass makes an incorrect 
transformation on big-endian targets. The source has a 32-bit bswap, but 
PA doesn't have a pattern for that. Still, we recognize that there is a 
16-bit bswap involved, and generate code for that - loading the halfword 
at offset 2 from the original memory, as per the proper big-endian 
correction.


The problem is that we recognized the rotation of the _high_ part, which 
is at offset 0 on big-endian. The symbolic number is 0x0304, rather than 
0x0102 as it should be. Only the latter form should ever be matched. The 
problem is caused by the patch for PR67781, which was intended to solve 
a different big-endian problem. Unfortunately, I think it is based on an 
incorrect analysis.


The real issue with the PR67781 testcase is in fact the masking loop, 
identified by Thomas in comment #7 for 67781.


for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++)
  ;
n->range = rsize;

If we have a value of 0x01020304, but a range of 5, it means that 
there's an "invisible" high-order byte that we don't care about. On 
little-endian, we can just ignore it. On big-endian, this implies that 
the data we're interested in is located at an offset. The code that does 
the replacements does not use the offset or bytepos fields, it assumes 
that the bytepos always matches that of the load instruction. The only 
offset we can introduce is the big-endian correction, but that assumes 
we're always dealing with lowparts.


So, I think the correct/conservative fix for both bugs is to revert the 
earlier change for PR67781, and then apply the following on top:


--- revert.tree-ssa-math-opts.c 2016-02-12 15:22:57.098895058 +0100
+++ tree-ssa-math-opts.c2016-02-12 15:23:08.482228474 +0100
@@ -2473,10 +2473,14 @@ find_bswap_or_nop (gimple *stmt, struct
   /* Find real size of result (highest non-zero byte).  */
   if (n->base_addr)
 {
-  int rsize;
+  unsigned HOST_WIDE_INT rsize;
   uint64_t tmpn;

   for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, 
rsize++);

+  if (BYTES_BIG_ENDIAN && n->range != rsize)
+   /* This implies an offset, which is currently not handled by
+  bswap_replace.  */
+   return NULL;
   n->range = rsize;
 }

I'm attaching a full patch. John David Anglin is currently running tests 
on PA which may take a while. He's confirmed it fixes his testcase, and 
the earlier 67781 testcase seems to be cured as well (only looked at 
generated assembly, don't have a full cross environment - Thomas, can 
you verify?)
I'm also doing an x86_64 test run. Ok everywhere if it passes? I'm also 
attaching a version of the testcase (LGPL, from ffmpeg) which I'd also 
apply to gcc.dg/torture if David (or Thomas) can confirm it works on a 
big-endian target after the fix.



There is room for more improvement in this code - hopefully we can get 
to that in gcc-7. As mentioned, it does not use the bytepos in the final 
replacement. We could extend the code to recognize smaller sub-bswaps at 
an offset. Currently, only one of the two rotates involved in a 32-bit 
bswap is recognized.


Also, for the crc testcase, the transformation actually pessimizes the 
code by inserting an extra memory load. If we already have loaded a 
larger value, we should build the subpart using a shift and convert.



Bernd
	PR tree-optimization/69714
	* tree-ssa-math-opts.c (find_bswap_or_nop): Revert previous change.
	Return NULL if we have irrelevant high bytes on BIG_ENDIAN.

Index: gcc/tree-ssa-math-opts.c
===
--- gcc/tree-ssa-math-opts.c	(revision 233211)
+++ gcc/tree-ssa-math-opts.c	(working copy)
@@ -2141,11 +2141,9 @@ find_bswap_or_nop_1 (gimple stmt, struct
 static gimple
 find_bswap_or_nop (gimple stmt, struct symbolic_number *n, bool *bswap)
 {
-  unsigned rsize;
-  uint64_t tmpn, mask;
-/* The number which the find_bswap_or_nop_1 result should match in order
-   to have a full byte swap.  The number is shifted to the right
-   according to the size of the symbolic number before using it.  */
+  /* The number which the find_bswap_or_nop_1 result should match in order
+ to have a full byte swap.  The number is shifted to the right
+ according to the size of the symbolic number before using it.  */
   uint64_t cmpxchg = CMPXCHG;
   uint64_t cmpnop = CMPNOP;
 
@@ -2166,38 +2164,28 @@ find_bswap_or_nop (gimple stmt, struct s
 
   /* Find real size of result (highest non-zero byte).  */
   if (n->base_addr)
-for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++);
-  else
-rsize = n->range;
+{
+  unsigned HOST_WIDE_INT rsize;
+  uint64_t tmpn;
 
-  /* Zero out the bits corresponding to untouched bytes in original gimple
- expression.  */
+  for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++);
+  if (BYTES_BIG_ENDIAN && n->range != rsize)
+	/* This implies an offset, 

Re: [PATCH ARM] RFC: PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-02-12 Thread Richard Earnshaw (lists)
On 12/02/16 14:56, Charles Baylis wrote:
> When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
> function are not generated as long calls.
> 
> This is encountered when building an allyesconfig Linux kernel because
> the Linux build system generates very large sections by partial
> linking a large number of object files. This causes link failures,
> which don't go away with -mlong-calls due to this bug. (However, with
> this patch linking still fails due to calls in inline asm)
> 
> For example:
> 
> extern void g(void);
> int f() { g(); return 0; }
> 
> compiles to:
> 
> push{r4, lr}
> push{lr}
> bl  __gnu_mcount_nc;// not a long call

Ouch!  That's left the stack misaligned.  That might be OK iff we can
assume __gnu_mcount_nc is not required to be an ABI-conforming call.

> ldr r3, .L2
> blx r3 ;// a long call to g()
> mov r0, #0
> pop {r4, pc}
> 
> The call to __gnu_mcount_nc is generated from ARM_FUNCTION_PROFILER in
> config/arm/bpabi.h.
> 
> For targets without MOVW/MOVT, the long call sequence requires a load
> from the literal pool, and it is too late to set up a literal pool
> entry from within ARM_FUNCTION_PROFILER. My approach to fix this is to
> modify the prologue generation to load the address of __gnu_mcount_nc
> into ip, so that it is ready when the call is generated.
> 
> This patch only implements the fix for ARM and Thumb-2. A similar fix
> is possible for Thumb-1, but requires more slightly complex changes to
> the prologue generation to make sure there is a low register
> available.

Can you check this with a nested function?  Eg.

int f()
{
  int a;
  int b ()
  {
return a+1;
  }

  a = 1;
  a += b();
  assert (a == 3);
  return a;
}

I think this will break with your patch since the closure will be passed
in IP.

R.

> 
> This feels like a bit of a hack to me, so ideas for a cleaner solution
> are welcome, if none, is this acceptable for trunk now, or should it
> wait until GCC 7?
> 
> 
> 0001-ARM-PR69770-fix-mlong-calls-with-pg.patch
> 
> 
> From 34993396a43fcfc263db5b02b2d1837c490f52ad Mon Sep 17 00:00:00 2001
> From: Charles Baylis 
> Date: Thu, 11 Feb 2016 18:07:00 +
> Subject: [PATCH] [ARM] PR69770 fix -mlong-calls with -pg
> 
> gcc/ChangeLog:
> 
> 2016-02-12  Charles Baylis  
> 
>   * config/arm/arm.c (arm_expand_prologue): Load address of
>   __gnu_mcount_nc in r12 if profiling and long calls are enabled.
> * config/arm/bpabi.h (ARM_FUNCTION_PROFILER): Emit long call to
>   __gnu_mcount_nc long calls are enabled.
>   (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New define.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-02-12  Charles Baylis  
> 
> * gcc.target/arm/pr69770.c: New test.
> 
> Change-Id: I4c639a5edf32fa8c67324d37faee1cb4ddd57a5c
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 27aecf7..9ce9a58 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -21739,6 +21739,15 @@ arm_expand_prologue (void)
>arm_load_pic_register (mask);
>  }
>  
> +  if (crtl->profile && TARGET_LONG_CALLS
> +  && ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS)
> +{
> +  rtx tmp = gen_rtx_SET (gen_rtx_REG (SImode, IP_REGNUM),
> +  gen_rtx_SYMBOL_REF (Pmode, "__gnu_mcount_nc"));
> +  emit_insn (tmp);
> +  emit_insn (gen_rtx_USE (VOIDmode, gen_rtx_REG (SImode, IP_REGNUM)));
> +}
> +
>/* If we are profiling, make sure no instructions are scheduled before
>   the call to mcount.  Similarly if the user has requested no
>   scheduling in the prolog.  Similarly if we want non-call exceptions
> diff --git a/gcc/config/arm/bpabi.h b/gcc/config/arm/bpabi.h
> index 82128ef..b734a24 100644
> --- a/gcc/config/arm/bpabi.h
> +++ b/gcc/config/arm/bpabi.h
> @@ -173,11 +173,21 @@
>  
>  #undef  NO_PROFILE_COUNTERS
>  #define NO_PROFILE_COUNTERS 1
> +#undef  ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS
> +#define ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS 1
>  #undef  ARM_FUNCTION_PROFILER
>  #define ARM_FUNCTION_PROFILER(STREAM, LABELNO)   \
>  {\
> -  fprintf (STREAM, "\tpush\t{lr}\n");
> \
> -  fprintf (STREAM, "\tbl\t__gnu_mcount_nc\n");   
> \
> +  if (TARGET_LONG_CALLS && TARGET_32BIT) \
> +  {  \
> +fprintf (STREAM, "\tpush\t{lr}\n");  
> \
> +/* arm_expand_prolog() has already set up ip to contain the */   \
> +/* address of __gnu_mcount_nc.  */   
> \
> +fprintf (STREAM, "\tblx\tip\n"); \
> +  } else {  

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Jakub Jelinek
On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote:
> >>-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> >>+  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1;
> >>if (xmode1 != VOIDmode && xmode1 != mode1)
> >>  {
> 
> Here, things aren't so clear, and the fact that the mode1 calculation now
> differs from the mode0 one may be overlooked by someone in the future.
> 
> Rather than use codes like "mode variable is VOIDmode", I'd prefer to use
> booleans with descriptive names, like "op1_may_need_conversion".

So do you prefer e.g. following?  Bootstrapped/regtested on x86_64-linux and
i686-linux.

2016-02-12  Jakub Jelinek  

PR rtl-optimization/69764
PR rtl-optimization/69771
* optabs.c (expand_binop_directly): For shift_optab_p, force
convert_modes with VOIDmode if xop1 has VOIDmode.

* c-c++-common/pr69764.c: New test.
* gcc.dg/torture/pr69771.c: New testcase.

--- gcc/optabs.c.jj 2016-02-12 00:50:30.410240366 +0100
+++ gcc/optabs.c2016-02-12 10:33:32.743492532 +0100
@@ -993,6 +993,7 @@ expand_binop_directly (machine_mode mode
   bool commutative_p;
   rtx_insn *pat;
   rtx xop0 = op0, xop1 = op1;
+  bool canonicalize_op1 = false;
 
   /* If it is a commutative operator and the modes would match
  if we would swap the operands, we can save the conversions.  */
@@ -1006,6 +1007,11 @@ expand_binop_directly (machine_mode mode
   xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
   if (!shift_optab_p (binoptab))
 xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  else
+/* Shifts and rotates often use a different mode for op1 from op0;
+   for VOIDmode constants we don't know the mode, so force it
+   to be canonicalized using convert_modes.  */
+canonicalize_op1 = true;
 
   /* In case the insn wants input operands in modes different from
  those of the actual operands, convert the operands.  It would
@@ -1020,7 +1026,8 @@ expand_binop_directly (machine_mode mode
   mode0 = xmode0;
 }
 
-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1)
+ ? GET_MODE (xop1) : mode;
   if (xmode1 != VOIDmode && xmode1 != mode1)
 {
   xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
--- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 10:27:34.522587995 
+0100
+++ gcc/testsuite/c-c++-common/pr69764.c2016-02-12 10:27:34.522587995 
+0100
@@ -0,0 +1,38 @@
+/* PR rtl-optimization/69764 */
+/* { dg-do compile { target int32plus } } */
+
+unsigned char
+fn1 (unsigned char a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned short
+fn2 (unsigned short a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned int
+fn3 (unsigned int a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned char
+fn4 (unsigned char a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned short
+fn5 (unsigned short a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned int
+fn6 (unsigned int a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
--- gcc/testsuite/gcc.dg/torture/pr69771.c.jj   2016-02-12 10:27:34.522587995 
+0100
+++ gcc/testsuite/gcc.dg/torture/pr69771.c  2016-02-12 10:27:34.522587995 
+0100
@@ -0,0 +1,12 @@
+/* PR rtl-optimization/69771 */
+/* { dg-do compile } */
+
+unsigned char a = 5, c;
+unsigned short b = 0;
+unsigned d = 0x76543210;
+
+void
+foo (void)
+{
+  c = d >> ~(a || ~b); /* { dg-warning "shift count is negative" } */
+}


Jakub


Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

2016-02-12 Thread Alessandro Fanfarillo
Committed on gcc-5-branch as rev. 233379.

2016-02-12 0:04 GMT+01:00 Alessandro Fanfarillo :
> Dear all,
>
> in attachment the EVENT patch for gcc-5-branch directly back-ported
> from the trunk.
>
> Built and regtested on x86_64-pc-linux-gnu. I plan to commit this
> patch this evening (Feb 12th, CET).
>
> Cheers,
>
> Alessandro
>
> 2015-12-17 17:19 GMT+01:00 Alessandro Fanfarillo :
>> Great! Thanks.
>>
>> 2015-12-17 15:57 GMT+01:00 Steve Kargl :
>>> On Thu, Dec 17, 2015 at 01:22:06PM +0100, Alessandro Fanfarillo wrote:

 I've noticed that this patch has been applied only on trunk and not on
 the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?

>>>
>>> No problem.  When I applied the EVENTS patch to trunk,
>>> the 5.3 release was being prepared.  I was going to
>>> wait for a week or two after 5.3 came out, then apply
>>> the patch.  Now that you have commit access, feel
>>> free to back port the patch.  Rememer to post the
>>> patch that you commit to both the fortran and gcc-patches
>>> list.
>>>
>>> --
>>> Steve


Re: [PATCH] PR driver/69779: fix bogus cleanup code used by libgccjit affecting s390x

2016-02-12 Thread David Malcolm
On Fri, 2016-02-12 at 00:51 -0700, Jeff Law wrote:
> On 02/11/2016 10:12 PM, David Malcolm wrote:
> > In r227188 I introduced driver::finalize () which resets
> > all state within gcc.c, allowing the driver code to embedded
> > inside libgccjit and run repeatedly in-process.
> > 
> > Running this on s390x revealed a bug in this cleanup:
> > I was cleaning up "specs" via:
> >  XDELETEVEC (specs);
> > and this was crashing on s390x with:
> >free(): invalid pointer: 0x03fffdf30568 ***
> > 
> > Closer investigation revealed that specs == static_specs,
> > a statically allocated array.
> > 
> > What's happening is that set_specs sets "specs" to static_specs
> > the first time it's called, and any calls to set_specs () with
> > a name that isn't in static_specs cause a new spec_list node
> > to be dynamically-allocated and put at the front of the list;
> > "specs"
> > then equals that.
> > 
> > All of my testing had been on x86_64, which inserts exactly one
> > spec
> > with a name not in the static array, hence the:
> >  XDELETEVEC (specs);
> > happened to work.
> > 
> > On s390x, the only names in the "specs" file are those in the
> > static
> > array, so specs == static_specs, and the bogus cleanup code
> > attempts
> > to free a statically-allocated array.
> > 
> > The following patch reworks this part of the cleanup, walking any
> > list
> > of specs, freeing the dynamically-allocated nodes until the
> > statically-allocated ones are reached.
> > 
> > driver::finalize is only called by libgccjit, not by the main
> > driver
> > program, so this should be relatively low-risk.
> > 
> > Verified directly by running under valgrind on x86_64 and s390x.
> > 
> > With the patch, jit.sum on s390x-ibm-linux-gnu (RHEL 7) goes from:
> ># of expected passes1799
> ># of unexpected failures61
> ># of unresolved testcases   2
> > to:
> ># of expected passes8514
> > 
> > Successfully bootstrapped on x86_64-pc-linux-gnu.
> > 
> > OK for trunk?
> > 
> > gcc/ChangeLog:
> > PR driver/69779
> > * gcc.c (driver::finalize): Fix cleanup of "specs".
> Can't you just "free (specs->name) rather than messing with the cast?
> 
> OK with that twiddle, assuming it works.

Thanks.

The problem is that struct spec_list's "name" field is declared const:

  const char *name; /* name of the spec.  */

and likewise for the "name" field within struct spec_list_1:

  const char *const name;

Some are statically allocated, others are dynamically allocated.

If I convert them to:

  char *name;   /* name of the spec.  */

and:

  char *const name;

respectively, then I run into lots of:
 warning: ISO C++ forbids converting a string constant to ‘char*’ [
-Wpedantic]

Some of these warnings are (relatively) easily fixed by adding a
  const_cast  
to gcc.c's INIT_STATIC_SPEC.  However, the other warnings are from
this:

 static const struct spec_list_1 extra_specs_1[] = { EXTRA_SPECS };

EXTRA_SPECS is defined in the config-specific subdirectories in
numerous ways, and touching those seems like too big a rathole to be
going down, especially in stage 4.

It seems simpler to keep the "const" on those string fields, and cast
it away when cleaning up the dynamically-allocated ones (as in the
candidate patch).

OK without the twiddle?

Dave


Re: [PATCH] PR driver/69779: fix bogus cleanup code used by libgccjit affecting s390x

2016-02-12 Thread Jeff Law

On 02/12/2016 11:12 AM, David Malcolm wrote:



The problem is that struct spec_list's "name" field is declared const:

   const char *name;/* name of the spec.  */

and likewise for the "name" field within struct spec_list_1:

   const char *const name;

Some are statically allocated, others are dynamically allocated.

If I convert them to:

   char *name;  /* name of the spec.  */

and:

   char *const name;

I'm not suggesting you remove the const for thees things.


It seems simpler to keep the "const" on those string fields, and cast
it away when cleaning up the dynamically-allocated ones (as in the
candidate patch).

Is it really necessary to cast it away to call free?  I suppose it is.

OK without the twiddle.

jeff


Re: [PATCH] PR other/69554: avoid excessive source printing for widely-separated locations

2016-02-12 Thread Jeff Law

On 02/09/2016 01:54 PM, David Malcolm wrote:


gcc/ChangeLog:
PR other/69554
* diagnostic-show-locus.c (struct line_span): New struct.
(layout::get_first_line): Delete.
(layout::get_last_line): Delete.
(layout::get_num_line_spans): New member function.
(layout::get_line_span): Likewise.
(layout::print_heading_for_line_span_index_p): Likewise.
(layout::get_expanded_location): Likewise.
(layout::calculate_line_spans): Likewise.
(layout::m_first_line): Delete.
(layout::m_last_line): Delete.
(layout::m_line_spans): New field.
(layout::layout): Update comment.  Replace m_first_line and
m_last_line with m_line_spans, replacing their initialization
with a call to calculate_line_spans.
(diagnostic_show_locus): When printing source lines and
annotations, rather than looping over a single span
of lines, instead loop over each line_span within
the layout, with an inner loop over the lines within them.
Call the context's start_span callback when changing line spans.
* diagnostic.c (diagnostic_initialize): Initialize start_span.
(diagnostic_build_prefix): Break out the building of the location
part of the string into...
(diagnostic_get_location_text): ...this new function, rewriting
it from nested ternary expressions to a sequence of "if"
statements.
(default_diagnostic_start_span_fn): New function.
* diagnostic.h (diagnostic_start_span_fn): New typedef.
(diagnostic_context::start_span): New field.
(default_diagnostic_start_span_fn): New prototype.

gcc/fortran/ChangeLog:
PR other/69554
* error.c (gfc_diagnostic_start_span): New function.
(gfc_diagnostics_init): Initialize global_dc's start_span.

gcc/testsuite/ChangeLog:
PR other/69554
* gcc.dg/pr69554-1.c: New test.
* gfortran.dg/pr69554-1.F90: New test.
* gfortran.dg/pr69554-2.F90: New test.
* lib/gcc-dg.exp (proc dg-locus): New function.
* lib/gfortran-dg.exp (proc gfortran-dg-test): Update comment to
distinguish between the caret-printing and non-caret-printing
cases.  If caret-printing has been explicitly enabled, bail out
without attempting to fix up the output.
---
  gcc/diagnostic-show-locus.c | 226 
  gcc/diagnostic.c|  62 ++---
  gcc/diagnostic.h|  11 ++
  gcc/fortran/error.c |  15 +++
  gcc/testsuite/gcc.dg/pr69554-1.c| 152 +
  gcc/testsuite/gfortran.dg/pr69554-1.F90 |  28 
  gcc/testsuite/gfortran.dg/pr69554-2.F90 |  21 +++
  gcc/testsuite/lib/gcc-dg.exp|  27 
  gcc/testsuite/lib/gfortran-dg.exp   |  19 ++-
  9 files changed, 520 insertions(+), 41 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr69554-1.c
  create mode 100644 gcc/testsuite/gfortran.dg/pr69554-1.F90
  create mode 100644 gcc/testsuite/gfortran.dg/pr69554-2.F90

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index d9b6750..698f42e 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c



@@ -437,8 +477,7 @@ layout::layout (diagnostic_context * context,
m_colorizer (context, diagnostic),
m_colorize_source_p (context->colorize_source_p),
m_layout_ranges (rich_location::MAX_RANGES),
-  m_first_line (m_exploc.line),
-  m_last_line  (m_exploc.line),
+  m_line_spans (1 + rich_location::MAX_RANGES),
m_x_offset (0)

Umm, does that allocate 1 + rich_location::MAX_RANGES linespans?




+  auto_vec tmp_spans (1 + rich_location::MAX_RANGES);


Similarly.

Jeff



Re: TR29124 C++ Special Maths - Make pull functions into global namespace.

2016-02-12 Thread Jonathan Wakely

On 12/02/16 09:52 -0800, Mike Stump wrote:

On Feb 11, 2016, at 3:57 AM, Jonathan Wakely  wrote:

On 10/02/16 22:36 -0800, Mike Stump wrote:

I’m seeing:

/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'
compiler exited with status 1
output is:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

FAIL: special_functions/18_riemann_zeta/check_value.cc (test for excess errors)
Excess errors:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

UNRESOLVED: special_functions/18_riemann_zeta/check_value.cc compilation failed 
to produce executable
extra_tool_flags are:
-D__STDCPP_WANT_MATH_SPEC_FUNCS__

on a recent trunk.  This is a typical newlib port.  Not sure if this is a real 
bug or not, but thought I’d forward it long.


This confused me at first, but I think what must be happening is that
your newlib port doesn't use c_model=c_global, so it uses a 
header that doesn't #include .



What does your $target/libstdc++-v3/config.log show instead of
c_global here?


configure:16304: "C" header strategy set to c_global

So, I don’t know of a better way to do this, other than just provide the .ii 
file.  You can see what it was including, and where it didn’t include what you 
wanted.  We can then go from there.  I fear a longer series of 20 questions.  
The good news is that this is the _only_ recent failure.

/home/mrs/work1/gcc-obj/./gcc/xg++ -shared-libgcc -B/home/mrs/work1/gcc-obj/./gcc 
-nostdinc++ -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src 
-L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/.libs 
-L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/libsupc++/.libs 
-B/home/mrs/work1/install/machine/bin/ -B/home/mrs/work1/install/machine/lib/ -isystem 
/home/mrs/work1/install/machine/include -isystem 
/home/mrs/work1/install/machine/sys-include 
-B/home/mrs/work1/gcc-obj/machine/./libstdc++-v3/src/.libs -D_GLIBCXX_ASSERT 
-fmessage-length=0 -ffunction-sections -fdata-sections -g -O2 -mno-oe 
-DLOCALEDIR="." -nostdinc++ 
-I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include/machine 
-I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include 
-I/home/mrs/work1/gcc/libstdc++-v3/libsupc++ 
-I/home/mrs/work1/gcc/libstdc++-v3/include/backward 
-I/home/mrs/work1/gcc/libstdc++-v3/testsuite/util 
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
 -DMAX_ITERATIONS=5 -fno-diagnostics-show-caret -fdiagnostics-color=never ./libtestc++.a 
-Wl,--gc-sections -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/filesystem/.libs 
-mno-oe -lm -o ./check_value.exe -E


Thanks, the preprocessed file did help, because it showed that
-D__STDCPP_WANT_MATH_SPEC_FUNCS__ was not being used, which is because
we have this in the test:

// { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
...
// { dg-options "-DMAX_ITERATIONS=5" { target simulator } }

So on a simulator we *only* get the MAX_ITERATIONS macro, and not the
__STDCPP_WANT_MATH_SPEC_FUNCS__ one which is needed for the test to
compile (I was misled by the "extra_tool_flags are:" above which
suggested that was set, so I didn't even consider it wasn't!)

Should the dg-options directive for simulators be using
dg-additional-options instead?




Re: Correct c-torture stkalign test

2016-02-12 Thread Jeff Law

On 02/07/2016 10:55 PM, Alan Modra wrote:

This test was added by git commit 7c5f55675 (svn 231569)

Here's the log message from that commit:
 avoid alignment of static variables affecting stack's

 Function (or more narrow) scope static variables (as well as others not
 placed on the stack) should also not have any effect on the stack
 alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
 construct using an 8-byte aligned sub-file-scope local variable.

However, the test assumes that a local var will normally not be 64-bit
aligned, causing it to fail on many targets.  So the test needs to
pass if the local var *is* normally 64-bit aligned.  Done as follows.
test2() is a duplicate of test() without the alignment on the static
vars.  Fails on x86_64 -m64 and -m32 if 7c5f55675 is reverted and
passes now for powerpc64-linux.  I expect sparc will pass too, so have
reverted Eric's change.

OK to apply?

PR testsuite/68886
* gcc.c-torture/execute/stkalign.c: Revise test.

OK.
jeff



Fix PR69648: lra removes set of PIC reg, then introduces new use

2016-02-12 Thread Bernd Schmidt
This is a PR where LRA creates a read from an uninitialized stack slot. 
That stack slot is supposed to hold the value of the PIC register.


What seems to happen is that we have two passes making different choices:

 Choosing alt 0 in insn 143:  (0) =x  (1) 0  (2) r {sse2_pinsrw}
[...]
 Choosing alt 1 in insn 143:  (0) x  (1) 0  (2) m {sse2_pinsrw}

The second choice causes a constant to be placed in memory, and a new 
reference to the PIC register is created. Unfortunately, before the 
second pass, we seem to have deleted an insn that stores a reload reg 
into the spilled pic register pseudo, because we think we've managed to 
inherit the reload reg into all uses. The new use isn't taken into account.


I came up with the following, initially as a hack, but it seems to work 
surprisingly well.  Bootstrapped and tested on x86_64-linux with -fPIC; 
it seems to cure the problem for the testcase (by inspection, it never 
crashed on my system).


Since I was worried about code generation differences (adding 
unnecessary stores) I've also run it against my collection of .i files. 
With -m64 -fPIC, I did not observe any changes. I hadn't looked at 
x86_64 pic generation previously, looks like we can use pc-relative 
addressing and don't need the pic reg at all usually? With -m32 -fPIC, I 
observe differences in 27 out of 4117 source files (taken from the Linux 
kernel, gcc, and several other packages). That doesn't seem very 
worrying, especially considering that several of these changes turn out 
not to be redundant stores, just placing the PIC reg into a different 
stack slot.


So... ok everywhere?


Bernd

PR rtl-optimization/69648
* lra-constraints.c (update_ebb_live_info): Don't remove sets of
pic_offset_table_rtx.

Index: lra-constraints.c
===
--- lra-constraints.c   (revision 232689)
+++ lra-constraints.c   (working copy)
@@ -5127,8 +5127,10 @@ update_ebb_live_info (rtx_insn *head, rt
   curr_id = lra_get_insn_recog_data (curr_insn);
   curr_static_id = curr_id->insn_static_data;
   remove_p = false;
-  if ((set = single_set (curr_insn)) != NULL_RTX && REG_P (SET_DEST 
(set))

+  if ((set = single_set (curr_insn)) != NULL_RTX
+ && REG_P (SET_DEST (set))
  && (regno = REGNO (SET_DEST (set))) >= FIRST_PSEUDO_REGISTER
+ && SET_DEST (set) != pic_offset_table_rtx
  && bitmap_bit_p (_only_regs, regno)
  && ! bitmap_bit_p (_regs, regno))
remove_p = true;


Re: [PATCH] PR other/69554: avoid excessive source printing for widely-separated locations

2016-02-12 Thread David Malcolm
On Fri, 2016-02-12 at 11:25 -0700, Jeff Law wrote:
> On 02/09/2016 01:54 PM, David Malcolm wrote:
> 
> > gcc/ChangeLog:
> > PR other/69554
> > * diagnostic-show-locus.c (struct line_span): New struct.
> > (layout::get_first_line): Delete.
> > (layout::get_last_line): Delete.
> > (layout::get_num_line_spans): New member function.
> > (layout::get_line_span): Likewise.
> > (layout::print_heading_for_line_span_index_p): Likewise.
> > (layout::get_expanded_location): Likewise.
> > (layout::calculate_line_spans): Likewise.
> > (layout::m_first_line): Delete.
> > (layout::m_last_line): Delete.
> > (layout::m_line_spans): New field.
> > (layout::layout): Update comment.  Replace m_first_line and
> > m_last_line with m_line_spans, replacing their initialization
> > with a call to calculate_line_spans.
> > (diagnostic_show_locus): When printing source lines and
> > annotations, rather than looping over a single span
> > of lines, instead loop over each line_span within
> > the layout, with an inner loop over the lines within them.
> > Call the context's start_span callback when changing line
> > spans.
> > * diagnostic.c (diagnostic_initialize): Initialize start_span.
> > (diagnostic_build_prefix): Break out the building of the
> > location
> > part of the string into...
> > (diagnostic_get_location_text): ...this new function, rewriting
> > it from nested ternary expressions to a sequence of "if"
> > statements.
> > (default_diagnostic_start_span_fn): New function.
> > * diagnostic.h (diagnostic_start_span_fn): New typedef.
> > (diagnostic_context::start_span): New field.
> > (default_diagnostic_start_span_fn): New prototype.
> > 
> > gcc/fortran/ChangeLog:
> > PR other/69554
> > * error.c (gfc_diagnostic_start_span): New function.
> > (gfc_diagnostics_init): Initialize global_dc's start_span.
> > 
> > gcc/testsuite/ChangeLog:
> > PR other/69554
> > * gcc.dg/pr69554-1.c: New test.
> > * gfortran.dg/pr69554-1.F90: New test.
> > * gfortran.dg/pr69554-2.F90: New test.
> > * lib/gcc-dg.exp (proc dg-locus): New function.
> > * lib/gfortran-dg.exp (proc gfortran-dg-test): Update comment
> > to
> > distinguish between the caret-printing and non-caret-printing
> > cases.  If caret-printing has been explicitly enabled, bail out
> > without attempting to fix up the output.
> > ---
> >   gcc/diagnostic-show-locus.c | 226
> > 
> >   gcc/diagnostic.c|  62 ++---
> >   gcc/diagnostic.h|  11 ++
> >   gcc/fortran/error.c |  15 +++
> >   gcc/testsuite/gcc.dg/pr69554-1.c| 152
> > +
> >   gcc/testsuite/gfortran.dg/pr69554-1.F90 |  28 
> >   gcc/testsuite/gfortran.dg/pr69554-2.F90 |  21 +++
> >   gcc/testsuite/lib/gcc-dg.exp|  27 
> >   gcc/testsuite/lib/gfortran-dg.exp   |  19 ++-
> >   9 files changed, 520 insertions(+), 41 deletions(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/pr69554-1.c
> >   create mode 100644 gcc/testsuite/gfortran.dg/pr69554-1.F90
> >   create mode 100644 gcc/testsuite/gfortran.dg/pr69554-2.F90
> > 
> > diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show
> > -locus.c
> > index d9b6750..698f42e 100644
> > --- a/gcc/diagnostic-show-locus.c
> > +++ b/gcc/diagnostic-show-locus.c
> 
> > @@ -437,8 +477,7 @@ layout::layout (diagnostic_context * context,
> > m_colorizer (context, diagnostic),
> > m_colorize_source_p (context->colorize_source_p),
> > m_layout_ranges (rich_location::MAX_RANGES),
> > -  m_first_line (m_exploc.line),
> > -  m_last_line  (m_exploc.line),
> > +  m_line_spans (1 + rich_location::MAX_RANGES),
> > m_x_offset (0)
> Umm, does that allocate 1 + rich_location::MAX_RANGES linespans?
> 
> 
> 
> > +  auto_vec tmp_spans (1 + rich_location::MAX_RANGES);
> 
> Similarly.

Yes, it's preallocating space in the vecs.  rich_location::MAX_RANGES
is 3, so there can be at most 4 line spans to consider.

A line_span is two linenum_type i.e. a pair of unsigned int, so
assuming sizeof(unsigned int) == 4 we have 4 * 2 * 4 = 32 bytes.

My thinking here was to preallocate these two vecs with the maximum
size they can be, to avoid growing the vec by one each time, thus
avoiding "exponential" reallocing and copying.

Typically there will be just one or two elements, rather than 4 in this
vec, but given the tiny sizes it seemed easiest to preallocate the
maximum possible size (granted, N is so small that O(N^2) isn't going
to get that big).

This pair of allocations is done once per call to diagnostic_show_locus
i.e. per diagnostic that ends up printing source code.

I believe vl_embed isn't usable here (though I find vec.h to be clear
as mud); would a simple array plus length be better?

Dave


Re: [PATCH] PR other/69554: avoid excessive source printing for widely-separated locations

2016-02-12 Thread Jeff Law

On 02/12/2016 12:06 PM, David Malcolm wrote:

On Fri, 2016-02-12 at 11:25 -0700, Jeff Law wrote:

On 02/09/2016 01:54 PM, David Malcolm wrote:


gcc/ChangeLog:
PR other/69554
* diagnostic-show-locus.c (struct line_span): New struct.
(layout::get_first_line): Delete.
(layout::get_last_line): Delete.
(layout::get_num_line_spans): New member function.
(layout::get_line_span): Likewise.
(layout::print_heading_for_line_span_index_p): Likewise.
(layout::get_expanded_location): Likewise.
(layout::calculate_line_spans): Likewise.
(layout::m_first_line): Delete.
(layout::m_last_line): Delete.
(layout::m_line_spans): New field.
(layout::layout): Update comment.  Replace m_first_line and
m_last_line with m_line_spans, replacing their initialization
with a call to calculate_line_spans.
(diagnostic_show_locus): When printing source lines and
annotations, rather than looping over a single span
of lines, instead loop over each line_span within
the layout, with an inner loop over the lines within them.
Call the context's start_span callback when changing line
spans.
* diagnostic.c (diagnostic_initialize): Initialize start_span.
(diagnostic_build_prefix): Break out the building of the
location
part of the string into...
(diagnostic_get_location_text): ...this new function, rewriting
it from nested ternary expressions to a sequence of "if"
statements.
(default_diagnostic_start_span_fn): New function.
* diagnostic.h (diagnostic_start_span_fn): New typedef.
(diagnostic_context::start_span): New field.
(default_diagnostic_start_span_fn): New prototype.

gcc/fortran/ChangeLog:
PR other/69554
* error.c (gfc_diagnostic_start_span): New function.
(gfc_diagnostics_init): Initialize global_dc's start_span.

gcc/testsuite/ChangeLog:
PR other/69554
* gcc.dg/pr69554-1.c: New test.
* gfortran.dg/pr69554-1.F90: New test.
* gfortran.dg/pr69554-2.F90: New test.
* lib/gcc-dg.exp (proc dg-locus): New function.
* lib/gfortran-dg.exp (proc gfortran-dg-test): Update comment
to
distinguish between the caret-printing and non-caret-printing
cases.  If caret-printing has been explicitly enabled, bail out
without attempting to fix up the output.
---
   gcc/diagnostic-show-locus.c | 226

   gcc/diagnostic.c|  62 ++---
   gcc/diagnostic.h|  11 ++
   gcc/fortran/error.c |  15 +++
   gcc/testsuite/gcc.dg/pr69554-1.c| 152
+
   gcc/testsuite/gfortran.dg/pr69554-1.F90 |  28 
   gcc/testsuite/gfortran.dg/pr69554-2.F90 |  21 +++
   gcc/testsuite/lib/gcc-dg.exp|  27 
   gcc/testsuite/lib/gfortran-dg.exp   |  19 ++-
   9 files changed, 520 insertions(+), 41 deletions(-)
   create mode 100644 gcc/testsuite/gcc.dg/pr69554-1.c
   create mode 100644 gcc/testsuite/gfortran.dg/pr69554-1.F90
   create mode 100644 gcc/testsuite/gfortran.dg/pr69554-2.F90

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show
-locus.c
index d9b6750..698f42e 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c



@@ -437,8 +477,7 @@ layout::layout (diagnostic_context * context,
 m_colorizer (context, diagnostic),
 m_colorize_source_p (context->colorize_source_p),
 m_layout_ranges (rich_location::MAX_RANGES),
-  m_first_line (m_exploc.line),
-  m_last_line  (m_exploc.line),
+  m_line_spans (1 + rich_location::MAX_RANGES),
 m_x_offset (0)

Umm, does that allocate 1 + rich_location::MAX_RANGES linespans?




+  auto_vec tmp_spans (1 + rich_location::MAX_RANGES);


Similarly.


Yes, it's preallocating space in the vecs.  rich_location::MAX_RANGES
is 3, so there can be at most 4 line spans to consider.

Ah, nevermind then.  If it's just 4, then I'm not worried.

OK for the trunk.
jeff



Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread James Greenhalgh
On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote:
> On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote:
> > >>-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> > >>+  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1;
> > >>if (xmode1 != VOIDmode && xmode1 != mode1)
> > >>  {
> > 
> > Here, things aren't so clear, and the fact that the mode1 calculation now
> > differs from the mode0 one may be overlooked by someone in the future.
> > 
> > Rather than use codes like "mode variable is VOIDmode", I'd prefer to use
> > booleans with descriptive names, like "op1_may_need_conversion".
> 
> So do you prefer e.g. following?  Bootstrapped/regtested on x86_64-linux and
> i686-linux.
> 
> 2016-02-12  Jakub Jelinek  
> 
>   PR rtl-optimization/69764
>   PR rtl-optimization/69771
>   * optabs.c (expand_binop_directly): For shift_optab_p, force
>   convert_modes with VOIDmode if xop1 has VOIDmode.
> 
>   * c-c++-common/pr69764.c: New test.
>   * gcc.dg/torture/pr69771.c: New testcase.
> 

These two new tests are failing for me on AArch64 as so:

.../gcc/testsuite/c-c++-common/pr69764.c:7:12: internal compiler error: in 
decompose, at rtl.h:2107
0x7d30be wi::int_traits >::decompose(long*, 
unsigned int, std::pair const&)
.../gcc/rtl.h:2105
0x7d30be wide_int_ref_storage::wide_int_ref_storage >(std::pair const&)
.../gcc/wide-int.h:936
0x7d30be generic_wide_int::generic_wide_int >(std::pair const&)
.../gcc/wide-int.h:714
0x7d30be convert_modes(machine_mode, machine_mode, rtx_def*, int)
.../gcc/expr.c:697
0x9ec7c6 widen_operand
.../gcc/optabs.c:208
0x9f1e79 expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*, 
int, optab_methods)
.../gcc/optabs.c:1280
0x7b7a95 expand_shift_1
.../gcc/expmed.c:2458
0x7bca49 expand_variable_shift(tree_code, machine_mode, rtx_def*, tree_node*, 
rtx_def*, int)
.../gcc/expmed.c:2517
0x7e1d43 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, 
expand_modifier)
.../gcc/expr.c:9029
0x6d9ed9 expand_gimple_stmt_1
.../gcc/cfgexpand.c:3642
0x6d9ed9 expand_gimple_stmt
.../gcc/cfgexpand.c:3702
0x6dc369 expand_gimple_basic_block
.../gcc/cfgexpand.c:5708
0x6dfcdc execute
.../gcc/cfgexpand.c:6323
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.

I can't dig deeper today, but I will look closer on Monday if the fix is
not obvious.

Thanks,
James



Re: bswap PRs 69714, 67781

2016-02-12 Thread Bernd Schmidt

On 02/13/2016 03:36 AM, John David Anglin wrote:

As far as the avcrc.c reduced testcase, it didn't trigger the original bug on 
hppa-unknown-linux-gnu.


Odd, I see the same transformation in the dumps. Do you think you could 
investigate a bit so we get a useful testcase to add?



Bernd




Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

2016-02-12 Thread Oleg Endo
On Sat, 2016-02-13 at 07:50 +, James Greenhalgh wrote:

> > So do you prefer e.g. following?  Bootstrapped/regtested on x86_64
> > -linux and
> > i686-linux.
> > 
> > 2016-02-12  Jakub Jelinek  
> > 
> > PR rtl-optimization/69764
> > PR rtl-optimization/69771
> > * optabs.c (expand_binop_directly): For shift_optab_p, force
> > convert_modes with VOIDmode if xop1 has VOIDmode.
> > 
> > * c-c++-common/pr69764.c: New test.
> > * gcc.dg/torture/pr69771.c: New testcase.
> > 
> 
> These two new tests are failing for me on AArch64 as so:
> 

I see the same failures on SH, too.

Cheers,
Oleg


Re: [PATCH] New flag for dumping information about constexpr function calls memoization (GCC 5.2.0)

2016-02-12 Thread Daniel Gutson
On Fri, Jan 29, 2016 at 11:46 AM, Andres Tiraboschi
 wrote:
> 2016-01-28 17:54 GMT-03:00 Joseph Myers :
>> Any patch adding a new option needs to add documentation for it to
>> invoke.texi (both substantive documentation, and inclusion in the summary
>> lists of options).
>>
>> --
>> Joseph S. Myers
>> jos...@codesourcery.com
>
> Hi,
>
> Thanks for the feedback, I've just updated the documentation.
>
> patch:

Ping, any chance to review this?

Thanks,

Daniel.

>
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 1218a71..bf0c7df 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1168,6 +1168,10 @@ fdump-passes
>  Common Var(flag_dump_passes) Init(0)
>  Dump optimization passes
>
> +fdump-memoization-hits
> +Common Var(flag_dump_memoization_hits) Init(0)
> +Dump info about constexpr calls memoized.
> +
>  fdump-unnumbered
>  Common Report Var(flag_dump_unnumbered)
>  Suppress output of instruction numbers, line number notes and
> addresses in debugging dumps
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index e250726..41ae5b3 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -42,6 +42,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "builtins.h"
>  #include "tree-inline.h"
>  #include "ubsan.h"
> +#include "tree-pretty-print.h"
> +#include "dumpfile.h"
>
>  static bool verify_constant (tree, bool, bool *, bool *);
>  #define VERIFY_CONSTANT(X)\
> @@ -1173,6 +1175,14 @@ cx_error_context (void)
>return r;
>  }
>
> +static void
> +dump_memoization_hit (FILE *file, tree call, int flags)
> +{
> +  fprintf(file, "Memoized call:\n");
> +  print_generic_decl(file, call, flags);
> +  fprintf(file, "\n");
> +}
> +
>  /* Subroutine of cxx_eval_constant_expression.
> Evaluate the call expression tree T in the context of OLD_CALL expression
> evaluation.  */
> @@ -1338,7 +1348,11 @@ cxx_eval_call_expression (const constexpr_ctx
> *ctx, tree t,
>entry->result = result = error_mark_node;
>  }
>else
> -result = entry->result;
> +{
> +  if (flag_dump_memoization_hits)
> +  dump_memoization_hit(stderr, t, 0);
> +  result = entry->result;
> +}
>  }
>
>if (!depth_ok)
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index f84a199..b78bd4b 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -322,6 +322,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fdump-class-hierarchy@r{[}-@var{n}@r{]} @gol
>  -fdump-ipa-all -fdump-ipa-cgraph -fdump-ipa-inline @gol
>  -fdump-passes @gol
> +-fdump-memoization-hits @gol
>  -fdump-statistics @gol
>  -fdump-tree-all @gol
>  -fdump-tree-original@r{[}-@var{n}@r{]}  @gol
> @@ -6771,6 +6772,10 @@ Dump after function inlining.
>  Dump the list of optimization passes that are turned on and off by
>  the current command-line options.
>
> +@item -fdump-memoization-hits
> +@opindex fdump-memoization-hits
> +Dump the list of constexpr function calls that were memoized.
> +
>  @item -fdump-statistics-@var{option}
>  @opindex fdump-statistics
>  Enable and control dumping of pass statistics in a separate file.  The



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson


[PATCH, committed] TILE-Gx: Use CXX_FOR_BUILD to compile c++ source.

2016-02-12 Thread Walter Lee
This patch uses g++ instead of gcc to compile a c++ source file. The GNU 
buildbot is currently failing with a c++ related link error that I have 
not been able to reproduce; I'm hoping using g++ will fix the issue.


2016-02-12  Walter Lee  

* config/tilepro/t-tilepro: Replace CC_FOR_BUILD with
  CXX_FOR_BUILD.
* config/tilegx/t-tilegx: Likewise.

Index: gcc/config/tilegx/t-tilegx
===
--- gcc/config/tilegx/t-tilegx(revision 233352)
+++ gcc/config/tilegx/t-tilegx(working copy)
@@ -12,7 +12,7 @@ tilegx-c.o: $(srcdir)/config/tilegx/tile

 $(srcdir)/config/tilegx/mul-tables.c: \
 $(srcdir)/config/tilepro/gen-mul-tables.cc
-$(CC_FOR_BUILD) $(BUILD_CPPFLAGS) -O2 -o gen-mul-tables -lstdc++ $<;
+$(CXX_FOR_BUILD) $(BUILD_CPPFLAGS) -O2 -o gen-mul-tables $<;
 ./gen-mul-tables > $@

 mul-tables.o: $(srcdir)/config/tilegx/mul-tables.c \
Index: gcc/config/tilepro/t-tilepro
===
--- gcc/config/tilepro/t-tilepro(revision 233352)
+++ gcc/config/tilepro/t-tilepro(working copy)
@@ -5,8 +5,8 @@ tilepro-c.o: $(srcdir)/config/tilepro/ti

 $(srcdir)/config/tilepro/mul-tables.c: \
 $(srcdir)/config/tilepro/gen-mul-tables.cc
-$(CC_FOR_BUILD) $(BUILD_CPPFLAGS) -O2 -DTILEPRO \
-  -o gen-mul-tables -lstdc++ $<;
+$(CXX_FOR_BUILD) $(BUILD_CPPFLAGS) -O2 -DTILEPRO \
+  -o gen-mul-tables $<;
 ./gen-mul-tables > $@


Re: TR29124 C++ Special Maths - Make pull functions into global namespace.

2016-02-12 Thread Mike Stump
On Feb 12, 2016, at 1:01 PM, Jonathan Wakely  wrote:
> OK, thanks.

t double checked the log file:

  -D__STDCPP_WANT_MATH_SPEC_FUNCS__ -DMAX_ITERATIONS=5

which I neglected to do the first time.  With that check looking good as well, 
I checked it in, thanks.

Re: TR29124 C++ Special Maths - Make pull functions into global namespace.

2016-02-12 Thread Jonathan Wakely

On 12/02/16 12:39 -0800, Mike Stump wrote:


On Feb 12, 2016, at 10:07 AM, Jonathan Wakely  wrote:


On 12/02/16 09:52 -0800, Mike Stump wrote:

On Feb 11, 2016, at 3:57 AM, Jonathan Wakely  wrote:

On 10/02/16 22:36 -0800, Mike Stump wrote:

I’m seeing:

/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'
compiler exited with status 1
output is:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

FAIL: special_functions/18_riemann_zeta/check_value.cc (test for excess errors)
Excess errors:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

UNRESOLVED: special_functions/18_riemann_zeta/check_value.cc compilation failed 
to produce executable
extra_tool_flags are:
-D__STDCPP_WANT_MATH_SPEC_FUNCS__

on a recent trunk.  This is a typical newlib port.  Not sure if this is a real 
bug or not, but thought I’d forward it long.


This confused me at first, but I think what must be happening is that
your newlib port doesn't use c_model=c_global, so it uses a 
header that doesn't #include .



What does your $target/libstdc++-v3/config.log show instead of
c_global here?


configure:16304: "C" header strategy set to c_global

So, I don’t know of a better way to do this, other than just provide the .ii 
file.  You can see what it was including, and where it didn’t include what you 
wanted.  We can then go from there.  I fear a longer series of 20 questions.  
The good news is that this is the _only_ recent failure.

/home/mrs/work1/gcc-obj/./gcc/xg++ -shared-libgcc -B/home/mrs/work1/gcc-obj/./gcc 
-nostdinc++ -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src 
-L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/.libs 
-L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/libsupc++/.libs 
-B/home/mrs/work1/install/machine/bin/ -B/home/mrs/work1/install/machine/lib/ -isystem 
/home/mrs/work1/install/machine/include -isystem 
/home/mrs/work1/install/machine/sys-include 
-B/home/mrs/work1/gcc-obj/machine/./libstdc++-v3/src/.libs -D_GLIBCXX_ASSERT 
-fmessage-length=0 -ffunction-sections -fdata-sections -g -O2 -mno-oe 
-DLOCALEDIR="." -nostdinc++ 
-I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include/machine 
-I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include 
-I/home/mrs/work1/gcc/libstdc++-v3/libsupc++ 
-I/home/mrs/work1/gcc/libstdc++-v3/include/backward 
-I/home/mrs/work1/gcc/libstdc++-v3/testsuite/util 
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
 -DMAX_ITERATIONS=5 -fno-diagnostics-show-caret -fdiagnostics-color=never ./libtestc++.a 
-Wl,--gc-sections -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/filesystem/.libs 
-mno-oe -lm -o ./check_value.exe -E


Thanks, the preprocessed file did help, because it showed that
-D__STDCPP_WANT_MATH_SPEC_FUNCS__ was not being used, which is because
we have this in the test:

// { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
...
// { dg-options "-DMAX_ITERATIONS=5" { target simulator } }



Should the dg-options directive for simulators be using
dg-additional-options instead?


I think that’s a good call.  Testing it out:

Old tests that failed, that have disappeared: (Eeek!)

: special_functions/18_riemann_zeta/check_value.cc (test for excess errors)


Ok?


OK, thanks.



Re: [PATCH] Fix tilegx libgcc with multilib

2016-02-12 Thread Walter Lee
Hi Bernd.  Thanks for looking into that.  Your patch looks good and I 
have checked it in myself.


Walter

On 1/1/2016 4:31 PM, Bernd Edlinger wrote:

Hi Walter,


while playing with the tilegx cross compiler I noticed another defect.

Currently building a tilegx cross compiler fails in libgcc multilib 
configuration,
because of the following static assert in _FP_FROM_INT:

   _FP_STATIC_ASSERT ((rsize) <= 2 * _FP_W_TYPE_SIZE,\
  "rsize too large");\

previously that macro used to abort at run-time.  This happens apparently in all
float to ti-int conversions in the 32-bit target configuration.  So I assume 
that
softfp_int_modes should only contain ti for 64-bit target configurations.

The following patch makes the multilib libgcc build succeed for me,
but I can not test if the result is actually usable.

Really AFAICT these machines must be pretty cool, but I don't own one...


Thanks
Bernd.




[PATCH, committed] TILE-Gx: Fix softfp build error.

2016-02-12 Thread Walter Lee
This patch removes ti from softfp_int_modes for 32-bit config, to fix a 
compile-time assert failure.


2016-02-12  Walter Lee  

* config.host (tilegx*-*-linux*): remove ti from
  softfp_int_modes for 32-bit configs.

Index: libgcc/config.host
===
--- libgcc/config.host  (revision 233388)
+++ libgcc/config.host  (working copy)
@@ -1277,7 +1277,10 @@ tic6x-*-elf)
unwind_header=config/c6x/unwind-c6x.h
;;
 tilegx*-*-linux*)
-   tmake_file="${tmake_file} tilegx/t-crtstuff t-softfp-sfdf 
tilegx/t-softfp t-softfp tilegx/t-tilegx"

+   if test "${host_address}" = 64; then
+   tmake_file="${tmake_file} tilegx/t-softfp"
+   fi
+   tmake_file="${tmake_file} tilegx/t-crtstuff t-softfp-sfdf 
t-softfp tilegx/t-tilegx"

md_unwind_header=tilepro/linux-unwind.h
 ;;
 tilepro*-*-linux*)



libgo patch committed: For c-archive, install signal handler synchronously

2016-02-12 Thread Ian Lance Taylor
This patch to libgo ports https://golang.org/cl/18150 from the master
library to the gccgo runtime.  This installs signal handlers
synchronously when building a Go archive, rather than potentially
racing against the main program.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 233290)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-28a9dfbc3cda0bf7fd4f3fb1506c547f6cdf41a5
+22278c6e8ce3982b0983bc6addf0184bef1f
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/go-libmain.c
===
--- libgo/runtime/go-libmain.c  (revision 232239)
+++ libgo/runtime/go-libmain.c  (working copy)
@@ -59,6 +59,10 @@ initfn (int argc, char **argv, char** en
   struct args *a;
   pthread_t tid;
 
+  runtime_isarchive = true;
+
+  runtime_initsig(true);
+
   a = (struct args *) malloc (sizeof *a);
   if (a == NULL)
 die ("malloc", errno);
@@ -88,8 +92,6 @@ gostart (void *arg)
 {
   struct args *a = (struct args *) arg;
 
-  runtime_isarchive = true;
-
   if (runtime_isstarted)
 return NULL;
   runtime_isstarted = true;
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 233260)
+++ libgo/runtime/proc.c(working copy)
@@ -1093,7 +1093,7 @@ runtime_mstart(void* mp)
runtime_newextram();
runtime_needextram = 0;
}
-   runtime_initsig();
+   runtime_initsig(false);
}

if(m->mstartfn)
Index: libgo/runtime/runtime.h
===
--- libgo/runtime/runtime.h (revision 233260)
+++ libgo/runtime/runtime.h (working copy)
@@ -550,7 +550,7 @@ void*   runtime_mal(uintptr);
 String runtime_gostring(const byte*);
 String runtime_gostringnocopy(const byte*);
 void   runtime_schedinit(void);
-void   runtime_initsig(void);
+void   runtime_initsig(bool);
 void   runtime_sigenable(uint32 sig);
 void   runtime_sigdisable(uint32 sig);
 void   runtime_sigignore(uint32 sig);
Index: libgo/runtime/signal_unix.c
===
--- libgo/runtime/signal_unix.c (revision 233110)
+++ libgo/runtime/signal_unix.c (working copy)
@@ -13,11 +13,16 @@
 extern SigTab runtime_sigtab[];
 
 void
-runtime_initsig(void)
+runtime_initsig(bool preinit)
 {
int32 i;
SigTab *t;
 
+   // For c-archive/c-shared this is called by go-libmain.c with
+   // preinit == true.
+   if(runtime_isarchive && !preinit)
+   return;
+
// First call: basic setup.
for(i = 0; runtime_sigtab[i].sig != -1; i++) {
t = _sigtab[i];
@@ -37,6 +42,9 @@ runtime_initsig(void)
}
}
 
+   if(runtime_isarchive && (t->flags) == 0)
+   continue;
+
t->flags |= SigHandling;
runtime_setsig(i, runtime_sighandler, true);
}


Re: Fix PR69648: lra removes set of PIC reg, then introduces new use

2016-02-12 Thread Vladimir Makarov

On 02/12/2016 01:49 PM, Bernd Schmidt wrote:
This is a PR where LRA creates a read from an uninitialized stack 
slot. That stack slot is supposed to hold the value of the PIC register.


What seems to happen is that we have two passes making different choices:

 Choosing alt 0 in insn 143:  (0) =x  (1) 0  (2) r {sse2_pinsrw}
[...]
 Choosing alt 1 in insn 143:  (0) x  (1) 0  (2) m {sse2_pinsrw}

The second choice causes a constant to be placed in memory, and a new 
reference to the PIC register is created. Unfortunately, before the 
second pass, we seem to have deleted an insn that stores a reload reg 
into the spilled pic register pseudo, because we think we've managed 
to inherit the reload reg into all uses. The new use isn't taken into 
account.


I came up with the following, initially as a hack, but it seems to 
work surprisingly well.  Bootstrapped and tested on x86_64-linux with 
-fPIC; it seems to cure the problem for the testcase (by inspection, 
it never crashed on my system).


Since I was worried about code generation differences (adding 
unnecessary stores) I've also run it against my collection of .i 
files. With -m64 -fPIC, I did not observe any changes. I hadn't looked 
at x86_64 pic generation previously, looks like we can use pc-relative 
addressing and don't need the pic reg at all usually? With -m32 -fPIC, 
I observe differences in 27 out of 4117 source files (taken from the 
Linux kernel, gcc, and several other packages). That doesn't seem very 
worrying, especially considering that several of these changes turn 
out not to be redundant stores, just placing the PIC reg into a 
different stack slot.


Thank you for checking this.  That is the only thing about the patch 
which I would worry too.

So... ok everywhere?



Yes. Thanks, Bernd.


PR rtl-optimization/69648
* lra-constraints.c (update_ebb_live_info): Don't remove sets of
pic_offset_table_rtx.





Re: TR29124 C++ Special Maths - Make pull functions into global namespace.

2016-02-12 Thread Mike Stump

On Feb 12, 2016, at 10:07 AM, Jonathan Wakely  wrote:

> On 12/02/16 09:52 -0800, Mike Stump wrote:
>> On Feb 11, 2016, at 3:57 AM, Jonathan Wakely  wrote:
>>> On 10/02/16 22:36 -0800, Mike Stump wrote:
 I’m seeing:
 
 /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
  In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
 /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
  error: 'riemann_zeta' is not a member of 'std'
 compiler exited with status 1
 output is:
 /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
  In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
 /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
  error: 'riemann_zeta' is not a member of 'std'
 
 FAIL: special_functions/18_riemann_zeta/check_value.cc (test for excess 
 errors)
 Excess errors:
 /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
  error: 'riemann_zeta' is not a member of 'std'
 
 UNRESOLVED: special_functions/18_riemann_zeta/check_value.cc compilation 
 failed to produce executable
 extra_tool_flags are:
 -D__STDCPP_WANT_MATH_SPEC_FUNCS__
 
 on a recent trunk.  This is a typical newlib port.  Not sure if this is a 
 real bug or not, but thought I’d forward it long.
>>> 
>>> This confused me at first, but I think what must be happening is that
>>> your newlib port doesn't use c_model=c_global, so it uses a 
>>> header that doesn't #include .
>> 
>>> What does your $target/libstdc++-v3/config.log show instead of
>>> c_global here?
>> 
>> configure:16304: "C" header strategy set to c_global
>> 
>> So, I don’t know of a better way to do this, other than just provide the .ii 
>> file.  You can see what it was including, and where it didn’t include what 
>> you wanted.  We can then go from there.  I fear a longer series of 20 
>> questions.  The good news is that this is the _only_ recent failure.
>> 
>> /home/mrs/work1/gcc-obj/./gcc/xg++ -shared-libgcc 
>> -B/home/mrs/work1/gcc-obj/./gcc -nostdinc++ 
>> -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src 
>> -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/.libs 
>> -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/libsupc++/.libs 
>> -B/home/mrs/work1/install/machine/bin/ 
>> -B/home/mrs/work1/install/machine/lib/ -isystem 
>> /home/mrs/work1/install/machine/include -isystem 
>> /home/mrs/work1/install/machine/sys-include 
>> -B/home/mrs/work1/gcc-obj/machine/./libstdc++-v3/src/.libs -D_GLIBCXX_ASSERT 
>> -fmessage-length=0 -ffunction-sections -fdata-sections -g -O2 -mno-oe 
>> -DLOCALEDIR="." -nostdinc++ 
>> -I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include/machine 
>> -I/home/mrs/work1/gcc-obj/machine/libstdc++-v3/include 
>> -I/home/mrs/work1/gcc/libstdc++-v3/libsupc++ 
>> -I/home/mrs/work1/gcc/libstdc++-v3/include/backward 
>> -I/home/mrs/work1/gcc/libstdc++-v3/testsuite/util 
>> /home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
>>  -DMAX_ITERATIONS=5 -fno-diagnostics-show-caret -fdiagnostics-color=never 
>> ./libtestc++.a -Wl,--gc-sections 
>> -L/home/mrs/work1/gcc-obj/machine/libstdc++-v3/src/filesystem/.libs -mno-oe 
>> -lm -o ./check_value.exe -E
> 
> Thanks, the preprocessed file did help, because it showed that
> -D__STDCPP_WANT_MATH_SPEC_FUNCS__ was not being used, which is because
> we have this in the test:
> 
> // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
> ...
> // { dg-options "-DMAX_ITERATIONS=5" { target simulator } }

> Should the dg-options directive for simulators be using
> dg-additional-options instead?

I think that’s a good call.  Testing it out:

Old tests that failed, that have disappeared: (Eeek!)

: special_functions/18_riemann_zeta/check_value.cc (test for excess errors)


Ok?

diff --git 
a/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc 
b/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
index d6b3171..f84edd8 100644
--- a/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
+++ b/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc
@@ -20,7 +20,7 @@
 //  riemann_zeta
 
 // This can take long on simulators, timing out the test.
-// { dg-options "-DMAX_ITERATIONS=5" { target simulator } }
+// { dg-additional-options "-DMAX_ITERATIONS=5" { target simulator } }
 
 #ifndef MAX_ITERATIONS
 #define MAX_ITERATIONS (sizeof(data001) / 
sizeof(testcase_riemann_zeta))
@@ -109,9 +109,6 @@ data001[55] =
 const double toler001 = 2.5020e-13;
 //  riemann_zeta
 
-// This can take long on simulators, timing out the test.
-// { dg-options "-DMAX_ITERATIONS=5" { target simulator } }
-
 #ifndef MAX_ITERATIONS
 

Re: AW: Wonly-top-basic-asm

2016-02-12 Thread Sandra Loosemore

On 02/12/2016 05:51 AM, Bernd Schmidt wrote:

On 02/12/2016 08:05 AM, David Wohlferd wrote:

Actually, it was my intent that this apply to v6.  It's not like there
is a significant change here.  We're documenting long-time behavior, and
adding a (disabled) warning.


The doc patch (minus mentioning the warning) could go in now, but for
gcc-6 we're at a stage where we're only accepting regression fixes with
very few exceptions. If you can convince a RM that this is important
enough then it could still go in.


I looked at the last version of the patch I saw and this is my 
conclusion as well.  If you would like me to commit just the doc change 
(minus the references to the new warning) now, please split the patch 
and I will do that.  But, I cannot commit the change to add the new 
warning during Stage 4 without approval from a RM.


-Sandra



[committed] Require alias support for gcc.dg/pr67964.c

2016-02-12 Thread John David Anglin
This test needs alias support.

Tested on hppa2.0w-hp-hpux11.11.   Committed to trunk.

Dave
--
John David Anglin   dave.ang...@bell.net


2016-02-12  John David Anglin  

* gcc.dg/pr67964.c: Add dg-require-alias.

Index: gcc.dg/pr67964.c
===
--- gcc.dg/pr67964.c(revision 233284)
+++ gcc.dg/pr67964.c(working copy)
@@ -1,5 +1,6 @@
 /* PR c/67964 */
 /* { dg-do compile } */
+/* { dg-require-alias "" } */
 
 extern int fn0 (void) __attribute__ ((const const)); /* { dg-error "expected" 
} */
 extern int fn1 (void) __attribute__ ((const, const));


Re: bswap PRs 69714, 67781

2016-02-12 Thread John David Anglin
On 2016-02-12, at 11:28 AM, Bernd Schmidt wrote:

> I'm attaching a full patch. John David Anglin is currently running tests on 
> PA which may take a while. He's confirmed it fixes his testcase, and the 
> earlier 67781 testcase seems to be cured as well (only looked at generated 
> assembly, don't have a full cross environment - Thomas, can you verify?)
> I'm also doing an x86_64 test run. Ok everywhere if it passes? I'm also 
> attaching a version of the testcase (LGPL, from ffmpeg) which I'd also apply 
> to gcc.dg/torture if David (or Thomas) can confirm it works on a big-endian 
> target after the fix.

I have tested the patch on hppa2.0w-hp-hpux11.11 and hppa-unknown-linux-gnu so 
far.  It fixes the original testcase
and I didn't see any regressions related to this change.  I also have a private 
communication that it also fixes the mips
testcase from Debian bug #813858.

As far as the avcrc.c reduced testcase, it didn't trigger the original bug on 
hppa-unknown-linux-gnu.

> 
> 
> There is room for more improvement in this code - hopefully we can get to 
> that in gcc-7. As mentioned, it does not use the bytepos in the final 
> replacement. We could extend the code to recognize smaller sub-bswaps at an 
> offset. Currently, only one of the two rotates involved in a 32-bit bswap is 
> recognized.
> 
> Also, for the crc testcase, the transformation actually pessimizes the code 
> by inserting an extra memory load. If we already have loaded a larger value, 
> we should build the subpart using a shift and convert.

I noticed that.  I'm testing a patch to add bswap patterns to pa.md.  The PA 
2.0 architecture manual actually
gives some optimized sequences for byte swap operations with half-word, word 
and double-word operands.
The compiler might be able to duplicate the half-word and word sequences, but 
there is no way it can
duplicate the double-word sequence.  It uses half-word instructions that aren't 
exposed in PA backend.

Dave
--
John David Anglin   dave.ang...@bell.net





Re: [PATCH] [ARC] Add single/double IEEE precission FPU support.

2016-02-12 Thread Joern Wolfgang Rennecke



On 10/02/16 13:31, Claudiu Zissulescu wrote:

Please find attached the amended patch for FPU instructions.

Ok to apply?

+(define_insn "*cmpdf_fpu"

I'm wondering - could you compare with +zero using a literal (adding an 
alternative)?
(No need to hold up the main patch, but you can consider it for a 
follow-up patch)


(define_insn "*cmpsf_fpu_uneq"
+  [(set (reg:CC_FPU_UNEQ CC_REG)
+   (compare:CC_FPU_UNEQ
+(match_operand:DF 0 "even_register_operand"  "r")

Typo: probably should be *cmpdf_fpu_uneq

+case CC_FPUmode:
+  return !((code == LTGT) || (code == UNEQ));
`
strictly speaking, this shouldn't accept unsigned comparisons,
although I can't think of a scenario where these would be presented
in this mode,
and the failure mode would just be an abort in get_arc_condition_code.

Otherwise, this is OK.


Re: [RFC] [PATCH] Add __array_size keyword

2016-02-12 Thread Stuart Brady
On Thu, Feb 11, 2016 at 10:41:02PM +, Joseph Myers wrote:
> For proposed features, I find documentation and testcases of much more 
> value than the rest of the implementation.

I can see the logic of that.  This is the part that I find a little
tricky, so please bear with me here.

> Critical issues to define and cover thoroughly in tests include the
> rules for when operands of sizeof are evaluated, as adapted
> appropriately for this keyword, and for when it returns various kinds
> of constants.

So in other words, adapting all of the sizeof tests would be appropriate,
and sizeof tests for non-array types would change from expected passes to
expected failures?

> Is the rule for your keyword that the operand is evaluated, and the
> result not an integer constant, iff the operand is an array with a
> variable number of elements (as opposed to an array with a constant
> number of elements that themselves are variable-sized, for example)?

If I've understood correctly, then yes:

   #include 
   void foo(int i) {
 int a[i], b[__array_size(a)];
 printf("%zi, %zi\n", __array_size(a), __array_size(b));
   };
   int main() { foo(42); }

This will print "42, 42".

> C11 6.5.3.4#2 (sizeof) would need testing, 

Does this section differ from the September 7th draft in any way?

Perhaps I should draw up an equivalent specification for __array_size
based on 6.5.3.4, only I'm unsure if it would be legal for me to do that?

> but so would 6.9#3 (rules about when declared identifiers need to be 
> defined), and 6.6#6 and #8 (constant expressions) - and anything else in 
> C11 that mentions sizeof.

Well, there are some mentions of sizeof such as those in connection with
offsetof where I do not think any tests or documentation would be needed
but yes, tests would be needed for equivalent behaviour with __array_size
instead of sizeof, for those sections which you mention.

> Presumably this keyword can be applied to an array at function prototype
> scope whose size is explicitly or implicitly [*], though nothing useful
> can be done with the results, as with [*]? (Cf. gcc.dg/vla-5.c.)

I'm not sure I quite understand the meaning of an implicit [*].  Does that
just mean __array_size(foo) with an int foo[*] as another parameter?

I have realised that my patch has problems in this area, which I will
fix, but it still makes sense to me to document the desired functionality
and test cases, so I shall continue along this path.

As a brief aside, I do get an ICE with the following source, without any
modifications of my own:

   int bar() { return foo(); }
   void baz(int c[foo()]) { return; }

I will look into submitting a PR for this properly soon, but will not
mind if someone wants to take this task upon themselves instead,
especially as we are into the release phase for GCC 6 and this may be
an issue worth fixing.  (Note that Debian's GCC 5.3.1 is also affected.)

For a hypothetical change to the C standard itself, I think one might use
the name "_ArraySize", but for a non-standard extension this would not be
appropriate.  I think "__array_size" is fine, though, and is consistent
with "__alignof".

Here's a list of places in the standard that reference sizeof, and the
changes that would need to be made to the standard if this keyword were
to become part of it (using the "wrong" name of __array_size):

   6.2.6.2 43) no change
   6.3.2.1#2: no change
  #3: extended to include __array_size
  #4: no change
   6.4.1#1: add __array_size to keyword list
   6.5.3#1: add __array_size unary-expression,
and __array_size ( type-name )
   6.5.3.4: duplicate the entire section, with considerable changes:
  #1: "shall only be applied to an expression that has array
  type or to the parenthesized name of such a type"
  #2: "yields the number of elements in the array"
  #3: remove entirely
  #5: remove entirely, or provide example using calloc()
  #6: remove entirely, also update original?
  #7: update appropriately
  88) remove?
   6.6#6: extend to include __array_size (both occurrences)
  98) update
  #8: extend to include __array_size (both occurrences)
   6.7.1 103) "only operators [...] are sizeof and __array_size"
   6.7.2.1#17: no change
  #18: no change
  #19: no change
  #20: no change
  #21: no change
  #22: no change
   6.7.3.1#12: no change
   6.7.5.2#5: extend to include __array_size
   6.9#3: extend to include __array_size
   6.9#5: extend to include __array_size
   7.12.3.1#4: no change
   7.17#2: extend to include __array_size
   A.1.2: add __array_size to keyword list
   A.2.1: add __array_size unary-expression,
  and __array_size ( type-name )
   J.1: extend to include __array_size
   J.2: extend to include __array_size (all four occurrences)
   J.3.13: no change

That leaves the matter of dealing with this for C++, Objective C and also
Objective C++, but 

[SH][committed] Fix PR 67636

2016-02-12 Thread Oleg Endo
Hi,

The attached patch fixes PR 67636 in a simple way by adding yet another
bit extraction pattern as mentioned in PR 64345#c3.

Tested on sh-elf with
make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,
-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

Committed to trunk as r233397.

Cheers,
Oleg

gcc/ChangeLog:
PR target/67636
PR target/64345
* config/sh/sh.md (*zero_extract_3): New insn_and_split pattern.


gcc/testsuite/ChangeLog:
PR target/67636
PR target/64345
* gcc.target/sh/pr54236-1.c: Adjust optimization level.Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md	(revision 233345)
+++ gcc/config/sh/sh.md	(working copy)
@@ -14616,6 +14616,31 @@
   [(set (reg:SI T_REG)
 	(zero_extract:SI (match_dup 0) (const_int 1) (match_dup 1)))])
 
+(define_insn_and_split "*zero_extract_3"
+  [(set (match_operand:SI 0 "arith_reg_dest")
+  	(and:SI (lshiftrt:SI (match_operand:SI 1 "arith_reg_operand")
+  			 (match_operand 2 "const_int_operand"))
+  		(match_operand 3 "const_int_operand")))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1 && can_create_pseudo_p ()
+   && exact_log2 (INTVAL (operands[3])) >= 0"
+  "#"
+  "&& 1"
+  [(const_int 0)]
+{
+  int rshift = INTVAL (operands[2]);
+  int lshift = exact_log2 (INTVAL (operands[3]));
+
+  rtx tmp = gen_reg_rtx (SImode);
+  emit_insn (gen_rtx_PARALLEL (VOIDmode,
+gen_rtvec (2,
+  gen_rtx_SET (tmp,
+		   gen_rtx_ZERO_EXTRACT (SImode, operands[1], const1_rtx,
+	 GEN_INT (rshift + lshift))),
+  gen_rtx_CLOBBER (VOIDmode, get_t_reg_rtx ();
+  emit_insn (gen_ashlsi3 (operands[0], tmp, GEN_INT (lshift)));
+})
+
 ;; -
 ;; SH2A instructions for bitwise operations.
 ;; FIXME: Convert multiple instruction insns to insn_and_split.
Index: gcc/testsuite/gcc.target/sh/pr54236-1.c
===
--- gcc/testsuite/gcc.target/sh/pr54236-1.c	(revision 233345)
+++ gcc/testsuite/gcc.target/sh/pr54236-1.c	(working copy)
@@ -2,7 +2,7 @@
special cases.  If everything works as expected we won't see any
movt instructions in these cases.  */
 /* { dg-do compile }  */
-/* { dg-options "-O1" } */
+/* { dg-options "-O2" } */
 /* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */
 /* { dg-final { scan-assembler-times "addc" 6 } } */
 /* { dg-final { scan-assembler-times "subc" 4 } } */