Re: [PATCH v5] c++: DR1710, template keyword in a typename-specifier [PR94057]
On 3/25/20 5:36 PM, Marek Polacek wrote:
On Wed, Mar 25, 2020 at 12:00:24AM -0400, Jason Merrill wrote:
On 3/24/20 11:45 AM, Marek Polacek wrote:
On Mon, Mar 23, 2020 at 10:41:28AM -0400, Jason Merrill wrote:
On 3/20/20 7:02 PM, Marek Polacek wrote:
On Fri, Mar 20, 2020 at 02:12:49PM -0400, Jason Merrill wrote:
On 3/20/20 1:06 PM, Marek Polacek wrote:
Wonderful. I've added a bunch of tests, and some from the related DRs too.
But I had a problem with the class-or-decltype case: if we have
template struct D : T::template B::template C {};
then we still require all the 'template' keywords here (as does clang). So I'm
kind of confused, but I don't think it's a big deal right now.
This seems related enough that I'd like to fix it at the same time; why
doesn't your patch fix it? Is it because typename_p is false?
Ah, I was mistaken. Of course we need the template keyword here: it's a member
of an
unknown specialization!
That's why it's needed in contexts where we don't know whether or not we're
naming a type. But where we do know that, as in a base-specifier, it isn't
necessary. This is exactly DR 1710: "The keyword template is optional in a
... class-or-decltype (Clause 11.7 [class.derived])"
Duh, not sure what I was thinking.
I've implemented that in cp_parser_nested_name_specifier_opt: assume 'template'
if we've seen 'typename'. If it turns out that this is wrong because it
triggers in contexts where it shouldn't, we'll have to introduce something like
CP_PARSER_FLAGS_TEMPLATE_OPTIONAL.
With this another problem revealed: we weren't accepting an alias template
after 'template' which DR1710 says is valid. Fixed by the TYPE_ALIAS_P hunk
in the new check_template_keyword_in_nested_name_spec. (alias-decl1.C)
But this is still not over: I noticed that
template struct S {
template
using U = TT;
};
template typename S::template U::type foo;
is still rejected and shouldn't be. Here
check_template_keyword_in_nested_name_spec
gets
>
Any suggestions how to handle this?
It should work to check for an alias whether or not the target is a
TYPENAME_TYPE.
Hmm, we don't have a TYPENAME_TYPE here at all, but since that
template_type_parm
is typedef_variant_p, it can be handled along with...
+static void
+check_template_keyword_in_nested_name_spec (tree name)
+{
+ if (CLASS_TYPE_P (name)
+ && ((CLASSTYPE_USE_TEMPLATE (name)
+ && PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (name)))
+ || CLASSTYPE_IS_TEMPLATE (name)))
+return;
+
+
+ if (TREE_CODE (name) == TYPENAME_TYPE)
+{
+ if (TREE_CODE (TYPENAME_TYPE_FULLNAME (name)) == TEMPLATE_ID_EXPR)
+ return;
+ /* Alias templates are also OK. */
+ else if (TYPE_ALIAS_P (name))
+ {
+ tree tinfo = TYPE_ALIAS_TEMPLATE_INFO (name);
+ if (tinfo && DECL_ALIAS_TEMPLATE_P (TI_TEMPLATE (tinfo)))
+ return;
+ }
+}
I think you want to check all typedefs like in e.g. find_parameter_packs_r;
if the name is a typedef, it's only suitable if
alias_template_specialization_p.
..this: Since alias_template_specialization_p already checks TYPE_P and
typedef_variant_p, can we simply use that?
But not all typedefs are aliases; 'typedef A type;' does not make
'::template type' valid just because '::template A' would be. If
it's an alias template specialization, OK; if it's any other typedef,
not OK.
/* Parse an (optional) nested-name-specifier.
nested-name-specifier: [C++98]
@@ -6389,7 +6422,17 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
/* Look for the optional `template' keyword, if this isn't the
first time through the loop. */
if (success)
- template_keyword_p = cp_parser_optional_template_keyword (parser);
+ {
+ template_keyword_p = cp_parser_optional_template_keyword (parser);
+ /* DR1710: "In a qualified-id used as the name in
+a typename-specifier, elaborated-type-specifier, using-declaration,
+or class-or-decltype, an optional keyword template appearing at
+the top level is ignored." */
+ if (!template_keyword_p
+ && typename_keyword_p
+ && cp_parser_nth_token_starts_template_argument_list_p (parser,
2))
+ template_keyword_p = true;
+ }
And since we now do this, we don't need...
@@ -23636,8 +23696,9 @@ cp_parser_class_name (cp_parser *parser,
&& decl != error_mark_node
&& !is_overloaded_fn (decl))
{
- decl = make_typename_type (scope, decl, typename_type,
-/*complain=*/tf_error);
+ tsubst_flags_t complain = (cp_parser_parsing_tentatively (parser)
+? tf_none : tf_error);
+ decl = make_typename_type (scope, decl, typename_type, complain);
Why?
...this hunk, so dropped. New test alias-decl2.C added, otherwise no changes.
Bootstrapped/regtested on x86_64-linux, ok
Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)
On 3/23/20 12:50 PM, Martin Sebor wrote: On 3/23/20 8:49 AM, Jason Merrill wrote: On 3/21/20 5:59 PM, Martin Sebor wrote: + /* Diagnose class/struct/union mismatches. IS_DECLARATION is false + for alias definition. */ + bool decl_class = (is_declaration + && cp_parser_declares_only_class_p (parser)); cp_parser_check_class_key (parser, key_loc, tag_type, type, false, cp_parser_declares_only_class_p (parser)); Don't you need to use the new variable? Don't your testcases exercise this? Of course they do. This was a leftover from an experiment after I put the initial updated patch together. On final review I decided to adjust some comments and forgot to restore the original use of the variable. + /* When TYPE is the use of an implicit specialization of a previously + declared template set TYPE_DECL to the type of the primary template + for the specialization and look it up in CLASS2LOC below. For uses + of explicit or partial specializations TYPE_DECL already points to + the declaration of the specialization. + IS_USE is clear so that the type of an implicit instantiation rather + than that of a partial specialization is determined. */ + type_decl = TREE_TYPE (type_decl); + if (TREE_CODE (type_decl) != TEMPLATE_DECL) + type_decl = TYPE_MAIN_DECL (type_decl); The comment is no longer relevant to the code. The remaining code also seems like it would have no effect; we already know type_decl is TYPE_MAIN_DECL (type). I removed the block of code. Martin PS I would have preferred to resolve just the reported problem in this patch and deal with the template specializations more fully (and with aliases) in a followup. As it is, it has grown bigger and more complex than I'm comfortable with, especially with the template specializations, harder for me to follow, and obviously a lot more time-consuming not just to put together but also to review. Although this revision handles many more template specialization cases correctly, there still are other (arguably corner) cases that it doesn't. I suspect getting those right might even require a design change, which I see as out of scope at this time (not to mention my ability). Sure, at this point in the cycle there's always a tradeoff between better functionality and risk from ballooning changes. It looks like the improved template handling could still be split out into a separate patch, if you'd prefer. + /* Number of usesn of the class. */ Typo. + definintion if one exists or the first declaration otherwise. */ typo. + if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0)) ... +the first reference to the instantiation. The primary must +be (and inevitably is) at index zero. */ I think CLASSTYPE_IMPLICIT_INSTANTIATION is more readable than testing the value 1. What is the !is_decl (0) intended to do? Changing it to an assert inside the 'if' doesn't seem to affect any of the testcases. + For implicit instantiations of a primary template it's + the class-key used to declare the primary with. The primary + must be at index zero. */ + const tag_types xpect_key += cdlguide->class_key (cdlguide == this ? idxguide : 0); A template can also be declared before it's defined; I think you want to move the def_p/idxdef/idxguide logic into another member function that you invoke on cdlguide to perhaps get the class_key_loc_t that you want to use as the pattern. Jason
[PATCH] Add XGCC_FLAGS_FOR_TARGET to CXX in POSTSTAGE1_CXX_EXPORT
This contains the necessary sysroot flags if configured with --with-build-sysroot, and matches the corresponding value for CC in POSTSTAGE1_HOST_EXPORTS. 2020-03-25 Michael Forney * Makefile.tpl (POSTSTAGE1_HOST_EXPORTS): Add XGCC_FLAGS_FOR_TARGET and TFLAGS to CXX. * Makefile.in: Regenerate. --- I ran into this when building gcc with --enable-bootstrap and --with-build-sysroot. Since the sysroot flag doesn't get passed to xg++, the build fails at configure-stage2-gcc, since it can't find the right headers: checking how to run the C++ preprocessor... /lib/cpp configure: error: in `/tmp/gcc/host-x86_64-linux-musl/gcc': configure: error: C++ preprocessor "/lib/cpp" fails sanity check See `config.log' for more details make[2]: *** [Makefile:4434: configure-stage2-gcc] Error 1 Makefile.in | 4 ++-- Makefile.tpl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.in b/Makefile.in index 86c0c6d5b2d..686983de233 100644 --- a/Makefile.in +++ b/Makefile.in @@ -248,8 +248,8 @@ POSTSTAGE1_CXX_EXPORT = \ `if $(LEAN); then echo ' -isystem '; else echo ' -I'; fi`$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/include \ `if $(LEAN); then echo ' -isystem '; else echo ' -I'; fi`$$s/libstdc++-v3/libsupc++ \ -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/src/.libs \ - -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/libsupc++/.libs"; \ - export CXX; \ + -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/libsupc++/.libs \ + $(XGCC_FLAGS_FOR_TARGET) $$TFLAGS"; export CXX; \ CXX_FOR_BUILD="$$CXX"; export CXX_FOR_BUILD; @endif target-libstdc++-v3-bootstrap diff --git a/Makefile.tpl b/Makefile.tpl index efed1511750..b627069e8c8 100644 --- a/Makefile.tpl +++ b/Makefile.tpl @@ -251,8 +251,8 @@ POSTSTAGE1_CXX_EXPORT = \ `if $(LEAN); then echo ' -isystem '; else echo ' -I'; fi`$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/include \ `if $(LEAN); then echo ' -isystem '; else echo ' -I'; fi`$$s/libstdc++-v3/libsupc++ \ -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/src/.libs \ - -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/libsupc++/.libs"; \ - export CXX; \ + -L$$r/prev-$(TARGET_SUBDIR)/libstdc++-v3/libsupc++/.libs \ + $(XGCC_FLAGS_FOR_TARGET) $$TFLAGS"; export CXX; \ CXX_FOR_BUILD="$$CXX"; export CXX_FOR_BUILD; @endif target-libstdc++-v3-bootstrap -- 2.26.0
[PATCH, OpenACC] Bug fix for processing OpenACC data clauses in C++
The attached patch fixes a bug I found in the C++ front end's handling
of OpenACC data clauses. The problem here is that the C++ front end
wraps the array bounds expressions in NON_LVALUE_EXPR tree nodes, and
the code here (which appears to have been copied from similar code in
the C front end) was failing to strip those before checking to see if
they were INTEGER_CST nodes, etc.
Sadly, I have no test case for this because it was only triggering an
error in conjunction with some other OpenACC patches that are not yet on
trunk, and the tree dumps don't show anything useful. I confirmed that
the problem exists on trunk without those other patches by examining
things in the debugger. This is the example I was using for my
hand-testing:
void f (float a[16][16], float b[16][16], float c[16][16])
{
int i, j, k;
#pragma acc kernels copyin(a[0:16][0:16], b[0:16][0:16])
copyout(c[0:16][0:16])
{
for (i = 0; i < 16; i++) {
for (j = 0; j < 16; j++) {
float t = 0;
for (k = 0; k < 16; k++)
t += a[i][k] * b[k][j];
c[i][j] = t;
}
}
}
}
Is this patch OK to commit now, or in Stage 1?
-Sandra
commit 3d2cda1e758a54111af04e80ea3dbaa27b3387e7
Author: Sandra Loosemore
Date: Wed Mar 25 21:29:17 2020 -0700
Bug fix for processing OpenACC data clauses in C++.
The C++ front end wraps the values in OpenACC data clause array range
specifications in NON_LVALUE_EXPR tree nodes. The existing code was
failing to strip these before checking for constant values.
2020-03-25 Sandra Loosemore
gcc/cp/
* semantics.c (handle_omp_array_sections_1): Call STRIP_NOPS
on length and low_bound to unwrap NON_LVALUE_EXPRs.
(handle_omp_array_sections): Likewise.
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 34ccb9f..8945939 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2020-03-25 Sandra Loosemore
+
+ * semantics.c (handle_omp_array_sections_1): Call STRIP_NOPS
+ on length and low_bound to unwrap NON_LVALUE_EXPRs.
+ (handle_omp_array_sections): Likewise.
+
2020-03-25 Patrick Palka
PR c++/94265
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 2721a55..2efc077 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -4861,6 +4861,10 @@ handle_omp_array_sections_1 (tree c, tree t, vec ,
length = mark_rvalue_use (length);
/* We need to reduce to real constant-values for checks below. */
if (length)
+STRIP_NOPS (length);
+ if (low_bound)
+STRIP_NOPS (low_bound);
+ if (length)
length = fold_simple (length);
if (low_bound)
low_bound = fold_simple (low_bound);
@@ -5160,6 +5164,11 @@ handle_omp_array_sections (tree c, enum c_omp_region_type ort)
tree low_bound = TREE_PURPOSE (t);
tree length = TREE_VALUE (t);
+ if (length)
+ STRIP_NOPS (length);
+ if (low_bound)
+ STRIP_NOPS (low_bound);
+
i--;
if (low_bound
&& TREE_CODE (low_bound) == INTEGER_CST
[PATCH] rs6000: Save/restore r31 if frame_pointer_needed is true
From: Xionghu Luo This P1 bug is exposed by FRE refactor of r263875. Comparing the fre dump file shows no obvious change of the segment fault function proves it to be a target issue. frame_pointer_needed is set to true in reload pass setup_can_eliminate, but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore r31 even it is used actually, causing CPU2006 465.tonto segment fault of loading from invalid addresses. Bootstrap and regression tested pass on Power8-LE. Backport to gcc-9 required once approved. gcc/ChangeLog 2020-03-26 Xiong Hu Luo PR target/91518 * config/rs6000/rs6000-logue.c (save_reg_p): Save r31 if frame_pointer_needed. --- gcc/config/rs6000/rs6000-logue.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c index 4cbf228eb79..7b62ff03afd 100644 --- a/gcc/config/rs6000/rs6000-logue.c +++ b/gcc/config/rs6000/rs6000-logue.c @@ -116,6 +116,9 @@ save_reg_p (int reg) return true; } + if (reg == HARD_FRAME_POINTER_REGNUM && frame_pointer_needed) +return true; + return !call_used_or_fixed_reg_p (reg) && df_regs_ever_live_p (reg); } -- 2.21.0.777.g83232e3864
[PING] [PATCH] [ARM] Adjust test expectations of unaligned-memcpy-2/3.c (PR 91614)
Hi,
I am pinging this because PR 91614 has been raised to P2 now:
https://gcc.gnu.org/legacy-ml/gcc-patches/2019-09/msg00370.html
Thanks
Bernd.
On 9/6/19 2:12 PM, Bernd Edlinger wrote:
> On 9/5/19 5:55 PM, Richard Earnshaw (lists) wrote:
>> On 03/09/2019 21:45, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> due to the introduction of unaligned_loaddi and unaligned_storedi,
>>> two test cases show some regression as PR 91614 points out.
>>>
>>> I would like to change the test expectations if these two test cases,
>>> since they seem to be bogus.
>>>
>>> That is the test case already failed for arm_prefer_ldrd_strd targets,
>>> since not a ldrd or strd is created but a ldm/stm instead, which
>>> is probably not what is intended.
>>>
>>> That is for arm_prefer_ldrd_strd the previously used instruction movdi:
>>>
>>> (insn 11 10 12 2 (set (mem/c:DI (reg:SI 112) [0 MEM
>>> [(voidD.71 *)]+0 S8 A32])
>>> (reg:DI 114)) "unaligned-memcpy-2.c":11:3 642 {*movdi_vfp}
>>> (nil))
>>>
>>> is split up very early in subreg1 into:
>>>
>>> (insn 21 10 22 2 (set (mem/c:SI (reg:SI 112) [0 MEM
>>> [(voidD.71 *)]+0 S4 A32])
>>> (reg:SI 119)) "unaligned-memcpy-2.c":11:3 640 {*arm_movsi_vfp}
>>> (nil))
>>> (insn 22 21 12 2 (set (mem/c:SI (plus:SI (reg:SI 112)
>>> (const_int 4 [0x4])) [0 MEM [(voidD.71
>>> *)]+4 S4 A32])
>>> (reg:SI 120 [+4 ])) "unaligned-memcpy-2.c":11:3 640
>>> {*arm_movsi_vfp}
>>> (nil))
>>>
>>> which is finally replaced by:
>>>
>>> (insn 35 10 12 2 (parallel [
>>> (set (mem/c:SI (reg/f:SI 3 r3 [111]) [0 MEM
>>> [(voidD.71 *)]+0 S4 A32])
>>> (reg:SI 1 r1))
>>> (set (mem/c:SI (plus:SI (reg/f:SI 3 r3 [111])
>>> (const_int 4 [0x4])) [0 MEM
>>> [(voidD.71 *)]+4 S4 A32])
>>> (reg:SI 2 r2))
>>> ]) "unaligned-memcpy-2.c":11:3 378 {*stm2_}
>>> (expr_list:REG_DEAD (reg:SI 2 r2)
>>> (expr_list:REG_DEAD (reg:SI 1 r1)
>>> (nil
>>>
>>> ... so stm instead of strd.
>>>
>>> Since the unalinged_storedi is an unspec, it is expanded as strd which is
>>> kind of okay, but there is register pair spilled which was not there
>>> previously:
>>>
>>> aligned_dest:
>>> @ args = 0, pretend = 0, frame = 0
>>> @ frame_needed = 0, uses_anonymous_args = 0
>>> @ link register save eliminated.
>>> strd r4, [sp, #-8]!
>>> ldr r4, [r0] @ unaligned
>>> movw r3, #:lower16:.LANCHOR0
>>> ldr r5, [r0, #4] @ unaligned
>>> movt r3, #:upper16:.LANCHOR0
>>> strd r4, [r3]
>>> ldr r2, [r0, #8] @ unaligned
>>> str r2, [r3, #8]
>>> ldrh r2, [r0, #12] @ unaligned
>>> strh r2, [r3, #12] @ movhi
>>> ldrb r2, [r0, #14] @ zero_extendqisi2
>>> strb r2, [r3, #14]
>>> ldrd r4, [sp]
>>> add sp, sp, #8
>>> bx lr
>>>
>>> The patch filters out ldrd/strd that sp-relative, and adds a few
>>> missing scan-assembler rules, in order to make the test results more stable.
>>>
>>> Alternative could be to use two movsi instead of the unaligned_load/storedi,
>>> but that would end up in ldm/stm which looks plain wrong to me.
>>>
>>>
>>> Tested using various arm-linux-gnueabihf cross-compilers.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>> 2019-09-03 Bernd Edlinger
>>
>> PR target/91614
>> * gcc.target/arm/unaligned-memcpy-2.c: Adjust test expectations.
>> * gcc.target/arm/unaligned-memcpy-3.c: Likewise.
>>
>> Index: gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c
>> ===
>> --- gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c (Revision 275341)
>> +++ gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c (Arbeitskopie)
>> @@ -15,9 +15,11 @@
>> loads/stores for the remainder. */
>>
>> /* { dg-final { scan-assembler-times "ldmia" 0 } } */
>> -/* { dg-final { scan-assembler-times "ldrd" 0 } } */
>> +/* { dg-final { scan-assembler-times "ldrd\(?!\[^\\n\]*sp\)" 0 } } */
>> +/* { dg-final { scan-assembler-times "ldm\(?!ia\)" 0 } } */
>> /* { dg-final { scan-assembler-times "stmia" 1 { target { ! {
>> arm_prefer_ldrd_strd } } } } } */
>> -/* { dg-final { scan-assembler-times "strd" 1 { target {
>> arm_prefer_ldrd_strd } } } } */
>> +/* { dg-final { scan-assembler-times "strd\(?!\[^\\n\]*sp\)" 1 { target {
>> arm_prefer_ldrd_strd } } } } */
>> +/* { dg-final { scan-assembler-times "stm\(?!ia\)" 0 } } */
>> /* { dg-final { scan-assembler-times "ldrh" 1 } } */
>> /* { dg-final { scan-assembler-times "strh" 1 } } */
>> /* { dg-final { scan-assembler-times "ldrb" 1 } } */
>> Index: gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c
>> ===
>> --- gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c (Revision 275341)
>> +++ gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c (Arbeitskopie)
>> @@
Re: [committed] Add missing T register clobber for SH port
On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote: The patch you sent, as well as what you committed as r10-7383, was just a ChangeLog entry. > Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu. Regression testing in > progress (won't finish for ~12 hours). No new test as it's covered by vector- > compare-1 in the testsuite. ...so I guess you've noticed by the time you read this. :] brgds, H-P
RE: [RFC] Should widening_mul should consider block frequency?
Hi! > -Original Message- > From: Richard Biener [mailto:richard.guent...@gmail.com] > Sent: Tuesday, March 24, 2020 10:14 PM > To: Yangfei (Felix) > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [RFC] Should widening_mul should consider block frequency? > > > > As written in the PR I'd follow fma generation and restrict defs to the > > > same > BB. > > > > Thanks for the suggestion. That should be more consistent. > > Attached please find the adapted patch. > > Bootstrap and tested on both x86_64 and aarch64 Linux platform. > > OK with moving the BB check before the is_widening_mult_p call since it's way > cheaper. That's a good point. I have attached the v2 patch. Also did a spec2017 test on aarch64, no obvious impact witnessed with this. Can you sponsor this patch please? My networking does not work well and I am having some trouble pushing it : - ( git commit msg: widening_mul: restrict ops to be defined in the same basic-block when convert plusminus to widen In the testcase for PR94269, widening_mul moves two multiply instructions from outside the loop to inside the loop, merging with two add instructions separately. This increases the cost of the loop. Like FMA detection in the same pass, simply restrict ops to be defined in the same basic-block to avoid possibly moving multiply to a different block with a higher execution frequency. 2020-03-26 Felix Yang gcc/ PR tree-optimization/94269 * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict this operation to single basic block. gcc/testsuite/ PR tree-optimization/94269 * gcc.dg/pr94269.c: New test. change log: gcc: +2020-03-26 Felix Yang + + PR tree-optimization/94269 + * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict this + operation to single basic block. gcc/testsuite: +2020-03-26 Felix Yang + + PR tree-optimization/94269 + * gcc.dg/pr94269.c: New test. + pr94269-v2.patch Description: pr94269-v2.patch
Re: [PATCH] Rearrange detection of temporary directory for NetBSD
On 25.03.2020 23:36, Jeff Law wrote: > On Wed, 2020-03-18 at 20:29 +0100, Kamil Rytarowski wrote: >> Set /tmp first, then /var/tmp. /tmp is volatile on NetBSD and >> /var/tmp not. This improves performance in the common use. >> The downstream copy of GCC was patched for this preference >> since 2015. >> >> Remove occurence of /usr/tmp as it was never valid for NetBSD. >> It was already activey disabled in the GCC manual page in 1996 and >> in the GCC source code at least in 1998. >> >> This change is not a matter of user-preference but Operating >> System defaults that disagree with the libiberty detection plan. >> >> No functional change for other Operataing Systems/environments. >> >> libiberty/ChangeLog: >> >> * make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths. >> --- >> libiberty/ChangeLog| 4 >> libiberty/make-temp-file.c | 8 +++- >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog >> index 106c107e91a..18b9357aaed 100644 >> --- a/libiberty/ChangeLog >> +++ b/libiberty/ChangeLog >> @@ -1,3 +1,7 @@ >> +2020-03-18 Kamil Rytarowski >> + >> +* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths. > I'd strongly recommend against this as-is. > > The whole reason we prefer /var/tmp is because it's often dramatically larger > than a ram-backed /tmp. > NetBSD supports swap whenever /tmp is too small. Whenever tmpfs+ram are too small, users use /tmp that is a regular directory (if I am not wrong, this is still the default setup from an installer). In NetBSD we want to use a non-persistent storage for performance reasons. /var/tmp also affects negatively modern SSD devices with needless writes. It would be enough to get these try_dir paths tuned at least from preprocessor during libiberty build for gdb/gcc/etc. > I wouldn't mind dropping /usr/tmp. That so antiquated that it'd be non- > controversial. Can you send that as a separate patch. > Behavior for !__NetBSD__ is out of interest. > Jeff >> >
Re: [PATCH 3/3] Implementation of -Wclobbered on tree-ssa
On Wed, 2020-01-29 at 18:32 +0300, Alexander Monakov wrote:
> On Tue, 28 Jan 2020, Jeff Law wrote:
>
> > On Tue, 2019-10-08 at 18:04 +0300, Alexander Monakov wrote:
> > > On Thu, 3 Oct 2019, Jeff Law wrote:
> > >
> > > > You may want to review the 2018 discussion:
> > > >
> > > > https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg185287.html
> > > >
> > > > THe 2018 discussion was primarily concerned with fixing the CFG
> > > > inaccuracies which would in turn fix 61118. But would not fix 21161.
> > >
> > > Well, PR 61118 was incorrectly triaged to begin with.
> > >
> > > Let me show starting from a simplified testcase:
> > >
> > > void bar(jmp_buf);
> > > void foo(int n)
> > > {
> > > jmp_buf jb;
> > > for (; n; n--)
> > > if (setjmp(jb))
> > > bar(jb);
> > > }
> > >
> > > Here we warn that "parameter 'n' might be clobbered..." despite that
> > > "obviously" there is no way 'n' might be modified between setjmp and
> > > bar... right?
> > Not necessarily. You have to look at what objects are live. N is
> > clearly going to be live throughout the entire loop and thus it will be
> > subject to setjmp/longjmp warnings.
>
> That N is live is not enough: we want to warn if N is also modified
> before an abnormal return (otherwise we'd attempt to warn for live
> readonly variables). But if you're agreeing with me that warning here
> is *not a false positive*, then I'm happy.
We have traditionally only bothered to analyze variables that are live at the
setjmp point. Objects which are not live at the setjmp point need not be
analyzed. That's all I was saying.
> >
> > > Now suppose 'bar' looks like
> > [ ... ]
> > It doesn't really matter what bar looks like.
>
> Well it did for the purpose of demonstration, but if you're in agreement
> anyway then we can say it's moot.
In general we're not going to necessarily have visibility into calls where the
longjmp may occur. Any analysis we do has to not depend on looking inside the
called routine.
>
> > > Still, I can imagine one can argue that the warning in the PR is bogus, as
> > > the loop assigns invariant values to problematic variables:
> > I'm not sure which PR you're referring to (21161, 65041 or some other?)
>
> I'm arguing that PR 61118 was incorrectly triaged (first sentence of my
> email).
Who's triage are you referring to? :-)
It's been a couple years since I dug into 61118, but my recollection was that
the
core problem was the multiple assignments to the pseudo registers holding
cancel_arg and cancel_routine. If there was a single assignment to those
pseudos
then the existing RTL Wclobbered analysis would not issue a warning.
While the assignments use the same source, the mere existence of multiple
assignments causes real headaches for the existing -Wclobbered analysis. The
multiple assignments occur due to the actions of CCP1 and FRE1 propagating into
a
PHI node, which they arguably shouldn't do for an abnormal PHI.
>
> > > void foo(struct ctx *arg)
> > > {
> > > while (arg->cond) {
> > > void *var1 = >field;
> > > void (*var2)(void*) = globalfn;
> > > jmp_buf jb;
> > > if (!setjmp(jb))
> > > var2(var1);
> > > }
> > > etc(arg);
> > > }
> > >
> > > and since "obviously" >field is invariant, there's no way setjmp will
> > > overwrite 'var1' with a different value, right?.. Except:
> > >
> > > If the while loop is not entered, a longjmp from inside 'etc' would
> > > restore
> > > default (uninitialized) values; note the compiler doesn't pay attention to
> > > the
> > > lifetime of jmp_buf, it simply assumes 'etc' may cause setjmp to return
> > > abnormally (this is correct in case of other returns_twice functions such
> > > as
> > > vfork).
> > Whether or not we should warn for restoring an uninitialized state I
> > think is somethign we haven't really considered. Depending on how many
> > assignments to the pseudo there are and the exact control flow we may
> > or may not warn.
>
> That restored values are uninitialized is not important; the problem remains
> if var1 is declared before the 'while' and initialized to 0.
Sorry, I don't follow your statement. From the fragment above var1 is assigned
only once and inside the loop. I don't see where it would be initialized to
zero.
>
> > > Now regarding claims that we emit "incorrect" CFG that is causing extra
> > > phi
> > > nodes, and that "fixing CFG" would magically eliminate those and help with
> > > false positives.
> > >
> > > While in principle it is perhaps nicer to have IR that more closely models
> > > the abnormal flow, I don't see how it would reduce phi nodes; we'd go from
> > It certainly does move PHI nodes and it can reduce the number of false
> > positives. It's not as effective as it could be because of lameness on
> > the RTL side (again see PR21161).
>
> There's some mixup here because
Re: [PATCH] Fix vextract* masked patterns (PR target/93069)
On Mon, 2019-12-30 at 00:46 +0100, Jakub Jelinek wrote:
> Hi!
>
> The AVX512F documentation clearly states that in instructions where the
> destination is a memory only merging-masking is possible, not zero-masking,
> and the assembler enforces that.
>
> The testcase in this patch fails to assemble because of
> Error: unsupported masking for `vextracti32x8'
> on
> vextracti32x8 $0x0, %zmm1, -64(%rsp){%k1}{z}
> For the vector extraction patterns, we apparently have 7 *_maskm patterns
> that only accept memory destinations and rtx_equal_p merge-masking source
> for it, 7 * corresponding patterns that allow memory destination
> only for the non-masked cases (through ), then 2
> * patterns (lo ssehalf V16FI and lo ssehalf VI8F_256 ones) which
> do allow memory destination even for masked cases and are the cause of the
> testsuite failure, because we must not allow C constraint if the destination
> is m, and finally one pair of patterns (separate * and *_mask, hi ssehalf
> VI4F_256), which has another issue (for which I don't have a testcase
> though), where if it would match zero-masking with register destination,
> it wouldn't emit the needed {z} into assembly.
> The attached patch fixes those 3 issues only, perhaps more suitable for
> backporting.
> But, even with that fixed, we are missing 3 further *_maskm patterns and
> more importantly, I find the split into 3 separate patterns after subst,
> *_maskm for masking with memory destination, *_mask for masking with
> register destination and * for non-masking unnecessarily complex and harder
> for reload, so the included patch below (non-attached) instead kills all
> *_maskm patterns and splits the * patterns into * and *_mask
> by hand instead of subst, where the *_mask ones make sure that with v
> destination they use 0C, while with m destination they use 0 and as
> condition enforce that either destination is not MEM, or rtx_equal_p between
> the destination and corresponding merging-masking operand source.
> If we had those 3 missing *_maskm patterns, this patch would actually result
> in both shorter sse.md and shorter machine description after subst (e.g.
> length of tmp-mddump.md), as we don't have them, the patch is actually 16
> lines longer sse.md, but still shorter tmp-mddump.md.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (and is
> the shorter patch ok for backports)?
>
> 2019-12-30 Jakub Jelinek
>
> PR target/93069
> * config/i386/subst.md (store_mask_constraint, store_mask_predicate):
> Remove.
> (avx512dq_vextract64x2_1_maskm,
> avx512f_vextract32x4_1_maskm,
> vec_extract_lo__maskm, vec_extract_hi__maskm): Remove.
> (avx512dq_vextract64x2_1): Split
> into ...
> (*avx512dq_vextract64x2_1,
> avx512dq_vextract64x2_1_mask): ... these new
> define_insns. Even in the masked variant allow memory output but in
> that case use 0 rather than 0C constraint on the source of masked-out
> elts.
> (avx512f_vextract32x4_1): Split
> into ...
> (*avx512f_vextract32x4_1,
> avx512f_vextract32x4_1_mask): ... these new define_insns.
> Even in the masked variant allow memory output but in that case use
> 0 rather than 0C constraint on the source of masked-out elts.
> (vec_extract_lo_): Split into ...
> (vec_extract_lo_, vec_extract_lo__mask): ... these new
> define_insns. Even in the masked variant allow memory output but in
> that case use 0 rather than 0C constraint on the source of masked-out
> elts.
> (vec_extract_hi_): Split into ...
> (vec_extract_hi_, vec_extract_hi__mask): ... these new
> define_insns. Even in the masked variant allow memory output but in
> that case use 0 rather than 0C constraint on the source of masked-out
> elts.
>
> * gcc.target/i386/avx512vl-pr93069.c: New test.
> * gcc.dg/vect/pr93069.c: New test.
Sorry. I know you asked me to look at this eons ago, but ever time I just get
lost.
I get the distinct impression that we could do something much simpler (the patch
you initially proposed for backporting to the release branches). Perhaps we go
with that now and the full patch for gcc-11?
Jeff
Re: [PATCH] Fix stack pointer handling in ms_hook_prologue functions for i386 target.
On Mon, 2020-02-10 at 19:22 +0300, Paul Gofman wrote: > ChangeLog: > PR target/91489 > * config/i386/i386.md (simple_return): Also check > for ms_hook_prologue function attribute. > * config/i386/i386.c (ix86_can_use_return_insn_p): > Also check for ms_hook_prologue function attribute. > > testsuite/ChangeLog: > PR target/91489 > * gcc.target/i386/ms_hook_prologue.c: Expand testcase > to reproduce PR target/91489 issue. I'm going to queue this for gcc-11. While I realize this is a codegen issue, I'm not familiar enough with the stdcall conventions to give this the review it would need at this stage in gcc-10 development. jeff >
Re: [PATCH] pch: Specify reason of -Winvalid-pch warning [PR86674]
On Mon, 2020-03-09 at 11:55 +0300, Nicholas Guriev wrote: > gcc/c-family/ChangeLog: > > PR pch/86674 > * c-pch.c (c_common_valid_pch): Use cpp_warning with CPP_W_INVALID_PCH > reason to fix -Werror=invalid-pch and -Wno-error=invalid-pch switches. THanks. This looks pretty reasonable, but I'm going to queue it for gcc-11. jeff >
Re: [PATCH] Rearrange detection of temporary directory for NetBSD
On Wed, 2020-03-18 at 20:29 +0100, Kamil Rytarowski wrote: > Set /tmp first, then /var/tmp. /tmp is volatile on NetBSD and > /var/tmp not. This improves performance in the common use. > The downstream copy of GCC was patched for this preference > since 2015. > > Remove occurence of /usr/tmp as it was never valid for NetBSD. > It was already activey disabled in the GCC manual page in 1996 and > in the GCC source code at least in 1998. > > This change is not a matter of user-preference but Operating > System defaults that disagree with the libiberty detection plan. > > No functional change for other Operataing Systems/environments. > > libiberty/ChangeLog: > > * make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths. > --- > libiberty/ChangeLog| 4 > libiberty/make-temp-file.c | 8 +++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog > index 106c107e91a..18b9357aaed 100644 > --- a/libiberty/ChangeLog > +++ b/libiberty/ChangeLog > @@ -1,3 +1,7 @@ > +2020-03-18 Kamil Rytarowski > + > + * make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths. I'd strongly recommend against this as-is. The whole reason we prefer /var/tmp is because it's often dramatically larger than a ram-backed /tmp. I wouldn't mind dropping /usr/tmp. That so antiquated that it'd be non- controversial. Can you send that as a separate patch. Jeff >
Re: [PATCH] var-tracking: Mark as sp based more VALUEs [PR92264]
On Fri, 2020-03-20 at 11:04 +0100, Jakub Jelinek via Gcc-patches wrote: > Hi! > > With this simple patch, on i686-linux and x86_64-linux with -m32 (no change > for -m64), the find_base_term visited_vals.length () > 100 find_base_term > statistics changed (fbt is before this patch, fbt2 with this patch): > for k in '' '1$'; do for i in 32 64; do for j in fbt fbt2; do \ > echo -n "$j $i $k "; LC_ALL=C grep ^$i.*"$k" $j | wc -l; done; done; done > fbt 32 5313355 > fbt2 32 4229854 > fbt 64 217523 > fbt2 64 217523 > fbt 32 1$ 1296 > fbt2 32 1$ 407 > fbt 64 1$ 0 > fbt2 64 1$ 0 > For frame_pointer_needed functions, we need to wait until we see the > fp_setter insn in the prologue at which point we disassociate the fp based > VALUEs from sp based VALUEs, but for !frame_pointer_needed functions, > we IMHO don't need to wait anything. For ACCUMULATE_OUTGOING_ARGS it isn't > IMHO worth doing anything, as there is a single sp adjustment and so there > is no risk of many thousands deep VALUE chains, but for > !ACCUMULATE_OUTGOING_ARGS the sp keeps changing constantly. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Perhaps even better would be if we'd be able already in cselib somehow > figure out these sp based VALUEs and be able to constantly reuse them and in > loc list for them remember (apart from the registers etc. that hold the > VALUE ATM) have just (plus:P (value:P sp0) (const_int NNN)) where sp0 would > be a VALUE of stack pointer at the start of !frame_pointer_needed functions > and for frame_pointer_needed functions say sp VALUE at the point of > fp_setter insn. Then if we have > sp -= 4 > sp -= 4 > sp -= 4 > sp += 8 > sp -= 4 > sp += 8 > sp -= 4 > sp -= 4 > sp += 8 > we wouldn't need 10 sp based VALUEs, but only one for orig sp, one for > orig_sp-4, another one for orig_sp-8, another one for orig_sp-12 and that's > it. > > 2020-03-19 Jakub Jelinek > > PR rtl-optimization/92264 > * var-tracking.c (add_stores): Call cselib_set_value_sp_based even > for sp based values in !frame_pointer_needed > && !ACCUMULATE_OUTGOING_ARGS functions. OK. jeff >
Re: [PATCH] remove struct == POD coding convention [PR61339]
On Wed, 2020-03-25 at 15:46 -0600, Martin Sebor via Gcc-patches wrote: > PR 61339 - mismatch between struct and class [-Wmismatched-tags] > > ChangeLog: > > * htdocs/codingconventions.html (Struct Definitions): Remove > old convention. > (Class Definitions): Same. > * htdocs/codingrationale.html (Struct Definitions): Document > convention change. OK jeff
[committed] libstdc++: Define and use chrono::is_clock for C++20
For C++20 the wait_until members of mutexes and condition variables are required to be ill-formed if given a clock that doesn't meet the requirements for a clock type. To implement that requirement this patch adds static assertions using the chrono::is_clock trait, and defines that trait. To avoid expensive checks for the common cases, the trait (and associated variable template) are explicitly specialized for the standard clock types. This also moves the filesystem::__file_clock type from to , so that chrono::file_clock and chrono::file_time can be defined in as required. * include/bits/fs_fwd.h (filesystem::__file_clock): Move to ... * include/std/chrono (filesystem::__file_clock): Here. (filesystem::__file_clock::from_sys, filesystem::__file_clock::to_sys): Define public member functions for C++20. (is_clock, is_clock_v): Define traits for C++20. * include/std/condition_variable (condition_variable::wait_until): Add check for valid clock. * include/std/future (_State_baseV2::wait_until): Likewise. * include/std/mutex (__timed_mutex_impl::_M_try_lock_until): Likewise. * include/std/shared_mutex (shared_timed_mutex::try_lock_shared_until): Likewise. * include/std/thread (this_thread::sleep_until): Likewise. * testsuite/30_threads/condition_variable/members/2.cc: Qualify slow_clock with new namespace. * testsuite/30_threads/condition_variable/members/clock_neg.cc: New test. * testsuite/30_threads/condition_variable_any/members/clock_neg.cc: New test. * testsuite/30_threads/future/members/clock_neg.cc: New test. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc: Qualify slow_clock with new namespace. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/ clock_neg.cc: New test. * testsuite/30_threads/shared_future/members/clock_neg.cc: New test. * testsuite/30_threads/shared_lock/locking/clock_neg.cc: New test. * testsuite/30_threads/shared_timed_mutex/try_lock_until/clock_neg.cc: New test. * testsuite/30_threads/timed_mutex/try_lock_until/3.cc: Qualify slow_clock with new namespace. * testsuite/30_threads/timed_mutex/try_lock_until/4.cc: Likewise. * testsuite/30_threads/timed_mutex/try_lock_until/clock_neg.cc: New test. * testsuite/30_threads/unique_lock/locking/clock_neg.cc: New test. * testsuite/std/time/traits/is_clock.cc: New test. * testsuite/util/slow_clock.h (slow_clock): Move to __gnu_test namespace. Tested powerpc64le-linux. Committed to master. commit bf1fc37bb4a3cab851e2acec811427d5243a22e9 Author: Jonathan Wakely Date: Wed Mar 25 22:07:02 2020 + libstdc++: Define and use chrono::is_clock for C++20 For C++20 the wait_until members of mutexes and condition variables are required to be ill-formed if given a clock that doesn't meet the requirements for a clock type. To implement that requirement this patch adds static assertions using the chrono::is_clock trait, and defines that trait. To avoid expensive checks for the common cases, the trait (and associated variable template) are explicitly specialized for the standard clock types. This also moves the filesystem::__file_clock type from to , so that chrono::file_clock and chrono::file_time can be defined in as required. * include/bits/fs_fwd.h (filesystem::__file_clock): Move to ... * include/std/chrono (filesystem::__file_clock): Here. (filesystem::__file_clock::from_sys, filesystem::__file_clock::to_sys): Define public member functions for C++20. (is_clock, is_clock_v): Define traits for C++20. * include/std/condition_variable (condition_variable::wait_until): Add check for valid clock. * include/std/future (_State_baseV2::wait_until): Likewise. * include/std/mutex (__timed_mutex_impl::_M_try_lock_until): Likewise. * include/std/shared_mutex (shared_timed_mutex::try_lock_shared_until): Likewise. * include/std/thread (this_thread::sleep_until): Likewise. * testsuite/30_threads/condition_variable/members/2.cc: Qualify slow_clock with new namespace. * testsuite/30_threads/condition_variable/members/clock_neg.cc: New test. * testsuite/30_threads/condition_variable_any/members/clock_neg.cc: New test. * testsuite/30_threads/future/members/clock_neg.cc: New test. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc: Qualify slow_clock with new namespace. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/ clock_neg.cc: New test. *
[committed] libstdc++ Add missing tests for std::shared_timed_mutex
These tests were supposed to be committed as part of r278904 (aka
b789efeae8c0620b83f25e4a0757c4871e02ab5f) but I didn't 'git add' them.
* testsuite/30_threads/shared_timed_mutex/try_lock_until/1.cc: New
test.
* testsuite/30_threads/shared_timed_mutex/try_lock_until/2.cc: New
test.
Tested powerpc64le-linux. Committed to master.
commit e3ef371982a1a5deac31adbe0eb305d3ee70e094
Author: Jonathan Wakely
Date: Wed Mar 25 22:16:22 2020 +
libstdc++ Add missing tests for std::shared_timed_mutex
These tests were supposed to be committed as part of r278904 (aka
b789efeae8c0620b83f25e4a0757c4871e02ab5f) but I didn't 'git add' them.
* testsuite/30_threads/shared_timed_mutex/try_lock_until/1.cc: New
test.
* testsuite/30_threads/shared_timed_mutex/try_lock_until/2.cc: New
test.
diff --git
a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/1.cc
b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/1.cc
new file mode 100644
index 000..f0c98ed571f
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/1.cc
@@ -0,0 +1,87 @@
+// { dg-do run }
+// { dg-options "-pthread" }
+// { dg-require-effective-target c++14 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 2019-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: [PATCH] avoid modifying type in attribute access handler [PR94098]
On 3/16/20 4:41 PM, Martin Sebor wrote: The recent fix to avoid modifying in place the type argument in handle_access_attribute (PR 92721) was incomplete and didn't fully resolve the problem (an ICE in the C++ front-end). The attached patch removes the remaining modification that causes the ICE. In addition, the change adjusts checking calls to functions declared with the attribute to scan all its instances. The attached patch was tested on x86_64-linux. attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs); This unchanged line can still modify existing types; I think if attrs already has a matching attribute you need to work harder to replace it. Jason
[PATCH] remove struct == POD coding convention [PR61339]
Last July we agreed to remove the convention that declarations of
POD classes use the class-key struct (and others class). Attached
is a patch to update the GCC Coding Conventions and the rationale
to reflect the decision. I replaced it with a mild suggestion to
use struct for C structs and class for all others, that I Jason
and Jonathan preferred but tried to make it clear it wasn't a hard
and fast rule.
Martin
PR 61339 - mismatch between struct and class [-Wmismatched-tags]
ChangeLog:
* htdocs/codingconventions.html (Struct Definitions): Remove
old convention.
(Class Definitions): Same.
* htdocs/codingrationale.html (Struct Definitions): Document
convention change.
diff --git a/htdocs/codingconventions.html b/htdocs/codingconventions.html
index 03a77063..f4732ef6 100644
--- a/htdocs/codingconventions.html
+++ b/htdocs/codingconventions.html
@@ -888,9 +888,20 @@ Variables may be simultaneously defined and tested in control expressions.
Struct Definitions
-Use the struct keyword for
-https://en.wikipedia.org/wiki/Plain_old_data_structure;>
-plain old data (POD) types.
+Some coding conventions, including GCC's own in the past, recommend
+using the struct keyword (also known as the class-key)
+for https://en.wikipedia.org/wiki/Plain_old_data_structure;>
+plain old data (POD) types. However, since the POD concept has been
+replaced in C++ by a set of much more nuanced distinctions, the current
+guidance (though not a requirement) is to use the struct
+class-key when defining structures that could be used without
+change in C, and use class for all other classes. It is
+recommended to use the same class-key consistently in all
+declarations and, if necessary, in uses of the class.
+The -Wmismatched-tags warning option helps detect mismatches.
+The -Wredundant-tags GCC option further helps identify places
+where the class-key can safely be omitted.
+
@@ -900,14 +911,13 @@ plain old data (POD) types.
Class Definitions
-Use the class keyword for
-https://en.wikipedia.org/wiki/Plain_old_data_structure;>
-non-POD types.
+See the guidance in Struct Definitions for
+the suggested choice of a class-key.
-A non-POD type will often (but not always)
-have a declaration of a
+A class defined with the class-key class type will often
+(but not always) ave a declaration of a
https://en.wikipedia.org/wiki/Special_member_functions;>
special member function.
If any one of these is declared,
diff --git a/htdocs/codingrationale.html b/htdocs/codingrationale.html
index a2618b64..0b44f1da 100644
--- a/htdocs/codingrationale.html
+++ b/htdocs/codingrationale.html
@@ -54,15 +54,16 @@ if (info *q = get_any_available_info ()) {
Struct Definitions
-In the C++ standard,
-structs and classes differ only in the default access rules.
-We prefer to use these two keywords to signal more information.
-
-
-
-Note that under this definition,
-structs may have member functions.
-This freedom is useful for transitional code.
+In the C++ standard, structs and classes differ only in the default
+access rules. In the past, there was a mild preference among some
+GCC developers for using these two keywords to indicate whether or
+not a class met the criteria for a Plain Old Data (POD) type.
+However, this convention was never consistently adhered to or fully
+socialized. A review of a patch to
+https://gcc.gnu.org/pipermail/gcc-patches/2019-July/525526.html;>
+add support for POD struct convention (PR 61339) revealed that
+the convention lacked broad enough support within the GCC developer
+community. As a result, the convention was removed.
[PATCH v3] coroutines: Implement n4849 recommended symmetric transfer.
Nathan Sidwell wrote:
> On 3/24/20 2:08 PM, Iain Sandoe wrote:
>
>>tree suspend = TREE_VEC_ELT (awaiter_calls, 1); /* await_suspend(). */
>> + tree susp_type;
>> + if (tree fndecl = cp_get_callee_fndecl_nofold (suspend))
>> +susp_type = TREE_TYPE (TREE_TYPE (fndecl));
>> + else
>> +susp_type = TREE_TYPE (suspend);
>
> Why, when there's a call of a named function, is it that TREE_TYPE (suspend)
> is insufficient? You mentioned TARGET_EXPR, but why is their type differenty?
it isn’t - the code imported from an earlier impl. was jumping through hoops
not needed here - changed to:
"susp_type = TREE_TYPE (suspend);”
>> @@ -2012,6 +2018,12 @@ build_actor_fn (location_t loc, tree coro_frame_type,
>> tree actor, tree fnbody,
>> = create_named_label_with_ctx (loc, "actor.begin", actor);
>>tree actor_frame = build1_loc (loc, INDIRECT_REF, coro_frame_type,
>> actor_fp);
>> + /* Declare the continuation handle. */
>> + tree ci = build_stmt (loc, DECL_EXPR, continuation);
>> + ci = build1_loc (loc, CONVERT_EXPR, void_type_node, ci);
>> + ci = maybe_cleanup_point_expr_void (ci);
>> + add_stmt (ci);
>
> you don't need to wrap in a CONVERT_EXPR, can you use add_decl_expr ?
ah, thanks, another api entry learned; done.
>> @@ -2368,6 +2383,35 @@ build_actor_fn (location_t loc, tree coro_frame_type,
>> tree actor, tree fnbody,
>
>> +
>> + /* Now we have the actual call, and we can mark it as a tail. */
>> + CALL_EXPR_TAILCALL (resume) = true;
>> + /* ... and for optimisation levels 0..1, mark it as requiring a tail-call
>> + for correctness. It seems that doing this for optimisation levels that
>> + normally perform tail-calling, confuses the ME (or it would be logical
>> + to put this on unilaterally). */
>> + if (optimize < 2)
>> +CALL_EXPR_MUST_TAIL_CALL (resume) = true;
>> + resume = coro_build_cvt_void_expr_stmt (resume, loc);
>
> I'd be happier with
> gcc_checking_assert (maybe_cleanup_point_expr_void (resume) == resume);
> here. I see we can wrap RETURN_EXPRs with cleanups, so perhaps adding is ok
> anyway?
On examining the cleanup code, AFAICT we will only wrap a return expr in a
cleanup if the RHS of the contained MODIFY_EXPR has side-effects. Since the
MODIFY_EXPR is NULL (which it is set to in the preceding line), we should be
able to omit the call to maybe_cleanup_point_expr_void() completely; so now
modified with a checking assert that this is true.
OK for master now?
thanks
Iain
revised patch
coroutines: Implement n4849 recommended symmetric transfer.
Although the note in the text [expr.await] / 5.1.1 is not normative,
it is asserted by users that an implementation that is unable to
perform unlimited symmetric transfers is not terribly useful.
This relates to the following circumstance:
try {
users-function-body:
{
{ some suspend context
continuation_handle = await_suspend (another handle);
continuation_handle.resume ();
'return' (actually a suspension operation).
}
}
} catch (...) {}
The call to 'continuation_handle.resume ()' needs to be a tail-
call in order that an arbitrary number of coroutines can be handled
in this manner. There are two issues with this:
1. That the user's function body is wrapped in a try/catch block and
one cannot tail-call from within those.
2. That GCC doesn't usually produce tail-calls when the optimisation
level is < O2.
After considerable discussion at WG21 meetings, it has been determined
that the intent is that the operation behaves as if the resume call is
executed in the context of the caller.
So, we can remap the fragment above like this:
{
void_coroutine_handle continuation;
try {
users-function-body:
{
{ some suspend context
continuation = await_suspend (another handle);
symmetric_transfer;
}
}
} catch (...) {}
symmetric_transfer:
continuation.resume(); [tail call] [must tail call]
}
Thus we take the call outside the try-catch block which solves
issue (1) and mark it as a tail call and as "must tail call" for
correctness which solves (2).
As bonuses, since we no longer need to differentiate handle types
returned from await_suspend() methods, nor do we need to keep them
in the coroutine frame, since they are ephemeral, we save entries in
the frame and reduce some code too.
gcc/cp/ChangeLog:
2020-03-20 Iain Sandoe
* coroutines.cc (coro_init_identifiers): Initialize an identifier
for the cororoutine handle 'address' method name.
(struct coro_aw_data): Add fields to cover the continuations.
(co_await_expander): Determine the kind of await_suspend in use.
Re: [PATCH v5] c++: DR1710, template keyword in a typename-specifier [PR94057]
On Wed, Mar 25, 2020 at 12:00:24AM -0400, Jason Merrill wrote:
> On 3/24/20 11:45 AM, Marek Polacek wrote:
> > On Mon, Mar 23, 2020 at 10:41:28AM -0400, Jason Merrill wrote:
> > > On 3/20/20 7:02 PM, Marek Polacek wrote:
> > > > On Fri, Mar 20, 2020 at 02:12:49PM -0400, Jason Merrill wrote:
> > > > > On 3/20/20 1:06 PM, Marek Polacek wrote:
> > > > > > Wonderful. I've added a bunch of tests, and some from the related
> > > > > > DRs too.
> > > > > > But I had a problem with the class-or-decltype case: if we have
> > > > > >
> > > > > > template struct D : T::template B::template C
> > > > > > {};
> > > > > >
> > > > > > then we still require all the 'template' keywords here (as does
> > > > > > clang). So I'm
> > > > > > kind of confused, but I don't think it's a big deal right now.
> > > > >
> > > > > This seems related enough that I'd like to fix it at the same time;
> > > > > why
> > > > > doesn't your patch fix it? Is it because typename_p is false?
> > > >
> > > > Ah, I was mistaken. Of course we need the template keyword here: it's
> > > > a member of an
> > > > unknown specialization!
> > >
> > > That's why it's needed in contexts where we don't know whether or not
> > > we're
> > > naming a type. But where we do know that, as in a base-specifier, it
> > > isn't
> > > necessary. This is exactly DR 1710: "The keyword template is optional in
> > > a
> > > ... class-or-decltype (Clause 11.7 [class.derived])"
> >
> > Duh, not sure what I was thinking.
> >
> > I've implemented that in cp_parser_nested_name_specifier_opt: assume
> > 'template'
> > if we've seen 'typename'. If it turns out that this is wrong because it
> > triggers in contexts where it shouldn't, we'll have to introduce something
> > like
> > CP_PARSER_FLAGS_TEMPLATE_OPTIONAL.
> >
> > With this another problem revealed: we weren't accepting an alias template
> > after 'template' which DR1710 says is valid. Fixed by the TYPE_ALIAS_P hunk
> > in the new check_template_keyword_in_nested_name_spec. (alias-decl1.C)
> >
> > But this is still not over: I noticed that
> >
> > template struct S {
> >template
> >using U = TT;
> > };
> > template typename S::template U::type foo;
> >
> > is still rejected and shouldn't be. Here
> > check_template_keyword_in_nested_name_spec
> > gets
> >
> >> align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> > 0x7fffeaa420a8
> > index 0 level 1 orig_level 2
> > chain >
> >
> > Any suggestions how to handle this?
>
> It should work to check for an alias whether or not the target is a
> TYPENAME_TYPE.
Hmm, we don't have a TYPENAME_TYPE here at all, but since that
template_type_parm
is typedef_variant_p, it can be handled along with...
> > +static void
> > +check_template_keyword_in_nested_name_spec (tree name)
> > +{
> > + if (CLASS_TYPE_P (name)
> > + && ((CLASSTYPE_USE_TEMPLATE (name)
> > + && PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (name)))
> > + || CLASSTYPE_IS_TEMPLATE (name)))
> > +return;
> > +
> > +
> > + if (TREE_CODE (name) == TYPENAME_TYPE)
> > +{
> > + if (TREE_CODE (TYPENAME_TYPE_FULLNAME (name)) == TEMPLATE_ID_EXPR)
> > + return;
> > + /* Alias templates are also OK. */
> > + else if (TYPE_ALIAS_P (name))
> > + {
> > + tree tinfo = TYPE_ALIAS_TEMPLATE_INFO (name);
> > + if (tinfo && DECL_ALIAS_TEMPLATE_P (TI_TEMPLATE (tinfo)))
> > + return;
> > + }
> > +}
>
> I think you want to check all typedefs like in e.g. find_parameter_packs_r;
> if the name is a typedef, it's only suitable if
> alias_template_specialization_p.
..this: Since alias_template_specialization_p already checks TYPE_P and
typedef_variant_p, can we simply use that?
> > /* Parse an (optional) nested-name-specifier.
> > nested-name-specifier: [C++98]
> > @@ -6389,7 +6422,17 @@ cp_parser_nested_name_specifier_opt (cp_parser
> > *parser,
> > /* Look for the optional `template' keyword, if this isn't the
> > first time through the loop. */
> > if (success)
> > - template_keyword_p = cp_parser_optional_template_keyword (parser);
> > + {
> > + template_keyword_p = cp_parser_optional_template_keyword (parser);
> > + /* DR1710: "In a qualified-id used as the name in
> > +a typename-specifier, elaborated-type-specifier, using-declaration,
> > +or class-or-decltype, an optional keyword template appearing at
> > +the top level is ignored." */
> > + if (!template_keyword_p
> > + && typename_keyword_p
> > + && cp_parser_nth_token_starts_template_argument_list_p (parser,
> > 2))
> > + template_keyword_p = true;
> > + }
And since we now do this, we don't need...
> > @@ -23636,8 +23696,9 @@ cp_parser_class_name (cp_parser *parser,
> > && decl != error_mark_node
> > && !is_overloaded_fn (decl))
> > {
> > - decl = make_typename_type (scope, decl, typename_type,
> > -
[committed] amdgcn: adjust testsuite
This adjusts the testsuite expectations for amdgcn. This reflects some
of the recent improvements to the backend.
There's one new failure because GCN can't vectorize division natively
(and there's no libgcc routine yet), but there's 24 fails turned to passes.
Andrew
testsuite: adjustments for amdgcn
2020-03-25 Andrew Stubbs
gcc/testsuite/
* gcc.dg/vect/bb-slp-pr69907.c: Disable the dump scan for amdgcn.
* lib/target-supports.exp (check_effective_target_vect_unpack):
Add amdgcn.
diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pr69907.c b/gcc/testsuite/gcc.dg/vect/bb-slp-pr69907.c
index 813b1af089a..fe52d18525a 100644
--- a/gcc/testsuite/gcc.dg/vect/bb-slp-pr69907.c
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pr69907.c
@@ -19,5 +19,6 @@ void foo(unsigned *p1, unsigned short *p2)
/* Disable for SVE because for long or variable-length vectors we don't
get an unrolled epilogue loop. Also disable for AArch64 Advanced SIMD,
- because there we can vectorize the epilogue using mixed vector sizes. */
-/* { dg-final { scan-tree-dump "BB vectorization with gaps at the end of a load is not supported" "slp1" { target { ! aarch64*-*-* } } } } */
+ because there we can vectorize the epilogue using mixed vector sizes.
+ Likewise for AMD GCN. */
+/* { dg-final { scan-tree-dump "BB vectorization with gaps at the end of a load is not supported" "slp1" { target { { ! aarch64*-*-* } && { ! amdgcn*-*-* } } } } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 10353af580a..3654e7bc232 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6717,7 +6717,8 @@ proc check_effective_target_vect_unpack { } {
|| ([istarget arm*-*-*] && [check_effective_target_arm_neon_ok]
&& [check_effective_target_arm_little_endian])
|| ([istarget s390*-*-*]
- && [check_effective_target_s390_vx]) }}]
+ && [check_effective_target_s390_vx])
+ || [istarget amdgcn*-*-*] }}]
}
# Return 1 if the target plus current options does not guarantee
Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)
Ping: Jason, is the latest patch acceptable or are there any other
changes you want me to make?
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542538.html
On 3/21/20 3:59 PM, Martin Sebor wrote:
On 3/20/20 3:53 PM, Jason Merrill wrote:
On 3/19/20 7:55 PM, Martin Sebor wrote:
On 3/18/20 9:07 PM, Jason Merrill wrote:
On 3/12/20 6:38 PM, Martin Sebor wrote:
...
+ declarations of a class from its uses doesn't work for type
aliases
+ (as in using T = class C;). */
Good point. Perhaps we could pass flags to
cp_parser_declares_only_class_p and have it return false if
CP_PARSER_FLAGS_TYPENAME_OPTIONAL, since that is set for an alias
but not for a normal type-specifier.
I wondered if there was a way to identify that we're dealing with
an alias. CP_PARSER_FLAGS_TYPENAME_OPTIONAL is set not just for
those but also for template declarations (in
cp_parser_single_declaration) but I was able to make it work by
tweaking cp_parser_type_specifier. It doesn't feel very clean
(it seems like either the bit or all of cp_parser_flags could be
a member of the parser class or some subobject of it) but it does
the job. Thanks for pointing me in the right direction!
Hmm, true, relying on that flag is probably too fragile. And now that
I look closer, I see that we already have is_declaration in
cp_parser_elaborated_type_specifier, we just need to check that before
cp_parser_declares_only_class_p like we do earlier in the function.
I changed it to use is_declaration instead.
+ if (!decl_p && !def_p && TREE_CODE (decl) == TEMPLATE_DECL)
{
+ /* When TYPE is the use of an implicit specialization of a
previously
+ declared template set TYPE_DECL to the type of the primary
template
+ for the specialization and look it up in CLASS2LOC below.
For uses
+ of explicit or partial specializations TYPE_DECL already
points to
+ the declaration of the specialization. */
+ type_decl = specialization_of (type_decl);
Here shouldn't is_use be true?
If it were set to true here we would find the partial specialization
corresponding to its specialization in the use when what we want is
the latter. As a result, for the following:
template struct S;
template struct S;
extern class S s1; // expect -Wmismatched-tags
extern struct S s2;
we'd end up with a warning for s2 pointing to the instantiation of
s1 as the "guiding declaration:"
z.C:5:15: warning: ‘template struct S’ declared with a
mismatched class-key ‘struct’ [-Wmismatched-tags]
5 | extern struct S s2;
| ^~~
z.C:5:15: note: remove the class-key or replace it with ‘class’
z.C:4:15: note: ‘template struct S’ first declared as
‘class’ here
4 | extern class S s1; // expect -Wmismatched-tags
| ^~~
I found this puzzling and wanted to see why that would be, but I can't
reproduce it; compiling with -Wmismatched-tags produces only
I'm not sure what you did differently. With the patch (the last one
or the one in the attachment) we get the expected warning below.
wa2.C:4:17: warning: ‘S’ declared with a mismatched class-key
‘class’ [-Wmismatched-tags]
4 | extern class S s1; // expect -Wmismatched-tags
| ^~~
wa2.C:4:17: note: remove the class-key or replace it with ‘struct’
wa2.C:2:29: note: ‘S’ first declared as ‘struct’ here
2 | template struct S;
So the only difference is whether we talk about S or S. I
agree that the latter is probably better, which is what you get
without my suggested change. But since specialization_of does nothing
if is_use is false, how about removing the call here and removing the
is_use parameter?
Sure.
+ if (tree spec = most_specialized_partial_spec (ret, tf_none))
+ if (spec != error_mark_node)
+ ret = TREE_VALUE (spec);
I think you want to take the TREE_TYPE of the template here, so you
don't need to do it here:
+ tree pt = specialization_of (TYPE_MAIN_DECL (type), true);
+ if (TREE_CODE (pt) == TEMPLATE_DECL)
+ pt = TREE_TYPE (pt);
+ pt = TYPE_MAIN_DECL (pt);
And also, since it takes a TYPE_DECL, it would be better to return
another TYPE_DECL rather than a _TYPE, especially since the only
caller immediately extracts a TYPE_DECL.
Okay. Attached is an revised patch with these changes.
Martin
Re: [PATCH] avoid modifying type in attribute access handler [PR94098]
Thanks for the suggestion to use type_cache_hasher::equal. I'll
revisit the idea of adding an assertion to decl_attribute in
stage 1 and will look into using it there.
Until then, unless you or anyone else has any concerns with
the original patch I will plan to commit tomorrow:
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542098.html
Martin
On 3/18/20 10:10 PM, Jason Merrill wrote:
On 3/18/20 6:04 PM, Martin Sebor wrote:
On 3/16/20 3:10 PM, Jason Merrill wrote:
On 3/16/20 4:41 PM, Martin Sebor wrote:
The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end). The attached
patch removes the remaining modification that causes the ICE. In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.
The attached patch was tested on x86_64-linux.
I'm puzzled that the ICE only triggers in the C++ front-end and not
also in C. The code that issues the internal error is in comptypes()
in cp/typeck.c and has this comment:
/* The two types are structurally equivalent, but their
canonical types were different. This is a failure of the
canonical type propagation code.*/
internal_error
("canonical types differ for identical types %qT and %qT",
t1, t2);
What is "the canonical type propagation code" it refers to?
Generally, code that makes sure that TYPE_CANONICAL equality is
equivalent to type identity, not any one specific place.
Is it valid to modify the type in an attribute handler
Only if (flags & ATTR_FLAG_TYPE_IN_PLACE).
If not, and if modifying a type in place is not valid I'd expect
decl_attributes to enforce it. I looked for other attribute handlers
to see if any also modify the type argument in place (by adding or
removing attributes) but couldn't really find any. So if it is
invalid I'd like to add such an assertion (probably for GCC 11) but
before I do I want to make sure I'm not missing something.
Generally messing with _ATTRIBUTES happens in decl_attributes:
changing it directly if it's a DECL or ATTR_FLAG_TYPE_IN_PLACE,
otherwise using build_type_attribute_variant. If you need to do it
in your handler, you should follow the same pattern.
That's the conclusion I came to as well, but thanks for confirming
it. With the patch I don't need to make the change but since it's
not obvious that it's a no-no and since it's apparently only detected
under very special conditions I'm wondering is if it's possible to
detect it more reliably than only in C++ comptypes. The trouble is
that I don't exactly what is allowed and what isn't and what to look
for to tell if the handler did something that's not allowed.
The C++ ICE triggered because the redeclared function's type is
considered the same as the original (structural_comptypes()
returns true) but the declarations' canonical types are different.
structural_comptypes() is C++-specific and I don't know what
alternative to call in the middle-end to compare the types and
get the equivalent result.
I think type_cache_hasher::equal is the closest, but it looks like it
doesn't check everything.
PS As a data point, I found just two attribute handlers in
c-attribs.c that modify a type in place: one for attribute const
and the other noreturn. They both do it for function pointers
to set the 'const' and 'noreturn' bits on the pointed to types,
and by calling build_type_variant.
Hmm, yes, that does sound like a bug.
Jason
Re: [PATCH] c++: Fix up user_provided_p [PR81349]
On 3/25/20 3:29 PM, Jakub Jelinek wrote:
Hi!
The standard says: "A function is user-provided if it is user-declared and
not explicitly defaulted or deleted on its first declaration."
I don't see anything about function templates having different rules here,
but user_provided_p does return true for all TEMPLATE_DECLs.
The following patch fixes it by treating as user-provided only templates
that aren't defaulted or deleted.
A template can't be defaulted, but indeed it can be deleted.
So far tested with check-c++-all and check-libstdc++-v3, ok for trunk if it
passes full bootstrap/regtest?
OK.
2020-03-25 Jakub Jelinek
PR c++/81349
* class.c (user_provided_p): Use STRIP_TEMPLATE instead of returning
true for all TEMPLATE_DECLs.
* g++.dg/cpp1z/pr81349.C: New test.
--- gcc/cp/class.c.jj 2020-03-09 21:03:42.345824043 +0100
+++ gcc/cp/class.c 2020-03-25 19:30:27.920029264 +0100
@@ -5159,12 +5159,10 @@ in_class_defaulted_default_constructor (
bool
user_provided_p (tree fn)
{
- if (TREE_CODE (fn) == TEMPLATE_DECL)
-return true;
- else
-return (!DECL_ARTIFICIAL (fn)
- && !(DECL_INITIALIZED_IN_CLASS_P (fn)
-&& (DECL_DEFAULTED_FN (fn) || DECL_DELETED_FN (fn;
+ fn = STRIP_TEMPLATE (fn);
+ return (!DECL_ARTIFICIAL (fn)
+ && !(DECL_INITIALIZED_IN_CLASS_P (fn)
+ && (DECL_DEFAULTED_FN (fn) || DECL_DELETED_FN (fn;
}
/* Returns true iff class T has a user-provided constructor. */
--- gcc/testsuite/g++.dg/cpp1z/pr81349.C.jj 2020-03-25 19:40:16.136248587
+0100
+++ gcc/testsuite/g++.dg/cpp1z/pr81349.C2020-03-25 19:40:37.016936883
+0100
@@ -0,0 +1,29 @@
+// PR c++/81349
+// { dg-do compile { target c++17_only } }
+
+#include
+
+struct A {
+ A (int) = delete;
+};
+
+struct B {
+ template
+ B (T) = delete;
+};
+
+template
+struct C {
+ C (U) = delete;
+};
+
+template
+struct D {
+ template
+ D (T, U) = delete;
+};
+
+static_assert (std::is_aggregate_v);
+static_assert (std::is_aggregate_v);
+static_assert (std::is_aggregate_v>);
+static_assert (std::is_aggregate_v>);
Jakub
[committed] Add missing T register clobber for SH port
Whee, more fallout from the pr90275 changes. Thankfully this time it's a target issue. The sh4/sh4eb ports are failing vector-compare-1 execution tests after the cse changes. They only run once a week, so this wasn't caught immediately and took some time to reproduce and analyze. Ultimately the problem is one define_insn_and_split which, when split, may clobber the T register. However, the define_insn_and_split does not show this in the insn pattern. This can lead to various passes assuming the previous value in the T register is still valid when in fact, it's going to be clobbered. The CSE changes made this scenario more likely and it showed up as execution failures in the sh4/sh4eb testsuite. All the other places where we use sh_split_treg_set_expr have the appropriate clobber of the T register. Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu. Regression testing in progress (won't finish for ~12 hours). No new test as it's covered by vector- compare-1 in the testsuite. Jeff commit eeb0c7c07133634eb5e98ba0348392684a763c95 Author: Jeff Law Date: Wed Mar 25 14:12:32 2020 -0600 Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern clobbering T reg without expressing that in its RTL. PR rtl-optimization/90275 * config/sh/sh.md (mov_neg_si_t): Clobber the T register in the pattern. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 06b06ab68de..3ad7a7aae0d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2020-03-25 Jeff Law + + PR rtl-optimization/90275 + * config/sh/sh.md (mov_neg_si_t): Clobber the T register in the + pattern. + 2020-03-25 Jakub Jelinek PR target/94292
Re: [PATCH], Set -mpcrel by default on Linux 64-bit systems for -mcpu=future
On Wed, Mar 25, 2020 at 11:07:04AM -0500, will schmidt wrote: > Subject: [Patch v327] set -mcprel by default ... > > Include some powerpc or rs6000 reference in the subject. My filters > missed this one. (I'm assuming this is powerpc target specific). rs6000: Enable pcrel by default (etc.) Subjects start with a capital, but end without dot. Just like in normal email. > > This is a revision of a patch that I've submitted several times. It makes > > -mpcrel the default on Linux 64-bit systems that use ELF v2, use the medium > > code mode, and if the user did not disable prefixed load/store instructions > > for > > -mcpu=future. > > The fewer words, the easier to review. History is important but so is > making the review easy. > > "This patch makes -mpcrel the default on Linux 64-bit systems that use > ELF V2, medium code model, and have not disabled prefix instructions." Yes :-) But, don't say "This patch", if you can avoid it... just use an imperative: "Make -mpcrel the default ..." > Is there need or desire to explicitly set TARGET_FUTURE or > TARGET_PREFIXED to zero in the header file now? There are no other > references to those in the header file. It is automatically defined as #define TARGET_FUTURE ((rs6000_isa_flags & OPTION_MASK_FUTURE) != 0) so that will work just fine. Thanks for the review Will. Mike, could you send a patch with those fixes please? Make sure the changelog agrees with the patch (and don't say "why" in changelog -- say that in the commit message. Which is free form, so you have much more freedom to explain things in a useful order). Segher
[PATCH] djgpp: emit "b" flag for named bss sections
Unlike ELF, named sections such as .bss.* and .gnu.linkonce.b.* have no special meaning in COFF, therefore they will have the CONTENTS and LOAD attributes set. The result is that these sections take up space in object files and executables. These attributes can be cleared by emitting the "b" flag in the .section directive. This can probably be added in default_coff_asm_named_section too. gcc/ 2020-03-25 Jan W. Jagersma * config/i386/djgpp.c (i386_djgpp_asm_named_section): Emit "b" flag for SECTION_BSS. --- gcc/config/i386/djgpp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/config/i386/djgpp.c b/gcc/config/i386/djgpp.c index ba6c2d4d3a4..88cf1e6506e 100644 --- a/gcc/config/i386/djgpp.c +++ b/gcc/config/i386/djgpp.c @@ -36,6 +36,8 @@ i386_djgpp_asm_named_section(const char *name, unsigned int flags, *f++ = 'w'; if (flags & SECTION_CODE) *f++ = 'x'; + if (flags & SECTION_BSS) +*f++ = 'b'; /* LTO sections need 1-byte alignment to avoid confusing the zlib decompression algorithm with trailing zero pad bytes. */ -- 2.25.2
[PATCH v2] coroutines: Implement n4849 changes to exception handling.
Hi,
Iain Sandoe wrote:
> This is the first of two remaining changes needed to bring the GCC
> implementation into line with the standard draft at n4849.
Here is a revised version with the “initial await resume called” variable
renamed to be more consistent, as we discussed off-list.
OK for master?
thanks
Iain
coroutines: Implement n4849 changes to exception handling.
The standard now calls up a revised mechanism to handle exceptions
where exceptions thrown by the await_resume () method of the
initial suspend expression are considered in the same manner as
exceptions thrown by the user's function body.
This implements [dcl.fct.def.coroutine] / 5.3.
gcc/cp/ChangeLog:
2020-03-25 Iain Sandoe
* coroutines.cc (co_await_expander): If we are expanding the
initial await expression, set a boolean flag to show that we
have now reached the initial await_resume() method call.
(expand_co_awaits): Handle the 'initial await resume called' flag.
(build_actor_fn): Insert the initial await expression into the
start of the user's function-body. Handle the 'initial await
resume called' flag.
(morph_fn_to_coro): Initialise the 'initial await resume called'
flag. Modify the unhandled exception catch clause to recognise
exceptions that occur before the initial await_resume() and re-
throw them.
gcc/testsuite/ChangeLog:
2020-03-25 Iain Sandoe
* g++.dg/coroutines/torture/exceptions-test-01-n4849-a.C: New test.
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index ef1ce150680..192d4e7faa1 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1348,6 +1348,7 @@ struct coro_aw_data
tree actor_fn; /* Decl for context. */
tree coro_fp;/* Frame pointer var. */
tree resume_idx; /* This is the index var in the frame. */
+ tree i_a_r_c;/* initial suspend await_resume() was called if true. */
tree self_h; /* This is a handle to the current coro (frame var). */
tree cleanup;/* This is where to go once we complete local destroy. */
tree cororet;/* This is where to go if we suspend. */
@@ -1445,6 +1446,8 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void
*d)
tree awaiter_calls = TREE_OPERAND (saved_co_await, 3);
tree source = TREE_OPERAND (saved_co_await, 4);
+ bool is_initial =
+(source && TREE_INT_CST_LOW (source) == (int) INITIAL_SUSPEND_POINT);
bool is_final = (source
&& TREE_INT_CST_LOW (source) == (int) FINAL_SUSPEND_POINT);
bool needs_dtor = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (var));
@@ -1586,6 +1589,16 @@ co_await_expander (tree *stmt, int * /*do_subtree*/,
void *d)
resume_label = build_stmt (loc, LABEL_EXPR, resume_label);
append_to_statement_list (resume_label, _list);
+ if (is_initial)
+{
+ /* Note that we are about to execute the await_resume() for the initial
+await expression. */
+ r = build2_loc (loc, MODIFY_EXPR, boolean_type_node, data->i_a_r_c,
+ boolean_true_node);
+ r = coro_build_cvt_void_expr_stmt (r, loc);
+ append_to_statement_list (r, _list);
+}
+
/* This will produce the value (if one is provided) from the co_await
expression. */
tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume(). */
@@ -1634,10 +1647,10 @@ co_await_expander (tree *stmt, int * /*do_subtree*/,
void *d)
static tree
expand_co_awaits (tree fn, tree *fnbody, tree coro_fp, tree resume_idx,
- tree cleanup, tree cororet, tree self_h)
+ tree i_a_r_c, tree cleanup, tree cororet, tree self_h)
{
coro_aw_data data
-= {fn, coro_fp, resume_idx, self_h, cleanup, cororet, 2};
+= {fn, coro_fp, resume_idx, i_a_r_c, self_h, cleanup, cororet, 2};
cp_walk_tree (fnbody, co_await_expander, , NULL);
return *fnbody;
}
@@ -2158,8 +2171,7 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* Get a reference to the initial suspend var in the frame. */
transform_await_expr (initial_await, );
- r = coro_build_expr_stmt (initial_await, loc);
- add_stmt (r);
+ tree initial_await_stmt = coro_build_expr_stmt (initial_await, loc);
/* co_return branches to the final_suspend label, so declare that now. */
tree fs_label = create_named_label_with_ctx (loc, "final.suspend", actor);
@@ -2167,6 +2179,34 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* Expand co_returns in the saved function body */
fnbody = expand_co_returns (, promise_proxy, ap, fs_label);
+ /* n4849 adds specific behaviour to treat exceptions thrown by the
+ await_resume () of the initial suspend expression. In order to
+ implement this, we need to treat the initial_suspend expression
+ as if it were part
[PATCH] c++: Fix up user_provided_p [PR81349]
Hi!
The standard says: "A function is user-provided if it is user-declared and
not explicitly defaulted or deleted on its first declaration."
I don't see anything about function templates having different rules here,
but user_provided_p does return true for all TEMPLATE_DECLs.
The following patch fixes it by treating as user-provided only templates
that aren't defaulted or deleted.
So far tested with check-c++-all and check-libstdc++-v3, ok for trunk if it
passes full bootstrap/regtest?
2020-03-25 Jakub Jelinek
PR c++/81349
* class.c (user_provided_p): Use STRIP_TEMPLATE instead of returning
true for all TEMPLATE_DECLs.
* g++.dg/cpp1z/pr81349.C: New test.
--- gcc/cp/class.c.jj 2020-03-09 21:03:42.345824043 +0100
+++ gcc/cp/class.c 2020-03-25 19:30:27.920029264 +0100
@@ -5159,12 +5159,10 @@ in_class_defaulted_default_constructor (
bool
user_provided_p (tree fn)
{
- if (TREE_CODE (fn) == TEMPLATE_DECL)
-return true;
- else
-return (!DECL_ARTIFICIAL (fn)
- && !(DECL_INITIALIZED_IN_CLASS_P (fn)
-&& (DECL_DEFAULTED_FN (fn) || DECL_DELETED_FN (fn;
+ fn = STRIP_TEMPLATE (fn);
+ return (!DECL_ARTIFICIAL (fn)
+ && !(DECL_INITIALIZED_IN_CLASS_P (fn)
+ && (DECL_DEFAULTED_FN (fn) || DECL_DELETED_FN (fn;
}
/* Returns true iff class T has a user-provided constructor. */
--- gcc/testsuite/g++.dg/cpp1z/pr81349.C.jj 2020-03-25 19:40:16.136248587
+0100
+++ gcc/testsuite/g++.dg/cpp1z/pr81349.C2020-03-25 19:40:37.016936883
+0100
@@ -0,0 +1,29 @@
+// PR c++/81349
+// { dg-do compile { target c++17_only } }
+
+#include
+
+struct A {
+ A (int) = delete;
+};
+
+struct B {
+ template
+ B (T) = delete;
+};
+
+template
+struct C {
+ C (U) = delete;
+};
+
+template
+struct D {
+ template
+ D (T, U) = delete;
+};
+
+static_assert (std::is_aggregate_v);
+static_assert (std::is_aggregate_v);
+static_assert (std::is_aggregate_v>);
+static_assert (std::is_aggregate_v>);
Jakub
Re: [PATCH] c++: Fix a -fcompare-debug issue with DEBUG_BEGIN_STMT stmts in STATEMENT_LISTs [PR94272]
On 3/24/20 3:58 AM, Jakub Jelinek wrote:
+ for (i = tsi_start (stmt); !tsi_end_p (i); tsi_next ())
+ {
+ tree t = tsi_stmt (i);
+ if (TREE_CODE (t) != DEBUG_BEGIN_STMT && nondebug_stmts < 2)
+ nondebug_stmts++;
+ cp_walk_tree (tsi_stmt_ptr (i), cp_genericize_r, data, NULL);
+ if (TREE_CODE (t) != DEBUG_BEGIN_STMT
+ && nondebug_stmts == 1
+ && TREE_SIDE_EFFECTS (tsi_stmt (i)))
How about (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))) here...
+ clear_side_effects = false;
+ }
+ if (nondebug_stmts < 2 && clear_side_effects)
So we don't need to look at nondebug_stmts here? OK with that change.
Jason
[Patch, fortran] PR fortran/94327 and PR fortran/94331 Bind(C) problems
Hi all!
Proposed patch to:
Bug 94327 - Bind(c) argument attributes are incorrectly set
and to:
Bug 94331 - Bind(C) corrupts array descriptors
Patch tested only on x86_64-pc-linux-gnu.
Sorry for the double patch but applying them separately would break things.
Fixing 94327 is simple, just fix the if clause assigning cfi_attribute
so that it will always have the attribute of the dummy argument not,
sometimes, the attribute of the effective argument.
The array descriptor corruption is caused by the overwriting of the GFC
array descriptor, on exit, with the internal bounds of the CFI
descriptor which will be different, if the attribute is CFI_attribute_other.
This conversion is AFAICT unnecessary if the dummy argument has the
CFI_attribute_other or the value attributes set or if the intent is in.
Any other case I might have forgotten?
The conversion procedures where adjusted so that on output, for
attribute CFI_attribute_other, the lower bound is set to 1 not 0 and on
input so that arrays are only marked as assumed-size if the attribute is
also CFI_attribute_other.
The ISO_Fortran_binding_1.f90 test c_establish procedure is somewhat
problematic, passing a dissociated pointer was clearly undefined
behavior, and I believe that the way CFI_establish is used and the
allocations are done is not kosher either.
Some of the tests are disabled because of PR93957 and PR94289, I have
previously posted a patch to PR93957.
Thank you very much.
Best regards,
José Rui
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* trans-decl.c (convert_CFI_desc): Only overwrite the array descriptor
if the dummy argument has the pointer or allocatable attribute set and
not if it has the value attribute set or if it is intent in.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94327
* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Change if clause in
order to set the dummy argiment's attribute to the correct value and
remove obsolete comment.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Only overwrite the
array descriptor if the dummy argument's attribute is
CFI_attribute_other and if it has not the value attribute set or if it
is intent in.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Set the array
descriptor lower bound to 1 if the attribute is CFI_attribute_other.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* ISO_Fortran_binding.c (gfc_desc_to_cfi_desc): Only mark the CFI
descriptor as assumed-size if the attribute is CFI_attribute_other.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94327
* ISO_Fortran_binding_1.f90: Add pointer attribute to c_establish
argument in the interface.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* bind_c_array_params_2.f90: Remove test for code that is no longer
emitted.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94327
* PR94327.f90: New test.
* PR94327.c: Additional source.
2020-3-25 José Rui Faustino de Sousa
PR fortran/94331
* PR94331.f90: New test.
* PR94331.c: Additional source.
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index e91a279..88e762a 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -4472,19 +4472,26 @@ convert_CFI_desc (gfc_wrapped_block * block,
gfc_symbol *sym)
/* Convert the gfc descriptor back to the CFI type before going
out of scope, if the CFI type was present at entry. */
- gfc_init_block (_block);
- gfc_init_block ();
+ outgoing = NULL_TREE;
+ if ((sym->attr.pointer
+ || sym->attr.allocatable)
+ && !sym->attr.value
+ && sym->attr.intent != INTENT_IN)
+ {
+ gfc_init_block (_block);
+ gfc_init_block ();
- tmp = gfc_build_addr_expr (ppvoid_type_node, CFI_desc_ptr);
- outgoing = build_call_expr_loc (input_location,
- gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
- gfc_add_expr_to_block (, outgoing);
+ tmp = gfc_build_addr_expr (ppvoid_type_node, CFI_desc_ptr);
+ outgoing = build_call_expr_loc (input_location,
+ gfor_fndecl_gfc_to_cfi, 2, tmp,
gfc_desc_ptr);
+ gfc_add_expr_to_block (, outgoing);
- outgoing = build3_v (COND_EXPR, present,
- gfc_finish_block (),
- build_empty_stmt (input_location));
- gfc_add_expr_to_block (_block, outgoing);
- outgoing = gfc_finish_block (_block);
+ outgoing = build3_v (COND_EXPR, present,
+ gfc_finish_block (),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (_block, outgoing);
+ outgoing = gfc_finish_block (_block);
+ }
/* Add the lot to the procedure init and finally blocks. */
Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]
On 3/25/20 12:17 PM, Patrick Palka wrote:
This PR reports that the requires-expression in
auto f = [] { };
static_assert(requires { f(); });
erroneously evaluates to false. The way we end up evaluating to false goes as
follows. During the parsing of the requires-expression, we call
finish_call_expr from cp_parser_requires_expression with the arguments
fn = VIEW_CONVERT_EXPR(f);
args = {}
which does the full processing of the call (since we're not inside a template)
and returns
::operator() ();
Later, when evaluating the requires-expression, we call finish_call_expr again,
this time from tsubst_expr from satisfy_atom, with the arguments
fn = operator()
args = { }
(which, as expected, correspond to the CALL_EXPR returned by finish_call_expr
the first time). But this time, finish_call_expr returns error_mark_node
because
it doesn't expect to see an explicit 'this' argument in the args array, treating
it instead as a user-written argument which causes the only candidate function
to be discarded. This causes the requires-expression to evaluate to false.
In short, it seems finish_call_expr is not idempotent on certain inputs when
!processing_template_decl. Assuming this idempotency issue is not specific to
finish_call_expr, it seems that the safest thing to do is to avoid doing full
semantic processing twice when parsing and evaluating a requires-expression that
lives outside of a template definition.
Absolutely. We shouldn't call tsubst_expr on non-template trees.
This patch achieves this by temporarily setting processing_template_decl to
non-zero when parsing the body of a requires-expression. This way, full
semantic processing will always be done only during evaluation and not during
parsing.
Hmm, interesting approach, but I think the standard requires us to treat
requires-expressions outside of template context like normal
non-template code: "[Note: If a requires-expression contains invalid
types or expressions in its requirements, and it does not appear within
the declaration of a templated entity, then the program is ill-formed.
--end note]"
So I think better to avoid the tsubst_expr later, either by immediately
evaluating the REQUIRES_EXPR or marking the ATOMIC_CONSTR. We could do
that by immediately resolving the requires-expression in non-template
context, or by marking it up somehow to prevent the substitution.
I notice that we currently fail to handle requires-expressions in
regular expression context:
int main() { return requires { 42; }; } // ICE
Testing is in progress. Does this look like the right solution?
gcc/cp/ChangeLog:
PR c++/94252
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily setting
processing_template_decl to non-zero.
* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a REQUIRES_EXPR.
gcc/testsuite/ChangeLog:
PR c++/94252
* g++.dg/concepts/diagnostic7.C: New test.
* g++.dg/concepts/pr94252.C: New test.
---
gcc/cp/parser.c | 7 ++-
gcc/cp/semantics.c | 6 --
gcc/testsuite/g++.dg/concepts/diagnostic7.C | 12
gcc/testsuite/g++.dg/concepts/pr94252.C | 14 ++
4 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic7.C
create mode 100644 gcc/testsuite/g++.dg/concepts/pr94252.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 03ecd7838f6..14a22b85318 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser)
else
parms = NULL_TREE;
-/* Parse the requirement body. */
+/* Always parse the requirement body as if we're inside a template so that
+ we always do full semantic processing only during evaluation of this
+ requires-expression and not also now during parsing. */
+processing_template_decl_sentinel ptds;
+if (!processing_template_decl)
+ processing_template_decl = 1;
reqs = cp_parser_requirement_body (parser);
if (reqs == error_mark_node)
return error_mark_node;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index bcb2e72fbb5..e998b373af4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9691,8 +9691,10 @@ finish_static_assert (tree condition, tree message,
location_t location,
error ("static assertion failed: %s",
TREE_STRING_POINTER (message));
- /* Actually explain the failure if this is a concept check. */
- if (concept_check_p (orig_condition))
+ /* Actually explain the failure if this is a concept check or a
+requires-expression. */
+ if (concept_check_p (orig_condition)
+ || TREE_CODE (orig_condition) == REQUIRES_EXPR)
[GCC][ARM][PATCH]: Add support for MVE ACLE intrinsics polymorphic variants for +mve.fp option.
Hello, For the following MVE ACLE intrinsics, polymorphic variant supports only +mve option, support for +mve.fp is missing. vabavq_p_s16, vabavq_p_s32, vabavq_p_s8, vabavq_p_u16, vabavq_p_u32, vabavq_p_u8, vabavq_s16, vabavq_s32, vabavq_s8, vabavq_u16, vabavq_u32, vabavq_u8, vaddlvaq_p_s32, vaddlvaq_p_u32, vaddlvaq_s32, vaddlvaq_u32, vaddlvq_p_s32, vaddlvq_p_u32, vaddlvq_u32, vaddvaq_p_s16, vaddvaq_p_s32, vaddvaq_p_s8, vaddvaq_p_u16, vaddvaq_p_u32, vaddvaq_p_u8, vaddvaq_s16, vaddvaq_s32, vaddvaq_s8, vaddvaq_u16, vaddvaq_u32, vaddvaq_u8, vaddvq_p_s16, vaddvq_p_s32, vaddvq_p_s8, vaddvq_p_u16, vaddvq_p_u32, vaddvq_p_u8, vaddvq_s8, vaddvq_u16, vaddvq_u32, vaddvq_u8, vcmpcsq_m_n_u16, vcmpcsq_m_n_u32, vcmpcsq_m_n_u8, vcmpcsq_m_u16, vcmpcsq_m_u32, vcmpcsq_m_u8, vcmpcsq_n_u16, vcmpcsq_n_u32, vcmpcsq_n_u8, vcmpcsq_u16, vcmpcsq_u32, vcmpcsq_u8, vcmpeqq_n_f16, vcmpeqq_n_f32, vcmpgeq_m_n_s16, vcmpgeq_m_n_s32, vcmpgeq_m_n_s8, vcmpgtq_m_n_f16, vcmpgtq_m_n_f32, vcmpgtq_n_f16, vcmpgtq_n_f32, vcmphiq_m_n_u16, vcmphiq_m_n_u32, vcmphiq_m_n_u8, vcmphiq_m_u16, vcmphiq_m_u32, vcmphiq_m_u8, vcmphiq_n_u16, vcmphiq_n_u32, vcmphiq_n_u8, vcmphiq_u16, vcmphiq_u32, vcmphiq_u8, vcmpleq_m_n_f16, vcmpleq_m_n_f32, vcmpleq_n_f16, vcmpleq_n_f32, vcmpltq_m_n_f16, vcmpltq_m_n_f32, vcmpneq_m_n_f16, vcmpneq_m_n_f32, vcmpneq_n_f16, vcmpneq_n_f32, vmaxavq_p_s16, vmaxavq_p_s32, vmaxavq_p_s8, vmaxavq_s16, vmaxavq_s32, vmaxavq_s8, vmaxq_x_s16, vmaxq_x_s32, vmaxq_x_s8, vmaxq_x_u16, vmaxq_x_u32, vmaxq_x_u8, vmaxvq_p_s16, vmaxvq_p_s32, vmaxvq_p_s8, vmaxvq_p_u16, vmaxvq_p_u32, vmaxvq_p_u8, vmaxvq_s16, vmaxvq_s32, vmaxvq_s8, vmaxvq_u16, vmaxvq_u32, vmaxvq_u8, vminavq_p_s16, vminavq_p_s32, vminavq_p_s8, vminavq_s16, vminavq_s32, vminavq_s8, vminq_x_s16, vminq_x_s32, vminq_x_s8, vminq_x_u16, vminq_x_u32, vminq_x_u8, vminvq_p_s16, vminvq_p_s32, vminvq_p_s8, vminvq_p_u16, vminvq_p_u32, vminvq_p_u8, vminvq_s16, vminvq_s32, vminvq_s8, vminvq_u16, vminvq_u32, vminvq_u8, vmladavaq_p_s16, vmladavaq_p_s32, vmladavaq_p_s8, vmladavaq_p_u16, vmladavaq_p_u32, vmladavaq_p_u8, vmladavaq_s16, vmladavaq_s32, vmladavaq_s8, vmladavaq_u16, vmladavaq_u32, vmladavaq_u8, vmladavaxq_s16, vmladavaxq_s32, vmladavaxq_s8, vmladavq_p_s16, vmladavq_p_s32, vmladavq_p_s8, vmladavq_p_u16, vmladavq_p_u32, vmladavq_p_u8, vmladavq_s16, vmladavq_s32, vmladavq_s8, vmladavq_u16, vmladavq_u32, vmladavq_u8, vmladavxq_p_s16, vmladavxq_p_s32, vmladavxq_p_s8, vmladavxq_s16, vmladavxq_s32, vmladavxq_s8, vmlaldavaq_s16, vmlaldavaq_s32, vmlaldavaq_u16, vmlaldavaq_u32, vmlaldavaxq_s16, vmlaldavaxq_s32, vmlaldavq_p_s16, vmlaldavq_p_s32, vmlaldavq_p_u16, vmlaldavq_p_u32, vmlaldavq_s16, vmlaldavq_s32, vmlaldavq_u16, vmlaldavq_u32, vmlaldavxq_p_s16, vmlaldavxq_p_s32, vmlsdavaq_s16, vmlsdavaq_s32, vmlsdavaq_s8, vmlsdavaxq_s16, vmlsdavaxq_s32, vmlsdavaxq_s8, vmlsdavq_p_s16, vmlsdavq_p_s32, vmlsdavq_p_s8, vmlsdavq_s16, vmlsdavq_s32, vmlsdavq_s8, vmlsdavxq_p_s16, vmlsdavxq_p_s32, vmlsdavxq_p_s8, vmlsdavxq_s16, vmlsdavxq_s32, vmlsdavxq_s8, vmlsldavaq_s16, vmlsldavaq_s32, vmlsldavaxq_s16, vmlsldavaxq_s32, vmlsldavq_p_s16, vmlsldavq_p_s32, vmlsldavq_s16, vmlsldavq_s32, vmlsldavxq_p_s16, vmlsldavxq_p_s32, vmlsldavxq_s16, vmlsldavxq_s32, vmovlbq_x_s16, vmovlbq_x_s8, vmovlbq_x_u16, vmovlbq_x_u8, vmovltq_x_s16, vmovltq_x_s8, vmovltq_x_u16, vmovltq_x_u8, vmulhq_x_s16, vmulhq_x_s32, vmulhq_x_s8, vmulhq_x_u16, vmulhq_x_u32, vmulhq_x_u8, vmullbq_int_x_s16, vmullbq_int_x_s32, vmullbq_int_x_s8, vmullbq_int_x_u16, vmullbq_int_x_u32, vmullbq_int_x_u8, vmullbq_poly_x_p16, vmullbq_poly_x_p8, vmulltq_int_x_s16, vmulltq_int_x_s32, vmulltq_int_x_s8, vmulltq_int_x_u16, vmulltq_int_x_u32, vmulltq_int_x_u8, vmulltq_poly_x_p16, vmulltq_poly_x_p8, vrmlaldavhaq_s32, vrmlaldavhaq_u32, vrmlaldavhaxq_s32, vrmlaldavhq_p_s32, vrmlaldavhq_p_u32, vrmlaldavhq_s32, vrmlaldavhq_u32, vrmlaldavhxq_p_s32, vrmlaldavhxq_s32, vrmlsldavhaq_s32, vrmlsldavhaxq_s32, vrmlsldavhq_p_s32, vrmlsldavhq_s32, vrmlsldavhxq_p_s32, vrmlsldavhxq_s32, vstrbq_p_s16, vstrbq_p_s32, vstrbq_p_s8, vstrbq_p_u16, vstrbq_p_u32, vstrbq_p_u8, vstrbq_s16, vstrbq_s32, vstrbq_s8, vstrbq_scatter_offset_p_s16, vstrbq_scatter_offset_p_s32, vstrbq_scatter_offset_p_s8, vstrbq_scatter_offset_p_u16, vstrbq_scatter_offset_p_u32, vstrbq_scatter_offset_p_u8, vstrbq_scatter_offset_s16, vstrbq_scatter_offset_s32, vstrbq_scatter_offset_s8, vstrbq_scatter_offset_u16, vstrbq_scatter_offset_u32, vstrbq_scatter_offset_u8, vstrbq_u16, vstrbq_u32, vstrbq_u8, vstrdq_scatter_base_p_s64, vstrdq_scatter_base_p_u64, vstrdq_scatter_base_s64, vstrdq_scatter_base_u64, vstrdq_scatter_offset_p_s64, vstrdq_scatter_offset_p_u64, vstrdq_scatter_offset_s64, vstrdq_scatter_offset_u64, vstrdq_scatter_shifted_offset_p_s64, vstrdq_scatter_shifted_offset_p_u64, vstrdq_scatter_shifted_offset_s64, vstrdq_scatter_shifted_offset_u64. This patch adds the support for MVE ACLE intrinsics polymorphic variants with +mve.fp option. Please refer to M-profile Vector Extension (MVE)
Re: [PATCH] c++: Remove redundant calls to type_dependent_expression_p
On 3/25/20 8:52 AM, Patrick Palka wrote:
This simplifies conditions that test both value_dependent_expression_p and
type_dependent_expression_p, since the former predicate now subsumes the latter.
Tested on x86_64-pc-linux-gnu, does this look OK?
I'm still a bit uneasy about testing value-dependence for a non-constant
expression, but I suppose that feeling is obsolete. OK.
gcc/cp/ChangeLog:
* decl.c (compute_array_index_type_loc): Remove redundant
type_dependent_expression_p check that is subsumed by
value_dependent_expression_p.
* decl2.c (is_late_template_attribute): Likewise.
* pt.c (uses_template_parms): Likewise.
(dependent_template_arg_p): Likewise.
---
gcc/cp/decl.c | 3 +--
gcc/cp/decl2.c | 3 +--
gcc/cp/pt.c| 6 ++
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 319b7ee5c1c..69a238997b4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10338,8 +10338,7 @@ compute_array_index_type_loc (location_t name_loc, tree
name, tree size,
/* We can only call value_dependent_expression_p on integral constant
expressions; treat non-constant expressions as dependent, too. */
if (processing_template_decl
- && (type_dependent_expression_p (size)
- || !TREE_CONSTANT (size) || value_dependent_expression_p (size)))
+ && (!TREE_CONSTANT (size) || value_dependent_expression_p (size)))
{
/* We cannot do any checking for a SIZE that isn't known to be
constant. Just build the index type and mark that it requires
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 2efb2e54f37..6cf72b432e2 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1191,8 +1191,7 @@ is_late_template_attribute (tree attr, tree decl)
&& identifier_p (t))
continue;
- if (value_dependent_expression_p (t)
- || type_dependent_expression_p (t))
+ if (value_dependent_expression_p (t))
return true;
}
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b82c51a3042..eaa84f43dcd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10525,8 +10525,7 @@ uses_template_parms (tree t)
else if (t == error_mark_node)
dependent_p = false;
else
-dependent_p = (type_dependent_expression_p (t)
- || value_dependent_expression_p (t));
+dependent_p = value_dependent_expression_p (t);
processing_template_decl = saved_processing_template_decl;
@@ -27030,8 +27029,7 @@ dependent_template_arg_p (tree arg)
else if (TYPE_P (arg))
return dependent_type_p (arg);
else
-return (type_dependent_expression_p (arg)
- || value_dependent_expression_p (arg));
+return value_dependent_expression_p (arg);
}
/* Returns true if ARGS (a collection of template arguments) contains
Re: [PATCH] arm: Fix ICE caused by arm_gen_discompare_reg [PR94292]
On 25/03/2020 07:21, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> The following testcase ICEs, because arm_gen_discompare_reg creates invalid
> RTL which then propagates into DEBUG_INSNs and ICEs while handling them.
> The problem is that this function emits
> (insn 18 17 19 2 (set (reg:CC_DNE 100 cc)
> (compare (ior:SI (ne:SI (subreg:SI (reg:DI 129) 0)
> (subreg:SI (reg:DI 114 [ _2 ]) 0))
> (ne:SI (subreg:SI (reg:DI 129) 4)
> (subreg:SI (reg:DI 114 [ _2 ]) 4)))
> (const_int 0 [0]))) "pr94292.c":7:11 325 {*cmp_ior}
> (nil))
> and the invalid thing is that the COMPARE has VOIDmode. Setting a
> non-VOIDmode SET_DEST to VOIDmode SET_SRC is only valid if the SET_SRC is
> CONST_INT/CONST_DOUBLE.
> The following patch fixes it by giving the COMPARE the same mode as it gives
> to the SET_DEST cc register.
Ooops!
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>
> 2020-03-25 Jakub Jelinek
>
> PR target/94292
> * config/arm/arm.c (arm_gen_discompare_reg): Set mode of COMPARE to
> mode rather than VOIDmode.
>
It's arm_gen_dicompare_reg (no 's'). Similarly in the summary line and
elsewhere in the description.
> * gcc.dg/pr94292.c: New test.
Otherwise, OK.
R.
>
> --- gcc/config/arm/arm.c.jj 2020-03-18 19:25:33.0 +0100
> +++ gcc/config/arm/arm.c 2020-03-24 13:14:38.568689174 +0100
> @@ -15763,7 +15763,7 @@ arm_gen_dicompare_reg (rtx_code code, rt
> cc_reg = gen_rtx_REG (mode, CC_REGNUM);
>
> emit_insn (gen_rtx_SET (cc_reg,
> - gen_rtx_COMPARE (VOIDmode, conjunction,
> + gen_rtx_COMPARE (mode, conjunction,
>const0_rtx)));
> return cc_reg;
>}
> --- gcc/testsuite/gcc.dg/pr94292.c.jj 2020-03-24 13:07:12.694449518 +0100
> +++ gcc/testsuite/gcc.dg/pr94292.c2020-03-24 13:06:53.720737198 +0100
> @@ -0,0 +1,13 @@
> +/* PR target/94292 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -g -fno-tree-dce" } */
> +
> +unsigned short a;
> +unsigned long long b;
> +
> +long long
> +foo (int d)
> +{
> + d >>= a != (unsigned long long) -a;
> + return a + b;
> +}
>
> Jakub
>
Re: [PATCH v3 4/4] libgomp/test: Remove a build sysroot fix regression
On 2020/2/28 9:18 AM, Maciej W. Rozycki wrote:
Fix a problem with commit c8e759b4215b ("libgomp/test: Fix compilation
for build sysroot") that caused a regression in some standalone test
environments where testsuite/libgomp-test-support.exp is used, but the
compiler is expected to be determined by `[find_gcc]', and set the
GCC_UNDER_TEST TCL variable in testsuite/libgomp-site-extra.exp instead.
libgomp/
* configure.ac: Add testsuite/libgomp-site-extra.exp to output
files.
* configure: Regenerate.
* testsuite/libgomp-site-extra.exp.in: New file.
* testsuite/libgomp-test-support.exp.in (GCC_UNDER_TEST): Remove
variable.
* testsuite/Makefile.am (EXTRA_DEJAGNU_SITE_CONFIG): New
variable.
* testsuite/Makefile.in: Regenerate.
---
Changes from v2:
- Do not use `--tool_exec' with AM_RUNTESTFLAGS.
- Move the definition of GCC_UNDER_TEST from
testsuite/libgomp-test-support.exp to
testsuite/libgomp-site-extra.exp.
Hi Maciej,
sorry, I didn't notice you were blocked on input from us.
I tested our testing with this patch and can confirm it works for us; didn't
notice
any breakage.
Chung-Lin
Applies on top of v1.
---
libgomp/configure |3 +++
libgomp/configure.ac |1 +
libgomp/testsuite/Makefile.am |2 ++
libgomp/testsuite/Makefile.in |6 +-
libgomp/testsuite/libgomp-site-extra.exp.in |1 +
libgomp/testsuite/libgomp-test-support.exp.in |2 --
6 files changed, 12 insertions(+), 3 deletions(-)
gcc-test-libgomp-site-extra.diff
Index: gcc/libgomp/configure
===
--- gcc.orig/libgomp/configure
+++ gcc/libgomp/configure
@@ -17047,6 +17047,8 @@ ac_config_files="$ac_config_files Makefi
ac_config_files="$ac_config_files testsuite/libgomp-test-support.pt.exp:testsuite/libgomp-test-support.exp.in"
+ac_config_files="$ac_config_files testsuite/libgomp-site-extra.exp"
+
cat >confcache <<\_ACEOF
# This file is a shell script that caches the results of configure
# tests run on this system so they can be shared between configure
@@ -18200,6 +18202,7 @@ do
"testsuite/Makefile") CONFIG_FILES="$CONFIG_FILES testsuite/Makefile" ;;
"libgomp.spec") CONFIG_FILES="$CONFIG_FILES libgomp.spec" ;;
"testsuite/libgomp-test-support.pt.exp") CONFIG_FILES="$CONFIG_FILES
testsuite/libgomp-test-support.pt.exp:testsuite/libgomp-test-support.exp.in" ;;
+"testsuite/libgomp-site-extra.exp") CONFIG_FILES="$CONFIG_FILES
testsuite/libgomp-site-extra.exp" ;;
*) as_fn_error $? "invalid argument: \`$ac_config_target'" "$LINENO" 5;;
esac
Index: gcc/libgomp/configure.ac
===
--- gcc.orig/libgomp/configure.ac
+++ gcc/libgomp/configure.ac
@@ -436,4 +436,5 @@ GCC_BASE_VER
AC_CONFIG_FILES(omp.h omp_lib.h omp_lib.f90 libgomp_f.h)
AC_CONFIG_FILES(Makefile testsuite/Makefile libgomp.spec)
AC_CONFIG_FILES([testsuite/libgomp-test-support.pt.exp:testsuite/libgomp-test-support.exp.in])
+AC_CONFIG_FILES([testsuite/libgomp-site-extra.exp])
AC_OUTPUT
Index: gcc/libgomp/testsuite/Makefile.am
===
--- gcc.orig/libgomp/testsuite/Makefile.am
+++ gcc/libgomp/testsuite/Makefile.am
@@ -12,6 +12,8 @@ _RUNTEST = $(shell if test -f $(top_srcd
echo $(top_srcdir)/../dejagnu/runtest; else echo runtest; fi)
RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir $$srcdir
+EXTRA_DEJAGNU_SITE_CONFIG = libgomp-site-extra.exp
+
# Instead of directly in ../testsuite/libgomp-test-support.exp.in, the
# following variables have to be "routed through" this Makefile, for expansion
# of the several (Makefile) variables used therein.
Index: gcc/libgomp/testsuite/Makefile.in
===
--- gcc.orig/libgomp/testsuite/Makefile.in
+++ gcc/libgomp/testsuite/Makefile.in
@@ -111,7 +111,8 @@ am__configure_deps = $(am__aclocal_m4_de
DIST_COMMON = $(srcdir)/Makefile.am
mkinstalldirs = $(SHELL) $(top_srcdir)/../mkinstalldirs
CONFIG_HEADER = $(top_builddir)/config.h
-CONFIG_CLEAN_FILES = libgomp-test-support.pt.exp
+CONFIG_CLEAN_FILES = libgomp-test-support.pt.exp \
+ libgomp-site-extra.exp
CONFIG_CLEAN_VPATH_FILES =
AM_V_P = $(am__v_P_@AM_V@)
am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
@@ -310,6 +311,7 @@ _RUNTEST = $(shell if test -f $(top_srcd
echo $(top_srcdir)/../dejagnu/runtest; else echo runtest; fi)
RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir $$srcdir
+EXTRA_DEJAGNU_SITE_CONFIG = libgomp-site-extra.exp
all: all-am
.SUFFIXES:
@@ -344,6 +346,8 @@ $(ACLOCAL_M4): @MAINTAINER_MODE_TRUE@ $(
$(am__aclocal_m4_deps):
libgomp-test-support.pt.exp: $(top_builddir)/config.status
$(srcdir)/libgomp-test-support.exp.in
cd $(top_builddir) &&
[og9] Fix og9 "Fix hang when running oacc exec with CUDA 9.0 nvprof"
Hi!
On 2018-02-22T12:23:25+0100, Tom de Vries wrote:
> when using cuda 9 nvprof with an openacc executable, the executable hangs.
>
> The scenario resulting in the hang is as follows:
> 1. goacc_lazy_initialize calls gomp_mutex_lock (_device_lock)
> 2. goacc_lazy_initialize calls acc_init_1
> 3. acc_init_1 calls goacc_profiling_dispatch (_info,
> _init_event_info, _info);
> 4. goacc_profiling_dispatch calls the registered callback in the cuda
> profiling library
> 5. the registered call back calls acc_get_device_type
> 6. acc_get_device_type calls gomp_mutex_lock (_device_lock)
> 7. The lock is not recursive, so we have deadlock
>
> The registered callback in cuda 8 does not call acc_get_device_type, so
> the hang doesn't occur there.
(ACK for the general problem description/analysis.)
> This patch fixes the hang by detecting in acc_get_device_type that the
> calling thread is a thread that is currently initializing the openacc
> part of the libgomp library, and returning acc_device_none, which is a
> legal value given that the openacc standard states "If the device type
> has not yet been selected, the value acc_device_none may be returned".
(This specific way of resolving the issue I still have to look into.
This may need a more general solution, to make all such libgomp OpenACC
entry points re-entrant.)
> Committed to og7 branch.
What Frederik has discovered today in the hard way... is that the og9
version of this patch did get its code altered in a way so that it no
longer resolves the problem it's meant to resolve -- the hang was back.
On Git-mirror-based openacc-gcc-9-branch that's:
commit 84af3c5a2fbb5023057e2ca319b0c22f5f7d4795
Author: Julian Brown
AuthorDate: Tue Feb 26 16:00:54 2019 -0800
Commit: Kwok Cheung Yeung
CommitDate: Fri May 31 13:40:07 2019 -0700
Fix hang when running oacc exec with CUDA 9.0 nvprof
2018-09-20 Tom de Vries
Cesar Philippidis
libgomp/
[...]
..., which got cherry-picked (automated, without any review) into current
devel/omp/gcc-9 in commit f752d880a5abc591a25ad22fb892363f6520bcf1.
Of course, it would've helped tremendously had the original og7 commit
included a test case... :'-/ (... by simply reproducing the nested calls
that CUDA 9 nvprof seems to be doing.)
Still without a test case, for now I have pushed the attached patch to
devel/omp/gcc-9 in commit 9ae129017c7fc1fa638d6beedd3802b515ca692b 'Fix
og9 "Fix hang when running oacc exec with CUDA 9.0 nvprof"'.
Grüße
Thomas
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
>From 9ae129017c7fc1fa638d6beedd3802b515ca692b Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Wed, 25 Mar 2020 17:57:02 +0100
Subject: [PATCH] Fix og9 "Fix hang when running oacc exec with CUDA 9.0
nvprof"
Compared to the original og7 version, and still-good og8 version, the og9
version of this patch did get its code altered in a way so that it no longer
resolves the problem it's meant to resolve -- the hang was back.
libgomp/
* oacc-init.c (acc_init_1): Move 'acc_init_state' logic to where
it belongs.
---
libgomp/ChangeLog.omp | 5 +
libgomp/oacc-init.c | 10 +-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 88957864a69..75c45917998 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,8 @@
+2020-03-25 Thomas Schwinge
+
+ * oacc-init.c (acc_init_1): Move 'acc_init_state' logic to where
+ it belongs.
+
2019-11-22 Kwok Cheung Yeung
* testsuite/libgomp.oacc-fortran/lib-16.f90: Fix async-safety issue.
diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
index beeeb48c106..765fa2f3b95 100644
--- a/libgomp/oacc-init.c
+++ b/libgomp/oacc-init.c
@@ -231,6 +231,11 @@ acc_dev_num_out_of_range (acc_device_t d, int ord, int ndevs)
static struct gomp_device_descr *
acc_init_1 (acc_device_t d, acc_construct_t parent_construct, int implicit)
{
+ gomp_mutex_lock (_init_state_lock);
+ acc_init_state = initializing;
+ acc_init_thread = pthread_self ();
+ gomp_mutex_unlock (_init_state_lock);
+
bool check_not_nested_p;
if (implicit)
{
@@ -293,11 +298,6 @@ acc_init_1 (acc_device_t d, acc_construct_t parent_construct, int implicit)
struct gomp_device_descr *base_dev, *acc_dev;
int ndevs;
- gomp_mutex_lock (_init_state_lock);
- acc_init_state = initializing;
- acc_init_thread = pthread_self ();
- gomp_mutex_unlock (_init_state_lock);
-
base_dev = resolve_device (d, true);
ndevs = base_dev->get_num_devices_func ();
--
2.17.1
Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]
On Wed, 25 Mar 2020, Marek Polacek wrote:
> On Wed, Mar 25, 2020 at 12:17:41PM -0400, Patrick Palka via Gcc-patches wrote:
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 03ecd7838f6..14a22b85318 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser)
> > else
> >parms = NULL_TREE;
> >
> > -/* Parse the requirement body. */
> > +/* Always parse the requirement body as if we're inside a template so
> > that
> > + we always do full semantic processing only during evaluation of this
> > + requires-expression and not also now during parsing. */
> > +processing_template_decl_sentinel ptds;
>
> This looks weird; this sentinel will always set processing_template_decl to 0,
> right? So...
>
> > +if (!processing_template_decl)
> > + processing_template_decl = 1;
>
> ...this will always be true? Did you mean to use temp_override instead?
Oops, good point. The intention was to set processing_template_decl to
1 when it is 0, and to otherwise not change it. So it looks like I
should pass false to processing_template_decl_sentinel's ctor here, like
so.
>
> I don't dare to say if this approach is OK or not, but I've certainly had
> my share of fun with CALL_EXPRs in templates :).
Hehe :)
-- >8 --
gcc/cp/ChangeLog:
PR c++/94252
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily setting
processing_template_decl to non-zero.
* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a REQUIRES_EXPR.
gcc/testsuite/ChangeLog:
PR c++/94252
* g++.dg/concepts/diagnostic7.C: New test.
* g++.dg/concepts/pr94252.C: New test.
---
gcc/cp/parser.c | 7 ++-
gcc/cp/semantics.c | 6 --
gcc/testsuite/g++.dg/concepts/diagnostic7.C | 12
gcc/testsuite/g++.dg/concepts/pr94252.C | 14 ++
4 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic7.C
create mode 100644 gcc/testsuite/g++.dg/concepts/pr94252.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 03ecd7838f6..6150814ea9e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser)
else
parms = NULL_TREE;
-/* Parse the requirement body. */
+/* Always parse the requirement body as if we're inside a template so that
+ we always do full semantic processing only during evaluation of this
+ requires-expression and not also now during parsing. */
+processing_template_decl_sentinel ptds (false);
+if (!processing_template_decl)
+ processing_template_decl = 1;
reqs = cp_parser_requirement_body (parser);
if (reqs == error_mark_node)
return error_mark_node;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index bcb2e72fbb5..e998b373af4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9691,8 +9691,10 @@ finish_static_assert (tree condition, tree message,
location_t location,
error ("static assertion failed: %s",
TREE_STRING_POINTER (message));
- /* Actually explain the failure if this is a concept check. */
- if (concept_check_p (orig_condition))
+ /* Actually explain the failure if this is a concept check or a
+requires-expression. */
+ if (concept_check_p (orig_condition)
+ || TREE_CODE (orig_condition) == REQUIRES_EXPR)
diagnose_constraints (location, orig_condition, NULL_TREE);
}
else if (condition && condition != error_mark_node)
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic7.C
b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
new file mode 100644
index 000..f78e9bb8240
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++2a } }
+
+template
+ concept same_as = __is_same(A, B); // { dg-message ".void. is not the same
as .int." }
+
+void f();
+
+static_assert(requires { { f() } noexcept -> same_as; });
+// { dg-error "static assertion failed" "" { target *-*-* } .-1 }
+// { dg-message "not .noexcept." "" { target *-*-* } .-2 }
+// { dg-message "return-type-requirement" "" { target *-*-* } .-3 }
+// { dg-error "does not satisfy placeholder constraints" "" { target *-*-* }
.-4 }
diff --git a/gcc/testsuite/g++.dg/concepts/pr94252.C
b/gcc/testsuite/g++.dg/concepts/pr94252.C
new file mode 100644
index 000..7b93997363a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr94252.C
@@ -0,0 +1,14 @@
+// PR c++/94252
+// { dg-do compile { target c++2a } }
+
+auto f = []{ return 0; };
+static_assert(requires { f(); });
+
+template
+ concept same_as = __is_same(A, B);
+
Re: [PATCH] avoid using TYPE_SIZE unless it's constant [PR94131]
On Wed, 2020-03-25 at 10:41 -0600, Martin Sebor wrote: > On 3/25/20 9:40 AM, Jeff Law wrote: > > On Tue, 2020-03-17 at 19:35 -0600, Martin Sebor via Gcc-patches wrote: > > > PR tree-optimization/94131 - ICE on printf with a VLA string and > > > -fno-tree- > > > ccp > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR tree-optimization/94131 > > > * gcc.dg/pr94131.c: New test. > > > > > > gcc/ChangeLog: > > > > > > PR tree-optimization/94131 > > > * gimple-fold.c (get_range_strlen_tree): Fail for variable-length > > > types and decls. > > > * tree-ssa-strlen.c (get_range_strlen_dynamic): Avoid assuming > > > types have constant sizes. > > I approved this a week or so ago. I went ahead and committed it to get the > > P1 > > resolved. > > Thanks. It was on my list of things to do this week, along with > the fixes for PR94098 and PR94004. No problem. I'm just whittling down outstanding stuff that potentially affects the release, starting with P1s, of course :-) jeff
Re: [PATCH] avoid using TYPE_SIZE unless it's constant [PR94131]
On 3/25/20 9:40 AM, Jeff Law wrote: On Tue, 2020-03-17 at 19:35 -0600, Martin Sebor via Gcc-patches wrote: PR tree-optimization/94131 - ICE on printf with a VLA string and -fno-tree-ccp gcc/testsuite/ChangeLog: PR tree-optimization/94131 * gcc.dg/pr94131.c: New test. gcc/ChangeLog: PR tree-optimization/94131 * gimple-fold.c (get_range_strlen_tree): Fail for variable-length types and decls. * tree-ssa-strlen.c (get_range_strlen_dynamic): Avoid assuming types have constant sizes. I approved this a week or so ago. I went ahead and committed it to get the P1 resolved. Thanks. It was on my list of things to do this week, along with the fixes for PR94098 and PR94004. Martin
Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]
On Wed, Mar 25, 2020 at 12:17:41PM -0400, Patrick Palka via Gcc-patches wrote: > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 03ecd7838f6..14a22b85318 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser) > else >parms = NULL_TREE; > > -/* Parse the requirement body. */ > +/* Always parse the requirement body as if we're inside a template so > that > + we always do full semantic processing only during evaluation of this > + requires-expression and not also now during parsing. */ > +processing_template_decl_sentinel ptds; This looks weird; this sentinel will always set processing_template_decl to 0, right? So... > +if (!processing_template_decl) > + processing_template_decl = 1; ...this will always be true? Did you mean to use temp_override instead? I don't dare to say if this approach is OK or not, but I've certainly had my share of fun with CALL_EXPRs in templates :). > reqs = cp_parser_requirement_body (parser); > if (reqs == error_mark_node) >return error_mark_node; Marek
[PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]
This PR reports that the requires-expression in
auto f = [] { };
static_assert(requires { f(); });
erroneously evaluates to false. The way we end up evaluating to false goes as
follows. During the parsing of the requires-expression, we call
finish_call_expr from cp_parser_requires_expression with the arguments
fn = VIEW_CONVERT_EXPR(f);
args = {}
which does the full processing of the call (since we're not inside a template)
and returns
::operator() ();
Later, when evaluating the requires-expression, we call finish_call_expr again,
this time from tsubst_expr from satisfy_atom, with the arguments
fn = operator()
args = { }
(which, as expected, correspond to the CALL_EXPR returned by finish_call_expr
the first time). But this time, finish_call_expr returns error_mark_node
because
it doesn't expect to see an explicit 'this' argument in the args array, treating
it instead as a user-written argument which causes the only candidate function
to be discarded. This causes the requires-expression to evaluate to false.
In short, it seems finish_call_expr is not idempotent on certain inputs when
!processing_template_decl. Assuming this idempotency issue is not specific to
finish_call_expr, it seems that the safest thing to do is to avoid doing full
semantic processing twice when parsing and evaluating a requires-expression that
lives outside of a template definition.
This patch achieves this by temporarily setting processing_template_decl to
non-zero when parsing the body of a requires-expression. This way, full
semantic processing will always be done only during evaluation and not during
parsing.
Testing is in progress. Does this look like the right solution?
gcc/cp/ChangeLog:
PR c++/94252
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily setting
processing_template_decl to non-zero.
* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a REQUIRES_EXPR.
gcc/testsuite/ChangeLog:
PR c++/94252
* g++.dg/concepts/diagnostic7.C: New test.
* g++.dg/concepts/pr94252.C: New test.
---
gcc/cp/parser.c | 7 ++-
gcc/cp/semantics.c | 6 --
gcc/testsuite/g++.dg/concepts/diagnostic7.C | 12
gcc/testsuite/g++.dg/concepts/pr94252.C | 14 ++
4 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic7.C
create mode 100644 gcc/testsuite/g++.dg/concepts/pr94252.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 03ecd7838f6..14a22b85318 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser)
else
parms = NULL_TREE;
-/* Parse the requirement body. */
+/* Always parse the requirement body as if we're inside a template so that
+ we always do full semantic processing only during evaluation of this
+ requires-expression and not also now during parsing. */
+processing_template_decl_sentinel ptds;
+if (!processing_template_decl)
+ processing_template_decl = 1;
reqs = cp_parser_requirement_body (parser);
if (reqs == error_mark_node)
return error_mark_node;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index bcb2e72fbb5..e998b373af4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9691,8 +9691,10 @@ finish_static_assert (tree condition, tree message,
location_t location,
error ("static assertion failed: %s",
TREE_STRING_POINTER (message));
- /* Actually explain the failure if this is a concept check. */
- if (concept_check_p (orig_condition))
+ /* Actually explain the failure if this is a concept check or a
+requires-expression. */
+ if (concept_check_p (orig_condition)
+ || TREE_CODE (orig_condition) == REQUIRES_EXPR)
diagnose_constraints (location, orig_condition, NULL_TREE);
}
else if (condition && condition != error_mark_node)
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic7.C
b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
new file mode 100644
index 000..f78e9bb8240
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++2a } }
+
+template
+ concept same_as = __is_same(A, B); // { dg-message ".void. is not the same
as .int." }
+
+void f();
+
+static_assert(requires { { f() } noexcept -> same_as; });
+// { dg-error "static assertion failed" "" { target *-*-* } .-1 }
+// { dg-message "not .noexcept." "" { target *-*-* } .-2 }
+// { dg-message "return-type-requirement" "" { target *-*-* } .-3 }
+// { dg-error "does not satisfy placeholder constraints" "" { target *-*-* }
.-4 }
diff --git a/gcc/testsuite/g++.dg/concepts/pr94252.C
New French PO file for 'gcc' (version 10.1-b20200322)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the French team of translators. The file is available at: https://translationproject.org/latest/gcc/fr.po (This file, 'gcc-10.1-b20200322.fr.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH], Set -mpcrel by default on Linux 64-bit systems for -mcpu=future
Hi, Comments inline. (Taking a pass with focus on cosmetic stuff. This is intended to help Segher focus on the harder parts :-) ). On Mon, 2020-03-23 at 16:38 -0400, Michael Meissner via Gcc-patches wrote: Subject: [Patch v327] set -mcprel by default ... Include some powerpc or rs6000 reference in the subject. My filters missed this one. (I'm assuming this is powerpc target specific). > This is a revision of a patch that I've submitted several times. It makes > -mpcrel the default on Linux 64-bit systems that use ELF v2, use the medium > code mode, and if the user did not disable prefixed load/store instructions > for > -mcpu=future. The fewer words, the easier to review. History is important but so is making the review easy. "This patch makes -mpcrel the default on Linux 64-bit systems that use ELF V2, medium code model, and have not disabled prefix instructions." > > Previous versions of the patch had two macros that the target os could set, > one > to say that the target os supported prefixed instructions with large numeric > offsets, and the second whether PC-relative prefixed load/store instructions > could be generated. Segher asked that we drop the capability to configure > whether prefixed numeric load/store instructions would be enabled by default, > and this patch does this. All of the PC-relative support is in the master > branch, we just need to enable it by default. [v327] dropped macros to indicate if target supports prefixed instructions pre previous feedback. > > Is this patch acceptable to be committed to the master branch. I have done > various tests with this patch, including most recently bootstraping and > running > make check. I have built the Spec 2017 benchmark suite with this patch. > > 2020-03-23 Michael Meissner > > * config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Enable -mpcrel > for -mcpu=future by default on 64-bit systems with medium code > model. > * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do not > define the bits for -mprefixed or -mpcrel here. The change to ISA_FUTURE_MASKS_SERVER only drops OPTION_MASK_PREFIXED. No change to PCREL there. > (OTHER_FUTURE_MASKS): Define the bits for -mprefixed and -mpcrel > here. No touches to OTHER_FUTURE_MASKS below. (accidental patch ommission or missed a changelog update?) > * config/rs6000/rs6000.c (PCREL_SUPPORTED_BY_OS): If not defined, > don't enable -mpcrel by default. I suggest s/If not defined// > (rs6000_option_override_internal): Enable -mpcrel on systems that > support it, if the user did not do -mno-pcrel. I suggest s/if the user ...// > > --- /tmp/QuuFm5_linux64.h 2020-03-20 20:15:38.321862650 -0400 > +++ gcc/config/rs6000/linux64.h 2020-03-20 18:36:33.654484833 -0400 > @@ -640,3 +640,11 @@ extern int dot_symbols; > enabling the __float128 keyword. */ > #undef TARGET_FLOAT128_ENABLE_TYPE > #define TARGET_FLOAT128_ENABLE_TYPE 1 > + > +/* Enable support for PC-relative addressing on the 'future' system. > Currently > + this support only exits for the ELF v2 object file format using the medium > + code model. */ > +#undef PCREL_SUPPORTED_BY_OS > +#define PCREL_SUPPORTED_BY_OS(TARGET_FUTURE && TARGET_PREFIXED > \ > + && ELFv2_ABI_CHECK \ > + && (TARGET_CMODEL == CMODEL_MEDIUM)) Is there need or desire to explicitly set TARGET_FUTURE or TARGET_PREFIXED to zero in the header file now? There are no other references to those in the header file. Otherwise OK. > --- /tmp/sO5cAE_rs6000-cpus.def 2020-03-20 20:15:38.331862575 -0400 > +++ gcc/config/rs6000/rs6000-cpus.def 2020-03-20 17:05:17.347638233 -0400 > @@ -75,11 +75,10 @@ >| OPTION_MASK_P8_VECTOR\ >| OPTION_MASK_P9_VECTOR) > > -/* Support for a future processor's features. Do not enable -mpcrel until it > - is fully functional. */ > +/* Support for a future processor's features. The addressing related options > + (like -mprefixed, -mpcrel) are not set here. */ So, where are they set? why is it important to say they are not set here? > #define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER > \ > - | OPTION_MASK_FUTURE \ > - | OPTION_MASK_PREFIXED) > + | OPTION_MASK_FUTURE) > > /* Flags that need to be turned off if -mno-future. */ > #define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \ > --- /tmp/xQt3Pd_rs6000.c 2020-03-20 20:15:38.343862485 -0400 > +++ gcc/config/rs6000/rs6000.c2020-03-20 20:11:02.942928364 -0400 > @@ -98,6 +98,12 @@ > #endif > #endif > > +/* Set up the defaults for whether PC-relative addressing is supported by the > + target system. */ >
Re: [PATCH] c++: Fix invalid -Wduplicated-cond warning [PR94265]
On 3/25/20 8:52 AM, Patrick Palka wrote:
This fixes a false-positive warning from -Wduplicate-cond in the presence of an
if-statement with a non-empty init-statement. Precisely determining whether a
non-empty init-statement has side effects seems tricky and error-prone, so this > patch takes the route of unconditionally invalidating the condition
chain when
it encounters such an if-statement.
Tested on x86_64-pc-linux-gnu, does this look OK?
OK.
gcc/cp/ChangeLog:
PR c++/94265
* parser.c (cp_parser_selection_statement) [RID_IF]: Invalidate the
current condition chain if the if-statement has a non-empty
init-statement.
gcc/testsuite/ChangeLog:
PR c++/94265
* g++.dg/warn/Wduplicated-cond1.C: New test.
---
gcc/cp/parser.c | 7 +++
gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C | 16
2 files changed, 23 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index cbd5510a8fb..05363653691 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11934,6 +11934,13 @@ cp_parser_selection_statement (cp_parser* parser, bool
*if_p,
pedwarn (cp_lexer_peek_token (parser->lexer)->location, 0,
"init-statement in selection statements only available "
"with %<-std=c++17%> or %<-std=gnu++17%>");
+ if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
+ {
+ /* A non-empty init-statement can have arbitrary side
+ effects. */
+ delete chain;
+ chain = NULL;
+ }
cp_parser_init_statement (parser, );
}
diff --git a/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C b/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
new file mode 100644
index 000..e85920aaedf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
@@ -0,0 +1,16 @@
+// PR c++/94265
+// { dg-do compile { target c++17 } }
+// { dg-additional-options "-Wduplicated-cond" }
+
+void
+foo ()
+{
+ if (int a = 0; a)
+ { }
+ else if (a = 5; a) // { dg-message "previously used here" }
+ { }
+ else if (; a) // { dg-warning "duplicated .if. condition" }
+ { }
+ else if (int b = ++a; a)
+ { }
+}
Re: [PATCH] c++: Fix invalid -Wduplicated-cond warning [PR94265]
On Wed, 25 Mar 2020, Marek Polacek wrote: > On Wed, Mar 25, 2020 at 08:52:47AM -0400, Patrick Palka via Gcc-patches wrote: > > This fixes a false-positive warning from -Wduplicate-cond in the presence > > of an > > if-statement with a non-empty init-statement. Precisely determining > > whether a > > non-empty init-statement has side effects seems tricky and error-prone, so > > this > > patch takes the route of unconditionally invalidating the condition chain > > when > > it encounters such an if-statement. > > > > Tested on x86_64-pc-linux-gnu, does this look OK? > > This looks ok (can't approve but I wrote -Wduplicated-cond), thanks. > > > gcc/cp/ChangeLog: > > > > PR c++/94265 > > * parser.c (cp_parser_selection_statement) [RID_IF]: Invalidate the > > We usually spell cases like , [ ] is for #ifdefs. Noted, thanks. Consider that fixed.
Re: [PATCH] avoid using TYPE_SIZE unless it's constant [PR94131]
On Tue, 2020-03-17 at 19:35 -0600, Martin Sebor via Gcc-patches wrote: > PR tree-optimization/94131 - ICE on printf with a VLA string and -fno-tree-ccp > > gcc/testsuite/ChangeLog: > > PR tree-optimization/94131 > * gcc.dg/pr94131.c: New test. > > gcc/ChangeLog: > > PR tree-optimization/94131 > * gimple-fold.c (get_range_strlen_tree): Fail for variable-length > types and decls. > * tree-ssa-strlen.c (get_range_strlen_dynamic): Avoid assuming > types have constant sizes. I approved this a week or so ago. I went ahead and committed it to get the P1 resolved. jeff
[testsuite, obvious] Fix gcc.dg/pr92301.c on targets that don't support argc/argv.
I've checked in this trivial patch to fix another test failure I found on nios2-elf, due to the BSP we use for simulator testing on this target not having support for argc/argv. This testcase doesn't actually use any command-line arguments and I think just does the argc test to avoid optimizing the interesting things away, so I tweaked it to allow argc == 0 in addition to argc == 1. -Sandra commit 0fca105f8ca95747fad3d3315e642ea9dc6936e0 Author: Sandra Loosemore Date: Wed Mar 25 08:01:50 2020 -0700 Fix gcc.dg/pr92301.c on targets that don't support argc/argv. 2020-03-25 Sandra Loosemore gcc/testsuite/ * gcc.dg/pr92301.c (main): Allow argc to be 0 to support embedded targets. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 86d3b71..5fa61f9 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-03-25 Sandra Loosemore + + * gcc.dg/pr92301.c (main): Allow argc to be 0 to support + embedded targets. + 2020-03-25 Jakub Jelinek PR debug/94296 diff --git a/gcc/testsuite/gcc.dg/pr92301.c b/gcc/testsuite/gcc.dg/pr92301.c index 9a47e12..3ade201 100644 --- a/gcc/testsuite/gcc.dg/pr92301.c +++ b/gcc/testsuite/gcc.dg/pr92301.c @@ -23,7 +23,7 @@ int main(int argc, char **argv) for (unsigned i = 0; i < N; i++) a[i] = i; - if (argc == 1) + if (argc < 2) m = 17; unsigned int r = df_count_refs(1);
Re: [PATCH] c++: Remove redundant calls to type_dependent_expression_p
On Wed, Mar 25, 2020 at 08:52:46AM -0400, Patrick Palka via Gcc-patches wrote: > This simplifies conditions that test both value_dependent_expression_p and > type_dependent_expression_p, since the former predicate now subsumes the > latter. > > Tested on x86_64-pc-linux-gnu, does this look OK? LGTM. Marek
Re: [PATCH] c++: Fix invalid -Wduplicated-cond warning [PR94265]
On Wed, Mar 25, 2020 at 08:52:47AM -0400, Patrick Palka via Gcc-patches wrote: > This fixes a false-positive warning from -Wduplicate-cond in the presence of > an > if-statement with a non-empty init-statement. Precisely determining whether a > non-empty init-statement has side effects seems tricky and error-prone, so > this > patch takes the route of unconditionally invalidating the condition chain when > it encounters such an if-statement. > > Tested on x86_64-pc-linux-gnu, does this look OK? This looks ok (can't approve but I wrote -Wduplicated-cond), thanks. > gcc/cp/ChangeLog: > > PR c++/94265 > * parser.c (cp_parser_selection_statement) [RID_IF]: Invalidate the We usually spell cases like , [ ] is for #ifdefs. Marek
Re: [PATCH] Fix stack pointer handling in ms_hook_prologue functions for i386 target.
Hello,
ping for patch https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00554.html
Regards,
Paul.
On 2/21/20 12:10, Paul Gofman wrote:
> Hello,
>
> ping for patch https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00554.html.
>
> Thanks,
> Paul.
>
> On 2/10/20 19:22, Paul Gofman wrote:
>> ChangeLog:
>> PR target/91489
>> * config/i386/i386.md (simple_return): Also check
>> for ms_hook_prologue function attribute.
>> * config/i386/i386.c (ix86_can_use_return_insn_p):
>> Also check for ms_hook_prologue function attribute.
>>
>> testsuite/ChangeLog:
>> PR target/91489
>> * gcc.target/i386/ms_hook_prologue.c: Expand testcase
>> to reproduce PR target/91489 issue.
>>
>> Signed-off-by: Paul Gofman
>> ---
>> gcc/config/i386/i386-protos.h| 1 +
>> gcc/config/i386/i386.c | 3 +++
>> gcc/config/i386/i386.md | 5 -
>> gcc/testsuite/gcc.target/i386/ms_hook_prologue.c | 13 -
>> 4 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
>> index 266381ca5a6..966ce426a18 100644
>> --- a/gcc/config/i386/i386-protos.h
>> +++ b/gcc/config/i386/i386-protos.h
>> @@ -26,6 +26,7 @@ extern bool ix86_handle_option (struct gcc_options *opts,
>> /* Functions in i386.c */
>> extern bool ix86_target_stack_probe (void);
>> extern bool ix86_can_use_return_insn_p (void);
>> +extern bool ix86_function_ms_hook_prologue (const_tree fn);
>> extern void ix86_setup_frame_addresses (void);
>> extern bool ix86_rip_relative_addr_p (struct ix86_address *parts);
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 44bc0e0176a..68e2a7519f4 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -4954,6 +4954,9 @@ symbolic_reference_mentioned_p (rtx op)
>> bool
>> ix86_can_use_return_insn_p (void)
>> {
>> + if (ix86_function_ms_hook_prologue (current_function_decl))
>> +return false;
>> +
>>if (ix86_function_naked (current_function_decl))
>> return false;
>>
>> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>> index f14683cd14f..a7302b886c6 100644
>> --- a/gcc/config/i386/i386.md
>> +++ b/gcc/config/i386/i386.md
>> @@ -13445,10 +13445,13 @@
>> ;; static chain pointer - the first instruction has to be pushl %esi
>> ;; and it can't be moved around, as we use alternate entry points
>> ;; in that case.
>> +;; Also disallow for ms_hook_prologue functions which have frame
>> +;; pointer set up in function label which is correctly handled in
>> +;; ix86_expand_{prologue|epligoue}() only.
>>
>> (define_expand "simple_return"
>>[(simple_return)]
>> - "!TARGET_SEH && !ix86_static_chain_on_stack"
>> + "!TARGET_SEH && !ix86_static_chain_on_stack &&
>> !ix86_function_ms_hook_prologue (cfun->decl)"
>> {
>>if (crtl->args.pops_args)
>> {
>> diff --git a/gcc/testsuite/gcc.target/i386/ms_hook_prologue.c
>> b/gcc/testsuite/gcc.target/i386/ms_hook_prologue.c
>> index e11bcc049cb..12e54c0e4ad 100644
>> --- a/gcc/testsuite/gcc.target/i386/ms_hook_prologue.c
>> +++ b/gcc/testsuite/gcc.target/i386/ms_hook_prologue.c
>> @@ -4,6 +4,8 @@
>> /* { dg-require-effective-target ms_hook_prologue } */
>> /* { dg-options "-O2 -fomit-frame-pointer" } */
>>
>> +#include
>> +
>> int __attribute__ ((__ms_hook_prologue__)) foo ()
>> {
>>unsigned char *ptr = (unsigned char *) foo;
>> @@ -32,7 +34,16 @@ int __attribute__ ((__ms_hook_prologue__)) foo ()
>>return 0;
>> }
>>
>> +unsigned int __attribute__ ((noinline, __ms_hook_prologue__)) test_func()
>> +{
>> + static int value;
>> +
>> + if (value++) puts("");
>> +
>> + return 0;
>> +}
>> +
>> int main ()
>> {
>> - return foo();
>> + return foo() || test_func();
>> }
>
Re: [PATCH] coroutines: Implement n4849 changes to exception handling.
On 3/20/20 10:54 AM, Iain Sandoe wrote:
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index a943ba01de6..bcf3514bbcf 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1348,6 +1348,7 @@ struct coro_aw_data
tree actor_fn; /* Decl for context. */
tree coro_fp;/* Frame pointer var. */
tree resume_idx; /* This is the index var in the frame. */
+ tree iarc_idx; /* This is the initial await resume called index. */
comment misleading it's a flag, not an index, see below.
tree self_h; /* This is a handle to the current coro (frame var). */
tree cleanup;/* This is where to go once we complete local destroy. */
tree cororet;/* This is where to go if we suspend. */
@@ -1445,6 +1446,8 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void
*d)
tree awaiter_calls = TREE_OPERAND (saved_co_await, 3);
tree source = TREE_OPERAND (saved_co_await, 4);
+ bool is_initial =
+(source && TREE_INT_CST_LOW (source) == (int) INITIAL_SUSPEND_POINT);
bool is_final = (source
&& TREE_INT_CST_LOW (source) == (int) FINAL_SUSPEND_POINT);
bool needs_dtor = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (var));
@@ -1586,6 +1589,16 @@ co_await_expander (tree *stmt, int * /*do_subtree*/,
void *d)
resume_label = build_stmt (loc, LABEL_EXPR, resume_label);
append_to_statement_list (resume_label, _list);
+ if (is_initial)
+{
+ /* Note that we are about to execute the await_resume() for the initial
+await expression. */
+ r = build2_loc (loc, MODIFY_EXPR, boolean_type_node, data->iarc_idx,
+ boolean_true_node);
It is confusing you're assigning a boolean to a variable refered to as
'idx'. Also, why is it a runtime variable? you're setting it from a
compile time constant -- just propagate the constant?
I'm finding this name too confusing to review properly.
nathan
--
Nathan Sidwell
[committed] coroutines: Fix missing dereference (PR94319).
Hi This fixes a typo spotted by static analysis. tested on x86_64-darwin, applied to master as obvious thanks Iain gcc/cp/ChangeLog: 2020-03-25 Iain Sandoe PR c++/94319 * coroutines.cc (captures_temporary): Fix a missing dereference. diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 4332b07cc5f..d9f87eca649 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,8 @@ +2020-03-25 Iain Sandoe + + PR c++/94319 + * coroutines.cc (captures_temporary): Fix a missing dereference. + 2020-03-24 Marek Polacek PR c++/94190 - wrong no post-decrement operator error in template. diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index a943ba01de6..ef1ce150680 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2651,7 +2651,7 @@ captures_temporary (tree *stmt, int *do_subtree, void *d) } /* As far as it's necessary, we've walked the subtrees of the call expr. */ - do_subtree = 0; + *do_subtree = 0; return NULL_TREE; }
[PATCH] c++: Fix invalid -Wduplicated-cond warning [PR94265]
This fixes a false-positive warning from -Wduplicate-cond in the presence of an
if-statement with a non-empty init-statement. Precisely determining whether a
non-empty init-statement has side effects seems tricky and error-prone, so this
patch takes the route of unconditionally invalidating the condition chain when
it encounters such an if-statement.
Tested on x86_64-pc-linux-gnu, does this look OK?
gcc/cp/ChangeLog:
PR c++/94265
* parser.c (cp_parser_selection_statement) [RID_IF]: Invalidate the
current condition chain if the if-statement has a non-empty
init-statement.
gcc/testsuite/ChangeLog:
PR c++/94265
* g++.dg/warn/Wduplicated-cond1.C: New test.
---
gcc/cp/parser.c | 7 +++
gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C | 16
2 files changed, 23 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index cbd5510a8fb..05363653691 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11934,6 +11934,13 @@ cp_parser_selection_statement (cp_parser* parser, bool
*if_p,
pedwarn (cp_lexer_peek_token (parser->lexer)->location, 0,
"init-statement in selection statements only available "
"with %<-std=c++17%> or %<-std=gnu++17%>");
+ if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
+ {
+ /* A non-empty init-statement can have arbitrary side
+ effects. */
+ delete chain;
+ chain = NULL;
+ }
cp_parser_init_statement (parser, );
}
diff --git a/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
b/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
new file mode 100644
index 000..e85920aaedf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wduplicated-cond1.C
@@ -0,0 +1,16 @@
+// PR c++/94265
+// { dg-do compile { target c++17 } }
+// { dg-additional-options "-Wduplicated-cond" }
+
+void
+foo ()
+{
+ if (int a = 0; a)
+ { }
+ else if (a = 5; a) // { dg-message "previously used here" }
+ { }
+ else if (; a) // { dg-warning "duplicated .if. condition" }
+ { }
+ else if (int b = ++a; a)
+ { }
+}
--
2.26.0.rc1.11.g30e9940356
[PATCH] c++: Remove redundant calls to type_dependent_expression_p
This simplifies conditions that test both value_dependent_expression_p and
type_dependent_expression_p, since the former predicate now subsumes the latter.
Tested on x86_64-pc-linux-gnu, does this look OK?
gcc/cp/ChangeLog:
* decl.c (compute_array_index_type_loc): Remove redundant
type_dependent_expression_p check that is subsumed by
value_dependent_expression_p.
* decl2.c (is_late_template_attribute): Likewise.
* pt.c (uses_template_parms): Likewise.
(dependent_template_arg_p): Likewise.
---
gcc/cp/decl.c | 3 +--
gcc/cp/decl2.c | 3 +--
gcc/cp/pt.c| 6 ++
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 319b7ee5c1c..69a238997b4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10338,8 +10338,7 @@ compute_array_index_type_loc (location_t name_loc, tree
name, tree size,
/* We can only call value_dependent_expression_p on integral constant
expressions; treat non-constant expressions as dependent, too. */
if (processing_template_decl
- && (type_dependent_expression_p (size)
- || !TREE_CONSTANT (size) || value_dependent_expression_p (size)))
+ && (!TREE_CONSTANT (size) || value_dependent_expression_p (size)))
{
/* We cannot do any checking for a SIZE that isn't known to be
constant. Just build the index type and mark that it requires
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 2efb2e54f37..6cf72b432e2 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1191,8 +1191,7 @@ is_late_template_attribute (tree attr, tree decl)
&& identifier_p (t))
continue;
- if (value_dependent_expression_p (t)
- || type_dependent_expression_p (t))
+ if (value_dependent_expression_p (t))
return true;
}
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b82c51a3042..eaa84f43dcd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10525,8 +10525,7 @@ uses_template_parms (tree t)
else if (t == error_mark_node)
dependent_p = false;
else
-dependent_p = (type_dependent_expression_p (t)
- || value_dependent_expression_p (t));
+dependent_p = value_dependent_expression_p (t);
processing_template_decl = saved_processing_template_decl;
@@ -27030,8 +27029,7 @@ dependent_template_arg_p (tree arg)
else if (TYPE_P (arg))
return dependent_type_p (arg);
else
-return (type_dependent_expression_p (arg)
- || value_dependent_expression_p (arg));
+return value_dependent_expression_p (arg);
}
/* Returns true if ARGS (a collection of template arguments) contains
--
2.26.0.rc1.11.g30e9940356
Re: [PATCH] coroutines: Implement n4849 recommended symmetric transfer.
On 3/24/20 2:08 PM, Iain Sandoe wrote: tree suspend = TREE_VEC_ELT (awaiter_calls, 1); /* await_suspend(). */ + tree susp_type; + if (tree fndecl = cp_get_callee_fndecl_nofold (suspend)) +susp_type = TREE_TYPE (TREE_TYPE (fndecl)); + else +susp_type = TREE_TYPE (suspend); Why, when there's a call of a named function, is it that TREE_TYPE (suspend) is insufficient? You mentioned TARGET_EXPR, but why is their type differenty? @@ -2012,6 +2018,12 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, = create_named_label_with_ctx (loc, "actor.begin", actor); tree actor_frame = build1_loc (loc, INDIRECT_REF, coro_frame_type, actor_fp); + /* Declare the continuation handle. */ + tree ci = build_stmt (loc, DECL_EXPR, continuation); + ci = build1_loc (loc, CONVERT_EXPR, void_type_node, ci); + ci = maybe_cleanup_point_expr_void (ci); + add_stmt (ci); you don't need to wrap in a CONVERT_EXPR, can you use add_decl_expr ? @@ -2368,6 +2383,35 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, + + /* Now we have the actual call, and we can mark it as a tail. */ + CALL_EXPR_TAILCALL (resume) = true; + /* ... and for optimisation levels 0..1, mark it as requiring a tail-call + for correctness. It seems that doing this for optimisation levels that + normally perform tail-calling, confuses the ME (or it would be logical + to put this on unilaterally). */ + if (optimize < 2) +CALL_EXPR_MUST_TAIL_CALL (resume) = true; + resume = coro_build_cvt_void_expr_stmt (resume, loc); I'd be happier with gcc_checking_assert (maybe_cleanup_point_expr_void (resume) == resume); here. I see we can wrap RETURN_EXPRs with cleanups, so perhaps adding is ok anyway? nathan -- Nathan Sidwell
Re: [PATCH] Do not error about missing zstd unless --with-zstd.
On 3/25/20 11:59 AM, Martin Liška wrote: Hi. It's error that I caused by the recent update to zstd detection. The patch fixes situation where zstd is missing and user doesn't provide --with-zstd option. Ready for master? Installed as it blocks bootstrap as 68c4570a4de. Martin Thanks, Martin gcc/ChangeLog: 2020-03-25 Martin Liska PR lto/94259 * configure.ac: Report error only when --with-zstd is used. * configure: Regenerate. --- gcc/configure | 2 +- gcc/configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
[PATCH] Do not error about missing zstd unless --with-zstd.
Hi. It's error that I caused by the recent update to zstd detection. The patch fixes situation where zstd is missing and user doesn't provide --with-zstd option. Ready for master? Thanks, Martin gcc/ChangeLog: 2020-03-25 Martin Liska PR lto/94259 * configure.ac: Report error only when --with-zstd is used. * configure: Regenerate. --- gcc/configure| 2 +- gcc/configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/configure b/gcc/configure index b4231fe7b7e..dfdc5d8f75a 100755 --- a/gcc/configure +++ b/gcc/configure @@ -10019,7 +10019,7 @@ if test $gcc_cv_header_zstd_h = yes; then $as_echo "#define HAVE_ZSTD_H 1" >>confdefs.h -else +elif test "x$with_zstd" != x; then as_fn_error $? "Unable to find zstd.h. See config.log for details." "$LINENO" 5 fi diff --git a/gcc/configure.ac b/gcc/configure.ac index 0051963c85d..fdee9ea587c 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -1371,7 +1371,7 @@ AC_MSG_RESULT($gcc_cv_header_zstd_h) if test $gcc_cv_header_zstd_h = yes; then AC_DEFINE(HAVE_ZSTD_H, 1, [Define if you have a working header file.]) -else +elif test "x$with_zstd" != x; then as_fn_error $? "Unable to find zstd.h. See config.log for details." "$LINENO" 5 fi
Re: [PATCH] testsuite: Mention cleanup-13.c test is incompatible with -fcompare-debug [PR94296]
On Wed, 25 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> As this test produces different code depending on whether
> __GCC_HAVE_DWARF2_CFI_ASM macro is defined or not, it is inherently
> incompatible with -fcompare-debug, as with
> -fcompare-debug -fno-asynchronous-unwind-tables -fno-exceptions
> the macro is defined only in the -g case and not otherwise.
> The purpose of the macro is to tell when the compiler is emitting
> .cfi* directives itself and thus it is desirable that user code uses them in
> inline asm as well, so I think it is fine the way it is.
> As -fexceptions makes the test pass even in that case because it emits
> .cfi* directives, the test actually doesn't FAIL even with
> make check-gcc
> RUNTESTFLAGS='--target_board=unix/-fcompare-debug/-fno-asynchronous-unwind-tables/-fno-exceptions
> dg.exp=cleanup-13.c'
> so the intent of this patch is help Martin and others who do run gcc tests
> out of the testsuite with random compiler flags to find out that this is a
> known issue.
>
> Ok for trunk?
OK.
> 2020-03-25 Jakub Jelinek
>
> PR debug/94296
> * gcc.dg/cleanup-13.c: Add a comment that the test is not
> -fcompare-debug compatible with certain other options.
>
> --- gcc/testsuite/gcc.dg/cleanup-13.c.jj 2020-01-12 11:54:37.396398578
> +0100
> +++ gcc/testsuite/gcc.dg/cleanup-13.c 2020-03-25 10:04:37.059790261 +0100
> @@ -1,3 +1,7 @@
> +/* This test is expected to FAIL to compile with
> + -fcompare-debug -fno-asynchronous-unwind-tables -fno-exceptions
> + because in that case, the __GCC_HAVE_DWARF2_CFI_ASM macro is only
> + defined during -g compilation and not defined with -g0. */
> /* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
> /* { dg-do run } */
> /* { dg-options "-fexceptions" } */
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308]
On Wed, Mar 25, 2020 at 9:05 AM Jakub Jelinek wrote:
>
> Hi!
>
> The following patch ICEs due to my recent change r10-6451-gb7b3378f91c.
> Since that patch, for explicit vzeroupper in the sources (when an intrinsic
> is used), we start with the *avx_vzeroupper_1 pattern which contains just the
> UNSPECV_VZEROUPPER and no sets/clobbers. The vzeroupper pass then adds some
> sets to those, but doesn't add clobbers and finally there is an
> && epilogue_completed splitter that splits this into the *avx_vzeroupper
> pattern which has the right number of sets/clobbers (16 on 64-bit, 8 on
> 32-bit) + the UNSPECV_VZEROUPPER first.
> The problem with this testcase on !TARGET_64BIT is that the vzeroupper pass
> adds 8 sets to the pattern, i.e. the maximum number, but INSN_CODE stays
> to be the one of the *avx_vzeroupper_1 pattern. The splitter doesn't do
> anything here, because it sees the number of rtxes in the PARALLEL already
> the right count, but during final we see that the *avx_vzeroupper_1 pattern
> has "#" output template and ICE that we forgot to split it.
>
> The following patch fixes it by forcing re-recognition of the insn after we
> make the changes to it in ix86_add_reg_usage_to_vzeroupper. Anything that
> will call recog_memoized later on will recog it and find out it is in this
> case already *avx_vzeroupper rather than *avx_vzeroupper_1.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-03-25 Jakub Jelinek
>
> PR target/94308
> * config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): Set
> INSN_CODE (insn) to -1 when changing the pattern.
>
> * gcc.target/i386/pr94308.c: New test.
OK.
Thanks,
Uros.
>
> --- gcc/config/i386/i386-features.c.jj 2020-03-17 13:50:52.955933209 +0100
> +++ gcc/config/i386/i386-features.c 2020-03-24 19:19:17.801609289 +0100
> @@ -1792,6 +1792,7 @@ ix86_add_reg_usage_to_vzeroupper (rtx_in
>RTVEC_ELT (vec, j) = gen_rtx_SET (reg, reg);
> }
>XVEC (pattern, 0) = vec;
> + INSN_CODE (insn) = -1;
>df_insn_rescan (insn);
> }
>
> --- gcc/testsuite/gcc.target/i386/pr94308.c.jj 2020-03-24 19:32:51.964436310
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr94308.c 2020-03-24 19:32:39.848617482
> +0100
> @@ -0,0 +1,31 @@
> +/* PR target/94308 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mfpmath=sse -mavx2 -mfma" } */
> +
> +#include
> +
> +void
> +foo (float *x, const float *y, const float *z, unsigned int w)
> +{
> + unsigned int a;
> + const unsigned int b = w / 8;
> + const float *c = y;
> + const float *d = z;
> + __m256 e = _mm256_setzero_ps ();
> + __m256 f, g;
> + for (a = 0; a < b; a++)
> +{
> + f = _mm256_loadu_ps (c);
> + g = _mm256_loadu_ps (d);
> + c += 8;
> + d += 8;
> + e = _mm256_fmadd_ps (f, g, e);
> +}
> + __attribute__ ((aligned (32))) float h[8];
> + _mm256_storeu_ps (h, e);
> + _mm256_zeroupper ();
> + float i = h[0] + h[1] + h[2] + h[3] + h[4] + h[5] + h[6] + h[7];
> + for (a = b * 8; a < w; a++)
> +i += (*c++) * (*d++);
> + *x = i;
> +}
>
> Jakub
>
Re: [PATCH] testsuite: Fix up FAILs in gfortran testsuite with -fcompare-debug [PR94280]
On Wed, Mar 25, 2020 at 10:41:57AM +0100, Tobias Burnus wrote: > Hence, I in my opinion the comment should be: > "! The compiler_options() function is dependent" … > > OK with that change, unless compiler_version() indeed > makes a difference – but then I would like to understand why. You're right, sorry for messing this up. Initially when I looked at it, I didn't notice compiler_options() and just saw compiler_version() and thought it behaved like -grecord-gcc-options which puts both version and compiler options into DW_AT_producer. It is indeed just compiler_options(). Jakub
Re: [PATCH] testsuite: Fix up FAILs in gfortran testsuite with -fcompare-debug [PR94280]
Hi Jakub,
thanks for the patch and to Martin for doing all the testing.
However, I fail so see why "compiler_version" should be
dependent on the debug flag. In the code, it is just:
"GCC version %s", version_string);
Hence, I in my opinion the comment should be:
"! The compiler_options() function is dependent" …
OK with that change, unless compiler_version() indeed
makes a difference – but then I would like to understand why.
Cheers,
Tobias
On 3/25/20 10:26 AM, Jakub Jelinek via Fortran wrote:
Hi!
These 3 tests use compiler_version() and/or compiler_options() functions,
which are inherently incompatible with -fcompare-debug compilation, as they
emit into a string literal in the assembly the exact f951 command line
options, which differs between the two compilations with -fcompare-debug,
where one has -gtoggle and -fcompare-debug-second options added and
different -fdump-final-insns= option argument.
The following patch adds dg-skip-if directives, so that these tests are
ignored during
make check-gfortran RUNTESTFLAGS='--target_board=unix/-fcompare-debug'
Tested on x86_64-linux without (where the 3 tests FAIL) and with the patch, ok
for
trunk?
2020-03-25 Jakub Jelinek
PR debug/94280
* gfortran.dg/iso_c_binding_compiler_1.f90: Add dg-skip-if for
-fcompare-debug.
* gfortran.dg/iso_c_binding_compiler_3.f90: Likewise.
* gfortran.dg/unlimited_polymorphic_31.f03: Likewise.
--- gcc/testsuite/gfortran.dg/iso_c_binding_compiler_1.f90.jj 2020-01-12
11:54:38.260385543 +0100
+++ gcc/testsuite/gfortran.dg/iso_c_binding_compiler_1.f902020-03-25
10:11:25.175595427 +0100
@@ -1,4 +1,7 @@
! { dg-do link }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } { "-fcompare-debug" } {
"" } } */
!
! PR fortran/40569
!
--- gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90.jj 2020-01-12
11:54:38.260385543 +0100
+++ gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f902020-03-25
10:11:42.208336885 +0100
@@ -1,5 +1,8 @@
! { dg-do compile }
! { dg-options "-Wall" }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } { "-fcompare-debug" } {
"" } } */
!
! PR fortran/45823
!
--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_31.f03.jj 2020-03-02
13:33:10.969494283 +0100
+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_31.f032020-03-25
10:12:08.222942008 +0100
@@ -1,4 +1,7 @@
! { dg-do run }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } { "-fcompare-debug" } {
"" } } */
!
! Test the fix for PR92785, where the array passed to 'write scalar' was not
! normalised to LBOUND = 1.
Jakub
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
[PATCH] testsuite: Fix up FAILs in gfortran testsuite with -fcompare-debug [PR94280]
Hi!
These 3 tests use compiler_version() and/or compiler_options() functions,
which are inherently incompatible with -fcompare-debug compilation, as they
emit into a string literal in the assembly the exact f951 command line
options, which differs between the two compilations with -fcompare-debug,
where one has -gtoggle and -fcompare-debug-second options added and
different -fdump-final-insns= option argument.
The following patch adds dg-skip-if directives, so that these tests are
ignored during
make check-gfortran RUNTESTFLAGS='--target_board=unix/-fcompare-debug'
Tested on x86_64-linux without (where the 3 tests FAIL) and with the patch, ok
for
trunk?
2020-03-25 Jakub Jelinek
PR debug/94280
* gfortran.dg/iso_c_binding_compiler_1.f90: Add dg-skip-if for
-fcompare-debug.
* gfortran.dg/iso_c_binding_compiler_3.f90: Likewise.
* gfortran.dg/unlimited_polymorphic_31.f03: Likewise.
--- gcc/testsuite/gfortran.dg/iso_c_binding_compiler_1.f90.jj 2020-01-12
11:54:38.260385543 +0100
+++ gcc/testsuite/gfortran.dg/iso_c_binding_compiler_1.f90 2020-03-25
10:11:25.175595427 +0100
@@ -1,4 +1,7 @@
! { dg-do link }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } {
"-fcompare-debug" } { "" } } */
!
! PR fortran/40569
!
--- gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90.jj 2020-01-12
11:54:38.260385543 +0100
+++ gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90 2020-03-25
10:11:42.208336885 +0100
@@ -1,5 +1,8 @@
! { dg-do compile }
! { dg-options "-Wall" }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } {
"-fcompare-debug" } { "" } } */
!
! PR fortran/45823
!
--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_31.f03.jj 2020-03-02
13:33:10.969494283 +0100
+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_31.f03 2020-03-25
10:12:08.222942008 +0100
@@ -1,4 +1,7 @@
! { dg-do run }
+! The compiler_version() or compiler_options() functions are dependent on the
+! command line options and thus incompatible with -fcompare-debug.
+! { dg-skip-if "-fcompare-debug incompatible test" { *-*-* } {
"-fcompare-debug" } { "" } } */
!
! Test the fix for PR92785, where the array passed to 'write scalar' was not
! normalised to LBOUND = 1.
Jakub
[PATCH] testsuite: Mention cleanup-13.c test is incompatible with -fcompare-debug [PR94296]
Hi!
As this test produces different code depending on whether
__GCC_HAVE_DWARF2_CFI_ASM macro is defined or not, it is inherently
incompatible with -fcompare-debug, as with
-fcompare-debug -fno-asynchronous-unwind-tables -fno-exceptions
the macro is defined only in the -g case and not otherwise.
The purpose of the macro is to tell when the compiler is emitting
.cfi* directives itself and thus it is desirable that user code uses them in
inline asm as well, so I think it is fine the way it is.
As -fexceptions makes the test pass even in that case because it emits
.cfi* directives, the test actually doesn't FAIL even with
make check-gcc
RUNTESTFLAGS='--target_board=unix/-fcompare-debug/-fno-asynchronous-unwind-tables/-fno-exceptions
dg.exp=cleanup-13.c'
so the intent of this patch is help Martin and others who do run gcc tests
out of the testsuite with random compiler flags to find out that this is a
known issue.
Ok for trunk?
2020-03-25 Jakub Jelinek
PR debug/94296
* gcc.dg/cleanup-13.c: Add a comment that the test is not
-fcompare-debug compatible with certain other options.
--- gcc/testsuite/gcc.dg/cleanup-13.c.jj2020-01-12 11:54:37.396398578
+0100
+++ gcc/testsuite/gcc.dg/cleanup-13.c 2020-03-25 10:04:37.059790261 +0100
@@ -1,3 +1,7 @@
+/* This test is expected to FAIL to compile with
+ -fcompare-debug -fno-asynchronous-unwind-tables -fno-exceptions
+ because in that case, the __GCC_HAVE_DWARF2_CFI_ASM macro is only
+ defined during -g compilation and not defined with -g0. */
/* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
/* { dg-do run } */
/* { dg-options "-fexceptions" } */
Jakub
Re: [PATCH] PPC64 builtin vec_rlnm() argument order is wrong
Hi Carl, Sorry I didn't reply to this patch yet :-( On Wed, Feb 19, 2020 at 08:16:13AM -0800, Carl Love wrote: > The implemented argument order for the vec_rlnm() builtin for PPC64 is > wrong. The following bugzilla was created for the issue: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93819 > I included a patch to fix the issue in the patch. Per the request from > Martin Liska I am posting the patch as well to the GCC mailing list. Please use a changelog entry like * config/rs6000/altivec.h (vec_rlmn): Fix swapped arguments. > If someone can verify the issue and the fix, it would be appreciated. It looks correct, yes. Is there some test that could catch this? And similar cases (*are* there any similar builtins / macros / etc.?) Okay for trunk either way. Thanks! Also okay for backporting, after letting it simmer for a bit. Segher > vec_rlnm fix to make builtin work according to ABI > > --- > gcc/config/rs6000/altivec.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h > index e0b6547c6..5f1f59244 100644 > --- a/gcc/config/rs6000/altivec.h > +++ b/gcc/config/rs6000/altivec.h > @@ -182,7 +182,7 @@ > #define vec_recipdiv __builtin_vec_recipdiv > #define vec_rlmi __builtin_vec_rlmi > #define vec_vrlnm __builtin_vec_rlnm > -#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((b)<<8)|(c))) > +#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((c)<<8)|(b))) > #define vec_rsqrt __builtin_vec_rsqrt > #define vec_rsqrte __builtin_vec_rsqrte > #define vec_signed __builtin_vec_vsigned
Re: [PATCH v2] Fix PR90332 by extending half size vector mode
On Tue, Mar 24, 2020 at 9:30 AM Kewen.Lin wrote: > > Hi, > > on 2020/3/18 下午11:10, Richard Biener wrote: > > On Wed, Mar 18, 2020 at 2:56 PM Kewen.Lin wrote: > >> > >> Hi Richi, > >> > >> Thanks for your comments. > >> > >> on 2020/3/18 下午6:39, Richard Biener wrote: > >>> On Wed, Mar 18, 2020 at 11:06 AM Kewen.Lin wrote: > > >> This path can define overrun_p to false, some case can fall into > >> "no peeling for gaps" hunk in vectorizable_load. Since I used > >> DR_GROUP_HALF_MODE to save the half mode, if some case matches > >> this condition, vectorizable_load hunk can get unitialized > >> DR_GROUP_HALF_MODE. But even with proposed recomputing way, I > >> think we still need to check the vec_init optab here if the > >> know_eq half size conditions hold? > > > > Hmm, but for the above case it's fine to access the excess elements. > > > > I guess the vectorizable_load code needs to be amended with > > the alignment check or we do need to store somewhere our > > decision to use smaller loads. > > > > OK, thanks. I'll investigate it separately. > > >> > >>> I don't like storing DR_GROUP_HALF_MODE very much, later > >>> you need a vector type and it looks cheap enough to recompute > >>> it where you need it? Iff then it doesn't belong to DR_GROUP > >>> but to the stmt-info. > >>> > >> > >> OK, I was intended not to recompute it for time saving, will > >> throw it away. > >> > >>> I realize the original optimization was kind of a hack (and I was too > >>> lazy to implement the integer mode construction path ...). > >>> > >>> So, can you factor out the existing code into a function returning > >>> the vector type for construction for a vector type and a > >>> pieces size? So for V16QI and a pieces-size of 4 we'd > >>> get either V16QI back (then construction from V4QI pieces > >>> should work) or V4SI (then construction from SImode pieces > >>> should work)? Eventually as secondary output provide that > >>> piece type (SI / V4QI). > >> > >> Sure. I'm very poor to get a function name, does function name > >> suitable_vector_and_pieces sound good? > >> ie. tree suitable_vector_and_pieces (tree vtype, tree *ptype); > > > > tree vector_vector_composition_type (tree vtype, poly_uint64 nelts, > > tree *ptype); > > > > where nelts specifies the number of vtype elements in a piece. > > > > Thanks, yep, "nelts" I forgot to get it. > > The new version with refactoring has been attached. > Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8 and P9. > > Is it ok for trunk? Yes. Thanks, Richard. > BR, > Kewen > - > gcc/ChangeLog > > 2020-MM-DD Kewen Lin > > PR tree-optimization/90332 > * gcc/tree-vect-stmts.c (vector_vector_composition_type): New > function. > (get_group_load_store_type): Adjust to call > vector_vector_composition_type, > extend it to construct with scalar types. > (vectorizable_load): Likewise. > > -
Re: [PATCH] varasm: Fix output_constructor where a RANGE_EXPR index needs to skip some elts [PR94303]
On Wed, 25 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled, because output_constructor doesn't
> output the initializer correctly. The FE creates {[1...2] = 9} in this
> case, and we emit .long 9; long 9; .zero 8 instead of the expected
> .zero 8; .long 9; .long 9. If the CONSTRUCTOR is {[1] = 9, [2] = 9},
> output_constructor_regular_field has code to notice that the current
> location (local->total_bytes) is smaller than the location we want to write
> to (1*sizeof(elt)) and will call assemble_zeros to skip those. But
> RANGE_EXPRs are handled by a different function which didn't do this,
> so for RANGE_EXPRs we emitted them properly only if local->total_bytes
> was always equal to the location where the RANGE_EXPR needs to start.
Ouch.
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
OK.
Thanks,
Richard.
> 2020-03-25 Jakub Jelinek
>
> PR middle-end/94303
> * varasm.c (output_constructor_array_range): If local->index
> RANGE_EXPR doesn't start at the current location in the constructor,
> skip needed number of bytes using assemble_zeros or assert we don't
> go backwards.
>
> PR middle-end/94303
> * g++.dg/torture/pr94303.C: New test.
>
> --- gcc/varasm.c.jj 2020-01-22 10:19:24.0 +0100
> +++ gcc/varasm.c 2020-03-24 18:03:08.532690584 +0100
> @@ -5152,6 +5152,26 @@ struct oc_local_state {
> static void
> output_constructor_array_range (oc_local_state *local)
> {
> + /* Perform the index calculation in modulo arithmetic but
> + sign-extend the result because Ada has negative DECL_FIELD_OFFSETs
> + but we are using an unsigned sizetype. */
> + unsigned prec = TYPE_PRECISION (sizetype);
> + offset_int idx = wi::sext (wi::to_offset (TREE_OPERAND (local->index, 0))
> + - wi::to_offset (local->min_index), prec);
> + tree valtype = TREE_TYPE (local->val);
> + HOST_WIDE_INT fieldpos
> += (idx * wi::to_offset (TYPE_SIZE_UNIT (valtype))).to_short_addr ();
> +
> + /* Advance to offset of this element. */
> + if (fieldpos > local->total_bytes)
> +{
> + assemble_zeros (fieldpos - local->total_bytes);
> + local->total_bytes = fieldpos;
> +}
> + else
> +/* Must not go backwards. */
> +gcc_assert (fieldpos == local->total_bytes);
> +
>unsigned HOST_WIDE_INT fieldsize
> = int_size_in_bytes (TREE_TYPE (local->type));
>
> --- gcc/testsuite/g++.dg/torture/pr94303.C.jj 2020-03-24 18:14:21.114688840
> +0100
> +++ gcc/testsuite/g++.dg/torture/pr94303.C2020-03-24 18:13:56.434055853
> +0100
> @@ -0,0 +1,17 @@
> +// PR middle-end/94303
> +// { dg-do run }
> +
> +struct A {
> + int d = 9;
> + A () = default;
> + A (int x) : d(x) {}
> + void foo () { if (d < 1) __builtin_abort (); }
> +};
> +
> +A a[3] = { 1 };
> +
> +int
> +main ()
> +{
> + a[2].foo ();
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] middle-end: Avoid using DECL_UID in ASM_FORMAT_PRIVATE_NAME [PR94223]
On Wed, 25 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, we don't guarantee DECL_UID to be the same between
> corresponding decls in -g and -g0 builds, -g can create more decls and all
> that is guaranteed is that the DECL_UIDs of the corresponding decls compare
> the same.
> The following testcase gets a -fcompare-debug failure because these
> functions use DECL_UID as the number in ASM_FORMAT_PRIVATE_NAME.
>
> The patch fixes it by using just a sequential number there instead.
> I don't think this can be called during PCH writing, this only happens for
> non-public decls and the C/C++ FEs shouldn't mangling those at that point
> (furthermore C++ FE uses a different set_decl_assembler_name hook and this
> one is something only the gimplifier calls on C. temporaries.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2020-03-25 Jakub Jelinek
>
> PR c++/94223
> * langhooks.c (lhd_set_decl_assembler_name): Use a static ulong
> counter instead of DECL_UID.
>
> * lto-lang.c (lto_set_decl_assembler_name): Use a static ulong
> counter instead of DECL_UID.
>
> * g++.dg/opt/pr94223.C: New test.
>
> --- gcc/langhooks.c.jj2020-01-12 11:54:36.670409531 +0100
> +++ gcc/langhooks.c 2020-03-24 16:05:26.728284808 +0100
> @@ -160,16 +160,17 @@ lhd_set_decl_assembler_name (tree decl)
>
> Can't use just the variable's own name for a variable whose scope
> is less than the whole compilation. Concatenate a distinguishing
> - number - we use the DECL_UID. */
> + number. */
>
>if (TREE_PUBLIC (decl) || DECL_FILE_SCOPE_P (decl))
> id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
>else
> {
>const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
> + static unsigned long num;
>char *label;
>
> - ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> + ASM_FORMAT_PRIVATE_NAME (label, name, num++);
>id = get_identifier (label);
> }
>
> --- gcc/lto/lto-lang.c.jj 2020-01-12 11:54:36.678409411 +0100
> +++ gcc/lto/lto-lang.c2020-03-24 13:55:10.463826320 +0100
> @@ -1179,8 +1179,9 @@ lto_set_decl_assembler_name (tree decl)
> {
>const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
>char *label;
> + static unsigned long num;
>
> - ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> + ASM_FORMAT_PRIVATE_NAME (label, name, num++);
>id = get_identifier (label);
> }
>
> --- gcc/testsuite/g++.dg/opt/pr94223.C.jj 2020-03-24 16:03:42.069863475
> +0100
> +++ gcc/testsuite/g++.dg/opt/pr94223.C2020-03-24 16:04:53.327788620
> +0100
> @@ -0,0 +1,5 @@
> +// PR c++/94223
> +// { dg-do compile }
> +// { dg-options "-O0 -std=c++2a -fcompare-debug" }
> +
> +#include "../cpp1z/init-statement6.C"
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] sccvn: Fix buffer overflow in push_partial_def [PR94300]
On Wed, 25 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled, because there is a buffer overflow
> in push_partial_def in the little-endian case when working 64-byte vectors.
> The code computes the number of bytes we need in the BUFFER: NEEDED_LEN,
> which is rounded up number of bits we need. Then the code
> native_encode_expr each (partially overlapping) pd into THIS_BUFFER.
> If pd.offset < 0, i.e. the pd.rhs store starts at some bits before the
> window we are interested in, we pass -pd.offset to native_encode_expr and
> shrink the size already earlier:
> HOST_WIDE_INT size = pd.size;
> if (pd.offset < 0)
> size -= ROUND_DOWN (-pd.offset, BITS_PER_UNIT);
> On this testcase, the problem is with a store with pd.offset > 0,
> in particular pd.offset 256, pd.size 512, i.e. a 64-byte store which doesn't
> fit into entirely into BUFFER.
> We have just:
> size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
> in this case for little-endian, which isn't sufficient, because needed_len
> is 64, the entire BUFFER (except of the last extra byte used for shifting).
> native_encode_expr fills the whole THIS_BUFFER (again, except the last extra
> byte), and the code then performs memcpy (BUFFER + 32, THIS_BUFFER, 64);
> which overflows BUFFER and as THIS_BUFFER is usually laid out after it,
> overflows it into THIS_BUFFER.
> The following patch fixes it by for pd.offset > 0 making sure size is
> reduced too. For big-endian the code does things differently and already
> handles this right.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2020-03-25 Jakub Jelinek
>
> PR tree-optimization/94300
> * tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): If pd.offset
> is positive, make sure that off + size isn't larger than needed_len.
>
> * gcc.target/i386/avx512f-pr94300.c: New test.
>
> --- gcc/tree-ssa-sccvn.c.jj 2020-03-12 14:28:13.365363769 +0100
> +++ gcc/tree-ssa-sccvn.c 2020-03-24 15:48:22.465737253 +0100
> @@ -2058,6 +2058,8 @@ vn_walk_cb_data::push_partial_def (pd_da
> shift_bytes_in_array_left (this_buffer, len + 1, amnt);
> unsigned int off = pd.offset / BITS_PER_UNIT;
> gcc_assert (off < needed_len);
> + size = MIN (size,
> + (HOST_WIDE_INT) (needed_len - off) * BITS_PER_UNIT);
> p = buffer + off;
> if (amnt + size < BITS_PER_UNIT)
> {
> --- gcc/testsuite/gcc.target/i386/avx512f-pr94300.c.jj2020-03-24
> 15:38:32.391643991 +0100
> +++ gcc/testsuite/gcc.target/i386/avx512f-pr94300.c 2020-03-24
> 15:39:09.874078217 +0100
> @@ -0,0 +1,21 @@
> +/* PR tree-optimization/94300 */
> +/* { dg-do run { target { avx512f } } } */
> +/* { dg-options "-O1 -mavx512f -mprefer-vector-width=512
> -mtune=skylake-avx512" } */
> +
> +#include "avx512f-check.h"
> +
> +typedef double V __attribute__((vector_size (64)));
> +
> +static void
> +avx512f_test (void)
> +{
> + double mem[16];
> + const V a = { 0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0 };
> + const V b = { 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0 };
> + V c;
> + __builtin_memcpy (mem, , 64);
> + __builtin_memcpy (mem + 8, , 64);
> + __builtin_memcpy (, mem + 4, 64);
> + if (c[5] != 9.0)
> +__builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
[PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308]
Hi!
The following patch ICEs due to my recent change r10-6451-gb7b3378f91c.
Since that patch, for explicit vzeroupper in the sources (when an intrinsic
is used), we start with the *avx_vzeroupper_1 pattern which contains just the
UNSPECV_VZEROUPPER and no sets/clobbers. The vzeroupper pass then adds some
sets to those, but doesn't add clobbers and finally there is an
&& epilogue_completed splitter that splits this into the *avx_vzeroupper
pattern which has the right number of sets/clobbers (16 on 64-bit, 8 on
32-bit) + the UNSPECV_VZEROUPPER first.
The problem with this testcase on !TARGET_64BIT is that the vzeroupper pass
adds 8 sets to the pattern, i.e. the maximum number, but INSN_CODE stays
to be the one of the *avx_vzeroupper_1 pattern. The splitter doesn't do
anything here, because it sees the number of rtxes in the PARALLEL already
the right count, but during final we see that the *avx_vzeroupper_1 pattern
has "#" output template and ICE that we forgot to split it.
The following patch fixes it by forcing re-recognition of the insn after we
make the changes to it in ix86_add_reg_usage_to_vzeroupper. Anything that
will call recog_memoized later on will recog it and find out it is in this
case already *avx_vzeroupper rather than *avx_vzeroupper_1.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-03-25 Jakub Jelinek
PR target/94308
* config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): Set
INSN_CODE (insn) to -1 when changing the pattern.
* gcc.target/i386/pr94308.c: New test.
--- gcc/config/i386/i386-features.c.jj 2020-03-17 13:50:52.955933209 +0100
+++ gcc/config/i386/i386-features.c 2020-03-24 19:19:17.801609289 +0100
@@ -1792,6 +1792,7 @@ ix86_add_reg_usage_to_vzeroupper (rtx_in
RTVEC_ELT (vec, j) = gen_rtx_SET (reg, reg);
}
XVEC (pattern, 0) = vec;
+ INSN_CODE (insn) = -1;
df_insn_rescan (insn);
}
--- gcc/testsuite/gcc.target/i386/pr94308.c.jj 2020-03-24 19:32:51.964436310
+0100
+++ gcc/testsuite/gcc.target/i386/pr94308.c 2020-03-24 19:32:39.848617482
+0100
@@ -0,0 +1,31 @@
+/* PR target/94308 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mfpmath=sse -mavx2 -mfma" } */
+
+#include
+
+void
+foo (float *x, const float *y, const float *z, unsigned int w)
+{
+ unsigned int a;
+ const unsigned int b = w / 8;
+ const float *c = y;
+ const float *d = z;
+ __m256 e = _mm256_setzero_ps ();
+ __m256 f, g;
+ for (a = 0; a < b; a++)
+{
+ f = _mm256_loadu_ps (c);
+ g = _mm256_loadu_ps (d);
+ c += 8;
+ d += 8;
+ e = _mm256_fmadd_ps (f, g, e);
+}
+ __attribute__ ((aligned (32))) float h[8];
+ _mm256_storeu_ps (h, e);
+ _mm256_zeroupper ();
+ float i = h[0] + h[1] + h[2] + h[3] + h[4] + h[5] + h[6] + h[7];
+ for (a = b * 8; a < w; a++)
+i += (*c++) * (*d++);
+ *x = i;
+}
Jakub
[PATCH] varasm: Fix output_constructor where a RANGE_EXPR index needs to skip some elts [PR94303]
Hi!
The following testcase is miscompiled, because output_constructor doesn't
output the initializer correctly. The FE creates {[1...2] = 9} in this
case, and we emit .long 9; long 9; .zero 8 instead of the expected
.zero 8; .long 9; .long 9. If the CONSTRUCTOR is {[1] = 9, [2] = 9},
output_constructor_regular_field has code to notice that the current
location (local->total_bytes) is smaller than the location we want to write
to (1*sizeof(elt)) and will call assemble_zeros to skip those. But
RANGE_EXPRs are handled by a different function which didn't do this,
so for RANGE_EXPRs we emitted them properly only if local->total_bytes
was always equal to the location where the RANGE_EXPR needs to start.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2020-03-25 Jakub Jelinek
PR middle-end/94303
* varasm.c (output_constructor_array_range): If local->index
RANGE_EXPR doesn't start at the current location in the constructor,
skip needed number of bytes using assemble_zeros or assert we don't
go backwards.
PR middle-end/94303
* g++.dg/torture/pr94303.C: New test.
--- gcc/varasm.c.jj 2020-01-22 10:19:24.0 +0100
+++ gcc/varasm.c2020-03-24 18:03:08.532690584 +0100
@@ -5152,6 +5152,26 @@ struct oc_local_state {
static void
output_constructor_array_range (oc_local_state *local)
{
+ /* Perform the index calculation in modulo arithmetic but
+ sign-extend the result because Ada has negative DECL_FIELD_OFFSETs
+ but we are using an unsigned sizetype. */
+ unsigned prec = TYPE_PRECISION (sizetype);
+ offset_int idx = wi::sext (wi::to_offset (TREE_OPERAND (local->index, 0))
+- wi::to_offset (local->min_index), prec);
+ tree valtype = TREE_TYPE (local->val);
+ HOST_WIDE_INT fieldpos
+= (idx * wi::to_offset (TYPE_SIZE_UNIT (valtype))).to_short_addr ();
+
+ /* Advance to offset of this element. */
+ if (fieldpos > local->total_bytes)
+{
+ assemble_zeros (fieldpos - local->total_bytes);
+ local->total_bytes = fieldpos;
+}
+ else
+/* Must not go backwards. */
+gcc_assert (fieldpos == local->total_bytes);
+
unsigned HOST_WIDE_INT fieldsize
= int_size_in_bytes (TREE_TYPE (local->type));
--- gcc/testsuite/g++.dg/torture/pr94303.C.jj 2020-03-24 18:14:21.114688840
+0100
+++ gcc/testsuite/g++.dg/torture/pr94303.C 2020-03-24 18:13:56.434055853
+0100
@@ -0,0 +1,17 @@
+// PR middle-end/94303
+// { dg-do run }
+
+struct A {
+ int d = 9;
+ A () = default;
+ A (int x) : d(x) {}
+ void foo () { if (d < 1) __builtin_abort (); }
+};
+
+A a[3] = { 1 };
+
+int
+main ()
+{
+ a[2].foo ();
+}
Jakub
[PATCH] middle-end: Avoid using DECL_UID in ASM_FORMAT_PRIVATE_NAME [PR94223]
Hi!
As mentioned in the PR, we don't guarantee DECL_UID to be the same between
corresponding decls in -g and -g0 builds, -g can create more decls and all
that is guaranteed is that the DECL_UIDs of the corresponding decls compare
the same.
The following testcase gets a -fcompare-debug failure because these
functions use DECL_UID as the number in ASM_FORMAT_PRIVATE_NAME.
The patch fixes it by using just a sequential number there instead.
I don't think this can be called during PCH writing, this only happens for
non-public decls and the C/C++ FEs shouldn't mangling those at that point
(furthermore C++ FE uses a different set_decl_assembler_name hook and this
one is something only the gimplifier calls on C. temporaries.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-03-25 Jakub Jelinek
PR c++/94223
* langhooks.c (lhd_set_decl_assembler_name): Use a static ulong
counter instead of DECL_UID.
* lto-lang.c (lto_set_decl_assembler_name): Use a static ulong
counter instead of DECL_UID.
* g++.dg/opt/pr94223.C: New test.
--- gcc/langhooks.c.jj 2020-01-12 11:54:36.670409531 +0100
+++ gcc/langhooks.c 2020-03-24 16:05:26.728284808 +0100
@@ -160,16 +160,17 @@ lhd_set_decl_assembler_name (tree decl)
Can't use just the variable's own name for a variable whose scope
is less than the whole compilation. Concatenate a distinguishing
- number - we use the DECL_UID. */
+ number. */
if (TREE_PUBLIC (decl) || DECL_FILE_SCOPE_P (decl))
id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
else
{
const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
+ static unsigned long num;
char *label;
- ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
+ ASM_FORMAT_PRIVATE_NAME (label, name, num++);
id = get_identifier (label);
}
--- gcc/lto/lto-lang.c.jj 2020-01-12 11:54:36.678409411 +0100
+++ gcc/lto/lto-lang.c 2020-03-24 13:55:10.463826320 +0100
@@ -1179,8 +1179,9 @@ lto_set_decl_assembler_name (tree decl)
{
const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
char *label;
+ static unsigned long num;
- ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
+ ASM_FORMAT_PRIVATE_NAME (label, name, num++);
id = get_identifier (label);
}
--- gcc/testsuite/g++.dg/opt/pr94223.C.jj 2020-03-24 16:03:42.069863475
+0100
+++ gcc/testsuite/g++.dg/opt/pr94223.C 2020-03-24 16:04:53.327788620 +0100
@@ -0,0 +1,5 @@
+// PR c++/94223
+// { dg-do compile }
+// { dg-options "-O0 -std=c++2a -fcompare-debug" }
+
+#include "../cpp1z/init-statement6.C"
Jakub
[PATCH] sccvn: Fix buffer overflow in push_partial_def [PR94300]
Hi!
The following testcase is miscompiled, because there is a buffer overflow
in push_partial_def in the little-endian case when working 64-byte vectors.
The code computes the number of bytes we need in the BUFFER: NEEDED_LEN,
which is rounded up number of bits we need. Then the code
native_encode_expr each (partially overlapping) pd into THIS_BUFFER.
If pd.offset < 0, i.e. the pd.rhs store starts at some bits before the
window we are interested in, we pass -pd.offset to native_encode_expr and
shrink the size already earlier:
HOST_WIDE_INT size = pd.size;
if (pd.offset < 0)
size -= ROUND_DOWN (-pd.offset, BITS_PER_UNIT);
On this testcase, the problem is with a store with pd.offset > 0,
in particular pd.offset 256, pd.size 512, i.e. a 64-byte store which doesn't
fit into entirely into BUFFER.
We have just:
size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
in this case for little-endian, which isn't sufficient, because needed_len
is 64, the entire BUFFER (except of the last extra byte used for shifting).
native_encode_expr fills the whole THIS_BUFFER (again, except the last extra
byte), and the code then performs memcpy (BUFFER + 32, THIS_BUFFER, 64);
which overflows BUFFER and as THIS_BUFFER is usually laid out after it,
overflows it into THIS_BUFFER.
The following patch fixes it by for pd.offset > 0 making sure size is
reduced too. For big-endian the code does things differently and already
handles this right.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-03-25 Jakub Jelinek
PR tree-optimization/94300
* tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): If pd.offset
is positive, make sure that off + size isn't larger than needed_len.
* gcc.target/i386/avx512f-pr94300.c: New test.
--- gcc/tree-ssa-sccvn.c.jj 2020-03-12 14:28:13.365363769 +0100
+++ gcc/tree-ssa-sccvn.c2020-03-24 15:48:22.465737253 +0100
@@ -2058,6 +2058,8 @@ vn_walk_cb_data::push_partial_def (pd_da
shift_bytes_in_array_left (this_buffer, len + 1, amnt);
unsigned int off = pd.offset / BITS_PER_UNIT;
gcc_assert (off < needed_len);
+ size = MIN (size,
+ (HOST_WIDE_INT) (needed_len - off) * BITS_PER_UNIT);
p = buffer + off;
if (amnt + size < BITS_PER_UNIT)
{
--- gcc/testsuite/gcc.target/i386/avx512f-pr94300.c.jj 2020-03-24
15:38:32.391643991 +0100
+++ gcc/testsuite/gcc.target/i386/avx512f-pr94300.c 2020-03-24
15:39:09.874078217 +0100
@@ -0,0 +1,21 @@
+/* PR tree-optimization/94300 */
+/* { dg-do run { target { avx512f } } } */
+/* { dg-options "-O1 -mavx512f -mprefer-vector-width=512
-mtune=skylake-avx512" } */
+
+#include "avx512f-check.h"
+
+typedef double V __attribute__((vector_size (64)));
+
+static void
+avx512f_test (void)
+{
+ double mem[16];
+ const V a = { 0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0 };
+ const V b = { 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0 };
+ V c;
+ __builtin_memcpy (mem, , 64);
+ __builtin_memcpy (mem + 8, , 64);
+ __builtin_memcpy (, mem + 4, 64);
+ if (c[5] != 9.0)
+__builtin_abort ();
+}
Jakub
[PATCH] arm: Fix ICE caused by arm_gen_discompare_reg [PR94292]
Hi!
The following testcase ICEs, because arm_gen_discompare_reg creates invalid
RTL which then propagates into DEBUG_INSNs and ICEs while handling them.
The problem is that this function emits
(insn 18 17 19 2 (set (reg:CC_DNE 100 cc)
(compare (ior:SI (ne:SI (subreg:SI (reg:DI 129) 0)
(subreg:SI (reg:DI 114 [ _2 ]) 0))
(ne:SI (subreg:SI (reg:DI 129) 4)
(subreg:SI (reg:DI 114 [ _2 ]) 4)))
(const_int 0 [0]))) "pr94292.c":7:11 325 {*cmp_ior}
(nil))
and the invalid thing is that the COMPARE has VOIDmode. Setting a
non-VOIDmode SET_DEST to VOIDmode SET_SRC is only valid if the SET_SRC is
CONST_INT/CONST_DOUBLE.
The following patch fixes it by giving the COMPARE the same mode as it gives
to the SET_DEST cc register.
Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
2020-03-25 Jakub Jelinek
PR target/94292
* config/arm/arm.c (arm_gen_discompare_reg): Set mode of COMPARE to
mode rather than VOIDmode.
* gcc.dg/pr94292.c: New test.
--- gcc/config/arm/arm.c.jj 2020-03-18 19:25:33.0 +0100
+++ gcc/config/arm/arm.c2020-03-24 13:14:38.568689174 +0100
@@ -15763,7 +15763,7 @@ arm_gen_dicompare_reg (rtx_code code, rt
cc_reg = gen_rtx_REG (mode, CC_REGNUM);
emit_insn (gen_rtx_SET (cc_reg,
- gen_rtx_COMPARE (VOIDmode, conjunction,
+ gen_rtx_COMPARE (mode, conjunction,
const0_rtx)));
return cc_reg;
}
--- gcc/testsuite/gcc.dg/pr94292.c.jj 2020-03-24 13:07:12.694449518 +0100
+++ gcc/testsuite/gcc.dg/pr94292.c 2020-03-24 13:06:53.720737198 +0100
@@ -0,0 +1,13 @@
+/* PR target/94292 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -g -fno-tree-dce" } */
+
+unsigned short a;
+unsigned long long b;
+
+long long
+foo (int d)
+{
+ d >>= a != (unsigned long long) -a;
+ return a + b;
+}
Jakub
