[PATCH] Remove UNSPEC_{COPYSIGN,XORSIGN}.
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
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]
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
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]
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]
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]
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
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
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
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
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]
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
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]
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)
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
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 } } */