[committed] Unbreak non-ELF target bootstrap (PR bootstrap/89539)

[Jakub Jelinek]

Hi!

The new early_lto_debug argument is right now used solely in
#if defined (OBJECT_FORMAT_ELF)
guarded parts of the function, so on other targets it warns about unused
parameter.

Fixed thusly, committed to trunk as obvious.

2019-03-01  Jakub Jelinek  

PR bootstrap/89539
* dwarf2out.c (output_comdat_type_unit): Add ATTRIBUTE_UNUSED to
early_lto_debug argument.

--- gcc/dwarf2out.c.jj  2019-02-27 15:49:28.567392391 +0100
+++ gcc/dwarf2out.c 2019-03-01 09:04:15.440751912 +0100
@@ -11234,7 +11234,8 @@ output_skeleton_debug_sections (dw_die_r
 /* Output a comdat type unit DIE and its children.  */
 
 static void
-output_comdat_type_unit (comdat_type_node *node, bool early_lto_debug)
+output_comdat_type_unit (comdat_type_node *node,
+bool early_lto_debug ATTRIBUTE_UNUSED)
 {
   const char *secname;
   char *tmp;

Jakub

Re: [patch] Fix wrong code for boolean negation in condition at -O

[Eric Botcazou]

> The BIT_AND_EXPR case is clearly correct for all possible values.  The code
> says that if the result of BIT_AND_EXPR is known to be a non-zero constant,
> and one or both of the BIT_AND_EXPR arguments have known value ranges [0,1]
> (or boolean or precision 1, not talking about those now), then that value
> with the [0,1] range actually had to be 1, otherwise BIT_AND_EXPR result
> would be 0.
> 
> The BIT_NOT_EXPR case is different, ~0 is -1 and ~1 is -2.  If an SSA_NAME
> has [0,1] range, then BIT_NOT_EXPR of that will be [-2,-1].  If value is one
> of those two, then with your today's patch the assumption will be correct,
> 1 or 0.  If value is some other one, which should hopefully happen only in
> dead code that we haven't been able to optimize, then we'd replace
> different values though.

I don't understand the last sentence: we'll precisely replace a bogus value, 
which we know is impossible given the [0, 1] range, by 0 or 1.

-- 
Eric Botcazou

[PATCH GCC10] ipa-inline.c: Trivial fix on function not declared inline check in want_inline_small_function_p

[JunMa]

Hi 
   Since MAX_INLINE_INSNS_AUTO should be below or equal to 
   MAX_INLINE_INSNS_SINGLE (see params.def), there is no need
   to do second inlining limit check on growth when function not
   declared inline, this patch removes it.
   Bootstrapped and tested on x86_64-unknown-linux-gnu, is it ok for trunk?

Regards 
Jun

2019-03-01  Jun Ma  

*ipa-inline.c(want_inline_small_function_p): Remove
redundant growth check when function not declared 
inline

0001-remove-redundant-check-on-growth.patch
Description: Binary data

Re: [PATCH] Fix up --enable-checking=fold (PR middle-end/89503)

[Richard Biener]

On Fri, 1 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> Some of the builtin folding sets TREE_NO_WARNING flags, which triggers as
> --enable-checking=fold errors.  I think we should allow setting of
> TREE_NO_WARNING flag when folding, so this patch arranges that.
> 
> Tested on:
> ../configure --enable-languages=c,c++,fortran --enable-checking=fold 
> --disable-bootstrap
> make -jN && make -jN -k check
> No extra FAILs compared to normal regtest.  Ok for trunk?

Hmm, I guess it's OK though for EXPR_P I think we may want the
TREE_NO_WARNING setter to unshare the tree?  Because IIRC it is
exactly because of tree sharing that we do this kind of checking?

Richard.

> 2019-03-01  Jakub Jelinek  
> 
>   PR middle-end/89503
>   * fold-const.c (fold_checksum_tree): Ignore TREE_NO_WARNING bit
>   on DECL_P and EXPR_P.
> 
> --- gcc/fold-const.c.jj   2019-02-21 00:01:05.714931643 +0100
> +++ gcc/fold-const.c  2019-02-27 12:27:38.749353680 +0100
> @@ -12130,6 +12130,7 @@ fold_checksum_tree (const_tree expr, str
>memcpy ((char *) &buf, expr, tree_size (expr));
>SET_DECL_ASSEMBLER_NAME ((tree)&buf, NULL);
>buf.decl_with_vis.symtab_node = NULL;
> +  buf.base.nowarning_flag = 0;
>expr = (tree) &buf;
>  }
>else if (TREE_CODE_CLASS (code) == tcc_type
> @@ -12155,6 +12156,13 @@ fold_checksum_tree (const_tree expr, str
> TYPE_CACHED_VALUES (tmp) = NULL;
>   }
>  }
> +  else if (TREE_NO_WARNING (expr) && (DECL_P (expr) || EXPR_P (expr)))
> +{
> +  /* Allow TREE_NO_WARNING to be set.  */
> +  memcpy ((char *) &buf, expr, tree_size (expr));
> +  buf.base.nowarning_flag = 0;
> +  expr = (tree) &buf;
> +}
>md5_process_bytes (expr, tree_size (expr), ctx);
>if (CODE_CONTAINS_STRUCT (code, TS_TYPED))
>  fold_checksum_tree (TREE_TYPE (expr), ctx, ht);
> 
>   Jakub
> 
> 

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

Re: [PATCH] Fix up --enable-checking=fold (PR middle-end/89503)

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 09:49:03AM +0100, Richard Biener wrote:
> On Fri, 1 Mar 2019, Jakub Jelinek wrote:
> > Some of the builtin folding sets TREE_NO_WARNING flags, which triggers as
> > --enable-checking=fold errors.  I think we should allow setting of
> > TREE_NO_WARNING flag when folding, so this patch arranges that.
> > 
> > Tested on:
> > ../configure --enable-languages=c,c++,fortran --enable-checking=fold 
> > --disable-bootstrap
> > make -jN && make -jN -k check
> > No extra FAILs compared to normal regtest.  Ok for trunk?
> 
> Hmm, I guess it's OK though for EXPR_P I think we may want the
> TREE_NO_WARNING setter to unshare the tree?  Because IIRC it is
> exactly because of tree sharing that we do this kind of checking?

Maybe, I just wonder if it wouldn't be too expensive.
These TREE_NO_WARNING sets are typically on arguments of builtin functions
so we'd need to unshare the argument (shallow copy) as well as the whole
builtin.  Some spots (usually older) in builtins.c do create new trees,
but some TREE_NO_WARNING setting is hidden even in APIs like c_strlen
which actually doesn't return the argument.  Plus, many of those functions
that set it are used both during expansion (where supposedly we don't care
about modifying in place) and folding.  What a mess?

So, do you prefer to drop this patch, or just take the  || EXPR_P (expr)
part away from it and let Martin (who has added pretty much all of these
TREE_NO_WARNING) fix it later?  c_strlen is probably the oldest with this
issue and most likely very hard to change; the thing is, it is usually used
during expansion and we heavily rely in that case that it modifies it
in-place and thus doesn't warn multiple times.  Just use it also in
fold_builtin_strlen :(.

> > 2019-03-01  Jakub Jelinek  
> > 
> > PR middle-end/89503
> > * fold-const.c (fold_checksum_tree): Ignore TREE_NO_WARNING bit
> > on DECL_P and EXPR_P.

Jakub

Re: [PATCH] Fix up --enable-checking=fold (PR middle-end/89503)

[Richard Biener]

On Fri, 1 Mar 2019, Jakub Jelinek wrote:

> On Fri, Mar 01, 2019 at 09:49:03AM +0100, Richard Biener wrote:
> > On Fri, 1 Mar 2019, Jakub Jelinek wrote:
> > > Some of the builtin folding sets TREE_NO_WARNING flags, which triggers as
> > > --enable-checking=fold errors.  I think we should allow setting of
> > > TREE_NO_WARNING flag when folding, so this patch arranges that.
> > > 
> > > Tested on:
> > > ../configure --enable-languages=c,c++,fortran --enable-checking=fold 
> > > --disable-bootstrap
> > > make -jN && make -jN -k check
> > > No extra FAILs compared to normal regtest.  Ok for trunk?
> > 
> > Hmm, I guess it's OK though for EXPR_P I think we may want the
> > TREE_NO_WARNING setter to unshare the tree?  Because IIRC it is
> > exactly because of tree sharing that we do this kind of checking?
> 
> Maybe, I just wonder if it wouldn't be too expensive.
> These TREE_NO_WARNING sets are typically on arguments of builtin functions
> so we'd need to unshare the argument (shallow copy) as well as the whole
> builtin.  Some spots (usually older) in builtins.c do create new trees,
> but some TREE_NO_WARNING setting is hidden even in APIs like c_strlen
> which actually doesn't return the argument.  Plus, many of those functions
> that set it are used both during expansion (where supposedly we don't care
> about modifying in place) and folding.  What a mess?

Uh.

> So, do you prefer to drop this patch, or just take the  || EXPR_P (expr)
> part away from it and let Martin (who has added pretty much all of these
> TREE_NO_WARNING) fix it later?  c_strlen is probably the oldest with this
> issue and most likely very hard to change; the thing is, it is usually used
> during expansion and we heavily rely in that case that it modifies it
> in-place and thus doesn't warn multiple times.  Just use it also in
> fold_builtin_strlen :(.

No, as said the patch is probably OK (setting TREE_NO_WARNING is harmless
from a tree-sharing perspective but still undesired IMHO).

Maybe you can add a comment reflecting the above and open a bug so we
can eventually revisit this in future.

Thanks,
Richard.

> > > 2019-03-01  Jakub Jelinek  
> > > 
> > >   PR middle-end/89503
> > >   * fold-const.c (fold_checksum_tree): Ignore TREE_NO_WARNING bit
> > >   on DECL_P and EXPR_P.
> 
>   Jakub

Re: [PATCH] Fix PR89497

[Richard Biener]

On Wed, 27 Feb 2019, Richard Biener wrote:

> 
> CFG cleanup is now set up to perform trivial unreachable code
> elimination before doing anything that would require up-to-date
> SSA form.  Unfortunately a pending SSA update still will cause
> breakage to stmt folding triggered for example by basic-block
> merging.
> 
> Fortunately it's now easy to properly "interleave" CFG cleanup
> and SSA update.
> 
> Done as follows, bootstrap & regtest running on x86_64-unknown-linux-gnu.

Testing went OK, two testcases need adjustments though.

FAIL: gcc.dg/tree-ssa/reassoc-43.c scan-tree-dump-not reassoc2 "0 != 0"

here we now end up with if (_20 != 0) matching.

FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding virtual 
function call to virtual unsigned int mozPersonalDictionary::AddRef" 1
FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding virtual 
function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1

here both foldings now alrady happen one pass earlier (during tracer
triggered CFG cleanup).  Previously the folding didn't happen because
the SSA names were marked for SSA update.

Committed as follows.

Richard.

2019-03-01  Richard Biener  

PR middle-end/89497
* tree-cfgcleanup.h (cleanup_tree_cfg): Add SSA update flags
argument, defaulted to zero.
* passes.c (execute_function_todo): Pass down SSA update flags
to cleanup_tree_cfg.
* tree-cfgcleanup.c: Include tree-into-ssa.h and tree-cfgcleanup.h.
(cleanup_tree_cfg_noloop): After cleanup_control_flow_pre update SSA
form if requested.
(cleanup_tree_cfg): Get and pass down SSA update flags.

* gcc.dg/tree-ssa/reassoc-43.c: Avoid false match in regex.
* g++.dg/tree-prof/devirt.C: Scan tracer dump for foldings
that happen now earlier.

Index: gcc/tree-cfgcleanup.h
===
--- gcc/tree-cfgcleanup.h   (revision 269251)
+++ gcc/tree-cfgcleanup.h   (working copy)
@@ -22,7 +22,7 @@ along with GCC; see the file COPYING3.
 
 /* In tree-cfgcleanup.c  */
 extern bitmap cfgcleanup_altered_bbs;
-extern bool cleanup_tree_cfg (void);
+extern bool cleanup_tree_cfg (unsigned = 0);
 extern bool fixup_noreturn_call (gimple *stmt);
 extern bool delete_unreachable_blocks_update_callgraph (cgraph_node *dst_node,
bool update_clones);
Index: gcc/passes.c
===
--- gcc/passes.c(revision 269251)
+++ gcc/passes.c(working copy)
@@ -1927,7 +1927,7 @@ execute_function_todo (function *fn, voi
   /* Always cleanup the CFG before trying to update SSA.  */
   if (flags & TODO_cleanup_cfg)
 {
-  cleanup_tree_cfg ();
+  cleanup_tree_cfg (flags & TODO_update_ssa_any);
 
   /* When cleanup_tree_cfg merges consecutive blocks, it may
 perform some simplistic propagation when removing single
Index: gcc/tree-cfgcleanup.c
===
--- gcc/tree-cfgcleanup.c   (revision 269251)
+++ gcc/tree-cfgcleanup.c   (working copy)
@@ -44,6 +44,9 @@ along with GCC; see the file COPYING3.
 #include "gimple-fold.h"
 #include "tree-ssa-loop-niter.h"
 #include "cgraph.h"
+#include "tree-into-ssa.h"
+#include "tree-cfgcleanup.h"
+
 
 /* The set of blocks in that at least one of the following changes happened:
-- the statement at the end of the block was changed
@@ -943,7 +946,7 @@ mfb_keep_latches (edge e)
Return true if the flowgraph was modified, false otherwise.  */
 
 static bool
-cleanup_tree_cfg_noloop (void)
+cleanup_tree_cfg_noloop (unsigned ssa_update_flags)
 {
   timevar_push (TV_TREE_CLEANUP_CFG);
 
@@ -1023,6 +1026,8 @@ cleanup_tree_cfg_noloop (void)
 
   /* After doing the above SSA form should be valid (or an update SSA
  should be required).  */
+  if (ssa_update_flags)
+update_ssa (ssa_update_flags);
 
   /* Compute dominator info which we need for the iterative process below.  */
   if (!dom_info_available_p (CDI_DOMINATORS))
@@ -1125,9 +1130,9 @@ repair_loop_structures (void)
 /* Cleanup cfg and repair loop structures.  */
 
 bool
-cleanup_tree_cfg (void)
+cleanup_tree_cfg (unsigned ssa_update_flags)
 {
-  bool changed = cleanup_tree_cfg_noloop ();
+  bool changed = cleanup_tree_cfg_noloop (ssa_update_flags);
 
   if (current_loops != NULL
   && loops_state_satisfies_p (LOOPS_NEED_FIXUP))
Index: gcc/testsuite/g++.dg/tree-prof/devirt.C
===
--- gcc/testsuite/g++.dg/tree-prof/devirt.C (revision 269301)
+++ gcc/testsuite/g++.dg/tree-prof/devirt.C (working copy)
@@ -1,5 +1,5 @@
 /* PR ipa/88561 */
-/* { dg-options "-O3 -fdump-tree-dom3-details" } */
+/* { dg-options "-O3 -fdump-tree-tracer-details -fdump-tree-dom3-details" } */
 
 struct nsISupports
 {
@@ -121,6 +121,6 @@ main ()
 __buil

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Kyrill Tkachov]

Hi Jakub,

On 2/28/19 9:43 PM, Jakub Jelinek wrote:

Hi!

The following testcase ICEs on ARM, because the backend creates CONST_INTs
that aren't valid for SImode, in which they are used (0x8000 rather than
the canonical -0x8000).  This is fixed by the 3 gen_int_mode calls
instead of just GEN_INT.
Once that is fixed, we ICE because if both the CONST_INTs are -0x8000,
then INTVAL (operands[2]) == -INTVAL (operands[3]) is no longer true and
thus cmpsi2_addneg doesn't match and no other pattern does.
Fixing that is pretty obvious thing to do as well, similarly to the recent
*subsi3_carryin_compare_const fix.

The next problem is that the result doesn't bootstrap.  The problem is that
the instruction emitted for that -0x8000 immediate - adds reg, reg, 
#-0x8000
actually doesn't do what the RTL pattern for it says.
The pattern is like subsi3_compare, except that the MINUS with
constant second argument is canonicalized as RTL normally does into PLUS of
the negated constant.  The mode on the condition code register is CCmode,
so all Z, N, C and V bits should be valid, and the pattern is like that of
a cmp instruction, so the behavior vs. condition codes should be like cmp
instruction, which is what the subs instruction does.  The adds r1, r2, #N
and subs r1, r2, #-N instructions actually behave the same most of the time.
The result is the same, Z and N flags are always the same as well.  And,
it seems that for all N except for 0 and 0x8000 also the C and V bits
are the same (I've proved that by using the valgrind subs condition code
algorithm (which is the same as cmp) vs. valgrind adds condition code
algorithm (which is the same as cmn) and even emulated it using 8-bit x
8-bit exhaustive check).  For 0 and 0x8000 it is different and as can be
seen by the bootstrap failure, it is significant.
Previously before the above change we got by by the pattern never triggering
by the combiner for 0x8000 (because the combiner would always try
canonical CONST_INTs for both and those don't have INTVAL == -INTVAL), but
it worked for the *compare_scc splitter/*negscc/*thumb2_negscc patterns
(which created invalid RTL and used adds rather than subs, but didn't care,
as it only tested the Z bit using ne condition).

My next attempt was just to switch the two alternatives, so that if
operands[2] matches both "I" and "L" constraints and we can choose, we'd
use subs.  That cured the bootstrap failure, but introduced
+FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
+FAIL: gcc.target/arm/thumb2-cmpneg2add-2.c scan-assembler adds
regressions, in those 2 testcases neither the problematic 0 nor INT_MIN
is used, so both adds and subs are equivalent, but
-   addsr2, r4, #1
+   subsr2, r4, #-1
results in larger code.
Note, I've seen the patch creating smaller code in some cases too,
when the combiner matched the cmpsi2_addneg pattern with INT_MIN and
previously emitted loading the constant into some register and performing
subs with 3 registers instead of 2 and immediate.

So, this is another alternative I'm proposing, just make sure we use
subs for #0 and #-2147483648 (where it is essential for correctness)
and for others keep previous behavior.

Ok for trunk?


Ok.




Or is there an easy way to estimate if a constant satisfies both "I" and "L"
constraints at the same time and is not one of 0 and INT_MIN, which
of the two (positive or negated) would have smaller encoding and decide
based on that?


Don't think there's a cleaner way of doing that. There are some helper 
functions in arm.c that can estimate the number instructions neeedd to 
synthesise a constant, but nothing that takes size into account. The 
encoding size can only change for TARGET_THUMB2 (not TARGET_ARM) but 
it's not worth gating that decision on TARGET_THUMB2 as well IMO.


Thanks,

Kyrill



2019-02-28  Jakub Jelinek  

PR target/89506
* config/arm/arm.md (cmpsi2_addneg): Use
trunc_int_for_mode (-INTVAL (...), SImode) instead of -INTVAL (...).
If operands[2] is 0 or INT_MIN, force use of subs.
(*compare_scc splitter): Use gen_int_mode.
(*negscc): Likewise.
* config/arm/thumb2.md (*thumb2_negscc): Likewise.

* gcc.dg/pr89506.c: New test.

--- gcc/config/arm/arm.md.jj2019-02-28 14:13:08.368267536 +0100
+++ gcc/config/arm/arm.md   2019-02-28 22:09:03.089588596 +0100
@@ -867,10 +867,21 @@ (define_insn "cmpsi2_addneg"
 (set (match_operand:SI 0 "s_register_operand" "=r,r")
(plus:SI (match_dup 1)
 (match_operand:SI 3 "arm_addimm_operand" "I,L")))]
-  "TARGET_32BIT && INTVAL (operands[2]) == -INTVAL (operands[3])"
-  "@
-   adds%?\\t%0, %1, %3
-   subs%?\\t%0, %1, #%n3"
+  "TARGET_32BIT
+   && (INTVAL (operands[2])
+   == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
+{
+  /* For 0 and INT_MIN it is essential that we use subs, as adds
+ will result in different condition codes (like cmn rather than
+ li

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 09:21:16AM +, Kyrill Tkachov wrote:
> > Ok for trunk?
> 
> Ok.

Thanks.  I'll wait for my regtest, previously I've regtested it only with
an older version of this patch.

> > Or is there an easy way to estimate if a constant satisfies both "I" and "L"
> > constraints at the same time and is not one of 0 and INT_MIN, which
> > of the two (positive or negated) would have smaller encoding and decide
> > based on that?
> 
> Don't think there's a cleaner way of doing that. There are some helper
> functions in arm.c that can estimate the number instructions neeedd to
> synthesise a constant, but nothing that takes size into account. The
> encoding size can only change for TARGET_THUMB2 (not TARGET_ARM) but it's
> not worth gating that decision on TARGET_THUMB2 as well IMO.

Well, with the patch the decision which insn is chosen is done in C code.
So it could look like:
  if (operands[2] == const0_rtx
  || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
subs; // mandatory
  else if (TARGET_THUMB2
   && arm_immediate_operand (operands[2], SImode)
   && arm_neg_immediate_operand (operands[2], SImode))
{
  // Both adds and subs can do the job here, and unlike
  // non-thumb mode, the instructions can have different
  // sizes.  Pick the shorter one.
}
  else if (which_alternative == 1)
subs;
  else
adds;

This can be done incrementally, I just have no idea what the rules
for thumb2 constant encoding are.

Jakub

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Kyrill Tkachov]

On 3/1/19 9:36 AM, Jakub Jelinek wrote:

On Fri, Mar 01, 2019 at 09:21:16AM +, Kyrill Tkachov wrote:

Ok for trunk?

Ok.

Thanks.  I'll wait for my regtest, previously I've regtested it only with
an older version of this patch.


Or is there an easy way to estimate if a constant satisfies both "I" and "L"
constraints at the same time and is not one of 0 and INT_MIN, which
of the two (positive or negated) would have smaller encoding and decide
based on that?

Don't think there's a cleaner way of doing that. There are some helper
functions in arm.c that can estimate the number instructions neeedd to
synthesise a constant, but nothing that takes size into account. The
encoding size can only change for TARGET_THUMB2 (not TARGET_ARM) but it's
not worth gating that decision on TARGET_THUMB2 as well IMO.

Well, with the patch the decision which insn is chosen is done in C code.
So it could look like:
   if (operands[2] == const0_rtx
   || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
 subs; // mandatory
   else if (TARGET_THUMB2
   && arm_immediate_operand (operands[2], SImode)
   && arm_neg_immediate_operand (operands[2], SImode))
 {
   // Both adds and subs can do the job here, and unlike
   // non-thumb mode, the instructions can have different
   // sizes.  Pick the shorter one.
 }
   else if (which_alternative == 1)
 subs;
   else
 adds;

This can be done incrementally, I just have no idea what the rules
for thumb2 constant encoding are.



Yeah, I had considered this. But I don't think it's worth the extra 
code. I'm not aware of any implementation where adds and subs differ in 
performance and I'd be surprised so when there's no size advantage I 
wouldn't be picky.


Thanks,

Kyrill




Jakub

Re: [PATCH] PR d/89177 - Fix unaligned access in std.digest.murmurhash

[Iain Buclaw]

On Sun, 24 Feb 2019 at 16:30, Johannes Pfau  wrote:
>
> Backport latest murmurhash version from upstream (2.084.1). Ran gdc testsuite
> on X86_64 linux and got feedback on the bugzilla this really fixes the issue.
>

Raise a pull request with upstream (dmd-cxx is the branch), then this
is OK to commit.

-- 
Iain

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Kyrill Tkachov]

On 3/1/19 9:42 AM, Kyrill Tkachov wrote:


On 3/1/19 9:36 AM, Jakub Jelinek wrote:
> On Fri, Mar 01, 2019 at 09:21:16AM +, Kyrill Tkachov wrote:
>>> Ok for trunk?
>> Ok.
> Thanks.  I'll wait for my regtest, previously I've regtested it only 
with

> an older version of this patch.
>
>>> Or is there an easy way to estimate if a constant satisfies both 
"I" and "L"

>>> constraints at the same time and is not one of 0 and INT_MIN, which
>>> of the two (positive or negated) would have smaller encoding and 
decide

>>> based on that?
>> Don't think there's a cleaner way of doing that. There are some helper
>> functions in arm.c that can estimate the number instructions neeedd to
>> synthesise a constant, but nothing that takes size into account. The
>> encoding size can only change for TARGET_THUMB2 (not TARGET_ARM) 
but it's

>> not worth gating that decision on TARGET_THUMB2 as well IMO.
> Well, with the patch the decision which insn is chosen is done in C 
code.

> So it could look like:
>    if (operands[2] == const0_rtx
>    || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
>  subs; // mandatory
>    else if (TARGET_THUMB2
>   && arm_immediate_operand (operands[2], SImode)
>   && arm_neg_immediate_operand (operands[2], SImode))
>  {
>    // Both adds and subs can do the job here, and unlike
>    // non-thumb mode, the instructions can have different
>    // sizes.  Pick the shorter one.
>  }
>    else if (which_alternative == 1)
>  subs;
>    else
>  adds;
>
> This can be done incrementally, I just have no idea what the rules
> for thumb2 constant encoding are.


Yeah, I had considered this. But I don't think it's worth the extra
code. I'm not aware of any implementation where adds and subs differ in
performance and I'd be surprised so when there's no size advantage I
wouldn't be picky.



Two thoughts collided in my head on the last sentence. I meant to say:

"I'm not aware of any implementation where adds and subs differ in
performance so when there's no size advantage I wouldn't be picky."

Thanks,

Kyrill



Thanks,

Kyrill


>
>    Jakub

Re: [arm][RFC] PR target/88469 fix incorrect argument passing with 64-bit bitfields

[Richard Earnshaw (lists)]

On 28/02/2019 14:51, Bernd Edlinger wrote:
> On 2/28/19 1:10 PM, Richard Earnshaw (lists) wrote:
>> On 27/01/2019 11:20, Bernd Edlinger wrote:
>>>
>>> $ arm-linux-gnueabihf-gcc -march=armv5te -O3 -S test.c
>>> $ cat test.s
>>> f:
>>> @ args = 12, pretend = 0, frame = 0
>>> @ frame_needed = 0, uses_anonymous_args = 0
>>> @ link register save eliminated.
>>> push{r4, r5}
>>> mov r0, #8
>>> ldrdr4, [sp, #12]
>>
>> So this is wrong; before the compiler can use 'f' it has to copy it to a
>> suitably aligned location; it can't directly reuse the value on the
>> stack as that is not sufficiently aligned for the type.
>>
> 
> Yes, meanwhile I found out that the value would be copied in a new place
> if an address was taken, but there is an issue with
> output_move_double not checking the value's MEM_ALIGN.

I think that's a symptom of an earlier problem: why is gen_movdi being
called with a 32-bit aligned DImode memory object?

R.

> 
>>>
>>> So isn't this wrong code, returning 8 for alignof when it is really 4,
>>> and wouldn't it crash on armv5 and armv6 with SCTLR.U=0 ?
>>
>> Returning 8 is correct; since that is the alignment of the type; but GCC
>> does need to copy underaligned types to suitably aligned memory before
>> it uses them; it must not use the *value* that is passed directly,
>> unless it can prove that doing so is safe (and as you point out, on
>> armv5 it is not).
>>
>> I think technically, this is separate bug from the PCS one that was
>> fixed, so needs a new PR.
>>
> 
> Could you have a look at the patch I sent to fix the wrong code issue:
> https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00248.html
> 
> Is there a chance that this can still go into gcc-9?
> Or do I have to to open a PR for it first?
> 
> 
> Thanks
> Bernd.
> 

[PATCH] Fix PR89541

[Richard Biener]

This follows last years change of allowing virtual operands for 
STRING_CSTs.  Here we run into stores into CONST_DECLs.  For
IL hygiene these are regular memory references (that they
trigger undefined behavior at runtime is another issue).

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-03-01  Richard Biener  

PR middle-end/89541
* tree-ssa-operands.c (add_stmt_operand): CONST_DECL may
get virtual operands.
(get_expr_operands): Handle CONST_DECL like other decls.

* gfortran.dg/pr89451.f90: New testcase.

Index: gcc/tree-ssa-operands.c
===
--- gcc/tree-ssa-operands.c (revision 269301)
+++ gcc/tree-ssa-operands.c (working copy)
@@ -515,7 +515,9 @@ add_stmt_operand (struct function *fn, t
 {
   tree var = *var_p;
 
-  gcc_assert (SSA_VAR_P (*var_p) || TREE_CODE (*var_p) == STRING_CST);
+  gcc_assert (SSA_VAR_P (*var_p)
+ || TREE_CODE (*var_p) == STRING_CST
+ || TREE_CODE (*var_p) == CONST_DECL);
 
   if (is_gimple_reg (var))
 {
@@ -741,6 +743,7 @@ get_expr_operands (struct function *fn,
 case PARM_DECL:
 case RESULT_DECL:
 case STRING_CST:
+case CONST_DECL:
   if (!(flags & opf_address_taken))
add_stmt_operand (fn, expr_p, stmt, flags);
   return;
@@ -859,7 +862,6 @@ get_expr_operands (struct function *fn,
 
 case FUNCTION_DECL:
 case LABEL_DECL:
-case CONST_DECL:
 case CASE_LABEL_EXPR:
   /* Expressions that make no memory references.  */
   return;
Index: gcc/testsuite/gfortran.dg/pr89451.f90
===
--- gcc/testsuite/gfortran.dg/pr89451.f90   (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr89451.f90   (working copy)
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! { dg-options "-O2" }
+program lh
+  call za(0)
+  call za(0)
+contains
+  subroutine za(wl)
+integer wl
+wl = 1
+  end subroutine za
+end program lh

[libgo] Properly determine executable path on Solaris

[Rainer Orth]

One of the remaining libgo testsuite failures on Solaris/SPARC is

--- FAIL: TestExecutable (0.04s)
executable_test.go:46: exec(self) failed: fork/exec .: permission denied

FAIL: os

This happens only for 64-bit.  truss indeed shows

3181:   execve(".", 0xC000170240, 0xC000178340) Err#13 EACCES

which is completely bogus.  Further investigation shows that
os.Executable() returns an empty string here.  I traced this down to
go/runtime/os3_solaris.go (solarisExecutablePath) which tries to locate
auxv from argv (I suppose the layout is prescribed by the psABIs, but
haven't looked) and extract the AT_PAGESZ and AT_SUN_EXECNAME members.

In doing so, it assumes that auxv ist just an uintptr[], which is wrong:
 has

typedef struct
{
int a_type;
union {
longa_val;
void*a_ptr;
void(*a_fcn)();
} a_un;
} auxv_t;

Interpreting this as uintptr[] works for 32-bit and accidentally on
little-endian 64-bit (amd64), but breaks on big-endian 64-bit (sparcv9)
as observed.

While this could be corrected, there's a far easier and more portable
way to get at the required information: AT_PAGESZ/pysPageSize can be
obtained via getpagesize(3C) and AT_SUN_EXECNAME/executablePath is
available via getexecname(3C), both of which are available as far back
as Solaris 10 at least.

The following patch does just that.  Tested on i386-pc-solaris2.11 and
sparc-sun-solaris2.11 (both 32 and 64-bit) without regressions, but
fixing the os failure on sparcv9.  I'm running Solaris 10 bootstraps
right now for good measure, but don't expect any issues there.

Rainer

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


2019-02-28  Rainer Orth  

* go/runtime/os3_solaris.go: Don't import "runtime/internal/sys".
(sysargs): Remove auxv determination.
Don't call sysauxv.
Determine physPageSize using getpagesize, executablePath using
getexecnam.
(getexecname, getpagesize): Declare.
(sysauxv): Remove.

# HG changeset patch
# Parent  3ec7f28626855dacbeef9ee1f5b470388110e16a
Properly determine executable path on Solaris

diff --git a/libgo/go/runtime/os3_solaris.go b/libgo/go/runtime/os3_solaris.go
--- a/libgo/go/runtime/os3_solaris.go
+++ b/libgo/go/runtime/os3_solaris.go
@@ -4,45 +4,19 @@
 
 package runtime
 
-import (
-	"runtime/internal/sys"
-	"unsafe"
-)
+import _ "unsafe"
 
 var executablePath string
 
-func sysargs(argc int32, argv **byte) {
-	n := argc + 1
-
-	// skip over argv, envp to get to auxv
-	for argv_index(argv, n) != nil {
-		n++
-	}
-
-	// skip NULL separator
-	n++
-
-	// now argv+n is auxv
-	auxv := (*[1 << 28]uintptr)(add(unsafe.Pointer(argv), uintptr(n)*sys.PtrSize))
-	sysauxv(auxv[:])
-}
+//extern getexecname
+func getexecname() *byte
 
-const (
-	_AT_NULL = 0// Terminates the vector
-	_AT_PAGESZ   = 6// Page size in bytes
-	_AT_SUN_EXECNAME = 2014 // exec() path name
-)
+//extern getpagesize
+func getpagesize() int32
 
-func sysauxv(auxv []uintptr) {
-	for i := 0; auxv[i] != _AT_NULL; i += 2 {
-		tag, val := auxv[i], auxv[i+1]
-		switch tag {
-		case _AT_PAGESZ:
-			physPageSize = val
-		case _AT_SUN_EXECNAME:
-			executablePath = gostringnocopy((*byte)(unsafe.Pointer(val)))
-		}
-	}
+func sysargs(argc int32, argv **byte) {
+	physPageSize = uintptr(getpagesize())
+	executablePath = gostringnocopy(getexecname())
 }
 
 //go:linkname solarisExecutablePath os.solarisExecutablePath

Re: [PATCH] [ARC] Fix logic set UNALIGNED_ACCESS

[Andrew Burgess]

* Vineet Gupta  [2019-02-28 10:04:24 -0800]:

> From: Claudiu Zissulescu 
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * config/arc/arc-c.def (__ARC_UNALIGNED__): Set it on
>   unaligned_access variable.
>   * config/arc/arc.c (arc_override_options): Set unaligned access
>   default on for HS CPUs.
>   * config/arc/arc.h (STRICT_ALIGNMENT): Fix logic.

This looks good.

Thanks,
Andrew


> 
> Signed-off-by: Vineet Gupta 
> ---
>  gcc/config/arc/arc-c.def | 2 +-
>  gcc/config/arc/arc.c | 4 
>  gcc/config/arc/arc.h | 2 +-
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/arc/arc-c.def b/gcc/config/arc/arc-c.def
> index 9c0ba2c9c294..9472b48aa7bc 100644
> --- a/gcc/config/arc/arc-c.def
> +++ b/gcc/config/arc/arc-c.def
> @@ -29,7 +29,7 @@ ARC_C_DEF ("__ARC_MUL64__", TARGET_MUL64_SET)
>  ARC_C_DEF ("__ARC_MUL32BY16__", TARGET_MULMAC_32BY16_SET)
>  ARC_C_DEF ("__ARC_SIMD__",   TARGET_SIMD_SET)
>  ARC_C_DEF ("__ARC_RF16__",   TARGET_RF16)
> -ARC_C_DEF ("__ARC_UNALIGNED__", !STRICT_ALIGNMENT)
> +ARC_C_DEF ("__ARC_UNALIGNED__", unaligned_access)
>  
>  ARC_C_DEF ("__ARC_BARREL_SHIFTER__", TARGET_BARREL_SHIFTER)
>  
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index de2c8d5df9cf..3b8c29981b9a 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -1313,6 +1313,10 @@ arc_override_options (void)
>if (TARGET_LONG_CALLS_SET)
>  target_flags &= ~MASK_MILLICODE_THUNK_SET;
>  
> +  /* Set unaligned to all HS cpus.  */
> +  if (!global_options_set.x_unaligned_access && TARGET_HS)
> +unaligned_access = 1;
> +
>/* These need to be done at start up.  It's convenient to do them here.  */
>arc_init ();
>  }
> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> index 12b4b62bba7b..894eb3946000 100644
> --- a/gcc/config/arc/arc.h
> +++ b/gcc/config/arc/arc.h
> @@ -285,7 +285,7 @@ if (GET_MODE_CLASS (MODE) == MODE_INT \
>  /* On the ARC the lower address bits are masked to 0 as necessary.  The chip
> won't croak when given an unaligned address, but the insn will still fail
> to produce the correct result.  */
> -#define STRICT_ALIGNMENT (!unaligned_access && !TARGET_HS)
> +#define STRICT_ALIGNMENT (!unaligned_access)
>  
>  /* Layout of source language data types.  */
>  
> -- 
> 2.7.4
> 

[PATCH, d] Committed merge with upstream dmd

[Iain Buclaw]

Hi,

This patch merges the D front-end implementation with dmd upstream ed71446aa.

Backports support for extern(C++, "namespace"), which makes the
core.stdcpp package compilable.

Added predefined condition for CppRuntime_Gcc unconditionally, as it
is unlikely that D code will be linking to anything other than
libstdc++ when extern(C++) is used.

Bootstrapped and regression tested on x86_64-linux-gnu.

Committed to trunk as r269304.

-- 
Iain
---

gcc/d/ChangeLog:

2019-03-01  Iain Buclaw  

* d-builtins.cc (d_init_versions): Add CppRuntime_Gcc as predefined
version condition.

---
diff --git a/gcc/d/d-builtins.cc b/gcc/d/d-builtins.cc
index b0a315a3ed9..f263aafbd59 100644
--- a/gcc/d/d-builtins.cc
+++ b/gcc/d/d-builtins.cc
@@ -452,6 +452,8 @@ d_init_versions (void)
   /* Emit all target-specific version identifiers.  */
   targetdm.d_cpu_versions ();
   targetdm.d_os_versions ();
+
+  VersionCondition::addPredefinedGlobalIdent ("CppRuntime_Gcc");
 }
 
 /* A helper for d_build_builtins_module.  Return a new ALIAS for TYPE.
diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 8b377015129..97aa40d1ace 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-39edbe17e7b5c761d780c9d1d4376a06df7bf3d8
+ed71446aaa2bd0e548c3bf2154a638826dfe3db0
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/cond.c b/gcc/d/dmd/cond.c
index 8df226fa9b6..9d7df5fd240 100644
--- a/gcc/d/dmd/cond.c
+++ b/gcc/d/dmd/cond.c
@@ -171,6 +171,8 @@ static bool isReserved(const char *ident)
 "SysV4",
 "Hurd",
 "Android",
+"PlayStation",
+"PlayStation4",
 "Cygwin",
 "MinGW",
 "FreeStanding",
@@ -197,8 +199,11 @@ static bool isReserved(const char *ident)
 "MIPS_EABI",
 "MIPS_SoftFloat",
 "MIPS_HardFloat",
+"MSP430",
 "NVPTX",
 "NVPTX64",
+"RISCV32",
+"RISCV64",
 "SPARC",
 "SPARC_V8Plus",
 "SPARC_SoftFloat",
@@ -219,6 +224,13 @@ static bool isReserved(const char *ident)
 "CRuntime_Digitalmars",
 "CRuntime_Glibc",
 "CRuntime_Microsoft",
+"CRuntime_Musl",
+"CRuntime_UClibc",
+"CppRuntime_Clang",
+"CppRuntime_DigitalMars",
+"CppRuntime_Gcc",
+"CppRuntime_Microsoft",
+"CppRuntime_Sun",
 "D_Coverage",
 "D_Ddoc",
 "D_InlineAsm_X86",
diff --git a/gcc/d/dmd/cppmangle.c b/gcc/d/dmd/cppmangle.c
index e26f7647d3c..b991417c35e 100644
--- a/gcc/d/dmd/cppmangle.c
+++ b/gcc/d/dmd/cppmangle.c
@@ -196,8 +196,8 @@ class CppMangleVisitor : public Visitor
 Expression *e = isExpression(o);
 if (d && d->isFuncDeclaration())
 {
-bool is_nested = d->toParent() &&
-!d->toParent()->isModule() &&
+bool is_nested = d->toParent3() &&
+!d->toParent3()->isModule() &&
 ((TypeFunction*)d->isFuncDeclaration()->type)->linkage == LINKcpp;
 if (is_nested)
 buf->writeByte('X');
@@ -271,7 +271,7 @@ class CppMangleVisitor : public Visitor
  */
 Dsymbol *getInstance(Dsymbol *s)
 {
-Dsymbol *p = s->toParent();
+Dsymbol *p = s->toParent3();
 if (p)
 {
 if (TemplateInstance *ti = p->isTemplateInstance())
@@ -292,7 +292,7 @@ class CppMangleVisitor : public Visitor
  */
 static Dsymbol *getQualifier(Dsymbol *s)
 {
-Dsymbol *p = s->toParent();
+Dsymbol *p = s->toParent3();
 return (p && !p->isModule()) ? p : NULL;
 }
 
@@ -324,7 +324,7 @@ class CppMangleVisitor : public Visitor
 Dsymbol *s = ((TypeStruct*)t)->toDsymbol(NULL);
 if (s->ident != ident)
 return false;
-Dsymbol *p = s->toParent();
+Dsymbol *p = s->toParent3();
 if (!p)
 return false;
 TemplateInstance *ti = p->isTemplateInstance();
@@ -427,7 +427,7 @@ class CppMangleVisitor : public Visitor
 void cpp_mangle_name(Dsymbol *s, bool qualified)
 {
 //printf("cpp_mangle_name(%s, %d)\n", s->toChars(), qualified);
-Dsymbol *p = s->toParent();
+Dsymbol *p = s->toParent3();
 Dsymbol *se = s;
 bool write_prefix = true;
 if (p && p->isTemplateInstance())
@@ -435,7 +435,7 @@ class CppMangleVisitor : public Visitor
 se = p;
 if (find(p->isTemplateInstance()->tempdecl) >= 0)
 write_prefix = false;
-p = p->toParent();
+p = p->toParent3();
 }
 
 if (p && !p->isModule())
@@ -521,7 +521,7 @@ class CppMangleVisitor : public Visitor
 fatal();
 }
 
-Dsymbol *p = d->toParent();
+Dsymbol *p = d->toParent3();
 if (p && !p->isModule()) //for exampl

Re: [PATCH] Fix not 8-byte aligned ldrd/strd on ARMv5

[Richard Earnshaw (lists)]

On 05/02/2019 15:07, Bernd Edlinger wrote:
> Hi,
> 
> due to the AAPCS parameter passing of 8-byte aligned structures, which happen 
> to
> be 8-byte aligned or only 4-byte aligned in the test case, ldrd instructions
> are generated that may access 4-byte aligned stack slots, which will trap on 
> ARMv5 and
> ARMv6 according to the following document:
> 
> 
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473m/dom1361290002364.html
> says:
> 
> "In ARMv5TE, or in ARMv6 when SCTLR.U is 0, LDRD and STRD doubleword data 
> transfers must be
> eight-byte aligned.  Use ALIGN 8 before memory allocation directives such as 
> DCQ if the data
> is to be accessed using LDRD or STRD.  This is not required in ARMv6 when 
> SCTLR.U is 1, or in
> ARMv7, because in these versions, doubleword data transfers can be 
> word-aligned."
> 
> 
> The reason why the ldrd instruction is generated seems to be a missing 
> alignment check in the
> function output_move_double.  But when that is fixed, it turns out that if 
> the parameter happens
> to be 8-byte aligned by chance, they still have MEM_ALIGN = 4, which prevents 
> the ldrd completely.
> 
> The reason for that is in function.c (assign_parm_find_stack_rtl), where 
> values that happen to be
> aligned to STACK_BOUNDARY, are only  aligned to PARM_BOUNDARY.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu and arm-linux-gnueabihf 
> with all languages.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-arm-align-abi.diff
> 
> 2019-02-05  Bernd Edlinger  
> 
>   * config/arm/arm.c (output_move_double): Check required memory
>   alignment for ldrd/strd instructions.
>   * function.c (assign_parm_find_stack_rtl): Use larger alignment
>   when possible.
> 
> testsuite:
> 2019-02-05  Bernd Edlinger  
> 
>   * gcc.target/arm/unaligned-argument-1.c: New test.
>   * gcc.target/arm/unaligned-argument-2.c: New test.
> 
> Index: gcc/config/arm/arm.c
> ===
> --- gcc/config/arm/arm.c  (revision 268337)
> +++ gcc/config/arm/arm.c  (working copy)
> @@ -18303,6 +18303,8 @@ output_move_double (rtx *operands, bool emit, int
>otherops[0] = gen_rtx_REG (SImode, 1 + reg0);
>  
>gcc_assert (code1 == MEM);  /* Constraints should ensure this.  */
> +  bool allow_ldrd = TARGET_LDRD
> + && align_ok_ldrd_strd (MEM_ALIGN (operands[1]), 0);
>  
>switch (GET_CODE (XEXP (operands[1], 0)))
>   {
> @@ -18310,8 +18312,8 @@ output_move_double (rtx *operands, bool emit, int
>  
> if (emit)
>   {
> -   if (TARGET_LDRD
> -   && !(fix_cm3_ldrd && reg0 == REGNO(XEXP (operands[1], 0
> +   if (allow_ldrd
> +   && !(fix_cm3_ldrd && reg0 == REGNO (XEXP (operands[1], 0
>   output_asm_insn ("ldrd%?\t%0, [%m1]", operands);
> else
>   output_asm_insn ("ldmia%?\t%m1, %M0", operands);
> @@ -18319,7 +18321,7 @@ output_move_double (rtx *operands, bool emit, int
> break;
>  
>   case PRE_INC:
> -   gcc_assert (TARGET_LDRD);
> +   gcc_assert (allow_ldrd);
> if (emit)
>   output_asm_insn ("ldrd%?\t%0, [%m1, #8]!", operands);
> break;
> @@ -18327,7 +18329,7 @@ output_move_double (rtx *operands, bool emit, int
>   case PRE_DEC:
> if (emit)
>   {
> -   if (TARGET_LDRD)
> +   if (allow_ldrd)
>   output_asm_insn ("ldrd%?\t%0, [%m1, #-8]!", operands);
> else
>   output_asm_insn ("ldmdb%?\t%m1!, %M0", operands);
> @@ -18337,7 +18339,7 @@ output_move_double (rtx *operands, bool emit, int
>   case POST_INC:
> if (emit)
>   {
> -   if (TARGET_LDRD)
> +   if (allow_ldrd)
>   output_asm_insn ("ldrd%?\t%0, [%m1], #8", operands);
> else
>   output_asm_insn ("ldmia%?\t%m1!, %M0", operands);
> @@ -18345,7 +18347,7 @@ output_move_double (rtx *operands, bool emit, int
> break;
>  
>   case POST_DEC:
> -   gcc_assert (TARGET_LDRD);
> +   gcc_assert (allow_ldrd);
> if (emit)
>   output_asm_insn ("ldrd%?\t%0, [%m1], #-8", operands);
> break;
> @@ -18483,7 +18485,7 @@ output_move_double (rtx *operands, bool emit, int
>   }
> otherops[0] = gen_rtx_REG(SImode, REGNO(operands[0]) + 1);
> operands[1] = otherops[0];
> -   if (TARGET_LDRD
> +   if (allow_ldrd
> && (REG_P (otherops[2])
> || TARGET_THUMB2
> || (CONST_INT_P (otherops[2])
> @@ -18544,7 +18546,7 @@ output_move_double (rtx *operands, bool emit, int
> if (count)
>   *count = 2;
>  
> -   if (TARGET_LDRD)
> +   if (allow_ldrd)
>   return "ldrd%?\t%0, [%1]";
>  
> return "ldmia%?\t%1, %M0";
> 

Fix mask type choice in vectorizable_call (PR 89535)

[Richard Sandiford]

This is another case in which we were failing to pass the expected
mask vector type to vect_get_vec_def_for_operand.  Really looking
forward to seeing this non-SLP structure go away :-)

Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
and x86_64-linux-gnu.  OK to install?

Richard


2019-03-01  Richard Sandiford  

gcc/
PR tree-optimization/89535
* tree-vect-stmts.c (vectorizable_call): Record the vector types
for each operand.  Calculate the fallback choice for mask operands
and pass it to vect_get_vec_def_for_operand.

gcc/testsuite/
PR tree-optimization/89535
* gfortran.dg/vect/pr89535.f90: New test.

Index: gcc/tree-vect-stmts.c
===
--- gcc/tree-vect-stmts.c   2019-01-25 12:10:12.913100840 +
+++ gcc/tree-vect-stmts.c   2019-03-01 10:43:26.652480564 +
@@ -3123,6 +3123,7 @@ vectorizable_call (stmt_vec_info stmt_in
   enum vect_def_type dt[4]
 = { vect_unknown_def_type, vect_unknown_def_type, vect_unknown_def_type,
vect_unknown_def_type };
+  tree vectypes[ARRAY_SIZE (dt)] = {};
   int ndts = ARRAY_SIZE (dt);
   int ncopies, j;
   auto_vec vargs;
@@ -3182,10 +3183,8 @@ vectorizable_call (stmt_vec_info stmt_in
 
   for (i = 0; i < nargs; i++)
 {
-  tree opvectype;
-
   op = gimple_call_arg (stmt, i);
-  if (!vect_is_simple_use (op, vinfo, &dt[i], &opvectype))
+  if (!vect_is_simple_use (op, vinfo, &dt[i], &vectypes[i]))
{
  if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
@@ -3211,9 +3210,9 @@ vectorizable_call (stmt_vec_info stmt_in
rhs_type = TREE_TYPE (op);
 
   if (!vectype_in)
-   vectype_in = opvectype;
-  else if (opvectype
-  && opvectype != vectype_in)
+   vectype_in = vectypes[i];
+  else if (vectypes[i]
+  && vectypes[i] != vectype_in)
{
  if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
@@ -3446,12 +3445,19 @@ vectorizable_call (stmt_vec_info stmt_in
  continue;
}
 
+ if (mask_opno >= 0 && !vectypes[mask_opno])
+   {
+ gcc_assert (modifier != WIDEN);
+ vectypes[mask_opno]
+   = build_same_sized_truth_vector_type (vectype_in);
+   }
+
  for (i = 0; i < nargs; i++)
{
  op = gimple_call_arg (stmt, i);
  if (j == 0)
vec_oprnd0
- = vect_get_vec_def_for_operand (op, stmt_info);
+ = vect_get_vec_def_for_operand (op, stmt_info, vectypes[i]);
  else
vec_oprnd0
  = vect_get_vec_def_for_stmt_copy (vinfo, orig_vargs[i]);
@@ -3584,7 +3590,8 @@ vectorizable_call (stmt_vec_info stmt_in
  if (j == 0)
{
  vec_oprnd0
-   = vect_get_vec_def_for_operand (op, stmt_info);
+   = vect_get_vec_def_for_operand (op, stmt_info,
+   vectypes[i]);
  vec_oprnd1
= vect_get_vec_def_for_stmt_copy (vinfo, vec_oprnd0);
}
Index: gcc/testsuite/gfortran.dg/vect/pr89535.f90
===
--- /dev/null   2019-02-27 08:05:34.202446820 +
+++ gcc/testsuite/gfortran.dg/vect/pr89535.f90  2019-03-01 10:43:26.652480564 
+
@@ -0,0 +1,18 @@
+! { dg-do compile }
+
+subroutine foo(tmp1, tmp2, tmp3)
+  integer, parameter :: n = 100
+  real :: tmp1(n,2), tmp2(n), tmp3(n)
+  integer :: i, c1, c2, c3
+  logical :: cond
+  common c1, c2, c3
+
+  c2 = c3
+  cond = c1 .eq. 1 .and. c3 .eq. 1
+  do i = 1,100
+ if (cond) tmp2(i) = tmp1(i,1) / tmp1(i,2)
+  end do
+  do i = 1,100
+ if (cond) tmp3(i) = tmp2(i)
+  end do
+end subroutine foo

Re: [PATCH PR89487]Avoid taking address of register variable in loop list

[Richard Biener]

On Fri, Mar 1, 2019 at 7:54 AM bin.cheng  wrote:
>
> Hi,
> This patch fixes PR89487 by following comments in PR.  It simply avoid 
> checking runtime
> alias by versioning in loop distribution if address of register variable may 
> need to be taken.
>
> One thing I am not sure is if we should avoid generating data reference in 
> the first place:
> Creating dr for pc
> analyze_innermost: success.
> base_address: &pc
> offset from base address: 0
> constant offset from base address: 0
> step: 0
> base alignment: 8
> base misalignment: 0
> offset alignment: 128
> step alignment: 128
> base_object: pc
> Here 'pc' is the register variable.

Hm, I think the DR is ok-ish, we are generating DRs dependent on
storage-order as well.

> Bootstrap and test on x86_64, any comment?

Patch is OK.

Thanks,
Richard.

> Thanks,
> bin
> 2019-02-28  Bin Cheng  
>
> PR tree-optimization/89487
> * tree-loop-distribution.c (has_nonaddressable_dataref_p): New.
> (create_rdg_vertices): Compute has_nonaddressable_dataref_p.
> (distribute_loop): Don't do runtime alias check if there is non-
> addressable data reference.
> * tree-ssa-loop-ivopts.c (may_be_nonaddressable_p): Check if VAR_DECL
> is a register variable.
>
> 2018-02-28  Bin Cheng  
>
> PR tree-optimization/89487
> * gcc/testsuite/gcc.dg/tree-ssa/pr89487.c: New test.

Re: Fix mask type choice in vectorizable_call (PR 89535)

[Richard Biener]

On Fri, Mar 1, 2019 at 11:45 AM Richard Sandiford
 wrote:
>
> This is another case in which we were failing to pass the expected
> mask vector type to vect_get_vec_def_for_operand.  Really looking
> forward to seeing this non-SLP structure go away :-)
>
> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
> and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2019-03-01  Richard Sandiford  
>
> gcc/
> PR tree-optimization/89535
> * tree-vect-stmts.c (vectorizable_call): Record the vector types
> for each operand.  Calculate the fallback choice for mask operands
> and pass it to vect_get_vec_def_for_operand.
>
> gcc/testsuite/
> PR tree-optimization/89535
> * gfortran.dg/vect/pr89535.f90: New test.
>
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c   2019-01-25 12:10:12.913100840 +
> +++ gcc/tree-vect-stmts.c   2019-03-01 10:43:26.652480564 +
> @@ -3123,6 +3123,7 @@ vectorizable_call (stmt_vec_info stmt_in
>enum vect_def_type dt[4]
>  = { vect_unknown_def_type, vect_unknown_def_type, vect_unknown_def_type,
> vect_unknown_def_type };
> +  tree vectypes[ARRAY_SIZE (dt)] = {};
>int ndts = ARRAY_SIZE (dt);
>int ncopies, j;
>auto_vec vargs;
> @@ -3182,10 +3183,8 @@ vectorizable_call (stmt_vec_info stmt_in
>
>for (i = 0; i < nargs; i++)
>  {
> -  tree opvectype;
> -
>op = gimple_call_arg (stmt, i);
> -  if (!vect_is_simple_use (op, vinfo, &dt[i], &opvectype))
> +  if (!vect_is_simple_use (op, vinfo, &dt[i], &vectypes[i]))
> {
>   if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> @@ -3211,9 +3210,9 @@ vectorizable_call (stmt_vec_info stmt_in
> rhs_type = TREE_TYPE (op);
>
>if (!vectype_in)
> -   vectype_in = opvectype;
> -  else if (opvectype
> -  && opvectype != vectype_in)
> +   vectype_in = vectypes[i];
> +  else if (vectypes[i]
> +  && vectypes[i] != vectype_in)
> {
>   if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> @@ -3446,12 +3445,19 @@ vectorizable_call (stmt_vec_info stmt_in
>   continue;
> }
>
> + if (mask_opno >= 0 && !vectypes[mask_opno])
> +   {
> + gcc_assert (modifier != WIDEN);
> + vectypes[mask_opno]
> +   = build_same_sized_truth_vector_type (vectype_in);
> +   }
> +
>   for (i = 0; i < nargs; i++)
> {
>   op = gimple_call_arg (stmt, i);
>   if (j == 0)
> vec_oprnd0
> - = vect_get_vec_def_for_operand (op, stmt_info);
> + = vect_get_vec_def_for_operand (op, stmt_info, vectypes[i]);
>   else
> vec_oprnd0
>   = vect_get_vec_def_for_stmt_copy (vinfo, orig_vargs[i]);
> @@ -3584,7 +3590,8 @@ vectorizable_call (stmt_vec_info stmt_in
>   if (j == 0)
> {
>   vec_oprnd0
> -   = vect_get_vec_def_for_operand (op, stmt_info);
> +   = vect_get_vec_def_for_operand (op, stmt_info,
> +   vectypes[i]);
>   vec_oprnd1
> = vect_get_vec_def_for_stmt_copy (vinfo, vec_oprnd0);
> }
> Index: gcc/testsuite/gfortran.dg/vect/pr89535.f90
> ===
> --- /dev/null   2019-02-27 08:05:34.202446820 +
> +++ gcc/testsuite/gfortran.dg/vect/pr89535.f90  2019-03-01 10:43:26.652480564 
> +
> @@ -0,0 +1,18 @@
> +! { dg-do compile }
> +
> +subroutine foo(tmp1, tmp2, tmp3)
> +  integer, parameter :: n = 100
> +  real :: tmp1(n,2), tmp2(n), tmp3(n)
> +  integer :: i, c1, c2, c3
> +  logical :: cond
> +  common c1, c2, c3
> +
> +  c2 = c3
> +  cond = c1 .eq. 1 .and. c3 .eq. 1
> +  do i = 1,100
> + if (cond) tmp2(i) = tmp1(i,1) / tmp1(i,2)
> +  end do
> +  do i = 1,100
> + if (cond) tmp3(i) = tmp2(i)
> +  end do
> +end subroutine foo

[PATCH][GCC][AArch64] Make every option in options.def one line

[Tamar Christina]

Hi All,

Due to config.gcc all the options need to be on one line because of the grep
lines which would select only the first line of the option.

This causes it not to select the right bits on options that are spread over
multiple lines when the --with-arch configure option is used.  The issue happens
silently and you just get a compiler with an incorrect set of default flags.

This solution just collapses everything back to one line as they were in GCC7.
Unfortunately this does make some lines quite long.

I do have an alternate patch which used the pre-processors to first flatten the
file in config.gcc.  I will send that one out for GCC 10.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/ChangeLog:

2019-03-01  Tamar Christina  

PR target/89517
* config/aarch64/aarch64-option-extensions.def (fp, simd, crypto, fp16,
rdma, dotprod, sha2, sha3, sm4, fp16fml, sve): Collapse line.

-- 
diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 1b2f4abbd5b850135af2cb7010921b45c03516a9..27d5f08c890d300d9c2a66ee6d3d04de3efd574a 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -46,29 +46,27 @@
  should contain a space (" ") separated list of the strings in 'Features'
  that are required.  Their order is not important.  */
 
+/* NOTE: This file is being parsed by config.gcc and so the
+   AARCH64_OPT_EXTENSION must adhere to a strict format:
+   1) No space between the AARCH64_OPT_EXTENSION and the opening (.
+   2) No space between the opening ( and the extension name.
+   3) No space after the extension name before the ,.
+   4) Spaces are only allowed after a , and around |.
+   5) Everything must be on one line.  */
+
 /* Enabling "fp" just enables "fp".
Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
"sha3", sm3/sm4 and "sve".  */
-AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPTO |\
-		  AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
-		  AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, \
-		  false, "fp")
+AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "fp")
 
 /* Enabling "simd" also enables "fp".
Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3",
"sm3/sm4" and "sve".  */
-AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, AARCH64_FL_CRYPTO |\
-		  AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
-		  AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE,
-		  false, "asimd")
+AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "asimd")
 
 /* Enabling "crypto" also enables "fp", "simd", "aes" and "sha2".
Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and "sm3/sm4".  */
-AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO,
-		  AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_AES |\
-		  AARCH64_FL_SHA2,\
-		  AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4,\
-		  true, "aes pmull sha1 sha2")
+AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_AES | AARCH64_FL_SHA2, AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4, true, "aes pmull sha1 sha2")
 
 /* Enabling or disabling "crc" only changes "crc".  */
 AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, false, "crc32")
@@ -78,21 +76,18 @@ AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, false, "atomics")
 
 /* Enabling "fp16" also enables "fp".
Disabling "fp16" disables "fp16", "fp16fml" and "sve".  */
-AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP,
-		  AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp")
+AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp")
 
 /* Enabling or disabling "rcpc" only changes "rcpc".  */
 AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, false, "lrcpc")
 
 /* Enabling "rdma" also enables "fp", "simd".
Disabling "rdma" just disables "rdma".  */
-AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, \
-		  AARCH64_FL_FP | AARCH64_FL_SIMD, 0, false, "asimdrdm")
+AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, AARCH64_FL_FP | AARCH64_FL_SIMD, 0, false, "asimdrdm")
 
 /* Enabling "dotprod" also enables "simd".
Disabling "dotprod" only disables "dotprod".  */
-AARCH64_OPT_EXTENSION("dotprod", AARCH64_FL_DOTPROD, AARCH64_FL_SIMD, 0, \
-		  false, "asimddp")
+AARCH64_OPT_EXTENSION("dotprod", AARCH64_FL_DOTPROD, AARCH64_FL_SIMD, 0, false, "asimddp")
 
 /* Enabling "aes" also enables "simd

[PATCH][GCC][AArch64] Make every option in options.def one line (GCC-8).

[Tamar Christina]

Hi All,

Due to config.gcc all the options need to be on one line because of the grep
lines which would select only the first line of the option.

This causes it not to select the right bits on options that are spread over
multiple lines when the --with-arch configure option is used.  The issue happens
silently and you just get a compiler with an incorrect set of default flags.

This solution just collapses everything back to one line as they were in GCC7.
Unfortunately this does make some lines quite long.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for GCC-8?

Thanks,
Tamar

gcc/ChangeLog:

2019-03-01  Tamar Christina  

PR target/89517
* config/aarch64/aarch64-option-extensions.def (fp, simd, crypto,
fp16): Collapse line.

-- 
diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 5fe5e3f7dddf622a48a5b9458ef30449a886f395..a1b71cedb14cebe48f6e9e7a5ce3dc202161f2dc 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -38,26 +38,27 @@
should contain a space (" ") separated list of the strings in 'Features'
that are required.  Their order is not important.  */
 
+/* NOTE: This file is being parsed by config.gcc and so the
+   AARCH64_OPT_EXTENSION must adhere to a strict format:
+   1) No space between the AARCH64_OPT_EXTENSION and the opening (.
+   2) No space between the opening ( and the extension name.
+   3) No space after the extension name before the ,.
+   4) Spaces are only allowed after a , and around |.
+   5) Everything must be on one line.  */
+
 /* Enabling "fp" just enables "fp".
Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
"sha3", sm3/sm4 and "sve".  */
-AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPTO |\
-		  AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
-		  AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "fp")
+AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "fp")
 
 /* Enabling "simd" also enables "fp".
Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3",
"sm3/sm4" and "sve".  */
-AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, AARCH64_FL_CRYPTO |\
-		  AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
-		  AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE,
-		  "asimd")
+AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "asimd")
 
 /* Enabling "crypto" also enables "fp" and "simd".
Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and "sm3/sm4".  */
-AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | AARCH64_FL_SIMD,\
-		  AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4,\
-		  "aes pmull sha1 sha2")
+AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | AARCH64_FL_SIMD, AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4, "aes pmull sha1 sha2")
 
 /* Enabling or disabling "crc" only changes "crc".  */
 AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, "crc32")
@@ -67,8 +68,7 @@ AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, "atomics")
 
 /* Enabling "fp16" also enables "fp".
Disabling "fp16" disables "fp16", "fp16fml" and "sve".  */
-AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP,
-		  AARCH64_FL_F16FML | AARCH64_FL_SVE, "fphp asimdhp")
+AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, AARCH64_FL_F16FML | AARCH64_FL_SVE, "fphp asimdhp")
 
 /* Enabling or disabling "rcpc" only changes "rcpc".  */
 AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, "lrcpc")

Re: [PATCH][GCC][AArch64] Make every option in options.def one line

[Richard Earnshaw (lists)]

On 01/03/2019 12:57, Tamar Christina wrote:
> Hi All,
> 
> Due to config.gcc all the options need to be on one line because of the grep
> lines which would select only the first line of the option.
> 
> This causes it not to select the right bits on options that are spread over
> multiple lines when the --with-arch configure option is used.  The issue 
> happens
> silently and you just get a compiler with an incorrect set of default flags.
> 
> This solution just collapses everything back to one line as they were in GCC7.
> Unfortunately this does make some lines quite long.
> 
> I do have an alternate patch which used the pre-processors to first flatten 
> the
> file in config.gcc.  I will send that one out for GCC 10.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 2019-03-01  Tamar Christina  
> 
>   PR target/89517
>   * config/aarch64/aarch64-option-extensions.def (fp, simd, crypto, fp16,
>   rdma, dotprod, sha2, sha3, sm4, fp16fml, sve): Collapse line.
> 

OK.

R.

> 
> rb10754.patch
> 
> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def 
> b/gcc/config/aarch64/aarch64-option-extensions.def
> index 
> 1b2f4abbd5b850135af2cb7010921b45c03516a9..27d5f08c890d300d9c2a66ee6d3d04de3efd574a
>  100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -46,29 +46,27 @@
>   should contain a space (" ") separated list of the strings in 'Features'
>   that are required.  Their order is not important.  */
>  
> +/* NOTE: This file is being parsed by config.gcc and so the
> +   AARCH64_OPT_EXTENSION must adhere to a strict format:
> +   1) No space between the AARCH64_OPT_EXTENSION and the opening (.
> +   2) No space between the opening ( and the extension name.
> +   3) No space after the extension name before the ,.
> +   4) Spaces are only allowed after a , and around |.
> +   5) Everything must be on one line.  */
> +
>  /* Enabling "fp" just enables "fp".
> Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
> "sha3", sm3/sm4 and "sve".  */
> -AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | 
> AARCH64_FL_CRYPTO |\
> -   AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
> -   AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, \
> -   false, "fp")
> +AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | 
> AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | 
> AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "fp")
>  
>  /* Enabling "simd" also enables "fp".
> Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3",
> "sm3/sm4" and "sve".  */
> -AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, 
> AARCH64_FL_CRYPTO |\
> -   AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
> -   AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE,
> -   false, "asimd")
> +AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, 
> AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | 
> AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "asimd")
>  
>  /* Enabling "crypto" also enables "fp", "simd", "aes" and "sha2".
> Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and 
> "sm3/sm4".  */
> -AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO,
> -   AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_AES |\
> -   AARCH64_FL_SHA2,\
> -   AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | 
> AARCH64_FL_SM4,\
> -   true, "aes pmull sha1 sha2")
> +AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | 
> AARCH64_FL_SIMD | AARCH64_FL_AES | AARCH64_FL_SHA2, AARCH64_FL_AES | 
> AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4, true, "aes pmull sha1 
> sha2")
>  
>  /* Enabling or disabling "crc" only changes "crc".  */
>  AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, false, "crc32")
> @@ -78,21 +76,18 @@ AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, false, 
> "atomics")
>  
>  /* Enabling "fp16" also enables "fp".
> Disabling "fp16" disables "fp16", "fp16fml" and "sve".  */
> -AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP,
> -   AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp")
> +AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, 
> AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp")
>  
>  /* Enabling or disabling "rcpc" only changes "rcpc".  */
>  AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, false, "lrcpc")
>  
>  /* Enabling "rdma" also enables "fp", "simd".
> Disabling "rdma" just disables "rdma".  */
> -AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, \
> -   AARCH64_FL_FP | AARCH64_FL_SIMD, 0, false, "asimdrdm")
> +A

Re: [PATCH][GCC][AArch64] Make every option in options.def one line (GCC-8).

[Richard Earnshaw (lists)]

On 01/03/2019 12:58, Tamar Christina wrote:
> Hi All,
> 
> Due to config.gcc all the options need to be on one line because of the grep
> lines which would select only the first line of the option.
> 
> This causes it not to select the right bits on options that are spread over
> multiple lines when the --with-arch configure option is used.  The issue 
> happens
> silently and you just get a compiler with an incorrect set of default flags.
> 
> This solution just collapses everything back to one line as they were in GCC7.
> Unfortunately this does make some lines quite long.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for GCC-8?
> 
> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 2019-03-01  Tamar Christina  
> 
>   PR target/89517
>   * config/aarch64/aarch64-option-extensions.def (fp, simd, crypto,
>   fp16): Collapse line.
> 

OK.

R.

> 
> rb10764.patch
> 
> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def 
> b/gcc/config/aarch64/aarch64-option-extensions.def
> index 
> 5fe5e3f7dddf622a48a5b9458ef30449a886f395..a1b71cedb14cebe48f6e9e7a5ce3dc202161f2dc
>  100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -38,26 +38,27 @@
> should contain a space (" ") separated list of the strings in 'Features'
> that are required.  Their order is not important.  */
>  
> +/* NOTE: This file is being parsed by config.gcc and so the
> +   AARCH64_OPT_EXTENSION must adhere to a strict format:
> +   1) No space between the AARCH64_OPT_EXTENSION and the opening (.
> +   2) No space between the opening ( and the extension name.
> +   3) No space after the extension name before the ,.
> +   4) Spaces are only allowed after a , and around |.
> +   5) Everything must be on one line.  */
> +
>  /* Enabling "fp" just enables "fp".
> Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
> "sha3", sm3/sm4 and "sve".  */
> -AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | 
> AARCH64_FL_CRYPTO |\
> -   AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
> -   AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "fp")
> +AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | 
> AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | 
> AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "fp")
>  
>  /* Enabling "simd" also enables "fp".
> Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3",
> "sm3/sm4" and "sve".  */
> -AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, 
> AARCH64_FL_CRYPTO |\
> -   AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
> -   AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE,
> -   "asimd")
> +AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, 
> AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | 
> AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "asimd")
>  
>  /* Enabling "crypto" also enables "fp" and "simd".
> Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and 
> "sm3/sm4".  */
> -AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | 
> AARCH64_FL_SIMD,\
> -   AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | 
> AARCH64_FL_SM4,\
> -   "aes pmull sha1 sha2")
> +AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | 
> AARCH64_FL_SIMD, AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | 
> AARCH64_FL_SM4, "aes pmull sha1 sha2")
>  
>  /* Enabling or disabling "crc" only changes "crc".  */
>  AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, "crc32")
> @@ -67,8 +68,7 @@ AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, 
> "atomics")
>  
>  /* Enabling "fp16" also enables "fp".
> Disabling "fp16" disables "fp16", "fp16fml" and "sve".  */
> -AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP,
> -   AARCH64_FL_F16FML | AARCH64_FL_SVE, "fphp asimdhp")
> +AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, 
> AARCH64_FL_F16FML | AARCH64_FL_SVE, "fphp asimdhp")
>  
>  /* Enabling or disabling "rcpc" only changes "rcpc".  */
>  AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, "lrcpc")
> 

[PATCH] C++2a Utility functions to implement uses-allocator construction (P0591R4)

[Jonathan Wakely]

* include/std/memory (uses_allocator_construction_args): New set of
overloaded functions.
(make_obj_using_allocator, uninitialized_construct_using_allocator):
New functions.
* include/std/memory_resource (polymorphic_allocator::construct)
[__cplusplus > 201703l]: Replace all overloads with a single function
using uses_allocator_construction_args.
* testsuite/20_util/polymorphic_allocator/construct_c++2a.cc: New
test.
* testsuite/20_util/uses_allocator/make_obj.cc: New test.

Tested powerpc64le-linux, committed to trunk.


commit e2d0fdcde8a7dcebfab8246372d3756c5c489717
Author: Jonathan Wakely 
Date:   Fri Mar 1 13:27:58 2019 +

C++2a Utility functions to implement uses-allocator construction (P0591R4)

* include/std/memory (uses_allocator_construction_args): New set of
overloaded functions.
(make_obj_using_allocator, uninitialized_construct_using_allocator):
New functions.
* include/std/memory_resource (polymorphic_allocator::construct)
[__cplusplus > 201703l]: Replace all overloads with a single 
function
using uses_allocator_construction_args.
* testsuite/20_util/polymorphic_allocator/construct_c++2a.cc: New
test.
* testsuite/20_util/uses_allocator/make_obj.cc: New test.

diff --git a/libstdc++-v3/include/std/memory b/libstdc++-v3/include/std/memory
index ff9add9cef2..00a85eef25e 100644
--- a/libstdc++-v3/include/std/memory
+++ b/libstdc++-v3/include/std/memory
@@ -91,6 +91,8 @@
 #include 
 #if __cplusplus > 201703L
 # include // for ispow2
+# include // for placement operator new
+# include   // for tuple, make_tuple, make_from_tuple
 #endif
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -166,6 +168,197 @@ get_pointer_safety() noexcept { return 
pointer_safety::relaxed; }
 }
 #endif // C++2a
 
+#if __cplusplus > 201703L
+  template
+struct __is_pair : false_type { };
+  template
+struct __is_pair> : true_type { };
+  template
+struct __is_pair> : true_type { };
+
+  template>>,
+  typename _Alloc, typename... _Args>
+constexpr auto
+__uses_alloc_args(const _Alloc& __a, _Args&&... __args) noexcept
+{
+  if constexpr (uses_allocator_v, _Alloc>)
+   {
+ if constexpr (is_constructible_v<_Tp, allocator_arg_t,
+  const _Alloc&, _Args...>)
+   {
+ return tuple(
+ allocator_arg, __a, std::forward<_Args>(__args)...);
+   }
+ else
+   {
+ static_assert(is_constructible_v<_Tp, _Args..., const _Alloc&>);
+
+ return tuple<_Args&&..., const _Alloc&>(
+ std::forward<_Args>(__args)..., __a);
+   }
+   }
+  else
+   {
+ static_assert(is_constructible_v<_Tp, _Args...>);
+
+ return tuple<_Args&&...>(std::forward<_Args>(__args)...);
+   }
+}
+
+#if __cpp_concepts
+  template
+concept bool _Std_pair = __is_pair<_Tp>::value;
+#endif
+
+// This is a temporary workaround until -fconcepts is implied by -std=gnu++2a
+#if __cpp_concepts
+# define _GLIBCXX_STD_PAIR_CONSTRAINT(T) _Std_pair T
+# define _GLIBCXX_STD_PAIR_CONSTRAINT_(T) _Std_pair T
+#else
+# define _GLIBCXX_STD_PAIR_CONSTRAINT(T) \
+  typename T, typename __ = _Require<__is_pair>
+# define _GLIBCXX_STD_PAIR_CONSTRAINT_(T) typename T, typename
+#endif
+
+  template>>,
+#endif
+  typename _Alloc, typename... _Args>
+constexpr auto
+uses_allocator_construction_args(const _Alloc& __a,
+_Args&&... __args) noexcept
+#if __cpp_concepts
+requires ! _Std_pair<_Tp>
+#endif
+{
+  return std::__uses_alloc_args<_Tp>(__a, std::forward<_Args>(__args)...);
+}
+
+  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
+  typename _Tuple1, typename _Tuple2>
+constexpr auto
+uses_allocator_construction_args(const _Alloc& __a, piecewise_construct_t,
+_Tuple1&& __x, _Tuple2&& __y) noexcept;
+
+  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc>
+constexpr auto
+uses_allocator_construction_args(const _Alloc&) noexcept;
+
+  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
+  typename _Up, typename _Vp>
+constexpr auto
+uses_allocator_construction_args(const _Alloc&, _Up&&, _Vp&&) noexcept;
+
+  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
+  typename _Up, typename _Vp>
+constexpr auto
+uses_allocator_construction_args(const _Alloc&,
+const pair<_Up, _Vp>&) noexcept;
+
+  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
+  typename _Up, typename _Vp>
+constexpr auto
+uses_allocator_construction_args(const _Alloc&, p

[PATCH] Fix test memory_resource to work without sized deallocation

[Jonathan Wakely]

The checking memory_resource in the testsuite assumes sized deallocation
is supported, which might not be true for other compilers.

* testsuite/util/testsuite_allocator.h (__gnu_test::memory_resource)
[!__cpp_sized_deallocation]: Do not pass size to operator delete.

Tested powerpc64le-linux, committed to trunk.


commit 9cc2b31ce0e23c7bae5f1e065628baec934478c6
Author: Jonathan Wakely 
Date:   Fri Mar 1 13:33:25 2019 +

Fix test memory_resource to work without sized deallocation

The checking memory_resource in the testsuite assumes sized deallocation
is supported, which might not be true for other compilers.

* testsuite/util/testsuite_allocator.h (__gnu_test::memory_resource)
[!__cpp_sized_deallocation]: Do not pass size to operator delete.

diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h 
b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index 044b9d5a90f..627749299d2 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -780,7 +780,11 @@ namespace __gnu_test
  throw bad_size();
if (alignment != a->alignment)
  throw bad_alignment();
+#if __cpp_sized_deallocation
::operator delete(p, bytes, std::align_val_t(alignment));
+#else
+   ::operator delete(p, std::align_val_t(alignment));
+#endif
*aptr = a->next;
a->next = lists->freed;
lists->freed = a;

Re: [PATCH] C++2a Utility functions to implement uses-allocator construction (P0591R4)

[Jonathan Wakely]

On 01/03/19 13:50 +, Jonathan Wakely wrote:

* include/std/memory (uses_allocator_construction_args): New set of
overloaded functions.
(make_obj_using_allocator, uninitialized_construct_using_allocator):
New functions.
* include/std/memory_resource (polymorphic_allocator::construct)
[__cplusplus > 201703l]: Replace all overloads with a single function
using uses_allocator_construction_args.
* testsuite/20_util/polymorphic_allocator/construct_c++2a.cc: New
test.
* testsuite/20_util/uses_allocator/make_obj.cc: New test.


If we don't care about providing the exact signatures from the C++2a
draft, we could do this and use it in C++17 as well ...



diff --git a/libstdc++-v3/include/std/memory b/libstdc++-v3/include/std/memory
index 00a85eef25e..045974a1b46 100644
--- a/libstdc++-v3/include/std/memory
+++ b/libstdc++-v3/include/std/memory
@@ -168,7 +168,7 @@ get_pointer_safety() noexcept { return pointer_safety::relaxed; }
 }
 #endif // C++2a
 
-#if __cplusplus > 201703L
+#if __cplusplus >= 201703L
   template
 struct __is_pair : false_type { };
   template
@@ -176,167 +176,94 @@ get_pointer_safety() noexcept { return pointer_safety::relaxed; }
   template
 struct __is_pair> : true_type { };
 
-  template>>,
-	   typename _Alloc, typename... _Args>
+  // Equivalent of uses_allocator_construction_args for internal use in C++17
+  template
 constexpr auto
 __uses_alloc_args(const _Alloc& __a, _Args&&... __args) noexcept
 {
-  if constexpr (uses_allocator_v, _Alloc>)
+  if constexpr (!__is_pair<_Tp>::value)
 	{
-	  if constexpr (is_constructible_v<_Tp, allocator_arg_t,
-	   const _Alloc&, _Args...>)
+	  if constexpr (uses_allocator_v, _Alloc>)
 	{
-	  return tuple(
-		  allocator_arg, __a, std::forward<_Args>(__args)...);
+	  if constexpr (is_constructible_v<_Tp, allocator_arg_t,
+		  const _Alloc&, _Args...>)
+		{
+		  return tuple(
+		  allocator_arg, __a, std::forward<_Args>(__args)...);
+		}
+	  else
+		{
+		  static_assert(
+		  is_constructible_v<_Tp, _Args..., const _Alloc&>);
+
+		  return tuple<_Args&&..., const _Alloc&>(
+		  std::forward<_Args>(__args)..., __a);
+		}
 	}
 	  else
 	{
-	  static_assert(is_constructible_v<_Tp, _Args..., const _Alloc&>);
+	  static_assert(is_constructible_v<_Tp, _Args...>);
 
-	  return tuple<_Args&&..., const _Alloc&>(
-		  std::forward<_Args>(__args)..., __a);
+	  return tuple<_Args&&...>(std::forward<_Args>(__args)...);
 	}
 	}
   else
 	{
-	  static_assert(is_constructible_v<_Tp, _Args...>);
-
-	  return tuple<_Args&&...>(std::forward<_Args>(__args)...);
-	}
-}
-
-#if __cpp_concepts
-  template
-concept bool _Std_pair = __is_pair<_Tp>::value;
-#endif
-
-// This is a temporary workaround until -fconcepts is implied by -std=gnu++2a
-#if __cpp_concepts
-# define _GLIBCXX_STD_PAIR_CONSTRAINT(T) _Std_pair T
-# define _GLIBCXX_STD_PAIR_CONSTRAINT_(T) _Std_pair T
-#else
-# define _GLIBCXX_STD_PAIR_CONSTRAINT(T) \
-  typename T, typename __ = _Require<__is_pair>
-# define _GLIBCXX_STD_PAIR_CONSTRAINT_(T) typename T, typename
-#endif
-
-  template>>,
-#endif
-	   typename _Alloc, typename... _Args>
-constexpr auto
-uses_allocator_construction_args(const _Alloc& __a,
- _Args&&... __args) noexcept
-#if __cpp_concepts
-requires ! _Std_pair<_Tp>
-#endif
-{
-  return std::__uses_alloc_args<_Tp>(__a, std::forward<_Args>(__args)...);
-}
-
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
-	   typename _Tuple1, typename _Tuple2>
-constexpr auto
-uses_allocator_construction_args(const _Alloc& __a, piecewise_construct_t,
- _Tuple1&& __x, _Tuple2&& __y) noexcept;
-
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc>
-constexpr auto
-uses_allocator_construction_args(const _Alloc&) noexcept;
-
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
-	   typename _Up, typename _Vp>
-constexpr auto
-uses_allocator_construction_args(const _Alloc&, _Up&&, _Vp&&) noexcept;
-
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
-	   typename _Up, typename _Vp>
-constexpr auto
-uses_allocator_construction_args(const _Alloc&,
- const pair<_Up, _Vp>&) noexcept;
-
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT(_Tp), typename _Alloc,
-	   typename _Up, typename _Vp>
-constexpr auto
-uses_allocator_construction_args(const _Alloc&, pair<_Up, _Vp>&&) noexcept;
+	  static_assert(sizeof...(__args) <= 3);
 
-  template<_GLIBCXX_STD_PAIR_CONSTRAINT_(_Tp), typename _Alloc,
-	   typename _Tuple1, typename _Tuple2>
-constexpr auto
-uses_allocator_construction_args(const _Alloc& __a, piecewise_construct_t,
- _Tuple1&& __x, _Tuple2&& __y) noexcept
-{
-  using _Tp1 = typename _Tp::first_type;
-  using _Tp2 = typename _Tp::second_type;
-
-  return std::make_tuple(piece

Re: C++ PATCH for c++/89537 - missing location for error with non-static member fn

[Jason Merrill]

On 2/28/19 2:45 PM, Marek Polacek wrote:

On Thu, Feb 28, 2019 at 02:33:37PM -0500, Jason Merrill wrote:

On 2/28/19 2:00 PM, Marek Polacek wrote:

Here we issued the "invalid use of non-static member function" error with
UNKNOWN_LOCATION, which merely shows "cc1plus" and no file/line.  We can
greatly improve this situation with the below.

The patch is IMHO trivial (though it's user-provided) to go in even at
this point.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-02-28  Marek Polacek  

PR c++/89537 - missing location for error with non-static member fn.
* call.c (resolve_args): Use EXPR_LOCATION.
* typeck.c (build_class_member_access_expr): Use input_location.

* g++.dg/diagnostic/member-fn-1.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index fb67d905acd..d9073d7c23d 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -4246,7 +4246,7 @@ resolve_args (vec *args, tsubst_flags_t 
complain)
error ("invalid use of void expression");
  return NULL;
}
-  else if (invalid_nonstatic_memfn_p (arg->exp.locus, arg, complain))
+  else if (invalid_nonstatic_memfn_p (EXPR_LOCATION (arg), arg, complain))


Maybe cp_expr_loc_or_loc (arg, input_location)?


That gives a suboptimal location:

   values(p1->values) {}
^
whereas with my fix:

   values(p1->values) {}
  ^~


return NULL;
   }
 return args;
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 1db9333b5ff..1bf9ad88141 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -2562,7 +2562,8 @@ build_class_member_access_expr (cp_expr object, tree 
member,
type = unknown_type_node;
 /* Note that we do not convert OBJECT to the BASELINK_BINFO
 base.  That will happen when the function is called.  */
-  result = build3 (COMPONENT_REF, type, object, member, NULL_TREE);
+  result = build3_loc (input_location, COMPONENT_REF, type, object, member,
+  NULL_TREE);


Then I think this change shouldn't be needed.


The build3 call above this block also uses input_location.


The patch is OK, then.

Jason

Re: [C++ PATCH] Reject constexpr functions with function-try-block (PR c++/89513, take 2)

[Jason Merrill]

On 2/28/19 6:03 PM, Jakub Jelinek wrote:

On Wed, Feb 27, 2019 at 06:48:13PM -0500, Jason Merrill wrote:

Or would you prefer to have P1002R1 implemented and thus make this perhaps a
pedwarn before cxx2a, similarly for the error on try block in the body and
tweak build_constexpr_constructor_member_initializers?  I think constexpr
evaluation handles TRY_BLOCK already.


That sounds better, yes.


Ok, here is an updated patch that does that.

Bootstrapped/regtested on x86_64-linux and i686-linux (98,11,14,17,2a in
both cases), ok for trunk?

2019-02-28  Jakub Jelinek  

Implement P1002R1, Try-catch blocks in constexpr functions
PR c++/89513
* parser.c (cp_parser_ctor_initializer_opt_and_function_body):
Diagnose constexpr ctor or function with function-try-block with
pedwarn for c++17 and earlier.  Formatting fix.
(cp_parser_try_block): Use pedwarn instead of error and only for
c++17 and earlier when try block appears in constexpr function.
* constexpr.c (build_constexpr_constructor_member_initializers):
Handle TRY_BLOCK here instead of erroring on it.


OK.

Jason

Re: C++ PATCH for c++/89532 - ICE with incomplete type in decltype

[Jason Merrill]

On 2/28/19 3:04 PM, Marek Polacek wrote:

If get_target_expr_sfinae gets an expression whose type is incomplete, it's
upset.  digest_init returns error_mark_node if it gets an expression with
incomplete type, so we need to respect that, and not call
get_target_expr_sfinae on ORIG_CL in that case.



Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-02-28  Marek Polacek  

PR c++/89532 - ICE with incomplete type in decltype.
* semantics.c (finish_compound_literal): Return error_mark_node
if digest_init_flags returns error_mark_node.


OK.

Jason

Re: [PATCH 3/3][GCC][AARCH64] Add support for pointer authentication B key

[Sam Tebbs]

On 31/01/2019 14:54, Sam Tebbs wrote:
> 
>> ping 3. The preceding two patches were committed a while ago but require
>> the minor libgcc changes in this patch, which are the only parts left to
>> be reviewed.
> ping 4

Attached is a rebased patch made to work on top of Sudi Das' BTI patch 
(by renaming UNSPEC_PACISP to UNSPEC_PACIASP and UNSPEC_PACIBSP in 
aarch64-bti-insert.c). The updated changelog is below.

Are the libgcc changes OK for trunk?

gcc/
2019-03-01  Sam Tebbs

* config/aarch64/aarch64-builtins.c (aarch64_builtins): Add
AARCH64_PAUTH_BUILTIN_AUTIB1716 and AARCH64_PAUTH_BUILTIN_PACIB1716.
* config/aarch64/aarch64-builtins.c (aarch64_init_pauth_hint_builtins):
Add autib1716 and pacib1716 initialisation.
* config/aarch64/aarch64-builtins.c (aarch64_expand_builtin): Add checks
for autib1716 and pacib1716.
* config/aarch64/aarch64-protos.h (aarch64_key_type,
aarch64_post_cfi_startproc): Define.
* config/aarch64/aarch64-protos.h (aarch64_ra_sign_key): Define extern.
* config/aarch64/aarch64.c (aarch64_handle_standard_branch_protection,
aarch64_handle_pac_ret_protection): Set default sign key to A.
* config/aarch64/aarch64.c (aarch64_expand_epilogue,
aarch64_expand_prologue): Add check for b-key.
* config/aarch64/aarch64.c (aarch64_ra_sign_key,
aarch64_post_cfi_startproc, aarch64_handle_pac_ret_b_key): Define.
* config/aarch64/aarch64.h (TARGET_ASM_POST_CFI_STARTPROC): Define.
* config/aarch64/aarch64.c (aarch64_pac_ret_subtypes): Add "b-key".
* config/aarch64/aarch64.md (unspec): Add UNSPEC_AUTIA1716,
UNSPEC_AUTIB1716, UNSPEC_AUTIASP, UNSPEC_AUTIBSP, UNSPEC_PACIA1716,
UNSPEC_PACIB1716, UNSPEC_PACIASP, UNSPEC_PACIBSP.
* config/aarch64/aarch64.md (do_return): Add check for b-key.
* config/aarch64/aarch64.md (sp): Replace
pauth_hint_num_a with pauth_hint_num.
* config/aarch64/aarch64.md (1716): Replace
pauth_hint_num_a with pauth_hint_num.
* config/aarch64/aarch64.opt (msign-return-address=): Deprecate.
* config/aarch64/iterators.md (PAUTH_LR_SP): Add UNSPEC_AUTIASP,
UNSPEC_AUTIBSP, UNSPEC_PACIASP, UNSPEC_PACIBSP.
* config/aarch64/iterators.md (PAUTH_17_16): Add UNSPEC_AUTIA1716,
UNSPEC_AUTIB1716, UNSPEC_PACIA1716, UNSPEC_PACIB1716.
* config/aarch64/iterators.md (pauth_mnem_prefix): Add UNSPEC_AUTIA1716,
UNSPEC_AUTIB1716, UNSPEC_PACIA1716, UNSPEC_PACIB1716, UNSPEC_AUTIASP,
UNSPEC_AUTIBSP, UNSPEC_PACIASP, UNSPEC_PACIBSP.
* config/aarch64/iterators.md (pauth_hint_num_a): Replace
UNSPEC_PACI1716 and UNSPEC_AUTI1716 with UNSPEC_PACIA1716 and
UNSPEC_AUTIA1716 respectively.
* config/aarch64/iterators.md (pauth_hint_num_a): Rename to 
pauth_hint_num
and add UNSPEC_PACIBSP, UNSPEC_AUTIBSP, UNSPEC_PACIB1716, 
UNSPEC_AUTIB1716.
* doc/invoke.texi (-mbranch-protection): Add b-key type.
* config/aarch64/aarch64-bti-insert.c (aarch64_pac_insn_p): Rename
UNSPEC_PACISP to UNSPEC_PACIASP and UNSPEC_PACIBSP.

gcc/testsuite
2019-03-01  Sam Tebbs

* gcc.target/aarch64/return_address_sign_b_1.c: New file.
* gcc.target/aarch64/return_address_sign_b_2.c: New file.
* gcc.target/aarch64/return_address_sign_b_3.c: New file.
* gcc.target/aarch64/return_address_sign_b_exception.c: New file.
* gcc.target/aarch64/return_address_sign_ab_exception.c: New file.
* gcc.target/aarch64/return_address_sign_builtin.c: New file

libgcc/
2019-03-01  Sam Tebbs

* config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key): New
function.
* config/aarch64/aarch64-unwind.h (aarch64_post_extract_frame_addr,
aarch64_post_frob_eh_handler_addr): Add check for b-key.
* config/aarch64/aarch64-unwind-h (aarch64_post_extract_frame_addr,
aarch64_post_frob_eh_handler_addr, aarch64_post_frob_update_context):
Rename RA_A_SIGNED_BIT to RA_SIGNED_BIT.
* unwind-dw2-fde.c (get_cie_encoding): Add check for 'B' in augmentation
string.
* unwind-dw2.c (extract_cie_info): Add check for 'B' in augmentation
string.
(RA_A_SIGNED_BIT): Rename to RA_SIGNED_BIT.

diff --git a/gcc/config/aarch64/aarch64-bti-insert.c 
b/gcc/config/aarch64/aarch64-bti-insert.c
index 
e519a0f0ac1751f4268e03381757bc1a10c13144..db8ebb1ba8e45b4bf7cd2f27ae8a2c606a1a6c89
 100644
--- a/gcc/config/aarch64/aarch64-bti-insert.c
+++ b/gcc/config/aarch64/aarch64-bti-insert.c
@@ -106,7 +106,9 @@ aarch64_pac_insn_p (rtx x)
  int unspec_val = XINT (sub, 1);
  switch (unspec_val)
{
-   case UNSPEC_PACISP:
+   case UNSPEC_PACIASP:
+/* fall-through.  */
+case UNSPEC_PACIBSP:
  return true;
 
default:
diff --git a/gcc/config/aarch64/aarch64-builtins.c

Re: [PATCH GCC10] ipa-inline.c: Trivial fix on function not declared inline check in want_inline_small_function_p

[Segher Boessenkool]

Hi!

On Fri, Mar 01, 2019 at 04:39:38PM +0800, JunMa wrote:
>Since MAX_INLINE_INSNS_AUTO should be below or equal to 
>MAX_INLINE_INSNS_SINGLE (see params.def), there is no need
>to do second inlining limit check on growth when function not
>declared inline, this patch removes it.
>Bootstrapped and tested on x86_64-unknown-linux-gnu, is it ok for trunk?

Your mail subject says this is for GCC 10, but you are asking for GCC 9
now; which is it?

> 2019-03-01  Jun Ma  
> 
> *ipa-inline.c(want_inline_small_function_p): Remove
> redundant growth check when function not declared 
> inline

Some spaces were lost in the first line.  Trailing space.  Sentences
should end with a full stop (or similar).

Don't send patches (or pretty much anything else) as
application/octet-stream attachments.


Segher

Re: [libgo] Properly determine executable path on Solaris

[Ian Lance Taylor]

On Fri, Mar 1, 2019 at 2:06 AM Rainer Orth  
wrote:
>
> One of the remaining libgo testsuite failures on Solaris/SPARC is
>
> --- FAIL: TestExecutable (0.04s)
> executable_test.go:46: exec(self) failed: fork/exec .: permission denied
>
> FAIL: os
>
> This happens only for 64-bit.  truss indeed shows
>
> 3181:   execve(".", 0xC000170240, 0xC000178340) Err#13 EACCES
>
> which is completely bogus.  Further investigation shows that
> os.Executable() returns an empty string here.  I traced this down to
> go/runtime/os3_solaris.go (solarisExecutablePath) which tries to locate
> auxv from argv (I suppose the layout is prescribed by the psABIs, but
> haven't looked) and extract the AT_PAGESZ and AT_SUN_EXECNAME members.
>
> In doing so, it assumes that auxv ist just an uintptr[], which is wrong:
>  has
>
> typedef struct
> {
> int a_type;
> union {
> longa_val;
> void*a_ptr;
> void(*a_fcn)();
> } a_un;
> } auxv_t;
>
> Interpreting this as uintptr[] works for 32-bit and accidentally on
> little-endian 64-bit (amd64), but breaks on big-endian 64-bit (sparcv9)
> as observed.
>
> While this could be corrected, there's a far easier and more portable
> way to get at the required information: AT_PAGESZ/pysPageSize can be
> obtained via getpagesize(3C) and AT_SUN_EXECNAME/executablePath is
> available via getexecname(3C), both of which are available as far back
> as Solaris 10 at least.
>
> The following patch does just that.  Tested on i386-pc-solaris2.11 and
> sparc-sun-solaris2.11 (both 32 and 64-bit) without regressions, but
> fixing the os failure on sparcv9.  I'm running Solaris 10 bootstraps
> right now for good measure, but don't expect any issues there.

Thanks.  Committed to mainline.

Ian

Re: [PATCH] C++2a Utility functions to implement uses-allocator construction (P0591R4)

[Jonathan Wakely]

On 01/03/19 14:06 +, Jonathan Wakely wrote:

+ if constexpr (sizeof...(__args) == 0)
+   {
+ return std::make_tuple(piecewise_construct,
+ std::__uses_alloc_args<_Tp1>(__a),
+ std::__uses_alloc_args<_Tp2>(__a));
+   }
+ else if constexpr (sizeof...(__args) == 1)
+   {


If we want to ensure we only accept std::pair and not other pair-like
types, then this branch would need:

 static_assert((__is_pair<_Args>::value && ...));


+ return std::make_tuple(piecewise_construct,
+ std::__uses_alloc_args<_Tp1>(__a,
+   std::forward<_Args>(__args).first...),
+ std::__uses_alloc_args<_Tp2>(__a,
+   std::forward<_Args>(__args).second...));
+   }
+ else if constexpr (sizeof...(__args) == 2)

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Wilco Dijkstra]

Hi Jakub,

> Well, with the patch the decision which insn is chosen is done in C code.
> So it could look like:
>  if (operands[2] == const0_rtx
>  || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
>    subs; // mandatory
>  else if (TARGET_THUMB2
>      && arm_immediate_operand (operands[2], SImode)
>      && arm_neg_immediate_operand (operands[2], SImode))
>    {
>  // Both adds and subs can do the job here, and unlike
>  // non-thumb mode, the instructions can have different
>  // sizes.  Pick the shorter one.
>    }
>  else if (which_alternative == 1)
>    subs;
>  else
>    adds;
>
> This can be done incrementally, I just have no idea what the rules
> for thumb2 constant encoding are.

This is overcomplicating something simple - adds/subs are completely
symmetrical on all Arm targets. So for addition you simply place adds 
alternative first and then subs. For comparison/subtraction place the 
subs alternative first, then adds. Now all special cases will be correct
automatically without needing any extra work or code.

In terms of codesize you need one little tweak for Thumb-2 given that
add(s) and sub(s) of -1 would use a 32-bit rather than 16-bit encoding.
This is handled by not allowing it as a valid immediate on Thumb-2, so
that add -1 is emitted as sub 1 instead (this is not really a loss given that
AND -1, OR -1, EOR -1 etc are all redundant).

Wilco

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 02:57:57PM +, Wilco Dijkstra wrote:
> > This can be done incrementally, I just have no idea what the rules
> > for thumb2 constant encoding are.
> 
> This is overcomplicating something simple - adds/subs are completely
> symmetrical on all Arm targets. So for addition you simply place adds 
> alternative first and then subs. For comparison/subtraction place the 

As I wrote, that is what I have done, and regtest revealed two code size
regressions because of that.  Is -1 vs. 1 the only case of immediate
valid for both "I" and "L" constraints where the former is longer than the
latter?

Jakub

[wwwdocs] update projects/cxx-status.html

[Jakub Jelinek]

On Tue, Feb 26, 2019 at 02:11:03PM -0500, Nathan Sidwell wrote:
> now modules and coroutines are slated to be in C++20, here's a doc patch for
> them, pointing at their respective wiki pages.

Several more papers have been accepted apparently, so I've incorporated your
changed into a larger patch.

I believe we now implement both P1002R1 (allow changing active union member)
and P1330R0 (try in constexpr), so that is noted too.

Not really sure if we don't implement the two "Stronger Unicode requirements"
papers without any work, someone would need to verify.

Ok for wwwdocs?

Index: htdocs/projects/cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.81
diff -u -p -r1.81 cxx-status.html
--- htdocs/projects/cxx-status.html 7 Feb 2019 09:06:57 -   1.81
+++ htdocs/projects/cxx-status.html 1 Mar 2019 15:26:03 -
@@ -146,7 +146,8 @@
Consistent comparison (operator<=>) 
   http://wg21.link/p0515r3";>P0515R3
http://wg21.link/p0905r1";>P0905R1
-   http://wg21.link/p1120r0";>P1120R0
+   http://wg21.link/p1120r0";>P1120R0
+   http://wg21.link/p1185r2";>P1185R2
No 
__cpp_impl_three_way_comparison >= 201711 
 
@@ -249,7 +250,8 @@
 
Support for contract based programming in C++ 
   http://wg21.link/p0542r5";>P0542R5
-  http://wg21.link/p1289r1";>P1289R1
+  http://wg21.link/p1289r1";>P1289R1
+  http://wg21.link/p1323r2";>P1323R2
No (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88102";>PR 88102)

 
@@ -290,20 +292,60 @@

 
 
-   Relaxations of constexpr restrictions 
-  http://wg21.link/p1002r1";>P1002R1
-  http://wg21.link/p1327r1";>P1327R1
-  http://wg21.link/p1330r0";>P1330R0
-   No 
+   Relaxations of constexpr restrictions 
+  http://wg21.link/p1002r1";>P1002R1
+   9 
+   
+
+
+  http://wg21.link/p1327r1";>P1327R1
+   No 
+   
+
+
+  http://wg21.link/p1330r0";>P1330R0
+   9 


 
 
Feature test macros 
-  http://wg21.link/p0941r2";>P0941R2
+  http://wg21.link/p0941r2";>P0941R2
4.9 
(__cpp_ macros) 
 5 
(__has_cpp_attribute) 

 
+
+   Modules 
+  http://wg21.link/p1103r3";>P1103R3
+  No (Modules 
wiki) 
+   
+
+
+   Coroutines 
+  http://wg21.link/p0912r5";>P0912R5
+  No (Coroutines 
Wiki) 
+   
+
+
+   Parenthesized initialization of aggregates 
+  http://wg21.link/p0960r3";>P0960R3
+   No 
+   __cpp_aggregate_paren_init >= 201902
+
+
+   Stronger Unicode requirements 
+  http://wg21.link/p1041r4";>P1041R4
+  http://wg21.link/p1139r2";>P1139R2
+   No 
+   
+
+
+   Structured binding extensions 
+  http://wg21.link/p1091r3";>P1091R3
+  http://wg21.link/p1381r1";>P1381R1
+   No 
+   
+
   
 
   C++17 Support in GCC


Jakub

Re: [wwwdocs] update projects/cxx-status.html

[Nathan Sidwell]

On 3/1/19 10:30 AM, Jakub Jelinek wrote:

On Tue, Feb 26, 2019 at 02:11:03PM -0500, Nathan Sidwell wrote:

now modules and coroutines are slated to be in C++20, here's a doc patch for
them, pointing at their respective wiki pages. >

Several more papers have been accepted apparently, so I've incorporated your
changed into a larger patch.


Thanks!


Index: htdocs/projects/cxx-status.html



+  http://wg21.link/p1103r3";>P1103R3
+  No (Modules wiki) 




+  http://wg21.link/p0912r5";>P0912R5
+  No (Coroutines 
Wiki) 


Inconsistent capitalization of 'wiki'.  Undoubtedly my fault.

nathan

--
Nathan Sidwell

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Wilco Dijkstra]

Hi Jakub,

>> This is overcomplicating something simple - adds/subs are completely
>> symmetrical on all Arm targets. So for addition you simply place adds 
>> alternative first and then subs. For comparison/subtraction place the 
>
> As I wrote, that is what I have done, 

That's not what your proposed patch does:

-  "TARGET_32BIT && INTVAL (operands[2]) == -INTVAL (operands[3])"
-  "@
-   adds%?\\t%0, %1, %3
-   subs%?\\t%0, %1, #%n3"
+  "TARGET_32BIT
+   && (INTVAL (operands[2])
+   == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
+{
+  /* For 0 and INT_MIN it is essential that we use subs, as adds
+ will result in different condition codes (like cmn rather than
+ like cmp).  For other immediates, we should choose whatever
+ will have smaller encoding.  */
+  if (operands[2] == const0_rtx
+  || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
+  || which_alternative == 1)
+return "subs%?\\t%0, %1, #%n3";
+  else
+return "adds%?\\t%0, %1, %3";
+}

Adds/subs were in the incorrect order before and should simply be swapped
rather than replaced by complex C code (which would be unique just to this 
pattern
when there are lot of similar patterns that do the right thing already).

> and regtest revealed two code size
> regressions because of that.  Is -1 vs. 1 the only case of immediate
> valid for both "I" and "L" constraints where the former is longer than the
> latter?

Yes -1 is the only case which can result in larger code on Thumb-2, so -1 should
be disallowed by the I constraint (or even better, the underlying query). That 
way
it will work correctly for all add/sub patterns, not just this one.

Wilco

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 03:41:33PM +, Wilco Dijkstra wrote:
> >> This is overcomplicating something simple - adds/subs are completely
> >> symmetrical on all Arm targets. So for addition you simply place adds 
> >> alternative first and then subs. For comparison/subtraction place the 
> >
> > As I wrote, that is what I have done, 
> 
> That's not what your proposed patch does:

But the earlier version of that patch did, see the PR, in particular
https://gcc.gnu.org/bugzilla/attachment.cgi?id=45840

> 
> -  "TARGET_32BIT && INTVAL (operands[2]) == -INTVAL (operands[3])"
> -  "@
> -   adds%?\\t%0, %1, %3
> -   subs%?\\t%0, %1, #%n3"
> +  "TARGET_32BIT
> +   && (INTVAL (operands[2])
> +   == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
> +{
> +  /* For 0 and INT_MIN it is essential that we use subs, as adds
> + will result in different condition codes (like cmn rather than
> + like cmp).  For other immediates, we should choose whatever
> + will have smaller encoding.  */
> +  if (operands[2] == const0_rtx
> +  || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x8000)
> +  || which_alternative == 1)
> +return "subs%?\\t%0, %1, #%n3";
> +  else
> +return "adds%?\\t%0, %1, %3";
> +}
> 
> Adds/subs were in the incorrect order before and should simply be swapped
> rather than replaced by complex C code (which would be unique just to this 
> pattern
> when there are lot of similar patterns that do the right thing already).

I don't see what's wrong on that, the code isn't really complex and actually
explains what it does and why.  If -1/1 is the only case where thumb2 cares,
it will be trivial to add that.  For operands[2] == const1_rtx we then want
to use always adds and for operands[2] == constm1_rtx we want to use subs.

Or swap the alternatives (then 0 and 0x8000 will come out naturaly out
of it) and just special case the operands[2] == const1_rtx case.
That would then be for this hunk only following.

Changing the "I" constraint or adding new constraints seems too risky or
overkill.

--- gcc/config/arm/arm.md.jj2019-03-01 09:05:56.911097368 +0100
+++ gcc/config/arm/arm.md   2019-03-01 17:00:23.284076436 +0100
@@ -863,14 +863,24 @@ (define_insn "cmpsi2_addneg"
   [(set (reg:CC CC_REGNUM)
(compare:CC
 (match_operand:SI 1 "s_register_operand" "r,r")
-(match_operand:SI 2 "arm_addimm_operand" "L,I")))
+(match_operand:SI 2 "arm_addimm_operand" "I,L")))
(set (match_operand:SI 0 "s_register_operand" "=r,r")
(plus:SI (match_dup 1)
-(match_operand:SI 3 "arm_addimm_operand" "I,L")))]
-  "TARGET_32BIT && INTVAL (operands[2]) == -INTVAL (operands[3])"
-  "@
-   adds%?\\t%0, %1, %3
-   subs%?\\t%0, %1, #%n3"
+(match_operand:SI 3 "arm_addimm_operand" "L,I")))]
+  "TARGET_32BIT
+   && (INTVAL (operands[2])
+   == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
+{
+  /* For 0 and INT_MIN it is essential that we use subs, as adds
+ will result in different condition codes (like cmn rather than
+ like cmp).  Both alternatives can match also for -1/1 with
+ TARGET_THUMB2, prefer instruction with #1 in that case as it
+ is shorter.  */
+  if (which_alternative == 0 && operands[1] != const1_rtx)
+return "subs%?\\t%0, %1, #%n3";
+  else
+return "adds%?\\t%0, %1, %3";
+}
   [(set_attr "conds" "set")
(set_attr "type" "alus_sreg")]
 )

Jakub

Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 05:04:24PM +0100, Jakub Jelinek wrote:
> +  /* For 0 and INT_MIN it is essential that we use subs, as adds
> + will result in different condition codes (like cmn rather than
> + like cmp).  Both alternatives can match also for -1/1 with
> + TARGET_THUMB2, prefer instruction with #1 in that case as it
> + is shorter.  */
> +  if (which_alternative == 0 && operands[1] != const1_rtx)
   ^^^ operands[3] != const1_rtx
or operands[2] != constm1_rtx.
Sorry.

> +return "subs%?\\t%0, %1, #%n3";
> +  else
> +return "adds%?\\t%0, %1, %3";
> +}
>[(set_attr "conds" "set")
> (set_attr "type" "alus_sreg")]
>  )

Jakub

[PATCH] rs6000: Add -mdejagnu-cpu=

[Segher Boessenkool]

This adds an option -mdejagnu-cpu=.  This option simply overrides what
is given in -mcpu=.  The reason for this is that with older versions
of DejaGnu the value given in the RUNTESTFLAGS will override the value
a testcase wants to have.

Ill commit this patch with the first changelog, and also the result of
these two one-liners with the second changelog:

perl -ni -e 'print unless /dg-skip-if "do not override -mcpu"/' \
  $(find gcc/testsuite/gcc.target/powerpc/ -type f)
perl -pi -e 's/(dg-options.*)-mcpu=/\1-mdejagnu-cpu=/'  \
  $(find gcc/testsuite/gcc.target/powerpc/ -type f)


Segher


2019-03-01  Segher Boessenkool  

* config/rs6000/rs6000.c (rs6000_option_override_internal): If
rs6000_dejagnu_cpu_index is set, use that to override rs6000_cpu_index.
* config/rs6000/rs6000.opt (mdejagnu-cpu=): New option.


2019-03-01  Segher Boessenkool  

gcc/testsuite/
* gcc.target/powerpc/ throughout: Delete dg-skip-if "do not override
-mcpu".  Use -mdejagnu-cpu= in dg-options instead of -mcpu=.

---
 gcc/config/rs6000/rs6000.c   | 3 +++
 gcc/config/rs6000/rs6000.opt | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b1249bc..b489bef 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3869,6 +3869,9 @@ rs6000_option_override_internal (bool global_init_p)
   /* Don't override by the processor default if given explicitly.  */
   set_masks &= ~rs6000_isa_flags_explicit;
 
+  if (global_init_p && rs6000_dejagnu_cpu_index >= 0)
+rs6000_cpu_index = rs6000_dejagnu_cpu_index;
+
   /* Process the -mcpu= and -mtune= argument.  If the user changed
  the cpu in a target attribute or pragma, but did not specify a tuning
  option, use the cpu for the tuning option rather than the option specified
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 2e90bf3..f4b5c91 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -385,6 +385,13 @@ mtune=
 Target RejectNegative Joined Var(rs6000_tune_index) Init(-1) 
Enum(rs6000_cpu_opt_value) Save
 -mtune=Schedule code for given CPU.
 
+; Only for use in the testsuite.  This simply overrides -mcpu=.  With older
+; versions of Dejagnu the command line arguments you set in RUNTESTFLAGS
+; override those set in the testcases; with this option, the testcase will
+; always win.
+mdejagnu-cpu=
+Target Undocumented RejectNegative Joined Var(rs6000_dejagnu_cpu_index) 
Init(-1) Enum(rs6000_cpu_opt_value) Save
+
 mtraceback=
 Target RejectNegative Joined Enum(rs6000_traceback_type) Var(rs6000_traceback)
 -mtraceback=[full,part,no] Select type of traceback table.
-- 
1.8.3.1

Re: [PATCH] rs6000: Add -mdejagnu-cpu=

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 05:02:58PM +, Segher Boessenkool wrote:
> This adds an option -mdejagnu-cpu=.  This option simply overrides what
> is given in -mcpu=.  The reason for this is that with older versions
> of DejaGnu the value given in the RUNTESTFLAGS will override the value
> a testcase wants to have.

Ugh, that is ugly.  Can't we just detect the old dejagnu and override
whatever tcl method is responsible for that?
Or just document higher minimum dejagnu version requirement?

> Ill commit this patch with the first changelog, and also the result of
> these two one-liners with the second changelog:
> 
> perl -ni -e 'print unless /dg-skip-if "do not override -mcpu"/' \
>   $(find gcc/testsuite/gcc.target/powerpc/ -type f)
> perl -pi -e 's/(dg-options.*)-mcpu=/\1-mdejagnu-cpu=/'  \
>   $(find gcc/testsuite/gcc.target/powerpc/ -type f)
> 
> 
> Segher
> 
> 
> 2019-03-01  Segher Boessenkool  
> 
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): If
>   rs6000_dejagnu_cpu_index is set, use that to override rs6000_cpu_index.
>   * config/rs6000/rs6000.opt (mdejagnu-cpu=): New option.
> 
> 
> 2019-03-01  Segher Boessenkool  
> 
> gcc/testsuite/
>   * gcc.target/powerpc/ throughout: Delete dg-skip-if "do not override
>   -mcpu".  Use -mdejagnu-cpu= in dg-options instead of -mcpu=.

Jakub

Re: A bug in vrp_meet?

[Qing Zhao]

Jeff,

thanks a lot for the reply.

this is really helpful.

I double checked the dumped intermediate file for pass “dom3", and located the 
following for _152:

BEFORE the pass “dom3”, there is no _152, the corresponding Block looks 
like:

   [local count: 12992277]:
  _98 = (int) ufcMSR_52(D);
  k_105 = (sword) ufcMSR_52(D);
  i_49 = _98 > 0 ? k_105 : 0;

***During the pass “doms”,  _152 is generated as following:

Optimizing block #4
….
Visiting statement:
i_49 = _98 > 0 ? k_105 : 0;
Meeting
  [0, 65535]
and
  [0, 0]
to
  [0, 65535]
Intersecting
  [0, 65535]
and
  [0, 65535]
to
  [0, 65535]
Optimizing statement i_49 = _98 > 0 ? k_105 : 0;
  Replaced 'k_105' with variable '_98'
gimple_simplified to _152 = MAX_EXPR <_98, 0>;
i_49 = _152;
  Folded to: i_49 = _152;
LKUP STMT i_49 = _152
 ASGN i_49 = _152

then bb 4 becomes:

   [local count: 12992277]:
  _98 = (int) ufcMSR_52(D);
  k_105 = _98;
  _152 = MAX_EXPR <_98, 0>;
  i_49 = _152;

and all the i_49 are replaced with _152. 

However, the value range info for _152 doesnot reflect the one for i_49, it 
keeps as UNDEFINED. 

is this the root problem?  

thanks a lot.

Qing

> On Feb 28, 2019, at 1:54 PM, Jeff Law  wrote:
> 
> On 2/28/19 10:05 AM, Qing Zhao wrote:
>> Hi,
>> 
>> I have been debugging a runtime error caused by value range propagation. and 
>> finally located to the following gcc routine:
>> 
>> vrp_meet_1 in gcc/tree-vrp.c
>> 
>> 
>> /* Meet operation for value ranges.  Given two value ranges VR0 and
>>   VR1, store in VR0 a range that contains both VR0 and VR1.  This
>>   may not be the smallest possible such range.  */
>> 
>> static void 
>> vrp_meet_1 (value_range *vr0, const value_range *vr1)
>> {
>>  value_range saved;
>> 
>>  if (vr0->type == VR_UNDEFINED)
>>{
>>  set_value_range (vr0, vr1->type, vr1->min, vr1->max, vr1->equiv);
>>  return;
>>}
>> 
>>  if (vr1->type == VR_UNDEFINED)
>>{
>>  /* VR0 already has the resulting range.  */
>>  return;
>>}
>> 
>> 
>> In the above, when one of vr0 or vr1 is VR_UNDEFINED,  the meet result of 
>> these two will be  the other VALUE. 
>> 
>> This seems not correct to me. 
> That's the optimistic nature of VRP.  It's generally desired behavior.
> 
>> 
>> For example, the following is the located incorrect value range propagation: 
>>  (portion from the dump file *.181t.dom3)
>> 
>> 
>> Visiting PHI node: i_83 = PHI <_152(20), 0(22)>
>>Argument #0 (20 -> 10 executable)
>>_152: UNDEFINED
>>Argument #1 (22 -> 10 executable)
>>0: [0, 0]
>> Meeting
>>  UNDEFINED
>> and
>>  [0, 0]
>> to
>>  [0, 0]
>> Intersecting
>>  [0, 0]
>> and
>>  [0, 65535]
>> to
>>  [0, 0]
>> 
>> 
>> 
>> In the above, “i_83” is defined as PHI <_152(20), 0(22)>,   the 1st argument 
>> is UNDEFINED at this time(but its value range definitely is NOT [0,0]),
>> and the 2nd argument is 0.
> If it's value is undefined then it can be any value we want.  We choose
> to make it equal to the other argument.
> 
> If VRP later finds that _152 changes, then it will go back and
> reevaluate the PHI.  That's one of the basic design principles of the
> optimistic propagators.
> 
>> 
>> “vrp_meet” generate a VR_RANGE with [0,0] for “i_83” based on the current 
>> algorithm.  Obviously, this result VR_RANGE with [0,0] does NOT 
>> contain the value ranges for _152. 
>> 
>> the result of “vrp_meet” is Not correct.  and this incorrect value range 
>> result finally caused the runtime error. 
>> 
>> I ‘d like to modify the vrp_meet_1 as following:
>> 
>> 
>> static void 
>> vrp_meet_1 (value_range *vr0, const value_range *vr1)
>> {
>>  value_range saved;
>> 
>>  if (vr0->type == VR_UNDEFINED)
>>{
>>  /* VR0 already has the resulting range. */
>>  return;
>>}
>> 
>>  if (vr1->type == VR_UNDEFINED)
>>{
>>  set_value_range_to_undefined (vr0)
>> return;
>>}
>> 
>> 
>> let me know your opinion.
>> 
>> thanks a lot for the help.
> I think we (Richi and I) went through this about a year ago and the
> conclusion was we should be looking at how you're getting into the
> vrp_meet with the VR_UNDEFINED.
> 
> If it happens because the user's code has an undefined use, then, the
> consensus has been to go ahead and optimize it.
> 
> If it happens for any other reason, then it's likely a bug in GCC.  We
> had a couple of these when we made EVRP a re-usable module and started
> exploiting its data in the jump threader.
> 
> So you need to work backwards from this point to figure out how you got
> here.
> 
> jeff

Re: [PATCH] rs6000: Add -mdejagnu-cpu=

[Segher Boessenkool]

On Fri, Mar 01, 2019 at 06:07:57PM +0100, Jakub Jelinek wrote:
> On Fri, Mar 01, 2019 at 05:02:58PM +, Segher Boessenkool wrote:
> > This adds an option -mdejagnu-cpu=.  This option simply overrides what
> > is given in -mcpu=.  The reason for this is that with older versions
> > of DejaGnu the value given in the RUNTESTFLAGS will override the value
> > a testcase wants to have.
> 
> Ugh, that is ugly.

Hey, it's a nice improvement over what we have had for years already.

> Can't we just detect the old dejagnu and override
> whatever tcl method is responsible for that?

Not sure.  We probably could.  But are the new semantics actually
better?  Sometimes you want the testcase flags to override the RUNTESTFLAGS,
but just as often you don't.  So now older dejagnu versions have one
behaviour (and we will see those for very many years still, certainly on
Power, where we mostly use slow^Wold^Wenterprise distros), and newer dejagnu
versions have another behaviour, some things break with the old, some things
break with the new, and it's all a big mess.

Having separate flags for the testcases that say "override the RUNTESTFLAGS"
gives the best of both worlds.  Even with with new dejagnu, where normally
the RUNTESTFLAGS always wins.

> Or just document higher minimum dejagnu version requirement?

Maybe in ten years time?  Or *maybe* in five.

Some systems still ship with 1.5.1...

(On powerpc we can get away with this because testcases do not (well, should
not) use -m32/-m64, and we only use -mcpu in the RUNTESTFLAGS otherwise).


Segher

Re: [PATCH] rs6000: Add -mdejagnu-cpu=

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 12:00:50PM -0600, Segher Boessenkool wrote:
> On Fri, Mar 01, 2019 at 06:07:57PM +0100, Jakub Jelinek wrote:
> > On Fri, Mar 01, 2019 at 05:02:58PM +, Segher Boessenkool wrote:
> > > This adds an option -mdejagnu-cpu=.  This option simply overrides what
> > > is given in -mcpu=.  The reason for this is that with older versions
> > > of DejaGnu the value given in the RUNTESTFLAGS will override the value
> > > a testcase wants to have.
> > 
> > Ugh, that is ugly.
> 
> Hey, it's a nice improvement over what we have had for years already.

Even if we don't document the new option, I'm sure some users will start using 
it,
report bugs for it, ...

> > Can't we just detect the old dejagnu and override
> > whatever tcl method is responsible for that?
> 
> Not sure.  We probably could.  But are the new semantics actually
> better?  Sometimes you want the testcase flags to override the RUNTESTFLAGS,
> but just as often you don't.  So now older dejagnu versions have one
> behaviour (and we will see those for very many years still, certainly on
> Power, where we mostly use slow^Wold^Wenterprise distros), and newer dejagnu
> versions have another behaviour, some things break with the old, some things
> break with the new, and it's all a big mess.

CCing Jonathan here, as he has I think dealt with this a lot in the
libstdc++ testsuite.

I believe the new dejagnu behavior is considered the desirable/right one, I
fail to see when the old behavior would be wanted.  If a testcase wants to
override something, it should be able to.
> 
> Having separate flags for the testcases that say "override the RUNTESTFLAGS"
> gives the best of both worlds.  Even with with new dejagnu, where normally
> the RUNTESTFLAGS always wins.

I thought new dejagnu behavior is RUNTESTFLAGS are placed early on the
command line and dg-options follow.

Jakub

Re: [PATCH] rs6000: Add -mdejagnu-cpu=

[Segher Boessenkool]

On Fri, Mar 01, 2019 at 07:09:05PM +0100, Jakub Jelinek wrote:
> > Having separate flags for the testcases that say "override the RUNTESTFLAGS"
> > gives the best of both worlds.  Even with with new dejagnu, where normally
> > the RUNTESTFLAGS always wins.
> 
> I thought new dejagnu behavior is RUNTESTFLAGS are placed early on the
> command line and dg-options follow.

Yes I wrote that backwards, sorry.


Segher

Re: [PATCH] rs6000: Add -mdejagnu-cpu=

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 07:09:05PM +0100, Jakub Jelinek wrote:
> > > Can't we just detect the old dejagnu and override
> > > whatever tcl method is responsible for that?
> > 
> > Not sure.  We probably could.  But are the new semantics actually
> > better?  Sometimes you want the testcase flags to override the RUNTESTFLAGS,
> > but just as often you don't.  So now older dejagnu versions have one
> > behaviour (and we will see those for very many years still, certainly on
> > Power, where we mostly use slow^Wold^Wenterprise distros), and newer dejagnu
> > versions have another behaviour, some things break with the old, some things
> > break with the new, and it's all a big mess.
> 
> CCing Jonathan here, as he has I think dealt with this a lot in the
> libstdc++ testsuite.
> 
> I believe the new dejagnu behavior is considered the desirable/right one, I
> fail to see when the old behavior would be wanted.  If a testcase wants to
> override something, it should be able to.
> > 
> > Having separate flags for the testcases that say "override the RUNTESTFLAGS"
> > gives the best of both worlds.  Even with with new dejagnu, where normally
> > the RUNTESTFLAGS always wins.
> 
> I thought new dejagnu behavior is RUNTESTFLAGS are placed early on the
> command line and dg-options follow.

We are talking about the
http://git.savannah.gnu.org/cgit/dejagnu.git/commit/?id=5256bd82343000c76bc0e48139003f90b6184347
change, right?
If we choose to require dejagnu with that fix, shouldn't be that hard to
apply that one-liner patch if whole dejagnu can't be upgraded.

Or just like for libstdc++, document that using --target_board multilib
options with older dejagnu is unsupported or has certain limitations.

Jakub

Re: [PATCH] Fix PR89497

[Jakub Jelinek]

On Fri, Mar 01, 2019 at 10:20:05AM +0100, Richard Biener wrote:
> 2019-03-01  Richard Biener  
> 
>   PR middle-end/89497
>   * tree-cfgcleanup.h (cleanup_tree_cfg): Add SSA update flags
>   argument, defaulted to zero.
>   * passes.c (execute_function_todo): Pass down SSA update flags
>   to cleanup_tree_cfg.
>   * tree-cfgcleanup.c: Include tree-into-ssa.h and tree-cfgcleanup.h.
>   (cleanup_tree_cfg_noloop): After cleanup_control_flow_pre update SSA
>   form if requested.
>   (cleanup_tree_cfg): Get and pass down SSA update flags.
> 
>   * gcc.dg/tree-ssa/reassoc-43.c: Avoid false match in regex.
>   * g++.dg/tree-prof/devirt.C: Scan tracer dump for foldings
>   that happen now earlier.

This broke the devirt.C testcase on i686-linux or x86_64-linux with
--target_board=unix/-m32.

Fixed thusly, committed as obvious:

2019-03-01  Jakub Jelinek  

PR middle-end/89497
* g++.dg/tree-prof/devirt.C: Adjust also the ilp32
scan-tree-dump-times from dom3 to tracer pass.

--- gcc/testsuite/g++.dg/tree-prof/devirt.C.jj  2019-03-01 10:26:01.252871860 
+0100
+++ gcc/testsuite/g++.dg/tree-prof/devirt.C 2019-03-01 20:03:12.464332566 
+0100
@@ -122,5 +122,5 @@ main ()
 }
 
 /* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function 
call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1 "tracer" { 
target { lp64 || llp64 } } } } */
-/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function 
call to virtual unsigned int mozPersonalDictionary::_ZThn8" 1 "dom3" { target 
ilp32 } } } */
+/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function 
call to virtual unsigned int mozPersonalDictionary::_ZThn8" 1 "tracer" { target 
ilp32 } } } */
 /* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function 
call to virtual unsigned int mozPersonalDictionary::AddRef" 1 "tracer" } } */

