Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes
On Fri, 2013-07-26 at 11:04 -0400, David Malcolm wrote:
> This patch is the hand-written part of the conversion of passes from
> C structs to C++ classes. It does not work without the subsequent
> autogenerated part, which is huge.
>
> Given that the autogenerated part of the conversion is very large
> (500k), for the sake of human comprehension I have kept the change as
> two separate patches to keep the hand-written changes separate from the
> automatically-generated ones. I would commit these as two separate
> changes to SVN in order to keep this readability for posterity in the
> logs as well as at review-time.
>
> This pair of patches eliminates the mutable global variables
> representing the passes, allowing for multiple compilation contexts in
> one process, potentially with different combinations of passes, and with
> pass instance owning additional data.
>
> It converts the hierarchy of opt_pass types into an actual C++ class
> hierarchy, where each of:
>
> * gimple_opt_pass
> * rtl_opt_pass
> * ipa_opt_pass_d
> * simple_ipa_opt_pass
>
> all become subclasses of opt_pass.
[...]
This looks good to me. I suggest adding into the `opt_pass` class two
constant fields for the approximate source location of the pass, e.g. a
field const char* _file; and another const unsigned _lineno; initialized
with __FILE__ and __LINE__ respectively.
This won't cost much (we don't have zillions of instances of
opt_pass) and would help a lot finding where (in which source file)
an actual pass is.
This is particularly useful for newbies writing plugins (which are
trying to add new passes). It takes a lot of time to them to find which
actual source file inside the compiler is implementing a given
(existing) pass.
Cheers
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [Patch] Refractor Thompson matcher
Hi, On 07/28/2013 06:13 AM, Tim Shen wrote: Refractor the whole Thompson matcher using the queue-based(BFS) Bellman-Ford algorithm. Fix the grouping problem. Refactor, refactoring, etc, no 'r'. If the grouping problem is now fixed, would it make sense to add corresponding testcases? Paolo.
Re: [Patch] Refractor Thompson matcher
On Sun, Jul 28, 2013 at 5:12 PM, Paolo Carlini wrote: > Refactor, refactoring, etc, no 'r'. Thanks :) > If the grouping problem is now fixed, would it make sense to add > corresponding testcases? They are already added by http://gcc.gnu.org/ml/gcc-cvs/2013-07/msg00643.html (though I found the changelog entry used old file names, I'll fix it later). This time it's the BFS approach that can correctly handle the problem instead of the DFS one. I think, for now, they can share the testcases. -- Tim Shen
[Patch, Aarch64]: Handle return address via. frame pointer
Hi Maintainers, This patch adds supports to handle return address via. frame pointer. gcc/ChangeLog - 2013-07-28 Venkataramanan Kumar * config/aarch64/aarch64.c (aarch64_return_addr): Handle returning address from a frame. Regression tested with aarch64-none-elf with V8 foundation model. regards, Venkat. gcc.return_addr_from_frame_pointer.diff Description: Binary data
[patch, fortran] PR Detect same values in vector expression subscripts
Hello world,
this patch yields an error for identical values in vector expression
subscripts. The algorithm is O(n**2) because
a) It would be impossible to detect a([i,i]) otherwise
b) This is not likely to be a performance bottleneck because
people don't use large vector indices.
(as noted by the different comments in the PR).
Regression-tested. OK for trunk?
Thomas
2013-07-28 Thomas Koenig
PR fortran/58009
* expr.c (gfc_check_vardef_context): Check for same values in
vector expression subscripts.
2013-07-28 Thomas Koenig
PR fortran/58009
* gfortran.dg/vector_subsript_7.f90: New test.
Index: expr.c
===
--- expr.c (Revision 200743)
+++ expr.c (Arbeitskopie)
@@ -4700,6 +4700,8 @@ gfc_check_vardef_context (gfc_expr* e, bool pointe
bool unlimited;
symbol_attribute attr;
gfc_ref* ref;
+ bool retval;
+ int i;
if (e->expr_type == EXPR_VARIABLE)
{
@@ -4922,5 +4924,54 @@ gfc_check_vardef_context (gfc_expr* e, bool pointe
}
}
- return true;
+ /* Check for same value in vector expression subscript. */
+ retval = true;
+
+ if (e->rank > 0)
+for (ref = e->ref; ref != NULL; ref = ref->next)
+ if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION)
+ for (i = 0; irank; i++)
+ if (ref->u.ar.dimen_type[i] == DIMEN_VECTOR)
+ {
+ gfc_expr *arr = ref->u.ar.start[i];
+ if (arr->expr_type == EXPR_ARRAY)
+ {
+ gfc_constructor *c, *n;
+ gfc_expr *ec, *en;
+
+ for (c = gfc_constructor_first (arr->value.constructor);
+ c != NULL; c = gfc_constructor_next (c))
+ {
+ if (c == NULL || c->iterator != NULL)
+ continue;
+
+ ec = c->expr;
+
+ for (n = gfc_constructor_next (c); n != NULL;
+ n = gfc_constructor_next (n))
+ {
+ if (n->iterator != NULL)
+ continue;
+
+ en = n->expr;
+ if (gfc_dep_compare_expr (ec, en) == 0)
+ {
+ gfc_error_now ("Elements with the same value at %L"
+ " and %L in vector subscript"
+ " in a variable definition"
+ " context (%s)", &(ec->where),
+ &(en->where), context);
+ retval = false;
+
+ /* Do not issue O(n**2) errors for n occurrences
+ of the same value. */
+ break;
+
+ }
+ }
+ }
+ }
+ }
+
+ return retval;
}
! { dg-do compile }
! PR 58009 - If a vector subscript has two or more elements with the
! same value, an array section with that vector subscript
! shall not appear in a variable definition context.
program main
real, dimension(4) :: a,b
read (*,*) a([1,2,3,2]),i ! { dg-error "Elements with the same value" }
b([1+i,1,i+1,2]) = a ! { dg-error "Elements with the same value" }
call foo (a([4,2,1,1])) ! { dg-error "Elements with the same value" }
print *,a,b
contains
subroutine foo(arg)
real, intent(inout) :: arg(:)
arg = arg + 1
end subroutine foo
end
Re: [Patch] Refractor Thompson matcher
Hi, On 07/28/2013 12:18 PM, Tim Shen wrote: They are already added by http://gcc.gnu.org/ml/gcc-cvs/2013-07/msg00643.html (though I found the changelog entry used old file names, I'll fix it later). This time it's the BFS approach that can correctly handle the problem instead of the DFS one. I think, for now, they can share the testcases. I see. I was wondering if in this development stage it would be convenient to have somewhere a parameter allowing to switch by hand such internal details, useful for testing purposes too. Eventually may or may not go away. Paolo.
Re: [Patch] Refractor Thompson matcher
On Sun, Jul 28, 2013 at 9:44 PM, Paolo Carlini wrote: > I see. I was wondering if in this development stage it would be convenient > to have somewhere a parameter allowing to switch by hand such internal > details, useful for testing purposes too. Eventually may or may not go away. Here it is :) Github : https://github.com/innocentim/gcc/tree/tim/regex/libstdc++-v3/include/bits -- Tim Shen bfs.patch Description: Binary data
Re: [patch, fortran] PR Detect same values in vector expression subscripts
Hello Thomas,
Thomas Koenig wrote:
this patch yields an error for identical values in vector expression
Regression-tested. OK for trunk?
+ {
+ if (n->iterator != NULL)
+ continue;
+
+ en = n->expr;
+ if (gfc_dep_compare_expr (ec, en) == 0)
+ {
+ gfc_error_now ("Elements with the same value at
%L"
+" and %L in vector subscript"
+" in a variable definition"
+" context (%s)", &(ec->where),
+&(en->where), context);
+ retval = false;
+
+ /* Do not issue O(n**2) errors for n occurrences
+of the same value. */
+ break;
+
+ }
+ }
Something went wrong with the indentation of the last two lines.
Additionally: How about simply returning with an "return false;"? One
error per vector subscript should be sufficient. In my humble opinion,
having multiple errors will clutter more the output than helping the
user - and in the code one can avoid "retval" which makes the code
(nearly undetectably) cleaner.
OK - with fixing the first item and considering the last item.
Thanks for the patch!
Tobias
Re: [patch] Add new -gmlt option for min. debug info with line tables (issue 4440072)
I'm not aware of any significant use of -g1. It is very rare for anyone to mention it in a bug report for instance. Once upon a time (before 2002-03-19), it was used for compiling libgcc, but that was just to ensure that it got tested somewhere. From my Cisco experience, I would agree that backtraces without line numbers are not very useful. It would be OK with me if these changes were added to -g1 instead of creating a new -gmlt option. Jim Is anyone still working on this? It would be very useful to include this option in gcc trunk, and have either -g1 or -gmlt emit line number information. This saves considerable space and time during compilation for large builds where full debug info is not needed, but line numbers in stack traces are still helpful (e.g. regression testing opt builds) If I can help somehow, please let me know how. Best regards, Martin https://codereview.appspot.com/4440072/
[PATCH]: Fix PR middle-end/56382 -- Only move MODE_COMPLEX_FLOAT by parts if we can create pseudos
This patch fixes PR middle-end/56382 on hppa64-hp-hpux11.11. The patch prevents moving a complex float by parts if we can't create pseudos. On a big endian 64-bit target, we need a psuedo to move a complex float and this fails during reload. OK for trunk? Dave -- John David Anglin dave.ang...@bell.net 2013-07-28 John David Anglin PR middle-end/56382 * expr.c (emit_move_complex): Only move mode MODE_COMPLEX_FLOAT by parts if we can create pseudos. Index: expr.c === --- expr.c (revision 201248) +++ expr.c (working copy) @@ -3236,6 +3236,7 @@ /* Move floating point as parts. */ if (GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT + && can_create_pseudo_p () && optab_handler (mov_optab, GET_MODE_INNER (mode)) != CODE_FOR_nothing) try_int = false; /* Not possible if the values are inherently not adjacent. */
Re: Testsuite tweaks for the SPARC
> That doesn't work on ia64. Yeah, there are so many vectorizer failures on IA-64 that I gave up looking at them some time ago. Maybe vect_pack_trunc should be false there too. At least bb-slp-32.c now passes, so the overall number of failures hasn't increased. :-) -- Eric Botcazou
Re: [PATCH] Enable non-complex math builtins from C99 for Bionic
On 07/27/13 15:18, Alexander Ivchenko wrote: Hi Joseph, thanks for your comments. I updated the patch: 2013/7/9 Joseph S. Myers : * It looks rather like microblaze*-*-* don't use elfos.h, so meaning semantics aren't preserved for those (non-Linux) targets either. Now, I don't know if there's a good reason for not using that file (ask the architecture maintainer), but in any case semantics should be preserved. I don't know why microblaze does not include elfos.h. It looks like it should, to be consistent with other targets. This would require some cleanup and verification. Your patch adds the following to microblaze.h, duplicating the change to elfos.h: +/* microblaze-unknown-elf target has no support of C99 runtime */ +#undef TARGET_LIBC_HAS_FUNCTION +#define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function I'm assuming that this means that no other change to microblaze is needed and the question about elfos.h is moot. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [patch, fortran] PR Detect same values in vector expression subscripts
Le 28/07/2013 14:57, Thomas Koenig a écrit : > Hello world, > > this patch yields an error for identical values in vector expression > subscripts. The algorithm is O(n**2) because > > a) It would be impossible to detect a([i,i]) otherwise > b) This is not likely to be a performance bottleneck because >people don't use large vector indices. > > (as noted by the different comments in the PR). > > Regression-tested. OK for trunk? > > Thomas > > > Index: expr.c > === > --- expr.c(Revision 200743) > +++ expr.c(Arbeitskopie) > @@ -4922,5 +4924,54 @@ gfc_check_vardef_context (gfc_expr* e, bool pointe > } > } > > - return true; > + /* Check for same value in vector expression subscript. */ > + retval = true; > + > + if (e->rank > 0) > +for (ref = e->ref; ref != NULL; ref = ref->next) > + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) > + for (i = 0; irank; i++) > + if (ref->u.ar.dimen_type[i] == DIMEN_VECTOR) I think it should be: > for (i = 0; i < GFC_MAX_DIMENSIONS; i++) otherwise, I'm afraid it will silently disregard array references of the form array(1, (/ ... /)) I haven't tested though. Otherwise looks good. Mikael
Re: [Patch] Refractor Thompson matcher
On 07/28/2013 05:50 PM, Tim Shen wrote: On Sun, Jul 28, 2013 at 9:44 PM, Paolo Carlini wrote: I see. I was wondering if in this development stage it would be convenient to have somewhere a parameter allowing to switch by hand such internal details, useful for testing purposes too. Eventually may or may not go away. Here it is :) Oh well, thanks. But then I expect specific testcases to come with it, for the various values of the parameter, both the default and the other other values. Otherwise, the idea isn't really immediately useful. See what I mean? Also, please make sure that this change is in any case a step in the right direction. I mean: I suppose that if we do this only the logic to control automatically the switch is missing and when we add it we are done. Let's not do it if I'm mistaken. Thanks! Paolo.
Re: Testsuite tweaks for the SPARC
Eric Botcazou writes: > At least bb-slp-32.c now passes, so the overall number of failures > hasn't increased. :-) It's still an XFAIL, though. 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] Add atomic type qualifier
On Fri, 26 Jul 2013, Andrew MacLeod wrote: > What it doesn't do: * It doesn't implement the stdatomic.h header - do you intend that to be provided by GCC or glibc? (Substantive review of the full patch still to come.) > * It doesn't implement the C11 expression expansion into atomic built-ins. > ie, you can't write: > _Atomic int x; > x = 0; > and have the result be an atomic operation calling __atomic_store (&x, > 0). That will be in a follow on patch. So none of the expression expansion > from C11 is included yet. This just enables that. The hardest part will probably be compound assignment to an atomic object where either operand of the assignment has floating-point type - see C11 footnote 113, but you can't actually use feholdexcept or feclearexcept or feupdateenv here because that would introduce libm dependencies, so back ends will need to provide appropriate insn patterns to do those operations inline. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch, fortran] PR Detect same values in vector expression subscripts
Hi Tobias and Mikael, > Something went wrong with the indentation of the last two lines. Fixed. > Additionally: How about simply returning with an "return false;"? After some more thinking, I used the option that you suggested. We'll see if we get feedback from users who want something else, if any :-) I think it should be: >for (i = 0; i < GFC_MAX_DIMENSIONS; i++) Fixed with + for (i = 0; i < GFC_MAX_DIMENSIONS + && ref->u.ar.dimen_type[i] != 0; i++) (I didn't want to loop over all dimensions unconditially). I also updated the test case to reflect this. Commit is at http://gcc.gnu.org/viewcvs?rev=201294&root=gcc&view=rev . Thanks a lot for the reviews! Thomas
[PATCH, libgcc] Fix licenses on several files
While verifying license compliance for GCC and its libraries I noticed that several libgcc files that end up in the final library are licensed under GPL-3.0+ instead of GPL-3.0-with-GCC-exception. This is, obviously, was not the intention of developers who just copied wrong boilerplate text, and this patch fixes the oversights. Just to avoid any possible fallout from this issue ... Marcus, did you and ARM intend to license config/aarch64/sfp-machine.h and config/aarch64/sync-cache.c under GPL-3.0-with-GCC-exception? Sriram, did you and Google intend to license config/i386/cpuinfo.c under GPL-3.0-with-GCC-exception? Richard, did you and Red Hat intend to license config/ia64/unwind-ia64.h under GPL-3.0-with-GCC-exception? DJ, did you and Red Hat intend to license config/mips/vr4120-div.S under GPL-3.0-with-GCC-exception? Once confirmed, I will apply this patch to all active branches: trunk, gcc-4_8-branch and gcc-4_7-branch. Thank you, -- Maxim Kuvyrkov www.kugelworks.com 0001-Fix-licenses-on-several-libgcc-files.patch Description: Binary data
Re: [Patch] Refractor Thompson matcher
On Mon, Jul 29, 2013 at 1:08 AM, Paolo Carlini wrote: > Oh well, thanks. But then I expect specific testcases to come with it, for > the various values of the parameter, both the default and the other other > values. Otherwise, the idea isn't really immediately useful. See what I > mean? So I modified testcases to test both of them. > Also, please make sure that this change is in any case a step in the right > direction. I mean: I suppose that if we do this only the logic to control > automatically the switch is missing and when we add it we are done. Let's > not do it if I'm mistaken. Never mind, this patch makes them switch to be automatic(based on whether there're back-references in the regex), thanks to the full-featured regex scanner. A testcase is given too. -- Tim Shen dispatch.patch Description: Binary data
Re: [patch] implement Cilk Plus simd loops on trunk
On 07/27/2013 05:31 AM, Aldy Hernandez wrote: trunk, but it depends on the OMP_SIMD patch which is also awaiting review (actually, just the vectorizer bits since Jakub wrote and can pre-approve the actual OMP changes): Oh, right. I can rebase off a more recent trunk if you prefer, or I can even post a combined OMP_SIMD + Cilk Plus #pragma simd patchset. However you prefer... No worries, I can just read the patch. :) Jason
Re: [PATCH] Enable non-complex math builtins from C99 for Bionic
2013/7/28 Michael Eager : > On 07/27/13 15:18, Alexander Ivchenko wrote: >> >> Hi Joseph, thanks for your comments. >> >> I updated the patch: > > >> >> 2013/7/9 Joseph S. Myers : > > >>> >>> * It looks rather like microblaze*-*-* don't use elfos.h, so meaning >>> semantics aren't preserved for those (non-Linux) targets either. Now, I >>> don't know if there's a good reason for not using that file (ask the >>> architecture maintainer), but in any case semantics should be preserved. > > > > I don't know why microblaze does not include elfos.h. It looks like > it should, to be consistent with other targets. This would require some > cleanup and verification. > > Your patch adds the following to microblaze.h, duplicating the change > to elfos.h: > +/* microblaze-unknown-elf target has no support of C99 runtime */ > +#undef TARGET_LIBC_HAS_FUNCTION > +#define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function > > I'm assuming that this means that no other change to microblaze is > needed and the question about elfos.h is moot. Yes, with this change in my patch the semantics for microblaze-unknown-elf is preserved. As for microblaze-unknown-linux-gnu case - the "linux_android_libc_has_function" version of TARGET_LIBC_HAS_FUNCTION from linux.h will be used, so the semantics is preserved as well. --Alexander
[Patch, Aarch64] : Macros for profile code generation to enable gprof support
Hi Maintainers, This patch defines some macros that are needed for profile generation support in Aarch64. I tested this patch on top of the patch http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01333.html Regression tested with aarch64-none-elf with V8 foundation model after re basing to latest gcc trunk. Also ran profile related tests against the latest trunk for aarch64-unknown-linux-gnu. Below tests are passing with the patch. Before: UNSUPPORTED: gcc.dg/nested-func-4.c UNSUPPORTED: gcc.dg/pr43643.c: UNSUPPORTED: gcc.dg/nest.c UNSUPPORTED: gcc.dg/20021014-1.c UNSUPPORTED: gcc.dg/pr32450.c UNSUPPORTED: g++.dg/other/profile1.C -std=gnu++98 UNSUPPORTED: g++.dg/other/profile1.C -std=gnu++11 After: --- PASS: gcc.dg/nested-func-4.c (test for excess errors) PASS: gcc.dg/nested-func-4.c execution test PASS: gcc.dg/pr43643.c (test for excess errors) PASS: gcc.dg/pr43643.c execution test PASS: gcc.dg/nest.c (test for excess errors) PASS: gcc.dg/nest.c execution test PASS: gcc.dg/20021014-1.c (test for excess errors) PASS: gcc.dg/20021014-1.c execution test PASS: gcc.dg/pr32450.c (test for excess errors) PASS: gcc.dg/pr32450.c execution test PASS: g++.dg/other/profile1.C -std=gnu++98 (test for excess errors) PASS: g++.dg/other/profile1.C -std=gnu++98 execution test PASS: g++.dg/other/profile1.C -std=gnu++11 (test for excess errors) PASS: g++.dg/other/profile1.C -std=gnu++11 execution test regards, Venkat. gcc.gprof.diff Description: Binary data
Re: [testsuite, android] Disabling thread_local4.C and thread_local4g.C for Android.
Hello, > OK. > Checked into MT: http://gcc.gnu.org/ml/gcc-cvs/2013-07/msg00731.html -- Thanks, K
Re: [Patch, Aarch64]: Handle return address via. frame pointer
On Sun, Jul 28, 2013 at 3:53 AM, Venkataramanan Kumar wrote: > Hi Maintainers, > > This patch adds supports to handle return address via. frame pointer. I noticed this patch causes undefined behavior when -fomit-frame-pointer and __builtin_return_address(1) is used. On PowerPC it is defined correctly that is it generates a frame for that case. Now on x86_64 it might be undefined but I think that is just wrong since __builtin_return_address is just used for debugging anyways. Thanks, Andrew > > gcc/ChangeLog > - > > 2013-07-28 Venkataramanan Kumar > >* config/aarch64/aarch64.c (aarch64_return_addr): Handle returning > address from a frame. > > > Regression tested with aarch64-none-elf with V8 foundation model. > > regards, > Venkat.
[ping**3] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3
On 07/20/2013 01:12 PM, Sandra Loosemore wrote:
On 07/09/2013 10:23 AM, Sandra Loosemore wrote:
On 06/30/2013 09:24 PM, Sandra Loosemore wrote:
Here is my third attempt at cleaning up -fstrict-volatile-bitfields.
Ping?
...and ping again.
...and again. Hmmm.
struct patch_status
{
volatile int approved:1;
volatile int rejected:1;
volatile int needs_changes:1;
int pinged;
};
extern struct patch_status s;
while (!s.approved && !s.rejected && !s.needs_changes)
{
sleep (a_week_or_two ());
pinged++;
}
Part 1 removes the warnings and packedp flag. It is the same as in the
last version, and has already been approved. I'll skip reposting it
since the patch is here already:
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00908.html
Part 2 replaces parts 2, 3, and 4 in the last version. I've re-worked
this code significantly to try to address Bernd Edlinger's comments on
the last version in PR56997.
Part 2: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg1.html
Part 3 is the test cases, which are the same as in the last version.
Nobody has reviewed these but I assume they are OK if Part 2 is
approved?
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00912.html
Part 4 is new; it makes -fstrict-volatile-bitfields not be the default
for any target any more. It is independent of the other changes.
Part 4: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg2.html
It seems that the change to the defaults in part 4 is still
controversial but my understanding based on discussion of the previous
version of the patches is that the maintainers were going to insist on
that as a condition of getting the other bug fixes in. From my
perspective, I'd be happy just to wrap up this patch series somehow or
another, so please let me know if there are additional changes I need to
make before this is suitable to check in.
Please note that I'm pinging my own 4-part patch series, not the Bernd's
followup patch confusingly posted in the same thread.
-Sandra
