Re: [PATCH] Improve debug info in ivopts optimized loops (PR debug/90231)
On Sat, Oct 19, 2019 at 08:27:31AM +0200, Jakub Jelinek wrote: > And as questioned in the PR, are there other cases where we can safely > assume no wrap (e.g. use it if use->iv->no_overflow && cand->iv->no_overflow > without those extra precision checks)? Like, is there a way to find out if an iv_cand has been created from pointer arithmetics or similar and thus at least with -fno-wrapv-pointer shouldn't wrap around? Jakub
[PPC, committed] Delete out of date comment.
As discussed in the list thread on PR65342, and pre-approved by Segher, this removes a comment that's no longer relevant. Bootstrap checked on powerpc64-linux-gnu, applied to mainline thanks Iain gcc/ChangeLog: 2019-10-19 Iain Sandoe * config/rs6000/rs6000.md: Delete out-of-date comment about special-casing integer loads. diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 29dd616520..d0cca1e650 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -6890,11 +6890,6 @@ UNSPEC_MOVSI_GOT))] "") -;; For SI, we special-case integers that can't be loaded in one insn. We -;; do the load 16-bits at a time. We could do this by loading from memory, -;; and this is even supposed to be faster, but it is simpler not to get -;; integers in the TOC. - ;; MR LA LWZ LFIWZX LXSIWZX ;; STW STFIWX STXSIWX LI LIS ;; #XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW
[Darwin, testsuite, committed] Fix Wnonnull on Darwin.
This test has failed always on Darwin, because Darwin does not mark
entries in string.h with nonnull attributes. Since the purpose of the test
is to check that the warnings are issued for an inlined function, not that
the target headers are marked up, we can provide locally marked up
function declarations for Darwin.
tested on x68_64-darwin16, x86_64-linux-gnu,
applied to mainline,
thanks,
Iain.
gcc/testsuite/ChangeLog:
2019-10-19 Iain Sandoe
* gcc.dg/Wnonnull.c: Add attributed function declarations for
memcpy and strlen for Darwin.
diff --git a/gcc/testsuite/gcc.dg/Wnonnull.c b/gcc/testsuite/gcc.dg/Wnonnull.c
index be89a5a755..a165baa99f 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull.c
@@ -2,7 +2,16 @@
{ dg-do compile }
{ dg-options "-O2 -Wall" } */
+#ifndef __APPLE__
#include
+#else
+/* OSX headers do not mark up the nonnull elements yet. */
+# include
+extern size_t strlen (const char *__s)
+ __attribute ((pure)) __attribute ((nonnull (1)));
+extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
+size_t __n) __attribute ((nonnull (1, 2)));
+#endif
char buf[100];
Re: [Darwin, testsuite, committed] Fix Wnonnull on Darwin.
On Okt 19 2019, Iain Sandoe wrote:
> This test has failed always on Darwin, because Darwin does not mark
> entries in string.h with nonnull attributes. Since the purpose of the test
> is to check that the warnings are issued for an inlined function, not that
> the target headers are marked up, we can provide locally marked up
> function declarations for Darwin.
If the test depends on the non-std declarations, then it should use them
everywhere.
> diff --git a/gcc/testsuite/gcc.dg/Wnonnull.c b/gcc/testsuite/gcc.dg/Wnonnull.c
> index be89a5a755..a165baa99f 100644
> --- a/gcc/testsuite/gcc.dg/Wnonnull.c
> +++ b/gcc/testsuite/gcc.dg/Wnonnull.c
> @@ -2,7 +2,16 @@
> { dg-do compile }
> { dg-options "-O2 -Wall" } */
>
> +#ifndef __APPLE__
> #include
> +#else
> +/* OSX headers do not mark up the nonnull elements yet. */
> +# include
> +extern size_t strlen (const char *__s)
> + __attribute ((pure)) __attribute ((nonnull (1)));
> +extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
> + size_t __n) __attribute ((nonnull (1, 2)));
> +#endif
Perhaps use __SIZE_TYPE__ instead of #include ?
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Re: [Darwin, testsuite, committed] Fix Wnonnull on Darwin.
Andreas Schwab wrote:
> On Okt 19 2019, Iain Sandoe wrote:
>
>> This test has failed always on Darwin, because Darwin does not mark
>> entries in string.h with nonnull attributes. Since the purpose of the test
>> is to check that the warnings are issued for an inlined function, not that
>> the target headers are marked up, we can provide locally marked up
>> function declarations for Darwin.
>
> If the test depends on the non-std declarations, then it should use them
> everywhere.
from my perspective, agreed, Martin?
>> diff --git a/gcc/testsuite/gcc.dg/Wnonnull.c
>> b/gcc/testsuite/gcc.dg/Wnonnull.c
>> index be89a5a755..a165baa99f 100644
>> --- a/gcc/testsuite/gcc.dg/Wnonnull.c
>> +++ b/gcc/testsuite/gcc.dg/Wnonnull.c
>> @@ -2,7 +2,16 @@
>>{ dg-do compile }
>>{ dg-options "-O2 -Wall" } */
>>
>> +#ifndef __APPLE__
>> #include
>> +#else
>> +/* OSX headers do not mark up the nonnull elements yet. */
>> +# include
>> +extern size_t strlen (const char *__s)
>> + __attribute ((pure)) __attribute ((nonnull (1)));
>> +extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
>> + size_t __n) __attribute ((nonnull (1, 2)));
>> +#endif
>
> Perhaps use __SIZE_TYPE__ instead of #include ?
if we make the definitions generic, then this would avoid any header includes.
Iain
Re: [wwwdocs] Improve markup/nicer formatting for GIT instructions.
And this makes it a bit nicer (and shorter). Committed, too. Gerald diff --git a/htdocs/about.html b/htdocs/about.html index a812a7f9..48918c83 100644 --- a/htdocs/about.html +++ b/htdocs/about.html @@ -51,13 +51,14 @@ a higher chance of being implemented soon. ;-) Using the git repository Assuming you have both git -and SSH installed, you can check out the web pages as follows: +and SSH installed, you can check out the web pages via git clone git+ssh://username@gcc.gnu.org/git/gcc-wwwdocs.git -where username is your user name at gcc.gnu.org +where username is your user name at gcc.gnu.org. + For anonymous access, use git://gcc.gnu.org/git/gcc-wwwdocs.git instead.
Re: [PATCH] Improve code generation of v += (c == 0) etc. on x86 (PR target/92140)
On Sat, Oct 19, 2019 at 7:54 AM Jakub Jelinek wrote:
>
> Hi!
>
> As mentioned in the PR, x == 0 can be equivalently tested as x < 1U
> and the latter form has the advantage that it sets the carry flag and if it
> is consumed by an instruction that can directly use the carry flag, it is a
> win.
> The following patch adds a couple of (pre-reload only) define_insn_and_split
> to handle the most common cases.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-10-18 Jakub Jelinek
> Uroš Bizjak
>
> PR target/92140
> * config/i386/predicates.md (int_nonimmediate_operand): New special
> predicate.
> * config/i386/i386.md (*add3_eq, *add3_ne,
> *add3_eq_0, *add3_ne_0, *sub3_eq, *sub3_ne,
> *sub3_eq_1, *sub3_eq_0, *sub3_ne_0): New
> define_insn_and_split patterns.
>
> * gcc.target/i386/pr92140.c: New test.
> * gcc.c-torture/execute/pr92140.c: New test.
OK.
Thanks,
Uros.
> --- gcc/config/i386/predicates.md.jj2019-10-07 13:09:06.486261815 +0200
> +++ gcc/config/i386/predicates.md 2019-10-18 15:47:50.781855838 +0200
> @@ -100,6 +100,15 @@ (define_special_predicate "ext_register_
> (match_test "GET_MODE (op) == SImode")
> (match_test "GET_MODE (op) == HImode"
>
> +;; Match a DI, SI, HI or QImode nonimmediate_operand.
> +(define_special_predicate "int_nonimmediate_operand"
> + (and (match_operand 0 "nonimmediate_operand")
> + (ior (and (match_test "TARGET_64BIT")
> +(match_test "GET_MODE (op) == DImode"))
> + (match_test "GET_MODE (op) == SImode")
> + (match_test "GET_MODE (op) == HImode")
> + (match_test "GET_MODE (op) == QImode"
> +
> ;; Match register operands, but include memory operands for TARGET_SSE_MATH.
> (define_predicate "register_ssemem_operand"
>(if_then_else
> --- gcc/config/i386/i386.md.jj 2019-09-20 12:25:48.0 +0200
> +++ gcc/config/i386/i386.md 2019-10-18 15:52:22.697717013 +0200
> @@ -6843,6 +6843,228 @@ (define_insn "*addsi3_zext_cc_overflow_2
>[(set_attr "type" "alu")
> (set_attr "mode" "SI")])
>
> +;; x == 0 with zero flag test can be done also as x < 1U with carry flag
> +;; test, where the latter is preferrable if we have some carry consuming
> +;; instruction.
> +;; For x != 0, we need to use x < 1U with negation of carry, i.e.
> +;; + (1 - CF).
> +(define_insn_and_split "*add3_eq"
> + [(set (match_operand:SWI 0 "nonimmediate_operand")
> + (plus:SWI
> + (plus:SWI
> + (eq:SWI (match_operand 3 "int_nonimmediate_operand") (const_int
> 0))
> + (match_operand:SWI 1 "nonimmediate_operand"))
> + (match_operand:SWI 2 "")))
> + (clobber (reg:CC FLAGS_REG))]
> + "ix86_binary_operator_ok (PLUS, mode, operands)
> + && can_create_pseudo_p ()"
> + "#"
> + "&& 1"
> + [(set (reg:CC FLAGS_REG)
> + (compare:CC (match_dup 3) (const_int 1)))
> + (parallel [(set (match_dup 0)
> + (plus:SWI
> +(plus:SWI (ltu:SWI (reg:CC FLAGS_REG) (const_int 0))
> + (match_dup 1))
> +(match_dup 2)))
> + (clobber (reg:CC FLAGS_REG))])])
> +
> +(define_insn_and_split "*add3_ne"
> + [(set (match_operand:SWI 0 "nonimmediate_operand")
> + (plus:SWI
> + (plus:SWI
> + (ne:SWI (match_operand 3 "int_nonimmediate_operand") (const_int
> 0))
> + (match_operand:SWI 1 "nonimmediate_operand"))
> + (match_operand:SWI 2 "")))
> + (clobber (reg:CC FLAGS_REG))]
> + "CONST_INT_P (operands[2])
> + && (mode != DImode
> + || INTVAL (operands[2]) != HOST_WIDE_INT_C (-0x8000))
> + && ix86_binary_operator_ok (PLUS, mode, operands)
> + && can_create_pseudo_p ()"
> + "#"
> + "&& 1"
> + [(set (reg:CC FLAGS_REG)
> + (compare:CC (match_dup 3) (const_int 1)))
> + (parallel [(set (match_dup 0)
> + (minus:SWI
> +(minus:SWI (match_dup 1)
> + (ltu:SWI (reg:CC FLAGS_REG) (const_int 0)))
> +(match_dup 2)))
> + (clobber (reg:CC FLAGS_REG))])]
> +{
> + operands[2] = gen_int_mode (~INTVAL (operands[2]),
> + mode == DImode ? SImode : mode);
> +})
> +
> +(define_insn_and_split "*add3_eq_0"
> + [(set (match_operand:SWI 0 "nonimmediate_operand")
> + (plus:SWI
> + (eq:SWI (match_operand 2 "int_nonimmediate_operand") (const_int 0))
> + (match_operand:SWI 1 "")))
> + (clobber (reg:CC FLAGS_REG))]
> + "ix86_unary_operator_ok (PLUS, mode, operands)
> + && can_create_pseudo_p ()"
> + "#"
> + "&& 1"
> + [(set (reg:CC FLAGS_REG)
> + (compare:CC (match_dup 2) (const_int 1)))
> + (parallel [(set (match_dup 0)
> + (plus:SWI (ltu:SWI (reg:CC FLAGS_REG) (const_int 0))
> +(match_dup 1)))
> + (clobber (reg:
Re: [PATCH 09/29] [arm] Correctly cost addition with a carry-in
On Fri, Oct 18, 2019 at 08:48:40PM +0100, Richard Earnshaw wrote: > > The cost routine for Arm and Thumb2 was not recognising the idioms that > describe the addition with carry, this results in the instructions > appearing more expensive than they really are, which occasionally can lead > to poor choices by combine. Recognising all the possible variants is > a little trickier than normal because the expressions can become complex > enough that this is no single canonical from. There also is the insn_cost hook, which especially for RISC-like targets is a lot easier to define. Segher
Re: [PATCH, OpenACC] Fortran deviceptr
On 18 October 2019 17:08:54 CEST, Chung-Lin Tang wrote: >Also, I've added a new libgomp.oacc-fortran/deviceptr-2.f90 testcase >that >actually copies out and verifies the deviceptr computation. In testcases please do not 'call abort' which is nonstandard but use 'stop N' which is standard, ideally with different stop integers so one can see easily which test failed. We went through all of the testsuite a while ago to remove the nonstandard abort, FYI. TIA, >Is this okay for trunk now? > >Thanks, >Chung-Lin > >2019-10-18 Cesar Philippidis > Chung-Lin Tang > > gcc/fortran/ > * trans-openmp.c (gfc_omp_finish_clause): Don't create pointer data > mappings for deviceptr clauses. > (gfc_trans_omp_clauses): Likewise. > > gcc/ > * gimplify.c (enum gimplify_omp_var_data): Add GOVD_DEVICETPR. > (omp_notice_variable): Add GOVD_DEVICEPTR attribute when appropriate. > (gimplify_scan_omp_clauses): Likewise. > (gimplify_adjust_omp_clauses_1): Set GOMP_MAP_FORCE_DEVICEPTR for > implicit deviceptr mappings. > gcc/testsuite/ > * c-c++-common/goacc/deviceptr-4.c: Update expected data mapping. > >2019-10-18 Chung-Lin Tang > James Norris > > libgomp/ > * oacc-parallel.c (handle_ftn_pointers): Delete function. > (GOACC_parallel_keyed): Remove call to handle_ftn_pointers. > * testsuite/libgomp.oacc-fortran/deviceptr-1.f90: New test. > * testsuite/libgomp.oacc-fortran/deviceptr-2.f90: New test.
Re: [PATCH, OpenACC] Fortran deviceptr
On 19 October 2019 15:04:39 CEST, Bernhard Reutner-Fischer
wrote:
>On 18 October 2019 17:08:54 CEST, Chung-Lin Tang
> wrote:
>
>>Also, I've added a new libgomp.oacc-fortran/deviceptr-2.f90 testcase
>>that
>>actually copies out and verifies the deviceptr computation.
>
>In testcases please do not 'call abort' which is nonstandard but use
>'stop N' which is standard, ideally with different stop integers so one
>can see easily which test failed.
>
>We went through all of the testsuite a while ago to remove the
>nonstandard abort, FYI.
Like (modulo typos, untested):
$ cat abort_to_stop.awk ; echo EOF
# awk -f ./abort_to_stop.awk < foo.f90 > x && mv x foo.f90
BEGIN { IGNORECASE = 1; i = 1 }
{ while (sub(/call\s\s*abort/, "stop " i)) {let i++;}; print $0; }
EOF
Re: [Darwin, testsuite, committed] Fix Wnonnull on Darwin.
On 10/19/19 2:56 AM, Iain Sandoe wrote:
Andreas Schwab wrote:
On Okt 19 2019, Iain Sandoe wrote:
This test has failed always on Darwin, because Darwin does not mark
entries in string.h with nonnull attributes. Since the purpose of the test
is to check that the warnings are issued for an inlined function, not that
the target headers are marked up, we can provide locally marked up
function declarations for Darwin.
If the test depends on the non-std declarations, then it should use them
everywhere.
from my perspective, agreed, Martin?
I don't see a problem with it. I prefer tests that don't depend
on system headers to avoid these kinds of issues.
That said, it shouldn't matter if the declaration of a built-in
function has the nonnull attribute. As long as the built-in has
one and isn't disabled GCC should use it. I'd be curious to know
what is actually preventing the warning from triggering there.
Martin
diff --git a/gcc/testsuite/gcc.dg/Wnonnull.c b/gcc/testsuite/gcc.dg/Wnonnull.c
index be89a5a755..a165baa99f 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull.c
@@ -2,7 +2,16 @@
{ dg-do compile }
{ dg-options "-O2 -Wall" } */
+#ifndef __APPLE__
#include
+#else
+/* OSX headers do not mark up the nonnull elements yet. */
+# include
+extern size_t strlen (const char *__s)
+ __attribute ((pure)) __attribute ((nonnull (1)));
+extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
+size_t __n) __attribute ((nonnull (1, 2)));
+#endif
Perhaps use __SIZE_TYPE__ instead of #include ?
if we make the definitions generic, then this would avoid any header includes.
Iain
Re: [PATCH 11/29] [arm] Reduce cost of insns that are simple reg-reg moves.
On Fri, Oct 18, 2019 at 08:48:42PM +0100, Richard Earnshaw wrote: > > Consider this sequence during combine: > > Trying 18, 7 -> 22: >18: r118:SI=r122:SI > REG_DEAD r122:SI > 7: r114:SI=0x1-r118:SI-ltu(cc:CC_RSB,0) > REG_DEAD r118:SI > REG_DEAD cc:CC_RSB >22: r1:SI=r114:SI > REG_DEAD r114:SI r122 is dead here. combine will have tried just 7+22 before, and that failed; it now only succeeds because the register copy is removed. All like you say and what your patch is about. But, this is a generic problem, that should be solved in combine itself (whether or not you also want to reduce the insn cost here). Maybe we should simply disallow pseudo-to-pseudo (or even all) copies when combining more than two insns, always? I'll experiment. > We don't want to prevent combine from eliminating such moves, as this > can expose more combine opportunities, but we shouldn't rate them as > profitable in themselves. We can do this be adjusting the costs > slightly so that the benefit of eliminating such a simple insn is > reduced. Yes, but combine should have removed the move in a 2->1 combination already, if it is beneficial: both 18->7 and 7->22 should have combined just fine. This also points to a potential target problem: why are those two combinations not allowed? Segher
Re: [PATCH 00/29] [arm] Rewrite DImode arithmetic support
Hi Richard,
On Fri, Oct 18, 2019 at 08:48:31PM +0100, Richard Earnshaw wrote:
>
> This series of patches rewrites all the DImode arithmetic patterns for
> the Arm backend when compiling for Arm or Thumb2 to split the
> operations during expand (the thumb1 code is unchanged and cannot
> benefit from early splitting as we are unable to expose the carry
> flag).
Very nice :-)
I have a bunch of testcases from when I did something similar for PowerPC
that I wanted to test... But I cannot get your series to apply. Do you
have a git repo I can pull from?
Here is one test case (it's a bit geared towards what our ISA can do):
===
typedef unsigned int u32;
typedef unsigned long long u64;
u64 add(u64 a, u64 b) { return a + b; }
u64 add1(u64 a) { return a + 1; }
u64 add42(u64 a) { return a + 42; }
u64 addm1(u64 a) { return a - 1; }
u64 addff(u64 a) { return a + 0xULL; }
u64 addH(u64 a) { return a + 0x12345678ULL; }
u64 addH0(u64 a) { return a + 0x1234ULL; }
u64 addS(u64 a, u32 b) { return a + b; }
u64 addSH(u64 a, u32 b) { return a + ((u64)b << 32); }
u64 addB1(u64 a) { return a + 0x1ULL; }
u64 addB8(u64 a) { return a + 0x8ULL; }
u64 addSH42(u64 a, u32 b) { return a + ((u64)b << 32) + 42; }
u64 addSHm1(u64 a, u32 b) { return a + ((u64)b << 32) - 1; }
u64 addSHff(u64 a, u32 b) { return a + ((u64)b << 32) + 0xULL; }
===
rs6000 -m32 currently has non-optimal code for addm1, addSHm1; trunk arm
has non-optimal code for addH0, addSH, addB1, addB8, addSH42, addSHm1, and
addSHff if I understand well enough. So I'd love to see what it does with
your series applied :-)
Segher
Re: [Patch, fortran] PR91926 - assumed rank optional
Tobias,
You are quite right to take me to task. As I wrote in the original
message to the list, I was trying to respond rapidly before stepping
out of the door on vacation. The original patch is attached. The fix
to this problem is to revert that part in
libgfortran/runtime/ISO_Fortran_binding.c. As you implicitly surmised,
I was assuming that 'd' would be initialised in the caller. I cannot
see why this should be the case but sometimes the optimizer seems to
cut away a bit too much code :-(
I have done the reversion in r277204 after regtesting, of course.
I am retesting an update to 9-branch, as requested. I will submit to
the list tomorrow.
Cheers
Paul
2019-10-19 Paul Thomas
PR fortran/91926
* runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Revert
the change made on 2019-10-05.
On Thu, 17 Oct 2019 at 14:39, Tobias Burnus wrote:
>
> See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027
> for a tracking bug – I just added also some analysis.
>
> Tobias
>
> PS: A better patch submission, with the actual patch attached, would
> have been nice. Please re-post the committed patch – and the new patch,
> which fixes the fall out. – Thanks!
>
> On 10/9/19 12:26 PM, Paul Richard Thomas wrote:
> > Hi Christophe,
> >
> > Thanks for flagging this up - I am back at base on Saturday and will
> > take it up then.
> >
> > Regards
> >
> > Paul
> >
> > On Wed, 9 Oct 2019 at 11:13, Christophe Lyon
> > wrote:
> >> Hi,
> >>
> >>
> >> On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas
> >> wrote:
> >>> I must apologise not posting this before committing. I left for a
> >>> vacation this morning and I thought that this problem and the one
> >>> posted by Gilles were best fixed before departing. The patch only
> >>> touches the new ISO_Fortran binding feature and so I thought that I
> >>> would be safe to do this.
> >>>
> >>> It was fully regtested and only applies to trunk.
> >>>
> >>> Paul
> >>>
> >>> Author: pault
> >>> Date: Sat Oct 5 08:17:55 2019
> >>> New Revision: 276624
> >>>
> >>> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
> >>> Log:
> >>> 2019-10-05 Paul Thomas
> >>>
> >>> PR fortran/91926
> >>> * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
> >>> assignment of the attribute field to account correctly for an
> >>> assumed shape dummy. Assign separately to the gfc and cfi
> >>> descriptors since the atribute can be different. Add btanch to
> >>> correctly handle missing optional dummies.
> >>>
> >>> 2019-10-05 Paul Thomas
> >>>
> >>> PR fortran/91926
> >>> * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
> >>> * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
> >>> * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
> >>>
> >>> 2019-10-05 Paul Thomas
> >>>
> >>> PR fortran/91926
> >>> * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
> >>> modify the bounds and offset for CFI_other.
> >>>
> >>> Added:
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
> >>> Modified:
> >>> trunk/gcc/fortran/ChangeLog
> >>> trunk/gcc/fortran/trans-expr.c
> >>> trunk/gcc/testsuite/ChangeLog
> >>> trunk/libgfortran/ChangeLog
> >>> trunk/libgfortran/runtime/ISO_Fortran_binding.c
> >>
> >>
> >> Since this was committed (r276624), I have noticed regressions on
> >> arm-linux-gnueabihf:
> >> FAIL: gfortran.dg/ISO_Fortran_binding_11.f90 -O3 -g execution test
> >> I've seen other reports on gcc-testresults too.
> >>
> >> Christophe
> >>
> >
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c (revision 276620)
--- gcc/fortran/trans-expr.c (working copy)
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5202,5208
--- 5202,5210
tree gfc_desc_ptr;
tree type;
tree cond;
+ tree desc_attr;
int attribute;
+ int cfi_attribute;
symbol_attribute attr = gfc_expr_attr (e);
stmtblock_t block;
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5211,5222
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
{
! if (fsym->attr.pointer)
attribute = 0;
! else if (fsym->attr.allocatable)
attribute = 1;
}
if (e->rank != 0)
{
parmse->force_no_tmp = 1;
--- 5213,5232
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
{
! if (attr.pointer)
attribute = 0;
! else if (attr.allocatable)
attribute = 1;
}
+ /* If the formal argument is assumed shape and neither a pointer nor
+ allocatable, it
Avoid recomputing data references in BB SLP
If the first attempt at applying BB SLP to a region fails, the main loop
in vect_slp_bb recomputes the region's bounds and datarefs for the next
vector size. AFAICT this isn't needed any more; we should be able
to reuse the datarefs from the first attempt instead.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2019-10-19 Richard Sandiford
gcc/
* tree-vect-slp.c (vect_slp_analyze_bb_1): Call save_datarefs
when processing the given datarefs for the first time and
check_datarefs subsequently.
(vect_slp_bb_region): New function, split out of...
(vect_slp_bb): ...here. Don't recompute the region bounds and
dataref sets when retrying with a different vector size.
Index: gcc/tree-vect-slp.c
===
*** gcc/tree-vect-slp.c 2019-10-11 15:43:55.439484181 +0100
--- gcc/tree-vect-slp.c 2019-10-19 16:03:30.0 +0100
*** vect_slp_analyze_bb_1 (gimple_stmt_itera
*** 2852,2857
--- 2852,2858
slp_instance instance;
int i;
poly_uint64 min_vf = 2;
+ bool first_time_p = shared->datarefs.is_empty ();
/* The first group of checks is independent of the vector size. */
fatal = true;
*** vect_slp_analyze_bb_1 (gimple_stmt_itera
*** 2871,2877
return NULL;
BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
! bb_vinfo->shared->save_datarefs ();
/* Analyze the data references. */
--- 2872,2881
return NULL;
BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
! if (first_time_p)
! bb_vinfo->shared->save_datarefs ();
! else
! bb_vinfo->shared->check_datarefs ();
/* Analyze the data references. */
*** vect_slp_analyze_bb_1 (gimple_stmt_itera
*** 3007,3022
return bb_vinfo;
}
!
! /* Main entry for the BB vectorizer. Analyze and transform BB, returns
!true if anything in the basic-block was vectorized. */
!
! bool
! vect_slp_bb (basic_block bb)
{
bb_vec_info bb_vinfo;
- gimple_stmt_iterator gsi;
- bool any_vectorized = false;
auto_vector_sizes vector_sizes;
/* Autodetect first vector size we try. */
--- 3011,3028
return bb_vinfo;
}
! /* Subroutine of vect_slp_bb. Try to vectorize the statements between
!REGION_BEGIN (inclusive) and REGION_END (exclusive), returning true
!on success. The region has N_STMTS statements and has the datarefs
!given by DATAREFS. */
!
! static bool
! vect_slp_bb_region (gimple_stmt_iterator region_begin,
! gimple_stmt_iterator region_end,
! vec datarefs,
! unsigned int n_stmts)
{
bb_vec_info bb_vinfo;
auto_vector_sizes vector_sizes;
/* Autodetect first vector size we try. */
*** vect_slp_bb (basic_block bb)
*** 3024,3069
targetm.vectorize.autovectorize_vector_sizes (&vector_sizes, false);
unsigned int next_size = 0;
! gsi = gsi_start_bb (bb);
poly_uint64 autodetected_vector_size = 0;
while (1)
{
- if (gsi_end_p (gsi))
- break;
-
- gimple_stmt_iterator region_begin = gsi;
- vec datarefs = vNULL;
- int insns = 0;
-
- for (; !gsi_end_p (gsi); gsi_next (&gsi))
- {
- gimple *stmt = gsi_stmt (gsi);
- if (is_gimple_debug (stmt))
- continue;
- insns++;
-
- if (gimple_location (stmt) != UNKNOWN_LOCATION)
- vect_location = stmt;
-
- if (!vect_find_stmt_data_reference (NULL, stmt, &datarefs))
- break;
- }
-
- /* Skip leading unhandled stmts. */
- if (gsi_stmt (region_begin) == gsi_stmt (gsi))
- {
- gsi_next (&gsi);
- continue;
- }
-
- gimple_stmt_iterator region_end = gsi;
-
bool vectorized = false;
bool fatal = false;
- vec_info_shared shared;
bb_vinfo = vect_slp_analyze_bb_1 (region_begin, region_end,
! datarefs, insns, fatal, &shared);
if (bb_vinfo
&& dbg_cnt (vect_slp))
{
--- 3030,3044
targetm.vectorize.autovectorize_vector_sizes (&vector_sizes, false);
unsigned int next_size = 0;
! vec_info_shared shared;
poly_uint64 autodetected_vector_size = 0;
while (1)
{
bool vectorized = false;
bool fatal = false;
bb_vinfo = vect_slp_analyze_bb_1 (region_begin, region_end,
! datarefs, n_stmts, fatal, &shared);
if (bb_vinfo
&& dbg_cnt (vect_slp))
{
*** vect_slp_bb (basic_block bb)
*** 3090,3097
}
delete bb_vinfo;
- any_vectorized |= vectorized;
-
if (next_size == 0)
autodetected_vector_size = current_vector_size;
--- 3065,3070
*** vect_slp_bb (basic_block bb)
*** 3105,3137
/* If vect_slp_analyze_bb_1 s
Re: [testsuite] Add test for PR91532
Prathamesh Kulkarni writes:
> Hi Richard,
> Sorry for not adding the test in PR91532 fix.
> Is the attached patch OK to commit ?
>
> Thanks,
> Prathamesh
>
> 2019-10-18 Prathamesh Kulkarni
>
> PR tree-optimization/91532
> testsuite/
> * gcc.target/aarch64/sve/fmla_2.c: Add dg-scan check for deleted store.
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/fmla_2.c
> b/gcc/testsuite/gcc.target/aarch64/sve/fmla_2.c
> index 5c04bcdb3f5..bebb073d1f8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/fmla_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/fmla_2.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-O3" } */
> +/* { dg-options "-O3 -fdump-tree-ifcvt-details" } */
>
> #include
>
> @@ -15,5 +15,6 @@ f (double *restrict a, double *restrict b, double *restrict
> c,
> }
> }
>
> +/* { dg-final { scan-tree-dump-times "Deleted dead store" 1 "ifcvt" } } */
> /* { dg-final { scan-assembler-times {\tfmla\tz[0-9]+\.d, p[0-7]/m,
> z[0-9]+\.d, z[0-9]+\.d\n} 2 } } */
> /* { dg-final { scan-assembler-not {\tfmad\t} } } */
I think it'd be better to have a scan-assembler-times for st1d instead,
so that we're testing the end result rather than how we get there.
Thanks,
Richard
Re: Avoid recomputing data references in BB SLP
On October 19, 2019 5:04:51 PM GMT+02:00, Richard Sandiford
wrote:
>If the first attempt at applying BB SLP to a region fails, the main
>loop
>in vect_slp_bb recomputes the region's bounds and datarefs for the next
>vector size. AFAICT this isn't needed any more; we should be able
>to reuse the datarefs from the first attempt instead.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
OK.
Richard.
>Richard
>
>
>2019-10-19 Richard Sandiford
>
>gcc/
> * tree-vect-slp.c (vect_slp_analyze_bb_1): Call save_datarefs
> when processing the given datarefs for the first time and
> check_datarefs subsequently.
> (vect_slp_bb_region): New function, split out of...
> (vect_slp_bb): ...here. Don't recompute the region bounds and
> dataref sets when retrying with a different vector size.
>
>Index: gcc/tree-vect-slp.c
>===
>*** gcc/tree-vect-slp.c2019-10-11 15:43:55.439484181 +0100
>--- gcc/tree-vect-slp.c2019-10-19 16:03:30.0 +0100
>*** vect_slp_analyze_bb_1 (gimple_stmt_itera
>*** 2852,2857
>--- 2852,2858
>slp_instance instance;
>int i;
>poly_uint64 min_vf = 2;
>+ bool first_time_p = shared->datarefs.is_empty ();
>
>/* The first group of checks is independent of the vector size. */
>fatal = true;
>*** vect_slp_analyze_bb_1 (gimple_stmt_itera
>*** 2871,2877
> return NULL;
>
>BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
>! bb_vinfo->shared->save_datarefs ();
>
>/* Analyze the data references. */
>
>--- 2872,2881
> return NULL;
>
>BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
>! if (first_time_p)
>! bb_vinfo->shared->save_datarefs ();
>! else
>! bb_vinfo->shared->check_datarefs ();
>
>/* Analyze the data references. */
>
>*** vect_slp_analyze_bb_1 (gimple_stmt_itera
>*** 3007,3022
>return bb_vinfo;
> }
>
>!
>! /* Main entry for the BB vectorizer. Analyze and transform BB,
>returns
>!true if anything in the basic-block was vectorized. */
>!
>! bool
>! vect_slp_bb (basic_block bb)
> {
>bb_vec_info bb_vinfo;
>- gimple_stmt_iterator gsi;
>- bool any_vectorized = false;
>auto_vector_sizes vector_sizes;
>
>/* Autodetect first vector size we try. */
>--- 3011,3028
>return bb_vinfo;
> }
>
>! /* Subroutine of vect_slp_bb. Try to vectorize the statements
>between
>!REGION_BEGIN (inclusive) and REGION_END (exclusive), returning
>true
>!on success. The region has N_STMTS statements and has the
>datarefs
>!given by DATAREFS. */
>!
>! static bool
>! vect_slp_bb_region (gimple_stmt_iterator region_begin,
>! gimple_stmt_iterator region_end,
>! vec datarefs,
>! unsigned int n_stmts)
> {
>bb_vec_info bb_vinfo;
>auto_vector_sizes vector_sizes;
>
>/* Autodetect first vector size we try. */
>*** vect_slp_bb (basic_block bb)
>*** 3024,3069
> targetm.vectorize.autovectorize_vector_sizes (&vector_sizes, false);
>unsigned int next_size = 0;
>
>! gsi = gsi_start_bb (bb);
>
>poly_uint64 autodetected_vector_size = 0;
>while (1)
> {
>- if (gsi_end_p (gsi))
>- break;
>-
>- gimple_stmt_iterator region_begin = gsi;
>- vec datarefs = vNULL;
>- int insns = 0;
>-
>- for (; !gsi_end_p (gsi); gsi_next (&gsi))
>- {
>-gimple *stmt = gsi_stmt (gsi);
>-if (is_gimple_debug (stmt))
>- continue;
>-insns++;
>-
>-if (gimple_location (stmt) != UNKNOWN_LOCATION)
>- vect_location = stmt;
>-
>-if (!vect_find_stmt_data_reference (NULL, stmt, &datarefs))
>- break;
>- }
>-
>- /* Skip leading unhandled stmts. */
>- if (gsi_stmt (region_begin) == gsi_stmt (gsi))
>- {
>-gsi_next (&gsi);
>-continue;
>- }
>-
>- gimple_stmt_iterator region_end = gsi;
>-
>bool vectorized = false;
>bool fatal = false;
>- vec_info_shared shared;
>bb_vinfo = vect_slp_analyze_bb_1 (region_begin, region_end,
>! datarefs, insns, fatal, &shared);
>if (bb_vinfo
> && dbg_cnt (vect_slp))
> {
>--- 3030,3044
> targetm.vectorize.autovectorize_vector_sizes (&vector_sizes, false);
>unsigned int next_size = 0;
>
>! vec_info_shared shared;
>
>poly_uint64 autodetected_vector_size = 0;
>while (1)
> {
>bool vectorized = false;
>bool fatal = false;
>bb_vinfo = vect_slp_analyze_bb_1 (region_begin, region_end,
>! datarefs, n_stmts, fatal, &shared);
>if (bb_vinfo
> && dbg_cnt (vect_slp))
> {
>*** vect_slp_bb (basic_block bb)
>*** 3090,3097
> }
>delete bb_vinfo;
>
>- any_vector
Move code out of vect_slp_analyze_bb_1
After the previous patch, it seems more natural to apply the
PARAM_SLP_MAX_INSNS_IN_BB threshold as soon as we know what
the region is, rather than delaying it to vect_slp_analyze_bb_1.
(But rather than carve out the biggest region possible and then
reject it, wouldn't it be better to stop when the region gets
too big, so we at least have a chance of vectorising something?)
It also seems more natural for vect_slp_bb_region to create the
bb_vec_info itself rather than (a) having to pass bits of data down
for the initialisation and (b) forcing vect_slp_analyze_bb_1 to free
on every failure return.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2019-10-19 Richard Sandiford
gcc/
* tree-vect-slp.c (vect_slp_analyze_bb_1): Take a bb_vec_info
and return a boolean success value. Move the allocation and
initialization of the bb_vec_info to...
(vect_slp_bb_region): ...here. Update call accordingly.
(vect_slp_bb): Apply PARAM_SLP_MAX_INSNS_IN_BB here rather
than in vect_slp_analyze_bb_1.
Index: gcc/tree-vect-slp.c
===
--- gcc/tree-vect-slp.c 2019-10-19 15:59:53.875379473 +0100
+++ gcc/tree-vect-slp.c 2019-10-19 16:01:13.714824484 +0100
@@ -2836,47 +2836,24 @@ vect_bb_vectorization_profitable_p (bb_v
return true;
}
-/* Check if the basic block can be vectorized. Returns a bb_vec_info
- if so and sets fatal to true if failure is independent of
- current_vector_size. */
-
-static bb_vec_info
-vect_slp_analyze_bb_1 (gimple_stmt_iterator region_begin,
- gimple_stmt_iterator region_end,
- vec datarefs, int n_stmts,
- bool &fatal, vec_info_shared *shared)
+/* Check if the region described by BB_VINFO can be vectorized, returning
+ true if so. When returning false, set FATAL to true if the same failure
+ would prevent vectorization at other vector sizes, false if it is still
+ worth trying other sizes. N_STMTS is the number of statements in the
+ region. */
+
+static bool
+vect_slp_analyze_bb_1 (bb_vec_info bb_vinfo, int n_stmts, bool &fatal)
{
DUMP_VECT_SCOPE ("vect_slp_analyze_bb");
- bb_vec_info bb_vinfo;
slp_instance instance;
int i;
poly_uint64 min_vf = 2;
- bool first_time_p = shared->datarefs.is_empty ();
/* The first group of checks is independent of the vector size. */
fatal = true;
- if (n_stmts > PARAM_VALUE (PARAM_SLP_MAX_INSNS_IN_BB))
-{
- if (dump_enabled_p ())
- dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-"not vectorized: too many instructions in "
-"basic block.\n");
- free_data_refs (datarefs);
- return NULL;
-}
-
- bb_vinfo = new _bb_vec_info (region_begin, region_end, shared);
- if (!bb_vinfo)
-return NULL;
-
- BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
- if (first_time_p)
-bb_vinfo->shared->save_datarefs ();
- else
-bb_vinfo->shared->check_datarefs ();
-
/* Analyze the data references. */
if (!vect_analyze_data_refs (bb_vinfo, &min_vf, NULL))
@@ -2885,9 +2862,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"not vectorized: unhandled data-ref in basic "
"block.\n");
-
- delete bb_vinfo;
- return NULL;
+ return false;
}
if (BB_VINFO_DATAREFS (bb_vinfo).length () < 2)
@@ -2896,9 +2871,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"not vectorized: not enough data-refs in "
"basic block.\n");
-
- delete bb_vinfo;
- return NULL;
+ return false;
}
if (!vect_analyze_data_ref_accesses (bb_vinfo))
@@ -2907,9 +2880,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"not vectorized: unhandled data access in "
"basic block.\n");
-
- delete bb_vinfo;
- return NULL;
+ return false;
}
/* If there are no grouped stores in the region there is no need
@@ -2921,9 +2892,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"not vectorized: no grouped stores in "
"basic block.\n");
-
- delete bb_vinfo;
- return NULL;
+ return false;
}
/* While the rest of the analysis below depends on it in some way. */
@@ -2943,9 +2912,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
"not vectorized: failed to find SLP opportunities "
"in basic block.\n");
}
-
- delete bb_vinfo;
- return NULL;
+ return false;
}
vect_record_base_alignments (bb_vinfo);
Re: Move code out of vect_slp_analyze_bb_1
On October 19, 2019 5:06:40 PM GMT+02:00, Richard Sandiford
wrote:
>After the previous patch, it seems more natural to apply the
>PARAM_SLP_MAX_INSNS_IN_BB threshold as soon as we know what
>the region is, rather than delaying it to vect_slp_analyze_bb_1.
>(But rather than carve out the biggest region possible and then
>reject it, wouldn't it be better to stop when the region gets
>too big, so we at least have a chance of vectorising something?)
>
>It also seems more natural for vect_slp_bb_region to create the
>bb_vec_info itself rather than (a) having to pass bits of data down
>for the initialisation and (b) forcing vect_slp_analyze_bb_1 to free
>on every failure return.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Ok. But I wonder what the reason was for this limit? Dependency analysis was
greatly simplified, being no longer quadratic here. Can you check where the
limit originally came from? But indeed splitting the region makes more sense
then, but at dataref group boundaries I'd say.
Richard.
>Richard
>
>
>2019-10-19 Richard Sandiford
>
>gcc/
> * tree-vect-slp.c (vect_slp_analyze_bb_1): Take a bb_vec_info
> and return a boolean success value. Move the allocation and
> initialization of the bb_vec_info to...
> (vect_slp_bb_region): ...here. Update call accordingly.
> (vect_slp_bb): Apply PARAM_SLP_MAX_INSNS_IN_BB here rather
> than in vect_slp_analyze_bb_1.
>
>Index: gcc/tree-vect-slp.c
>===
>--- gcc/tree-vect-slp.c2019-10-19 15:59:53.875379473 +0100
>+++ gcc/tree-vect-slp.c2019-10-19 16:01:13.714824484 +0100
>@@ -2836,47 +2836,24 @@ vect_bb_vectorization_profitable_p (bb_v
> return true;
> }
>
>-/* Check if the basic block can be vectorized. Returns a bb_vec_info
>- if so and sets fatal to true if failure is independent of
>- current_vector_size. */
>-
>-static bb_vec_info
>-vect_slp_analyze_bb_1 (gimple_stmt_iterator region_begin,
>- gimple_stmt_iterator region_end,
>- vec datarefs, int n_stmts,
>- bool &fatal, vec_info_shared *shared)
>+/* Check if the region described by BB_VINFO can be vectorized,
>returning
>+ true if so. When returning false, set FATAL to true if the same
>failure
>+ would prevent vectorization at other vector sizes, false if it is
>still
>+ worth trying other sizes. N_STMTS is the number of statements in
>the
>+ region. */
>+
>+static bool
>+vect_slp_analyze_bb_1 (bb_vec_info bb_vinfo, int n_stmts, bool &fatal)
> {
> DUMP_VECT_SCOPE ("vect_slp_analyze_bb");
>
>- bb_vec_info bb_vinfo;
> slp_instance instance;
> int i;
> poly_uint64 min_vf = 2;
>- bool first_time_p = shared->datarefs.is_empty ();
>
> /* The first group of checks is independent of the vector size. */
> fatal = true;
>
>- if (n_stmts > PARAM_VALUE (PARAM_SLP_MAX_INSNS_IN_BB))
>-{
>- if (dump_enabled_p ())
>- dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>- "not vectorized: too many instructions in "
>- "basic block.\n");
>- free_data_refs (datarefs);
>- return NULL;
>-}
>-
>- bb_vinfo = new _bb_vec_info (region_begin, region_end, shared);
>- if (!bb_vinfo)
>-return NULL;
>-
>- BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
>- if (first_time_p)
>-bb_vinfo->shared->save_datarefs ();
>- else
>-bb_vinfo->shared->check_datarefs ();
>-
> /* Analyze the data references. */
>
> if (!vect_analyze_data_refs (bb_vinfo, &min_vf, NULL))
>@@ -2885,9 +2862,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>"not vectorized: unhandled data-ref in basic "
>"block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> if (BB_VINFO_DATAREFS (bb_vinfo).length () < 2)
>@@ -2896,9 +2871,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>"not vectorized: not enough data-refs in "
>"basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> if (!vect_analyze_data_ref_accesses (bb_vinfo))
>@@ -2907,9 +2880,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
>dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: unhandled data access in "
> "basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> /* If there are no grouped stores in the region there is no need
>@@ -2921,9 +2892,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>"not vectorized: no grouped stores in "
>"basic
Re: [ C++ ] [ PATCH ] [ RFC ] p1301 - [[nodiscard("should have a reason")]]
Hi!
On Sat, Oct 19, 2019 at 12:46:56AM -0400, Jason Merrill wrote:
> gcc/testsuite/g++.dg/cpp2a
> * nodiscard-construct.C: New test.
> * nodiscard-once.C: New test.
> * nodiscard-reason-nonstring.C: New test.
> * nodiscard-reason-only-one.C: New test.
> * nodiscard-reason.C: New test.
Unfortunately, the tests break testing with check-c++-all,
ERROR: tcl error sourcing /home/jakub/src/gcc/gcc/testsuite/g++.dg/dg.exp.
ERROR: couldn't compile regular expression pattern: quantifier operand invalid
while executing
"regsub -all "(^|\n)(\[^\n\]+$line\[^\n\]*($pattern)\[^\n\]*\n?)+" $comp_output
"\n" comp_output"
(procedure "saved-dg-test" line 125)
invoked from within
"saved-dg-test
/home/jakub/src/gcc/gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-only-one.C {
-std=gnu++2a} { -pedantic-errors -Wno-long-long}"
("eval" body line 1)
invoked from within
"eval saved-dg-test $args "
(procedure "dg-test" line 4)
invoked from within
"dg-test $test "$flags $flags_t" ${default-extra-flags}"
(procedure "g++-dg-runtest" line 40)
invoked from within
"g++-dg-runtest $tests "" $DEFAULT_CXXFLAGS"
(file "/home/jakub/src/gcc/gcc/testsuite/g++.dg/dg.exp" line 46)
invoked from within
"source /home/jakub/src/gcc/gcc/testsuite/g++.dg/dg.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source /home/jakub/src/gcc/gcc/testsuite/g++.dg/dg.exp"
invoked from within
"catch "uplevel #0 source $test_file_name""
testcase /home/jakub/src/gcc/gcc/testsuite/g++.dg/dg.exp completed in 291
seconds
The problem is that tcl allows (?n) only at the start of the regex string,
but for e.g. dg-warning or dg-error, gcc_error_prefix etc. is added to the
start of the regex, e.g. (fatal )?error:
so
{ dg-error "(?n)wrong number of arguments..*nodiscard" }
will result in regex
"(fatal )?error:(?n)wrong number of arguments..*nodiscard"
and tcl will complain.
I've just committed as obvious a mechanical change to unbreak this.
Even with this change there are some FAILs, but not tcl errors already:
FAIL: g++.dg/cpp2a/nodiscard-once.C -std=gnu++2a (test for excess errors)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
16)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
17)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
176)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
178)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
183)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
185)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
190)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for warnings, line
192)
FAIL: g++.dg/cpp2a/nodiscard-reason.C -std=gnu++2a (test for excess errors)
I'll defer to the patch author to resolve those, nodiscard-once is missing
one dg-warning line, but in the other tests there are quite a few warning
differences and the question is if it is desirable to change the testcase to
match what the compiler is doing, or change what the compiler is doing.
2019-10-20 Jakub Jelinek
* g++.dg/cpp2a/nodiscard-reason-only-one.C: In dg-error or dg-warning
remove (?n) uses and replace .* with \[^\n\r]*.
* g++.dg/cpp2a/nodiscard-reason.C: Likewise.
* g++.dg/cpp2a/nodiscard-once.C: Likewise.
* g++.dg/cpp2a/nodiscard-reason-nonstring.C: Likewise.
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-only-one.C.jj 2019-10-19
09:22:16.167872983 +0200
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-only-one.C 2019-10-20
00:19:51.556357897 +0200
@@ -2,7 +2,7 @@
/* { dg-do compile { target c++2a } } */
/* { dg-options "-O -ftrack-macro-expansion=0" } */
-[[nodiscard("not", "allowed")]] int check1 (void); /* { dg-error "(?n)wrong
number of arguments..*nodiscard" } */
+[[nodiscard("not", "allowed")]] int check1 (void); /* { dg-error "wrong number
of arguments.\[^\n\r]*nodiscard" } */
void
test (void)
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-reason.C.jj2019-10-19
09:22:16.167872983 +0200
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-reason.C 2019-10-20
00:21:06.320234961 +0200
@@ -13,8 +13,8 @@ typedef struct { char big[1024]; fnt fn;
struct [[nodiscard("exact_D_message")]] D { int i; D(); ~D(); };
NODIS E check1 (void);
-NODIS void check2 (void); /* { dg-warning "(?n)10:.nodiscard.*exact_message" }
*/
-NODIS int foo; /* { dg-warning "(?n)9:.nodiscard.*exact_message" } */
+NODIS void check2 (void); /* { dg-warning
"10:.nodiscard\[^\n\r]*exact_message" } */
+NODIS int foo; /* { dg-warning "9:.nodiscard\[^\n\r]*exact_message" }
*/
int bar (void);
NODISAI E check3 (void) { return (E)bar (); }
NODIS A check4 (void);
@@ -52,9 +52,9 @@ test (void)
return;
i += check1 ();
i += ({ check1 (); });
- check1 ();
Re: [Patch][Demangler] Fix for complex values
Updated patch that uses `_Complex` and `_Imaginary`
Thanks,
Miguel Saldivar
>From 742b37c88bea0118046ac359cabe5f250d69ee30 Mon Sep 17 00:00:00 2001
From: Miguel Saldivar
Date: Sat, 19 Oct 2019 21:06:07 -0700
Subject: [PATCH] Fix for complex and imaginary values
gcc/libiberty/
* cp-demangle.c (d_print_mod): Print " _Complex" and " _Imaginary",
as opposed to "complex " and "imaginary "
gcc/libiberty/
* testsuite/demangle-expected: Adjust test.
---
libiberty/ChangeLog | 5 +
libiberty/cp-demangle.c | 4 ++--
libiberty/testsuite/demangle-expected | 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 97d9767c2ea..62d5527b95b 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-17 Miguel Saldivar
+ * cp-demangle.c (d_print_mod): Add a space before printing `complex`
+ and `imaginary`, as opposed to after.
+ * testsuite/demangle-expected: Adjust test.
+
2019-10-03 Eduard-Mihai Burtescu
* rust-demangle.c (looks_like_rust): Remove.
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index aa78c86dd44..877ad359be1 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -5977,10 +5977,10 @@ d_print_mod (struct d_print_info *dpi, int options,
d_append_string (dpi, "&&");
return;
case DEMANGLE_COMPONENT_COMPLEX:
- d_append_string (dpi, "complex ");
+ d_append_string (dpi, " _Complex");
return;
case DEMANGLE_COMPONENT_IMAGINARY:
- d_append_string (dpi, "imaginary ");
+ d_append_string (dpi, " _Imaginary");
return;
case DEMANGLE_COMPONENT_PTRMEM_TYPE:
if (d_last_char (dpi) != '(')
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index f21ed00e559..bdeb69ae487 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -1278,7 +1278,7 @@ int& int_if_addable(A*)
#
--format=gnu-v3
_Z3bazIiEvP1AIXszcl3foocvT__ELCf_
-void baz(A*)
+void baz(A*)
#
--format=gnu-v3
_Z3fooI1FEN1XIXszdtcl1PclcvT__EEE5arrayEE4TypeEv
--
2.23.0
On Fri, Oct 18, 2019 at 11:30 AM Miguel Saldivar
wrote:
> The only reason I wanted `float complex` was for interoperability
> between the two other demanglers. Although the go demangler
> does use `_Complex` and `_Imaginary`, so I guess it's sort of split.
> But I agree, `_Complex` and `_Imaginary` is probably the
> better option.
>
> Thanks,
> Miguel Saldivar
>
> On Fri, Oct 18, 2019 at 9:04 AM Ian Lance Taylor wrote:
>
>> On Thu, Oct 17, 2019 at 10:20 PM Miguel Saldivar
>> wrote:
>> >
>> > This is a small fix for Bug 67299, where symbol: `Z1fCf` which would
>> become
>> > `f(float complex)` instead of `f(floatcomplex )`.
>> > I thought this would be the preferred way of printing, because both
>> > `llvm-cxxfilt` and `cpp_filt` both print the the mangled name in this
>> > fashion.
>>
>> Thanks. Personally I think it would be better to change the strings
>> to " _Complex" and " _Imaginary". I'm open to discussion on this.
>>
>> Ian
>>
>> > From 4ca98c0749bae1389594b31ee7f6ef575aafcd8f Mon Sep 17 00:00:00 2001
>> > From: Miguel Saldivar
>> > Date: Thu, 17 Oct 2019 16:36:19 -0700
>> > Subject: [PATCH][Demangler] Small fix for complex values
>> >
>> > gcc/libiberty/
>> > * cp-demangle.c (d_print_mod): Add a space before printing `complex`
>> > and `imaginary`, as opposed to after.
>> >
>> > gcc/libiberty/
>> > * testsuite/demangle-expected: Adjust test.
>> > ---
>> > libiberty/ChangeLog | 5 +
>> > libiberty/cp-demangle.c | 4 ++--
>> > libiberty/testsuite/demangle-expected | 2 +-
>> > 3 files changed, 8 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
>> > index 97d9767c2ea..62d5527b95b 100644
>> > --- a/libiberty/ChangeLog
>> > +++ b/libiberty/ChangeLog
>> > @@ -1,3 +1,8 @@
>> > +2019-10-17 Miguel Saldivar
>> > + * cp-demangle.c (d_print_mod): Add a space before printing `complex`
>> > + and `imaginary`, as opposed to after.
>> > + * testsuite/demangle-expected: Adjust test.
>> > +
>> > 2019-10-03 Eduard-Mihai Burtescu
>> >
>> > * rust-demangle.c (looks_like_rust): Remove.
>> > diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
>> > index aa78c86dd44..bd4dfb785a9 100644
>> > --- a/libiberty/cp-demangle.c
>> > +++ b/libiberty/cp-demangle.c
>> > @@ -5977,10 +5977,10 @@ d_print_mod (struct d_print_info *dpi, int
>> options,
>> >d_append_string (dpi, "&&");
>> >return;
>> > case DEMANGLE_COMPONENT_COMPLEX:
>> > - d_append_string (dpi, "complex ");
>> > + d_append_string (dpi, " complex");
>> >return;
>> > case DEMANGLE_COMPONENT_IMAGINARY:
>> > - d_append_string (dpi, "imaginary ");
>> > + d_append_string (dpi, " imaginary");
>> >return;
>> > case DEMANGLE_COMPONENT_PTRME
Re: [Patch][Demangler] Fix for complex values
On Sat, Oct 19, 2019 at 9:11 PM Miguel Saldivar wrote:
>
> Updated patch that uses `_Complex` and `_Imaginary`
>
> Thanks,
> Miguel Saldivar
>
> From 742b37c88bea0118046ac359cabe5f250d69ee30 Mon Sep 17 00:00:00 2001
> From: Miguel Saldivar
> Date: Sat, 19 Oct 2019 21:06:07 -0700
> Subject: [PATCH] Fix for complex and imaginary values
>
> gcc/libiberty/
> * cp-demangle.c (d_print_mod): Print " _Complex" and " _Imaginary",
> as opposed to "complex " and "imaginary "
>
> gcc/libiberty/
> * testsuite/demangle-expected: Adjust test.
This is OK.
Thanks.
Ian
> ---
> libiberty/ChangeLog | 5 +
> libiberty/cp-demangle.c | 4 ++--
> libiberty/testsuite/demangle-expected | 2 +-
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
> index 97d9767c2ea..62d5527b95b 100644
> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,8 @@
> +2019-10-17 Miguel Saldivar
> + * cp-demangle.c (d_print_mod): Add a space before printing `complex`
> + and `imaginary`, as opposed to after.
> + * testsuite/demangle-expected: Adjust test.
> +
> 2019-10-03 Eduard-Mihai Burtescu
>
> * rust-demangle.c (looks_like_rust): Remove.
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index aa78c86dd44..877ad359be1 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -5977,10 +5977,10 @@ d_print_mod (struct d_print_info *dpi, int options,
>d_append_string (dpi, "&&");
>return;
> case DEMANGLE_COMPONENT_COMPLEX:
> - d_append_string (dpi, "complex ");
> + d_append_string (dpi, " _Complex");
>return;
> case DEMANGLE_COMPONENT_IMAGINARY:
> - d_append_string (dpi, "imaginary ");
> + d_append_string (dpi, " _Imaginary");
>return;
> case DEMANGLE_COMPONENT_PTRMEM_TYPE:
>if (d_last_char (dpi) != '(')
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index f21ed00e559..bdeb69ae487 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -1278,7 +1278,7 @@ int& int_if_addable(A ((*((Y*)(0)))+(*((Y*)(0>*)
> #
> --format=gnu-v3
> _Z3bazIiEvP1AIXszcl3foocvT__ELCf_
> -void baz(A*)
> +void baz(A*)
> #
> --format=gnu-v3
> _Z3fooI1FEN1XIXszdtcl1PclcvT__EEE5arrayEE4TypeEv
> --
> 2.23.0
>
>
> On Fri, Oct 18, 2019 at 11:30 AM Miguel Saldivar
> wrote:
>>
>> The only reason I wanted `float complex` was for interoperability
>> between the two other demanglers. Although the go demangler
>> does use `_Complex` and `_Imaginary`, so I guess it's sort of split.
>> But I agree, `_Complex` and `_Imaginary` is probably the
>> better option.
>>
>> Thanks,
>> Miguel Saldivar
>>
>> On Fri, Oct 18, 2019 at 9:04 AM Ian Lance Taylor wrote:
>>>
>>> On Thu, Oct 17, 2019 at 10:20 PM Miguel Saldivar
>>> wrote:
>>> >
>>> > This is a small fix for Bug 67299, where symbol: `Z1fCf` which would
>>> > become
>>> > `f(float complex)` instead of `f(floatcomplex )`.
>>> > I thought this would be the preferred way of printing, because both
>>> > `llvm-cxxfilt` and `cpp_filt` both print the the mangled name in this
>>> > fashion.
>>>
>>> Thanks. Personally I think it would be better to change the strings
>>> to " _Complex" and " _Imaginary". I'm open to discussion on this.
>>>
>>> Ian
>>>
>>> > From 4ca98c0749bae1389594b31ee7f6ef575aafcd8f Mon Sep 17 00:00:00 2001
>>> > From: Miguel Saldivar
>>> > Date: Thu, 17 Oct 2019 16:36:19 -0700
>>> > Subject: [PATCH][Demangler] Small fix for complex values
>>> >
>>> > gcc/libiberty/
>>> > * cp-demangle.c (d_print_mod): Add a space before printing `complex`
>>> > and `imaginary`, as opposed to after.
>>> >
>>> > gcc/libiberty/
>>> > * testsuite/demangle-expected: Adjust test.
>>> > ---
>>> > libiberty/ChangeLog | 5 +
>>> > libiberty/cp-demangle.c | 4 ++--
>>> > libiberty/testsuite/demangle-expected | 2 +-
>>> > 3 files changed, 8 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
>>> > index 97d9767c2ea..62d5527b95b 100644
>>> > --- a/libiberty/ChangeLog
>>> > +++ b/libiberty/ChangeLog
>>> > @@ -1,3 +1,8 @@
>>> > +2019-10-17 Miguel Saldivar
>>> > + * cp-demangle.c (d_print_mod): Add a space before printing `complex`
>>> > + and `imaginary`, as opposed to after.
>>> > + * testsuite/demangle-expected: Adjust test.
>>> > +
>>> > 2019-10-03 Eduard-Mihai Burtescu
>>> >
>>> > * rust-demangle.c (looks_like_rust): Remove.
>>> > diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
>>> > index aa78c86dd44..bd4dfb785a9 100644
>>> > --- a/libiberty/cp-demangle.c
>>> > +++ b/libiberty/cp-demangle.c
>>> > @@ -5977,10 +5977,10 @@ d_print_mod (struct d_print_info *dpi, int
>>> > options,
>>> >d_append_string (dpi, "&&");
>>> >return;
>>> > case DEMANGLE_COMP