Jakub

Re: A bug in vrp_meet?

[Richard Biener]

On March 1, 2019 6:49:20 PM GMT+01:00, Qing Zhao  wrote:
>Jeff,
>
>thanks a lot for the reply.
>
>this is really helpful.
>
>I double checked the dumped intermediate file for pass “dom3", and
>located the following for _152:
>
>BEFORE the pass “dom3”, there is no _152, the corresponding Block
>looks like:
>
>   [local count: 12992277]:
>  _98 = (int) ufcMSR_52(D);
>  k_105 = (sword) ufcMSR_52(D);
>  i_49 = _98 > 0 ? k_105 : 0;
>
>***During the pass “doms”,  _152 is generated as following:
>
>Optimizing block #4
>….
>Visiting statement:
>i_49 = _98 > 0 ? k_105 : 0;
>Meeting
>  [0, 65535]
>and
>  [0, 0]
>to
>  [0, 65535]
>Intersecting
>  [0, 65535]
>and
>  [0, 65535]
>to
>  [0, 65535]
>Optimizing statement i_49 = _98 > 0 ? k_105 : 0;
>  Replaced 'k_105' with variable '_98'
>gimple_simplified to _152 = MAX_EXPR <_98, 0>;
>i_49 = _152;
>  Folded to: i_49 = _152;
>LKUP STMT i_49 = _152
> ASGN i_49 = _152
>
>then bb 4 becomes:
>
>   [local count: 12992277]:
>  _98 = (int) ufcMSR_52(D);
>  k_105 = _98;
>  _152 = MAX_EXPR <_98, 0>;
>  i_49 = _152;
>
>and all the i_49 are replaced with _152. 
>
>However, the value range info for _152 doesnot reflect the one for
>i_49, it keeps as UNDEFINED. 
>
>is this the root problem?  

