Re: [PATCH] Add fields to struct gomp_thread for debugging purposes
On Tue, 31 Oct 2017 08:03:22 +0100
Jakub Jelinek wrote:
> On Mon, Oct 30, 2017 at 04:06:15PM -0700, Kevin Buettner wrote:
> > This patch adds a new member named "pthread_id" to the gomp_thread
> > struct. It is initialized in team.c.
>
> That part is reasonable, though it is unclear how the debugger will
> query it (through OMPD, or through hardcoded name lookup of the struct and
> field in libgomp's debug info, something else). But the field certainly
> has to be guarded by #ifdef LIBGOMP_USE_PTHREADS, otherwise it will break
> NVPTX offloading or any other pthread-less libgomp ports.
> Another question is exact placement of the field, struct gomp_thread
> vs. struct gomp_team_state etc. Maybe it is ok, as the pthread_id is
> the same once the thread is created, doesn't change when we create more
> levels.
Assuming we can figure out how to work the rest of it out, I'll submit
a new patch with the appropriate ifdef.
> > It also adds a field named "parent" which is initialized to the thread
> > which created the thread in question. For non-nested parallel
> > regions, this is always the master thread.
>
> What do you need it for and why isn't the current way of querying
> parent (see e.g. omp_get_ancestor_thread_num or omp_get_team_size)
> sufficient for the debugger? Even if gomp_team_state doesn't contain
> pthread_id, perhaps it would be more space and performance efficient
> to store some pointer into struct gomp_team, gomp_team_state/gomp_thread
> structs are in TLS which should be kept as small as possible.
> Why do you care about which thread called pthread_create, rather than
> what actually owns it right now (is the master thread)?
The cases that I'm considering are variants of this example:
#include
#include
int
main (int artc, char **argv)
{
int i = 42;
int x = 10;
omp_set_nested (1);
omp_set_dynamic (0);
#pragma omp parallel num_threads (2) firstprivate(x)
{
int j = 43;
int y = 20;
x += omp_get_thread_num ();
#pragma omp parallel num_threads (2) firstprivate(y)
{
y += omp_get_thread_num ();
#pragma omp critical
printf ("inner threads: x=%d, y=%d\n", x, y);
}
#pragma omp critical
printf ("outer threads: x=%d, y=%d, j=%d\n", x, y, j);
}
printf ("i = %d\n", i);
}
For this example, neither i nor j appear in the innermost parallel
region.
The variable i will be found on the stack of the master thread.
j, however, is a bit more interesting. If the GDB user sets a
breakpoint on the "inner threads..." printf, j will either be found on
the on the stack of the master thread or on the stack of the first
thread spawned by the master thread.
So, in order for the debugger to find j, it should first see if it can
be found in the current thread (under consideration). Next, it should
check the stack of the parent, and then the parent's parent, etc.
I've looked at the implementation of omp_get_ancestor_thread_num(),
but do not see a way to map the value returned to a gomp_thread.
(Actually, it might be made to work for this example, but if we add
one more level of nesting, we'll need to find a parent thread which
does not appear in the thread pool.) However, I admit that there's
much that I don't understand about libgomp, so it's possible that I've
missed something. I'd appreciate a pointer on how the existing
mechanisms might be used if that's the case.
(I should also note that in my testing of the above code, it can't work
at the moment anyway since j is optimized out. I'm assuming that
this can be fixed so that it's not a moot point.)
Kevin
Re: [PATCH] RISC-V: Emit "i" suffix for instructions with immediate operands
Committed, after adding a second space before the email and before the closing
comment.
On Fri, 03 Nov 2017 09:14:25 PDT (-0700), Palmer Dabbelt wrote:
> From: Michael Clark
>
> This changes makes GCC asm output use instruction names that are
> consistent with the RISC-V ISA manual. The assembler accepts
> immediate-operand instructions without the "i" suffix, so this all
> worked before, it's just a bit cleaner to match the ISA manual more
> closely.
>
> gcc/ChangeLog
>
> 2017-10-03 Michael Clark
>
> * config/riscv/riscv.c (riscv_print_operand): Add a 'i' format.
> config/riscv/riscv.md (addsi3): Use 'i' for immediates.
> (adddi3): Likewise.
> (*addsi3_extended): Likewise.
> (*addsi3_extended2): Likewise.
> (si3): Likewise.
> (di3): Likewise.
> (3): Likewise.
> (<*optabe>si3_internal): Likewise.
> (zero_extendqi2): Likewise.
> (*addhi3): Likewise.
> (*xorhi3): Likewise.
> (di3): Likewise.
> (*si3_extend): Likewise.
> (*sge_): Likewise.
> (*slt_): Likewise.
> (*sle_): Likewise.
> ---
> gcc/config/riscv/riscv.c | 8 +++-
> gcc/config/riscv/riscv.md | 36 ++--
> 2 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index f0b05d7eaeda..d7e6bd0f205e 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -2733,7 +2733,8 @@ riscv_memmodel_needs_release_fence (enum memmodel model)
> 'C' Print the integer branch condition for comparison OP.
> 'A' Print the atomic operation suffix for memory model OP.
> 'F' Print a FENCE if the memory model requires a release.
> - 'z' Print x0 if OP is zero, otherwise print OP normally. */
> + 'z' Print x0 if OP is zero, otherwise print OP normally.
> + 'i' Print i if the operand is not a register. */
>
> static void
> riscv_print_operand (FILE *file, rtx op, int letter)
> @@ -2768,6 +2769,11 @@ riscv_print_operand (FILE *file, rtx op, int letter)
> fputs ("fence iorw,ow; ", file);
>break;
>
> +case 'i':
> + if (code != REG)
> +fputs ("i", file);
> + break;
> +
> default:
>switch (code)
> {
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 9f056bbcda4f..53e1db97db7d 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -414,7 +414,7 @@
> (plus:SI (match_operand:SI 1 "register_operand" " r,r")
>(match_operand:SI 2 "arith_operand"" r,I")))]
>""
> - { return TARGET_64BIT ? "addw\t%0,%1,%2" : "add\t%0,%1,%2"; }
> + { return TARGET_64BIT ? "add%i2w\t%0,%1,%2" : "add%i2\t%0,%1,%2"; }
>[(set_attr "type" "arith")
> (set_attr "mode" "SI")])
>
> @@ -423,7 +423,7 @@
> (plus:DI (match_operand:DI 1 "register_operand" " r,r")
>(match_operand:DI 2 "arith_operand"" r,I")))]
>"TARGET_64BIT"
> - "add\t%0,%1,%2"
> + "add%i2\t%0,%1,%2"
>[(set_attr "type" "arith")
> (set_attr "mode" "DI")])
>
> @@ -433,7 +433,7 @@
>(plus:SI (match_operand:SI 1 "register_operand" " r,r")
> (match_operand:SI 2 "arith_operand"" r,I"]
>"TARGET_64BIT"
> - "addw\t%0,%1,%2"
> + "add%i2w\t%0,%1,%2"
>[(set_attr "type" "arith")
> (set_attr "mode" "SI")])
>
> @@ -444,7 +444,7 @@
> (match_operand:DI 2 "arith_operand"" r,I"))
>0)))]
>"TARGET_64BIT"
> - "addw\t%0,%1,%2"
> + "add%i2w\t%0,%1,%2"
>[(set_attr "type" "arith")
> (set_attr "mode" "SI")])
>
> @@ -705,7 +705,7 @@
> (any_div:SI (match_operand:SI 1 "register_operand" " r")
> (match_operand:SI 2 "register_operand" " r")))]
>"TARGET_DIV"
> - { return TARGET_64BIT ? "w\t%0,%1,%2" : "\t%0,%1,%2"; }
> + { return TARGET_64BIT ? "%i2w\t%0,%1,%2" : "%i2\t%0,%1,%2"; }
>[(set_attr "type" "idiv")
> (set_attr "mode" "SI")])
>
> @@ -714,7 +714,7 @@
> (any_div:DI (match_operand:DI 1 "register_operand" " r")
> (match_operand:DI 2 "register_operand" " r")))]
>"TARGET_DIV && TARGET_64BIT"
> - "\t%0,%1,%2"
> + "%i2\t%0,%1,%2"
>[(set_attr "type" "idiv")
> (set_attr "mode" "DI")])
>
> @@ -724,7 +724,7 @@
> (any_div:SI (match_operand:SI 1 "register_operand" " r")
> (match_operand:SI 2 "register_operand" " r"]
>"TARGET_DIV && TARGET_64BIT"
> - "w\t%0,%1,%2"
> + "%i2w\t%0,%1,%2"
>[(set_attr "type" "idiv")
> (set_attr "mode" "DI")])
>
> @@ -928,7 +928,7 @@
> (any_bitwise:X (match_operand:X 1 "register_operand" "%r,r")
> (match_operand:X 2 "arith_operand"" r,I")))]
>""
> - "\t%0,%1,%2"
> + "%i2\t%0,%1,%2"
>[(set_attr "type" "logical")
> (set_attr "mode" "")])
>
> @@ -937,7 +937,7 @@
> (any_bitwise:SI (ma
Re: [PATCH] RISC-V: If -m[no-]strict-align is not passed, assume its value from -mtune
On Fri, 03 Nov 2017 09:14:10 PDT (-0700), Palmer Dabbelt wrote: > From: Andrew Waterman > > 2017-11-03 Andrew Waterman > > * config/riscv/riscv.c (riscv_option_override): Conditionally set > TARGET_STRICT_ALIGN based upon -mtune argument. > --- > gcc/config/riscv/riscv.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c > index b81a2d29fbfd..f0b05d7eaeda 100644 > --- a/gcc/config/riscv/riscv.c > +++ b/gcc/config/riscv/riscv.c > @@ -3772,9 +3772,13 @@ riscv_option_override (void) > >/* Use -mtune's setting for slow_unaligned_access, even when optimizing > for size. For architectures that trap and emulate unaligned accesses, > - the performance cost is too great, even for -Os. */ > - riscv_slow_unaligned_access_p = (cpu->tune_info->slow_unaligned_access > -|| TARGET_STRICT_ALIGN); > + the performance cost is too great, even for -Os. Similarly, if > + -m[no-]strict-align is left unspecified, heed -mtune's advice. */ > + riscv_slow_unaligned_access = (cpu->tune_info->slow_unaligned_access > + || TARGET_STRICT_ALIGN); > + if ((target_flags_explicit & MASK_STRICT_ALIGN) == 0 > + && cpu->tune_info->slow_unaligned_access) > +target_flags |= MASK_STRICT_ALIGN; > >/* If the user hasn't specified a branch cost, use the processor's > default. */ This was a bit broken, we missed a cleanup patch. I committed the following diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c index b81a2d29fbfd..52bbc25d0cce 100644 --- a/gcc/config/riscv/riscv.c +++ b/gcc/config/riscv/riscv.c @@ -3772,9 +3772,13 @@ riscv_option_override (void) /* Use -mtune's setting for slow_unaligned_access, even when optimizing for size. For architectures that trap and emulate unaligned accesses, - the performance cost is too great, even for -Os. */ + the performance cost is too great, even for -Os. Similarly, if + -m[no-]strict-align is left unspecified, heed -mtune's advice. */ riscv_slow_unaligned_access_p = (cpu->tune_info->slow_unaligned_access || TARGET_STRICT_ALIGN); + if ((target_flags_explicit & MASK_STRICT_ALIGN) == 0 + && cpu->tune_info->slow_unaligned_access) +target_flags |= MASK_STRICT_ALIGN; /* If the user hasn't specified a branch cost, use the processor's default. */
Re: [PATCH] RISC-V: Set SLOW_BYTE_ACCESS=1
Committed.
On Fri, 03 Nov 2017 08:34:38 PDT (-0700), Palmer Dabbelt wrote:
> From: Andrew Waterman
>
> When implementing the RISC-V port, I took the name of this macro at
> face value. It appears we were mistaken in what this means, here's a
> quote from the SPARC port that better describes what SLOW_BYTE_ACCESS
> does
>
> /* Nonzero if access to memory by bytes is slow and undesirable.
>For RISC chips, it means that access to memory by bytes is no
>better than access by words when possible, so grab a whole word
>and maybe make use of that. */
>
> I've added the comment to our port as well.
>
> See https://gcc.gnu.org/ml/gcc/2017-08/msg00202.html for more
> discussion. Thanks to Michael Clark and Andrew Pinski for the help!
>
> gcc/ChangeLog
>
> 2017-10-03 Andrew Waterman
>
> * config/riscv/riscv.h (SLOW_BYTE_ACCESS): Change to 1.
> ---
> gcc/config/riscv/riscv.h | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index e53555efe82f..a802a3f8cbbb 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -615,7 +615,12 @@ typedef struct {
> #define MOVE_MAX UNITS_PER_WORD
> #define MAX_MOVE_MAX 8
>
> -#define SLOW_BYTE_ACCESS 0
> +/* The SPARC port says:
> + Nonzero if access to memory by bytes is slow and undesirable.
> + For RISC chips, it means that access to memory by bytes is no
> + better than access by words when possible, so grab a whole word
> + and maybe make use of that. */
> +#define SLOW_BYTE_ACCESS 1
>
> #define SHIFT_COUNT_TRUNCATED 1
Re: [C++ Patch] PR 80955 (Macros expanded in definition of user-defined literals)
On 11/3/2017 7:31 AM, Paolo Carlini wrote:
Hi,
On 02/11/2017 15:42, Jason Merrill wrote:
This is a good suggestion. I have attached the revised patch. Thanks.
OK, thanks!
Thanks Jason.
I was about to volunteer committing the patch but then noticed that
the testcase includes quite a lot, eg, too, which we
never include in the whole C++ testsuite. Can we have something
simpler? Also, we don't need to include the whole and
for a couple of declarations, we can simply provide by hand
the declarations of sprintf and strcmp at the beginning of the file
(plenty of examples in the testsuite). Mukesh, can you please work on
that? Also, please watch out trailing blank lines.
I had included to get the definition of macro PRId64. I
have now modified the test case to remove all includes. I have added the
definition of the macro in the test case and also added declarations of
functions sprintf() strcmp(). I have attached the revised patch. Thanks.
Mukesh
Index: gcc/testsuite/g++.dg/cpp0x/udlit-macros.C
===
--- gcc/testsuite/g++.dg/cpp0x/udlit-macros.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/udlit-macros.C (working copy)
@@ -0,0 +1,32 @@
+// PR c++/80955
+// { dg-do compile { target c++11 } }
+
+extern "C" int sprintf(char *__restrict s,
+ const char *__restrict format, ...);
+extern "C" int strcmp(const char *s1, const char *s2);
+
+# define __PRI64_PREFIX"l"
+# define PRId64 __PRI64_PREFIX "d"
+
+using size_t = decltype(sizeof(0));
+#define _zero
+#define _ID _xx
+int operator""_zero(const char*, size_t) { return 0; }
+int operator""_ID(const char*, size_t) { return 0; }
+
+int main()
+{
+ int i64 = 123;
+ char buf[100];
+ sprintf(buf, "%"PRId64"abc", i64); // { dg-warning "invalid suffix on
literal" }
+ return strcmp(buf, "123abc")
++ ""_zero
++ "bob"_zero
++ R"#(raw
+ string)#"_zero
++ "xx"_ID
++ ""_ID
++ R"AA(another
+ raw
+ string)AA"_ID;
+}
Index: libcpp/lex.c
===
--- libcpp/lex.c(revision 254048)
+++ libcpp/lex.c(working copy)
@@ -1871,8 +1871,9 @@
/* If a string format macro, say from inttypes.h, is placed touching
a string literal it could be parsed as a C++11 user-defined string
literal thus breaking the program.
-Try to identify macros with is_macro. A warning is issued. */
- if (is_macro (pfile, cur))
+Try to identify macros with is_macro. A warning is issued.
+The macro name should not start with '_' for this warning. */
+ if ((*cur != '_') && is_macro (pfile, cur))
{
/* Raise a warning, but do not consume subsequent tokens. */
if (CPP_OPTION (pfile, warn_literal_suffix) && !pfile->state.skipping)
@@ -2001,8 +2002,9 @@
/* If a string format macro, say from inttypes.h, is placed touching
a string literal it could be parsed as a C++11 user-defined string
literal thus breaking the program.
-Try to identify macros with is_macro. A warning is issued. */
- if (is_macro (pfile, cur))
+Try to identify macros with is_macro. A warning is issued.
+The macro name should not start with '_' for this warning. */
+ if ((*cur != '_') && is_macro (pfile, cur))
{
/* Raise a warning, but do not consume subsequent tokens. */
if (CPP_OPTION (pfile, warn_literal_suffix) && !pfile->state.skipping)
Re: [PATCH] Initialize variable in order to survive PGO bootstrap.
On November 4, 2017 7:53:12 PM GMT+01:00, Richard Sandiford
wrote:
>Richard Biener writes:
>> On Fri, Nov 3, 2017 at 9:52 AM, Martin Liška wrote:
>>> Hi.
>>>
>>> This is oneliner that fixes PGO bootstrap. I've discussed that with
>>> Richi and the core is correct.
>>> However we probably don't have an attribute that will ignore the
>warning?
>>
>> I think
>>
>> wide_int res = res;
>>
>> might do (untested).
>>
>>> Only option is to push/pop Wuninitialized warning.
>>
>> Too ugly...
>>
>>> Ready for trunk?
>>
>> Any better idea? Richard?
>
>It looks like we're deliberately returning an uninitialised value if
>there's an overflow, is that right? If so, how about switching it
>so that we return success directly and the value by pointer?
>Doing it that way should be more efficient, since I don't think
>we'll benefit from NRV optimisation as things stand.
>
>It'd be best to avoid wi::zero (0) since 0-precision values aren't
>valid.
Works for me.
Thanks,
Richard.
>Thanks,
>Richard
>
>
>2017-11-04 Richard Sandiford
>
>gcc/
> * tree-vrp.c (vrp_int_const_binop): Return true on success and
> return the value by pointer.
> (extract_range_from_multiplicative_op_1): Update accordingly.
> Return as soon as an operation fails.
>
>Index: gcc/tree-vrp.c
>===
>*** gcc/tree-vrp.c 2017-11-03 12:15:44.102059976 +
>--- gcc/tree-vrp.c 2017-11-04 18:46:32.512257350 +
>*** extract_range_from_ssa_name (value_range
>*** 1619,1639
> }
>
>
>! /* Wrapper around int_const_binop. If the operation overflows and
>!overflow is undefined, then adjust the result to be
>!-INF or +INF depending on CODE, VAL1 and VAL2. Sets *OVERFLOW_P
>!to whether the operation overflowed. For division by zero
>!the result is indeterminate but *OVERFLOW_P is set. */
>!
>! static wide_int
>! vrp_int_const_binop (enum tree_code code, tree val1, tree val2,
>! bool *overflow_p)
> {
>bool overflow = false;
>signop sign = TYPE_SIGN (TREE_TYPE (val1));
>- wide_int res;
>-
>- *overflow_p = false;
>
>switch (code)
> {
>--- 1619,1638
> }
>
>
>! /* Wrapper around int_const_binop. Return true if we can compute the
>!result; i.e. if the operation doesn't overflow or if the overflow
>is
>!undefined. In the latter case (if the operation overflows and
>!overflow is undefined), then adjust the result to be -INF or +INF
>!depending on CODE, VAL1 and VAL2. Return the value in *RES.
>!
>!Return false for division by zero, for which the result is
>!indeterminate. */
>!
>! static bool
>! vrp_int_const_binop (enum tree_code code, tree val1, tree val2,
>wide_int *res)
> {
>bool overflow = false;
>signop sign = TYPE_SIGN (TREE_TYPE (val1));
>
>switch (code)
> {
>*** vrp_int_const_binop (enum tree_code code
>*** 1654,1710
> /* It's unclear from the C standard whether shifts can overflow.
>The following code ignores overflow; perhaps a C standard
>interpretation ruling is needed. */
>!res = wi::rshift (wi::to_wide (val1), wval2, sign);
> else
>!res = wi::lshift (wi::to_wide (val1), wval2);
> break;
>}
>
> case MULT_EXPR:
>! res = wi::mul (wi::to_wide (val1),
>! wi::to_wide (val2), sign, &overflow);
>break;
>
> case TRUNC_DIV_EXPR:
> case EXACT_DIV_EXPR:
>if (val2 == 0)
>! {
>!*overflow_p = true;
>!return res;
>! }
>else
>! res = wi::div_trunc (wi::to_wide (val1),
>! wi::to_wide (val2), sign, &overflow);
>break;
>
> case FLOOR_DIV_EXPR:
>if (val2 == 0)
>! {
>!*overflow_p = true;
>!return res;
>! }
>! res = wi::div_floor (wi::to_wide (val1),
>! wi::to_wide (val2), sign, &overflow);
>break;
>
> case CEIL_DIV_EXPR:
>if (val2 == 0)
>! {
>!*overflow_p = true;
>!return res;
>! }
>! res = wi::div_ceil (wi::to_wide (val1),
>!wi::to_wide (val2), sign, &overflow);
>break;
>
> case ROUND_DIV_EXPR:
>if (val2 == 0)
>! {
>!*overflow_p = 0;
>!return res;
>! }
>! res = wi::div_round (wi::to_wide (val1),
>! wi::to_wide (val2), sign, &overflow);
>break;
>
> default:
>--- 1653,1697
> /* It's unclear from the C standard whether shifts can overflow.
>The following code ignores overflow; perhaps a C standard
>interpretation ruling is needed. */
>!*res = wi::rshift (wi::to_wide (val1), wval2, sign);
> else
>!*res = wi::lshift (wi::to_wide (val1), wval2);
> break;
>}
>
> case MULT_EXPR:
>! *res = wi::mul (wi::to
Re: [PATCH v3 1/14] D: The front-end (DMD) language implementation and license.
On 10/24/2017 4:58 PM, Jeff Law wrote: On 10/03/2017 03:36 PM, Joseph Myers wrote: On Tue, 3 Oct 2017, Jeff Law wrote: /* Copyright (c) 2010-2014 by Digital Mars * All Rights Reserved, written by Walter Bright * http://www.digitalmars.com * Distributed under the Boost Software License, Version 1.0. * (See accompanying file LICENSE or copy at http://www.boost.org/LICENSE_1_0.txt) If the code was assigned to the FSF in 2011, then the FSF would have ownership of the code. And the FSF would be the only entity that could change the license (which according to your message changed to Boost in 2014). So something seems wrong here. The standard FSF assignment would allow the contributor to distribute their own code under such terms as they see fit. Right. But for the copy distributed in GCC we should have FSF ownership and a standard GCC copyright. Anything else would seem to require FSF approval, particularly for the compiler proper (as opposed to the runtime systems where we have looser requirements). I'm certainly not comfortable going outside the box here without SC and/or FSF approval. Jeff Iain has my approval to change the copyright and licenses as required by the FSF, but as a fork. I.e. the stuff the D Language Foundation and Digital Mars releases, like DMD, will remain as is. -- Walter Bright *Digital Mars* C, C++, D and Javascript compilers
Re: [PATCH] Initialize variable in order to survive PGO bootstrap.
Richard Biener writes:
> On Fri, Nov 3, 2017 at 9:52 AM, Martin Liška wrote:
>> Hi.
>>
>> This is oneliner that fixes PGO bootstrap. I've discussed that with
>> Richi and the core is correct.
>> However we probably don't have an attribute that will ignore the warning?
>
> I think
>
> wide_int res = res;
>
> might do (untested).
>
>> Only option is to push/pop Wuninitialized warning.
>
> Too ugly...
>
>> Ready for trunk?
>
> Any better idea? Richard?
It looks like we're deliberately returning an uninitialised value if
there's an overflow, is that right? If so, how about switching it
so that we return success directly and the value by pointer?
Doing it that way should be more efficient, since I don't think
we'll benefit from NRV optimisation as things stand.
It'd be best to avoid wi::zero (0) since 0-precision values aren't valid.
Thanks,
Richard
2017-11-04 Richard Sandiford
gcc/
* tree-vrp.c (vrp_int_const_binop): Return true on success and
return the value by pointer.
(extract_range_from_multiplicative_op_1): Update accordingly.
Return as soon as an operation fails.
Index: gcc/tree-vrp.c
===
*** gcc/tree-vrp.c 2017-11-03 12:15:44.102059976 +
--- gcc/tree-vrp.c 2017-11-04 18:46:32.512257350 +
*** extract_range_from_ssa_name (value_range
*** 1619,1639
}
! /* Wrapper around int_const_binop. If the operation overflows and
!overflow is undefined, then adjust the result to be
!-INF or +INF depending on CODE, VAL1 and VAL2. Sets *OVERFLOW_P
!to whether the operation overflowed. For division by zero
!the result is indeterminate but *OVERFLOW_P is set. */
!
! static wide_int
! vrp_int_const_binop (enum tree_code code, tree val1, tree val2,
!bool *overflow_p)
{
bool overflow = false;
signop sign = TYPE_SIGN (TREE_TYPE (val1));
- wide_int res;
-
- *overflow_p = false;
switch (code)
{
--- 1619,1638
}
! /* Wrapper around int_const_binop. Return true if we can compute the
!result; i.e. if the operation doesn't overflow or if the overflow is
!undefined. In the latter case (if the operation overflows and
!overflow is undefined), then adjust the result to be -INF or +INF
!depending on CODE, VAL1 and VAL2. Return the value in *RES.
!
!Return false for division by zero, for which the result is
!indeterminate. */
!
! static bool
! vrp_int_const_binop (enum tree_code code, tree val1, tree val2, wide_int *res)
{
bool overflow = false;
signop sign = TYPE_SIGN (TREE_TYPE (val1));
switch (code)
{
*** vrp_int_const_binop (enum tree_code code
*** 1654,1710
/* It's unclear from the C standard whether shifts can overflow.
The following code ignores overflow; perhaps a C standard
interpretation ruling is needed. */
! res = wi::rshift (wi::to_wide (val1), wval2, sign);
else
! res = wi::lshift (wi::to_wide (val1), wval2);
break;
}
case MULT_EXPR:
! res = wi::mul (wi::to_wide (val1),
!wi::to_wide (val2), sign, &overflow);
break;
case TRUNC_DIV_EXPR:
case EXACT_DIV_EXPR:
if (val2 == 0)
! {
! *overflow_p = true;
! return res;
! }
else
! res = wi::div_trunc (wi::to_wide (val1),
!wi::to_wide (val2), sign, &overflow);
break;
case FLOOR_DIV_EXPR:
if (val2 == 0)
! {
! *overflow_p = true;
! return res;
! }
! res = wi::div_floor (wi::to_wide (val1),
! wi::to_wide (val2), sign, &overflow);
break;
case CEIL_DIV_EXPR:
if (val2 == 0)
! {
! *overflow_p = true;
! return res;
! }
! res = wi::div_ceil (wi::to_wide (val1),
! wi::to_wide (val2), sign, &overflow);
break;
case ROUND_DIV_EXPR:
if (val2 == 0)
! {
! *overflow_p = 0;
! return res;
! }
! res = wi::div_round (wi::to_wide (val1),
! wi::to_wide (val2), sign, &overflow);
break;
default:
--- 1653,1697
/* It's unclear from the C standard whether shifts can overflow.
The following code ignores overflow; perhaps a C standard
interpretation ruling is needed. */
! *res = wi::rshift (wi::to_wide (val1), wval2, sign);
else
! *res = wi::lshift (wi::to_wide (val1), wval2);
break;
}
case MULT_EXPR:
! *res = wi::mul (wi::to_wide (val1),
! wi::to_wide (val2), sign, &overflow);
break;
case TRUNC_DIV_EXPR:
case EXACT_DIV_EXPR:
if (val2 == 0)
! return false;
else
! *res = wi::div
Re: [Patch, fortran] PR81447 - [7/8] gfortran fails to recognize the exact dynamic type of a polymorphic entity that was allocated in a external procedure
Hi Andre, Shouldn't that better be if ((gfc_option.allow_std & GFC_STD_F2003) > 0 I think that if ((gfc_option.allow_std & GFC_STD_F2003) would be better - the allow_std field is signed, and things could get hariy if we ever have close to 32 standards we would like to support. Hm, come to think of it, is there a special reason to keep this signed, or could we just change it to unsigned? Regards Thomas
Re: [Patch, fortran] PR81447 - [7/8] gfortran fails to recognize the exact dynamic type of a polymorphic entity that was allocated in a external procedure
Hi Paul,
On Thu, 2 Nov 2017 20:11:29 +
Paul Richard Thomas wrote:
> *** resolve_fl_derived (gfc_symbol *sym)
> *** 14075,14080
> --- 14078,14097
> if (!resolve_typebound_procedures (sym))
> return false;
>
> + /* Generate module vtables subject to their accessibility and their not
> + being vtables or pdt templates. If this is not done class declarations
> + in external procedures wind up with their own version and so SELECT
> TYPE
> + fails because the vptrs do not have the same address. */
> + if (gfc_option.allow_std & GFC_STD_F2003
Shouldn't that better be
if ((gfc_option.allow_std & GFC_STD_F2003) > 0
? I regularly fall on my nose because of the compiler binding this not as
expected.
> + && sym->ns->proc_name
> + && sym->ns->proc_name->attr.flavor == FL_MODULE
> + && sym->attr.access != ACCESS_PRIVATE
> + && !(sym->attr.use_assoc || sym->attr.vtype ||
> sym->attr.pdt_template))
> + {
> + gfc_symbol *vtab = gfc_find_derived_vtab (sym);
> + gfc_set_sym_referenced (vtab);
> + }
> +
> return true;
> }
Besides that I think this is OK.
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
Re: [PATCH][AArch64] Improve aarch64_legitimate_constant_p
FAIL: gfortran.dg/class_array_1.f03 -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (test for excess errors)
Excess errors:
/opt/gcc/gcc-20171104/gcc/testsuite/gfortran.dg/class_array_1.f03:31:0: Error:
could not split insn
(insn 527 1444 562 (set (reg:TI 0 x0 [847])
(const_wide_int 0x10001))
"/opt/gcc/gcc-20171104/gcc/testsuite/gfortran.dg/class_array_1.f03":21 50
{*movti_aarch64}
(nil))
during RTL pass: final
/opt/gcc/gcc-20171104/gcc/testsuite/gfortran.dg/class_array_1.f03:31:0:
internal compiler error: in final_scan_insn, at final.c:3018
0x58be1b _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
../../gcc/rtl-error.c:108
0x8aeed7 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
../../gcc/final.c:3018
0x8af12f final(rtx_insn*, _IO_FILE*, int)
../../gcc/final.c:2046
0x8af483 rest_of_handle_final
../../gcc/final.c:4477
0x8af483 execute
../../gcc/final.c:4551
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: [Patch, fortran] PR78641 - [6/7/8 Regression] [OOP] ICE on polymorphic allocatable function in array constructor
On Sat, Nov 04, 2017 at 12:02:58PM +, Paul Richard Thomas wrote: > Dear All, > > This patch allows the assignment of class array constructors to > derived type arrays. It is straightforward enough that the ChangeLogs > and the comment are sufficient explanation. > > Bootstraps and regtests on FC23/x86_64 - OK for all three branches? > OK. -- Steve
Re: [Committed, Fortran, Patch, v1] Three small patches for character arrays
Hi Paul,
thanks for the review. Committed as r254407 to trunk and r254408 to gcc-7.
Regards,
Andre
On Sat, 4 Nov 2017 12:06:15 +
Paul Richard Thomas wrote:
> Hi Andre,
>
> These patches look fine to me. Thanks for taking account of the array
> descriptor change between trunk and 7-branch :-)
>
> OK for trunk and 7-branch.
>
> Thanks
>
> Paul
>
>
> On 4 November 2017 at 10:27, Andre Vehreschild wrote:
> > Ping!
> >
> > On Wed, 1 Nov 2017 12:27:11 +0100
> > Andre Vehreschild wrote:
> >
> >> Hi all,
> >>
> >> during development on OpenCoarrays I found these three issues in gfortran:
> >>
> >> - caf_send: Did not treat char arrays as arrays when calling caf_send.
> >> - gfc_trans_assignment_1: Conversion of character kind creates a loop
> >> variant temporary which must not be put before the loop body, but within.
> >> - get_array_span: When doing pointer arithmetic on char arrays the
> >> character kind was not applied.
> >>
> >> The attached patch fixes all of these issues for trunk and gcc-7.
> >> Bootstrapped and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7?
> >>
> >> Regards,
> >> Andre
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (Revision 254406)
+++ gcc/fortran/ChangeLog (Arbeitskopie)
@@ -1,3 +1,12 @@
+2017-11-04 Andre Vehreschild
+
+ * trans-expr.c (gfc_trans_assignment_1): Character kind conversion may
+ create a loop variant temporary, too.
+ * trans-intrinsic.c (conv_caf_send): Treat char arrays as arrays and
+ not as scalars.
+ * trans.c (get_array_span): Take the character kind into account when
+ doing pointer arithmetic.
+
2017-11-04 Thomas Koenig
PR fortran/29600
Index: gcc/fortran/trans-expr.c
===
--- gcc/fortran/trans-expr.c (Revision 254406)
+++ gcc/fortran/trans-expr.c (Arbeitskopie)
@@ -10084,12 +10084,16 @@
NOTE: This relies on having the exact dependence of the length type
parameter available to the caller; gfortran saves it in the .mod files.
NOTE ALSO: The concatenation operation generates a temporary pointer,
- whose allocation must go to the innermost loop. */
+ whose allocation must go to the innermost loop.
+ NOTE ALSO (2): A character conversion may generate a temporary, too. */
if (flag_realloc_lhs
&& expr2->ts.type == BT_CHARACTER && expr1->ts.deferred
&& !(lss != gfc_ss_terminator
- && expr2->expr_type == EXPR_OP
- && expr2->value.op.op == INTRINSIC_CONCAT))
+ && ((expr2->expr_type == EXPR_OP
+ && expr2->value.op.op == INTRINSIC_CONCAT)
+ || (expr2->expr_type == EXPR_FUNCTION
+ && expr2->value.function.isym != NULL
+ && expr2->value.function.isym->id == GFC_ISYM_CONVERSION
gfc_add_block_to_block (&block, &rse.pre);
/* Nullify the allocatable components corresponding to those of the lhs
Index: gcc/fortran/trans-intrinsic.c
===
--- gcc/fortran/trans-intrinsic.c (Revision 254406)
+++ gcc/fortran/trans-intrinsic.c (Arbeitskopie)
@@ -1871,12 +1871,21 @@
gfc_init_se (&lhs_se, NULL);
if (lhs_expr->rank == 0)
{
- symbol_attribute attr;
- gfc_clear_attr (&attr);
- gfc_conv_expr (&lhs_se, lhs_expr);
- lhs_type = TREE_TYPE (lhs_se.expr);
- lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, attr);
- lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ if (lhs_expr->ts.type == BT_CHARACTER && lhs_expr->ts.deferred)
+ {
+ lhs_se.expr = gfc_get_tree_for_caf_expr (lhs_expr);
+ lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ }
+ else
+ {
+ symbol_attribute attr;
+ gfc_clear_attr (&attr);
+ gfc_conv_expr (&lhs_se, lhs_expr);
+ lhs_type = TREE_TYPE (lhs_se.expr);
+ lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr,
+ attr);
+ lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ }
}
else if ((lhs_caf_attr.alloc_comp || lhs_caf_attr.pointer_comp)
&& lhs_caf_attr.codimension)
Index: gcc/fortran/trans.c
===
--- gcc/fortran/trans.c (Revision 254406)
+++ gcc/fortran/trans.c (Arbeitskopie)
@@ -320,8 +320,12 @@
|| DECL_CONTEXT (TYPE_MAX_VALUE (TYPE_DOMAIN (type)))
== DECL_CONTEXT (decl)))
{
- span = TYPE_MAX_VALUE (TYPE_DOMAIN (type));
- span = fold_convert (gfc_array_index_type, span);
+ span = fold_convert (gfc_array_index_type,
+ TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
+ span = fold_build2 (MULT_EXPR, gfc_array_index_type,
+ fold_convert (gfc_array_index_type,
+ TYPE_SIZE_UNIT (TREE_TYPE (type))),
+ span);
}
/* Likewise for class array or pointer array
[test case, fortran, committed] Test case for PR 70330
Hi, I have just committed the test case from PR 70330 to trunk and closed the (error-recovery) PR. Regards Thomas https://gcc.gnu.org/viewcvs?rev=254406&root=gcc&view=rev
RE: [PATCH 08/22] Add Intel CET support for EH in libgcc.
> -Original Message- > From: Jeff Law [mailto:l...@redhat.com] > Sent: Tuesday, October 31, 2017 5:49 AM > To: Tsimbalist, Igor V ; gcc- > patc...@gcc.gnu.org > Cc: i...@airs.com > Subject: Re: [PATCH 08/22] Add Intel CET support for EH in libgcc. > > On 10/12/2017 01:56 PM, Tsimbalist, Igor V wrote: > > Control-flow Enforcement Technology (CET), published by Intel, Introduces > > the Shadow Stack feature, which ensures a return from a function is done > > to exactly the same location from where the function was called. When EH > > is present the control-flow transfer may skip some stack frames and the > > shadow stack has to be adjusted not to signal a violation of a > > control-flow transfer. It's done by counting a number of skipping frames > > and adjusting shadow stack pointer by this number. > > > > gcc/ > > * config/i386/i386.c (ix86_expand_epilogue): Change simple > > return to indirect jump for EH return. Change explicit 'false' > > argument in pro_epilogue_adjust_stack with a value of > > flag_cf_protection. > > * config/i386/i386.md (simple_return_indirect_internal): Remove > > SImode restriction to support 64-bit. > > > > libgcc/ > > * config/i386/linux-unwind.h: Include > > config/i386/shadow-stack-unwind.h. > > * config/i386/shadow-stack-unwind.h: New file. > > * unwind-dw2.c: (uw_install_context): Add a FRAMES argument and > > pass it to _Unwind_Frames_Extra. > > * unwind-generic.h (FRAMES_P_DECL): New. > > (FRAMES_VAR): Likewise. > > (FRAMES_VAR_P): Likewise. > > (FRAMES_VAR_DECL): Likewise. > > (FRAMES_VAR_DECL_1): Likewise. > > (FRAMES_VAR_INC): Likewise. > > (FRAMES_P_UPDATE): Likewise. > > (_Unwind_Frames_Extra): Likewise. > > * unwind.inc (_Unwind_RaiseException_Phase2): Use > FRAMES_P_DECL, > > FRAMES_VAR_DECL_1, FRAMES_VAR_INC and FRAMES_P_UPDATE. > > (_Unwind_RaiseException): Use FRAMES_VAR_DECL, > FRAMES_VAR_P and > > FRAMES_VAR. > > (_Unwind_ForcedUnwind_Phase2): Use FRAMES_P_DECL, > > FRAMES_VAR_DECL_1, FRAMES_VAR_INC, FRAMES_P_UPDATE. > > (_Unwind_ForcedUnwind): Use FRAMES_VAR_DECL, > FRAMES_VAR_P and > > FRAMES_VAR. > > (_Unwind_Resume): Use FRAMES_VAR_DECL, FRAMES_VAR_P and > > FRAMES_VAR. > > (_Unwind_Resume_or_Rethrow): Use FRAMES_VAR_DECL, > FRAMES_VAR_P > > and FRAMES_VAR. > > > > Igor > > > > > > > > 0008-Add-Intel-CET-support-for-EH-in-libgcc.patch > > > > > > From 16eb1d0d9239e039fba28f1ae71762f19061b157 Mon Sep 17 00:00:00 > 2001 > > From: Igor Tsimbalist > > Date: Wed, 19 Jul 2017 03:04:46 +0300 > > Subject: [PATCH 08/22] Add Intel CET support for EH in libgcc. > > > > Control-flow Enforcement Technology (CET), published by Intel, > > introduces > > the Shadow Stack feature, which ensures a return from a function is done > > to exactly the same location from where the function was called. When EH > > is present the control-flow transfer may skip some stack frames and the > > shadow stack has to be adjusted not to signal a violation of a > > control-flow transfer. It's done by counting a number of skiping frames > > and adjasting shadow stack pointer by this number. > > > > Having new semantic of the 'ret' instruction if CET is supported in HW > > the 'ret' instruction cannot be generated in ix86_expand_epilogue when > > we are returning after EH is processed. Added a code in > > ix86_expand_epilogue to adjust Shadow Stack pointer and to generate an > > indirect jump instead of 'ret'. As sp register is used during this > > adjustment thus the argument in pro_epilogue_adjust_stack is changed > > to update cfa_reg based on whether control-flow instrumentation is set. > > Without updating the cfa_reg field there is an assert later in dwarf2 > > pass related to mismatch the stack register and cfa_reg value. > > > > gcc/ > > * config/i386/i386.c (ix86_expand_epilogue): Change simple > > return to indirect jump for EH return. Change explicit 'false' > > argument in pro_epilogue_adjust_stack with a value of > > flag_cf_protection. > > * config/i386/i386.md (simple_return_indirect_internal): Remove > > SImode restriction to support 64-bit. > > > > libgcc/ > > * config/i386/linux-unwind.h: Include > > config/i386/shadow-stack-unwind.h. > > * config/i386/shadow-stack-unwind.h: New file. > > * unwind-dw2.c: (uw_install_context): Add a FRAMES argument and > > pass it to _Unwind_Frames_Extra. > > * unwind-generic.h (FRAMES_P_DECL): New. > > (FRAMES_VAR): Likewise. > > (FRAMES_VAR_P): Likewise. > > (FRAMES_VAR_DECL): Likewise. > > (FRAMES_VAR_DECL_1): Likewise. > > (FRAMES_VAR_INC): Likewise. > > (FRAMES_P_UPDATE): Likewise. > > (_Unwind_Frames_Extra): Likewise. > > * unwind.inc (_Unwind_RaiseException_Phase2): Use > FRAMES_P_DECL, > > FRAMES_VAR_DECL_1, FRAMES_VAR_INC and FRAMES_P_UPDATE. > > (_Unwind_RaiseException): Use FRAMES_VAR_DECL, > FRAMES_VAR_P and > >
Re: [Ping, Fortran, Patch, v1] Three small patches for character arrays
Hi Andre, These patches look fine to me. Thanks for taking account of the array descriptor change between trunk and 7-branch :-) OK for trunk and 7-branch. Thanks Paul On 4 November 2017 at 10:27, Andre Vehreschild wrote: > Ping! > > On Wed, 1 Nov 2017 12:27:11 +0100 > Andre Vehreschild wrote: > >> Hi all, >> >> during development on OpenCoarrays I found these three issues in gfortran: >> >> - caf_send: Did not treat char arrays as arrays when calling caf_send. >> - gfc_trans_assignment_1: Conversion of character kind creates a loop variant >> temporary which must not be put before the loop body, but within. >> - get_array_span: When doing pointer arithmetic on char arrays the character >> kind was not applied. >> >> The attached patch fixes all of these issues for trunk and gcc-7. >> Bootstrapped >> and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7? >> >> Regards, >> Andre > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR78641 - [6/7/8 Regression] [OOP] ICE on polymorphic allocatable function in array constructor
Dear All,
This patch allows the assignment of class array constructors to
derived type arrays. It is straightforward enough that the ChangeLogs
and the comment are sufficient explanation.
Bootstraps and regtests on FC23/x86_64 - OK for all three branches?
Paul
2017-11-04 Paul Thomas
PR fortran/78641
* resolve.c (resolve_ordinary_assign): Do not add the _data
component for class valued array constructors being assigned
to derived type arrays.
* trans-array.c (gfc_trans_array_ctor_element): Take the _data
of class valued elements for assignment to derived type arrays.
2017-11-04 Paul Thomas
PR fortran/78641
* gfortran.dg/class_66.f90: New test.
Index: gcc/fortran/resolve.c
===
*** gcc/fortran/resolve.c (revision 254403)
--- gcc/fortran/resolve.c (working copy)
*** resolve_ordinary_assign (gfc_code *code,
*** 10324,10330
/* Assign the 'data' of a class object to a derived type. */
if (lhs->ts.type == BT_DERIVED
! && rhs->ts.type == BT_CLASS)
gfc_add_data_component (rhs);
bool caf_convert_to_send = flag_coarray == GFC_FCOARRAY_LIB
--- 10324,10331
/* Assign the 'data' of a class object to a derived type. */
if (lhs->ts.type == BT_DERIVED
! && rhs->ts.type == BT_CLASS
! && rhs->expr_type != EXPR_ARRAY)
gfc_add_data_component (rhs);
bool caf_convert_to_send = flag_coarray == GFC_FCOARRAY_LIB
Index: gcc/fortran/trans-array.c
===
*** gcc/fortran/trans-array.c (revision 254403)
--- gcc/fortran/trans-array.c (working copy)
*** gfc_trans_array_ctor_element (stmtblock_
*** 1580,1585
--- 1580,1596
}
}
}
+ else if (GFC_CLASS_TYPE_P (TREE_TYPE (se->expr))
+ && !GFC_CLASS_TYPE_P (gfc_get_element_type (TREE_TYPE (desc
+ {
+ /* Assignment of a CLASS array constructor to a derived type array. */
+ if (expr->expr_type == EXPR_FUNCTION)
+ se->expr = gfc_evaluate_now (se->expr, pblock);
+ se->expr = gfc_class_data_get (se->expr);
+ se->expr = build_fold_indirect_ref_loc (input_location, se->expr);
+ se->expr = fold_convert (TREE_TYPE (tmp), se->expr);
+ gfc_add_modify (&se->pre, tmp, se->expr);
+ }
else
{
/* TODO: Should the frontend already have done this conversion? */
Index: gcc/testsuite/gfortran.dg/class_66.f90
===
*** gcc/testsuite/gfortran.dg/class_66.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/class_66.f90 (working copy)
***
*** 0
--- 1,28
+ ! { dg- do run }
+ !
+ ! Test the fix for PR78641 in which an ICE occured on assignment
+ ! of a class array constructor to a derived type array.
+ !
+ ! Contributed by Damian Rouson
+ !
+ implicit none
+ type foo
+ integer :: i = 99
+ end type
+ type(foo) :: bar(4)
+ class(foo), allocatable :: barfoo
+
+ allocate(barfoo,source = f(11))
+ bar = [f(33), [f(22), barfoo], f(1)]
+ if (any (bar%i .ne. [33, 22, 11, 1])) call abort
+ deallocate (barfoo)
+
+ contains
+
+ function f(arg) result(foobar)
+ class(foo), allocatable :: foobar
+ integer :: arg
+ allocate(foobar,source = foo(arg))
+ end function
+
+ end program
Re: Drop frequencies from basic blocks
This breaks gfortran.dg/bounds_check_15.f90 with -O3 -funroll-loops -fbounds-check on ia64: Error: qsort comparator non-negative on sorted output: 1 during RTL pass: mach ../gcc/testsuite/gfortran.dg/bounds_check_15.f90:32:0: internal compiler error: qsort checking failed 0x401cc1ff qsort_chk_error ../../gcc/vec.c:222 0x4223130f qsort_chk(void*, unsigned long, unsigned long, int (*)(void const*, void const*)) ../../gcc/vec.c:274 0x4114ba4f vec<_expr*, va_heap, vl_embed>::qsort(int (*)(void const*, void const*)) ../../gcc/vec.h:973 0x4114ba4f vec<_expr*, va_heap, vl_ptr>::qsort(int (*)(void const*, void const*)) ../../gcc/vec.h:1735 0x4114ba4f fill_vec_av_set ../../gcc/sel-sched.c:3725 0x411517df fill_ready_list ../../gcc/sel-sched.c:4022 0x411517df find_best_expr ../../gcc/sel-sched.c:4382 0x411517df fill_insns ../../gcc/sel-sched.c:5539 0x411517df schedule_on_fences ../../gcc/sel-sched.c:7356 0x411517df sel_sched_region_2 ../../gcc/sel-sched.c:7494 0x41158d0f sel_sched_region_1 ../../gcc/sel-sched.c:7536 0x41158d0f sel_sched_region(int) ../../gcc/sel-sched.c:7637 0x4115a0af run_selective_scheduling() ../../gcc/sel-sched.c:7713 0x41a50a5f ia64_reorg ../../gcc/config/ia64/ia64.c:9854 0x410c2a8f execute ../../gcc/reorg.c:3947 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: [Ping, Fortran, Patch, v1] Three small patches for character arrays
Ping!
On Wed, 1 Nov 2017 12:27:11 +0100
Andre Vehreschild wrote:
> Hi all,
>
> during development on OpenCoarrays I found these three issues in gfortran:
>
> - caf_send: Did not treat char arrays as arrays when calling caf_send.
> - gfc_trans_assignment_1: Conversion of character kind creates a loop variant
> temporary which must not be put before the loop body, but within.
> - get_array_span: When doing pointer arithmetic on char arrays the character
> kind was not applied.
>
> The attached patch fixes all of these issues for trunk and gcc-7. Bootstrapped
> and regtested on x86_64-linux-gnu/f25. Ok for trunk and gcc-7?
>
> Regards,
> Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
2017-11-01 Andre Vehreschild
* trans-expr.c (gfc_trans_assignment_1): Character kind conversion may
create a loop variant temporary, too.
* trans-intrinsic.c (conv_caf_send): Treat char arrays as arrays and
not as scalars.
* trans.c (get_array_span): Take the character kind into account when
doing pointer arithmetic.
gcc/testsuite/ChangeLog:
2017-11-01 Andre Vehreschild
* gfortran.dg/coarray/send_char_array_1.f90: New test.
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 1a3e3d4..57b62a6 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -10084,12 +10084,16 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag,
NOTE: This relies on having the exact dependence of the length type
parameter available to the caller; gfortran saves it in the .mod files.
NOTE ALSO: The concatenation operation generates a temporary pointer,
- whose allocation must go to the innermost loop. */
+ whose allocation must go to the innermost loop.
+ NOTE ALSO (2): A character conversion may generate a temporary, too. */
if (flag_realloc_lhs
&& expr2->ts.type == BT_CHARACTER && expr1->ts.deferred
&& !(lss != gfc_ss_terminator
- && expr2->expr_type == EXPR_OP
- && expr2->value.op.op == INTRINSIC_CONCAT))
+ && ((expr2->expr_type == EXPR_OP
+ && expr2->value.op.op == INTRINSIC_CONCAT)
+ || (expr2->expr_type == EXPR_FUNCTION
+ && expr2->value.function.isym != NULL
+ && expr2->value.function.isym->id == GFC_ISYM_CONVERSION
gfc_add_block_to_block (&block, &rse.pre);
/* Nullify the allocatable components corresponding to those of the lhs
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 532d3ab..b0f0ab2 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -1871,12 +1871,21 @@ conv_caf_send (gfc_code *code) {
gfc_init_se (&lhs_se, NULL);
if (lhs_expr->rank == 0)
{
- symbol_attribute attr;
- gfc_clear_attr (&attr);
- gfc_conv_expr (&lhs_se, lhs_expr);
- lhs_type = TREE_TYPE (lhs_se.expr);
- lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr, attr);
- lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ if (lhs_expr->ts.type == BT_CHARACTER && lhs_expr->ts.deferred)
+ {
+ lhs_se.expr = gfc_get_tree_for_caf_expr (lhs_expr);
+ lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ }
+ else
+ {
+ symbol_attribute attr;
+ gfc_clear_attr (&attr);
+ gfc_conv_expr (&lhs_se, lhs_expr);
+ lhs_type = TREE_TYPE (lhs_se.expr);
+ lhs_se.expr = gfc_conv_scalar_to_descriptor (&lhs_se, lhs_se.expr,
+ attr);
+ lhs_se.expr = gfc_build_addr_expr (NULL_TREE, lhs_se.expr);
+ }
}
else if ((lhs_caf_attr.alloc_comp || lhs_caf_attr.pointer_comp)
&& lhs_caf_attr.codimension)
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 53bc428..4115308 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -320,8 +320,12 @@ get_array_span (tree type, tree decl)
|| DECL_CONTEXT (TYPE_MAX_VALUE (TYPE_DOMAIN (type)))
== DECL_CONTEXT (decl)))
{
- span = TYPE_MAX_VALUE (TYPE_DOMAIN (type));
- span = fold_convert (gfc_array_index_type, span);
+ span = fold_convert (gfc_array_index_type,
+ TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
+ span = fold_build2 (MULT_EXPR, gfc_array_index_type,
+ fold_convert (gfc_array_index_type,
+ TYPE_SIZE_UNIT (TREE_TYPE (type))),
+ span);
}
/* Likewise for class array or pointer array references. */
else if (TREE_CODE (decl) == FIELD_DECL
diff --git a/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90
new file mode 100644
index 000..800a8ac
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90
@@ -0,0 +1,54 @@
+!{ dg-do run }
+
+program send_convert_char_array
+
+ implicit none
+
+ character(kind=1, len=:), allocatable, codimension[:] :: co_str_k1_scal
+ character(kind=1, len=:), allocatable :: str_k1_scal
+ character(kind=4, len=:), allocatable, codimension[:] ::
Re: libbacktrace patch committed: Support compressed debug sections
On Fri, 6 Oct 2017, Ian Lance Taylor wrote: > Thanks for the report. I committed this patch, which I hope will fix > the problem. > * ztest.c (test_large): Pass unsigned long *, not size_t *, to > zlib uncompress function. Thank you, yes it did. (Sorry, I thought I had responded back then, but apparently only in my mind, not with my keyboard.) I'm seeing the following as part of my nightly builds, though: test hello: got uncompressed length 0, want 13 test goodbye: got uncompressed length 0, want 14 inflate large: got uncompressed length 0, want 387851 Any idea what this might be? Gerald
More fold_negate in match.pd
Hello,
this copies some more transformations from fold_negate_expr to match.pd.
Mostly, I wanted to add (negate (pointer_diff @0 @1)), and couldn't find
the corresponding transformation with minus_expr... We can see about
generalizing a bit with conversions later, I wanted to have something at
least.
Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
gcc/ChangeLog:
2017-11-04 Marc Glisse
* fold-const.c (negate_expr_p) [PLUS_EXPR, MINUS_EXPR]: Handle
non-scalar integral types.
* match.pd (negate_expr_p): Handle MINUS_EXPR.
(-(A-B), -(~A)): New transformations.
gcc/testsuite/ChangeLog:
2017-11-04 Marc Glisse
* gcc.dg/tree-ssa/negminus.c: New test.
--
Marc GlisseIndex: gcc/fold-const.c
===
--- gcc/fold-const.c (revision 254399)
+++ gcc/fold-const.c (working copy)
@@ -421,34 +421,34 @@ negate_expr_p (tree t)
case COMPLEX_EXPR:
return negate_expr_p (TREE_OPERAND (t, 0))
&& negate_expr_p (TREE_OPERAND (t, 1));
case CONJ_EXPR:
return negate_expr_p (TREE_OPERAND (t, 0));
case PLUS_EXPR:
if (HONOR_SIGN_DEPENDENT_ROUNDING (element_mode (type))
|| HONOR_SIGNED_ZEROS (element_mode (type))
- || (INTEGRAL_TYPE_P (type)
+ || (ANY_INTEGRAL_TYPE_P (type)
&& ! TYPE_OVERFLOW_WRAPS (type)))
return false;
/* -(A + B) -> (-B) - A. */
if (negate_expr_p (TREE_OPERAND (t, 1)))
return true;
/* -(A + B) -> (-A) - B. */
return negate_expr_p (TREE_OPERAND (t, 0));
case MINUS_EXPR:
/* We can't turn -(A-B) into B-A when we honor signed zeros. */
return !HONOR_SIGN_DEPENDENT_ROUNDING (element_mode (type))
&& !HONOR_SIGNED_ZEROS (element_mode (type))
- && (! INTEGRAL_TYPE_P (type)
+ && (! ANY_INTEGRAL_TYPE_P (type)
|| TYPE_OVERFLOW_WRAPS (type));
case MULT_EXPR:
if (TYPE_UNSIGNED (type))
break;
/* INT_MIN/n * n doesn't overflow while negating one operand it does
if n is a (negative) power of two. */
if (INTEGRAL_TYPE_P (TREE_TYPE (t))
&& ! TYPE_OVERFLOW_WRAPS (TREE_TYPE (t))
&& ! ((TREE_CODE (TREE_OPERAND (t, 0)) == INTEGER_CST
Index: gcc/match.pd
===
--- gcc/match.pd (revision 254399)
+++ gcc/match.pd (working copy)
@@ -951,35 +951,50 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(negate @0)
(if (!TYPE_OVERFLOW_SANITIZED (type
(match negate_expr_p
REAL_CST
(if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (t)
/* VECTOR_CST handling of non-wrapping types would recurse in unsupported
ways. */
(match negate_expr_p
VECTOR_CST
(if (FLOAT_TYPE_P (TREE_TYPE (type)) || TYPE_OVERFLOW_WRAPS (type
+(match negate_expr_p
+ (minus @0 @1)
+ (if ((ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_WRAPS (type))
+ || (FLOAT_TYPE_P (type)
+ && !HONOR_SIGN_DEPENDENT_ROUNDING (type)
+ && !HONOR_SIGNED_ZEROS (type)
/* (-A) * (-B) -> A * B */
(simplify
(mult:c (convert1? (negate @0)) (convert2? negate_expr_p@1))
(if (tree_nop_conversion_p (type, TREE_TYPE (@0))
&& tree_nop_conversion_p (type, TREE_TYPE (@1)))
(mult (convert @0) (convert (negate @1)
/* -(A + B) -> (-B) - A. */
(simplify
(negate (plus:c @0 negate_expr_p@1))
(if (!HONOR_SIGN_DEPENDENT_ROUNDING (element_mode (type))
&& !HONOR_SIGNED_ZEROS (element_mode (type)))
(minus (negate @1) @0)))
+/* -(A - B) -> B - A. */
+(simplify
+ (negate (minus @0 @1))
+ (if ((ANY_INTEGRAL_TYPE_P (type) && !TYPE_OVERFLOW_SANITIZED (type))
+ || (FLOAT_TYPE_P (type)
+ && !HONOR_SIGN_DEPENDENT_ROUNDING (type)
+ && !HONOR_SIGNED_ZEROS (type)))
+ (minus @1 @0)))
+
/* A - B -> A + (-B) if B is easily negatable. */
(simplify
(minus @0 negate_expr_p@1)
(if (!FIXED_POINT_TYPE_P (type))
(plus @0 (negate @1
/* Try to fold (type) X op CST -> (type) (X op ((type-x) CST))
when profitable.
For bitwise binary operations apply operand conversions to the
binary operation result instead of to the operands. This allows
@@ -1075,20 +1090,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(bit_not (bit_not @0))
@0)
/* Convert ~ (-A) to A - 1. */
(simplify
(bit_not (convert? (negate @0)))
(if (element_precision (type) <= element_precision (TREE_TYPE (@0))
|| !TYPE_UNSIGNED (TREE_TYPE (@0)))
(convert (minus @0 { build_each_one_cst (TREE_TYPE (@0)); }
+/* Convert - (~A) to A + 1. */
+(simplify
+ (negate (nop_convert (bit_not @0)))
+ (plus (view_convert @0) { build_each_one_cst (type); }))
+
/* Convert ~ (A - 1) or ~ (A + -1) to -A. */
(simplify
(bit_not (convert? (minus @0 integer_each_onep)))
(if (element_precision (type) <= element_precision (TREE_TYPE (@0))
|| !TYPE_UNSIGNED (TREE_TYPE (@0)))
(convert (negate @0
(simplify
(bit_not (convert? (plus @0 integer_all_onesp)))
(if (element_precision (type) <= element_p
Re: PR82816: Widening multiplies of bitfields
Richard Sandiford writes:
> In this PR we tried to create a widening multiply of two 3-bit numbers,
> but that isn't a widening multiply at the optab/rtl level, since both
> the input and output still have the same mode.
>
> We could trap this either in is_widening_mult_p or (as the patch does)
> in the routines that actually ask for an optab. The latter seemed
> more natural since is_widening_mult_p doesn't otherwise care about modes.
>
> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.
> OK to install?
>
> Richard
>
>
> 2017-11-03 Richard Sandiford
> Alan Hayward
> David Sherwood
Oops, I instinctively used the SVE ChangeLog Header of the Day
for this patch too, but it was just me for this one. (In case it
seemed strange that it took three people to write four lines...)
> gcc/
> PR tree-optimization/82816
> * tree-ssa-math-opts.c (convert_mult_to_widen): Return false
> if the modes of the two types are the same.
> (convert_plusminus_to_widen): Likewise.
>
> gcc/testsuite/
> * gcc.c-torture/compile/pr82816.c: New test.
>
> Index: gcc/tree-ssa-math-opts.c
> ===
> --- gcc/tree-ssa-math-opts.c 2017-11-01 12:29:40.203534002 +
> +++ gcc/tree-ssa-math-opts.c 2017-11-03 11:18:03.046411241 +
> @@ -3259,6 +3259,9 @@ convert_mult_to_widen (gimple *stmt, gim
>
>to_mode = SCALAR_INT_TYPE_MODE (type);
>from_mode = SCALAR_INT_TYPE_MODE (type1);
> + if (to_mode == from_mode)
> +return false;
> +
>from_unsigned1 = TYPE_UNSIGNED (type1);
>from_unsigned2 = TYPE_UNSIGNED (type2);
>
> @@ -3449,6 +3452,9 @@ convert_plusminus_to_widen (gimple_stmt_
>
>to_mode = SCALAR_TYPE_MODE (type);
>from_mode = SCALAR_TYPE_MODE (type1);
> + if (to_mode == from_mode)
> +return false;
> +
>from_unsigned1 = TYPE_UNSIGNED (type1);
>from_unsigned2 = TYPE_UNSIGNED (type2);
>optype = type1;
> Index: gcc/testsuite/gcc.c-torture/compile/pr82816.c
> ===
> --- /dev/null 2017-11-03 10:40:07.002381728 +
> +++ gcc/testsuite/gcc.c-torture/compile/pr82816.c 2017-11-03
> 11:18:03.045411265 +
> @@ -0,0 +1,12 @@
> +struct A
> +{
> + int b:3;
> +} d, e;
> +
> +int c;
> +
> +void f ()
> +{
> + char g = d.b * e.b;
> + c = g;
> +}
Re: [Patch, fortran] PR81735 - [6/7/8 Regression] double free or corruption (fasttop) error (SIGABRT) with character(:) and custom return type with allocatable
Corrected in trunk in revision 254404. Cheers Paul On 3 November 2017 at 19:22, Paul Richard Thomas wrote: > This was already fixed by the patch for PR82375 on trunk, albeit less > well than the patch applied here. I will correct trunk tomorrow. > > Committed as 'obvious' on 6-branch(r254393) and 7-branch(r254389) > after bootstrapping and regtesting. > > Cheers > > Paul > 2017-11-03 Paul Thomas > > PR fortran/81735 > * trans-decl.c (gfc_trans_deferred_vars): Correct case where > 'tmp' can be used unititialized. > > 2017-11-03 Paul Thomas > > PR fortran/81735 > * gfortran.dg/pr81735.f90: New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH 2/2] [i386] PR82002 Part 2: Correct non-immediate offset/invalid INSN
On Fri, Nov 3, 2017 at 10:22 PM, Daniel Santos wrote: > On 11/03/2017 02:09 AM, Uros Bizjak wrote: >> On Thu, Nov 2, 2017 at 11:43 PM, Daniel Santos >> wrote: >> >int_registers_saved = (frame.nregs == 0); >sse_registers_saved = (frame.nsseregs == 0); > + save_stub_call_needed = (m->call_ms2sysv); > + gcc_assert (!(!sse_registers_saved && save_stub_call_needed)); Oooh, double negation :( >>> I'm just saying that we shouldn't be saving SSE registers inline and via >>> the stub. If I followed the naming convention of e.g., >>> "see_registers_saved" then my variable would end up being called >>> "save_stub_called" which would be incorrect and misleading, similar to >>> how "see_registers_saved" is misleading when there are in fact no SSE >>> register that need to be saved. Maybe I should rename >>> (int|sse)_registers_saved to (int|sse)_register_saves_needed with >>> inverted logic instead. >> But, we can just say >> >> gcc_assert (sse_registers_saved || !save_stub_call_needed); >> >> No? >> >> Uros. >> > > Oh yes, I see. Because "sse_registers_saved" really means that we've > either already saved them or don't have to, and not literally that they > have been saved. I ranted about it's name but didn't think it all the > way through. :) > > How does this patch look? (Also, I've updated comments for > choose_baseaddr.) Currently re-running tests. - tmp = choose_baseaddr (rsi_offset, &align); + addr = choose_baseaddr (rsi_offset, &align, SI_REG); gcc_assert (align >= GET_MODE_ALIGNMENT (V4SFmode)); - emit_insn (gen_rtx_SET (rsi, tmp)); + + /* If choose_baseaddr returned our scratch register, then we don't need to + do another SET. */ + if (!REG_P (addr) || REGNO (addr) != SI_REG) +emit_insn (gen_rtx_SET (rsi, addr)); The above condition will always be true, so this change is not needed. I guess that you ony need to add SI_REG to choose_baseaddr. Otherwise, modulo formatting of the long line, LGTM. Uros.
