Re: *ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
Dear Mikael, With your tweak it bootstrapped and regtested OK. Also changed the comment after Steve's pointing out that the last sentence was incomprehensible:-) Committed to trunk as revision 223234. Cheers Paul 2015-05-16 Mikael Morin PR fortran/65792 * trans-expr.c (gfc_trans_subcomponent_assign): Always assign the expression component to the destination. In addition, if the component has allocatable components, copy them and deallocate those of the expression, if it is not a variable. The expression is fixed if not a variable to prevent multiple evaluations. 2015-05-16 Mikael Morin PR fortran/65792 * gfortran.dg/derived_constructor_components_5: New test On 15 May 2015 at 19:01, Mikael Morin wrote: > Le 09/05/2015 15:12, Mikael Morin a écrit : >> Le 01/05/2015 20:25, Paul Richard Thomas a écrit : >>> Dear All, >>> >>> By the time I went to commit, something had changed and the patch >>> caused a regression. I presume that the version that I had of Andre's >>> patch was not the same as the one committed. I'll cast an eye over it >>> this weekend and see if I can understand what gives. >>> >> Hello Paul, >> >> to get things moving again, I propose the attached fix to your patch. >> Tested on alloc_comp_deep_copy_1 only for now. > > Hello Paul, > > any news? > > Mikael -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [patch] libstdc++/66055 add missing constructors to unordered containers
On 14/05/2015 15:47, Jonathan Wakely wrote:
Reported by Nathan and fixed by his patch. I added the tests.
Tested powerpc64le-linux, committed to trunk. This should be
backported too.
While backporting to debug and profile mode I noticed that those
constructors were not the only missing ones. So here is a patch to
complete them with debug and profile modes.
Moreover this patch:
- Remove explicit keyword on one of the unordered_map constructor,
surely the result of a copy/paste
- Use constructor delegation as proposed by the Standard
- Move code to follow Standard description order, it is easier this way
to check that nothing is missing.
* include/bits/unordered_map.h (unordered_map, unordered_multimap): Add
missing constructors.
* include/bits/unordered_set.h (unordered_set, unordered_multiset):
Likewise.
* include/debug/unordered_map (unordered_map, unordered_multimap): Add
missing constructors.
* include/debug/unordered_set (unordered_set, unordered_multiset):
Likewise.
* include/profile/unordered_map (unordered_map,
unordered_multimap): Add
missing constructors.
* include/profile/unordered_set (unordered_set, unordered_multiset):
Likewise.
* testsuite/23_containers/unordered_map/cons/66055.cc: Add constructor
invocations.
* testsuite/23_containers/unordered_multimap/cons/66055.cc: Likewise.
* testsuite/23_containers/unordered_multiset/cons/66055.cc: Likewise.
* testsuite/23_containers/unordered_set/cons/66055.cc: Likewise.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/unordered_map.h b/libstdc++-v3/include/bits/unordered_map.h
index 069b859..6ace59d 100644
--- a/libstdc++-v3/include/bits/unordered_map.h
+++ b/libstdc++-v3/include/bits/unordered_map.h
@@ -146,17 +146,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_h(__n, __hf, __eql, __a)
{ }
- unordered_map(size_type __n, const allocator_type& __a)
- : _M_h(__n, hasher(), key_equal(), __a)
- { }
-
- explicit
- unordered_map(size_type __n,
- const hasher& __hf,
- const allocator_type& __a)
- : _M_h(__n, __hf, key_equal(), __a)
- { }
-
/**
* @brief Builds an %unordered_map from a range.
* @param __first An input iterator.
@@ -233,6 +222,41 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_h(__l, __n, __hf, __eql, __a)
{ }
+ unordered_map(size_type __n, const allocator_type& __a)
+ : unordered_map(__n, hasher(), key_equal(), __a)
+ { }
+
+ unordered_map(size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_map(__n, __hf, key_equal(), __a)
+ { }
+
+ template
+ unordered_map(_InputIterator __first, _InputIterator __last,
+ size_type __n,
+ const allocator_type& __a)
+ : unordered_map(__first, __last, __n, hasher(), key_equal(), __a)
+ { }
+
+ template
+ unordered_map(_InputIterator __first, _InputIterator __last,
+ size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_map(__first, __last, __n, __hf, key_equal(), __a)
+ { }
+
+ unordered_map(initializer_list __l,
+ size_type __n,
+ const allocator_type& __a)
+ : unordered_map(__l, __n, hasher(), key_equal(), __a)
+ { }
+
+ unordered_map(initializer_list __l,
+ size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_map(__l, __n, __hf, key_equal(), __a)
+ { }
+
/// Copy assignment operator.
unordered_map&
operator=(const unordered_map&) = default;
@@ -872,16 +896,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_h(__n, __hf, __eql, __a)
{ }
- unordered_multimap(size_type __n, const allocator_type& __a)
- : _M_h(__n, hasher(), key_equal(), __a)
- { }
-
- unordered_multimap(size_type __n,
- const hasher& __hf,
- const allocator_type& __a)
- : _M_h(__n, __hf, key_equal(), __a)
- { }
-
/**
* @brief Builds an %unordered_multimap from a range.
* @param __first An input iterator.
@@ -958,6 +972,41 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_h(__l, __n, __hf, __eql, __a)
{ }
+ unordered_multimap(size_type __n, const allocator_type& __a)
+ : unordered_multimap(__n, hasher(), key_equal(), __a)
+ { }
+
+ unordered_multimap(size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_multimap(__n, __hf, key_equal(), __a)
+ { }
+
+ template
+ unordered_multimap(_InputIterator __first, _InputIterator __last,
+ size_type __n,
+ const allocator_type& __a)
+ : unordered_multimap(__first, __last, __n, hasher(), key_equal(), __a)
+ { }
+
+ template
+ unordered_multimap(_InputIterator __first, _InputIterator __last,
+ size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_multimap(__first, __last, __n, __hf, key_equal(), __a)
+ { }
+
+ unordere
[patch, fortran] Fix PR 66113 error with deeply nested blocks
Hello world,
this (rather obvious) patch fixes array declarations in deeply nested
BLOCKs.
Regression-tested. OK for trunk?
Thomas
2015-05-16 Thomas Koenig
PR fortran/66113
* expr.c (is_parent_of_current_ns): New function.
(check_restricted): Use it.
2015-05-16 Thomas Koenig
PR fortran/66113
* gfortran.dg/block_14.f90: New test.
Index: expr.c
===
--- expr.c (Revision 223202)
+++ expr.c (Arbeitskopie)
@@ -2841,7 +2841,19 @@ check_references (gfc_ref* ref, bool (*checker) (g
return check_references (ref->next, checker);
}
+/* Return true if ns is a parent of the current ns. */
+static bool
+is_parent_of_current_ns (gfc_namespace *ns)
+{
+ gfc_namespace *p;
+ for (p = gfc_current_ns->parent; p; p = p->parent)
+if (ns == p)
+ return true;
+
+ return false;
+}
+
/* Verify that an expression is a restricted expression. Like its
cousin check_init_expr(), an error message is generated if we
return false. */
@@ -2929,9 +2941,7 @@ check_restricted (gfc_expr *e)
|| sym->attr.dummy
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
- || (sym->ns && sym->ns == gfc_current_ns->parent)
- || (sym->ns && gfc_current_ns->parent
- && sym->ns == gfc_current_ns->parent->parent)
+ || is_parent_of_current_ns (sym->ns)
|| (sym->ns->proc_name != NULL
&& sym->ns->proc_name->attr.flavor == FL_MODULE)
|| (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
! { dg-do run }
! PR 66113 - this used to ICE with deeply nested BLOCKS.
program main
integer :: n
real :: s
n = 3
block
block
block
block
block
real, dimension(n) :: a
a = 3.
s = sum(a)
end block
end block
end block
end block
end block
if (s /= 9) call abort
end program main
Re: [patch, fortran] Fix PR 66113 error with deeply nested blocks
Le 16/05/2015 12:35, Thomas Koenig a écrit : > Hello world, > > this (rather obvious) patch fixes array declarations in deeply nested > BLOCKs. > > Regression-tested. OK for trunk? > OK, thanks. Mikael
Re: [PATCH] fortran/66052 -- Prevent dereference of NULL pointer
Le 15/05/2015 17:19, Steve Kargl a écrit :
> Regression tested on trunk. OK to commit?
>
Hello,
> Index: gcc/fortran/decl.c
> ===
> --- gcc/fortran/decl.c(revision 223094)
> +++ gcc/fortran/decl.c(working copy)
> @@ -6968,7 +6968,8 @@ gfc_match_protected (void)
>gfc_symbol *sym;
>match m;
>
> - if (gfc_current_ns->proc_name->attr.flavor != FL_MODULE)
> + if (gfc_current_ns->proc_name
> + && gfc_current_ns->proc_name->attr.flavor != FL_MODULE)
> {
> gfc_error ("PROTECTED at %C only allowed in specification "
> "part of a module");
Wouldn't one get a slightly better error message if using
!gfc_current_ns->proc_name
|| gfc_current_ns->proc_name->attr.flavor != FL_MODULE
as condition ?
OK with that change. Thanks.
Mikael
MAINTAINERS (Write After Approval): Add myself.
Adding myself as Write After Approval. Regards Iain --- ChangeLog | 4 MAINTAINERS | 1 + 2 files changed, 5 insertions(+) --- ChangeLog +++ ChangeLog @@ -1,3 +1,7 @@ +2015-05-16 Iain Buclaw + + * MAINTAINERS (Write After Approval): Add myself. + 2015-05-11 Paulo Matos * MAINTAINERS (Write After Approval): Add myself. --- MAINTAINERS +++ MAINTAINERS @@ -351,6 +351,7 @@ Joel Brobecker Dave Brolley Julian Brown Christian Bruel +Iain Buclaw Kevin Buettner Adam Butcher Andrew Cagney
Re: [PATCH 6/7] [D] libiberty: Improve support for demangling D2 templates
On 14 May 2015 at 19:47, Joseph Myers wrote: > On Thu, 14 May 2015, Iain Buclaw wrote: > >> On another note, I've found out why the remaining 20 symbols in my 75k >> sample failed. They don't fail at all! It's just that they were all >> greater than 33,000 characters in length, and my test used c++filt, >> which trims anything bigger than 32767 (whoops!). > > That would be a bug in c++filt (failure to follow the GNU Coding Standards > regarding having no arbitrary limits). > Good to know. I'll query the binutils maintainers on a fix for that then. Iain
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 4:49 PM, Rich Felker wrote: > On Fri, May 15, 2015 at 04:34:57PM -0700, H.J. Lu wrote: >> On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: >> > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: >> >> My relax branch proposal works even without LTO. >> >> >> > >> > I will borrow GOTPCREL from x86-64 and do >> > >> > [hjl@gnu-6 relax-4]$ cat b.S >> > call *foo@GOTPCREL(%eax) >> >> call *foo@GOTPLT(%eax) >> >> is a better choice. > > foo@GOTPCREL is preferable (but does not yet exist for ia32, so the > reloc type would have to be added) since it saves a useless add. > Instead of: > > call __x86.get_pc_thunk.ax > addl $_GLOBAL_OFFSET_TABLE_, %eax > call *foo@GOTPLT(%eax) > > you can just do: > > call __x86.get_pc_thunk.ax > call *foo@GOTPCREL(%eax) > > Note that it also works to have extra instructions between: > > call __x86.get_pc_thunk.ax > 1: ... > call *foo@GOTPCREL+(1b-.)(%eax) > > I may not have gotten the syntax quite right, but hopefully yoy get > the idea. This same approach (with GOTPCREL) can be used for _all_ GOT > accesses, including global data, to eliminate the useless add. > This is a good idea. But I'd like to use something for both i386 and x86-64. I am proposing call/jmp *foo@GOTPCRELAX+addend(%reg) It is similar to @GOTPCREL, but with a new relax relocation. Before I can do that, I need to fix https://sourceware.org/bugzilla/show_bug.cgi?id=18423 first. -- H.J.
Re: [RFC]: Remove Mem/address type assumption in combiner
On Fri, May 15, 2015 at 10:40:48PM -0400, Hans-Peter Nilsson wrote:
> I confess the test-case-"guarded" addi pattern should have been
> expressed with a shift in addition to the multiplication.
But they wouldn't ever match so they might very well have bitrotted
by now :-(
> ("In
> addition to" as the canonically wrong one used to be the
> combine-matching pattern; I'm not sure I should really drop that
> just yet.)
It is harmless to leave it in. It will rot though, eventually --
better take it out before then. Add some gcc_unreachable, perhaps.
> Supposedly more noteworthy: this now-stricter canonicalization
> leads to a requirement to rewrite patterns that used to be:
>
> (parallel
> [(set reg0 (mem (plus (mult reg1 N) reg2)))
>(set reg3 (plus (mult reg1 N) reg2))])
>
> into the awkwardly asymmetric:
>
> (parallel
> [(set reg0 (mem (plus (mult reg1 2) reg2)))
>(set reg3 (plus (ashift reg1 M) reg2))])
>
> (where M = log2 N)
Yeah. This is true of parallels in general: canonicalisation works
on each arm separately. I'm not sure what can be done about it.
Looks like quite some work for you, I'm sorry about that,
Segher
Re: [PATCH 6/7] [D] libiberty: Improve support for demangling D2 templates
On 14 May 2015 at 17:30, Iain Buclaw wrote:
> On 14 May 2015 at 15:24, Jeff Law wrote:
>> On 05/13/2015 02:51 AM, Iain Buclaw wrote:
>>>
>>> In my tests, this gives the demangler near-complete support. Of a
>>> sample of about 75k symbols pulled from the standard library
>>> unittester, all but 20 were successfully parsed.
>>>
>>> ---
>>> libiberty/ChangeLog:
>>>
>>> 2015-05-13 Iain Buclaw
>>>
>>> * d-demangle.c (dlang_symbol_kinds): New enum.
>>> (dlang_parse_symbol): Update signature. Handle an ambiguity between
>>> mangle
>>> symbol for pascal and template value arguments. Only check for a
>>> type
>>> if parsing a function, or at the top level. Return failure if the
>>> entire symbol was not successfully demangled.
>>> (dlang_identifier): Update signature. Handle an ambiguity between
>>> two
>>> adjacent digits in a mangled symbol string.
>>> (dlang_type): Update call to dlang_parse_symbol.
>>> (dlang_template_args): Likewise.
>>> (dlang_parse_template): Likewise.
>>> (dlang_demangle): Likewise.
>>> * testsuite/d-demangle-expected: Fix bad tests found, and add
>>> problematic
>>> examples to the unittests.
>>
>> OK.
>>
>> I'm going to trust the code to dis-ambiguate the adjacent digits in
>> dlang_identifier is correct. The rest of the changes were pretty easy to
>> follow :-0
>>
>> Jeff
>>
>
> Actually, the one snippest that we should be OK without in that
> dis-ambiguate section is the while loop with the comment: "handle any
> overflow".
>
Also discovered that an infinite loop is possible in that
dis-ambiguate section when was testing gdb with these changes on
various programs.
So FYI, I've removed the while loop and fixed the problem as mentioned above.
Unfortunately it also seems that the second ambiguity with
extern(Pascal) vs Template value parameters is a little bit more
difficult to get right. I've got an idea for a way to handle it, but
I don't like it, so I've raised a bug in upstream D. :-)
Iain
---
libiberty/d-demangle.c | 185
libiberty/testsuite/d-demangle-expected | 44 ++--
2 files changed, 175 insertions(+), 54 deletions(-)
diff --git a/libiberty/d-demangle.c b/libiberty/d-demangle.c
index 0af926c..c697b00 100644
--- a/libiberty/d-demangle.c
+++ b/libiberty/d-demangle.c
@@ -165,6 +165,21 @@ string_prepend (string *p, const char *s)
}
}
+/* What kinds of symbol we could be parsing. */
+enum dlang_symbol_kinds
+{
+ /* Top-level symbol, needs it's type checked. */
+ dlang_top_level,
+ /* Function symbol, needs it's type checked. */
+ dlang_function,
+ /* Strongly typed name, such as for classes, structs and enums. */
+ dlang_type_name,
+ /* Template identifier. */
+ dlang_template_ident,
+ /* Template symbol parameter. */
+ dlang_template_param
+};
+
/* Prototypes for forward referenced functions */
static const char *dlang_function_args (string *, const char *);
@@ -172,7 +187,8 @@ static const char *dlang_type (string *, const char *);
static const char *dlang_value (string *, const char *, const char *, char);
-static const char *dlang_parse_symbol (string *, const char *);
+static const char *dlang_parse_symbol (string *, const char *,
+ enum dlang_symbol_kinds);
static const char *dlang_parse_tuple (string *, const char *);
@@ -527,7 +543,7 @@ dlang_type (string *decl, const char *mangled)
case 'E': /* enum T */
case 'T': /* typedef T */
mangled++;
- return dlang_parse_symbol (decl, mangled);
+ return dlang_parse_symbol (decl, mangled, dlang_type_name);
case 'D': /* delegate T */
{
string mods;
@@ -662,114 +678,162 @@ dlang_type (string *decl, const char *mangled)
/* Extract the identifier from MANGLED and append it to DECL.
Return the remaining string on success or NULL on failure. */
static const char *
-dlang_identifier (string *decl, const char *mangled)
+dlang_identifier (string *decl, const char *mangled,
+ enum dlang_symbol_kinds kind)
{
+ char *endptr;
+ long len;
+
if (mangled == NULL || *mangled == '\0')
return NULL;
- if (ISDIGIT (*mangled))
+ len = strtol (mangled, &endptr, 10);
+
+ if (endptr == NULL || len <= 0)
+return NULL;
+
+ /* In template parameter symbols, the first character of the mangled
+ name can be a digit. This causes ambiguity issues because the
+ digits of the two numbers are adjacent. */
+ if (kind == dlang_template_param)
{
- char *endptr;
- long i = strtol (mangled, &endptr, 10);
+ long psize = len;
+ char *pend;
+ int saved = string_length (decl);
+
+ /* Work backwards until a match is found. */
+ for (pend = endptr; endptr != NULL; pend--)
+ {
+ mangled = pend;
- if (endptr == NULL || i <= 0 || strlen (endptr) < (size_t) i)
+ /* Reached the beginning of the pointer to the name length,
+ try parsing the entire symbol. */
+ if
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
* ping * On 05/13/2015 06:58 PM, Jerry DeLisle wrote: > The attached patch reverts a change I made for pr65456 which caused this > regression and adds a check for quotes farther down in the function. This > avoids treating a '!' in a string as a comment and wiping the rest of the > line. > > I found the added code is needed because an interposing quote was falling > through and being changed into an empty space at around line 1393. (I do not > recall the history of why that space is being done) > > The patch also updates test case continuation_13.f90. I found another > compiler I > use for comparison interprets the continuation differently and is inserting a > space that is not there. So the format at line 800 is handled differently now > with gfortran and the line 900 format example is added to test both cases with > and without a space just before the continuation. > > I think now gfortran has this right, but I do not rule out that I am missing > some obscure rule. This begs a question about the space character substituted > near line 1393 in scanner.c, but at the moment I do not think it is related. > > I have also added a test for the the original problem reported in this PR to > avoid future repeating of the problem. I will provide a Changelog entry for > the > testsuite changes. > > Regression tested on x86-64-linux. OK to trunk? followed to 5.1? > > Regards, > > Jerry > > > 2015-05-14 Jerry DeLisle > > PR fortran/65903 > * io.c (format_lex): Change to NONSTRING when checking for > possible doubled quote. > * scanner.c (gfc_next_char_literal): Revert change from 64506, > add a check for quotes, and return. >
Re: [PATCH] fortran/66045 -- NULL() on RHS of assignment
>PR fortran/66045 >* expr.c (gfc_check_assign): Check for assignment of NULL() instead >of the (intended) pointer assignment. OK
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
On Sat, May 16, 2015 at 07:52:38AM -0700, Jerry DeLisle wrote: > * ping * > > > 2015-05-14 Jerry DeLisle > > > > PR fortran/65903 > > * io.c (format_lex): Change to NONSTRING when checking for > > possible doubled quote. > > * scanner.c (gfc_next_char_literal): Revert change from 64506, > > add a check for quotes, and return. > > OK. -- Steve
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
Le 14/05/2015 03:58, Jerry DeLisle a écrit :
> The attached patch reverts a change I made for pr65456 which caused this
> regression and adds a check for quotes farther down in the function. This
> avoids treating a '!' in a string as a comment and wiping the rest of the
> line.
>
> I found the added code is needed because an interposing quote was falling
> through and being changed into an empty space at around line 1393. (I do not
> recall the history of why that space is being done)
>
> The patch also updates test case continuation_13.f90. I found another
> compiler I
> use for comparison interprets the continuation differently and is inserting a
> space that is not there. So the format at line 800 is handled differently now
> with gfortran and the line 900 format example is added to test both cases with
> and without a space just before the continuation.
>
> I think now gfortran has this right, but I do not rule out that I am missing
> some obscure rule. This begs a question about the space character substituted
> near line 1393 in scanner.c, but at the moment I do not think it is related.
>
> I have also added a test for the the original problem reported in this PR to
> avoid future repeating of the problem. I will provide a Changelog entry for
> the
> testsuite changes.
>
> Regression tested on x86-64-linux. OK to trunk? followed to 5.1?
>
> Regards,
>
> Jerry
>
>
> 2015-05-14 Jerry DeLisle
>
> PR fortran/65903
> * io.c (format_lex): Change to NONSTRING when checking for
> possible doubled quote.
> * scanner.c (gfc_next_char_literal): Revert change from 64506,
> add a check for quotes, and return.
>
> Index: gcc/testsuite/gfortran.dg/continuation_13.f90
> ===
> --- gcc/testsuite/gfortran.dg/continuation_13.f90 (revision 223105)
> +++ gcc/testsuite/gfortran.dg/continuation_13.f90 (working copy)
> @@ -19,6 +19,8 @@ character(25) :: astring
> )
> 800 format('This is actually ok.'& !comment
> ' end' )
> +900 format('This is actually ok.' & !comment
> + ' end' )
> write(astring,100)
> if (astring.ne."This format is OK.") call abort
> write(astring,200)
Hello, is the new format labelled 900 correct?
It seems to me it is equivalent to
900 format('This is actually ok.' ' end')
which is missing a comma, no?
This is a real question, I'm no format guru.
By the way, the existing 800 format is also not right, it shouldn't have
a comment, and it should have a '&' at the beginning of the next line.
Section 3.3.2.4 "Free form statement continuation", last paragraph has:
> If a character context is to be continued, an “&” shall be the last
> nonblank character on the line and shall not be followed by commentary.
> There shall be a later line that is not a comment; an “&” shall be the
> first nonblank character on the next such line and the statement
> continues with the next character following that “&”.
To be honest, I see little value in supporting continuation between the
two consecutive quotes coding one quote inside a string constant, it's
confusing. But if it's supported, the above applies, doesn't it?
Regarding the patch itself, I haven't looked at it in great details yet,
but as it's a revert basically, it shouldn't do a lot of harm.
I see that the fixed form part isn't reverted, is it on purpose?
Mikael
Re: Refactor gimple_expr_type
On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote:
> Hi,
> I have tried to refactor gimple_expr_type to make it more readable. Removed
> the switch block and redundant if.
>
> Please review this patch.
for some reason your mail client seems to be inserting non breaking
spaces all over the place. Please either configure it to not do that,
or use git send-email for patches.
>
> Thanks,
> -Aditya
>
>
> gcc/ChangeLog:
>
> 2015-05-15 hiraditya
>
> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove
> redundant if.
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 95e4fc8..168d3ba 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -5717,35 +5717,28 @@ static inline tree
> gimple_expr_type (const_gimple stmt)
> {
> enum gimple_code code = gimple_code (stmt);
> -
> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
> + tree type;
> + /* In general we want to pass out a type that can be substituted
> + for both the RHS and the LHS types if there is a possibly
> + useless conversion involved. That means returning the
> + original RHS type as far as we can reconstruct it. */
> + if (code == GIMPLE_CALL)
> {
> - tree type;
> - /* In general we want to pass out a type that can be substituted
> - for both the RHS and the LHS types if there is a possibly
> - useless conversion involved. That means returning the
> - original RHS type as far as we can reconstruct it. */
> - if (code == GIMPLE_CALL)
> - {
> - const gcall *call_stmt = as_a (stmt);
> - if (gimple_call_internal_p (call_stmt)
> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
> - else
> - type = gimple_call_return_type (call_stmt);
> - }
> + const gcall *call_stmt = as_a (stmt);
> + if (gimple_call_internal_p (call_stmt)
> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
> + else
> + type = gimple_call_return_type (call_stmt);
> + return type;
You might as well return the value directly and not use the variable.
> + }
> + else if (code == GIMPLE_ASSIGN)
else after return is kind of silly.
> + {
> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> else
> - switch (gimple_assign_rhs_code (stmt))
> - {
> - case POINTER_PLUS_EXPR:
> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> - break;
> -
> - default:
> - /* As fallback use the type of the LHS. */
> - type = TREE_TYPE (gimple_get_lhs (stmt));
> - break;
> - }
> + /* As fallback use the type of the LHS. */
> + type = TREE_TYPE (gimple_get_lhs (stmt));
> return type;
might as well not use type here either.
Trev
> }
> else if (code == GIMPLE_COND)
>
>
Re: [RFC]: Remove Mem/address type assumption in combiner
On Sat, 16 May 2015, Segher Boessenkool wrote:
> On Fri, May 15, 2015 at 10:40:48PM -0400, Hans-Peter Nilsson wrote:
> > I confess the test-case-"guarded" addi pattern should have been
> > expressed with a shift in addition to the multiplication.
>
> But they wouldn't ever match so they might very well have bitrotted
> by now :-(
It seems you're saying that the canonicalization to "ashift"
didn't work *at all*, when starting with an expression from an
address? I knew it failed in part, but always thought it was
just a partial failure.
> > ("In
> > addition to" as the canonically wrong one used to be the
> > combine-matching pattern; I'm not sure I should really drop that
> > just yet.)
>
> It is harmless to leave it in. It will rot though, eventually --
> better take it out before then. Add some gcc_unreachable, perhaps.
I've been trying to come up with valid reasons there'd be ever
be canonicalization by multiplication, but failed so I guess
I'll rip it out.
> > Supposedly more noteworthy: this now-stricter canonicalization
> > leads to a requirement to rewrite patterns that used to be:
> >
> > (parallel
> > [(set reg0 (mem (plus (mult reg1 N) reg2)))
> >(set reg3 (plus (mult reg1 N) reg2))])
> >
> > into the awkwardly asymmetric:
> >
> > (parallel
> > [(set reg0 (mem (plus (mult reg1 2) reg2)))
> >(set reg3 (plus (ashift reg1 M) reg2))])
> >
> > (where M = log2 N)
>
> Yeah. This is true of parallels in general: canonicalisation works
> on each arm separately. I'm not sure what can be done about it.
>
>
> Looks like quite some work for you, I'm sorry about that,
It's almost over, at least the editing part...
brgds, H-P
[Patch, fortran] PR61831 side-effect deallocation of variable components
Hello,
this is about PR61831 where in code like:
type :: string_t
character(LEN=1), dimension(:), allocatable :: chars
end type string_t
type(string_t) :: prt_in
(...)
tmp = new_prt_spec ([prt_in])
the deallocation of the argument's allocatable components after the
procedure call (to new_prt_spec) has the side effect of freeing prt_in's
allocatable components, as the array constructor temporary for [prt_in]
is a shallow copy of prt_in.
This bug is a regression caused by the Dominique's PR41936 memory leak
fix, itself based on a patch originally from me.
The attached patch is basically a revert of that fix. It avoids the
problem by not deallocating allocatable components in the problematic
case, at the price of a (possible) memory leak. A new function is
introduced telling whether there is aliasing, so that we don't regress
on PR41936's memory leak when there is no aliasing, and we don't free
components when there is aliasing.
The possible remaining memory leak case is the case of a "mixed" array
constructor with some parts aliasing variables, and some non-aliasing parts.
The patch takes also the opportunity to reassemble the scattered
procedure argument deallocation code into a single place.
The test needs pr65792's fix (thanks Paul), so for the 4.9 branch I
propose commenting the parts that depend on PR65792 in the test.
Regression tested on x86_64-linux. OK for 6/5/4.9 ?
Mikael
2015-05-16 Mikael Morin
PR fortran/61831
* trans-array.c (gfc_conv_array_parameter): Remove allocatable
component deallocation code generation.
* trans-expr.c (gfc_conv_expr_reference): Ditto.
(expr_may_alias_variables): New function.
(gfc_conv_procedure_call): Use it to decide whether generate
allocatable component deallocation code.
(gfc_trans_subarray_assign): Set deep copy flag.
2015-05-16 Mikael Morin
PR fortran/61831
* gfortran.dg/derived_constructor_components_6.f90: New.
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 8267f6a..210b2ec 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -7305,19 +7305,6 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
expr, size);
}
- /* Deallocate the allocatable components of structures that are
- not variable. */
- if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
- && expr->ts.u.derived->attr.alloc_comp
- && expr->expr_type != EXPR_VARIABLE)
-{
- tmp = build_fold_indirect_ref_loc (input_location, se->expr);
- tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, tmp, expr->rank);
-
- /* The components shall be deallocated before their containing entity. */
- gfc_prepend_expr_to_block (&se->post, tmp);
-}
-
if (g77 || (fsym && fsym->attr.contiguous
&& !gfc_is_simply_contiguous (expr, false)))
{
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 9be8a42..e375453 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -4536,6 +4536,36 @@ conv_arglist_function (gfc_se *se, gfc_expr *expr, const char *name)
}
+/* Temporary arrays generated to represent array constructors are made
+ using simple copies, so that their elements may alias some variable
+ they were copied from.
+ This function tells whether the expression given as input may alias
+ some other variable, under the assumption that only variables and array
+ constructor may alias (in particular structure constructors don't alias),
+ and array constructor elements alias iff they are copied from a variable.
+ This function is used to decide whether freeing an expression's allocatable
+ components is safe or should be avoided. */
+
+static bool
+expr_may_alias_variables (gfc_expr *e)
+{
+ gfc_constructor *c;
+
+ if (e->expr_type == EXPR_VARIABLE)
+return true;
+ else if (e->expr_type != EXPR_ARRAY)
+return false;
+
+ for (c = gfc_constructor_first (e->value.constructor);
+ c; c = gfc_constructor_next (c))
+if (c->expr
+ && expr_may_alias_variables (c->expr))
+ return true;
+
+ return false;
+}
+
+
/* Generate code for a procedure call. Note can return se->post != NULL.
If se->direct_byref is set then se->expr contains the return parameter.
Return nonzero, if the call has alternate specifiers.
@@ -5328,7 +5358,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
if (e && (e->ts.type == BT_DERIVED || e->ts.type == BT_CLASS)
&& e->ts.u.derived->attr.alloc_comp
&& !(e->symtree && e->symtree->n.sym->attr.pointer)
- && (e->expr_type != EXPR_VARIABLE && !e->rank))
+ && !expr_may_alias_variables (e))
{
int parm_rank;
tmp = build_fold_indirect_ref_loc (input_location,
@@ -6642,7 +6672,7 @@ gfc_trans_subarray_assign (tree dest, gfc_component * cm, gfc_expr * expr)
gfc_conv_expr (&rse,
Re: [RFC]: Remove Mem/address type assumption in combiner
On Sat, May 16, 2015 at 12:36:38PM -0400, Hans-Peter Nilsson wrote:
> On Sat, 16 May 2015, Segher Boessenkool wrote:
> > On Fri, May 15, 2015 at 10:40:48PM -0400, Hans-Peter Nilsson wrote:
> > > I confess the test-case-"guarded" addi pattern should have been
> > > expressed with a shift in addition to the multiplication.
> >
> > But they wouldn't ever match so they might very well have bitrotted
> > by now :-(
>
> It seems you're saying that the canonicalization to "ashift"
> didn't work *at all*, when starting with an expression from an
> address? I knew it failed in part, but always thought it was
> just a partial failure.
With a plus or minus combine would always write it as a mult.
I don't think any other pass would create this combination. I
haven't tested it though.
> > > ("In
> > > addition to" as the canonically wrong one used to be the
> > > combine-matching pattern; I'm not sure I should really drop that
> > > just yet.)
> >
> > It is harmless to leave it in. It will rot though, eventually --
> > better take it out before then. Add some gcc_unreachable, perhaps.
>
> I've been trying to come up with valid reasons there'd be ever
> be canonicalization by multiplication, but failed so I guess
> I'll rip it out.
The "unreachable" thing should quickly tell you if that guess is wrong.
Not something you want to leave in a production compiler, of course.
> > Looks like quite some work for you, I'm sorry about that,
>
> It's almost over, at least the editing part...
Great to hear that!
Segher
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
On 05/16/2015 08:17 AM, Mikael Morin wrote:
snip
>>
>> Index: gcc/testsuite/gfortran.dg/continuation_13.f90
>> ===
>> --- gcc/testsuite/gfortran.dg/continuation_13.f90(revision 223105)
>> +++ gcc/testsuite/gfortran.dg/continuation_13.f90(working copy)
>> @@ -19,6 +19,8 @@ character(25) :: astring
>> )
>> 800 format('This is actually ok.'& !comment
>> ' end' )
>> +900 format('This is actually ok.' & !comment
>> + ' end' )
>> write(astring,100)
>> if (astring.ne."This format is OK.") call abort
>> write(astring,200)
>
> Hello, is the new format labelled 900 correct?
> It seems to me it is equivalent to
> 900 format('This is actually ok.' ' end')
> which is missing a comma, no?
> This is a real question, I'm no format guru.
Yes, I agree one ought to use a comma. See below.
>
> By the way, the existing 800 format is also not right, it shouldn't have
> a comment, and it should have a '&' at the beginning of the next line.
> Section 3.3.2.4 "Free form statement continuation", last paragraph has:
>
>> If a character context is to be continued, an “&” shall be the last
>> nonblank character on the line and shall not be followed by commentary.
>> There shall be a later line that is not a comment; an “&” shall be the
>> first nonblank character on the next such line and the statement
>> continues with the next character following that “&”.
The "right" way is to put the "&" at the beginning of the next line. It is
common practice to allow this as an extension and it is assumed to begin at the
first non-space character encountered. This does require special handling of
continued literal strings. This is why we try to give errors with -std=f95, etc.
$ gfc -std=f95 continuation_13.f90
continuation_13.f90:24:10:
' end' )
1
Error: GNU Extension: Missing comma at (1)
continuation_13.f90:41:17:
write(astring,900)
1
Error: FORMAT label 900 at (1) not defined
I think these are cases where even the error detection is not perfect. User
beware.
>
> To be honest, I see little value in supporting continuation between the
> two consecutive quotes coding one quote inside a string constant, it's
> confusing. But if it's supported, the above applies, doesn't it?
>
>
> Regarding the patch itself, I haven't looked at it in great details yet,
> but as it's a revert basically, it shouldn't do a lot of harm.
> I see that the fixed form part isn't reverted, is it on purpose?
Fixed form continuations are handled differently and I am not trying to address
issues there, if any. As I have time, I will dabble further into this. Getting
ready to move here from Southern California back to Washington State.
Regards,
Jerry
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
On 05/16/2015 08:11 AM, Steve Kargl wrote: > On Sat, May 16, 2015 at 07:52:38AM -0700, Jerry DeLisle wrote: >> * ping * >> >>> 2015-05-14 Jerry DeLisle >>> >>> PR fortran/65903 >>> * io.c (format_lex): Change to NONSTRING when checking for >>> possible doubled quote. >>> * scanner.c (gfc_next_char_literal): Revert change from 64506, >>> add a check for quotes, and return. >>> > > OK. > Thanks Steve, Committed revision 223248.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Sat, May 16, 2015 at 7:19 AM, H.J. Lu wrote: > On Fri, May 15, 2015 at 4:49 PM, Rich Felker wrote: >> On Fri, May 15, 2015 at 04:34:57PM -0700, H.J. Lu wrote: >>> On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: >>> > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: >>> >> My relax branch proposal works even without LTO. >>> >> >>> > >>> > I will borrow GOTPCREL from x86-64 and do >>> > >>> > [hjl@gnu-6 relax-4]$ cat b.S >>> > call *foo@GOTPCREL(%eax) >>> >>> call *foo@GOTPLT(%eax) >>> >>> is a better choice. >> >> foo@GOTPCREL is preferable (but does not yet exist for ia32, so the >> reloc type would have to be added) since it saves a useless add. >> Instead of: >> >> call __x86.get_pc_thunk.ax >> addl $_GLOBAL_OFFSET_TABLE_, %eax >> call *foo@GOTPLT(%eax) >> >> you can just do: >> >> call __x86.get_pc_thunk.ax >> call *foo@GOTPCREL(%eax) >> >> Note that it also works to have extra instructions between: >> >> call __x86.get_pc_thunk.ax >> 1: ... >> call *foo@GOTPCREL+(1b-.)(%eax) >> >> I may not have gotten the syntax quite right, but hopefully yoy get >> the idea. This same approach (with GOTPCREL) can be used for _all_ GOT >> accesses, including global data, to eliminate the useless add. >> > > This is a good idea. But I'd like to use something for both i386 and > x86-64. I am proposing > > call/jmp *foo@GOTPCRELAX+addend(%reg) > > It is similar to @GOTPCREL, but with a new relax relocation. Before > I can do that, I need to fix It doesn't work. REG must hold GOT base for other GOT relocations. We need to keep addl $_GLOBAL_OFFSET_TABLE_, %eax -- H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Sat, May 16, 2015 at 11:59:56AM -0700, H.J. Lu wrote: > On Sat, May 16, 2015 at 7:19 AM, H.J. Lu wrote: > > On Fri, May 15, 2015 at 4:49 PM, Rich Felker wrote: > >> On Fri, May 15, 2015 at 04:34:57PM -0700, H.J. Lu wrote: > >>> On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: > >>> > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: > >>> >> My relax branch proposal works even without LTO. > >>> >> > >>> > > >>> > I will borrow GOTPCREL from x86-64 and do > >>> > > >>> > [hjl@gnu-6 relax-4]$ cat b.S > >>> > call *foo@GOTPCREL(%eax) > >>> > >>> call *foo@GOTPLT(%eax) > >>> > >>> is a better choice. > >> > >> foo@GOTPCREL is preferable (but does not yet exist for ia32, so the > >> reloc type would have to be added) since it saves a useless add. > >> Instead of: > >> > >> call __x86.get_pc_thunk.ax > >> addl $_GLOBAL_OFFSET_TABLE_, %eax > >> call *foo@GOTPLT(%eax) > >> > >> you can just do: > >> > >> call __x86.get_pc_thunk.ax > >> call *foo@GOTPCREL(%eax) > >> > >> Note that it also works to have extra instructions between: > >> > >> call __x86.get_pc_thunk.ax > >> 1: ... > >> call *foo@GOTPCREL+(1b-.)(%eax) > >> > >> I may not have gotten the syntax quite right, but hopefully yoy get > >> the idea. This same approach (with GOTPCREL) can be used for _all_ GOT > >> accesses, including global data, to eliminate the useless add. > >> > > > > This is a good idea. But I'd like to use something for both i386 and > > x86-64. I am proposing > > > > call/jmp *foo@GOTPCRELAX+addend(%reg) > > > > It is similar to @GOTPCREL, but with a new relax relocation. Before > > I can do that, I need to fix > > It doesn't work. REG must hold GOT base for other GOT relocations. > We need to keep > > addl $_GLOBAL_OFFSET_TABLE_, %eax Like I just said, all foo@GOT(%gotreg) can be replaced with foo@GOTPCREL+[label-.](%labelreg) where %labelreg is a register pointing to the referenced label (the point at which the program counter was saved). This is a minor but useful optimization that can be made for all GOT accesses, not just ones for [relaxable] function calls. Rich
Re: [patch] libstdc++/66055 add missing constructors to unordered containers
On 16/05/15 11:39 +0200, François Dumont wrote:
On 14/05/2015 15:47, Jonathan Wakely wrote:
Reported by Nathan and fixed by his patch. I added the tests.
Tested powerpc64le-linux, committed to trunk. This should be
backported too.
While backporting to debug and profile mode I noticed that those
constructors were not the only missing ones. So here is a patch to
complete them with debug and profile modes.
Great, thanks.
@@ -233,6 +222,41 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_h(__l, __n, __hf, __eql, __a)
{ }
+ unordered_map(size_type __n, const allocator_type& __a)
+ : unordered_map(__n, hasher(), key_equal(), __a)
+ { }
+
+ unordered_map(size_type __n, const hasher& __hf,
+ const allocator_type& __a)
+ : unordered_map(__n, __hf, key_equal(), __a)
+ { }
+
+ template
+ unordered_map(_InputIterator __first, _InputIterator __last,
+ size_type __n,
+ const allocator_type& __a)
+ : unordered_map(__first, __last, __n, hasher(), key_equal(), __a)
The indentation is inconsistent here, the ctor-initializer-list is
indented further than necessary
@@ -891,7 +941,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
* in the initializer list @a __l.
*
* Note that the assignment completely changes the %unordered_multiset
- * and that the resulting %unordered_set's size is the same as the
number
+ * and that the resulting %unordered_multiset's size is the same as the
number
* of elements assigned. Old data may be lost.
Please reformat this to stay below 80 columns.
OK with those two tiny adjustments, thanks!
Re: [patch, fortran] [5/6 Regression] Line continuation followed by comment character in string fails to compile
On 05/16/2015 10:45 AM, Jerry DeLisle wrote:
--- snip ---
> Thanks Steve,
>
> Committed revision 223248.
>
>
I had some time to play with this a little more this afternoon.
I am going to commit the following little patchlet that gives us the nice
warning we should have. (After full regression testing of course)
gfc -Wall continuation_13.f90
continuation_13.f90:22:4: Warning: Missing ‘&’ in continued character constant
at (1) [-Wampersand]
continuation_13.f90:24:4: Warning: Missing ‘&’ in continued character constant
at (1) [-Wampersand]
Index: scanner.c
===
--- scanner.c (revision 223250)
+++ scanner.c (working copy)
@@ -1383,7 +1383,12 @@
"constant at %C");
}
else if (!in_string && (c == '\'' || c == '"'))
+ {
+ gfc_warning (OPT_Wampersand,
+"Missing %<&%> in continued character "
+"constant at %C");
goto done;
+ }
/* Both !$omp and !$ -fopenmp continuation lines have & on the
continuation line only optionally. */
else if (openmp_flag || openacc_flag || openmp_cond_flag)
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Sat, May 16, 2015 at 12:03 PM, Rich Felker wrote: > On Sat, May 16, 2015 at 11:59:56AM -0700, H.J. Lu wrote: >> On Sat, May 16, 2015 at 7:19 AM, H.J. Lu wrote: >> > On Fri, May 15, 2015 at 4:49 PM, Rich Felker wrote: >> >> On Fri, May 15, 2015 at 04:34:57PM -0700, H.J. Lu wrote: >> >>> On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: >> >>> > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: >> >>> >> My relax branch proposal works even without LTO. >> >>> >> >> >>> > >> >>> > I will borrow GOTPCREL from x86-64 and do >> >>> > >> >>> > [hjl@gnu-6 relax-4]$ cat b.S >> >>> > call *foo@GOTPCREL(%eax) >> >>> >> >>> call *foo@GOTPLT(%eax) >> >>> >> >>> is a better choice. >> >> >> >> foo@GOTPCREL is preferable (but does not yet exist for ia32, so the >> >> reloc type would have to be added) since it saves a useless add. >> >> Instead of: >> >> >> >> call __x86.get_pc_thunk.ax >> >> addl $_GLOBAL_OFFSET_TABLE_, %eax >> >> call *foo@GOTPLT(%eax) >> >> >> >> you can just do: >> >> >> >> call __x86.get_pc_thunk.ax >> >> call *foo@GOTPCREL(%eax) >> >> >> >> Note that it also works to have extra instructions between: >> >> >> >> call __x86.get_pc_thunk.ax >> >> 1: ... >> >> call *foo@GOTPCREL+(1b-.)(%eax) >> >> >> >> I may not have gotten the syntax quite right, but hopefully yoy get >> >> the idea. This same approach (with GOTPCREL) can be used for _all_ GOT >> >> accesses, including global data, to eliminate the useless add. >> >> >> > >> > This is a good idea. But I'd like to use something for both i386 and >> > x86-64. I am proposing >> > >> > call/jmp *foo@GOTPCRELAX+addend(%reg) >> > >> > It is similar to @GOTPCREL, but with a new relax relocation. Before >> > I can do that, I need to fix >> >> It doesn't work. REG must hold GOT base for other GOT relocations. >> We need to keep >> >> addl $_GLOBAL_OFFSET_TABLE_, %eax > > Like I just said, all foo@GOT(%gotreg) can be replaced with > foo@GOTPCREL+[label-.](%labelreg) where %labelreg is a register > pointing to the referenced label (the point at which the program > counter was saved). This is a minor but useful optimization that can > be made for all GOT accesses, not just ones for [relaxable] function > calls. There is also foo@GOTOFF(%reg). Remove addl is independent of relax branch. I will leave it out. Relax branch will support call/jmp *bar@GOTRELAX(%reg) for both i386 and x86-64. -- H.J.
Do not output debug info into slim LTO files
Hi, while debugging verify_type ICE I noticed that we output debug info to slim-lto files. This debug info is never used for anything and should be omitted. I wonder if this can go also to rleease branches since it will likely make the slim files smaller. Of course things will change with early debug, but even there we do not want to output all debug this way. Bootstrapped/rgtested x86_64-linux, comitted. Honza Index: ChangeLog === --- ChangeLog (revision 223259) +++ ChangeLog (working copy) @@ -1,5 +1,10 @@ 2015-05-16 Jan HUbicka + * toplev.c (emit_debug_global_declarations): Do not output debug info + when doing slim LTO objects. + +2015-05-16 Jan HUbicka + * ipa-utils.h (warn_types_mismatch, odr_or_derived_type_p, odr_types_equivalent_p): Declare. (odr_type_p): Use gcc_checking_assert. Index: toplev.c === --- toplev.c(revision 223258) +++ toplev.c(working copy) @@ -570,6 +570,9 @@ emit_debug_global_declarations (tree *ve /* Avoid confusing the debug information machinery when there are errors. */ if (seen_error ()) return; + /* No need for debug info in object files when producing slimLTO. */ + if (!in_lto_p && flag_lto && !flag_fat_lto_objects) +return; timevar_push (TV_SYMOUT); for (i = 0; i < len; i++)
Re: [RFC]: Remove Mem/address type assumption in combiner
On Sat, 16 May 2015, Segher Boessenkool wrote:
> On Sat, May 16, 2015 at 12:36:38PM -0400, Hans-Peter Nilsson wrote:
> > On Sat, 16 May 2015, Segher Boessenkool wrote:
> > > On Fri, May 15, 2015 at 10:40:48PM -0400, Hans-Peter Nilsson wrote:
> > > > I confess the test-case-"guarded" addi pattern should have been
> > > > expressed with a shift in addition to the multiplication.
> > >
> > > But they wouldn't ever match so they might very well have bitrotted
> > > by now :-(
> >
> > It seems you're saying that the canonicalization to "ashift"
> > didn't work *at all*, when starting with an expression from an
> > address? I knew it failed in part, but always thought it was
> > just a partial failure.
>
> With a plus or minus combine would always write it as a mult.
> I don't think any other pass would create this combination. I
> haven't tested it though.
Ports probably also generate that internally at various RTL
passes, something that takes a bit more than an at-a-glance code
inspection.
> > > > ("In
> > > > addition to" as the canonically wrong one used to be the
> > > > combine-matching pattern; I'm not sure I should really drop that
> > > > just yet.)
> > >
> > > It is harmless to leave it in. It will rot though, eventually --
> > > better take it out before then. Add some gcc_unreachable, perhaps.
> >
> > I've been trying to come up with valid reasons there'd be ever
> > be canonicalization by multiplication, but failed so I guess
> > I'll rip it out.
>
> The "unreachable" thing should quickly tell you if that guess is wrong.
> Not something you want to leave in a production compiler, of course.
I think you misunderstood; I mean that I pondered
"philosophical" reasons to change the canonical representation;
if there was some reasoning, current or looking forward, where
we'd "add back" mult as non-address-canonical RTL. For the
current scheme, keeping the matching-patterns but with
assertions added might be helpful, yes.
(People looking for practical clues: for spotting dead target
code there are besides the alluded-to gcc_unreachable(), also
the options to call internal_error() with a friendly string or
generating invalid assembly with a descriptive mnemonic.)
Actually, there are two things you could help with:
- (pointer-to) step-by-step instructions to recreate the Linux
(kernel :) build, as those you did before for a multiple of
targets. I'd like to know I fixed your observations.
- add a preferred canonicalization function to do conversion
to/from memory-address-canonical RTL. Like
fwprop.c:canonicalize_address (just not static :) and maybe also
a canonicalize_nonaddress. At the moment, ports call
replace_equiv_address (family of functions) when changing
address RTL, but that code-path (at a glance) doesn't
canonicalize whatever non-address-canonical RTL you throw at it.
Maybe it should?
> > > Looks like quite some work for you, I'm sorry about that,
> >
> > It's almost over, at least the editing part...
>
> Great to hear that!
Thanks for your enthusiasm!
Well, I've known about this wart for 6+ years as seen in PR37939
(but before that likely at least as long), so I *should* thank
you for fixing it; it's just that the added churn and required
tweaks has somewhat of a sour taste. :)
brgds, H-P