It looks like DOM fails to visit stmts generated by simplification. Can you 
open a bug report with a testcase?

Richard. 

>thanks a lot.
>
>Qing
>
>> On Feb 28, 2019, at 1:54 PM, Jeff Law  wrote:
>> 
>> On 2/28/19 10:05 AM, Qing Zhao wrote:
>>> Hi,
>>> 
>>> I have been debugging a runtime error caused by value range
>propagation. and finally located to the following gcc routine:
>>> 
>>> vrp_meet_1 in gcc/tree-vrp.c
>>> 
>>> 
>>> /* Meet operation for value ranges.  Given two value ranges VR0 and
>>>   VR1, store in VR0 a range that contains both VR0 and VR1.  This
>>>   may not be the smallest possible such range.  */
>>> 
>>> static void 
>>> vrp_meet_1 (value_range *vr0, const value_range *vr1)
>>> {
>>>  value_range saved;
>>> 
>>>  if (vr0->type == VR_UNDEFINED)
>>>{
>>>  set_value_range (vr0, vr1->type, vr1->min, vr1->max,
>vr1->equiv);
>>>  return;
>>>}
>>> 
>>>  if (vr1->type == VR_UNDEFINED)
>>>{
>>>  /* VR0 already has the resulting range.  */
>>>  return;
>>>}
>>> 
>>> 
>>> In the above, when one of vr0 or vr1 is VR_UNDEFINED,  the meet
>result of these two will be  the other VALUE. 
>>> 
>>> This seems not correct to me. 
>> That's the optimistic nature of VRP.  It's generally desired
>behavior.
>> 
>>> 
>>> For example, the following is the located incorrect value range
>propagation:  (portion from the dump file *.181t.dom3)
>>> 
>>> 
>>> Visiting PHI node: i_83 = PHI <_152(20), 0(22)>
>>>Argument #0 (20 -> 10 executable)
>>>_152: UNDEFINED
>>>Argument #1 (22 -> 10 executable)
>>>0: [0, 0]
>>> Meeting
>>>  UNDEFINED
>>> and
>>>  [0, 0]
>>> to
>>>  [0, 0]
>>> Intersecting
>>>  [0, 0]
>>> and
>>>  [0, 65535]
>>> to
>>>  [0, 0]
>>> 
>>> 
>>> 
>>> In the above, “i_83” is defined as PHI <_152(20), 0(22)>,   the 1st
>argument is UNDEFINED at this time(but its value range definitely is
>NOT [0,0]),
>>> and the 2nd argument is 0.
>> If it's value is undefined then it can be any value we want.  We
>choose
>> to make it equal to the other argument.
>> 
>> If VRP later finds that _152 changes, then it will go back and
>> reevaluate the PHI.  That's one of the basic design principles of the
>> optimistic propagators.
>> 
>>> 
>>> “vrp_meet” generate a VR_RANGE with [0,0] for “i_83” based on the
>current algorithm.  Obviously, this result VR_RANGE with [0,0] does NOT
>
>>> contain the value ranges for _152. 
>>> 
>>> the result of “vrp_meet” is Not correct.  and this incorrect value
>range result finally caused the runtime error. 
>>> 
>>> I ‘d like to modify the vrp_meet_1 as following:
>>> 
>>> 
>>> static void 
>>> vrp_meet_1 (value_range *vr0, const value_range *vr1)
>>> {
>>>  value_range saved;
>>> 
>>>  if (vr0->type == VR_UNDEFINED)
>>>{
>>>  /* VR0 already has the resulting range. */
>>>  return;
>>>}
>>> 
>>>  if (vr1->type == VR_UNDEFINED)
>>>{
>>>  set_value_range_to_undefined (vr0)
>>> return;
>>>}
>>> 
>>> 
>>> let me know your opinion.
>>> 
>>> thanks a lot for the help.
>> I think we (Richi and I) went through this about a year ago and the
>> conclusion was we should be looking at how you're getting into the
>> vrp_meet with the VR_UNDEFINED.
>> 
>> If it happens because the user's code has an undefined use, then, the
>> consensus has been to go ahead and optimize it.
>> 
>> If it happens for any other reason, then it's likely a bug in GCC. 
>We
>> had a couple of these when we made EVRP a re-usable module and
>started
>> exploiting its data in the jump threader.
>> 
>> So you need to work backwards from this point to figure out how you
>got
>> here.
>> 
>> jeff

[PATCH, rs6000] Fix PR88845: ICE in lra_set_insn_recog_data

[Peter Bergner]

PR88845 shows a problem where LRA spilled an input operand of an inline
asm statement by calling our generic movsf pattern which ended up generating
an insn we don't have a pattern for, so we ICE.  The insn was:

  (insn (set (reg:SF 125)
 (subreg:SF (reg:SI 124) 0)))

The problem is that rs6000_emit_move_si_sf_subreg() is disabled for LRA
and so wasn't able to call gen_movsf_from_si() which generates the correct
pattern for moving a 32-bit value from a GPR to a FPR.  The patch below
fixes the issue by allowing rs6000_emit_move_si_sf_subreg() to be called
during LRA as well as creating an expander so that when it is called during
LRA, we can create the scratch register that is required for its associated
splitter.  We have to do this, since LRA has already converted all of the
scratches into real registers before it does any spilling.

This passed bootstrap and regtesting on powerpc64le-linux with no
regressions.  Ok for mainline?

Peter

gcc/
PR rtl-optimization/88845
* config/rs6000/rs6000.c (rs6000_emit_move_si_sf_subreg): Enable during
LRA.
* config/rs6000/rs6000.md (movsf_from_si): New expander; old insn and
splitter renamed from this ...
(movsf_from_si_internal): ... to this.

gcc/testsuite/
PR rtl-optimization/88845
* gcc.target/powerpc/pr88845.c: New test.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 269263)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -9887,7 +9887,7 @@ valid_sf_si_move (rtx dest, rtx src, mac
 static bool
 rs6000_emit_move_si_sf_subreg (rtx dest, rtx source, machine_mode mode)
 {
-  if (TARGET_DIRECT_MOVE_64BIT && !lra_in_progress && !reload_completed
+  if (TARGET_DIRECT_MOVE_64BIT && !reload_completed
   && (!SUBREG_P (dest) || !sf_subreg_operand (dest, mode))
   && SUBREG_P (source) && sf_subreg_operand (source, mode))
 {
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 269263)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -7353,9 +7353,31 @@ (define_insn "*mov_softfloat"
 ;; This function is called before reload, and it creates the temporary as
 ;; needed.
 
+(define_expand "movsf_from_si"
+  [(parallel [(set (match_operand:SF 0 "nonimmediate_operand")
+  (unspec:SF [(match_operand:SI 1 "input_operand" )]
+ UNSPEC_SF_FROM_SI))
+ (clobber (match_scratch:DI 2))])]
+  "TARGET_NO_SF_SUBREG
+   && (register_operand (operands[0], SFmode)
+   || register_operand (operands[1], SImode))"
+{
+  if (lra_in_progress
+  && REG_P (operands[0])
+  && REG_P (operands[1]))
+{
+  /* If LRA is generating a direct move from a GPR to a FPR,
+then the splitter is going to need a scratch register.  */
+  rtx insn = gen_movsf_from_si_internal (operands[0], operands[1]);
+  XEXP (XVECEXP (insn, 0, 1), 0) = gen_reg_rtx (DImode);
+  emit_insn (insn);
+  DONE;
+}
+})
+
 ;; LWZ  LFSLXSSP  LXSSPX STWSTFIWX
 ;; STXSIWX  GPR->VSX   VSX->GPR   GPR->GPR
-(define_insn_and_split "movsf_from_si"
+(define_insn_and_split "movsf_from_si_internal"
   [(set (match_operand:SF 0 "nonimmediate_operand"
"=!r,   f, wb,wu,m, Z,
 Z, wy,?r,!r")
Index: gcc/testsuite/gcc.target/powerpc/pr88845.c
===
--- gcc/testsuite/gcc.target/powerpc/pr88845.c  (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr88845.c  (working copy)
@@ -0,0 +1,25 @@
+/* { dg-do compile { target powerpc*-*-linux* } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-skip-if "" { powerpc*-*-*spe* } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2" } */
+/* { dg-final { scan-assembler {\mmtvsrd\M} { target { lp64 } } } } */
+/* { dg-final { scan-assembler {\mxscvspdpn\M} { target { lp64 } } } } */
+
+/* Verify that we do not ICE and that we generate a direct move
+   for float types when compiling for 64-bit.  */
+
+struct a {
+  unsigned ui;
+  float f;
+};
+
+void
+foo (void)
+{
+  float e;
+  struct a s;
+  e = s.f;
+  __asm__("" : : "d" (e));
+}

[wwwdocs] Buildstat update for 8.x

[Tom G. Christensen]

Latest results for 8.x.

-tgc

Testresults for 8.3.0:
  aarch64-unknown-linux-gnu
  mips64el-unknown-linux-gnu
  mips64-unknown-linux-gnu
  powerpc64le-unknown-linux-gnu
  powerpc-unknown-linux-gnu
  x86_64-pc-linux-gnu
  x86_64-w64-mingw32

Testresults for 8.2.0:
  aarch64-unknown-linux-gnu
  armv4tl-unknown-linux-gnueabi
  armv5tel-unknown-linux-gnueabi
  hppa2.0-unknown-linux-gnu
  mips64-unknown-linux-gnu
  powerpc64-unknown-linux-gnu (2)
  powerpc-unknown-linux-gnu
  sparc64-sun-solaris2.10
  sparc-unknown-linux-gnu
  x86_64-pc-linux-gnu (2)
  x86_64-w64-mingw32

--- /home/tgc/projects/gcc/wwwdocs/htdocs/gcc-8/buildstat.html  2019-02-27 
18:33:53.563979120 +0100
+++ /tmp/tmp.dppSvEWFlb 2019-02-27 21:47:06.120041371 +0100
@@ -25,6 +25,39 @@
 
 
 
+aarch64-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg03090.html";>8.3.0,
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg02127.html";>8.2.0
+
+
+
+
+armv4tl-unknown-linux-gnueabi
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg03093.html";>8.2.0
+
+
+
+
+armv5tel-unknown-linux-gnueabi
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg02126.html";>8.2.0
+
+
+
+
+hppa2.0-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg00103.html";>8.2.0
+
+
+
+
 i386-pc-solaris2.10
  
 Test results:
@@ -41,6 +74,57 @@
 
 
 
+mips64el-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg03235.html";>8.3.0
+
+
+
+
+mips64-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg02945.html";>8.3.0,
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg01683.html";>8.2.0
+
+
+
+
+powerpc64le-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg02682.html";>8.3.0
+
+
+
+
+powerpc64-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2018-10/msg03744.html";>8.2.0,
+https://gcc.gnu.org/ml/gcc-testresults/2018-10/msg03206.html";>8.2.0
+
+
+
+
+powerpc-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg03317.html";>8.3.0,
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg01801.html";>8.2.0
+
+
+
+
+sparc64-sun-solaris2.10
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2018-10/msg03050.html";>8.2.0
+
+
+
+
 sparc-sun-solaris2.10
  
 Test results:
@@ -57,6 +141,14 @@
 
 
 
+sparc-unknown-linux-gnu
+ 
+Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg03184.html";>8.2.0
+
+
+
+
 x86_64-apple-darwin11.4.2
  
 Test results:
@@ -68,6 +160,9 @@
 x86_64-pc-linux-gnu
  
 Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg02892.html";>8.3.0,
+https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg01684.html";>8.2.0,
+https://gcc.gnu.org/ml/gcc-testresults/2018-08/msg00023.html";>8.2.0,
 https://gcc.gnu.org/ml/gcc-testresults/2018-05/msg00374.html";>8.1.0,
 https://gcc.gnu.org/ml/gcc-testresults/2018-05/msg00132.html";>8.1.0,
 https://gcc.gnu.org/ml/gcc-testresults/2018-05/msg00127.html";>8.1.0
@@ -78,6 +173,8 @@
 x86_64-w64-mingw32
  
 Test results:
+https://gcc.gnu.org/ml/gcc-testresults/2019-02/msg03095.html";>8.3.0,
+https://gcc.gnu.org/ml/gcc-testresults/2018-08/msg02651.html";>8.2.0,
 https://gcc.gnu.org/ml/gcc-testresults/2018-05/msg00583.html";>8.1.0
 
 

Re: A bug in vrp_meet?

[Qing Zhao]

> On Mar 1, 2019, at 1:25 PM, Richard Biener  wrote:
> 
> On March 1, 2019 6:49:20 PM GMT+01:00, Qing Zhao  > wrote:
>> Jeff,
>> 
>> thanks a lot for the reply.
>> 
>> this is really helpful.
>> 
>> I double checked the dumped intermediate file for pass “dom3", and
>> located the following for _152:
>> 
>> BEFORE the pass “dom3”, there is no _152, the corresponding Block
>> looks like:
>> 
>>  [local count: 12992277]:
>> _98 = (int) ufcMSR_52(D);
>> k_105 = (sword) ufcMSR_52(D);
>> i_49 = _98 > 0 ? k_105 : 0;
>> 
>> ***During the pass “doms”,  _152 is generated as following:
>> 
>> Optimizing block #4
>> ….
>> Visiting statement:
>> i_49 = _98 > 0 ? k_105 : 0;
>> Meeting
>> [0, 65535]
>> and
>> [0, 0]
>> to
>> [0, 65535]
>> Intersecting
>> [0, 65535]
>> and
>> [0, 65535]
>> to
>> [0, 65535]
>> Optimizing statement i_49 = _98 > 0 ? k_105 : 0;
>> Replaced 'k_105' with variable '_98'
>> gimple_simplified to _152 = MAX_EXPR <_98, 0>;
>> i_49 = _152;
>> Folded to: i_49 = _152;
>> LKUP STMT i_49 = _152
>>  ASGN i_49 = _152
>> 
>> then bb 4 becomes:
>> 
>>  [local count: 12992277]:
>> _98 = (int) ufcMSR_52(D);
>> k_105 = _98;
>> _152 = MAX_EXPR <_98, 0>;
>> i_49 = _152;
>> 
>> and all the i_49 are replaced with _152. 
>> 
>> However, the value range info for _152 doesnot reflect the one for
>> i_49, it keeps as UNDEFINED. 
>> 
>> is this the root problem?  
> 
> It looks like DOM fails to visit stmts generated by simplification. Can you 
> open a bug report with a testcase?

The problem is, It took me quite some time in order to come up with a small and 
independent testcase for this problem,
a little bit change made the error disappear.  

do you have any suggestion on this?  or can you give me some hint on how to fix 
this in DOM?  then I can try the fix on my side?

Thanks a lot.

Qing


> 
> Richard. 
> 

Re: [C++ PATCH] Partially fix designated-initializer-list handling in overload resolution (PR c++/71446)

[Jason Merrill]

On 3/1/19 1:53 PM, Jakub Jelinek wrote:

Hi!

http://eel.is/c++draft/dcl.init.list#3.1 says that for designated
initializers only aggregate conversion should be performed,
while our current code would happily try std::initializer_list
initialization, or (as extension) complex initialization, or scalar
initialization if there is { .foobarbaz = 1 }.

Unfortunately, e.g. reshape_init seems to add ce->index values, so checking
whether ce->index is NULL for all elements of the CONSTRUCTOR doesn't seem
to work.

This patch instead remembers in a bit on the CONSTRUCTOR whether there
were any user designators.  Bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?


OK, thanks.


Note, this is just a partial fix for the PR, for the second part (making
struct S { void *a; int b; };
void foo (S);
... foo ({.b = 1}) work), I'm afraid I don't really understand what the C++
standard wants to say in http://eel.is/c++draft/over.ics.list#2 and what
build_aggr_conv should do, such that the cpp2a/desig{4,6}.C testcases taken
from the standard as well as the desig11.C in the earlier version of the
patch (would need to be renamed to desig12.C) can pass.


It seems to me that for designated initializers, we should make sure 
that all the designators match fields, and otherwise check 
convertibility like build_aggr_conv already does.


Jason

Re: [PATCH] PR c/89524 - Ignore -Wno-error=

[Joseph Myers]

On Thu, 28 Feb 2019, Alex Henrie wrote:

> * opts.c: Ignore -Wno-error=.

I'd expect this to follow the same logic as how -Wno- 
is ignored (see the comment on postpone_unknown_option_warning).  That is, 
the option would still be diagnosed if there is some error message that 
might have been turned into a warning by the -Wno-error= option with a 
newer compiler.

-- 
Joseph S. Myers
jos...@codesourcery.com

libgo patch committed: Restore passing D to ar

[Ian Lance Taylor]

In the update of libgo to Go1.12beta2 I accidentally lost the code
that passes D to the ar program.  This patch restores it.
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 269315)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-805c82cc141c593ea2f27d8614ecd204e2b5e76e
+a72eca1f435002076655fd6a54ce869ac39856dc
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/cmd/go/internal/work/gccgo.go
===
--- libgo/go/cmd/go/internal/work/gccgo.go  (revision 269299)
+++ libgo/go/cmd/go/internal/work/gccgo.go  (working copy)
@@ -205,11 +205,14 @@ func (tools gccgoToolchain) pack(b *Buil
if cfg.Goos == "aix" && cfg.Goarch == "ppc64" {
// AIX puts both 32-bit and 64-bit objects in the same archive.
// Tell the AIX "ar" command to only care about 64-bit objects.
-   // AIX "ar" command does not know D option.
arArgs = []string{"-X64"}
}
-
-   return b.run(a, p.Dir, p.ImportPath, nil, tools.ar(), arArgs, "rc", 
mkAbs(objdir, afile), absOfiles)
+   absAfile := mkAbs(objdir, afile)
+   // Try with D modifier first, then without if that fails.
+   if b.run(a, p.Dir, p.ImportPath, nil, tools.ar(), arArgs, "rcD", 
absAfile, absOfiles) != nil {
+   return b.run(a, p.Dir, p.ImportPath, nil, tools.ar(), arArgs, 
"rc", absAfile, absOfiles)
+   }
+   return nil
 }
 
 func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg 
string, allactions []*Action, buildmode, desc string) error {

[PATCH] Fix overflowing integer to an integer exponent

[Steve Kargl]

Consider, the program

   program foo
 i = 10**23
 x = 6.02 * 10**23
 print *, i, x
   end

% gfc8 -o z a.f90 && ./z
  -159383552   6.0181E+23

Now, if 10**23 = -159383552, then 6.02 * 10**23 should be -959488983.04.  
Moreover raising a positive number to a positive exponent should result
in a positive number.  Yes, I'm aware of twos-complement wrap around.
And, finally, the code compiles without so much as a warning.

The attached patch brings some consistency to gfortran.

% gfcx -c a.f90 
a.f90:1:9:

1 |i = 10**23
  | 1
Error: Result of exponentiation at (1) exceeds the range of INTEGER(4)
a.f90:2:16:

2 |x = 6.02 * 10**23
  |1
Error: Result of exponentiation at (1) exceeds the range of INTEGER(4)

It has been regression tested on x86_64-*-freebsd.  OK to commit?

2019-03-01  Steven G. Kargl  

* arith.c (arith_power): Rework overflow of an integer to an integer
exponent.

2019-03-01  Steven G. Kargl  

* gfortran.dg/integer_exponentiation_4.f90: Update test.
* gfortran.dg/integer_exponentiation_5.F90: Ditto.
* gfortran.dg/no_range_check_1.f90: Ditto.

-- 
Steve
Index: gcc/fortran/arith.c
===
--- gcc/fortran/arith.c	(revision 269260)
+++ gcc/fortran/arith.c	(working copy)
@@ -848,8 +848,6 @@ arith_power (gfc_expr *op1, gfc_expr *op2, gfc_expr **
 	{
 	case BT_INTEGER:
 	  {
-		int power;
-
 		/* First, we simplify the cases of op1 == 1, 0 or -1.  */
 		if (mpz_cmp_si (op1->value.integer, 1) == 0)
 		  {
@@ -884,29 +882,36 @@ arith_power (gfc_expr *op1, gfc_expr *op2, gfc_expr **
    "exponent of integer has zero "
    "result at %L", &result->where);
 		  }
-		else if (gfc_extract_int (op2, &power))
+		else
 		  {
-		/* If op2 doesn't fit in an int, the exponentiation will
-		   overflow, because op2 > 0 and abs(op1) > 1.  */
-		mpz_t max;
-		int i;
-		i = gfc_validate_kind (BT_INTEGER, result->ts.kind, false);
+		/* We have abs(op1) > 1 and op2 > 1.
+		   If op2 > bit_size(op1), we'll have an out-of-range
+		   result.  */
+		int k, power;
 
-		if (flag_range_check)
-		  rc = ARITH_OVERFLOW;
-
-		/* Still, we want to give the same value as the
-		   processor.  */
-		mpz_init (max);
-		mpz_add_ui (max, gfc_integer_kinds[i].huge, 1);
-		mpz_mul_ui (max, max, 2);
-		mpz_powm (result->value.integer, op1->value.integer,
-			  op2->value.integer, max);
-		mpz_clear (max);
+		k = gfc_validate_kind (BT_INTEGER, op1->ts.kind, false);
+		power = gfc_integer_kinds[k].bit_size;
+		if (mpz_cmp_si (op2->value.integer, power) < 0)
+		  {
+			gfc_extract_int (op2, &power);
+			mpz_pow_ui (result->value.integer, op1->value.integer,
+power);
+			rc = gfc_range_check (result);
+			if (rc == ARITH_OVERFLOW)
+			  gfc_error_now ("Result of exponentiation at %L "
+	 "exceeds the range of %s", &op1->where,
+	 gfc_typename (&(op1->ts)));
+		  }
+		else
+		  {
+			/* Provide a nonsense value to propagate up. */
+			mpz_set (result->value.integer,
+ gfc_integer_kinds[k].huge);
+			mpz_add_ui (result->value.integer,
+result->value.integer, 1);
+			rc = ARITH_OVERFLOW;
+		  }
 		  }
-		else
-		  mpz_pow_ui (result->value.integer, op1->value.integer,
-			  power);
 	  }
 	  break;
 
Index: gcc/testsuite/gfortran.dg/integer_exponentiation_4.f90
===
--- gcc/testsuite/gfortran.dg/integer_exponentiation_4.f90	(revision 269260)
+++ gcc/testsuite/gfortran.dg/integer_exponentiation_4.f90	(working copy)
@@ -21,10 +21,10 @@ program test
   print *, (-1)**huge(0_8)
   print *, (-1)**(-huge(0_8)-1_8)
 
-  print *, 2**huge(0) ! { dg-error "Arithmetic overflow" }
-  print *, 2**huge(0_8) ! { dg-error "Arithmetic overflow" }
-  print *, (-2)**huge(0) ! { dg-error "Arithmetic overflow" }
-  print *, (-2)**huge(0_8) ! { dg-error "Arithmetic overflow" }
+  print *, 2**huge(0) ! { dg-error "Arithmetic overflow|exceeds the range" }
+  print *, 2**huge(0_8) ! { dg-error "Arithmetic overflow|exceeds the range" }
+  print *, (-2)**huge(0) ! { dg-error "Arithmetic overflow|exceeds the range" }
+  print *, (-2)**huge(0_8) ! { dg-error "Arithmetic overflow|exceeds the range" }
 
   print *, 2**(-huge(0)-1)
   print *, 2**(-huge(0_8)-1_8)
Index: gcc/testsuite/gfortran.dg/integer_exponentiation_5.F90
===
--- gcc/testsuite/gfortran.dg/integer_exponentiation_5.F90	(revision 269260)
+++ gcc/testsuite/gfortran.dg/integer_exponentiation_5.F90	(working copy)
@@ -67,8 +67,6 @@ program test
   TEST(3_8,43_8,i8)
   TEST(-3_8,43_8,i8)
 
-  TEST(17_8,int(huge(0_4),kind=8)+1,i8)
-
 ! REAL BASE !
   TEST(0.0,-1,r4)
   TEST(0.0,-huge(0)-1,r4)
Index: gcc/testsuite/gfortran.dg/no_range_check_1.f90
==

[PR fortran/77583, patch ]- ICE in pp_quoted_string, at pretty-print.c:966

[Harald Anlauf]

The attached patch (originally by Steve Kargl) fixes a NULL pointer
dereference that may occur when checking for a conflict.

Regtested successfully.

OK for trunk?  Backport to active branches?

Thanks,
Harald

2019-03-02  Harald Anlauf  
Steve Kargl  

PR fortran/77583
* symbol.c (check_conflict): Check for valid procedure name
passed to error reporting routine.

2019-03-02  Harald Anlauf  

PR fortran/77583
* gfortran.dg/pr77583.f90: New test.

Index: gcc/fortran/symbol.c
===
--- gcc/fortran/symbol.c(revision 269332)
+++ gcc/fortran/symbol.c(working copy)
@@ -525,7 +525,7 @@
   /* The copying of procedure dummy arguments for module procedures in
  a submodule occur whilst the current state is COMP_CONTAINS. It
  is necessary, therefore, to let this through.  */
-  if (attr->dummy
+  if (name && attr->dummy
   && (attr->function || attr->subroutine)
   && gfc_current_state () == COMP_CONTAINS
   && !(gfc_new_block && gfc_new_block->abr_modproc_decl))
Index: gcc/testsuite/gfortran.dg/pr77583.f90
===
--- gcc/testsuite/gfortran.dg/pr77583.f90   (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77583.f90   (working copy)
@@ -0,0 +1,10 @@
+! { dg-do compile }
+!
+! PR fortran/77583 - ICE in pp_quoted_string, at pretty-print.c:966
+! Contributed by Gerhard Steinmetz 
+
+pure subroutine sub(s)
+contains
+   pure subroutine s  ! { dg-error "conflicts with DUMMY argument" }
+   end
+end

Re: [PR fortran/77583, patch ]- ICE in pp_quoted_string, at pretty-print.c:966

[Steve Kargl]

On Sat, Mar 02, 2019 at 12:12:10AM +0100, Harald Anlauf wrote:
> The attached patch (originally by Steve Kargl) fixes a NULL pointer
> dereference that may occur when checking for a conflict.
> 
> Regtested successfully.
> 
> OK for trunk?  Backport to active branches?
> 
> 
> 2019-03-02  Harald Anlauf  
>   Steve Kargl  

Steven G. Kargl  

;-)

I, of course, approve of the patch, but you might give
others a chance to disagree.

-- 
Steve

libgo patch committed: Remove temporary directories in gccgoimporter test

[Ian Lance Taylor]

This libgo patches fixes the go/internal/gccgoimporter test to remove
temporary directories.  This is for PR 89406.  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 269333)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-a72eca1f435002076655fd6a54ce869ac39856dc
+decbbfb563ecf4609a3148dc789ae77ab1c62768
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/go/internal/gccgoimporter/importer_test.go
===
--- libgo/go/go/internal/gccgoimporter/importer_test.go (revision 269196)
+++ libgo/go/go/internal/gccgoimporter/importer_test.go (working copy)
@@ -143,17 +143,21 @@ func TestObjImporter(t *testing.T) {
}
t.Logf("gccgo version %d.%d", major, minor)
 
-   tmpdir, err := ioutil.TempDir("", "")
+   tmpdir, err := ioutil.TempDir("", "TestObjImporter")
if err != nil {
t.Fatal(err)
}
+   defer os.RemoveAll(tmpdir)
+
initmap := make(map[*types.Package]InitData)
imp := GetImporter([]string{tmpdir}, initmap)
 
-   artmpdir, err := ioutil.TempDir("", "")
+   artmpdir, err := ioutil.TempDir("", "TestObjImporter")
if err != nil {
t.Fatal(err)
}
+   defer os.RemoveAll(artmpdir)
+
arinitmap := make(map[*types.Package]InitData)
arimp := GetImporter([]string{artmpdir}, arinitmap)
 
@@ -198,8 +202,4 @@ func TestObjImporter(t *testing.T) {
t.Fatal(err)
}
}
-
-   if err = os.Remove(tmpdir); err != nil {
-   t.Fatal(err)
-   }
 }

Re: [PATCH v2 0/2] RISC-V: Support ELF attribute for GCC

[Jim Wilson]

On Tue, Feb 26, 2019 at 7:00 PM Kito Cheng  wrote:
> This version 2 of patch series for of RISC-V ELF attribute[1], first version 
> can be found in[2-4].
> Most changes between last version is typo fixing and coding sytle fix, and a 
> missing re-generated
> configure file.

This looks good.  I see you also fixed a few typos and coding style
issues I missed.  I found one place where you still ended a line with
an open paren, and a couple of places where the English needed a
little improvement.  I fixed those and checked it in, after testing to
verify my changes.

Jim

Re: [patch, fortran] Fix pointers not escaping via C_PTR

[Steve Kargl]

On Thu, Feb 28, 2019 at 09:14:48PM +0100, Thomas Koenig wrote:
> 
> the attached patch fixes a wrong-code bug for gfortran where pointers
> were not marked as escaping.  A C_PTR can be stashed away and reused
> later (at least as long as the variable it points to remains active).
> 
> This is not a regression, but IMHO a bad wrong-code bug. The chances
> of this patch introducing regressions are really, really low.
> 
> Regression-tested.  OK for trunk?
> 

Is this a wrong code bug or a clever user wandering off into
undefined behavior?

> subroutine init()
> use, intrinsic :: iso_c_binding
> implicit none
> integer(c_int), pointer :: a
> allocate(a)
> call save_cptr(c_loc(a))
> a = 100
> end subroutine init

Of particular note, 'a' is an unsaved local variable.  When init() 
returns, 'a' goes out-of-scope.  Fortran normally deallocates 
an unsaved allocatable entity, but 'a' is an unsaved local variable
with the pointer attribute.  For lack of a better term, 'allocate(a)'
allocates an anonymous target and associates the anonymous target
with the pointer 'a'.  save_ptr() caches the address of the anonymous
target.  When init() returns does the anonymous target get deallocated
or does the program leak memory?  In addition, when init() returns,
what happens to the pointer association of 'a'?  I think it becomes
undefined, which means the anonymous memory is inaccessible at least
by Fortran means as there are no pointers associated with the 
anonymous target.  The Fortran standard does not what happens to
leaked memory.  In particular, the Fortran standard does not
guarantee that leaked memory is not reaped by some garbage collection
or that it must retain value 100.

-- 
Steve