[PATCH] Remove UNSPEC_{COPYSIGN,XORSIGN}.

2021-09-12 Thread liuhongt via Gcc-patches
Hi:
  UNSPEC_COPYSIGN/XORSIGN are only used by related post_reload splitters
which have been removed by r12-3417 and r12-3435.

  Bootstrapped and regtest on x86_64-linux-gnu{-m32,}.
  Pushed to trunk.
 
gcc/ChangeLog:

* config/i386/i386.md: (UNSPEC_COPYSIGN): Remove.
(UNSPEC_XORSIGN): Ditto.
---
 gcc/config/i386/i386.md | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index c415487bb06..13f6f57cdcc 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -129,8 +129,6 @@ (define_c_enum "unspec" [
   UNSPEC_SCALEF
 
   ;; Generic math support
-  UNSPEC_COPYSIGN
-  UNSPEC_XORSIGN
   UNSPEC_IEEE_MIN  ; not commutative
   UNSPEC_IEEE_MAX  ; not commutative
 
-- 
2.27.0



Re: [PATCH] Remove dbx.h, do not set PREFERRED_DEBUGGING_TYPE from dbxcoff.h, lynx.h

2021-09-12 Thread Jan-Benedict Glaw
Hi Richard,

On Fri, 2021-09-10 08:02:00 +0200, Richard Biener via Gcc-patches 
 wrote:
> > On 9/9/2021 7:19 AM, Richard Biener via Gcc-patches wrote:
> > > The patch also removes the PREFERRED_DEBUGGING_TYPE define from
> > > lynx.h which always follows elfos.h already defaulting to DWARF,
> > > so the comment about STABS being the default is misleading and
> > > outdated.  There's no listed maintainer for Lynx OS.
> > >
> > > I have not tested this in any ways but I also have no idea how
> > > to meaningfully do so.

I'm not actually running such a configuration and cannot properly test
it, but automated mass-building broke for --target=i686-lynxos:

[all 2021-09-13 00:17:48] /usr/lib/gcc-snapshot/bin/g++ -c   -g -O2 -DIN_GCC  
-DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H  -DGENERATOR_FILE 
-fno-PIE -I. -Ibuild -I../../gcc/gcc -I../../gcc/gcc/build 
-I../../gcc/gcc/../include  -I../../gcc/gcc/../libcpp/include  \
[all 2021-09-13 00:17:48]  -o build/genconstants.o ../../gcc/gcc/genconstants.c
[all 2021-09-13 00:17:49] /usr/lib/gcc-snapshot/bin/g++   -g -O2 -DIN_GCC  
-DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H  -DGENERATOR_FILE 
-fno-PIE -static-libstdc++ -static-libgcc  -no-pie -o build/genconstants \
[all 2021-09-13 00:17:49] build/genconstants.o build/read-md.o 
build/errors.o ../build-x86_64-pc-linux-gnu/libiberty/libiberty.a
[all 2021-09-13 00:17:49] build/genconstants ../../gcc/gcc/common.md 
../../gcc/gcc/config/i386/i386.md \
[all 2021-09-13 00:17:49]> tmp-constants.h
[all 2021-09-13 00:17:49] /bin/bash ../../gcc/gcc/../move-if-change 
tmp-constants.h insn-constants.h
[all 2021-09-13 00:17:49] echo timestamp > s-constants
[all 2021-09-13 00:17:49] /usr/lib/gcc-snapshot/bin/g++ -c   -g -O2 -DIN_GCC  
-DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H  -DGENERATOR_FILE 
-fno-PIE -I. -Ibuild -I../../gcc/gcc -I../../gcc/gcc/build 
-I../../gcc/gcc/../include  -I../../gcc/gcc/../libcpp/include  \
[all 2021-09-13 00:17:49]  -o build/genpreds.o ../../gcc/gcc/genpreds.c
[all 2021-09-13 00:17:49] In file included from ./tm.h:37,
[all 2021-09-13 00:17:49]  from ../../gcc/gcc/genpreds.c:26:
[all 2021-09-13 00:17:49] ../../gcc/gcc/defaults.h:910:2: error: #error You 
must define PREFERRED_DEBUGGING_TYPE
[all 2021-09-13 00:17:49]   910 | #error You must define 
PREFERRED_DEBUGGING_TYPE
[all 2021-09-13 00:17:49]   |  ^
[all 2021-09-13 00:17:50] make[1]: *** [Makefile:2831: build/genpreds.o] Error 1
[all 2021-09-13 00:17:50] make[1]: Leaving directory 
'/var/lib/laminar/run/gcc-i686-lynxos/8/toolchain-build/gcc'
[all 2021-09-13 00:17:50] make: *** [Makefile:4423: all-gcc] Error 2

Thanks,
  Jan-Benedict

-- 


signature.asc
Description: PGP signature


Re: [PATCH] c++: parameter pack inside constexpr if [PR101764]

2021-09-12 Thread Jason Merrill via Gcc-patches

On 9/12/21 7:48 PM, Patrick Palka wrote:

On Thu, 2 Sep 2021, Jason Merrill wrote:


On 8/30/21 10:05 PM, Patrick Palka wrote:

Here when partially substituting into the pack expansion, substitution
into the constexpr if yields a still-dependent tree, so tsubst_expr
returns an IF_STMT with an unsubstituted IF_COND and with
IF_STMT_EXTRA_ARGS added to.  Hence after partial substitution
the pack expansion pattern still refers to the parameter pack 'ts' of
level 2 (and it's thus represented in the new
PACK_EXPANSION_PARAMETER_PACKS)
even though the partially instantiated generic lambda admits only one
level of template arguments.



This causes us to crash during the
subsequent instantiation with the lambda's template arguments because of
the level mismatch.  (Likewise when the constexpr if is replaced by a
requires-expr, which too uses the extra args mechanism for delaying
partial instantiation.)



So essentially, a pack expansion pattern that contains a parameter pack
inside an "extra args" tree doesn't play well with partial substitution
thereof.  This patch fixes this by forcing such pack expansions to use
the extra args mechanism as well.


Why is this specific to parameter packs?  Won't non-pack template parameters
also suffer from the level mismatch?


IIUC it's specific to parameter packs because each parameter pack in the
pattern is also recorded in PACK_EXPANSION_PARAMETER_PACKS, which
tsubst_pack_expansion looks at to extra all argument packs from 'args'
that are relevant to the pattern.

I should clarify it's during the loop over PACK_EXPANSION_PARAMETER_PACKS
that we crash, because we fail to find an argument pack for 'ts' (which
still has the unlowered level 2), and we trip over the assert:

{
  /* We can't substitute for this parameter pack.  We use a flag as
 well as the missing_level counter because function parameter
 packs don't have a level.  */
  gcc_assert (processing_template_decl || is_auto (parm_pack));
  unsubstituted_packs = true;
}

For non-pack template parameters (even those within extra args trees),
ordinary substitution is sufficient and does the right thing.


I'd think it would be simpler to just
note when the pattern contains a constexpr if or requires-expression, for
which we can't substitute into the pattern like a pack expansion, and know we
need to use extra args in that case.


Sounds good.  We'd force the extra args mechanism more than is strictly
necessary, but IIUC that should be harmless.


Agreed.


Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

PR c++/101764

gcc/cp/ChangeLog:

* cp-tree.h (PACK_EXPANSION_FORCE_EXTRA_ARGS_P): New accessor
macro.
* pt.c (uses_extra_args_mechanism_p): New function.
(find_parameter_pack_data::found_pack_within_extra_args_tree_p):
New data member.
(find_parameter_pack_data::inside_extra_args_tree_p): Likewise.
(find_parameter_packs_r): Detect parameter packs within "extra
args" trees and set found_pack_within_extra_args_tree_p
appropriately.
(make_pack_expansion): Set PACK_EXPANSION_FORCE_EXTRA_ARGS_P if
found_pack_within_extra_args_tree_p.
(use_pack_expansion_extra_args_p): Return true if there were
unsubstituted packs and PACK_EXPANSION_FORCE_EXTRA_ARGS_P.
(tsubst_pack_expansion): Pass the pack expansion to
use_pack_expansion_extra_args_p.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/constexpr-if35.C: New test.
---
   gcc/cp/cp-tree.h|  5 ++
   gcc/cp/pt.c | 69 -
   gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C | 18 ++
   3 files changed, 90 insertions(+), 2 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ce7ca53a113..06dec495428 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -493,6 +493,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
 CONSTRUCTOR_C99_COMPOUND_LITERAL (in CONSTRUCTOR)
 OVL_NESTED_P (in OVERLOAD)
 DECL_MODULE_EXPORT_P (in _DECL)
+  PACK_EXPANSION_FORCE_EXTRA_ARGS_P (in *_PACK_EXPANSION)
  4: IDENTIFIER_MARKED (IDENTIFIER_NODEs)
 TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
  CALL_EXPR, or FIELD_DECL).
@@ -3902,6 +3903,10 @@ struct GTY(()) lang_decl {
   /* True iff this pack expansion is for auto... in lambda init-capture.  */
   #define PACK_EXPANSION_AUTO_P(NODE) TREE_LANG_FLAG_2 (NODE)
   +/* True if we must use PACK_EXPANSION_EXTRA_ARGS to avoid partial
+   substitution into this pack expansion.  */
+#define PACK_EXPANSION_FORCE_EXTRA_ARGS_P(NODE) TREE_LANG_FLAG_3 (NODE)
+
   /* True iff the wildcard can match a template parameter pack.  */
   #define WILDCARD_PACK_P(NODE) TREE_LANG_FLAG_0 (NODE)
   diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

Re: [PATCH] tree-optimization/102155 - fix LIM fill_always_executed_in CFG walk

2021-09-12 Thread Xionghu Luo via Gcc-patches




On 2021/9/10 21:54, Xionghu Luo via Gcc-patches wrote:



On 2021/9/9 18:55, Richard Biener wrote:

diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 5d6845478e7..4b187c2cdaf 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -3074,15 +3074,13 @@ fill_always_executed_in_1 (class loop *loop, 
sbitmap contains_call)

  break;
    if (bb->loop_father->header == bb)
-    {
-  if (!dominated_by_p (CDI_DOMINATORS, loop->latch, bb))
-    break;
-
-  /* In a loop that is always entered we may proceed anyway.
- But record that we entered it and stop once we leave it
- since it might not be finite.  */
-  inn_loop = bb->loop_father;
-    }
+    /* Record that we enter into a subloop since it might not
+   be finite.  */
+    /* ???  Entering into a not always executed subloop makes
+   fill_always_executed_in quadratic in loop depth since
+   we walk those loops N times.  This is not a problem
+   in practice though, see PR102253 for a worst-case 
testcase.  */

+    inn_loop = bb->loop_father;



Yes your two patches extracted the get_loop_body_in_dom_order out and 
removed
the inn_loop break logic when it doesn't dominate outer loop.  Confirmed 
the replacement
could improve for saving ~10% build time due to not full DOM walker and 
marked the previously

ignored ALWAYS_EXECUTED bbs.
But if we don't break for inner loop again, why still keep the 
*inn_loop* variable?
It seems unnecessary and confusing, could we just remove it and restore 
the original

infinte loop check in bb->succs for better understanding?



What's more, the refine of this fix is incorrect for PR78185.


commit 483e400870601f650c80f867ec781cd5f83507d6
Author: Richard Biener 
Date:   Thu Sep 2 10:47:35 2021 +0200

Refine fix for PR78185, improve LIM for code after inner loops

This refines the fix for PR78185 after understanding that the code

regarding to the comment 'In a loop that is always entered we may
proceed anyway.  But record that we entered it and stop once we leave
it.' was supposed to protect us from leaving possibly infinite inner
loops.  The simpler fix of moving the misplaced stopping code
can then be refined to continue processing when the exited inner
loop is finite, improving invariant motion for cases like in the
added testcase.

2021-09-02  Richard Biener  

* tree-ssa-loop-im.c (fill_always_executed_in_1): Refine

fix for PR78185 and continue processing when leaving
finite inner loops.

* gcc.dg/tree-ssa/ssa-lim-16.c: New testcase.



3<---
||
6<---|
| \  |   |
|  \ |   |
48   |
|--- |
|  | |
5  7--
|
1

loop 2 is an infinite loop, it is only ALWAYS_EXECUTED for loop 2,
but r12-3313-g483e40087 sets it ALWAYS_EXECUTED for loop 1.
We need to restore it like this:
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579195.html


diff of pr78185.c.138t.lim2:
;;
 ;; Loop 1
 ;;  header 3, latch 7
 ;;  depth 1, outer 0
 ;;  nodes: 3 7 4 6 8
 ;;
 ;; Loop 2
 ;;  header 6, latch 8
 ;;  depth 2, outer 1
 ;;  nodes: 6 8
 ;; 2 succs { 3 }
 ;; 3 succs { 6 }
 ;; 6 succs { 4 8 }
 ;; 8 succs { 6 }
 ;; 4 succs { 7 5 }
 ;; 7 succs { 3 }
 ;; 5 succs { 1 }
 Memory reference 1: var1
-BB 6 is always executed in loop 1
 BB 3 is always executed in loop 1
+BB 6 is always executed in loop 2
 Basic block 3 (loop 1 -- depth 1):

 Basic block 6 (loop 2 -- depth 2):





diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index d1e2104233b..82a0509e0c4 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -3200,7 +3200,6 @@ fill_always_executed_in_1 (class loop *loop, 
sbitmap contains_call)

  {
    basic_block bb = NULL, last = NULL;
    edge e;
-  class loop *inn_loop = loop;

    if (ALWAYS_EXECUTED_IN (loop->header) == NULL)
  {
@@ -3213,17 +3212,6 @@ fill_always_executed_in_1 (class loop *loop, 
sbitmap contains_call)

   edge_iterator ei;
   bb = worklist.pop ();

- if (!flow_bb_inside_loop_p (inn_loop, bb))
-   {
- /* When we are leaving a possibly infinite inner loop
-    we have to stop processing.  */
- if (!finite_loop_p (inn_loop))
-   break;
- /* If the loop was finite we can continue with processing
-    the loop we exited to.  */
- inn_loop = bb->loop_father;
-   }
-
   if (dominated_by_p (CDI_DOMINATORS, loop->latch, bb))
     last = bb;

@@ -3232,8 +3220,15 @@ fill_always_executed_in_1 (class loop *loop, 
sbitmap contains_call)


  /* If LOOP exits from this BB stop processing.  */
   FOR_EACH_EDGE (e, ei, bb->succs)
+ {
     if (!flow_bb_inside_loop_p (loop, e->dest))
   break;
+   /* Or we enter a possibly non-finite loop.  */
+ 

Re: [PATCH] c++: parameter pack inside constexpr if [PR101764]

2021-09-12 Thread Patrick Palka via Gcc-patches
On Thu, 2 Sep 2021, Jason Merrill wrote:

> On 8/30/21 10:05 PM, Patrick Palka wrote:
> > Here when partially substituting into the pack expansion, substitution
> > into the constexpr if yields a still-dependent tree, so tsubst_expr
> > returns an IF_STMT with an unsubstituted IF_COND and with
> > IF_STMT_EXTRA_ARGS added to.  Hence after partial substitution
> > the pack expansion pattern still refers to the parameter pack 'ts' of
> > level 2 (and it's thus represented in the new
> > PACK_EXPANSION_PARAMETER_PACKS)
> > even though the partially instantiated generic lambda admits only one
> > level of template arguments.
> 
> > This causes us to crash during the
> > subsequent instantiation with the lambda's template arguments because of
> > the level mismatch.  (Likewise when the constexpr if is replaced by a
> > requires-expr, which too uses the extra args mechanism for delaying
> > partial instantiation.)
> 
> > So essentially, a pack expansion pattern that contains a parameter pack
> > inside an "extra args" tree doesn't play well with partial substitution
> > thereof.  This patch fixes this by forcing such pack expansions to use
> > the extra args mechanism as well.
> 
> Why is this specific to parameter packs?  Won't non-pack template parameters
> also suffer from the level mismatch?

IIUC it's specific to parameter packs because each parameter pack in the
pattern is also recorded in PACK_EXPANSION_PARAMETER_PACKS, which
tsubst_pack_expansion looks at to extra all argument packs from 'args'
that are relevant to the pattern.

I should clarify it's during the loop over PACK_EXPANSION_PARAMETER_PACKS
that we crash, because we fail to find an argument pack for 'ts' (which
still has the unlowered level 2), and we trip over the assert:

{
  /* We can't substitute for this parameter pack.  We use a flag as
 well as the missing_level counter because function parameter
 packs don't have a level.  */
  gcc_assert (processing_template_decl || is_auto (parm_pack));
  unsubstituted_packs = true;
}

For non-pack template parameters (even those within extra args trees),
ordinary substitution is sufficient and does the right thing.

> I'd think it would be simpler to just
> note when the pattern contains a constexpr if or requires-expression, for
> which we can't substitute into the pattern like a pack expansion, and know we
> need to use extra args in that case.

Sounds good.  We'd force the extra args mechanism more than is strictly
necessary, but IIUC that should be harmless.

> 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > PR c++/101764
> > 
> > gcc/cp/ChangeLog:
> > 
> > * cp-tree.h (PACK_EXPANSION_FORCE_EXTRA_ARGS_P): New accessor
> > macro.
> > * pt.c (uses_extra_args_mechanism_p): New function.
> > (find_parameter_pack_data::found_pack_within_extra_args_tree_p):
> > New data member.
> > (find_parameter_pack_data::inside_extra_args_tree_p): Likewise.
> > (find_parameter_packs_r): Detect parameter packs within "extra
> > args" trees and set found_pack_within_extra_args_tree_p
> > appropriately.
> > (make_pack_expansion): Set PACK_EXPANSION_FORCE_EXTRA_ARGS_P if
> > found_pack_within_extra_args_tree_p.
> > (use_pack_expansion_extra_args_p): Return true if there were
> > unsubstituted packs and PACK_EXPANSION_FORCE_EXTRA_ARGS_P.
> > (tsubst_pack_expansion): Pass the pack expansion to
> > use_pack_expansion_extra_args_p.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/cpp1z/constexpr-if35.C: New test.
> > ---
> >   gcc/cp/cp-tree.h|  5 ++
> >   gcc/cp/pt.c | 69 -
> >   gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C | 18 ++
> >   3 files changed, 90 insertions(+), 2 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
> > 
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index ce7ca53a113..06dec495428 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -493,6 +493,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
> > CONSTRUCTOR_C99_COMPOUND_LITERAL (in CONSTRUCTOR)
> > OVL_NESTED_P (in OVERLOAD)
> > DECL_MODULE_EXPORT_P (in _DECL)
> > +  PACK_EXPANSION_FORCE_EXTRA_ARGS_P (in *_PACK_EXPANSION)
> >  4: IDENTIFIER_MARKED (IDENTIFIER_NODEs)
> > TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
> >   CALL_EXPR, or FIELD_DECL).
> > @@ -3902,6 +3903,10 @@ struct GTY(()) lang_decl {
> >   /* True iff this pack expansion is for auto... in lambda init-capture.  */
> >   #define PACK_EXPANSION_AUTO_P(NODE) TREE_LANG_FLAG_2 (NODE)
> >   +/* True if we must use PACK_EXPANSION_EXTRA_ARGS to avoid partial
> > +   substitution into this pack expansion.  */
> > +#define PACK_EXPANSION_FORCE_EXTRA_ARGS_P(NODE) TREE_LANG_FLAG_3 (NODE)
> > +
> >   /* True 

[PATCH] c++: shortcut bad convs during overload resolution, part 2 [PR101904]

2021-09-12 Thread Patrick Palka via Gcc-patches
The r12-3346 patch makes us avoid computing excess argument conversions
during overload resolution, but only when it turns out there's a
strictly viable candidate in the overload set.  If there is no such
candidate then we still need to compute more conversions than strictly
necessary because subsequent conversions after the first bad conversion
can turn a non-strictly viable candidate into unviable one, and that
affects the outcome of overload resolution and the behavior of its
callers (in light of -fpermissive).

But at least in a SFINAE context, the distinction between a non-strictly
viable and an unviable candidate shouldn't matter all that much since
performing a bad conversion is always an error (even with -fpermissive),
and so forming a call to a non-strictly viable candidate will end up
being a SFINAE error anyway, just like in the unviable case.  Hence a
non-strictly viable candidate is effectively unviable (in a SFINAE
context), and we don't really need to distinguish between the two kinds.
We can take advantage of this observation to avoid computing excess
argument conversions even when there's no strictly viable candidate in
the overload set.

This patch implements this idea.  We usually detect a SFINAE context by
looking for the absence of the tf_error flag, but that's not specific
enough: we can also get here from build_user_type_conversion with
tf_error cleared, and there the distinction between a non-strictly
viable candidate and an unviable candidate still matters (it determines
whether a user-defined conversion is bad or just doesn't exist).  So this
patch sets and checks for the tf_conv flag to detect this situation too,
which avoids regressing conv2.C below.

Unlike the previous patch, this one does change the outcome of overload
resolution, but it should do so only in a way that preserves backwards
compatibility with -fpermissive.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

PR c++/101904

gcc/cp/ChangeLog:

* call.c (build_user_type_conversion_1): Add tf_conv to complain.
(add_candidates): When in a SFINAE context, instead of adding a
candidate to bad_fns just mark it unviable.

gcc/testsuite/ChangeLog:

* g++.dg/ext/conv2.C: New test.
* g++.dg/template/conv17.C: Augment test.
---
 gcc/cp/call.c  | 17 +++--
 gcc/testsuite/g++.dg/ext/conv2.C   | 13 +
 gcc/testsuite/g++.dg/template/conv17.C |  7 +++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/conv2.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b6011c1a282..ab0d118e34e 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4175,6 +4175,9 @@ build_user_type_conversion_1 (tree totype, tree expr, int 
flags,
   flags |= LOOKUP_NO_CONVERSION;
   if (BRACE_ENCLOSED_INITIALIZER_P (expr))
 flags |= LOOKUP_NO_NARROWING;
+  /* Prevent add_candidates from treating a non-strictly viable candidate
+ as an unviable one.  */
+  complain |= tf_conv;
 
   /* It's OK to bind a temporary for converting constructor arguments, but
  not in converting the return value of a conversion operator.  */
@@ -6232,8 +6235,18 @@ add_candidates (tree fns, tree first_arg, const 
vec *args,
 stopped at the first bad conversion).  Add the function to BAD_FNS
 to fully reconsider later if we don't find any strictly viable
 candidates.  */
- bad_fns = lookup_add (fn, bad_fns);
- *candidates = (*candidates)->next;
+ if (complain & (tf_error | tf_conv))
+   {
+ bad_fns = lookup_add (fn, bad_fns);
+ *candidates = (*candidates)->next;
+   }
+ else
+   /* But if we're in a SFINAE context, just mark this candidate as
+  unviable outright and avoid potentially reconsidering it.
+  This is safe to do since performing a bad conversion is always
+  erroneous in a SFINAE context (even with -fpermissive), so a
+  non-strictly viable candidate is effectively unviable anyway.  */
+   cand->viable = 0;
}
 }
   if (which == non_templates && !seen_perfect)
diff --git a/gcc/testsuite/g++.dg/ext/conv2.C b/gcc/testsuite/g++.dg/ext/conv2.C
new file mode 100644
index 000..baf2a43b2ae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/conv2.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fpermissive" }
+
+struct A {
+  A(int*, int);
+};
+
+void f(A);
+
+int main() {
+  const int n = 0;
+  f({, 42}); // { dg-warning "invalid conversion from 'const int\\*' to 
'int\\*'" }
+}
diff --git a/gcc/testsuite/g++.dg/template/conv17.C 
b/gcc/testsuite/g++.dg/template/conv17.C
index ba012c9d1fa..e40da8f1f24 100644
--- a/gcc/testsuite/g++.dg/template/conv17.C
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -53,4 +53,11 @@ concept D = requires (const T t) {
 };
 
 static_assert(D);
+

Re: [PATCH, rs6000] optimization for vec_reve builtin [PR100868]

2021-09-12 Thread Segher Boessenkool
Hi!

On Sun, Sep 12, 2021 at 10:50:17AM -0500, Bill Schmidt wrote:
> On 9/8/21 1:42 AM, HAO CHEN GUI wrote:
> >+;; Vector reverse elements for V2DI V2DF
> >+(define_expand "altivec_vreve2"
> >+  [(set (match_operand:VEC_64 0 "register_operand" "=v")
> >+   (unspec:VEC_64 [(match_operand:VEC_64 1 "register_operand" 
> >"v")]
> >+ UNSPEC_VREVEV))]
> >+  "TARGET_ALTIVEC"

(Your quoted text is mangled again)

> "TARGET_VSX" for V2DI and V2DF.  (This is the only good reason for
> splitting this into two patterns; you need different criteria.)

The *good* reason for splitting the pattern is they have completely
different expansions as well.  Which is why I asked for it.

(I'll review the patch later).


Segher


Re: [PATCH] rs6000: Disable optimizing multiple xxsetaccz instructions into one xxsetaccz

2021-09-12 Thread Segher Boessenkool
Hi!

On Fri, Aug 27, 2021 at 02:58:05PM -0500, Peter Bergner wrote:
> Fwprop will happily optimize two xxsetaccz instructions into one xxsetaccz
> by propagating the results of the first to the uses of the second.
> We really don't want that to happen given the late priming/depriming of
> accumulators.  I fixed this by making the xxsetaccz source operand an
> unspec volatile.

Good good.

> I also removed the mma_xxsetaccz define_expand and
> define_insn_and_split and replaced it with a simple define_insn.

In the future pleaase do that in a separate patch.  That makes it *much*
easier to read and review this.

> GCC10 suffers from the same issue, but since the code is different, I'll
> have to determine a different solution which I'll post as a separate
> patch.

It doesn't currently have an unspec at all, so you cannot give it a
side effect.  It shouldn't be hard to make it use an unspec, just a lot
of mechanics (and testing :-/ )

>   * config/rs6000/mma.md (unspec): Delete UNSPEC_MMA_XXSETACCZ.
>   (unspecv): Add UNSPECV_MMA_XXSETACCZ.

Unrelated to this patch, but I have been wondering this for years:
should we have an unspecv enum at all?  It causes some churn, and you
can name the volatile ones UNSPECV_ in either case.

>   (mma_xxsetaccz): Change to define_insn.  Remove match_operand.
>   Use UNSPECV_MMA_XXSETACCZ.

It still has the match_operand.

>  ;; We can't have integer constants in XOmode so we wrap this in an UNSPEC.

Does the comment need updating?  It may help to point out here that itr
 needs to be volatile.

> (set_attr "length" "4")])

Not new of course: the default length is 4, most insns have that, it
helps to be less verbose.

Okay for trunk with that changelog fix.  Thanks!


Segher


[PATCH] PR fortran/85130 - Handling of substring range

2021-09-12 Thread Harald Anlauf via Gcc-patches
Dear all,

in find_substring_ref we erroneously handled given substring start and end
indices as unsigned integers.  However, gives indices could be negative,
which is legal as long as end < start, leading to a string of length zero.
The current behavior could lead to a wrong length as well as an invalid read
from (compiler) memory.

The fix allows to reintroduce code in testcase substr_6.f90 that was
erroneously considered as illegal.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

As this is invalid code, I'd like to backport this fix.

Thanks,
Harald


Fortran - fix handling of substring start and end indices

gcc/fortran/ChangeLog:

PR fortran/85130
* expr.c (find_substring_ref): Handle given substring start and
end indices as signed integers, not unsigned.

gcc/testsuite/ChangeLog:

PR fortran/85130
* gfortran.dg/substr_6.f90: Revert commit r8-7574, adding again
test that was erroneously considered as illegal.

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index dfecc3012e1..604e63e6164 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -1724,8 +1724,8 @@ find_substring_ref (gfc_expr *p, gfc_expr **newp)
   *newp = gfc_copy_expr (p);
   free ((*newp)->value.character.string);

-  end = (gfc_charlen_t) mpz_get_ui (p->ref->u.ss.end->value.integer);
-  start = (gfc_charlen_t) mpz_get_ui (p->ref->u.ss.start->value.integer);
+  end = (gfc_charlen_t) mpz_get_si (p->ref->u.ss.end->value.integer);
+  start = (gfc_charlen_t) mpz_get_si (p->ref->u.ss.start->value.integer);
   if (end >= start)
 length = end - start + 1;
   else
diff --git a/gcc/testsuite/gfortran.dg/substr_6.f90 b/gcc/testsuite/gfortran.dg/substr_6.f90
index 0d5e3d75e88..83e788a55a6 100644
--- a/gcc/testsuite/gfortran.dg/substr_6.f90
+++ b/gcc/testsuite/gfortran.dg/substr_6.f90
@@ -6,6 +6,8 @@ CHARACTER(5), parameter :: c0(1) = (/ "123" // ACHAR(0) // "5" /)
 CHARACTER*5 c(1)
 CHARACTER(1), parameter :: c1(5) = (/ "1", "2", "3", ACHAR(0), "5" /)

+c = c0(1)(-5:-8)
+if (c(1) /= " ") STOP 1
 c = (/ c0(1)(1:5) /)
 do i=1,5
if (c(1)(i:i) /= c1(i)) STOP 2


*PING* [PATCH] PR fortran/82314 - ICE in gfc_conv_expr_descriptor, at fortran/trans-array.c:6972

2021-09-12 Thread Harald Anlauf via Gcc-patches
Early *PING*.

> Gesendet: Dienstag, 07. September 2021 um 23:44 Uhr
> Von: "Harald Anlauf" 
> An: "fortran" , "gcc-patches" 
> Betreff: [PATCH] PR fortran/82314 - ICE in gfc_conv_expr_descriptor, at 
> fortran/trans-array.c:6972
>
> When adding the initializer for an array, we need to make sure that
> array bounds are properly simplified if that array is a PARAMETER.
> Otherwise the generated initializer could be wrong and screw up
> subsequent simplifications, see PR.
>
> The minimal solution is to attempt simplification of array bounds
> before adding the initializer as in the attached patch.  (We could
> place that part in a helper function if this functionality is
> considered useful elsewhere).
>
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>
> Thanks,
> Harald
>
>
> Fortran - ensure simplification of bounds of array-valued named constants
>
> gcc/fortran/ChangeLog:
>
>   PR fortran/82314
>   * decl.c (add_init_expr_to_sym): For proper initialization of
>   array-valued named constants the array bounds need to be
>   simplified before adding the initializer.
>
> gcc/testsuite/ChangeLog:
>
>   PR fortran/82314
>   * gfortran.dg/pr82314.f90: New test.
>
>


[PATCH] Fix multi-statement define for alpha-dec-vms

2021-09-12 Thread Jan-Benedict Glaw
Hi!

While mass-building a cross-gcc, I noticed that for
alpha-dec-vms/alpha64-dec-vms, recent GCC versions correctly throw a warning
due to a multi-statement define that gets rippen in an if/else case:

[all 2021-09-12 15:51:55] /usr/lib/gcc-snapshot/bin/g++  -fno-PIE -c   -g -O2 
-DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. 
-I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include 
-I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  
-I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o value-prof.o -MT 
value-prof.o -MMD -MP -MF ./.deps/value-prof.TPo ../../gcc/gcc/value-prof.c
[all 2021-09-12 15:52:01] /usr/lib/gcc-snapshot/bin/g++  -fno-PIE -c   -g -O2 
-DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. 
-I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include 
-I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  
-I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o var-tracking.o -MT 
var-tracking.o -MMD -MP -MF ./.deps/var-tracking.TPo 
../../gcc/gcc/var-tracking.c
[all 2021-09-12 15:52:03] In file included from ./tm.h:21,
[all 2021-09-12 15:52:03]  from ../../gcc/gcc/backend.h:28,
[all 2021-09-12 15:52:03]  from ../../gcc/gcc/var-tracking.c:91:
[all 2021-09-12 15:52:03] ../../gcc/gcc/var-tracking.c: In function 'void 
prepare_call_arguments(basic_block, rtx_insn*)':
[all 2021-09-12 15:52:03] ../../gcc/gcc/config/alpha/vms.h:148:3: error: macro 
expands to multiple statements [-Werror=multistatement-macros]
[all 2021-09-12 15:52:03]   148 |   (CUM).num_args = 0; 
  \
[all 2021-09-12 15:52:03]   |   ^
[all 2021-09-12 15:52:03] ../../gcc/gcc/var-tracking.c:6334:17: note: in 
expansion of macro 'INIT_CUMULATIVE_ARGS'
[all 2021-09-12 15:52:03]  6334 | INIT_CUMULATIVE_ARGS 
(args_so_far_v, type, NULL_RTX, fndecl,
[all 2021-09-12 15:52:03]   | ^~~~
[all 2021-09-12 15:52:03] ../../gcc/gcc/var-tracking.c:6332:15: note: some 
parts of macro expansion are not guarded by this 'else' clause
[all 2021-09-12 15:52:03]  6332 |   else
[all 2021-09-12 15:52:03]   |   ^~~~
[all 2021-09-12 15:52:20] cc1plus: all warnings being treated as errors
[all 2021-09-12 15:52:20] make[1]: *** [Makefile:1143: var-tracking.o] Error 1
[all 2021-09-12 15:52:20] make[1]: Leaving directory 
'/var/lib/laminar/run/gcc-alpha64-dec-vms/8/toolchain-build/gcc'
[all 2021-09-12 15:52:20] make: *** [Makefile:4425: all-gcc] Error 2




gcc/ChangeLog:

* config/alpha/vms.h (INIT_CUMULATIVE_ARGS): Wrap multi-statment
define into a block.


diff --git a/gcc/config/alpha/vms.h b/gcc/config/alpha/vms.h
index b8673b6b6fb..e979aef10c7 100644
--- a/gcc/config/alpha/vms.h
+++ b/gcc/config/alpha/vms.h
@@ -145,9 +145,13 @@ typedef struct {int num_args; enum avms_arg_type 
atypes[6];} avms_arg_info;
 
 #undef INIT_CUMULATIVE_ARGS
 #define INIT_CUMULATIVE_ARGS(CUM, FNTYPE, LIBNAME, INDIRECT, N_NAMED_ARGS) \
-  (CUM).num_args = 0;  \
-  (CUM).atypes[0] = (CUM).atypes[1] = (CUM).atypes[2] = I64;   \
-  (CUM).atypes[3] = (CUM).atypes[4] = (CUM).atypes[5] = I64;
+  do   \
+{  \
+  (CUM).num_args = 0;  \
+  (CUM).atypes[0] = (CUM).atypes[1] = (CUM).atypes[2] = I64;   \
+  (CUM).atypes[3] = (CUM).atypes[4] = (CUM).atypes[5] = I64;   \
+}  \
+  while (0)
 
 #define DEFAULT_PCC_STRUCT_RETURN 0
 



Okay for trunk?

Thanks,
  Jan-Benedict

-- 


signature.asc
Description: PGP signature


Re: [PATCH v2] ipa-inline: Add target info into fn summary [PR102059]

2021-09-12 Thread Bill Schmidt via Gcc-patches

Hi Kewen,

I'll leave the continued review of the back-end parts of this to Segher, 
but I do have one long-term comment.  The rs6000_builtin_info[code].mask 
field that you're relying on is going away as part of the built-in 
function rewrite.  There will be a "bifattrs" field that replaces this 
in the table-driven data structures.  I'm including those below.  Please 
think about whether what you're building will easily translate to using 
these data structures in the near future.  I don't think there are any 
show-stoppers here, but if the new table needs adjustment to make your 
changes easier, let's discuss.


Thanks,
Bill

#define PPC_MAXRESTROPNDS 3
struct GTY((user)) bifdata
{
  const char *bifname;
  bif_enable enable;
  tree fntype;
  insn_code icode;
  int  nargs;
  int  bifattrs;
  int  restr_opnd[PPC_MAXRESTROPNDS];
  restriction restr[PPC_MAXRESTROPNDS];
  int  restr_val1[PPC_MAXRESTROPNDS];
  int  restr_val2[PPC_MAXRESTROPNDS];
  const char *attr_string;
  rs6000_gen_builtins assoc_bif;
};

#define bif_init_bit(0x0001)
#define bif_set_bit (0x0002)
#define bif_extract_bit (0x0004)
#define bif_nosoft_bit  (0x0008)
#define bif_ldvec_bit   (0x0010)
#define bif_stvec_bit   (0x0020)
#define bif_reve_bit(0x0040)
#define bif_pred_bit(0x0080)
#define bif_htm_bit (0x0100)
#define bif_htmspr_bit  (0x0200)
#define bif_htmcr_bit   (0x0400)
#define bif_mma_bit (0x0800)
#define bif_quad_bit(0x1000)
#define bif_pair_bit(0x2000)
#define bif_mmaint_bit  (0x4000)
#define bif_no32bit_bit (0x8000)
#define bif_32bit_bit   (0x0001)
#define bif_cpu_bit (0x0002)
#define bif_ldstmask_bit(0x0004)
#define bif_lxvrse_bit  (0x0008)
#define bif_lxvrze_bit  (0x0010)
#define bif_endian_bit  (0x0020)

#define bif_is_init(x)  ((x).bifattrs & bif_init_bit)
#define bif_is_set(x)   ((x).bifattrs & bif_set_bit)
#define bif_is_extract(x)   ((x).bifattrs & bif_extract_bit)
#define bif_is_nosoft(x)((x).bifattrs & bif_nosoft_bit)
#define bif_is_ldvec(x) ((x).bifattrs & bif_ldvec_bit)
#define bif_is_stvec(x) ((x).bifattrs & bif_stvec_bit)
#define bif_is_reve(x)  ((x).bifattrs & bif_reve_bit)
#define bif_is_predicate(x) ((x).bifattrs & bif_pred_bit)
#define bif_is_htm(x)   ((x).bifattrs & bif_htm_bit)
#define bif_is_htmspr(x)((x).bifattrs & bif_htmspr_bit)
#define bif_is_htmcr(x) ((x).bifattrs & bif_htmcr_bit)
#define bif_is_mma(x)   ((x).bifattrs & bif_mma_bit)
#define bif_is_quad(x)  ((x).bifattrs & bif_quad_bit)
#define bif_is_pair(x)  ((x).bifattrs & bif_pair_bit)
#define bif_is_mmaint(x)((x).bifattrs & bif_mmaint_bit)
#define bif_is_no32bit(x)   ((x).bifattrs & bif_no32bit_bit)
#define bif_is_32bit(x) ((x).bifattrs & bif_32bit_bit)
#define bif_is_cpu(x)   ((x).bifattrs & bif_cpu_bit)
#define bif_is_ldstmask(x)  ((x).bifattrs & bif_ldstmask_bit)
#define bif_is_lxvrse(x)((x).bifattrs & bif_lxvrse_bit)
#define bif_is_lxvrze(x)((x).bifattrs & bif_lxvrze_bit)
#define bif_is_endian(x)((x).bifattrs & bif_endian_bit)

extern bifdata rs6000_builtin_info_x[RS6000_BIF_MAX];

On 9/8/21 2:43 AM, Kewen.Lin wrote:

Hi!

Power ISA 2.07 (Power8) introduces transactional memory feature
but ISA3.1 (Power10) removes it.  It exposes one troublesome
issue as PR102059 shows.  Users define some function with
target pragma cpu=power10 then it calls one function with
attribute always_inline which inherits command line option
cpu=power8 which enables HTM implicitly.  The current isa_flags
check doesn't allow this inlining due to "target specific
option mismatch" and error mesasge is emitted.

Normally, the callee function isn't intended to exploit HTM
feature, but the default flag setting make it look it has.
As Richi raised in the PR, we have fp_expressions flag in
function summary, and allow us to check the function actually
contains any floating point expressions to avoid overkill.
So this patch follows the similar idea but is more target
specific, for this rs6000 port specific requirement on HTM
feature check, we would like to check rs6000 specific HTM
built-in functions and inline assembly, it allows targets
to do their own customized checks and updates.

It introduces two target hooks need_ipa_fn_target_info and
update_ipa_fn_target_info.  The former allows target to do
some previous check and decides to collect target specific
information for this function or not.  For some special case,
it can predict the analysis result and push it early without
any scannings.  The latter allows the analyze_function_body
to pass gimple stmts down just like fp_expressions handlings,
target can do its own tricks.  

[PATCH] Fix PR lto/49664: liblto_plugin.so exports too many symbols

2021-09-12 Thread apinski--- via Gcc-patches
From: Andrew Pinski 

So right now liblto_plugin.so exports many libiberty symbols and
simple_object file symbols but really it just needs to export onload.

This fixes the problem by using "-export-symbols-regex onload" on
the libtool link line.

lto-plugin/ChangeLog:

* Makefile.am: Export only onload.
* Makefile.in: Regenerate.
---
 lto-plugin/Makefile.am | 3 ++-
 lto-plugin/Makefile.in | 7 ---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am
index 8b20e1d1d87..988d7a78294 100644
--- a/lto-plugin/Makefile.am
+++ b/lto-plugin/Makefile.am
@@ -21,7 +21,8 @@ in_gcc_libs = $(foreach lib, $(libexecsub_LTLIBRARIES), 
$(gcc_build_dir)/$(lib))
 liblto_plugin_la_SOURCES = lto-plugin.c
 # Note that we intentionally override the bindir supplied by ACX_LT_HOST_FLAGS.
 liblto_plugin_la_LDFLAGS = $(AM_LDFLAGS) \
-   $(lt_host_flags) -module -avoid-version -bindir $(libexecsubdir)
+   $(lt_host_flags) -module -avoid-version -bindir $(libexecsubdir) \
+   -export-symbols-regex onload
 # Can be simplified when libiberty becomes a normal convenience library.
 libiberty = $(with_libiberty)/libiberty.a
 libiberty_noasan = $(with_libiberty)/noasan/libiberty.a
diff --git a/lto-plugin/Makefile.in b/lto-plugin/Makefile.in
index 20611c6b1e6..f8df31bb1e8 100644
--- a/lto-plugin/Makefile.in
+++ b/lto-plugin/Makefile.in
@@ -323,6 +323,7 @@ prefix = @prefix@
 program_transform_name = @program_transform_name@
 psdir = @psdir@
 real_target_noncanonical = @real_target_noncanonical@
+runstatedir = @runstatedir@
 sbindir = @sbindir@
 sharedstatedir = @sharedstatedir@
 srcdir = @srcdir@
@@ -350,9 +351,9 @@ libexecsub_LTLIBRARIES = liblto_plugin.la
 in_gcc_libs = $(foreach lib, $(libexecsub_LTLIBRARIES), 
$(gcc_build_dir)/$(lib))
 liblto_plugin_la_SOURCES = lto-plugin.c
 # Note that we intentionally override the bindir supplied by ACX_LT_HOST_FLAGS.
-liblto_plugin_la_LDFLAGS = $(AM_LDFLAGS) $(lt_host_flags) -module 
-avoid-version \
-   -bindir $(libexecsubdir) $(if $(wildcard \
-   $(libiberty_noasan)),, $(if $(wildcard \
+liblto_plugin_la_LDFLAGS = $(AM_LDFLAGS) $(lt_host_flags) -module \
+   -avoid-version -bindir $(libexecsubdir) -export-symbols-regex \
+   onload $(if $(wildcard $(libiberty_noasan)),, $(if $(wildcard \
$(libiberty_pic)),,-Wc,$(libiberty)))
 # Can be simplified when libiberty becomes a normal convenience library.
 libiberty = $(with_libiberty)/libiberty.a
-- 
2.17.1



Re: [PATCH, rs6000] optimization for vec_reve builtin [PR100868]

2021-09-12 Thread Bill Schmidt via Gcc-patches

Hi Haochen,

On 9/8/21 1:42 AM, HAO CHEN GUI wrote:

Hi,

    The patch optimized for vec_reve builtin on rs6000. For V2DI and
V2DF, it is implemented by xxswapd on all targets. For V16QI, V8HI, V4SI
and V4SF, it is implemented by quadword byte reverse plus halfword/word
byte reverse when p9_vector is defined.

    Bootstrapped and tested on powerpc64le-linux with no regressions. Is
this okay for trunk? Any recommendations? Thanks a lot.


ChangeLog

2021-09-08 Haochen Gui 

gcc/
      * config/rs6000/altivec.md (altivec_vreve2 for VEC_K):
      Use xxbrq for v16qi, xxbrq + xxbrh for v8hi and xxbrq + xxbrw
      for v4si or v4sf when p9_vector is defined.
      (altivec_vreve2 for VEC_64): Defined. Implemented by
      xxswapd.

gcc/testsuite/
      * gcc.target/powerpc/vec_reve_1.c: New test.
      * gcc.target/powerpc/vec_reve_2.c: Likewise.


patch.diff

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 1351dafbc41..a1698ce85c0 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -4049,13 +4049,43 @@ (define_expand "altivec_negv4sf2"
     DONE;
   })

-;; Vector reverse elements
+;; Vector reverse elements for V16QI V8HI V4SI V4SF
   (define_expand "altivec_vreve2"
-  [(set (match_operand:VEC_A 0 "register_operand" "=v")
-   (unspec:VEC_A [(match_operand:VEC_A 1 "register_operand" "v")]
+  [(set (match_operand:VEC_K 0 "register_operand" "=v")
+   (unspec:VEC_K [(match_operand:VEC_K 1 "register_operand" "v")]
    UNSPEC_VREVEV))]
     "TARGET_ALTIVEC"
   {
+  if (TARGET_P9_VECTOR)
+    {
+  if (mode == V16QImode)
+   emit_insn (gen_p9_xxbrq_v16qi (operands[0], operands[1]));
+  else if (mode == V8HImode)
+   {
+ rtx subreg1 = simplify_gen_subreg (V1TImode, operands[1],
+    mode, 0);
+ rtx temp = gen_reg_rtx (V1TImode);
+ emit_insn (gen_p9_xxbrq_v1ti (temp, subreg1));
+ rtx subreg2 = simplify_gen_subreg (mode, temp,
+    V1TImode, 0);
+ emit_insn (gen_p9_xxbrh_v8hi (operands[0], subreg2));
+   }
+  else /* V4SI and V4SF.  */
+   {
+ rtx subreg1 = simplify_gen_subreg (V1TImode, operands[1],
+    mode, 0);
+ rtx temp = gen_reg_rtx (V1TImode);
+ emit_insn (gen_p9_xxbrq_v1ti (temp, subreg1));
+ rtx subreg2 = simplify_gen_subreg (mode, temp,
+    V1TImode, 0);
+ if (mode == V4SImode)
+   emit_insn (gen_p9_xxbrw_v4si (operands[0], subreg2));
+ else
+   emit_insn (gen_p9_xxbrw_v4sf (operands[0], subreg2));
+   }
+  DONE;
+    }
+
     int i, j, size, num_elements;
     rtvec v = rtvec_alloc (16);
     rtx mask = gen_reg_rtx (V16QImode);
@@ -4074,6 +4104,17 @@ (define_expand "altivec_vreve2"
     DONE;
   })

+;; Vector reverse elements for V2DI V2DF
+(define_expand "altivec_vreve2"
+  [(set (match_operand:VEC_64 0 "register_operand" "=v")
+   (unspec:VEC_64 [(match_operand:VEC_64 1 "register_operand" "v")]
+ UNSPEC_VREVEV))]
+  "TARGET_ALTIVEC"



"TARGET_VSX" for V2DI and V2DF.  (This is the only good reason for
splitting this into two patterns; you need different criteria.)


+{
+  emit_insn (gen_xxswapd_ (operands[0], operands[1]));
+  DONE;
+})
+
   ;; Vector SIMD PEM v2.06c defines LVLX, LVLXL, LVRX, LVRXL,
   ;; STVLX, STVLXL, STVVRX, STVRXL are available only on Cell.
   (define_insn "altivec_lvlx"
diff --git a/gcc/testsuite/gcc.target/powerpc/vec_reve_1.c
b/gcc/testsuite/gcc.target/powerpc/vec_reve_1.c
new file mode 100644
index 000..83a9206758b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec_reve_1.c
@@ -0,0 +1,16 @@
+/* { dg-require-effective-target powerpc_altivec_ok } */



powerpc_vsx_ok to handle vector double and vector long long.


+/* { dg-options "-O2 -maltivec" } */


-mvsx

Looks okay to me with those things fixed.  Maintainers?

Thanks for the patch!
Bill


+
+#include 
+
+vector double foo1 (vector double a)
+{
+   return vec_reve (a);
+}
+
+vector long long foo2 (vector long long a)
+{
+   return vec_reve (a);
+}
+
+/* { dg-final { scan-assembler-times {\mxxpermdi\M} 2 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/vec_reve_2.c
b/gcc/testsuite/gcc.target/powerpc/vec_reve_2.c
new file mode 100644
index 000..b6dd33d6d79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec_reve_2.c
@@ -0,0 +1,28 @@
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-mdejagnu-cpu=power9 -O2 -maltivec" } */
+
+#include 
+
+vector int foo1 (vector int a)
+{
+   return vec_reve (a);
+}
+
+vector float foo2 (vector float a)
+{
+   return vec_reve (a);
+}
+
+vector short foo3 (vector short a)
+{
+   return vec_reve (a);
+}
+
+vector char foo4 (vector char a)
+{
+   return vec_reve (a);
+}
+
+/* { dg-final { 

[committed] d: Don't include terminating null pointer in string expression conversion (PR102185)

2021-09-12 Thread Iain Buclaw via Gcc-patches
Hi,

This patch fixes an issue with the routine that converts STRING_CST to a
StringExp for the dmd front-end to use during the semantic pass.

The null terminator gets re-added by the ExprVisitor when lowering
StringExp back into a STRING_CST during the code generator pass.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32/-mx32,
committed to mainline and backported to the releases/gcc-11 branch.

Regards,
Iain.

---
gcc/d/ChangeLog:

PR d/102185
* d-builtins.cc (d_eval_constant_expression): Don't include
terminating null pointer in string expression conversion.

gcc/testsuite/ChangeLog:

PR d/102185
* gdc.dg/pr102185.d: New test.
---
 gcc/d/d-builtins.cc | 2 +-
 gcc/testsuite/gdc.dg/pr102185.d | 7 +++
 2 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gdc.dg/pr102185.d

diff --git a/gcc/d/d-builtins.cc b/gcc/d/d-builtins.cc
index ab39d69c294..33347a14c67 100644
--- a/gcc/d/d-builtins.cc
+++ b/gcc/d/d-builtins.cc
@@ -380,7 +380,7 @@ d_eval_constant_expression (const Loc , tree cst)
   else if (code == STRING_CST)
{
  const void *string = TREE_STRING_POINTER (cst);
- size_t len = TREE_STRING_LENGTH (cst);
+ size_t len = TREE_STRING_LENGTH (cst) - 1;
  return StringExp::create (loc, CONST_CAST (void *, string), len);
}
   else if (code == VECTOR_CST)
diff --git a/gcc/testsuite/gdc.dg/pr102185.d b/gcc/testsuite/gdc.dg/pr102185.d
new file mode 100644
index 000..39823a3c556
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr102185.d
@@ -0,0 +1,7 @@
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102185
+// { dg-do compile }
+
+static assert(__traits(getTargetInfo, "floatAbi").length == 0 ||
+  __traits(getTargetInfo, "floatAbi") == "hard" ||
+  __traits(getTargetInfo, "floatAbi") == "soft" ||
+  __traits(getTargetInfo, "floatAbi") == "softfp");
-- 
2.30.2



Re: [PATCH] rs6000: Disable optimizing multiple xxsetaccz instructions into one xxsetaccz

2021-09-12 Thread Bill Schmidt via Gcc-patches

Hi Peter,

This patch looks fine to me.  The approach to avoiding incorrect 
optimization is reasonable.  Maintainers?


Thanks for the patch!
Bill

On 8/27/21 2:58 PM, Peter Bergner via Gcc-patches wrote:

Fwprop will happily optimize two xxsetaccz instructions into one xxsetaccz
by propagating the results of the first to the uses of the second.
We really don't want that to happen given the late priming/depriming of
accumulators.  I fixed this by making the xxsetaccz source operand an
unspec volatile.  I also removed the mma_xxsetaccz define_expand and
define_insn_and_split and replaced it with a simple define_insn.
The expand and splitter patterns were leftovers from the pre opaque mode
code when the xxsetaccz code was part of the movpxi pattern, and we don't
need them now.

Rather than a new test case, I was able to just modify the current test case
to add another __builtin_mma_xxsetaccz call which shows the bad code gen
with unpatched compilers.

This passed bootstrap on powerpc64le-linux with no regressions.
Ok for trunk?  We'll need this for sure in GCC11.  Ok there too after
some trunk burn in time?

GCC10 suffers from the same issue, but since the code is different, I'll
have to determine a different solution which I'll post as a separate
patch.

Peter


gcc/
* config/rs6000/mma.md (unspec): Delete UNSPEC_MMA_XXSETACCZ.
(unspecv): Add UNSPECV_MMA_XXSETACCZ.
(*mma_xxsetaccz): Delete.
(mma_xxsetaccz): Change to define_insn.  Remove match_operand.
Use UNSPECV_MMA_XXSETACCZ.
* config/rs6000/rs6000.c (rs6000_rtx_costs): Use UNSPECV_MMA_XXSETACCZ.

gcc/testsuite/
* gcc.target/powerpc/mma-builtin-6.c: Add second call to xxsetacc
built-in.  Update instruction counts.

diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
index 1f6fc03d2ac..b26ae7a5d04 100644
--- a/gcc/config/rs6000/mma.md
+++ b/gcc/config/rs6000/mma.md
@@ -91,7 +91,10 @@ (define_c_enum "unspec"
 UNSPEC_MMA_XVI8GER4SPP
 UNSPEC_MMA_XXMFACC
 UNSPEC_MMA_XXMTACC
-   UNSPEC_MMA_XXSETACCZ
+  ])
+
+(define_c_enum "unspecv"
+  [UNSPECV_MMA_XXSETACCZ
])

  ;; MMA instructions with 1 accumulator argument
@@ -469,26 +472,12 @@ (define_insn "mma_"

  ;; We can't have integer constants in XOmode so we wrap this in an UNSPEC.

-(define_expand "mma_xxsetaccz"
-  [(set (match_operand:XO 0 "fpr_reg_operand")
-   (const_int 0))]
-  "TARGET_MMA"
-{
-  rtx xo0 = gen_rtx_UNSPEC (XOmode, gen_rtvec (1, const0_rtx),
-   UNSPEC_MMA_XXSETACCZ);
-  emit_insn (gen_rtx_SET (operands[0], xo0));
-  DONE;
-})
-
-(define_insn_and_split "*mma_xxsetaccz"
+(define_insn "mma_xxsetaccz"
[(set (match_operand:XO 0 "fpr_reg_operand" "=d")
-   (unspec:XO [(match_operand 1 "const_0_to_1_operand" "O")]
-UNSPEC_MMA_XXSETACCZ))]
+   (unspec_volatile:XO [(const_int 0)]
+   UNSPECV_MMA_XXSETACCZ))]
"TARGET_MMA"
"xxsetaccz %A0"
-  "&& reload_completed"
-  [(set (match_dup 0) (unspec:XO [(match_dup 1)] UNSPEC_MMA_XXSETACCZ))]
-  ""
[(set_attr "type" "mma")
 (set_attr "length" "4")])

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e073b26b430..40dc71c8171 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21919,7 +21919,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int 
outer_code,
break;

  case UNSPEC:
-  if (XINT (x, 1) == UNSPEC_MMA_XXSETACCZ)
+  if (XINT (x, 1) == UNSPECV_MMA_XXSETACCZ)
{
  *total = 0;
  return true;
diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-6.c 
b/gcc/testsuite/gcc.target/powerpc/mma-builtin-6.c
index 0c6517211e3..715b28138e9 100644
--- a/gcc/testsuite/gcc.target/powerpc/mma-builtin-6.c
+++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-6.c
@@ -5,14 +5,16 @@
  void
  foo (__vector_quad *dst)
  {
-  __vector_quad acc;
-  __builtin_mma_xxsetaccz ();
-  *dst = acc;
+  __vector_quad acc0, acc1;
+  __builtin_mma_xxsetaccz ();
+  __builtin_mma_xxsetaccz ();
+  dst[0] = acc0;
+  dst[1] = acc1;
  }

  /* { dg-final { scan-assembler-not {\mlxv\M} } } */
  /* { dg-final { scan-assembler-not {\mlxvp\M} } } */
  /* { dg-final { scan-assembler-not {\mxxmtacc\M} } } */
-/* { dg-final { scan-assembler-times {\mxxsetaccz\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mxxmfacc\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mstxvp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxxsetaccz\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxxmfacc\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mstxvp\M} 4 } } */