Re: [PATCH] Fix up ix86_expand_set_or_movmem (PR target/61060)
> Hi!
>
> As discussed in the PR, ix86_expand_set_or_movmem assumes that count == 0
> means !CONST_INT_P (count_exp), but that is not necessarily true with
> -O0 -ftree-ter, when the middle-end doesn't immediately see the length
> argument be integer_zerop, yet TER results in it being expanded as
> const0_rtx.
>
> There is nothing to set or move in that case, so this patch makes the
> backend more tolerand just by returning true immediately.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK,
Honza
>
> 2014-05-12 Jakub Jelinek
>
> PR target/61060
> * config/i386/i386.c (ix86_expand_set_or_movmem): If count_exp
> is const0_rtx, return immediately. Don't test count == 0 when
> it is always true.
>
> * gcc.dg/pr61060.c: New test.
>
> --- gcc/config/i386/i386.c.jj 2014-05-11 22:21:28.0 +0200
> +++ gcc/config/i386/i386.c2014-05-12 10:59:18.940416162 +0200
> @@ -24156,8 +24156,13 @@ ix86_expand_set_or_movmem (rtx dst, rtx
> align = MEM_ALIGN (dst) / BITS_PER_UNIT;
>
>if (CONST_INT_P (count_exp))
> -min_size = max_size = probable_max_size = count = expected_size
> - = INTVAL (count_exp);
> +{
> + min_size = max_size = probable_max_size = count = expected_size
> + = INTVAL (count_exp);
> + /* When COUNT is 0, there is nothing to do. */
> + if (!count)
> + return true;
> +}
>else
> {
>if (min_size_exp)
> @@ -24166,7 +24171,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx
> max_size = INTVAL (max_size_exp);
>if (probable_max_size_exp)
> probable_max_size = INTVAL (probable_max_size_exp);
> - if (CONST_INT_P (expected_size_exp) && count == 0)
> + if (CONST_INT_P (expected_size_exp))
> expected_size = INTVAL (expected_size_exp);
> }
>
> --- gcc/testsuite/gcc.dg/pr61060.c.jj 2014-05-12 11:03:10.797222337 +0200
> +++ gcc/testsuite/gcc.dg/pr61060.c2014-05-12 11:03:02.882263281 +0200
> @@ -0,0 +1,19 @@
> +/* PR target/61060 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -ftree-ter" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +
> +extern inline __attribute__ ((gnu_inline, always_inline, artificial))
> +void *memset (void *dest, int ch, size_t len)
> +{
> + return __builtin_memset (dest, ch, len);
> +}
> +
> +char buf[10];
> +
> +void
> +foo (void)
> +{
> + memset (buf, sizeof (buf), 0);
> +}
>
> Jakub
Re: Ping2: [PATCH] PR debug/16063. Add DW_AT_type to DW_TAG_enumeration.
On Mon, May 12, 2014 at 11:22:11PM -0400, Jason Merrill wrote: > On 04/28/2014 08:37 AM, Mark Wielaard wrote: > >The debugger cares about the actual underlying type used if the language > >can use multiple. Either explicitly assigned by the user or implicitly > >as derived by the language/compile flags used. So the lang hook should > >provide one in both cases, if appropriate. > > Why do you want to do this for C? It seems to me that this is only > interesting in C++ because of overload resolution. So the debugger doesn't have to guess the properties of the enum's underlying base type, like size, encoding and signedness. Which are implementation defined and might differ between enums even in C depending on whether -fshort-enums was used (which is the default on some arches). Cheers, Mark
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Mon, May 12, 2014 at 03:44:42PM +, Joseph S. Myers wrote:
> On Sun, 11 May 2014, Marek Polacek wrote:
>
> > > FAIL: c-c++-common/pr50459.c -std=gnu++1y (test for excess errors)
> > > FAIL: c-c++-common/pr50459.c -Wc++-compat (test for excess errors)
> > >
> > > The errors are
> > >
> > > /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:8:1: error:
> > > constructor priorities are not supported
> > > /opt/gcc/work/gcc/testsuite/c-c++-common/pr50459.c:9:1: error: destructor
> > > priorities are not supported
> >
> > Ah. The following untested patch should skip that test on Darwin.
> >
> > Ok?
>
> I don't think the whole test should be skipped for that issue; I think the
> part requiring this feature should be split out into a separate testcase,
> so that as much as possible is still tested on Darwin.
Yeah, I should've done that in the first place, sorry. Is the
following ok then?
2014-05-13 Marek Polacek
* c-c++-common/pr50459.c: Move cdtor tests to a separate testcase.
* c-c++-common/pr50459-2.c: New test.
diff --git gcc/testsuite/c-c++-common/pr50459-2.c
gcc/testsuite/c-c++-common/pr50459-2.c
index e69de29..0e8fec3 100644
--- gcc/testsuite/c-c++-common/pr50459-2.c
+++ gcc/testsuite/c-c++-common/pr50459-2.c
@@ -0,0 +1,7 @@
+/* PR c/50459 */
+/* { dg-do compile { target init_priority } } */
+/* { dg-options "-Wall -Wextra" } */
+
+enum { A = 128, B = 1 };
+void fn3 (void) __attribute__((constructor (A)));
+void fn4 (void) __attribute__((destructor (A)));
diff --git gcc/testsuite/c-c++-common/pr50459.c
gcc/testsuite/c-c++-common/pr50459.c
index f837b63..8d75228 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -5,8 +5,6 @@
enum { A = 128, B = 1 };
void *fn1 (void) __attribute__((assume_aligned (A)));
void *fn2 (void) __attribute__((assume_aligned (A, 4)));
-void fn3 (void) __attribute__((constructor (A)));
-void fn4 (void) __attribute__((destructor (A)));
void *fn5 (int) __attribute__((alloc_size (B)));
void *fn6 (int) __attribute__((alloc_align (B)));
void fn7 (const char *, ...) __attribute__ ((sentinel (B)));
Marek
Re: [PATCH] Clean up shrink-wrapping codes
On Tue, May 13, 2014 at 03:14:34PM +0800, Zhenqiang Chen wrote: > Thanks. Committed the patch @r210351 with changes: > (1) Create shrink-wrap.h. > (2) Move all shrink-wrapping related interfaces from function.h to > shrink-wrap.h. > (3) shrink-wrap.h is included in function.c, shrink-wrap.c and > config/i386/i386.c. Could you please update the comment at the beginning of shrink-wrap.c? It certainly shouldn't say the same what function.c says. Marek
Re: libsanitizer merge from upstream r208536
I've committed this upstream and will include it into my next updated patch:
+#if defined(__x86_64__) && !defined(_LP64)
+ typedef long long __sanitizer_time_t;
+#else
+ typedef long __sanitizer_time_t;
+#endif
+
struct __sanitizer_timeb {
-long time;
+__sanitizer_time_t time;
unsigned short millitm;
If this is not enough, please contribute patches directly upstream --
I can not accept them from here.
Also, what's the story with x32 in LLVM?
Is there any chance you can set up a public build bot for x32 asan (upstream)?
On Mon, May 12, 2014 at 9:53 PM, H.J. Lu wrote:
> On Mon, May 12, 2014 at 4:20 AM, Konstantin Serebryany
> wrote:
>> This is the first libsanitizer merge in 4.10 (the last merge was in
>> December 2013).
>>
>> Tested on Ubuntu 12.04 like this:
>> rm -rf */{*/,}libsanitizer && make -j 50
>> make -j 40 -C gcc check-g{cc,++}
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp' && \
>> make -j 40 -C gcc check-g{cc,++}
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tsan.exp' && \
>> make -j 40 -C gcc check
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp' && \
>> echo PASS
>>
>> 5 months' worth of changes may break any platform we are not testing
>> ourselves
>> (that includes Ubuntu 12.04, 13.10, 14.04, Mac 10.9, Windows 7, Android ARM),
>> please help us test this patch on your favorite platform.
>>
>> Expected ChangeLog entries:
>> === gcc/testsuite/ChangeLog
>> 2014-05-XX Kostya Serebryany
>>
>> * c-c++-common/tsan/mutexset1.c: Update the test to match
>> upstream r208536.
>> * g++.dg/asan/symbolize-callback-1.C: Delete the deprecated test.
>>
>> === libsanitizer/ChangeLog
>> 2014-05-XX Kostya Serebryany
>>
>> * All source files: Merge from upstream r208536.
>> * asan/Makefile.am (asan_files): Added new files.
>> * asan/Makefile.in: Regenerate.
>> * tsan/Makefile.am (tsan_files): Added new files.
>> * tsan/Makefile.in: Regenerate.
>> * sanitizer_common/Makefile.am (sanitizer_common_files): Added
>> new files.
>> * sanitizer_common/Makefile.in: Regenerate.
>>
>> --kcc
>
> sanitizer_common/sanitizer_platform_limits_posix.h has
>
> struct __sanitizer_timeb {
> long time;
> unsigned short millitm;
> short timezone;
> short dstflag;
> };
>
> On Linux, timeb is
>
> struct timeb
> {
> time_t time; /* Seconds since epoch, as from `time'. */
> unsigned short int millitm; /* Additional milliseconds. */
> short int timezone; /* Minutes west of GMT. */
> short int dstflag; /* Nonzero if Daylight Savings Time used. */
> };
>
> For x32, long is 32-bit and time_t is 64-bit. We need something
> like
>
> struct __sanitizer_timeb {
> #if defined(__x86_64__) && !defined(_LP64)
> long long time;
> #else
> long time;
> #endif
>
> similar to other places.
>
>
> --
> H.J.
Re: libsanitizer merge from upstream r208536
> On May 13, 2014, at 12:59 AM, Konstantin Serebryany
> wrote:
>
> I've committed this upstream and will include it into my next updated patch:
>
> +#if defined(__x86_64__) && !defined(_LP64)
> + typedef long long __sanitizer_time_t;
> +#else
> + typedef long __sanitizer_time_t;
> +#endif
> +
> struct __sanitizer_timeb {
> -long time;
> +__sanitizer_time_t time;
> unsigned short millitm;
This will also be needed for ilp32 aarch64 too.
>
>
> If this is not enough, please contribute patches directly upstream --
> I can not accept them from here.
I am still against the way Libsanitizer is being maintained. I really think the
maintainers of the source in gcc should submit the patches upstream and not
burden the target maintainers with it. Or maybe having libsantizer as another
project separate from gcc and LLVM would be even better.
Thanks,
Andrew
>
> Also, what's the story with x32 in LLVM?
> Is there any chance you can set up a public build bot for x32 asan (upstream)?
>
>> On Mon, May 12, 2014 at 9:53 PM, H.J. Lu wrote:
>> On Mon, May 12, 2014 at 4:20 AM, Konstantin Serebryany
>> wrote:
>>> This is the first libsanitizer merge in 4.10 (the last merge was in
>>> December 2013).
>>>
>>> Tested on Ubuntu 12.04 like this:
>>> rm -rf */{*/,}libsanitizer && make -j 50
>>> make -j 40 -C gcc check-g{cc,++}
>>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp' && \
>>> make -j 40 -C gcc check-g{cc,++}
>>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tsan.exp' && \
>>> make -j 40 -C gcc check
>>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp' && \
>>> echo PASS
>>>
>>> 5 months' worth of changes may break any platform we are not testing
>>> ourselves
>>> (that includes Ubuntu 12.04, 13.10, 14.04, Mac 10.9, Windows 7, Android
>>> ARM),
>>> please help us test this patch on your favorite platform.
>>>
>>> Expected ChangeLog entries:
>>> === gcc/testsuite/ChangeLog
>>> 2014-05-XX Kostya Serebryany
>>>
>>>* c-c++-common/tsan/mutexset1.c: Update the test to match
>>>upstream r208536.
>>>* g++.dg/asan/symbolize-callback-1.C: Delete the deprecated test.
>>>
>>> === libsanitizer/ChangeLog
>>> 2014-05-XX Kostya Serebryany
>>>
>>>* All source files: Merge from upstream r208536.
>>>* asan/Makefile.am (asan_files): Added new files.
>>>* asan/Makefile.in: Regenerate.
>>>* tsan/Makefile.am (tsan_files): Added new files.
>>>* tsan/Makefile.in: Regenerate.
>>>* sanitizer_common/Makefile.am (sanitizer_common_files): Added
>>> new files.
>>>* sanitizer_common/Makefile.in: Regenerate.
>>>
>>> --kcc
>>
>> sanitizer_common/sanitizer_platform_limits_posix.h has
>>
>> struct __sanitizer_timeb {
>>long time;
>>unsigned short millitm;
>>short timezone;
>>short dstflag;
>> };
>>
>> On Linux, timeb is
>>
>> struct timeb
>> {
>>time_t time; /* Seconds since epoch, as from `time'. */
>>unsigned short int millitm; /* Additional milliseconds. */
>>short int timezone; /* Minutes west of GMT. */
>>short int dstflag; /* Nonzero if Daylight Savings Time used. */
>> };
>>
>> For x32, long is 32-bit and time_t is 64-bit. We need something
>> like
>>
>> struct __sanitizer_timeb {
>> #if defined(__x86_64__) && !defined(_LP64)
>>long long time;
>> #else
>>long time;
>> #endif
>>
>> similar to other places.
>>
>>
>> --
>> H.J.
Re: [PATCH] Clean up shrink-wrapping codes
On 13 May 2014 15:55, Marek Polacek wrote: > On Tue, May 13, 2014 at 03:14:34PM +0800, Zhenqiang Chen wrote: >> Thanks. Committed the patch @r210351 with changes: >> (1) Create shrink-wrap.h. >> (2) Move all shrink-wrapping related interfaces from function.h to >> shrink-wrap.h. >> (3) shrink-wrap.h is included in function.c, shrink-wrap.c and >> config/i386/i386.c. > > Could you please update the comment at the beginning of shrink-wrap.c? > It certainly shouldn't say the same what function.c says. There are two /*..*/ blocks at the beginning. As I understand the first /*...*/ is the copyright/license, which should be same for all files. And I had changed the second /*...*/ as /* This file handles shrink-wrapping related optimizations. */ Which comment do you mean? Thanks! -Zhenqiang
Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)
On 12 May 2014 23:39, Oleg Endo wrote: > This is the same as changing/setting the FP modes (PR, SZ) on SH while > preserving the other FPSCR bits, or did I miss something? It's more like if you have to control multiple bits at once to get a specific mode. Say you have to turn SZ off and PR on. You you knew that only one bit needs changing, you can do with one less arithmetic operation.
Re: [PATCH] Clean up shrink-wrapping codes
On Tue, May 13, 2014 at 04:08:21PM +0800, Zhenqiang Chen wrote: > On 13 May 2014 15:55, Marek Polacek wrote: > > On Tue, May 13, 2014 at 03:14:34PM +0800, Zhenqiang Chen wrote: > >> Thanks. Committed the patch @r210351 with changes: > >> (1) Create shrink-wrap.h. > >> (2) Move all shrink-wrapping related interfaces from function.h to > >> shrink-wrap.h. > >> (3) shrink-wrap.h is included in function.c, shrink-wrap.c and > >> config/i386/i386.c. > > > > Could you please update the comment at the beginning of shrink-wrap.c? > > It certainly shouldn't say the same what function.c says. > > There are two /*..*/ blocks at the beginning. > > As I understand the first /*...*/ is the copyright/license, which > should be same for all files. > > And I had changed the second /*...*/ as > > /* This file handles shrink-wrapping related optimizations. */ > > Which comment do you mean? I meant the very first line that says "Expands front end tree to back end RTL for GCC.". I guess best would be just to replace it with the "This file handles shrink-wrapping related optimizations." line. Marek
Re: libsanitizer merge from upstream r208536
On Mon, May 12, 2014 at 03:20:37PM +0400, Konstantin Serebryany wrote: > This is the first libsanitizer merge in 4.10 (the last merge was in > December 2013). Worked on x86_64-linux and i686-linux (Fedora 20 and 17), bootstrap/regtest showed no regressions. Jakub
Re: libsanitizer merge from upstream r208536
On Tue, May 13, 2014 at 12:05 PM, wrote:
>
>
>> On May 13, 2014, at 12:59 AM, Konstantin Serebryany
>> wrote:
>>
>> I've committed this upstream and will include it into my next updated patch:
>>
>> +#if defined(__x86_64__) && !defined(_LP64)
>> + typedef long long __sanitizer_time_t;
>> +#else
>> + typedef long __sanitizer_time_t;
>> +#endif
>> +
>> struct __sanitizer_timeb {
>> -long time;
>> +__sanitizer_time_t time;
>> unsigned short millitm;
>
>
> This will also be needed for ilp32 aarch64 too.
>>
>>
>> If this is not enough, please contribute patches directly upstream --
>> I can not accept them from here.
>
>
> I am still against the way Libsanitizer is being maintained.
I do not enjoy it either. Suggestions are welcome, and see below!
> I really think the maintainers of the source in gcc should submit the patches
> upstream and not burden the target maintainers with it.
This is impossible, sorry.
We do not know how to fix a platform we do not have access to
ourselves and we can not accept patches for upstream
other than by following the standard process because this will violate
the LLVM developer policy.
> Or maybe having libsantizer as another project separate from gcc and LLVM
> would be even better.
libsanitizer is essentially a separate project.
It lives in a separate svn tree (compiler-rt).
We've recently made a set of changes to allow the libsanitizer tests
to be run with an arbitrary compiler
(i.e. this could be GCC, or some other build of Clang, or any other
compiler that implements appropriate instrumentation).
Ideally for me, GCC would pull the sanitizer sources as "svn external"
and the merge will be simply a change of revision.
--kcc
>
> Thanks,
> Andrew
>
>
>>
>> Also, what's the story with x32 in LLVM?
>> Is there any chance you can set up a public build bot for x32 asan
>> (upstream)?
>>
>>> On Mon, May 12, 2014 at 9:53 PM, H.J. Lu wrote:
>>> On Mon, May 12, 2014 at 4:20 AM, Konstantin Serebryany
>>> wrote:
This is the first libsanitizer merge in 4.10 (the last merge was in
December 2013).
Tested on Ubuntu 12.04 like this:
rm -rf */{*/,}libsanitizer && make -j 50
make -j 40 -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp' && \
make -j 40 -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tsan.exp' && \
make -j 40 -C gcc check
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp' && \
echo PASS
5 months' worth of changes may break any platform we are not testing
ourselves
(that includes Ubuntu 12.04, 13.10, 14.04, Mac 10.9, Windows 7, Android
ARM),
please help us test this patch on your favorite platform.
Expected ChangeLog entries:
=== gcc/testsuite/ChangeLog
2014-05-XX Kostya Serebryany
* c-c++-common/tsan/mutexset1.c: Update the test to match
upstream r208536.
* g++.dg/asan/symbolize-callback-1.C: Delete the deprecated test.
=== libsanitizer/ChangeLog
2014-05-XX Kostya Serebryany
* All source files: Merge from upstream r208536.
* asan/Makefile.am (asan_files): Added new files.
* asan/Makefile.in: Regenerate.
* tsan/Makefile.am (tsan_files): Added new files.
* tsan/Makefile.in: Regenerate.
* sanitizer_common/Makefile.am (sanitizer_common_files): Added
new files.
* sanitizer_common/Makefile.in: Regenerate.
--kcc
>>>
>>> sanitizer_common/sanitizer_platform_limits_posix.h has
>>>
>>> struct __sanitizer_timeb {
>>>long time;
>>>unsigned short millitm;
>>>short timezone;
>>>short dstflag;
>>> };
>>>
>>> On Linux, timeb is
>>>
>>> struct timeb
>>> {
>>>time_t time; /* Seconds since epoch, as from `time'. */
>>>unsigned short int millitm; /* Additional milliseconds. */
>>>short int timezone; /* Minutes west of GMT. */
>>>short int dstflag; /* Nonzero if Daylight Savings Time used. */
>>> };
>>>
>>> For x32, long is 32-bit and time_t is 64-bit. We need something
>>> like
>>>
>>> struct __sanitizer_timeb {
>>> #if defined(__x86_64__) && !defined(_LP64)
>>>long long time;
>>> #else
>>>long time;
>>> #endif
>>>
>>> similar to other places.
>>>
>>>
>>> --
>>> H.J.
Re: libsanitizer merge from upstream r208536
pins...@gmail.com writes: >> If this is not enough, please contribute patches directly upstream -- >> I can not accept them from here. > > > I am still against the way Libsanitizer is being maintained. I really think > the maintainers of the source in gcc should submit the patches upstream and > not burden the target maintainers with it. Or maybe having libsantizer as Agreed: this works just fine for Go/libgo (and used to work similarly for the copy of classpath in libjava, where the Java maintainers handled this). If I contribute patches on the GCC side, Ian takes care of getting them in upstream. I do need a Google contributor agreement, but that's ok. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH, PR52252] Vectorization for load/store groups of size 3.
On Mon, 12 May 2014, Evgeny Stupachenko wrote:
> The test is on general changes. However I was able to test it on x86 only.
> I see 2 possible solutions:
> 1. Set the test for x86 only.
> 2. Modify it so that it will pass on sparc-sun-solaris2.
>
> If 2. is not acceptable I'll create patch for 1.
> Currently I don't see why "in0_9 = *in_27" is not supported. Does the
> test fail because of unsupported permutation?
The test uses
/* { dg-options "-O2 -g -ftree-vectorize -mssse3 -fdump-tree-vect-details"
{ target { i?86-*-* x86_64-*-* } } } */
that's bogus. You shouldn't add any dg-options. Instead use proper
dg-effective-target checks for the mssse3 feature you are using.
Note that the dg-final checking is applied regardless of the options
above are applied or not.
Why does the test only succeed with -mssse3 btw?
The proper way to restrict the test to a single target is to use
/* { dg-skip-if "why" { ! { x86_64-*-* i?86-*-* } } } */
Sorry for not catching this in the review.
Richard.
>
> On Mon, May 12, 2014 at 7:14 PM, Rainer Orth
> wrote:
> > Evgeny Stupachenko writes:
> >
> >> Patch with fixes attached.
> >> Currently if-structure is as following:
> >> + if (count == 3)
> >> ...
> >> + else
> >> + {
> >> + /* If length is not equal to 3 then only power of 2 is
> >> supported. */
> >> + gcc_assert (exact_log2 (count) != -1);
> >>
> >> For stores group I've created another mail thread.
> > [...]
> 2014-05-06 Evgeny Stupachenko
>
> PR tree-optimization/52252
> * gcc.dg/vect/pr52252-ld.c: Test on loads group of size 3.
> >
> > This test FAILs on sparc-sun-solaris2.11, both 32 and 64-bit:
> >
> > FAIL: gcc.dg/vect/pr52252-ld.c scan-tree-dump-times vect "vectorized 1
> > loops" 1
> > FAIL: gcc.dg/vect/pr52252-ld.c -flto -ffat-lto-objects
> > scan-tree-dump-times vect "vectorized 1 loops" 1
> >
> > The dumps have
> >
> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3:
> > note: not vectorized: relevant stmt not supported: in0_9 = *in_27;
> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/vect/pr52252-ld.c:7:1:
> > note: vectorized 0 loops in function.
> >
> > Rainer
> >
> > --
> > -
> > Rainer Orth, Center for Biotechnology, Bielefeld University
>
>
--
Richard Biener
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
Re: [PATCH] Clean up shrink-wrapping codes
On 13 May 2014 16:13, Marek Polacek wrote: > On Tue, May 13, 2014 at 04:08:21PM +0800, Zhenqiang Chen wrote: >> On 13 May 2014 15:55, Marek Polacek wrote: >> > On Tue, May 13, 2014 at 03:14:34PM +0800, Zhenqiang Chen wrote: >> >> Thanks. Committed the patch @r210351 with changes: >> >> (1) Create shrink-wrap.h. >> >> (2) Move all shrink-wrapping related interfaces from function.h to >> >> shrink-wrap.h. >> >> (3) shrink-wrap.h is included in function.c, shrink-wrap.c and >> >> config/i386/i386.c. >> > >> > Could you please update the comment at the beginning of shrink-wrap.c? >> > It certainly shouldn't say the same what function.c says. >> >> There are two /*..*/ blocks at the beginning. >> >> As I understand the first /*...*/ is the copyright/license, which >> should be same for all files. >> >> And I had changed the second /*...*/ as >> >> /* This file handles shrink-wrapping related optimizations. */ >> >> Which comment do you mean? > > I meant the very first line that says "Expands front end tree to back > end RTL for GCC.". I guess best would be just to replace it with the > "This file handles shrink-wrapping related optimizations." line. I see. Thank you very much! I have not noticed the line before Copyright is a description, not part of the Copyright. I will update it in later patch. Thanks! -Zhenqiang
Re: libsanitizer merge from upstream r208536
Konstantin Serebryany writes: >> I am still against the way Libsanitizer is being maintained. > > I do not enjoy it either. Suggestions are welcome, and see below! > >> I really think the maintainers of the source in gcc should submit the >> patches upstream and not burden the target maintainers with it. > > This is impossible, sorry. > We do not know how to fix a platform we do not have access to > ourselves and we can not accept patches for upstream Nobody expects you to: if things break for Solaris in Go/libgo by a merge from upstream, I try to get things fixed in the gcc tree and the changes are pushed back later (or rather: first integrated upstream and than, often simulaneously, merged back into gcc). > other than by following the standard process because this will violate > the LLVM developer policy. Which says what? Your current policy seems to massively impede contributions. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH, PR52252] Vectorization for load/store groups of size 3.
Evgeny Stupachenko writes: > The test is on general changes. However I was able to test it on x86 only. > I see 2 possible solutions: > 1. Set the test for x86 only. > 2. Modify it so that it will pass on sparc-sun-solaris2. > > If 2. is not acceptable I'll create patch for 1. > Currently I don't see why "in0_9 = *in_27" is not supported. Does the > test fail because of unsupported permutation? The .vect dump has gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: ==> examining statement: in0_9 = *in_27; gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: vect_is_simple_use: operand *in_27 gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: not ssa-name. gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: use not simple. gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: vect_is_simple_use: operand *in_27 gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: not ssa-name. gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: use not simple. gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: no array mode for V8QI[3] gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: shuffle of 3 loads is not supported by target gcc/testsuite/gcc.dg/vect/pr52252-ld.c:10:3: note: not vectorized: relevant stmt not supported: in0_9 = *in_27; I can send you the full dump if necessary. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [RFC] Using function clones for Pointer Bounds Checker
2014-05-13 1:20 GMT+04:00 Jeff Law : > On 01/14/14 02:15, Ilya Enkovich wrote: >> >> Hi, >> >> I've been working for some time on the prototype of the Pointer Bounds >> Checker which uses function clones for instrumentation >> (http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03327.html). After >> several experiments with this approach I want to share my results and >> ask for some feedback to make a decision about the future steps. > > Sorry for the delayed reply, when you sent this message I was ignoring > anything that wasn't an open issue for GCC 4.9 -- it all gets thrown into a > massive queue of things to come back to once stage1 reopens. > > So I really like the idea of trying to use function clones to simplify some > of the details of implementing MPX support. > > >> >> Clones approach does not use special built-in function calls to >> associate bounds with call arguments and input parameters. Each >> function which should be instrumented gets an additional version and >> only this special version will be instrumented. > > I like this. > > > > >> This new version gets >> additional bound arguments to express input bounds. When function call >> is instrumented, it is redirected to instrumented version and all >> bounds are passed as explicit call arguments. Thus we have explicit >> pointer bounds flow similar to regular function parameters. It should >> allow to avoid changes in optimization, avoid miss-optimizations, >> allow existing IPA optimizations to work with bound args (e.g. >> propagate constant bounds value and remove checks in called function). > > So from a linking standpoint, presumably you have to mangle the instrumented > caller/callee in some manner. Right? Or are you dynamically dispatching > somehow? Originally the idea was o have instrumented clone to have the same assembler name as the original function. Since instrumented code is fully compatible with not instrumented code, we always emit only one version. Usage of the same assembler name allows instrumented and not instrumented calls to look similar in assembler. It worked fine until I tried it with LTO where assembler name is used as a unique identifier. With linker resolutions files it became even more harder to use such approach. To resolve these issues I started to use new assembler name with postfix, but linked with the original name using IDENTIFIER_TRANSPARENT_ALIAS. It gives different assembler names for clones and originals during compilation, but both clone and original functions have similar name in output assembler. > > > >> >> 1. Nodes reachability. Instrumented version is actually always >> reachable when original function is reachable because it is always >> emitted instead of the original. Thus I had to fix reachability >> analysis to achieve it. Another similar problem is check whether node >> can be removed after inline when inlining instrumented function. Not >> hard to fix but probably other similar problems exist. > > Jan is going to be the best person to discuss these issues with you. He's > the architect behind most of the IPA bits we've got right now. For now I managed to handle this via introducing a new type of thunk for original functions. If we have instrumented function it means we do not emit original one and therefore may remove its body. At this point I transform the original function into a new kind of thunk which has call edge to the instrumented version. It makes reachability analysis to work OK for instrumented functions with no modifications. The only problem here is to avoid function clones removal before instrumentation pass happens. I achieve it by forbidding to remove not instrumented instrumentation clones by adding new case into cgraph_can_remove_if_no_direct_calls_and_refs_p. > > >>> >> In general I feel that having special function version for >> instrumentation has a better potential, should lead to less intrusive >> changes in the compiler and better code quality. > > Agreed. It seems *much* cleaner. > > jeff Thanks for your review. Ilya
Re: [PATCH] Add missing -fdump-* options
On Fri, May 9, 2014 at 5:54 PM, Teresa Johnson wrote:
> I discovered that the support for the documented -fdump-* options
> "optimized", "missed", "note" and "optall" was missing. Added that and
> fixed a minor typo in the documentation.
>
> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk?
I'm not sure they were intented for user-consumption. ISTR they
are just an implementation detail exposed by -fopt-info-X (which is
where they are documented).
The typo fix is ok, also adding a comment before the dump flags
definition to the above fact.
David, do I remember correctly?
Thanks,
Richard.
> Thanks,
> Teresa
>
> 2014-05-09 Teresa Johnson
>
> * doc/invoke.texi: Fix typo.
> * dumpfile.c: Add support for documented -fdump-* options
> optimized/missed/note/optall.
>
> Index: doc/invoke.texi
> ===
> --- doc/invoke.texi (revision 210157)
> +++ doc/invoke.texi (working copy)
> @@ -6278,7 +6278,7 @@ passes).
> @item missed
> Enable showing missed optimization information (only available in certain
> passes).
> -@item notes
> +@item note
> Enable other detailed optimization information (only available in
> certain passes).
> @item =@var{filename}
> Index: dumpfile.c
> ===
> --- dumpfile.c (revision 210157)
> +++ dumpfile.c (working copy)
> @@ -107,6 +107,10 @@ static const struct dump_option_value_info dump_op
>{"nouid", TDF_NOUID},
>{"enumerate_locals", TDF_ENUMERATE_LOCALS},
>{"scev", TDF_SCEV},
> + {"optimized", MSG_OPTIMIZED_LOCATIONS},
> + {"missed", MSG_MISSED_OPTIMIZATION},
> + {"note", MSG_NOTE},
> + {"optall", MSG_ALL},
>{"all", ~(TDF_RAW | TDF_SLIM | TDF_LINENO | TDF_TREE | TDF_RTL | TDF_IPA
> | TDF_STMTADDR | TDF_GRAPH | TDF_DIAGNOSTIC | TDF_VERBOSE
> | TDF_RHS_ONLY | TDF_NOUID | TDF_ENUMERATE_LOCALS | TDF_SCEV)},
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [PATCH, PR58066] preferred_stack_boundary update for tls expanded call
Wei Mi writes: > Thanks for trying the testcase. rtl scanning will be slightly better > than assembly scanning. So how about this one? This one works fine for me. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libsanitizer merge from upstream r208536
>> other than by following the standard process because this will violate >> the LLVM developer policy. > > Which says what? http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch "Once your patch is ready, submit it by emailing it to the appropriate project’s commit mailing list (or commit it directly if applicable). Alternatively, some patches get sent to the project’s development list or component of the LLVM bug tracker, but the commit list is the primary place for reviews and should generally be preferred." > Your current policy seems to massively impede contributions. How? (BTW, I am not the one who sets these policies and discussing them with me makes little sense) Or do you refer to our own (non-LLVM) requirement to use arc for non-trivial libsanitizer patches? The tool requires 3 minutes to set up for a patch author and saves us (reviewers) much more time, so we will continue to require it. --kcc
Re: [PATCH, AArch64] Implement HARD_REGNO_CALLER_SAVE_MODE
On 12/05/14 17:30, Ian Bolton wrote:
> Currently, on AArch64, when a caller-save register is saved/restored,
> GCC is accessing the maximum size of the hard register.
>
> So an SImode integer (4 bytes) value is being stored as DImode (8 bytes)
> because the int registers are 8 bytes wide, and an SFmode float (4 bytes)
> and DFmode double (8 bytes) are being stored as TImode (16 bytes) to
> capture the full 128-bits of the vector register.
>
> This patch corrects this, by implementing the HARD_REGNO_CALLER_SAVE_MODE
> hook, which is called by LRA to determine the minimise size it might need
> to save/restore.
>
> Tested on GCC regression suite and verified impact on a number of examples.
>
> OK for trunk?
>
> Cheers,
> Ian
>
>
> 2014-05-12 Ian Bolton
>
> * config/aarch64/aarch64-protos.h
> (aarch64_hard_regno_caller_save_mode): New prototype.
> * config/aarch64/aarch64.c (aarch64_hard_regno_caller_save_mode):
> New function.
> * config/aarch64/aarch64.h (HARD_REGNO_CALLER_SAVE_MODE): New macro.
>
This is ok.
Watch your changelog entry though. Lines should be wrapped before 80
columns (and continuations indented accordingly).
R.
>
> aarch64-caller-save-fix-patch-v2.txt
>
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index 04cbc78..7cf7d9f 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -202,6 +202,8 @@ enum aarch64_symbol_type aarch64_classify_symbol (rtx,
> enum aarch64_symbol_type aarch64_classify_tls_symbol (rtx);
> enum reg_class aarch64_regno_regclass (unsigned);
> int aarch64_asm_preferred_eh_data_format (int, int);
> +enum machine_mode aarch64_hard_regno_caller_save_mode (unsigned, unsigned,
> +enum machine_mode);
> int aarch64_hard_regno_mode_ok (unsigned, enum machine_mode);
> int aarch64_hard_regno_nregs (unsigned, enum machine_mode);
> int aarch64_simd_attr_length_move (rtx);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 8655f04..c2cc81b 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -424,6 +424,24 @@ aarch64_hard_regno_mode_ok (unsigned regno, enum
> machine_mode mode)
>return 0;
> }
>
> +/* Implement HARD_REGNO_CALLER_SAVE_MODE. */
> +enum machine_mode
> +aarch64_hard_regno_caller_save_mode (unsigned regno, unsigned nregs,
> + enum machine_mode mode)
> +{
> + /* Handle modes that fit within single registers. */
> + if (nregs == 1 && GET_MODE_SIZE (mode) <= 16)
> +{
> + if (GET_MODE_SIZE (mode) >= 4)
> +return mode;
> + else
> +return SImode;
> +}
> + /* Fall back to generic for multi-reg and very large modes. */
> + else
> +return choose_hard_reg_mode (regno, nregs, false);
> +}
> +
> /* Return true if calls to DECL should be treated as
> long-calls (ie called via a register). */
> static bool
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index c9b30d0..0574593 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -824,6 +824,11 @@ do {
> \
>
> #define SHIFT_COUNT_TRUNCATED !TARGET_SIMD
>
> +/* Choose appropriate mode for caller saves, so we do the minimum
> + required size of load/store. */
> +#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
> + aarch64_hard_regno_caller_save_mode ((REGNO), (NREGS), (MODE))
> +
> /* Callee only saves lower 64-bits of a 128-bit register. Tell the
> compiler the callee clobbers the top 64-bits when restoring the
> bottom 64-bits. */
>
Re: [GSoC] use obstack in parse_c_expr
On Fri, May 9, 2014 at 10:30 PM, Jeff Law wrote: > On 04/25/14 05:39, Prathamesh Kulkarni wrote: >> >> On Thu, Apr 24, 2014 at 9:18 PM, Diego Novillo >> wrote: >>> >>> Please remember to send patches to gcc-patches. >>> >>> I'm wondering if you couldn't just use std::string here. No need to >>> change >>> one complicated allocation scheme with another. >> >> Since we were using c-styled strings throughout genmatch, I thought of >> using obstack instead of std::string. >> This patch uses std::string in parse_c_expr to build c-code string. >> Should we also change AST (operand and subclasses) to use std::string >> instead of c-styled strings ? > > Yes, let's go with this. Btw, how is the status with the copyright assignments? Can we give the GSOC students svn write access so they can commit to the branches? Or should we direct them to use git branches off the github.com git mirror? Thanks, Richard. > Jeff
Re: [PATCH] Fix omp declare simd cloning (PR tree-optimization/60823)
Jakub Jelinek writes: > 2014-04-18 Jakub Jelinek > > PR tree-optimization/60823 > * omp-low.c (ipa_simd_modify_function_body): Go through > all SSA_NAMEs and for those refering to vector arguments > which are going to be replaced adjust SSA_NAME_VAR and, > if it is a default definition, change it into a non-default > definition assigned at the beginning of function from new_decl. > (ipa_simd_modify_stmt_ops): Rewritten. > * tree-dfa.c (set_ssa_default_def): When removing default def, > check for NULL loc instead of NULL *loc. > > * c-c++-common/gomp/pr60823-1.c: New test. > * c-c++-common/gomp/pr60823-2.c: New test. > * c-c++-common/gomp/pr60823-3.c: New test. The second test FAILs on Solaris/x86 with Sun as: ld.so.1: pr60823-2.exe: fatal: pr60823-2.exe: hardware capability (CA_SUNW_HW_1) unsupported: 0x2000 [ AVX ] FAIL: c-c++-common/gomp/pr60823-2.c execution test If this is expected, I can extend the code in gcc.target/i386/i386.exp (or rather move it to a new lib/clearcap.exp) to handle that via linker maps. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: RFA: Fix type incompatibility in get_addr_base_and_unit_offset_1
On Sat, May 10, 2014 at 9:19 PM, Richard Sandiford
wrote:
> get_addr_base_and_unit_offset_1 and get_ref_base_and_extent have similar
> code to handle array indices. The code in tree-dfa.c extends from the
> precision of the index, as before wide-int, but the tree-dfa.h version
> was updated in a way that could introduce a type inconsistency (flagged
> by a patch I'm working on). This version makes them the same again,
> as per the patch for PR59356.
>
> Tested on x86_64-limux-gnu. OK to install?
Ok.
Thanks,
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * tree-dfa.h (get_addr_base_and_unit_offset_1): Update array index
> calculation to match get_ref_base_and_extent.
>
> Index: gcc/tree-dfa.h
> ===
> --- gcc/tree-dfa.h 2014-05-10 14:02:40.112187344 +0100
> +++ gcc/tree-dfa.h 2014-05-10 14:02:50.414275258 +0100
> @@ -103,7 +103,8 @@ get_addr_base_and_unit_offset_1 (tree ex
> TREE_CODE (unit_size) == INTEGER_CST))
> {
> offset_int woffset
> - = offset_int::from (wi::sub (index, low_bound), SIGNED);
> + = wi::sext (wi::to_offset (index) - wi::to_offset
> (low_bound),
> + TYPE_PRECISION (TREE_TYPE (index)));
> woffset *= wi::to_offset (unit_size);
> byte_offset += woffset.to_shwi ();
> }
Re: libsanitizer merge from upstream r208536
Konstantin Serebryany writes: >>> other than by following the standard process because this will violate >>> the LLVM developer policy. >> >> Which says what? > > http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch > > "Once your patch is ready, submit it by emailing it to the appropriate > project’s commit mailing list (or commit it directly if applicable). > Alternatively, some patches get sent to the project’s development list > or component of the LLVM bug tracker, but the commit list is the > primary place for reviews and should generally be preferred." > >> Your current policy seems to massively impede contributions. > How? (BTW, I am not the one who sets these policies and discussing > them with me makes little sense) Requiring target maintainers to interact with several different upstream projects to keep their port in gcc in shape just doesn't scale. There's no such requirement for boehm-gc, libffi, libgo, classpath, but instead people directly involved in those projects (or the upstream maintainers themselves) act as liaisons between gcc and their project, removing that burden. Just libsanitizer is the odd man out here. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libsanitizer merge from upstream r208536
Your current policy seems to massively impede contributions. How? (BTW, I am not the one who sets these policies and discussing them with me makes little sense) I think people are mainly worried with checking out Clang, making it to build (install Cmake, C++11-friendly compiler, etc.), then making testsuites to run (install Python, etc.) on their particular platform, etc. (BTW I totally agree that forcing all patches to go through compiler-rt greatly simplifies maintenance of libsanitizer.) -Y
Re: [Ada] RFA: Add some "missing" integer_one_node conversions
On Sat, May 10, 2014 at 9:25 PM, Richard Sandiford wrote: > While testing another patch, I hit cases where Ada was building or > folding additions involving integer_one_node without converting > it to the type of the other operand. This looked unintentional, > since all other uses of integer_one_node seemed to have the conversion. > > Tested on x86_64-linux-gnu. OK to install? Hum. Unless convert () is required for some odd reason I'd prefer to convert these to build_int_cst (gnu_string_index_type, 1) and the like. Or finally add an int_const_binop overload with a HWI 2nd operand. Of course in the end it's Ada maintainers call. Richard. > Thanks, > Richard > > > gcc/ada/ > * gcc-interface/decl.c (gnat_to_gnu_entity): Convert > integer_one_node to the appropriate type. > * gcc-interface/trans.c (gnat_to_gnu): Likewise. > (pos_to_constructor): Likewise. > > Index: gcc/ada/gcc-interface/decl.c > === > --- gcc/ada/gcc-interface/decl.c2014-05-10 19:45:34.404009508 +0100 > +++ gcc/ada/gcc-interface/decl.c2014-05-10 20:21:24.169587544 +0100 > @@ -2840,7 +2840,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entit > = build_binary_op (PLUS_EXPR, gnu_string_index_type, > gnu_lower_bound, > int_const_binop (MINUS_EXPR, gnu_length, > - integer_one_node)); > + convert (gnu_string_index_type, > + integer_one_node))); > tree gnu_index_type > = create_index_type (convert (sizetype, gnu_lower_bound), >convert (sizetype, gnu_upper_bound), > Index: gcc/ada/gcc-interface/trans.c > === > --- gcc/ada/gcc-interface/trans.c 2014-05-10 19:45:34.404009508 +0100 > +++ gcc/ada/gcc-interface/trans.c 2014-05-10 20:21:24.160587477 +0100 > @@ -5597,6 +5597,7 @@ gnat_to_gnu (Node_Id gnat_node) > int length = String_Length (gnat_string); > int i; > tree gnu_idx = TYPE_MIN_VALUE (TYPE_DOMAIN (gnu_result_type)); > + tree gnu_one_node = convert (TREE_TYPE (gnu_idx), integer_one_node); > vec *gnu_vec; > vec_alloc (gnu_vec, length); > > @@ -5606,7 +5607,7 @@ gnat_to_gnu (Node_Id gnat_node) > Get_String_Char (gnat_string, i + 1)); > > CONSTRUCTOR_APPEND_ELT (gnu_vec, gnu_idx, t); > - gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, > integer_one_node); > + gnu_idx = int_const_binop (PLUS_EXPR, gnu_idx, gnu_one_node); > } > > gnu_result = gnat_build_constructor (gnu_result_type, gnu_vec); > @@ -9092,7 +9093,9 @@ pos_to_constructor (Node_Id gnat_expr, t >CONSTRUCTOR_APPEND_ELT (gnu_expr_vec, gnu_index, > convert (TREE_TYPE (gnu_array_type), gnu_expr)); > > - gnu_index = int_const_binop (PLUS_EXPR, gnu_index, integer_one_node); > + gnu_index = int_const_binop (PLUS_EXPR, gnu_index, > + convert (TREE_TYPE (gnu_index), > + integer_one_node)); > } > >return gnat_build_constructor (gnu_array_type, gnu_expr_vec);
Re: [PATCH GCC]Pick up more address lowering cases for ivopt and tree-affine.c
On Sun, May 11, 2014 at 2:49 PM, Bin.Cheng wrote:
> On Thu, May 8, 2014 at 5:08 PM, Bin.Cheng wrote:
>> On Tue, May 6, 2014 at 6:44 PM, Richard Biener
>> wrote:
>>> On Tue, May 6, 2014 at 10:39 AM, Bin.Cheng wrote:
On Fri, Dec 6, 2013 at 6:19 PM, Richard Biener
wrote:
>>
Hi,
I split the patch into two and updated the test case.
The patches pass bootstrap/tests on x86/x86_64, also pass test on arm
cortex-m3.
Is it OK?
Thanks,
bin
PATCH 1:
2014-05-06 Bin Cheng
* gcc/tree-affine.c (tree_to_aff_combination): Handle MEM_REF for
core part of address expressions.
>>>
>>> No gcc/ in the changelog
>>>
>>> Simplify that by using aff_combination_add_cst:
>>>
>>> + if (TREE_CODE (core) == MEM_REF)
>>> + {
>>> + aff_combination_add_cst (comb, mem_ref_offset (core));
>>> + core = TREE_OPERAND (core, 0);
>>>
>>> patch 1 is ok with that change.
>>
>> Installed with below change because of wide-int merge:
>> - core = build_fold_addr_expr (core);
>> + if (TREE_CODE (core) == MEM_REF)
>> +{
>> + aff_combination_add_cst (comb, wi::to_widest (TREE_OPERAND (core,
>> 1)));
>> + core = TREE_OPERAND (core, 0);
>>
>>>
PATCH 2:
2014-05-06 Bin Cheng
* gcc/tree-ssa-loop-ivopts.c (contain_complex_addr_expr): New.
(alloc_iv): Lower base expressions containing ADDR_EXPR.
>>>
>>> So this "lowers" addresses(?) which are based on &,
>>> like &a[0] + 4 but not &a + 4. I question this odd choice. ISTR
>> &a+4 is already in its lowered form, what we want to handle is &EXPR +
>> 4, in which EXPR is MEM_REF/ARRAY_REF, etc..
>>
>>> when originally introducing address lowering (in rev. 204497) I was
>>> concerned about the cost?
>> Yes, you did. I still think the iv number is relative small for each
>> loop, thus the change won't cause compilation time issue considering
>> the existing tree<->affine transformation in ivopt.
>> I would like to collect more accurate time information for ivopt in
>> gcc bootstrap. Should I use "-ftime-report" then add all time slices
>> in TV_TREE_LOOP_IVOPTS category? Is there any better solutions?
>> Thanks.
>>
>>>
>>> Now I wonder why we bother to convert the lowered form back
>>> to trees to store it in ->base and not simply keep (and always
>>> compute) the affine expansion only. Thus, change struct iv
>>> to have aff_tree base instead of tree base?
>>>
>>> Can you see what it takes to do such change? At the point
>>> we replace uses we go into affine representation again anyway.
>> Good idea, I may have a look.
> After going through work flow of ivopt, I think it's practical to make
> such change and can help compilation time. Ivopt calls
> tree_to_aff_combination to convert iv->base into affine_tree whenever
> it tries to represent an iv_use by an iv_cand, i.e., N*M times for
> computing cost of each iv_use represented by each iv_cand, and M times
> for rewriting each iv_use. Here, N and M stands for number of iv_use
> and iv_cand. Also strip_offset (which is a recursive function) is
> called for iv->base also at complexity of O(NM), so it may be improved
> too.
> To make a start, it's possible to store both tree and affine_tree of
> base in struct iv, and use either of them whenever needed.
>
> It seems to me there are two potential issues of this change. First,
> though affine_tree of base is stored, we can't operate on it directly,
> which means we have to copy it before using it.
You'd use it as unchanged operand to some aff_* function to avoid
actual copying of course.
> Second, ivopt
> sometimes computes affine_tree of base in different type other than
> TREE_TYPE(base), we may need to do additional calls to
> aff_combination_convert to get wanted type. All these will introduce
> additional computation and compromise benefit of the change.
Sure.
> At last, I did some experiments on how many additional calls to
> tree_to_aff_combination are introduced by this patch. The data of
> bootstrap shows it's less than 3% comparing to calls made by current
> implementation of ivopt, which confirms that this patch shouldn't have
> issue of compilation time.
>
> Any comments?
I'd see the use of affines as making the algorithmic structure of
IVOPTs clearer and more consistent (also with possibly using
the _expand variants later to get even more simplifications).
I don't want to force you to do this change but I thought it may
help further changes (as you seem to be doing a lot of IVOPTs
changes lately).
So - the patch is ok as-is then, but please consider refactoring
IVOPTs code when you do changes. It's current structure
is somewhat awkward.
Thanks,
Richard.
> Thanks,
> bin
>
>
> --
> Best Regards.
Re: [PATCH] Fix issue in uninit analysis (PR middle-end/61112)
On Mon, May 12, 2014 at 5:42 AM, Patrick Palka wrote:
> Hi,
>
> This patch fixes a bogus warning generated by -Wmaybe-uninitialized.
> The problem is that we sometimes fail to acknowledge a defining edge
> belonging to a control-dependence chain because we assume that each
> defining edge shares the same control-dependence root. But this may not
> be true if a defining edge flows into a PHI node that is different from
> the root PHI node. This faulty assumption may result in an incomplete
> control-dependency chain being computed, ultimately causing a
> false-positive warning like in the test case.
>
> To fix this, this patch computes the control-dependency root on a
> per-defining-edge basis, using the function nearest_common_dominator to
> find a common dominator (i.e. a BB before which control flow is
> irrelevant) between the control-dependency root of the root PHI node and
> that of the edge's dest PHI node.
>
> However, that change alone is not enough. We must also tweak the
> function collect_phi_def_edges to allow recursing on PHI nodes that are
> not dominated by the root PHI node's control dependency root as we can
> still figure out a control dependency chain in such cases as long the
> PHI node postdominates the PHI argument, i.e. as long as the control
> flow from the PHI argument edge down to the root PHI node is irrelevant.
>
> Both these changes are required to make the below test case compile
> without emitting a bogus warning.
>
> I bootstrapped and regtested this change on x86_64-unknown-linux-gnu.
> Is it OK for trunk?
CCing the author of the code.
Given the lengthy comment above I miss comments in the code
that reflect the complexity of this issue and explains the implementation.
Other than that I defer to David, but any improvement to this pass
is welcome! Can you assess the effect on compile-time this patch has?
(from an algorithmic standpoint?)
Thanks,
Richard.
> 2014-05-10 Patrick Palka
>
> * tree-ssa-uninit.c (collect_phi_def_edges): Remove cd_root
> parameter. Refactor to consolidate duplicate code. Tweak dump
> message.
> (find_def_preds): Add dump messages. Adjust call to
> collect_phi_def_edges. Adjust the control dependency root on
> a per-edge basis.
> ---
> gcc/testsuite/gcc.dg/uninit-pred-11.c | 20 ++
> gcc/tree-ssa-uninit.c | 75
> +++
> 2 files changed, 60 insertions(+), 35 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/uninit-pred-11.c
>
> diff --git a/gcc/testsuite/gcc.dg/uninit-pred-11.c
> b/gcc/testsuite/gcc.dg/uninit-pred-11.c
> new file mode 100644
> index 000..493be68
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/uninit-pred-11.c
> @@ -0,0 +1,20 @@
> +// PR middle-end/61112
> +// { dg-options "-Wuninitialized -O2" }
> +
> +int p;
> +
> +void
> +foo (int x, int y, int z)
> +{
> + int w;
> + if (x)
> +w = z;
> + if (y)
> +w = 10;
> + if (p)
> +w = 20;
> +
> + if (x || y || p)
> +p = w; // { dg-bogus "uninitialized" }
> +}
> +
> diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
> index 8b298fa..472b8e5 100644
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -641,13 +641,11 @@ find_predicates (pred_chain_union *preds,
>
> /* Computes the set of incoming edges of PHI that have non empty
> definitions of a phi chain. The collection will be done
> - recursively on operands that are defined by phis. CD_ROOT
> - is the control dependence root. *EDGES holds the result, and
> - VISITED_PHIS is a pointer set for detecting cycles. */
> + recursively on operands that are defined by phis. *EDGES holds
> + the result, and VISITED_PHIS is a pointer set for detecting cycles. */
>
> static void
> -collect_phi_def_edges (gimple phi, basic_block cd_root,
> - vec *edges,
> +collect_phi_def_edges (gimple phi, vec *edges,
> pointer_set_t *visited_phis)
> {
>size_t i, n;
> @@ -663,34 +661,23 @@ collect_phi_def_edges (gimple phi, basic_block cd_root,
>opnd_edge = gimple_phi_arg_edge (phi, i);
>opnd = gimple_phi_arg_def (phi, i);
>
> - if (TREE_CODE (opnd) != SSA_NAME)
> -{
> - if (dump_file && (dump_flags & TDF_DETAILS))
> -{
> - fprintf (dump_file, "\n[CHECK] Found def edge %d in ", (int)i);
> - print_gimple_stmt (dump_file, phi, 0, 0);
> -}
> - edges->safe_push (opnd_edge);
> -}
> - else
> -{
> - gimple def = SSA_NAME_DEF_STMT (opnd);
> -
> - if (gimple_code (def) == GIMPLE_PHI
> - && dominated_by_p (CDI_DOMINATORS,
> - gimple_bb (def), cd_root))
> -collect_phi_def_edges (def, cd_root, edges,
> - visited_phis);
> - else if (!uninit_undefined_value_p (opnd))
> -{
> - if (dump_file &&
RE: [Patch,Microblaze]: Added Break Handler Support
Hello Michael:
The following patch is to handle Software and Hardware breaks in Microblaze
Architecture.
Deja GNU testcase does not have any regressions and the testcase attached
passes through.
Review comments are incorporated.
Okay for trunk?
Thanks & Regards
Ajit
From 15dfaee8feef37430745d3dbc58f74bed876aabb Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal
Date: Tue, 13 May 2014 13:25:52 +0530
Subject: [PATCH] [Patch, microblaze] Added Break Handler support
Added Break Handler support to incorporate the hardware and software break. The
Break Handler routine
will be generating the rtbd instruction. At the call point where the software
breaks are generated with
the instruction brki with register operand as r16.
2014-05-13 Ajit Agarwal
* config/microblaze/microblaze.c
(microblaze_break_function_p,microblaze_is_break_handler) : New
* config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro
* config/microblaze/microblaze.md :
Extended support for generation of brki instruction and rtbd instruction.
* config/microblaze/microblaze-protos.h
(microblaze_break_function_p,microblaze_is_break_handler) : New Declaration.
* testsuite/gcc.target/microblaze/others/break_handler.c : New.
Signed-off-by:Nagaraju
---
gcc/config/microblaze/microblaze-protos.h |4 +-
gcc/config/microblaze/microblaze.c | 47 +---
gcc/config/microblaze/microblaze.h |2 +-
gcc/config/microblaze/microblaze.md| 34 ++
.../gcc.target/microblaze/others/break_handler.c | 15 ++
5 files changed, 83 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/microblaze/others/break_handler.c
diff --git a/gcc/config/microblaze/microblaze-protos.h
b/gcc/config/microblaze/microblaze-protos.h
index b03e9e1..f3cc099 100644
--- a/gcc/config/microblaze/microblaze-protos.h
+++ b/gcc/config/microblaze/microblaze-protos.h
@@ -40,10 +40,12 @@ extern void print_operand_address (FILE *, rtx);
extern void init_cumulative_args (CUMULATIVE_ARGS *,tree, rtx);
extern bool microblaze_legitimate_address_p (enum machine_mode, rtx, bool);
extern int microblaze_is_interrupt_variant (void);
+extern int microblaze_is_break_handler (void);
+extern int microblaze_break_function_p (tree func);
extern rtx microblaze_return_addr (int, rtx);
extern int simple_memory_operand (rtx, enum machine_mode);
extern int double_memory_operand (rtx, enum machine_mode);
-
+extern void microblaze_order_regs_for_local_alloc (void);
extern int microblaze_regno_ok_for_base_p (int, int);
extern HOST_WIDE_INT microblaze_initial_elimination_offset (int, int);
extern void microblaze_declare_object (FILE *, const char *, const char *,
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index ba8109b..fc458a5 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -209,6 +209,7 @@ enum reg_class microblaze_regno_to_class[] =
and epilogue and use appropriate interrupt return.
save_volatiles- Similar to interrupt handler, but use normal return. */
int interrupt_handler;
+int break_handler;
int fast_interrupt;
int save_volatiles;
@@ -217,6 +218,8 @@ const struct attribute_spec microblaze_attribute_table[] = {
affects_type_identity */
{"interrupt_handler", 0, 0, true,false, false,NULL,
false },
+ {"break_handler", 0, 0, true,false, false,NULL,
+false },
{"fast_interrupt",0, 0, true,false, false,NULL,
false },
{"save_volatiles" , 0, 0, true,false, false,NULL,
@@ -1866,7 +1869,18 @@ microblaze_fast_interrupt_function_p (tree func)
a = lookup_attribute ("fast_interrupt", DECL_ATTRIBUTES (func));
return a != NULL_TREE;
}
+int
+microblaze_break_function_p (tree func)
+{
+ tree a;
+ if (!func)
+return 0;
+ if (TREE_CODE (func) != FUNCTION_DECL)
+return 0;
+ a = lookup_attribute ("break_handler", DECL_ATTRIBUTES (func));
+ return a != NULL_TREE;
+}
/* Return true if FUNC is an interrupt function which uses
normal return, indicated by the "save_volatiles" attribute. */
@@ -1891,6 +1905,13 @@ microblaze_is_interrupt_variant (void)
{
return (interrupt_handler || fast_interrupt);
}
+int
+microblaze_is_break_handler (void)
+{
+ return break_handler;
+}
+
+
/* Determine of register must be saved/restored in call. */
static int
@@ -1994,9 +2015,14 @@ compute_frame_size (HOST_WIDE_INT size)
interrupt_handler =
microblaze_interrupt_function_p (current_function_decl);
+ break_handler =
+microblaze_break_function_p (current_function_decl);
+
fast_interrupt =
microblaze_fast_interrupt_function_p (current_function_decl);
save_volatiles = microblaze_save_volatiles (current_function_decl);
+ if (break_handler)
+interrupt_handler = break_handler;
gp_reg_size = 0;
mask = 0;
Re: [PATCH][x86] Support clflushopt, xsaves, xsavec.
On 12 May 15:42, Uros Bizjak wrote:
> On Mon, May 12, 2014 at 3:25 PM, Ilya Tocar wrote:
>
> > This patch add support for xsavec, xsaves ISA extensions, introduced in
> > [1], and clflushopt introduced in [2].
> >
> > [1]http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html
> > [2]http://software.intel.com/en-us/file/319433-018pdf
> >
> > Bootstraps, passes make-check.
>
> Please also add new options to g++.dg/other/i386-{2,3}.C and
> gcc.target/i386/sse-{14,15,22,23}.c.
>
> Uros.
Done.
Looks like sse-15 doesn't need new options, I've assumed sse-12/13.
Changelog:
2014-05-12 Ilya Tocar
* common/config/i386/i386-common.c
(OPTION_MASK_ISA_CLFLUSHOPT_SET): Define.
(OPTION_MASK_ISA_XSAVES_SET): Ditto.
(OPTION_MASK_ISA_XSAVEC_SET): Ditto.
(OPTION_MASK_ISA_CLFLUSHOPT_UNSET): Ditto.
(OPTION_MASK_ISA_XSAVES_UNSET): Ditto.
(OPTION_MASK_ISA_XSAVEC_UNSET): Ditto.
(ix86_handle_option): Handle OPT_mxsavec, OPT_mxsaves,
OPT_mclflushopt.
* config.gcc (i[34567]86-*-*): Add clflushoptintrin.h,
xsavecintrin.h, xsavesintrin.h.
(x86_64-*-*): Ditto.
* config/i386/clflushoptintrin.h: New.
* config/i386/xsavecintrin.h: Ditto.
* config/i386/xsavesintrin.h: Ditto.
* config/i386/cpuid.h (bit_CLFLUSHOPT): Define.
(bit_XSAVES): Ditto.
(bit_XSAVES): Ditto.
* config/i386/driver-i386.c (host_detect_local_cpu): Handle
-mclflushopt, -mxsavec, -mxsaves, -mno-xsaves, -mno-xsavec,
-mno-clflushopt.
* config/i386/i386-c.c (ix86_target_macros_internal): Handle
OPTION_MASK_ISA_CLFLUSHOPT, OPTION_MASK_ISA_XSAVEC,
OPTION_MASK_ISA_XSAVES.
* config/i386/i386.c (ix86_target_string): Handle -mclflushopt,
-mxsavec, -mxsaves.
(PTA_CLFLUSHOPT) Define.
(PTA_XSAVEC): Ditto.
(PTA_XSAVES): Ditto.
(ix86_option_override_internal): Handle new options.
(ix86_valid_target_attribute_inner_p): Ditto.
(ix86_builtins): Add IX86_BUILTIN_XSAVEC, IX86_BUILTIN_XSAVEC64,
IX86_BUILTIN_XSAVES, IX86_BUILTIN_XRSTORS, IX86_BUILTIN_XSAVES64,
IX86_BUILTIN_XRSTORS64, IX86_BUILTIN_CLFLUSHOPT.
(bdesc_special_args): Add __builtin_ia32_xsaves, __builtin_ia32_xrstors,
__builtin_ia32_xsavec, __builtin_ia32_xsaves64,
__builtin_ia32_xrstors64,
__builtin_ia32_xsavec64.
(ix86_init_mmx_sse_builtins): Add __builtin_ia32_clflushopt.
(ix86_expand_builtin): Handle new builtins.
* config/i386/i386.h (TARGET_CLFLUSHOPT) Define.
(TARGET_CLFLUSHOPT_P): Ditto.
(TARGET_XSAVEC): Ditto.
(TARGET_XSAVEC_P): Ditto.
(TARGET_XSAVES): Ditto.
(TARGET_XSAVES_P): Ditto.
* config/i386/i386.md (ANY_XSAVE): Add UNSPECV_XSAVEC, UNSPECV_XSAVES.
(ANY_XSAVE64)" Add UNSPECV_XSAVEC64, UNSPECV_XSAVES64.
(attr xsave): Add xsavec, xsavec64, xsaves, xsaves64.
(ANY_XRSTOR): New.
(ANY_XRSTOR64): Ditto.
(xrstor): Ditto.
(xrstor): Change into .
(xrstor_rex64): Change into _rex64.
(xrstor64): Change into 64
(clflushopt): New.
* config/i386/i386.opt (mclflushopt): New.
(mxsavec): Ditto.
(mxsaves): Ditto.
* config/i386/x86intrin.h: Add clflushoptintrin.h, xsavesintrin.h,
xsavecintrin.h.
* doc/invoke.texi: Document new options.
And for tests:
2014-05-12 Ilya Tocar
* gcc.target/i386/clflushopt-1.c: New.
* gcc.target/i386/xsavec-1.c: Ditto.
* gcc.target/i386/xsavec64-1.c: Ditto.
* gcc.target/i386/xsaves-1.c: Ditto.
* gcc.target/i386/xsaves64-1.c: Ditto.
* gcc.target/i386/sse-12.c: Test new options.
* gcc.target/i386/sse-13.c: Ditto.
* gcc.target/i386/sse-14.c: Ditto.
* gcc.target/i386/sse-22.c: Ditto.
* gcc.target/i386/sse-23.c: Ditto.
* g++.dg/other/i386-2.C: Ditto.
* g++.dg/other/i386-3.C: Ditto.
Updated patch below:
---
gcc/common/config/i386/i386-common.c | 47
gcc/config.gcc | 6 +-
gcc/config/i386/clflushoptintrin.h | 49
gcc/config/i386/cpuid.h | 3 +
gcc/config/i386/driver-i386.c| 12 +++-
gcc/config/i386/i386-c.c | 6 ++
gcc/config/i386/i386.c | 83 +++-
gcc/config/i386/i386.h | 6 ++
gcc/config/i386/i386.md | 64 +
gcc/config/i386/i386.opt | 12
gcc/config/i386/x86intrin.h | 6 ++
gcc/config/i386/xsavecintrin.h | 58 +++
gcc/config/i386/xsavesintrin.h | 72
gcc/doc/invoke.texi | 7 +++
gcc/testsuite/g++.
Re: [PATCH 1/5] rm a bunch of _stat allocation functions
On Tue, May 13, 2014 at 8:41 AM, wrote:
> From: Trevor Saunders
>
> Hi,
>
> basically this patch is unchanged from the last time I sent this series,
> except
> I reordered the includes in rtl.c to have ggc.h come before vec.h because
> vec.h
> declares some functions from ggc.h if building a binary to run on the build
> machine. The problem is if the function takes default args it can't be
> declared with the default args twice. So the simplest solution is include
> ggc.h first then vec.h will never try and declare those functions.
>
> Trev
>
> bootstrapped + regtested again, just to be safe ok?
Ok.
Thanks,
Richard.
>
>
> gcc/ChangeLog:
>
> 2014-04-23 tbsaunde
>
> * dwarf2out.c (tree_add_const_value_attribute): Call
> ggc_internal_cleared_alloc instead of ggc_alloc_cleared_atomic.
> * gengtype.c (write_typed_alloc_def): Call ggc_internal_alloc
> instead of ggc_internal_alloc_stat.
> * ggc-common.c (ggc_internal_cleared_alloc): Drop _stat suffix.
> (ggc_realloc): Likewise.
> * ggc-none.c (ggc_internal_alloc): Likewise.
> (ggc_internal_cleared_alloc): Likewise.
> * ggc-page.c: Likewise.
> * ggc.h (ggc_internal_alloc_stat): Likewise.
> (ggc_internal_alloc): Remove macro.
> (ggc_internal_cleared_alloc_stat): Drop _stat suffix.
> (ggc_internal_cleared_alloc): Remove macro.
> (GGC_RESIZEVEC): Adjust.
> (ggc_resizevar): Remove macro.
> (ggc_internal_vec_alloc_stat): Drop _stat suffix.
> (ggc_internal_cleared_vec_alloc_stat): Likewise.
> (ggc_internal_vec_cleared_alloc): Remove macro.
> (ggc_alloc_atomic_stat): Drop _stat suffix.
> (ggc_alloc_atomic): Remove macro.
> (ggc_alloc_cleared_atomic): Remove macro.
> (ggc_alloc_string_stat): Drop _stat suffix.
> (ggc_alloc_string): Remove macro.
> (ggc_alloc_rtx_def_stat): Adjust.
> (ggc_alloc_tree_node_stat): Likewise.
> (ggc_alloc_cleared_tree_node_stat): Likewise.
> (ggc_alloc_cleared_gimple_statement_stat): Likewise.
> (ggc_alloc_cleared_simd_clone_stat): Likewise.
> * gimple.c (gimple_build_omp_for): Likewise.
> (gimple_copy): Likewise.
> * stringpool.c (ggc_alloc_string_stat): Drop _stat suffix.
> * toplev.c (realloc_for_line_map): Adjust.
> * tree-data-ref.h (lambda_vector_new): Likewise.
> * tree-phinodes.c (allocate_phi_node): Likewise.
> * tree.c (grow_tree_vec_stat): Likewise.
> * vec.h (va_gc::reserve): Adjust.
>
> gcc/java/ChangeLog:
>
> 2014-04-23 tbsaunde
>
> * constants.c (set_constant_entry): Adjust.
> ---
> gcc/dwarf2out.c | 2 +-
> gcc/gengtype.c | 2 +-
> gcc/ggc-common.c | 10
> gcc/ggc-none.c | 4 ++--
> gcc/ggc-page.c | 2 +-
> gcc/ggc.h| 65
> +++-
> gcc/gimple.c | 8 +++
> gcc/java/constants.c | 4 ++--
> gcc/rtl.c| 2 +-
> gcc/stringpool.c | 4 ++--
> gcc/toplev.c | 2 +-
> gcc/tree-data-ref.h | 2 +-
> gcc/tree-phinodes.c | 2 +-
> gcc/tree.c | 2 +-
> gcc/vec.h| 5 ++--
> 15 files changed, 49 insertions(+), 67 deletions(-)
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 5874d73..21833ed 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -16048,7 +16048,7 @@ tree_add_const_value_attribute (dw_die_ref die, tree
> t)
>if (size > 0 && (int) size == size)
> {
> unsigned char *array = (unsigned char *)
> - ggc_alloc_cleared_atomic (size);
> + ggc_internal_cleared_alloc (size);
>
> if (native_encode_initializer (init, array, size))
> {
> diff --git a/gcc/gengtype.c b/gcc/gengtype.c
> index 1c13eea..bb2c9fb 100644
> --- a/gcc/gengtype.c
> +++ b/gcc/gengtype.c
> @@ -4974,7 +4974,7 @@ write_typed_alloc_def (outf_p f,
>(two_args ? ", " : ""),
>(quantity == vector) ? "n" : "");
>oprintf (f, "((%s%s *)", type_specifier, type_name);
> - oprintf (f, "(ggc_internal_%salloc_stat (", allocator_type);
> + oprintf (f, "(ggc_internal_%salloc (", allocator_type);
>if (variable_size)
> oprintf (f, "SIZE");
>else
> diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
> index 97c9b87..e1cc48e 100644
> --- a/gcc/ggc-common.c
> +++ b/gcc/ggc-common.c
> @@ -174,22 +174,22 @@ ggc_mark_roots (void)
>
> /* Allocate a block of memory, then clear it. */
> void *
> -ggc_internal_cleared_alloc_stat (size_t size MEM_STAT_DECL)
> +ggc_internal_cleared_alloc (size_t size MEM_STAT_DECL)
> {
> - void *buf = ggc_internal_alloc_stat (size PASS_MEM_STAT);
> + void *buf = ggc_internal_alloc (size PASS_MEM_STAT);
>memset (buf, 0, size);
>return buf;
> }
>
> /* Resize a block of memory, possibly re-allocating it. */
> void *
> -ggc_realloc_stat (void *x, size_t size MEM_S
Re: [PATCH 4/5] add finalizers to ggc
On Tue, May 13, 2014 at 8:41 AM, wrote:
> From: Trevor Saunders
>
> This implements finalizers by keeping a list of registered finalizers
> and after every mark but before sweeping check to see if any of them are
> for unmarked blocks.
>
> This uses the two vector and forward iteration approach I think richi agreed
> to.
>
> bootstrapped + regtested on x86_64-unknown-linux-gnu ok?
Ok with a comment before ggc_handle_finalizers.
Thanks,
Richard.
> Trev
>
> gcc/ChangeLog:
>
> * ggc-common.c (ggc_internal_cleared_alloc): Adjust.
> * ggc-none.c (ggc_internal_alloc): Assert if a finalizer is passed.
> (ggc_internal_cleared_alloc): Likewise.
> * ggc-page.c (finalizer): New class.
> (vec_finalizer): Likewise.
> (globals::finalizers): New member.
> (globals::vec_finalizers): Likewise.
> (ggc_internal_alloc): Record the finalizer if any for the block being
> allocated.
> (ggc_handle_finalizers): New function.
> (ggc_collect): Call ggc_handle_finalizers.
> * ggc.h (ggc_internal_alloc): Add arguments to allow installing a
> finalizer.
> (ggc_internal_cleared_alloc): Likewise.
> (finalize): New function.
> (need_finalization_p): Likewise.
> (ggc_alloc): Install the type's destructor as the finalizer if it
> might do something.
> (ggc_cleared_alloc): Likewise.
> (ggc_vec_alloc): Likewise.
> (ggc_cleared_vec_alloc): Likewise.
> ---
> gcc/ggc-common.c | 5 ++--
> gcc/ggc-none.c | 8 --
> gcc/ggc-page.c | 87
> +++-
> gcc/ggc.h| 71 +++--
> 4 files changed, 158 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
> index e89cc64..b11a10c 100644
> --- a/gcc/ggc-common.c
> +++ b/gcc/ggc-common.c
> @@ -174,9 +174,10 @@ ggc_mark_roots (void)
>
> /* Allocate a block of memory, then clear it. */
> void *
> -ggc_internal_cleared_alloc (size_t size MEM_STAT_DECL)
> +ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t s, size_t
> n
> + MEM_STAT_DECL)
> {
> - void *buf = ggc_internal_alloc (size PASS_MEM_STAT);
> + void *buf = ggc_internal_alloc (size, f, s, n PASS_MEM_STAT);
>memset (buf, 0, size);
>return buf;
> }
> diff --git a/gcc/ggc-none.c b/gcc/ggc-none.c
> index aad89bf..97d3566 100644
> --- a/gcc/ggc-none.c
> +++ b/gcc/ggc-none.c
> @@ -41,14 +41,18 @@ ggc_round_alloc_size (size_t requested_size)
> }
>
> void *
> -ggc_internal_alloc (size_t size MEM_STAT_DECL)
> +ggc_internal_alloc (size_t size, void (*f)(void *), size_t, size_t
> + MEM_STAT_DECL)
> {
> + gcc_assert (!f); // ggc-none doesn't support finalizers
>return xmalloc (size);
> }
>
> void *
> -ggc_internal_cleared_alloc (size_t size MEM_STAT_DECL)
> +ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t, size_t
> + MEM_STAT_DECL)
> {
> + gcc_assert (!f); // ggc-none doesn't support finalizers
>return xcalloc (size, 1);
> }
>
> diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
> index ae5e88a..b3a1a2a 100644
> --- a/gcc/ggc-page.c
> +++ b/gcc/ggc-page.c
> @@ -332,6 +332,41 @@ typedef struct page_table_chain
>
> #endif
>
> +class finalizer
> +{
> +public:
> + finalizer (void *addr, void (*f)(void *)) : m_addr (addr), m_function (f)
> {}
> +
> + void *addr () const { return m_addr; }
> +
> + void call () const { m_function (m_addr); }
> +
> +private:
> + void *m_addr;
> + void (*m_function)(void *);
> +};
> +
> +class vec_finalizer
> +{
> +public:
> + vec_finalizer (uintptr_t addr, void (*f)(void *), size_t s, size_t n) :
> +m_addr (addr), m_function (f), m_object_size (s), m_n_objects (n) {}
> +
> + void call () const
> +{
> + for (size_t i = 0; i < m_n_objects; i++)
> + m_function (reinterpret_cast (m_addr + (i * m_object_size)));
> +}
> +
> + void *addr () const { return reinterpret_cast (m_addr); }
> +
> +private:
> + uintptr_t m_addr;
> + void (*m_function)(void *);
> + size_t m_object_size;
> + size_t m_n_objects;
> + };
> +
> #ifdef ENABLE_GC_ALWAYS_COLLECT
> /* List of free objects to be verified as actually free on the
> next collection. */
> @@ -425,6 +460,12 @@ static struct globals
> better runtime data access pattern. */
>unsigned long **save_in_use;
>
> + /* Finalizers for single objects. */
> + vec finalizers;
> +
> + /* Finalizers for vectors of objects. */
> + vec vec_finalizers;
> +
> #ifdef ENABLE_GC_ALWAYS_COLLECT
>/* List of free objects to be verified as actually free on the
> next collection. */
> @@ -1202,7 +1243,8 @@ ggc_round_alloc_size (size_t requested_size)
> /* Allocate a chunk of memory of SIZE bytes. Its contents are undefined. */
>
> void *
> -ggc_internal_alloc (size_t size MEM_STAT_DECL)
> +ggc_internal_alloc (size_t size, void (*f)(void
Re: [PATCH, 2/2] shrink wrap a function with a single loop: split live_edge
On 9 May 2014 14:08, Jeff Law wrote:
> On 05/08/14 02:07, Zhenqiang Chen wrote:
>>
>> Hi,
>>
>> The patch splits the live_edge for move_insn_for_shrink_wrap to sink
>> the copy out of the entry block.
>>
>> Bootstrap and no make check regression on X86-64 and ARM.
>>
>> OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2014-05-08 Zhenqiang Chen
>>
>> * function.c (next_block_for_reg): Allow live_edge->dest has two
>> predecessors.
>> (move_insn_for_shrink_wrap): Split live_edge.
>> (prepre_shrink_wrap): One more parameter for
>> move_insn_for_shrink_wrap.
>>
>>
>> diff --git a/gcc/function.c b/gcc/function.c
>> index 764ac82..0be58e2 100644
>> --- a/gcc/function.c
>> +++ b/gcc/function.c
>> @@ -5381,7 +5381,7 @@ requires_stack_frame_p (rtx insn, HARD_REG_SET
>> prologue_used,
>> and if BB is its only predecessor. Return that block if so,
>> otherwise return null. */
>>
>> -static basic_block
>> +static edge
>> next_block_for_reg (basic_block bb, int regno, int end_regno)
>
> Comment for this function needs to be changed. You're no longer returning a
> block, but the edge leading to the block. It also seems the name of the
> function ought to change.
The comment and function name are updated.
> This looks basically OK. I'd like to see the requested cleanups made, then
> the resulting new patch reposted for a final review.
Here is the updated patch based on the cleaned shrink-wrapping code:
ChangeLog:
2014-05-13 Zhenqiang Chen
* shrink-wrap.c: Update comment.
(next_block_for_reg): Rename to live_edge_for_reg.
(live_edge_for_reg): Allow live_edge->dest has two predecessors.
(move_insn_for_shrink_wrap): Split live_edge.
(prepre_shrink_wrap): One more parameter for move_insn_for_shrink_wrap.
diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index b302777..f11e920 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -1,4 +1,4 @@
-/* Expands front end tree to back end RTL for GCC.
+/* Shrink-wrapping related optimizations.
Copyright (C) 1987-2014 Free Software Foundation, Inc.
This file is part of GCC.
@@ -110,12 +110,12 @@ requires_stack_frame_p (rtx insn, HARD_REG_SET
prologue_used,
return false;
}
-/* See whether BB has a single successor that uses [REGNO, END_REGNO),
- and if BB is its only predecessor. Return that block if so,
- otherwise return null. */
+/* See whether there has a single live edge from BB, which dest uses
+ [REGNO, END_REGNO). Return the live edge if its dest bb has
+ one or two predecessors. Otherwise return NULL. */
-static basic_block
-next_block_for_reg (basic_block bb, int regno, int end_regno)
+static edge
+live_edge_for_reg (basic_block bb, int regno, int end_regno)
{
edge e, live_edge;
edge_iterator ei;
@@ -148,25 +148,30 @@ next_block_for_reg (basic_block bb, int regno,
int end_regno)
if (live_edge->flags & EDGE_ABNORMAL)
return NULL;
- if (EDGE_COUNT (live_edge->dest->preds) > 1)
+ /* When live_edge->dest->preds == 2, we can create a new block on
+ the edge to make it meet the requirement. */
+ if (EDGE_COUNT (live_edge->dest->preds) > 2)
return NULL;
- return live_edge->dest;
+ return live_edge;
}
/* Try to move INSN from BB to a successor. Return true on success.
USES and DEFS are the set of registers that are used and defined
- after INSN in BB. */
+ after INSN in BB. SPLIT_P indicates whether a live edge from BB
+ is splitted or not. */
static bool
move_insn_for_shrink_wrap (basic_block bb, rtx insn,
const HARD_REG_SET uses,
- const HARD_REG_SET defs)
+ const HARD_REG_SET defs,
+ bool *split_p)
{
rtx set, src, dest;
bitmap live_out, live_in, bb_uses, bb_defs;
unsigned int i, dregno, end_dregno, sregno, end_sregno;
basic_block next_block;
+ edge live_edge;
/* Look for a simple register copy. */
set = single_set (insn);
@@ -191,10 +196,24 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
return false;
/* See whether there is a successor block to which we could move INSN. */
- next_block = next_block_for_reg (bb, dregno, end_dregno);
- if (!next_block)
+ live_edge = live_edge_for_reg (bb, dregno, end_dregno);
+ if (!live_edge)
return false;
+ next_block = live_edge->dest;
+ /* Create a new basic block on the edge. */
+ if (EDGE_COUNT (next_block->preds) == 2)
+{
+ next_block = split_edge (live_edge);
+
+ bitmap_copy (df_get_live_in (next_block), df_get_live_out (bb));
+ df_set_bb_dirty (next_block);
+
+ /* We should not split more than once for a function. */
+ gcc_assert (!(*split_p));
+ *split_p = true;
+}
+
/* At this point we are committed to moving INSN, but let's try to
move it as far as we can. */
do
@@ -212,7 +231,9 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
Re: [PATCH, 1/2] shrink wrap a function with a single loop: copy propagation
After reading the code in regcprop.c, I think I should reuse the
copyprop_hardreg_forward_1. So rewrite the patch, which is much simple
and should handle HAVE_cc0. But not sure we'd handle DEBUG_INSN or
not.
2014-05-13 Zhenqiang Chen
* regcprop.c (skip_debug_insn_p): New decl.
(replace_oldest_value_reg): Check skip_debug_insn_p.
(copyprop_hardreg_forward_bb_without_debug_insn.): New function.
* shrink-wrap.c (prepare_shrink_wrap):
Call copyprop_hardreg_forward_bb_without_debug_insn.
* function.h (copyprop_hardreg_forward_bb_without_debug_insn):
New prototype.
testsuite/ChangeLog:
2014-05-13 Zhenqiang Chen
* shrink-wrap-loop.c: New test case.
diff --git a/gcc/regcprop.c b/gcc/regcprop.c
index a710cc38..f76a944 100644
--- a/gcc/regcprop.c
+++ b/gcc/regcprop.c
@@ -77,6 +77,7 @@ struct value_data
};
static alloc_pool debug_insn_changes_pool;
+static bool skip_debug_insn_p = false;
static void kill_value_one_regno (unsigned, struct value_data *);
static void kill_value_regno (unsigned, unsigned, struct value_data *);
@@ -485,7 +486,7 @@ replace_oldest_value_reg (rtx *loc, enum reg_class
cl, rtx insn,
struct value_data *vd)
{
rtx new_rtx = find_oldest_value_reg (cl, *loc, vd);
- if (new_rtx)
+ if (new_rtx && (!DEBUG_INSN_P (insn) || !skip_debug_insn_p))
{
if (DEBUG_INSN_P (insn))
{
@@ -1112,6 +1113,26 @@ debug_value_data (struct value_data *vd)
vd->e[i].next_regno);
}
+/* Do copyprop_hardreg_forward_1 for a single basic block BB.
+ DEBUG_INSN is skipped since we do not want to involve DF related
+ staff as how it is handled in function pass_cprop_hardreg::execute.
+
+ NOTE: Currently it is only used for shrink-wrap. Maybe extend it
+ to handle DEBUG_INSN for other uses. */
+
+void
+copyprop_hardreg_forward_bb_without_debug_insn (basic_block bb)
+{
+ struct value_data *vd;
+ vd = XNEWVEC (struct value_data, 1);
+ init_value_data (vd);
+
+ skip_debug_insn_p = true;
+ copyprop_hardreg_forward_1 (bb, vd);
+ free (vd);
+ skip_debug_insn_p = false;
+}
+
#ifdef ENABLE_CHECKING
static void
validate_value_data (struct value_data *vd)
diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index f11e920..ce49f16 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -320,6 +320,15 @@ prepare_shrink_wrap (basic_block entry_block)
df_ref *ref;
bool split_p = false;
+ if (JUMP_P (BB_END (entry_block)))
+{
+ /* To have more shrink-wrapping opportunities, prepare_shrink_wrap tries
+to sink the copies from parameter to callee saved register out of
+entry block. copyprop_hardreg_forward_bb_without_debug_insn is called
+to release some dependences. */
+ copyprop_hardreg_forward_bb_without_debug_insn (entry_block);
+}
+
CLEAR_HARD_REG_SET (uses);
CLEAR_HARD_REG_SET (defs);
FOR_BB_INSNS_REVERSE_SAFE (entry_block, insn, curr)
diff --git a/gcc/shrink-wrap.h b/gcc/shrink-wrap.h
index 22b1d5c..9058d34 100644
--- a/gcc/shrink-wrap.h
+++ b/gcc/shrink-wrap.h
@@ -45,6 +45,8 @@ extern edge get_unconverted_simple_return (edge, bitmap_head,
extern void convert_to_simple_return (edge entry_edge, edge orig_entry_edge,
bitmap_head bb_flags, rtx returnjump,
vec unconverted_simple_returns);
+/* In regcprop.c */
+extern void copyprop_hardreg_forward_bb_without_debug_insn (basic_block bb);
#endif
#endif /* GCC_SHRINK_WRAP_H */
diff --git a/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
b/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
new file mode 100644
index 000..17dca4e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target { { x86_64-*-* } || { arm_thumb2 } } } } */
+/* { dg-options "-O2 -fdump-rtl-pro_and_epilogue" } */
+
+int foo (int *p1, int *p2);
+
+int
+test (int *p1, int *p2)
+{
+ int *p;
+
+ for (p = p2; p != 0; p++)
+{
+ if (!foo (p, p1))
+return 0;
+}
+
+ return 1;
+}
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping"
"pro_and_epilogue" } } */
+/* { dg-final { cleanup-rtl-dump "pro_and_epilogue" } } */
On 9 May 2014 14:00, Jeff Law wrote:
> On 05/08/14 02:06, Zhenqiang Chen wrote:
>>
>> Hi,
>>
>> Similar issue was discussed in thread
>> http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01145.html. The patches
>> are close to Jeff's suggestion: "sink just the moves out of the
>> incoming argument registers".
>>
>> The patch and following one try to shrink wrap a function with a
>> single loop, which can not be handled by
>> split_live_ranges_for_shrink_wrap and prepare_shrink_wrap, since the
>> induction variable has more than one definitions. Take the test case
>> in the patch as example, the pseudo code before shrink-wrap is like:
>>
>> p = p2
>> if (!p) goto return
>> L1: ...
>> p = ...
>> ...
>> goto L1
>> ...
>> ret
Re: [PATCH][x86] Support clflushopt, xsaves, xsavec.
On Tue, May 13, 2014 at 11:18 AM, Ilya Tocar wrote:
>> > This patch add support for xsavec, xsaves ISA extensions, introduced in
>> > [1], and clflushopt introduced in [2].
>> >
>> > [1]http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html
>> > [2]http://software.intel.com/en-us/file/319433-018pdf
>> >
>> > Bootstraps, passes make-check.
>>
>> Please also add new options to g++.dg/other/i386-{2,3}.C and
>> gcc.target/i386/sse-{14,15,22,23}.c.
>>
>> Uros.
>
> Done.
> Looks like sse-15 doesn't need new options, I've assumed sse-12/13.
Yes, you are right.
> Changelog:
>
> 2014-05-12 Ilya Tocar
>
> * common/config/i386/i386-common.c
> (OPTION_MASK_ISA_CLFLUSHOPT_SET): Define.
> (OPTION_MASK_ISA_XSAVES_SET): Ditto.
> (OPTION_MASK_ISA_XSAVEC_SET): Ditto.
> (OPTION_MASK_ISA_CLFLUSHOPT_UNSET): Ditto.
> (OPTION_MASK_ISA_XSAVES_UNSET): Ditto.
> (OPTION_MASK_ISA_XSAVEC_UNSET): Ditto.
> (ix86_handle_option): Handle OPT_mxsavec, OPT_mxsaves,
> OPT_mclflushopt.
> * config.gcc (i[34567]86-*-*): Add clflushoptintrin.h,
> xsavecintrin.h, xsavesintrin.h.
> (x86_64-*-*): Ditto.
> * config/i386/clflushoptintrin.h: New.
> * config/i386/xsavecintrin.h: Ditto.
> * config/i386/xsavesintrin.h: Ditto.
> * config/i386/cpuid.h (bit_CLFLUSHOPT): Define.
> (bit_XSAVES): Ditto.
> (bit_XSAVES): Ditto.
> * config/i386/driver-i386.c (host_detect_local_cpu): Handle
> -mclflushopt, -mxsavec, -mxsaves, -mno-xsaves, -mno-xsavec,
> -mno-clflushopt.
> * config/i386/i386-c.c (ix86_target_macros_internal): Handle
> OPTION_MASK_ISA_CLFLUSHOPT, OPTION_MASK_ISA_XSAVEC,
> OPTION_MASK_ISA_XSAVES.
> * config/i386/i386.c (ix86_target_string): Handle -mclflushopt,
> -mxsavec, -mxsaves.
> (PTA_CLFLUSHOPT) Define.
> (PTA_XSAVEC): Ditto.
> (PTA_XSAVES): Ditto.
> (ix86_option_override_internal): Handle new options.
> (ix86_valid_target_attribute_inner_p): Ditto.
> (ix86_builtins): Add IX86_BUILTIN_XSAVEC, IX86_BUILTIN_XSAVEC64,
> IX86_BUILTIN_XSAVES, IX86_BUILTIN_XRSTORS, IX86_BUILTIN_XSAVES64,
> IX86_BUILTIN_XRSTORS64, IX86_BUILTIN_CLFLUSHOPT.
> (bdesc_special_args): Add __builtin_ia32_xsaves,
> __builtin_ia32_xrstors,
> __builtin_ia32_xsavec, __builtin_ia32_xsaves64,
> __builtin_ia32_xrstors64,
> __builtin_ia32_xsavec64.
> (ix86_init_mmx_sse_builtins): Add __builtin_ia32_clflushopt.
> (ix86_expand_builtin): Handle new builtins.
> * config/i386/i386.h (TARGET_CLFLUSHOPT) Define.
> (TARGET_CLFLUSHOPT_P): Ditto.
> (TARGET_XSAVEC): Ditto.
> (TARGET_XSAVEC_P): Ditto.
> (TARGET_XSAVES): Ditto.
> (TARGET_XSAVES_P): Ditto.
> * config/i386/i386.md (ANY_XSAVE): Add UNSPECV_XSAVEC, UNSPECV_XSAVES.
> (ANY_XSAVE64)" Add UNSPECV_XSAVEC64, UNSPECV_XSAVES64.
> (attr xsave): Add xsavec, xsavec64, xsaves, xsaves64.
> (ANY_XRSTOR): New.
> (ANY_XRSTOR64): Ditto.
> (xrstor): Ditto.
> (xrstor): Change into .
> (xrstor_rex64): Change into _rex64.
> (xrstor64): Change into 64
> (clflushopt): New.
> * config/i386/i386.opt (mclflushopt): New.
> (mxsavec): Ditto.
> (mxsaves): Ditto.
> * config/i386/x86intrin.h: Add clflushoptintrin.h, xsavesintrin.h,
> xsavecintrin.h.
> * doc/invoke.texi: Document new options.
>
> And for tests:
>
> 2014-05-12 Ilya Tocar
> * gcc.target/i386/clflushopt-1.c: New.
> * gcc.target/i386/xsavec-1.c: Ditto.
> * gcc.target/i386/xsavec64-1.c: Ditto.
> * gcc.target/i386/xsaves-1.c: Ditto.
> * gcc.target/i386/xsaves64-1.c: Ditto.
> * gcc.target/i386/sse-12.c: Test new options.
> * gcc.target/i386/sse-13.c: Ditto.
> * gcc.target/i386/sse-14.c: Ditto.
> * gcc.target/i386/sse-22.c: Ditto.
> * gcc.target/i386/sse-23.c: Ditto.
> * g++.dg/other/i386-2.C: Ditto.
> * g++.dg/other/i386-3.C: Ditto.
This is OK for mainline.
Thanks,
Uros.
Re: [PATCH GCC]Pick up more address lowering cases for ivopt and tree-affine.c
On Tue, May 13, 2014 at 4:59 PM, Richard Biener
wrote:
> On Sun, May 11, 2014 at 2:49 PM, Bin.Cheng wrote:
>> On Thu, May 8, 2014 at 5:08 PM, Bin.Cheng wrote:
>>> On Tue, May 6, 2014 at 6:44 PM, Richard Biener
>>> wrote:
On Tue, May 6, 2014 at 10:39 AM, Bin.Cheng wrote:
> On Fri, Dec 6, 2013 at 6:19 PM, Richard Biener
> wrote:
>>>
> Hi,
> I split the patch into two and updated the test case.
> The patches pass bootstrap/tests on x86/x86_64, also pass test on arm
> cortex-m3.
> Is it OK?
>
> Thanks,
> bin
>
> PATCH 1:
>
> 2014-05-06 Bin Cheng
>
> * gcc/tree-affine.c (tree_to_aff_combination): Handle MEM_REF for
> core part of address expressions.
No gcc/ in the changelog
Simplify that by using aff_combination_add_cst:
+ if (TREE_CODE (core) == MEM_REF)
+ {
+ aff_combination_add_cst (comb, mem_ref_offset (core));
+ core = TREE_OPERAND (core, 0);
patch 1 is ok with that change.
>>>
>>> Installed with below change because of wide-int merge:
>>> - core = build_fold_addr_expr (core);
>>> + if (TREE_CODE (core) == MEM_REF)
>>> +{
>>> + aff_combination_add_cst (comb, wi::to_widest (TREE_OPERAND (core,
>>> 1)));
>>> + core = TREE_OPERAND (core, 0);
>>>
> PATCH 2:
>
> 2014-05-06 Bin Cheng
>
> * gcc/tree-ssa-loop-ivopts.c (contain_complex_addr_expr): New.
> (alloc_iv): Lower base expressions containing ADDR_EXPR.
So this "lowers" addresses(?) which are based on &,
like &a[0] + 4 but not &a + 4. I question this odd choice. ISTR
>>> &a+4 is already in its lowered form, what we want to handle is &EXPR +
>>> 4, in which EXPR is MEM_REF/ARRAY_REF, etc..
>>>
when originally introducing address lowering (in rev. 204497) I was
concerned about the cost?
>>> Yes, you did. I still think the iv number is relative small for each
>>> loop, thus the change won't cause compilation time issue considering
>>> the existing tree<->affine transformation in ivopt.
>>> I would like to collect more accurate time information for ivopt in
>>> gcc bootstrap. Should I use "-ftime-report" then add all time slices
>>> in TV_TREE_LOOP_IVOPTS category? Is there any better solutions?
>>> Thanks.
>>>
Now I wonder why we bother to convert the lowered form back
to trees to store it in ->base and not simply keep (and always
compute) the affine expansion only. Thus, change struct iv
to have aff_tree base instead of tree base?
Can you see what it takes to do such change? At the point
we replace uses we go into affine representation again anyway.
>>> Good idea, I may have a look.
>> After going through work flow of ivopt, I think it's practical to make
>> such change and can help compilation time. Ivopt calls
>> tree_to_aff_combination to convert iv->base into affine_tree whenever
>> it tries to represent an iv_use by an iv_cand, i.e., N*M times for
>> computing cost of each iv_use represented by each iv_cand, and M times
>> for rewriting each iv_use. Here, N and M stands for number of iv_use
>> and iv_cand. Also strip_offset (which is a recursive function) is
>> called for iv->base also at complexity of O(NM), so it may be improved
>> too.
>> To make a start, it's possible to store both tree and affine_tree of
>> base in struct iv, and use either of them whenever needed.
>>
>> It seems to me there are two potential issues of this change. First,
>> though affine_tree of base is stored, we can't operate on it directly,
>> which means we have to copy it before using it.
>
> You'd use it as unchanged operand to some aff_* function to avoid
> actual copying of course.
>
>> Second, ivopt
>> sometimes computes affine_tree of base in different type other than
>> TREE_TYPE(base), we may need to do additional calls to
>> aff_combination_convert to get wanted type. All these will introduce
>> additional computation and compromise benefit of the change.
>
> Sure.
>
>> At last, I did some experiments on how many additional calls to
>> tree_to_aff_combination are introduced by this patch. The data of
>> bootstrap shows it's less than 3% comparing to calls made by current
>> implementation of ivopt, which confirms that this patch shouldn't have
>> issue of compilation time.
>>
>> Any comments?
>
> I'd see the use of affines as making the algorithmic structure of
> IVOPTs clearer and more consistent (also with possibly using
> the _expand variants later to get even more simplifications).
Yes, one example, it's possible to rewrite iv uses in a loop-invariant
sensitive manner if we can use affine in the whole process. Currently
fold_tree routines tend to decompose invariant into different
expressions.
>
> I don't want to force you to do this change but I thought it may
> help further changes (as you seem to be doing a lot of IVOPTs
> changes lately).
>
> So -
[PATCH] Fix PR60973
As discussed in the PR, tailcall settings have to be cleared by
the inliner.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
and 4.9 branch.
Richard.
2014-05-13 Richard Biener
PR ipa/60973
* tree-inline.c (remap_gimple_stmt): Clear tail call flag,
it needs revisiting whether the call still may be tail-called.
Index: gcc/tree-inline.c
===
--- gcc/tree-inline.c (revision 210263)
+++ gcc/tree-inline.c (working copy)
@@ -1483,6 +1487,11 @@ remap_gimple_stmt (gimple stmt, copy_bod
/* Create a new deep copy of the statement. */
copy = gimple_copy (stmt);
+ /* Clear flags that need revisiting. */
+ if (is_gimple_call (copy)
+ && gimple_call_tail_p (copy))
+ gimple_call_set_tail (copy, false);
+
/* Remap the region numbers for __builtin_eh_{pointer,filter},
RESX and EH_DISPATCH. */
if (id->eh_map)
Re: [DOC Patch] symbol rename pragmas
Thank you for taking the time to review this.
the current text makes reference to "compatibility with the Solaris system
headers," the remaining pragma is (according to the existing text)
"currently on all platforms." This makes referring to Solaris both
superfluous and potentially confusing.
I disagree: #pragma redefine_extname does exist for Solaris
compatibility only, even if it now works on all platforms. So please
continue to state so.
Hmm. Looking at the original text:
"For compatibility with the Solaris system headers, GCC supports two
#pragma directives that change the name used in assembly for a given
declaration."
As I read that, I only saw the motivation for why this pragma was
added. Even now I don't read this as *restricting* these pragmas to
Solaris. Especially when you combine that with:
- The text below which states that redefine_extname is "currently on all
platforms."
- This pragma isn't under the "Solaris Pragmas" headings, but under its
own (apparently platform-neutral) heading.
- It does in fact work on other platforms. Even the
__PRAGMA_REDEFINE_EXTNAME macro is there.
Considering all these points, someone could easily conclude this was not
Solaris-specific. While that may not have been the intent when this
text was written, that's how I read it, and others may have too. Are
you sure you want me to make this change?
I'm not trying to pick a fight here. I'm not using the pragma, so this
doesn't affect me. It's just that this has been out there for a long
time, affects platforms other than Solaris, and I'm not immediately
clear on what problems supporting other platforms creates.||
In the end I guess I'd just like to be sure some other OS Port
Maintainer (or some distraught user) isn't going to be emailing me
demanding to know why "I" dropped support for this from their platform.
That said.
If this does need to change, I'd recommend something like the attached.
Note that since the previous patch was checked in, this patch updates
from that point. It deletes the "Symbol-Renaming Pragmas" node, and
moves (most of) the redefine_extname text into the "Solaris Pragmas" node.
If reading the patch doesn't make it clear what I've done, you can see
the end result here: http://www.LimeGreenSocks.com/gcc/Solaris-Pragmas.html
I believe this makes it crystal clear that the only platform where gcc
supports this pragma is Solaris. If you approve (and I don't hear from
some other Port Maintainer or distraught user), I'll submit this as a
new patch. Be aware, I plan to put your name in the changelog.
dw
Index: extend.texi
===
--- extend.texi (revision 210355)
+++ extend.texi (working copy)
@@ -16657,7 +16657,6 @@
* RS/6000 and PowerPC Pragmas::
* Darwin Pragmas::
* Solaris Pragmas::
-* Symbol-Renaming Pragmas::
* Structure-Packing Pragmas::
* Weak Pragmas::
* Diagnostic Pragmas::
@@ -16851,8 +16850,7 @@
@node Solaris Pragmas
@subsection Solaris Pragmas
-The Solaris target supports @code{#pragma redefine_extname}
-(@pxref{Symbol-Renaming Pragmas}). It also supports additional
+The Solaris target supports additional
@code{#pragma} directives for compatibility with the system compiler.
@table @code
@@ -16880,23 +16878,13 @@
initialization (before @code{main}) or during shared module loading, by
adding a call to the @code{.init} section.
-@end table
-
-@node Symbol-Renaming Pragmas
-@subsection Symbol-Renaming Pragmas
-
-GCC supports a @code{#pragma} directive that changes the name used in
-assembly for a given declaration. This effect can also be achieved
-using the asm labels extension (@pxref{Asm Labels}).
-
-@table @code
@item redefine_extname @var{oldname} @var{newname}
@cindex pragma, redefine_extname
This pragma gives the C function @var{oldname} the assembly symbol
@var{newname}. The preprocessor macro @code{__PRAGMA_REDEFINE_EXTNAME}
-is defined if this pragma is available (currently on all platforms).
-@end table
+is defined if this pragma is available. This effect can also be achieved
+using the asm labels extension (@pxref{Asm Labels}).
This pragma and the asm labels extension interact in a complicated
manner. Here are some corner cases you may want to be aware of:
@@ -16919,6 +16907,8 @@
always the C-language name.
@end enumerate
+@end table
+
@node Structure-Packing Pragmas
@subsection Structure-Packing Pragmas
[patch] libstdc++/60497 (again) - qualify more calls
My fix for PR 60497 didn't cover the rvalue overloads of std::get.
I also noticed we don't handle comparisons properly if tuple elements
have perverse overloaded comparison operators.
Tested x86_64-linux, committed to trunk.
commit 0f87ec2cdf390a1234a8562e6b3aba95182a6951
Author: Jonathan Wakely
Date: Tue May 13 11:43:32 2014 +0100
PR libstdc++/60497
* include/std/tuple (get, __tuple_compare): Qualify more calls to
prevent ADL. Cast comparison results to bool.
* testsuite/20_util/tuple/60497.cc: Test accessing rvalues.
* testsuite/20_util/tuple/comparison_operators/overloaded.cc: New.
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 03d87d7..5e8766c 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -775,7 +775,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>::type
get(tuple<_Elements...>&& __t) noexcept
{ return std::forward>::type&&>(get<__i>(__t)); }
+ tuple<_Elements...>>::type&&>(std::get<__i>(__t)); }
#if __cplusplus > 201103L
template
@@ -815,16 +815,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static constexpr bool
__eq(const _Tp& __t, const _Up& __u)
{
- return (get<__i>(__t) == get<__i>(__u) &&
- __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__eq(__t, __u));
+ return bool(std::get<__i>(__t) == std::get<__i>(__u))
+ && __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__eq(__t, __u);
}
static constexpr bool
__less(const _Tp& __t, const _Up& __u)
{
- return ((get<__i>(__t) < get<__i>(__u))
- || !(get<__i>(__u) < get<__i>(__t)) &&
- __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__less(__t, __u));
+ return bool(std::get<__i>(__t) < std::get<__i>(__u))
+ || !bool(std::get<__i>(__u) < std::get<__i>(__t))
+ && __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__less(__t, __u);
}
};
diff --git a/libstdc++-v3/testsuite/20_util/tuple/60497.cc b/libstdc++-v3/testsuite/20_util/tuple/60497.cc
index 76d4223..40d053b 100644
--- a/libstdc++-v3/testsuite/20_util/tuple/60497.cc
+++ b/libstdc++-v3/testsuite/20_util/tuple/60497.cc
@@ -35,3 +35,9 @@ auto a = std::get<0>(t);
auto b = std::get<0>(ct);
auto c = std::get(t);
auto d = std::get(ct);
+
+// same again for rvalues
+auto e = std::get<0>(std::move(t));
+auto f = std::get<0>(std::move(ct));
+auto g = std::get(std::move(t));
+auto h = std::get(std::move(ct));
diff --git a/libstdc++-v3/testsuite/20_util/tuple/comparison_operators/overloaded.cc b/libstdc++-v3/testsuite/20_util/tuple/comparison_operators/overloaded.cc
new file mode 100644
index 000..0a343ae
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/tuple/comparison_operators/overloaded.cc
@@ -0,0 +1,52 @@
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: [DOC Patch] symbol rename pragmas
Hi David, > Thank you for taking the time to review this. > >>> the current text makes reference to "compatibility with the Solaris system >>> headers," the remaining pragma is (according to the existing text) >>> "currently on all platforms." This makes referring to Solaris both >>> superfluous and potentially confusing. >> I disagree: #pragma redefine_extname does exist for Solaris >> compatibility only, even if it now works on all platforms. So please >> continue to state so. > > Hmm. Looking at the original text: > > "For compatibility with the Solaris system headers, GCC supports two > #pragma directives that change the name used in assembly for a given > declaration." > > As I read that, I only saw the motivation for why this pragma was added. right, and that's why I want to keep this info. If it weren't for Solaris compatibility, this pragma wouldn't exist, and given that heritage, I don't want to encourage its use elsewhere, even though it does work. > Even now I don't read this as *restricting* these pragmas to Solaris. > Especially when you combine that with: > > - The text below which states that redefine_extname is "currently on all > platforms." > - This pragma isn't under the "Solaris Pragmas" headings, but under its own > (apparently platform-neutral) heading. > - It does in fact work on other platforms. Even the > __PRAGMA_REDEFINE_EXTNAME macro is there. I've never claimed otherwise. > Considering all these points, someone could easily conclude this was not > Solaris-specific. While that may not have been the intent when this text > was written, that's how I read it, and others may have too. Are you sure > you want me to make this change? I am, for the reasons stated above. > I'm not trying to pick a fight here. I'm not using the pragma, so this > doesn't affect me. It's just that this has been out there for a long time, > affects platforms other than Solaris, and I'm not immediately clear on what > problems supporting other platforms creates.|| > > In the end I guess I'd just like to be sure some other OS Port Maintainer > (or some distraught user) isn't going to be emailing me demanding to know > why "I" dropped support for this from their platform. > > That said. > > If this does need to change, I'd recommend something like the attached. > Note that since the previous patch was checked in, this patch updates from > that point. It deletes the "Symbol-Renaming Pragmas" node, and moves (most > of) the redefine_extname text into the "Solaris Pragmas" node. > > If reading the patch doesn't make it clear what I've done, you can see the > end result here: http://www.LimeGreenSocks.com/gcc/Solaris-Pragmas.html > > I believe this makes it crystal clear that the only platform where gcc > supports this pragma is Solaris. If you approve (and I don't hear from > some other Port Maintainer or distraught user), I'll submit this as a new > patch. Be aware, I plan to put your name in the changelog. You misunderstood completely: I'm not at all demanding to remove support for that pragma in the cross-platform part of the docs or even in the code, just ask to keep documenting the heritage so it's clear why it exists and when to use it (or not). Just restoring the first sentence you deleted describing the heritage should be enough to do so. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
[testsuite] Centralise clearing hardware capabilities with Sun ld
Prompted by the recent failures of c-c++-common/gomp/pr60823-2.c on
Solaris/x86 with Sun as
http://gcc.gnu.org/ml/gcc-patches/2014-05/msg00943.html
I've reworked the clearing of hardware capabilities via linker maps for
Sun ld, which is currently replicated in several places all over the
testsuite. I've chosen to use TEST_ALWAYS_FLAGS or ALWAYS_CXXFLAGS to
pass on the necessary linker flags.
Initial testing on i386-pc-solaris2.11 (just the affected .exp files)
was successful, full bootstraps on i386-pc-solaris2.10 (as/ld, gas/gld),
sparc-sun-solaris2.11, and x86_64-unknown-linux-gnu in progress.
Will commit to mainline once testing completed, backport to the 4.9
branch which is also affected later on.
Rainer
2014-05-13 Rainer Orth
* lib/clearcap.exp: New file.
* gcc.dg/gomp/gomp.exp: Load clearcap.exp.
Call clearcap-init, clearcap-finish.
* g++.dg/gomp/gomp.exp: Likewise
* gcc.dg/vect/vect.exp: Load clearcap.exp.
Remove clearcap_ldflags handling.
Call clearcap-init, clearcap-finish.
* gcc.target/i386/i386.exp: Likewise.
* gcc.target/x86_64/abi/avx/abi-avx.exp: Likewise.
* gcc.target/x86_64/abi/avx512f/abi-avx512f.exp: Likewise.
# HG changeset patch
# Parent 5d86fdfa234897877ba6e3a30076efeb52220a53
Centralise clearing hardware capabilities with Sun ld
diff --git a/gcc/testsuite/g++.dg/gomp/gomp.exp b/gcc/testsuite/g++.dg/gomp/gomp.exp
--- a/gcc/testsuite/g++.dg/gomp/gomp.exp
+++ b/gcc/testsuite/g++.dg/gomp/gomp.exp
@@ -18,6 +18,7 @@
# Load support procs.
load_lib g++-dg.exp
+load_lib clearcap.exp
if ![check_effective_target_fopenmp] {
return
@@ -25,6 +26,7 @@ if ![check_effective_target_fopenmp] {
# Initialize `dg'.
dg-init
+clearcap-init
# Main loop.
g++-dg-runtest [lsort [concat \
@@ -32,4 +34,5 @@ g++-dg-runtest [lsort [concat \
[find $srcdir/c-c++-common/gomp *.c]]] "-fopenmp"
# All done.
+clearcap-finish
dg-finish
diff --git a/gcc/testsuite/gcc.dg/gomp/gomp.exp b/gcc/testsuite/gcc.dg/gomp/gomp.exp
--- a/gcc/testsuite/gcc.dg/gomp/gomp.exp
+++ b/gcc/testsuite/gcc.dg/gomp/gomp.exp
@@ -20,6 +20,7 @@
# Load support procs.
load_lib gcc-dg.exp
+load_lib clearcap.exp
if ![check_effective_target_fopenmp] {
return
@@ -27,6 +28,7 @@ if ![check_effective_target_fopenmp] {
# Initialize `dg'.
dg-init
+clearcap-init
# Main loop.
dg-runtest [lsort [concat \
@@ -34,4 +36,5 @@ dg-runtest [lsort [concat \
[find $srcdir/c-c++-common/gomp *.c]]] "" "-fopenmp"
# All done.
+clearcap-finish
dg-finish
diff --git a/gcc/testsuite/gcc.dg/vect/vect.exp b/gcc/testsuite/gcc.dg/vect/vect.exp
--- a/gcc/testsuite/gcc.dg/vect/vect.exp
+++ b/gcc/testsuite/gcc.dg/vect/vect.exp
@@ -18,6 +18,7 @@
# Load support procs.
load_lib gcc-dg.exp
+load_lib clearcap.exp
# Set up flags used for tests that don't specify options.
global DEFAULT_VECTCFLAGS
@@ -41,30 +42,9 @@ if ![check_vect_support_and_set_flags] {
# These flags are used for all targets.
lappend DEFAULT_VECTCFLAGS "-ftree-vectorize" "-fno-vect-cost-model" "-fno-common"
-# If the linker used understands -M , pass it to clear hardware
-# capabilities set by the Sun assembler.
-# Try mapfile syntax v2 first which is the only way to clear hwcap_2 flags.
-set clearcap_ldflags "-Wl,-M,$srcdir/gcc.target/i386/clearcapv2.map"
-
-if ![check_no_compiler_messages mapfilev2 executable {
-int main (void) { return 0; }
-} $clearcap_ldflags ] {
-# If this doesn't work, fall back to the less capable v1 syntax.
-set clearcap_ldflags "-Wl,-M,$srcdir/gcc.target/i386/clearcap.map"
-
-if ![check_no_compiler_messages mapfile executable {
- int main (void) { return 0; }
-} $clearcap_ldflags ] {
- unset clearcap_ldflags
-}
-}
-
-if [info exists clearcap_ldflags] {
-lappend DEFAULT_VECTCFLAGS $clearcap_ldflags
-}
-
# Initialize `dg'.
dg-init
+clearcap-init
global VEC_FLAGS
set VEC_FLAGS $DEFAULT_VECTCFLAGS
@@ -308,4 +288,5 @@ dg-runtest [lsort [glob -nocomplain $src
set dg-do-what-default ${save-dg-do-what-default}
# All done.
+clearcap-finish
dg-finish
diff --git a/gcc/testsuite/gcc.target/i386/i386.exp b/gcc/testsuite/gcc.target/i386/i386.exp
--- a/gcc/testsuite/gcc.target/i386/i386.exp
+++ b/gcc/testsuite/gcc.target/i386/i386.exp
@@ -23,6 +23,7 @@ if { ![istarget i?86*-*-*] && ![istarget
# Load support procs.
load_lib gcc-dg.exp
+load_lib clearcap.exp
# Return 1 if attribute ms_hook_prologue is supported.
proc check_effective_target_ms_hook_prologue { } {
@@ -307,39 +308,6 @@ proc check_effective_target_sha { } {
} "-O2 -msha" ]
}
-# If the linker used understands -M , pass it to clear hardware
-# capabilities set by the Sun assembler.
-# Try mapfile syntax v2 first which is the only way to clear hwcap_2 flags.
-set clearcap_ldflags "-Wl,-M,$srcdir/$subdir/clearcapv2.map"
-
-if ![check_no_compiler_messages mapfilev2 executable {
-int main (void) { return 0; }
-} $clearcap_ldflags ] {
Re: [PATCH 0/3] Compile-time gimple checking v4
Hi, On Mon, 12 May 2014, David Malcolm wrote: > The "gfoo" type names are pleasantly terse, though I'm a little unhappy > about how they no longer match the prefix of the accessor functions e.g. > gimple_switch_num_labels (const gswitch *gs) > vs > gimple_switch_num_labels (const gimple_switch *gs) > But it works. That could also be changed with a followup to make it consistent again (i.e. rename the accessors to gswitch_num_labels). I'd be in favor of such renaming later. Ciao, Michael.
Re: [Patch, avr] Propagate -mrelax gcc driver flag to assembler
On Mon, May 12, 2014 at 01:19:37PM +0200, Georg-Johann Lay wrote:
> Am 04/18/2014 11:52 AM, schrieb Senthil Kumar Selvaraj:
> >
> >On Sat, Apr 12, 2014 at 06:36:01PM +0200, Georg-Johann Lay wrote:
> >>Senthil Kumar Selvaraj schrieb:
> >>>This patch modifies AVR target's ASM spec to pass -mlink-relax to the
> >>>assembler if -mrelax is passed to the compiler driver. This was already
> >>>being passed on to the linker, this patch merely makes the assembler
> >>>also aware of it.
> >>>
> >>>The corresponding patch in binutils to handle the -mlink-relax patch is
> >>>already committed in the binutils repo. I'm not sure how to manage a
> >>>running a newer gcc with an older version of binutils though - how is this
> >>>generally handled?
> >>
> >>The right place is gcc/configure.ac and have a macro defined depending on
> >>whether gas supports -mlink-relax.
> >>
> >>
> >>Same should be done for -mrmw, IMO, for similar reasons, e.g. something like
> >>
> >>case "$target" in
> >> ...
> >> avr-*-*)
> >> ...
> >> gcc_GAS_CHECK_FEATURE([-mrmw option], gcc_cv_as_avr_mrmw,,
> >> [-mrmw], [.text],,
> >> [AC_DEFINE(HAVE_AS_AVR_MRMW_OPTION, 1,
> >>[Define if your assembler supports -mrmw option.])])
> >>
> >>or
> >>
> >> gcc_GAS_CHECK_FEATURE([-mrmw option], gcc_cv_as_avr_mrmw,,
> >> [-mrmw], [.text],,,)
> >> if test x$gcc_cv_as_avr_mrmw = xyes; then
> >> AC_DEFINE(HAVE_AS_AVR_MRMW_OPTION, 1,
> >> [Define if your assembler supports the -mrmw option.])
> >>
> >
> >Thanks Johann. The below patch adds the configury check for -mlink-relax,
> >along with the change to ASM_SPEC to propagate the -mrelax flag. I
> >modified the original patch a bit to make it easier to handle
> >conditional additions to SPECs (inspired by the sparc target).
> >
> >>
> >>However, the gcc-4_9-branch has already been created...
> >
> >Yes, but I figured it would be useful anyway - if this eventually gets
> >backported to 4_9, for example.
> >
> >If the below patch looks ok, could someone commit please? I don't have
> >commit access.
> >
> >Regards
> >Senthil
> >
> >gcc/ChangeLog
> >
> >2014-04-18 Senthil Kumar Selvaraj
> >
> > * config/avr/avr.h: Pass on mlink-relax to assembler.
> > * configure.ac: Test for mlink-relax support in assembler.
> > * configure: Regenerate.
> >
> >diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h
> >index 78434ec..b4e3eb1 100644
> >--- gcc/config/avr/avr.h
> >+++ gcc/config/avr/avr.h
> >@@ -512,7 +512,28 @@ extern const char *avr_device_to_sp8 (int argc, const
> >char **argv);
> > %{!fenforce-eh-specs:-fno-enforce-eh-specs} \
> > %{!fexceptions:-fno-exceptions}"
> >
> >-#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*}) "
> >+#ifdef HAVE_AS_RELAX_OPTION
> >+#define ASM_RELAX_SPEC "%{mrelax:-mlink-relax}"
> >+#else
> >+#define ASM_RELAX_SPEC ""
> >+#endif
> >+
> >+#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*})\
> >+%(asm_relax)"
> >+
> >+/* This macro defines names of additional specifications to put in the specs
> >+ that can be used in various specifications like CC1_SPEC. Its definition
> >+ is an initializer with a subgrouping for each command option.
> >+
> >+ Each subgrouping contains a string constant, that defines the
> >+ specification name, and a string constant that used by the GCC driver
> >+ program.
> >+
> >+ Do not define this macro if it does not need to do anything. */
> >+
> >+#define EXTRA_SPECS \
> >+ { "asm_relax",ASM_RELAX_SPEC }
> >+
>
> Hi, wouldn't it be easier to add just a line to driver-avr.c:avr_device_to_as
> ?
Well, I couldn't figure out how to do it without passing in the nested spec and
then do argument checking inside avr_device_to_as. Something like
#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*} %{mrelax:-mlink-relax})"
and then handle argc==0, 1 and 2 cases by strcmp'ing against
-mlink-relax if HAVE_AVR_AS_LINK_RELAX_OPTION.
Did I miss something?
>
> >
> > #define LINK_SPEC "\
> > %{mrelax:--relax\
> >diff --git gcc/configure gcc/configure
> >index bfb1525..7815038 100755
> >--- gcc/configure
> >+++ gcc/configure
> >@@ -24142,6 +24142,39 @@ $as_echo "#define HAVE_AS_JSRDIRECT_RELOCS 1"
> >>>confdefs.h
> > fi
> > ;;
> >
> >+ avr-*-*)
> >+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
> >-mlink-relax option" >&5
> >+$as_echo_n "checking assembler for -mlink-relax option... " >&6; }
> >+if test "${gcc_cv_as_avr_relax+set}" = set; then :
> >+ $as_echo_n "(cached) " >&6
> >+else
> >+ gcc_cv_as_avr_relax=no
> >+ if test x$gcc_cv_as != x; then
> >+$as_echo '.text' > conftest.s
> >+if { ac_try='$gcc_cv_as $gcc_cv_as_flags -mlink-relax -o conftest.o
> >conftest.s >&5'
> >+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> >+ (eval $ac_try) 2>&5
> >+ ac_status=$?
> >+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> >+ test $ac_status = 0; }; }
> >+then
> >+gcc_cv_as
Re: [PATCH][ARM] Remove unused vtrn, vzip, vuzp NEON builtins
Ping. Thanks, Kyrill On 06/05/14 14:37, Kyrill Tkachov wrote: Hi all, This patch removes the NEON builtin functions for vtrn, vzip, vuzp and their associated wiring and machine descriptions. The builtins were initially used to implement the corresponding intrinsics in arm_neon.h but those have since been reimplemented using the more generic __builtin_shuffle. This allows us to clean up the NEON builtin initialisation code somewhat. Tested arm-none-eabi on qemu. Made sure that the recently added tests in simd.exp for vzip, vuzp etc. pass. Ok for trunk? Thanks, Kyrill 2014-05-06 Kyrylo Tkachov * config/arm/arm.c (neon_itype): Remove NEON_RESULTPAIR. (arm_init_neon_builtins): Remove handling of NEON_RESULTPAIR. Remove associated type declarations and initialisations. (arm_expand_neon_builtin): Likewise. (neon_emit_pair_result_insn): Delete. * config/arm/arm_neon_builtins (vtrn, vzip, vuzp): Delete. * config/arm/neon.md (neon_vtrn): Delete. (neon_vzip): Likewise. (neon_vuzp): Likewise.
Re: [PATCH, ARM] Enable shrink-wrap for apcs frame
On 25/03/14 08:13, Zhenqiang Chen wrote:
> Hi
>
> The patch enables shrink-wrap for apcs frame.
>
> Bootstrap and no make check regression in ARM, THUMB1 and THUMB2 modes.
> No make check regression with "-g/-mapcs/-marm".
> Build linux-3.14-rc7 without error.
>
> Is it OK for next stage1?
>
> Thanks!
> -Zhenqiang
>
> ChangeLog:
> 2014-03-25 Zhenqiang Chen
>
> * config/arm/arm.c (arm_option_override): Enable shrink-wrap for
> TARGET_APCS_FRAME.
> (arm_emit_multi_reg_pop): Set correct dwarf info.
> (arm_expand_epilogue_apcs_frame): Add more dwarf info.
>
> testsuite/ChangeLog:
> 2014-03-25 Zhenqiang Chen
>
> * gcc.target/arm/shrink-wrap-alloca.c: New test case.
> * gcc.target/arm/shrink-wrap-sibcall.c: New test case.
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 0240cc7..fa86942 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2811,9 +2811,6 @@ arm_option_override (void)
> generate additional returns. */
>if (optimize_function_for_size_p (cfun) && TARGET_THUMB2)
> flag_shrink_wrap = false;
> - /* TBD: Dwarf info for apcs frame is not handled yet. */
> - if (TARGET_APCS_FRAME)
> -flag_shrink_wrap = false;
>
>/* We only support -mslow-flash-data on armv7-m targets. */
>if (target_slow_flash_data
> @@ -19840,7 +19837,14 @@ arm_emit_multi_reg_pop (unsigned long
> saved_regs_mask)
> par = emit_insn (par);
>
>REG_NOTES (par) = dwarf;
> - if (!return_in_pc)
> +
> + if (!emit_update)
> +{
> + /* SP is restored from stack. So reset the frame info. */
> + RTX_FRAME_RELATED_P (par) = 1;
> + add_reg_note (par, REG_CFA_DEF_CFA, stack_pointer_rtx);
> +}
> + else if (!return_in_pc)
> arm_add_cfa_adjust_cfa_note (par, UNITS_PER_WORD * num_regs,
> stack_pointer_rtx, stack_pointer_rtx);
> }
> @@ -27226,6 +27230,9 @@ arm_expand_epilogue_apcs_frame (bool really_return)
>REG_NOTES (insn) = alloc_reg_note (REG_CFA_RESTORE,
> gen_rtx_REG (SImode, IP_REGNUM),
> NULL_RTX);
> + arm_add_cfa_adjust_cfa_note (insn, UNITS_PER_WORD,
> + stack_pointer_rtx,
> + stack_pointer_rtx);
This can't be related to $SUBJECT, surely? Shrink-wrapping an interrupt
routine?
If this is as I think, please resubmit that part as a separate patch.
The other changes look ok.
R.
> }
>
>if (!really_return || (saved_regs_mask & (1 << PC_REGNUM)))
> diff --git a/gcc/testsuite/gcc.target/arm/shrink-wrap-alloca.c
> b/gcc/testsuite/gcc.target/arm/shrink-wrap-alloca.c
> new file mode 100644
> index 000..318240b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/shrink-wrap-alloca.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g -mapcs " } */
> +
> +int *p;
> +
> +void
> +test (int a)
> +{
> + if (a > 0)
> +p = __builtin_alloca (4);
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/shrink-wrap-sibcall.c
> b/gcc/testsuite/gcc.target/arm/shrink-wrap-sibcall.c
> new file mode 100644
> index 000..2efe5d0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/shrink-wrap-sibcall.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g -mapcs " } */
> +
> +unsigned char a, b, d, f, g;
> +
> +int test (void);
> +
> +int
> +baz (int c)
> +{
> + if (c == 0) return test ();
> + if (b & 1)
> +{
> + g = 0;
> + int e = (a & 0x0f) - (g & 0x0f);
> +
> + if (!a) b |= 0x80;
> + a = e + test ();
> + f = g/5 + a*3879 + b *2985;
> +}
> + else
> + {
> + f = g + a*39879 + b *25;
> + }
> + return test ();
> +}
>
Re: [PATCH][ARM] Remove unused vtrn, vzip, vuzp NEON builtins
On 05/06/14 14:37, Kyrill Tkachov wrote: Hi all, This patch removes the NEON builtin functions for vtrn, vzip, vuzp and their associated wiring and machine descriptions. The builtins were initially used to implement the corresponding intrinsics in arm_neon.h but those have since been reimplemented using the more generic __builtin_shuffle. This allows us to clean up the NEON builtin initialisation code somewhat. Tested arm-none-eabi on qemu. Made sure that the recently added tests in simd.exp for vzip, vuzp etc. pass. Ok for trunk? Yes this is OK and thanks for the cleanup. Sorry about the late review. Ramana Thanks, Kyrill 2014-05-06 Kyrylo Tkachov * config/arm/arm.c (neon_itype): Remove NEON_RESULTPAIR. (arm_init_neon_builtins): Remove handling of NEON_RESULTPAIR. Remove associated type declarations and initialisations. (arm_expand_neon_builtin): Likewise. (neon_emit_pair_result_insn): Delete. * config/arm/arm_neon_builtins (vtrn, vzip, vuzp): Delete. * config/arm/neon.md (neon_vtrn): Delete. (neon_vzip): Likewise. (neon_vuzp): Likewise.
Re: [PATCH 0/3] Compile-time gimple checking v4
On Tue, May 13, 2014 at 2:37 PM, Michael Matz wrote: > Hi, > > On Mon, 12 May 2014, David Malcolm wrote: > >> The "gfoo" type names are pleasantly terse, though I'm a little unhappy >> about how they no longer match the prefix of the accessor functions e.g. >> gimple_switch_num_labels (const gswitch *gs) >> vs >> gimple_switch_num_labels (const gimple_switch *gs) >> But it works. > > That could also be changed with a followup to make it consistent again > (i.e. rename the accessors to gswitch_num_labels). I'd be in favor of > such renaming later. Yeah, or go all the way to member functions. I'd like to see this addresses as followup, together with a discussion on whether we want standalone or member functions here. Thanks, Richard. > > Ciao, > Michael.
Re: [PATCH] Add missing -fdump-* options
On Tue, May 13, 2014 at 1:39 AM, Richard Biener
wrote:
> On Fri, May 9, 2014 at 5:54 PM, Teresa Johnson wrote:
>> I discovered that the support for the documented -fdump-* options
>> "optimized", "missed", "note" and "optall" was missing. Added that and
>> fixed a minor typo in the documentation.
>>
>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk?
>
> I'm not sure they were intented for user-consumption. ISTR they
> are just an implementation detail exposed by -fopt-info-X (which is
> where they are documented).
I'm not sure what was intended originally, but they are documented as
sub options both for -fopt-info-X and for -fdump-tree-X. The typo that
I fixed was in the -fdump-tree-X option description in fact. Since
they were documented, which I noticed when I was doing something where
I wanted to be able to get these in the dump, I added the
implementation.
Note that because of the way the -fdump-tree-all flag is setup (by
taking the NOT of a few flags that are not wanted in the 'all' dump),
you already get these MSG_* flags enabled in dumps when you specify
-fdump-tree-all, and it seems like it therefore makes sense to be able
to specify getting just those by adding the documented support.
Teresa
>
> The typo fix is ok, also adding a comment before the dump flags
> definition to the above fact.
>
> David, do I remember correctly?
>
> Thanks,
> Richard.
>
>> Thanks,
>> Teresa
>>
>> 2014-05-09 Teresa Johnson
>>
>> * doc/invoke.texi: Fix typo.
>> * dumpfile.c: Add support for documented -fdump-* options
>> optimized/missed/note/optall.
>>
>> Index: doc/invoke.texi
>> ===
>> --- doc/invoke.texi (revision 210157)
>> +++ doc/invoke.texi (working copy)
>> @@ -6278,7 +6278,7 @@ passes).
>> @item missed
>> Enable showing missed optimization information (only available in certain
>> passes).
>> -@item notes
>> +@item note
>> Enable other detailed optimization information (only available in
>> certain passes).
>> @item =@var{filename}
>> Index: dumpfile.c
>> ===
>> --- dumpfile.c (revision 210157)
>> +++ dumpfile.c (working copy)
>> @@ -107,6 +107,10 @@ static const struct dump_option_value_info dump_op
>>{"nouid", TDF_NOUID},
>>{"enumerate_locals", TDF_ENUMERATE_LOCALS},
>>{"scev", TDF_SCEV},
>> + {"optimized", MSG_OPTIMIZED_LOCATIONS},
>> + {"missed", MSG_MISSED_OPTIMIZATION},
>> + {"note", MSG_NOTE},
>> + {"optall", MSG_ALL},
>>{"all", ~(TDF_RAW | TDF_SLIM | TDF_LINENO | TDF_TREE | TDF_RTL | TDF_IPA
>> | TDF_STMTADDR | TDF_GRAPH | TDF_DIAGNOSTIC | TDF_VERBOSE
>> | TDF_RHS_ONLY | TDF_NOUID | TDF_ENUMERATE_LOCALS | TDF_SCEV)},
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
[linaro/gcc-4_9-branch] Merge from gcc-4_9-branch and backports
Hi, we have merged the gcc-4_9-branch into linaro/gcc-4_9-branch up to revision 210052 as r210370. We also have backported Ada AArch64 support as r210372 and 2 other upstream contributions as r210373 and r210376. This will be part of our 2014.05 release. Thanks, Yvan
Re: [PATCH 0/3] Compile-time gimple checking v4
On Tue, 2014-05-13 at 15:10 +0200, Richard Biener wrote:
> On Tue, May 13, 2014 at 2:37 PM, Michael Matz wrote:
> > Hi,
> >
> > On Mon, 12 May 2014, David Malcolm wrote:
> >
> >> The "gfoo" type names are pleasantly terse, though I'm a little unhappy
> >> about how they no longer match the prefix of the accessor functions e.g.
> >> gimple_switch_num_labels (const gswitch *gs)
> >> vs
> >> gimple_switch_num_labels (const gimple_switch *gs)
> >> But it works.
> >
> > That could also be changed with a followup to make it consistent again
> > (i.e. rename the accessors to gswitch_num_labels). I'd be in favor of
> > such renaming later.
>
> Yeah, or go all the way to member functions.
>
> I'd like to see this addresses as followup, together with a discussion
> on whether we want standalone or member functions here.
Or both, perhaps as a transition strategy e.g. something like:
struct GTY((tag("GSS_WITH_OPS")))
gswitch : public gimple_with_ops
{
public:
unsigned num_labels () const
{
unsigned num_ops = num_ops ();
gcc_gimple_checking_assert (num_ops > 1);
return num_ops - 1;
}
};
static inline unsigned
gimple_switch_num_labels (const gswitch *gs)
{
return gs->num_labels ();
}
where the accessor function simply calls the member fn??? This
both-worlds approach might not be desirable as a end-goal, but might be
a useful tactic when creating the patches.
FWIW, if we want to do a grand renaming - which would touch most lines
in the middle-end - I'd prefer to go the whole way and have member
functions (though the gimple_ -> gswitch_ would be much easier to
automate :) )
I can have a look at making the data fields be private too, I guess.
(with judicious use of "friend", it may be possible to do that patch
without needing to make the accessors be members).
An idea I had for a method-naming convention: keep the type as a prefix
of the name, so that instead of:
(A) tree label = gimple_switch_label (switch_stmt, i);
becoming:
(B) tree label = switch_stmt->label (i);
you'd have:
(C) tree label = switch_stmt->switch_label (i);
^^^ note the prefix
which might allow us to keep most of the "grepability" of the existing
code (you can't easily find "label" *method* calls in approach (B),
whereas "switch_label" should give much better results in approach (C)).
(and perhaps omit the prefix when it's unambiguous e.g.
gimple_cond_make_false would become
simply cond_stmt->make_false () ?)
Dave
Re: Contributing new gcc targets: i386-*-dragonfly and x86-64-*-dragonfly
On 12 May 2014 18:14, Jeff Law wrote: > On 05/12/14 11:10, John Marino wrote: >> >> On 5/12/2014 18:59, Jeff Law wrote: >>> >>> On 05/09/14 01:14, John Marino wrote: 1) Patch updated online as requested 2) At this exact point in time, we probably can share the files 3) I might debate that we should share the files - that would imply reviewing the existing counterpart files for NetBSD and OpenBSD and combining when equivalent. >>> >>> Let's go ahead and keep the files separate. We can always go back and >>> combine them at a later date if we see the need. >>> >>> So with that in mind, the patch is good to go with the gnat thread stuff >>> removed. >>> >>> Do you have write access, or do you you need someone to commit the >>> change for you? >> >> >> Thanks, Jeff! >> I do not have write access, but jwakely has graciously offered to commit >> this patchset when it achieves approval, so I guess he's on deck! > > OK. It's Jon's :-) Great, I'll coordinate with John and aim to get it committed before Friday.
Ping^2 [patch, toplevel] configure nios2-elf libraries to build with -mno-gpopt
On 05/05/2014 02:32 PM, Sandra Loosemore wrote: Ping! http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01618.html And ping again -Sandra
Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope
On 05/12/2014 11:53 PM, Mike Stump wrote:
I put in one small fix:
Doing diffs in testsuite/objc.dg/ivar-visibility-4.m.~1~:
--- testsuite/objc.dg/ivar-visibility-4.m.~1~ 2014-05-12 12:04:16.0
-0700
+++ testsuite/objc.dg/ivar-visibility-4.m 2014-05-12 13:50:53.0
-0700
@@ -29,7 +29,7 @@
{
int a;
- /* someivar is public so we shoudn't get any errors here. */
+ /* someivar is public so we shouldn't get any errors here. */
a = object->someivar;
}
———
Ok, thanks for that and for all your help in general. I tried checking
out the current master branch and running objective-c and objective-c++
related tests and everything seems to be in order so hopefully that
concludes the business with this patch.
Dimitris
Re: Ping^2 [patch, toplevel] configure nios2-elf libraries to build with -mno-gpopt
> I understand that this patch, although target-specific, needs to be > approved by a global reviewer Target-specific fragments of build files are generally approved by the target maintainer. A global maintainer's approval is not required. > and then propagated to the sourceware.org binutils-gdb and newlib > repositories as well. Yup.
Re: [PATCH 0/3] Compile-time gimple checking v4
On Tue, May 13, 2014 at 3:27 PM, David Malcolm wrote:
> On Tue, 2014-05-13 at 15:10 +0200, Richard Biener wrote:
>> On Tue, May 13, 2014 at 2:37 PM, Michael Matz wrote:
>> > Hi,
>> >
>> > On Mon, 12 May 2014, David Malcolm wrote:
>> >
>> >> The "gfoo" type names are pleasantly terse, though I'm a little unhappy
>> >> about how they no longer match the prefix of the accessor functions e.g.
>> >> gimple_switch_num_labels (const gswitch *gs)
>> >> vs
>> >> gimple_switch_num_labels (const gimple_switch *gs)
>> >> But it works.
>> >
>> > That could also be changed with a followup to make it consistent again
>> > (i.e. rename the accessors to gswitch_num_labels). I'd be in favor of
>> > such renaming later.
>>
>> Yeah, or go all the way to member functions.
>>
>> I'd like to see this addresses as followup, together with a discussion
>> on whether we want standalone or member functions here.
>
> Or both, perhaps as a transition strategy e.g. something like:
>
> struct GTY((tag("GSS_WITH_OPS")))
> gswitch : public gimple_with_ops
> {
> public:
> unsigned num_labels () const
> {
> unsigned num_ops = num_ops ();
> gcc_gimple_checking_assert (num_ops > 1);
> return num_ops - 1;
> }
> };
>
> static inline unsigned
> gimple_switch_num_labels (const gswitch *gs)
> {
> return gs->num_labels ();
> }
>
> where the accessor function simply calls the member fn??? This
> both-worlds approach might not be desirable as a end-goal, but might be
> a useful tactic when creating the patches.
Well, we can then also simply rename the functions first. And not bother
touching each line twice eventually.
> FWIW, if we want to do a grand renaming - which would touch most lines
> in the middle-end - I'd prefer to go the whole way and have member
> functions (though the gimple_ -> gswitch_ would be much easier to
> automate :) )
>
> I can have a look at making the data fields be private too, I guess.
> (with judicious use of "friend", it may be possible to do that patch
> without needing to make the accessors be members).
>
> An idea I had for a method-naming convention: keep the type as a prefix
> of the name, so that instead of:
> (A) tree label = gimple_switch_label (switch_stmt, i);
> becoming:
> (B) tree label = switch_stmt->label (i);
> you'd have:
> (C) tree label = switch_stmt->switch_label (i);
> ^^^ note the prefix
> which might allow us to keep most of the "grepability" of the existing
> code (you can't easily find "label" *method* calls in approach (B),
> whereas "switch_label" should give much better results in approach (C)).
Eh :/ But then we can as well keep the function as gswitch_label
(switch_stmt, i).
C++ isn't really a good greppable language. But well. We voted for it.
Richard.
> (and perhaps omit the prefix when it's unambiguous e.g.
> gimple_cond_make_false would become
> simply cond_stmt->make_false () ?)
>
> Dave
>
>
Re: [PATCH] Add missing -fdump-* options
On Tue, May 13, 2014 at 1:39 AM, Richard Biener
wrote:
> On Fri, May 9, 2014 at 5:54 PM, Teresa Johnson wrote:
>> I discovered that the support for the documented -fdump-* options
>> "optimized", "missed", "note" and "optall" was missing. Added that and
>> fixed a minor typo in the documentation.
>>
>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk?
>
> I'm not sure they were intented for user-consumption. ISTR they
> are just an implementation detail exposed by -fopt-info-X (which is
> where they are documented).
>
> The typo fix is ok, also adding a comment before the dump flags
> definition to the above fact.
>
> David, do I remember correctly?
I remember we talked about content filtering dump flags. Things like
-fdump-xxx-ir <-- dump IR only
-fdump-xxx-transformation <-- optimization note
-fdump-xxx-debug <-- other debug traces
Other than that, now I think 'details' and 'all' seem redundant.
'verbose' flag/modifier can achieve the same effect depending on the
context.
-fdump-xxx-ir-verbose <-- dump IR, and turn on IR modifiers such as
vops, lineno, etc
-fdump-xxx-transforamtion-verbose <-- dump transformations + missed
optimizations + notes
-fdump-xxx-debug-verbose <-- turn on detailed trace.
thanks,
David
>
> Thanks,
> Richard.
>
>> Thanks,
>> Teresa
>>
>> 2014-05-09 Teresa Johnson
>>
>> * doc/invoke.texi: Fix typo.
>> * dumpfile.c: Add support for documented -fdump-* options
>> optimized/missed/note/optall.
>>
>> Index: doc/invoke.texi
>> ===
>> --- doc/invoke.texi (revision 210157)
>> +++ doc/invoke.texi (working copy)
>> @@ -6278,7 +6278,7 @@ passes).
>> @item missed
>> Enable showing missed optimization information (only available in certain
>> passes).
>> -@item notes
>> +@item note
>> Enable other detailed optimization information (only available in
>> certain passes).
>> @item =@var{filename}
>> Index: dumpfile.c
>> ===
>> --- dumpfile.c (revision 210157)
>> +++ dumpfile.c (working copy)
>> @@ -107,6 +107,10 @@ static const struct dump_option_value_info dump_op
>>{"nouid", TDF_NOUID},
>>{"enumerate_locals", TDF_ENUMERATE_LOCALS},
>>{"scev", TDF_SCEV},
>> + {"optimized", MSG_OPTIMIZED_LOCATIONS},
>> + {"missed", MSG_MISSED_OPTIMIZATION},
>> + {"note", MSG_NOTE},
>> + {"optall", MSG_ALL},
>>{"all", ~(TDF_RAW | TDF_SLIM | TDF_LINENO | TDF_TREE | TDF_RTL | TDF_IPA
>> | TDF_STMTADDR | TDF_GRAPH | TDF_DIAGNOSTIC | TDF_VERBOSE
>> | TDF_RHS_ONLY | TDF_NOUID | TDF_ENUMERATE_LOCALS | TDF_SCEV)},
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [PATCH] gengtype: Support explicit pointers in template arguments
On Mon, 2014-05-05 at 10:18 -0600, Jeff Law wrote:
> On 04/30/14 21:07, David Malcolm wrote:
> > Currently, gengtype does not support template arguments that are
> > explicitly pointers, such as:
> >static GTY(()) vec test_gimple; giving this
> > error:
> >../../src/gcc/gimple-expr.c:902: parse error: expected a string constant
> > or ',', have '*' instead
> > requiring them to be typedefs.
> >
> > This patch removes this restriction, supporting up to a single pointer
> > in each template argument.
> >
> > It also adds support for inheritance, allowing:
> >static GTY(()) vec *test_phis;
> > where the generated gt-FOO.c file contains:
> >
> >void
> >gt_ggc_mx (struct gimple_statement_phi *& x)
> >{
> > if (x)
> >gt_ggc_mx_gimple_statement_base ((void *) x);
> >}
> >
> > i.e. handling the subclass by calling the marking function for the base
> > class.
> >
> > Doing so uncovered an issue within write_user_func_for_structure_ptr,
> > which would unconditionally write out a func. This would lead to
> > gt-FOO.c containing duplicate functions e.g.:
> > gtype-desc.c:280: multiple definition of
> > `gt_ggc_mx(gimple_statement_base*&)'
> > if more than one GTY template type referenced such a type; for example like
> > this:
> >
> >static GTY(()) vec *test_old_style_gimple;
> >static GTY(()) vec
> > *test_new_style_gimple;
> >
> > where "gimple" is just an alias for "gimple_statement_base *". This
> > could be worked around by ensuring that the source tree consistently
> > uses vec<> of just one kind, but for robustness the patch also makes
> > write_user_func_for_structure_ptr idempotent, so it only writes out the
> > func once for each (struct, which-of-ggc/pch) combination.
> >
> > Motivation:
> > In the "Compile-time gimple-checking" discussion, Richi asked me to look
> > at eliminating the const_gimple typedef in favor of having "gimple" be
> > the struct, thus making the pointer to a gimple be explicit in the type:
> >http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html
> > as in:
> >http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01632.html
> >> [...] And I never liked the
> >> const_ typedef variants that were introduced. The main reason for
> >> them was probably to avoid all the churn of replacing the tree pointer
> >> typedef with a tree (non-pointer) typedef. The mistake was to
> >> repeat that for 'gimple' ...
> >
> > I'm attempting a patch for that, but in the meantime, this patch adds
> > the needed support to gengtype.
> >
> > Successfully bootstrapped®tested on x86_64-unknown-linux-gnu (Fedora
> > 20).
> >
> > OK for trunk?
> Basically OK. Per the wide-int folks request, please hold off until the
> wide-int merge is done.
>
> If after wide-int, the only changes are trivial, then consider the patch
> pre-approved (post the final version here so that its archived).
>
> If after wide-int merges the changes to this patch are non-trivial, then
> repost the RFA.
>
> Obviously, you get to exercise some judgement as to what constitutes
> trivial vs non-trivial.
Thanks.
The changes required after the wide-int merge were trivial.
Successfully rebootstrapped®rtested the patch, both individually, and
followed up with v4 of the proposed gimple class patch series from:
http://gcc.gnu.org/ml/gcc-patches/2014-05/msg00887.html
in both cases with equal testcase results compared to a control build of
r210222.
Committed to trunk as r210379; attached for reference.
Index: gcc/gengtype.c
===
--- gcc/gengtype.c (revision 210378)
+++ gcc/gengtype.c (revision 210379)
@@ -145,6 +145,14 @@
s = s->u.s.base_class;
return s;
}
+
+static type_p
+get_ultimate_base_class (type_p s)
+{
+ while (s->u.s.base_class)
+s = s->u.s.base_class;
+ return s;
+}
/* Input file handling. */
@@ -589,7 +597,7 @@
/* We only accept simple template declarations (see
require_template_declaration), so we only need to parse a
comma-separated list of strings, implicitly assumed to
- be type names. */
+ be type names, potentially with "*" characters. */
char *arg = open_bracket + 1;
char *type_id = strtok (arg, ",>");
pair_p fields = 0;
@@ -597,8 +605,28 @@
{
/* Create a new field for every type found inside the template
parameter list. */
- const char *field_name = xstrdup (type_id);
- type_p arg_type = resolve_typedef (field_name, pos);
+
+ /* Support a single trailing "*" character. */
+ const char *star = strchr (type_id, '*');
+ int is_ptr = (star != NULL);
+ size_t offset_to_star = star - type_id;
+ if (is_ptr)
+ offset_to_star = star - type_id;
+
+ char *field_name = xstrdup (type_id);
+
+ type_p arg_type;
+ if (is_ptr)
+ {
+ /* Strip off the first '*' character (and any subsequent text). */
+ *(field_name + offset_to_star) = '\0';
+
+ arg_type = find_structure (field_name, TYPE_STRUCT);
[PATCH] Install sanitizer public headers (fix for PR sanitizer/61100)
Hi,
Asan and Tsan allow sanitized applications to tweak runtime behavior via
API defined in headers in libsanitizer/include/sanitizer. This patch
adds installation code for these headers and a small test.
Bootstrapped and regtested on x64.
-Y
libsanitizer/ChangeLog:
2014-05-13 Yury Gribov
PR sanitizer/61100
* Makefile.am (nodist_saninclude_HEADERS): Install
public headers.
* Makefile.in: Regenerate.
gcc/testsuite/ChangeLog:
2014-05-13 Yury Gribov
PR sanitizer/61100
* c-c++-common/asan/asan-interface-1.c: New test.
* lib/asan-dg.exp (asan_include_flags): New function.
(asan_init): Call asan_include_flags to obtain path
to sanitizer headers.
diff --git a/gcc/testsuite/c-c++-common/asan/asan-interface-1.c b/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
new file mode 100644
index 000..68f763d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/asan-interface-1.c
@@ -0,0 +1,13 @@
+/* Check that interface headers work. */
+
+/* { dg-do run { target { *-*-linux* } } } */
+
+#include
+
+int main() {
+ char tmp;
+ if (__asan_address_is_poisoned((volatile char *)&tmp + 1))
+return 0;
+ return 1;
+}
+
diff --git a/gcc/testsuite/lib/asan-dg.exp b/gcc/testsuite/lib/asan-dg.exp
index 9ba39db..7a12160 100644
--- a/gcc/testsuite/lib/asan-dg.exp
+++ b/gcc/testsuite/lib/asan-dg.exp
@@ -23,6 +23,21 @@ proc check_effective_target_faddress_sanitizer {} {
} "-fsanitize=address"]
}
+proc asan_include_flags {} {
+global srcdir
+global TESTING_IN_BUILD_TREE
+
+set flags ""
+
+if { [is_remote host] || ! [info exists TESTING_IN_BUILD_TREE] } {
+ return "${flags}"
+}
+
+set flags "-I$srcdir/../../libsanitizer/include"
+
+return "$flags"
+}
+
#
# asan_link_flags -- compute library path and flags to find libasan.
# (originally from g++.exp)
@@ -80,17 +95,19 @@ proc asan_init { args } {
}
}
+set include_flags "[asan_include_flags]"
+
if [info exists TEST_ALWAYS_FLAGS] {
set asan_saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
}
if [info exists ALWAYS_CXXFLAGS] {
set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
- set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=address -g}" $ALWAYS_CXXFLAGS]
+ set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=address -g $include_flags}" $ALWAYS_CXXFLAGS]
} else {
if [info exists TEST_ALWAYS_FLAGS] {
- set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=address -g $TEST_ALWAYS_FLAGS"
+ set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=address -g $include_flags $TEST_ALWAYS_FLAGS"
} else {
- set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=address -g"
+ set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=address -g $include_flags"
}
}
if { $link_flags != "" } {
diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index b0dc582..6b0c571 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -1,7 +1,13 @@
ACLOCAL_AMFLAGS = -I .. -I ../config
+sanincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/include/sanitizer
+
+nodist_saninclude_HEADERS =
+
if SANITIZER_SUPPORTED
SUBDIRS = sanitizer_common
+nodist_saninclude_HEADERS += \
+ include/sanitizer/common_interface_defs.h
if !USING_MAC_INTERPOSE
SUBDIRS += interception
endif
@@ -9,6 +15,9 @@ if LIBBACKTRACE_SUPPORTED
SUBDIRS += libbacktrace
endif
SUBDIRS += lsan asan ubsan
+nodist_saninclude_HEADERS += \
+ include/sanitizer/lsan_interface.h \
+ include/sanitizer/asan_interface.h
if TSAN_SUPPORTED
SUBDIRS += tsan
endif
diff --git a/libsanitizer/Makefile.in b/libsanitizer/Makefile.in
index 60cbe2e..0b89245 100644
--- a/libsanitizer/Makefile.in
+++ b/libsanitizer/Makefile.in
@@ -35,9 +35,12 @@ POST_UNINSTALL = :
build_triplet = @build@
host_triplet = @host@
target_triplet = @target@
-@SANITIZER_SUPPORTED_TRUE@@USING_MAC_INTERPOSE_FALSE@am__append_1 = interception
-@LIBBACKTRACE_SUPPORTED_TRUE@@SANITIZER_SUPPORTED_TRUE@am__append_2 = libbacktrace
-@SANITIZER_SUPPORTED_TRUE@@TSAN_SUPPORTED_TRUE@am__append_3 = tsan
+@SANITIZER_SUPPORTED_TRUE@am__append_1 = include/sanitizer/common_interface_defs.h \
+@SANITIZER_SUPPORTED_TRUE@ include/sanitizer/lsan_interface.h \
+@SANITIZER_SUPPORTED_TRUE@ include/sanitizer/asan_interface.h
+@SANITIZER_SUPPORTED_TRUE@@USING_MAC_INTERPOSE_FALSE@am__append_2 = interception
+@LIBBACKTRACE_SUPPORTED_TRUE@@SANITIZER_SUPPORTED_TRUE@am__append_3 = libbacktrace
+@SANITIZER_SUPPORTED_TRUE@@TSAN_SUPPORTED_TRUE@am__append_4 = tsan
subdir = .
DIST_COMMON = ChangeLog $(srcdir)/Makefile.in $(srcdir)/Makefile.am \
$(top_srcdir)/configure $(am__configure_deps) \
@@ -98,8 +101,9 @@ am__nobase_list = $(am__nobase_strip_setup); \
am__base_list = \
sed '$$!N;$$!N;$$!N;$$!N;$$!N;$$!N;$$!N;s/\n/ /g' | \
sed '$$!N;$$!N;$$!N;$$!N;s/\n/ /g'
-am__installdirs = "$(DESTDIR)$(toolexeclibdir)"
-HEADERS = $(nodist_toolexeclib_HEADERS)
+am__installdirs = "$(DESTDIR)$(sanincludedir)" \
+ "$(DESTDIR)$(toolexeclibdir)"
+HEADERS = $(
[GOOGLE] Updates highest_location when updating next_discriminator_location
The previous checkin will break build for most application: http://gcc.gnu.org/viewcvs/gcc/branches/google/gcc-4_9/gcc/?view=log This patch fixes the regression by updating highest_location. Testing on-going, OK for google-4_9 branch? Thanks, Dehao Index: gcc/input.c === --- gcc/input.c (revision 210338) +++ gcc/input.c (working copy) @@ -910,6 +910,8 @@ location_with_discriminator (location_t locus, int : next_discriminator_location); next_discriminator_location++; + if (next_discriminator_location > line_table->highest_location) +line_table->highest_location = next_discriminator_location; return ret; }
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
> Index: gcc/input.c > === > --- gcc/input.c (revision 210338) > +++ gcc/input.c (working copy) > @@ -910,6 +910,8 @@ location_with_discriminator (location_t locus, int >: next_discriminator_location); > >next_discriminator_location++; > + if (next_discriminator_location > line_table->highest_location) > +line_table->highest_location = next_discriminator_location; >return ret; > } It seems to me that if something is breaking because highest_location isn't getting updated, there's a deeper problem. The discriminator handling depends on the fact that the line table doesn't ever add any new values after min_discriminator_location is set, and the routines in libcpp shouldn't ever see a location_t value above min_discriminator_location (it should be converted to a "real" location_t via map_discriminator_location()). If libcpp now assigns new location_t values after we start handing out discriminators, then we'll probably need to have libcpp start handling discriminator values (which is the solution for LTO as well). If you're just seeing leakage of discriminator locations into libcpp, there's probably a spot where we need to call map_discriminator_location(). -cary
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
The problem is that linemap_location_from_macro_expansion_p will always return true if locus has discriminator. And in linemap_lookup, this will lead to call linemap_macro_map_lookup, in which there is an assertion: linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set)); However, line is actually not a macro location. Dehao On Tue, May 13, 2014 at 9:47 AM, Cary Coutant wrote: >> Index: gcc/input.c >> === >> --- gcc/input.c (revision 210338) >> +++ gcc/input.c (working copy) >> @@ -910,6 +910,8 @@ location_with_discriminator (location_t locus, int >>: next_discriminator_location); >> >>next_discriminator_location++; >> + if (next_discriminator_location > line_table->highest_location) >> +line_table->highest_location = next_discriminator_location; >>return ret; >> } > > It seems to me that if something is breaking because highest_location > isn't getting updated, there's a deeper problem. The discriminator > handling depends on the fact that the line table doesn't ever add any > new values after min_discriminator_location is set, and the routines > in libcpp shouldn't ever see a location_t value above > min_discriminator_location (it should be converted to a "real" > location_t via map_discriminator_location()). If libcpp now assigns > new location_t values after we start handing out discriminators, then > we'll probably need to have libcpp start handling discriminator values > (which is the solution for LTO as well). If you're just seeing leakage > of discriminator locations into libcpp, there's probably a spot where > we need to call map_discriminator_location(). > > -cary
Re: [Patch,Microblaze]: Added Break Handler Support
On 05/13/14 02:14, Ajit Kumar Agarwal wrote: Hello Michael: The following patch is to handle Software and Hardware breaks in Microblaze Architecture. Deja GNU testcase does not have any regressions and the testcase attached passes through. Review comments are incorporated. Okay for trunk? Just saying OK would only be appropriate if you had write access to the repository. Added Break Handler support to incorporate the hardware and software break. The Break Handler routine will be generating the rtbd instruction. At the call point where the software breaks are generated with the instruction brki with register operand as r16. 2014-05-13 Ajit Agarwal * config/microblaze/microblaze.c (microblaze_break_function_p,microblaze_is_break_handler) : New * config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro * config/microblaze/microblaze.md : Extended support for generation of brki instruction and rtbd instruction. A better ChangeLog entry is * config/microblaze/microblaze.md (*,_internal): Add microblaze_is_break_handler () test. Give specifics, naming functions, rather than making general comments. As the ChangeLog standard says: It’s important to name the changed function or variable in full. Don’t abbreviate function or variable names, and don’t combine them. Subsequent maintainers will often search for a function name to find all the change log entries that pertain to it; if you abbreviate the name, they won’t find it when they search. Mention each place where there are changes. There should be a ChangeLog entry for each non-trivial change. Your patch made four significant changes to microblaze.md. There appear to be several changes in microblaze.c, not just the definition of the new functions as shown in your entry. * config/microblaze/microblaze-protos.h (microblaze_break_function_p,microblaze_is_break_handler) : New Declaration. * testsuite/gcc.target/microblaze/others/break_handler.c : New. Thanks for the test case. As mentioned previously, add documentation for _break_handler. diff --git a/gcc/config/microblaze/microblaze-protos.h b/gcc/config/microblaze/microblaze-protos.h index b03e9e1..f3cc099 100644 --- a/gcc/config/microblaze/microblaze-protos.h +++ b/gcc/config/microblaze/microblaze-protos.h Please include the patch only once, not both inline and again as an attachment. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
> The problem is that linemap_location_from_macro_expansion_p will > always return true if locus has discriminator. And in linemap_lookup, > this will lead to call linemap_macro_map_lookup, in which there is an > assertion: > > linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set)); > > However, line is actually not a macro location. That sounds like we're leaking a discriminator location into the linemap code. Before you can call linemap_location_from_macro_expansion_p, you need to do this (as in expand_location_1): /* If LOC describes a location with a discriminator, extract the discriminator and map it to the real location. */ if (min_discriminator_location != UNKNOWN_LOCATION && loc >= min_discriminator_location && loc < next_discriminator_location) loc = map_discriminator_location (loc); -cary
[PATCH] Implement -fsanitize=float-cast-overflow
Here's an attempt to add the -fsanitize=float-cast-overflow
instrumentation. It should issue a runtime error when a floating-point
to integer type conversion overflows. Eventually it should instrument
even floating-point to floating-point conversions to detect e.g.
(float)1e39 overflow, but I'd like to settle first on float to int
before implementing that.
Current patch should detect e.g.
unsigned char uc = 250.0;
int i = (float) __INT_MAX__ + 1;
and similar.
In essence, the gist of this instrumentation is:
if (x u<= TYPE_MIN - 1.0 || x u>= TYPE_MAX + 1.0)
__ubsan_builtin ();
this checks even +-Inf for free, and because the comparison is
unordered, it detects even NaNs.
The question is how this should interact with feenableexcept (FE_INVALID)
- currently it in some cases raises the Floating point exception, so I
didn't include that in the testsite... I don't know floating-point
stuff well enough to judge what would be best.
Regtested/bootstrapped on x86_64. Ubsan testsuite passes with
-m32/-m64 on x86_64/ppc64. Ran bootstrap-ubsan.
2014-05-13 Marek Polacek
* convert.c: Include "ubsan.h".
(convert_to_integer): Instrument float to integer conversion.
* flag-types.h (enum sanitize_code): Add SANITIZE_FLOAT_CAST and or
it into SANITIZE_UNDEFINED.
* opts.c (common_handle_option): Add -fsanitize=float-cast-overflow.
* sanitizer.def (BUILT_IN_UBSAN_HANDLE_FLOAT_CAST_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_FLOAT_CAST_OVERFLOW_ABORT): New built-ins.
* ubsan.c (get_ubsan_type_info_for_type): Handle REAL_TYPEs.
(ubsan_instrument_float_cast): New function.
* ubsan.h (ubsan_instrument_float_cast): Declare.
* doc/invoke.texi: Document -fsanitize=float-cast-overflow.
* c-c++-common/ubsan/float-cast-overflow-1.c: New test.
diff --git gcc/convert.c gcc/convert.c
index 91c1da2..de9e4b3 100644
--- gcc/convert.c
+++ gcc/convert.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic-core.h"
#include "target.h"
#include "langhooks.h"
+#include "ubsan.h"
/* Convert EXPR to some pointer or reference type TYPE.
EXPR must be pointer, reference, integer, enumeral, or literal zero;
@@ -394,6 +395,7 @@ convert_to_integer (tree type, tree expr)
tree intype = TREE_TYPE (expr);
unsigned int inprec = element_precision (intype);
unsigned int outprec = element_precision (type);
+ location_t loc = EXPR_LOCATION (expr);
/* An INTEGER_TYPE cannot be incomplete, but an ENUMERAL_TYPE can
be. Consider `enum E = { a, b = (enum E) 3 };'. */
@@ -844,7 +846,15 @@ convert_to_integer (tree type, tree expr)
return build1 (CONVERT_EXPR, type, expr);
case REAL_TYPE:
- return build1 (FIX_TRUNC_EXPR, type, expr);
+ if (flag_sanitize & SANITIZE_FLOAT_CAST)
+ {
+ expr = save_expr (expr);
+ tree check = ubsan_instrument_float_cast (loc, type, expr);
+ expr = build1 (FIX_TRUNC_EXPR, type, expr);
+ return fold_build2 (COMPOUND_EXPR, TREE_TYPE (expr), check, expr);
+ }
+ else
+ return build1 (FIX_TRUNC_EXPR, type, expr);
case FIXED_POINT_TYPE:
return build1 (FIXED_CONVERT_EXPR, type, expr);
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index da9694c..85d8a10 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5426,6 +5426,13 @@ Detect floating-point division by zero. Unlike other
similar options,
@option{-fsanitize=undefined}, since floating-point division by zero can
be a legitimate way of obtaining infinities and NaNs.
+@item -fsanitize=float-cast-overflow
+@opindex fsanitize=float-cast-overflow
+
+This option enables floating-point type to integer conversion checking.
+We check that the result of the conversion does not overflow.
+This option does not work well with @code{FE_INVALID} exceptions enabled.
+
@item -fsanitize-recover
@opindex fsanitize-recover
By default @option{-fsanitize=undefined} sanitization (and its suboptions
diff --git gcc/flag-types.h gcc/flag-types.h
index caf4039..2965734 100644
--- gcc/flag-types.h
+++ gcc/flag-types.h
@@ -229,9 +229,11 @@ enum sanitize_code {
SANITIZE_BOOL = 1 << 10,
SANITIZE_ENUM = 1 << 11,
SANITIZE_FLOAT_DIVIDE = 1 << 12,
+ SANITIZE_FLOAT_CAST = 1 << 13,
SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
| SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN
| SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM
+ | SANITIZE_FLOAT_CAST
};
/* flag_vtable_verify initialization levels. */
diff --git gcc/opts.c gcc/opts.c
index f15852d..9d38690 100644
--- gcc/opts.c
+++ gcc/opts.c
@@ -1463,6 +1463,8 @@ common_handle_option (struct gcc_options *opts,
{ "enum", SANITIZE_ENUM, sizeof "enum" - 1 },
{ "float-divide-by-zero", SANITIZE_FLOAT_DIVIDE,
sizeof "float-divide-by-zero" - 1 },
+
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On Mon, 12 May 2014, Hans-Peter Nilsson wrote: > On Thu, 24 Apr 2014, Jeff Law wrote: > > On 04/16/14 18:20, seg...@kernel.crashing.org wrote: > > > PR target/60822 > > > 2014-04-16 Segher Boessenkool > > > > > > * config/m68k/m68k.md (extendplussidi): Don't allow memory for > > > operand 1. > > Thanks. I tweaked the comment and added the testcase to the regression > > suite > > and installed the fix on the trunk. > > In the commit message: > > Added: > trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.c > trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.x > > I hope it's not too officious to remind people that dg- markup > can be used *even* in the c-torture test-suite these days; > gating in .x files is not necessary (since a few years, IIRC). I believe bug 20567 is still current: gcc.c-torture/execute ignores dg- directives and still needs .x files, until someone converts it to the dg harness. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC][PING^2] Do not consider volatile asms as optimization barriers #1
> The point of it is that the release branches are actually used by GCC users, > many people don't use just official releases, but arbitrary snapshots from > the release branches. If a potentially risky patch is applied immediately > also to release branches and soon needs follow-ups (happened many times in > the recent history), then that results in unnecessary breakage. IMO that's a rather weak point, if people use arbitrary snapshots then they can revert to the previous one; similarly, if they use the SVN tree, then they can back out the patch manually. You cannot do that for an official release. -- Eric Botcazou
Re: [RFC][PING^2] Do not consider volatile asms as optimization barriers #1
> OK, thanks. Richard also gave an RM's OK on IRC so I've now applied it. Thanks! -- Eric Botcazou
Re: [RFC][PING^2] Do not consider volatile asms as optimization barriers #1
On Tue, May 13, 2014 at 07:18:56PM +0200, Eric Botcazou wrote: > > The point of it is that the release branches are actually used by GCC users, > > many people don't use just official releases, but arbitrary snapshots from > > the release branches. If a potentially risky patch is applied immediately > > also to release branches and soon needs follow-ups (happened many times in > > the recent history), then that results in unnecessary breakage. > > IMO that's a rather weak point, if people use arbitrary snapshots then they > can revert to the previous one; similarly, if they use the SVN tree, then > they > can back out the patch manually. You cannot do that for an official release. Even with official release you can apply a patch, that is not the point. The point is that many people expect the release branches (and IMHO rightly so) to be supposedly stable all the time, rather than being seriously unstable most of the time and only converging to stability around the official releases. So, if there is something possibly risky, it is better to give it some time on the trunk rather than breaking everybody and fixing it a few days afterwards. That doesn't mean you should forget about your patch and only remember it days before the release of course. Jakub
[patch] libstdc++/60497 again - more std::get qualification
This patch ensures everywhere we use std::get we qualify it. It also
changes some uses of the built-in operator& with std::__addressof()
which doesn't do ADL and so doesn't try to complete its argument.
As noted in the bug report, the operator& part of this affects most of
the library, so I don't think it's feasible to do anything about it
right now.
Tested x86_64-linux, committed to trunk.
commit d3d2b701493ab363cc9820283bf78250f5a52d31
Author: Jonathan Wakely
Date: Tue May 13 15:35:29 2014 +0100
PR libstdc++/60497
* include/debug/array (get): Qualify call to other get overload.
* include/profile/array (get): Likewise.
* include/std/array (get): Likewise.
* include/std/functional (_Mu, _Bind, _Bind_result): Qualify std::get.
* include/std/mutex (unique_lock, call_once): Use __addressof.
(__unlock_impl): Remove unused template.
(__try_to_lock): Declare inline.
(__try_lock_impl::__do_try_lock): Qualify function calls.
(lock): Avoid narrowing conversion.
* testsuite/20_util/bind/60497.cc: New.
* testsuite/23_containers/array/element_access/60497.cc: New.
* testsuite/30_threads/call_once/60497.cc: New.
* testsuite/30_threads/unique_lock/cons/60497.cc: New.
diff --git a/libstdc++-v3/include/debug/array b/libstdc++-v3/include/debug/array
index ef01c98..2266a53 100644
--- a/libstdc++-v3/include/debug/array
+++ b/libstdc++-v3/include/debug/array
@@ -278,7 +278,7 @@ namespace __debug
get(array<_Tp, _Nm>&& __arr) noexcept
{
static_assert(_Int < _Nm, "index is out of bounds");
- return std::move(get<_Int>(__arr));
+ return std::move(__debug::get<_Int>(__arr));
}
template
diff --git a/libstdc++-v3/include/profile/array b/libstdc++-v3/include/profile/array
index 1c38b05..1a43a48 100644
--- a/libstdc++-v3/include/profile/array
+++ b/libstdc++-v3/include/profile/array
@@ -240,7 +240,7 @@ namespace __profile
get(array<_Tp, _Nm>&& __arr) noexcept
{
static_assert(_Int < _Nm, "index is out of bounds");
- return std::move(get<_Int>(__arr));
+ return std::move(__profile::get<_Int>(__arr));
}
template
diff --git a/libstdc++-v3/include/std/array b/libstdc++-v3/include/std/array
index 67680d6..22947ce 100644
--- a/libstdc++-v3/include/std/array
+++ b/libstdc++-v3/include/std/array
@@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
get(array<_Tp, _Nm>&& __arr) noexcept
{
static_assert(_Int < _Nm, "index is out of bounds");
- return std::move(get<_Int>(__arr));
+ return std::move(_GLIBCXX_STD_C::get<_Int>(__arr));
}
template
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 295022d..2bc3d8d 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -1120,7 +1120,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
const _Index_tuple<_Indexes...>&) const volatile
-> decltype(__arg(declval<_Args>()...))
{
- return __arg(std::forward<_Args>(get<_Indexes>(__tuple))...);
+ return __arg(std::forward<_Args>(std::get<_Indexes>(__tuple))...);
}
};
@@ -1261,7 +1261,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
__call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>)
{
return _M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call as const
@@ -1270,7 +1270,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
__call_c(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>) const
{
return _M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call as volatile
@@ -1393,7 +1393,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
typename __disable_if_void<_Res>::type = 0)
{
return _M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call unqualified, return void
@@ -1403,7 +1403,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
typename __enable_if_void<_Res>::type = 0)
{
_M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call as const
@@ -1413,7 +1413,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
typename __disable_if_void<_Res>::type = 0) const
{
return _M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call as const, return void
@@ -1423,7 +1423,7 @@ _GLIBCXX_HAS_NESTED_TYPE(result_type)
typename __enable_if_void<_Res>::type = 0) const
{
_M_f(_Mu<_Bound_args>()
- (get<_Indexes>(_M_bound_args), __args)...);
+ (std::get<_Indexes>(_M_bound_args), __args)...);
}
// Call as volatile
diff --git a/libstdc++-v3/include/std/mut
Re: [PATCH, PR C++/61038] - g++ -E is unusable with UDL strings
On Mon, 12 May 2014, Ed Smith-Rowland wrote:
> This patch is really a libcpp patch. But UDLs are like that ;-)
>
> Add string user-defined literals and char user-defined literals to the list of
> things to look out for while escaping strings in macro args.
>
> I'm not sure how to test this really. we want to write out *.ii files and
> verify that internal quotes are escaped.
You should be able to check the results of stringizing twice, e.g.:
extern "C" int strcmp (const char *, const char *);
extern "C" void abort (void);
extern "C" void exit (int);
void operator "" _s(const char *, unsigned long)
{
}
#define QUOTE(s) #s
#define QQUOTE(s) QUOTE(s)
const char *s = QQUOTE(QUOTE("hello"_s));
const char *t = QUOTE("\"hello\"_s");
int main()
{
if (strcmp(s, t) == 0)
exit(0);
else
abort();
}
(at least, this fails for me with unmodified GCC, and I think it should
pass).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [C PATCH] Better locinfo for -Wc++-compat (PR c/61162)
On Tue, 13 May 2014, Marek Polacek wrote: > Tiny patch, just use available location instead of input_location. > > Tested x86_64. Ok? > > 2014-05-13 Marek Polacek > > PR c/61162 > * c-typeck.c (convert_for_assignment): Pass location to > WARN_FOR_ASSIGNMENT instead of input_location. > > * gcc.dg/pr61162.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Make attributes accept enum values (PR c/50459)
On Tue, 13 May 2014, Marek Polacek wrote: > Yeah, I should've done that in the first place, sorry. Is the > following ok then? > > 2014-05-13 Marek Polacek > > * c-c++-common/pr50459.c: Move cdtor tests to a separate testcase. > * c-c++-common/pr50459-2.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC][PING^2] Do not consider volatile asms as optimization barriers #1
> Even with official release you can apply a patch, that is not the point. > The point is that many people expect the release branches (and IMHO rightly > so) to be supposedly stable all the time, rather than being seriously > unstable most of the time and only converging to stability around the > official releases. Possibly, but with our scheme it's the opposite: it's stable most of the time and series of patches are applied just before the releases... -- Eric Botcazou
Re: [PATCH] Implement -fsanitize=float-cast-overflow
On Tue, May 13, 2014 at 07:08:01PM +0200, Marek Polacek wrote:
> In essence, the gist of this instrumentation is:
> if (x u<= TYPE_MIN - 1.0 || x u>= TYPE_MAX + 1.0)
> __ubsan_builtin ();
> this checks even +-Inf for free, and because the comparison is
> unordered, it detects even NaNs.
>
> The question is how this should interact with feenableexcept (FE_INVALID)
> - currently it in some cases raises the Floating point exception, so I
> didn't include that in the testsite... I don't know floating-point
> stuff well enough to judge what would be best.
What kind of exceptions do you get and where? For sNaN I guess an exception
should be expectable, not sure if we raise one when casting sNaN to integer
without instrumentation.
> --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c
> +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c
> @@ -0,0 +1,244 @@
> +/* { dg-do run } */
I'd say you should limit the test to ilp32 || lp64 targets.
> +/* { dg-options "-fsanitize=float-cast-overflow" } */
> +/* { dg-additional-options "-msse -mfpmath=sse" { target { { i?86-*-*
> x86_64-*-* } && ia32 } } } */
And this needs to be guarded by && sse2_runtime. Supposedly
you should also pass -msse2 rather than -msse, the test uses double
rather than float.
Jakub
Re: [PATCH] Fix omp declare simd cloning (PR tree-optimization/60823)
Rainer Orth writes: > Jakub Jelinek writes: > >> 2014-04-18 Jakub Jelinek >> >> PR tree-optimization/60823 >> * omp-low.c (ipa_simd_modify_function_body): Go through >> all SSA_NAMEs and for those refering to vector arguments >> which are going to be replaced adjust SSA_NAME_VAR and, >> if it is a default definition, change it into a non-default >> definition assigned at the beginning of function from new_decl. >> (ipa_simd_modify_stmt_ops): Rewritten. >> * tree-dfa.c (set_ssa_default_def): When removing default def, >> check for NULL loc instead of NULL *loc. >> >> * c-c++-common/gomp/pr60823-1.c: New test. >> * c-c++-common/gomp/pr60823-2.c: New test. >> * c-c++-common/gomp/pr60823-3.c: New test. > > The second test FAILs on Solaris/x86 with Sun as: > > ld.so.1: pr60823-2.exe: fatal: pr60823-2.exe: hardware capability > (CA_SUNW_HW_1) unsupported: 0x2000 [ AVX ] > FAIL: c-c++-common/gomp/pr60823-2.c execution test > > If this is expected, I can extend the code in gcc.target/i386/i386.exp > (or rather move it to a new lib/clearcap.exp) to handle that via linker > maps. Something else seems to be amiss here: the new libgomp.fortran/declare-simd-[12].f90 tests fail in just the same way, although avx_runtime is false and they are only compiled with -msse2. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [DOC Patch] symbol rename pragmas
On Tue, 13 May 2014, Rainer Orth wrote: > right, and that's why I want to keep this info. If it weren't for > Solaris compatibility, this pragma wouldn't exist, and given that > heritage, I don't want to encourage its use elsewhere, even though it > does work. I can see definite cases where it could be useful in glibc headers, to redirect functions to the appropriate version for a particular standard in the implicitly preincluded stdc-predef.h even if the header declaring the function in question isn't included (ISO C and POSIX allow you to declare standard functions yourself if the standard prototype for the function doesn't involve any type defined in a header). So, I think it's quite right for it to be available on all platforms as a feature that can help in implementing certain standard requirements. -- Joseph S. Myers jos...@codesourcery.com
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
As discussed offline, this is actually due to missing parts of the
previous patch (some changes does not appear in the change log of
r199154). I've updated the patch to include those missing pieces.
Testing on going.
Dehao
On Tue, May 13, 2014 at 10:04 AM, Cary Coutant wrote:
>> The problem is that linemap_location_from_macro_expansion_p will
>> always return true if locus has discriminator. And in linemap_lookup,
>> this will lead to call linemap_macro_map_lookup, in which there is an
>> assertion:
>>
>> linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
>>
>> However, line is actually not a macro location.
>
> That sounds like we're leaking a discriminator location into the
> linemap code. Before you can call
> linemap_location_from_macro_expansion_p, you need to do this (as in
> expand_location_1):
>
> /* If LOC describes a location with a discriminator, extract the
> discriminator and map it to the real location. */
> if (min_discriminator_location != UNKNOWN_LOCATION
> && loc >= min_discriminator_location
> && loc < next_discriminator_location)
> loc = map_discriminator_location (loc);
>
> -cary
Index: gcc/tree-ssa-uninit.c
===
--- gcc/tree-ssa-uninit.c (revision 210386)
+++ gcc/tree-ssa-uninit.c (working copy)
@@ -149,6 +149,7 @@ warn_uninit (enum opt_code wc, tree t,
location = (context != NULL && gimple_has_location (context))
? gimple_location (context)
: DECL_SOURCE_LOCATION (var);
+ location = map_discriminator_location (location);
location = linemap_resolve_location (line_table, location,
LRK_SPELLING_LOCATION,
NULL);
Index: gcc/tree-diagnostic.c
===
--- gcc/tree-diagnostic.c (revision 210386)
+++ gcc/tree-diagnostic.c (working copy)
@@ -108,6 +108,8 @@ maybe_unwind_expanded_macro_loc (diagnostic_contex
unsigned ix;
loc_map_pair loc, *iter;
+ if (has_discriminator (where))
+where = map_discriminator_location (where);
map = linemap_lookup (line_table, where);
if (!linemap_macro_expansion_map_p (map))
return;
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 210386)
+++ gcc/builtins.c (working copy)
@@ -59,6 +59,7 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "ubsan.h"
#include "cilk.h"
+#include "input.h"
static tree do_mpc_arg1 (tree, tree, int (*)(mpc_ptr, mpc_srcptr, mpc_rnd_t));
@@ -12064,6 +12065,10 @@ fold_builtin_next_arg (tree exp, bool va_start_p)
tree fntype = TREE_TYPE (current_function_decl);
int nargs = call_expr_nargs (exp);
tree arg;
+ location_t loc = LOCATION_LOCUS (input_location);
+ if (has_discriminator (loc))
+loc = map_discriminator_location (loc);
+
/* There is good chance the current input_location points inside the
definition of the va_start macro (perhaps on the token for
builtin) in a system header, so warnings will not be emitted.
Index: gcc/diagnostic.c
===
--- gcc/diagnostic.c(revision 210386)
+++ gcc/diagnostic.c(working copy)
@@ -188,6 +188,7 @@ diagnostic_set_info_translated (diagnostic_info *d
va_list *args, location_t location,
diagnostic_t kind)
{
+ location = map_discriminator_location (location);
diagnostic->message.err_no = errno;
diagnostic->message.args_ptr = args;
diagnostic->message.format_spec = msg;
@@ -514,6 +515,9 @@ diagnostic_report_current_module (diagnostic_conte
if (where <= BUILTINS_LOCATION)
return;
+ if (has_discriminator (where))
+where = map_discriminator_location (where);
+
linemap_resolve_location (line_table, where,
LRK_MACRO_DEFINITION_LOCATION,
&map);
[PATCH] Use -ggnu-pubnames with -gsplit-dwarf
This patch forces the use of -ggnu-pubnames when using -gsplit-dwarf. This is necessary so that the gold linker can generate .gdb_index version 7. No new regressions. Committed as trivial (has no effect if you're not using -gsplit-dwarf). -cary 2014-05-13 Cary Coutant gcc/ * opts.c (finish_options): Use -ggnu-pubnames with -gsplit-dwarf. Index: opts.c === --- opts.c (revision 210389) +++ opts.c (working copy) @@ -857,9 +857,9 @@ finish_options (struct gcc_options *opts maybe_set_param_value (PARAM_MAX_STORES_TO_SINK, 0, opts->x_param_values, opts_set->x_param_values); - /* The -gsplit-dwarf option requires -gpubnames. */ + /* The -gsplit-dwarf option requires -ggnu-pubnames. */ if (opts->x_dwarf_split_debug_info) -opts->x_debug_generate_pub_sections = 1; +opts->x_debug_generate_pub_sections = 2; } #define LEFT_COLUMN27
Re: [RFC][PING^2] Do not consider volatile asms as optimization barriers #1
On Tue, May 13, 2014 at 07:34:51PM +0200, Eric Botcazou wrote: > > Even with official release you can apply a patch, that is not the point. > > The point is that many people expect the release branches (and IMHO rightly > > so) to be supposedly stable all the time, rather than being seriously > > unstable most of the time and only converging to stability around the > > official releases. > > Possibly, but with our scheme it's the opposite: it's stable most of the time > and series of patches are applied just before the releases... That's not completely true. Some people do (at least for the non-risky changes) backports immediately, others often do it in batches, days to weeks to months after trunk commits, but most of the batches aren't close to releases. If you look e.g. at 4.8 branch, there were less than 50 (non-DATESTAMP) commits in the last month, but more than 300 (non-DATESTAMP) commits this year (and quite a few in between 4.8.2 release and New Year). So I'd say the last month on 4.8 branch was pretty much a normal month on the branch, nothing exceptional. Jakub
Re: [PATCH] Fix omp declare simd cloning (PR tree-optimization/60823)
On Tue, May 13, 2014 at 07:38:52PM +0200, Rainer Orth wrote: > Rainer Orth writes: > > > Jakub Jelinek writes: > > > >> 2014-04-18 Jakub Jelinek > >> > >>PR tree-optimization/60823 > >>* omp-low.c (ipa_simd_modify_function_body): Go through > >>all SSA_NAMEs and for those refering to vector arguments > >>which are going to be replaced adjust SSA_NAME_VAR and, > >>if it is a default definition, change it into a non-default > >>definition assigned at the beginning of function from new_decl. > >>(ipa_simd_modify_stmt_ops): Rewritten. > >>* tree-dfa.c (set_ssa_default_def): When removing default def, > >>check for NULL loc instead of NULL *loc. > >> > >>* c-c++-common/gomp/pr60823-1.c: New test. > >>* c-c++-common/gomp/pr60823-2.c: New test. > >>* c-c++-common/gomp/pr60823-3.c: New test. > > > > The second test FAILs on Solaris/x86 with Sun as: > > > > ld.so.1: pr60823-2.exe: fatal: pr60823-2.exe: hardware capability > > (CA_SUNW_HW_1) unsupported: 0x2000 [ AVX ] > > FAIL: c-c++-common/gomp/pr60823-2.c execution test > > > > If this is expected, I can extend the code in gcc.target/i386/i386.exp > > (or rather move it to a new lib/clearcap.exp) to handle that via linker > > maps. > > Something else seems to be amiss here: the new > libgomp.fortran/declare-simd-[12].f90 tests fail in just the same way, > although avx_runtime is false and they are only compiled with -msse2. If OpenMP declare simd doesn't work on Solaris/x86 (due to the bogus hw cap stuff), then supposedly vect_simd_clones effective target should fail there. OpenMP declare simd results in cloning of the functions for SSE2, AVX and AVX2. Jakub
[google/gcc-4_9] Force the use of -ggnu-pubnames when using -gsplit-dwarf
I've backported this patch from trunk at r210395. -cary gcc/ * opts.c (finish_options): Use -ggnu-pubnames with -gsplit-dwarf.
Re: [PATCH] Implement -fsanitize=float-cast-overflow
On Tue, 13 May 2014, Marek Polacek wrote: > Here's an attempt to add the -fsanitize=float-cast-overflow > instrumentation. It should issue a runtime error when a floating-point > to integer type conversion overflows. Eventually it should instrument As with divide-by-zero, this should not be part of -fsanitize=undefined because under Annex F this is not undefined behavior (instead it raises "invalid" and returns an unspecified value, C11 F.4). > even floating-point to floating-point conversions to detect e.g. > (float)1e39 overflow, but I'd like to settle first on float to int > before implementing that. I think that should be a separate option. If the type has infinities, it's not undefined even in the absence of Annex F (because infinities count as part of the range of the type). And of course overflow depends on the rounding mode when the type of the result is a floating-point type. (However, conversions of integers to floating point can probably count the same as conversions of floating point to floating point if you add such an option. __int128 to float can overflow.) It would be a good idea for the testcases to cover conversions to __int128 / unsigned __int128, where supported. What do you do for overflowing conversions to bit-fields? I think the correct rule is: * For C, if the floating-point value, truncated toward 0, is outside the range of a signed or unsigned type of the specified number of bits, then you should get the (invalid, unspecified value (this isn't actually implemented in GCC)), and get a runtime error for the new option. * For C++, bit-fields don't count as separate types, so it should act as converting to the declared type and then converting from that to the bit-field (as a modulo operation). Thus, for an unsigned:1 bit-field, for example, values outside the interval (-1, 2) would produce the error for C, but only those outside (-1, 0x1p32) would do so for C++ (presuming 32-bit int). > + tree min = TYPE_MIN_VALUE (type); > + tree max = TYPE_MAX_VALUE (type); > + /* Add/subtract 1.0 so we can avoid truncating the value of EXPR. */ > + min = fold_build2 (MINUS_EXPR, expr_type, > + build_real_from_int_cst (expr_type, min), > + build_one_cst (expr_type)); > + max = fold_build2 (PLUS_EXPR, expr_type, > + build_real_from_int_cst (expr_type, max), > + build_one_cst (expr_type)); It looks to me like this will first round the max value to the floating-point type, then add 1 to the rounded value and round again. Which I think is in fact safe at least for IEEE binary floating-point types, but that isn't immediately obvious. Possible issues: * Does the folding of the addition occur in all cases for IBM long double? * Is this correct for decimal floating point? There, the overflow condition (value >= max+1) should be using a value of (max+1) rounded upward rather than to-nearest, if max+1 isn't exactly representable (and in general it isn't - powers of two 0x1p24 and above aren't representable in decimal32, 0x1p54 and above in decimal64, 0x1p113 and above in decimal128, so you just need to find a case where the double-rounding computation you have produces the wrong value). * Likewise, (value <= min-1) for both binary and decimal floating point - you need to round once, away from 0. For float converted to signed int, the relevant condition is values < -0x1p31 - 1, i.e. <= 0x1.02p31f once you allow for which values are representable as float, which is not min-1 (min-1 rounds to -0x1p31, but a conversion of that to signed int is fully defined with no exceptions). -- Joseph S. Myers jos...@codesourcery.com
C++ PATCH for c++/61151 (ICE with nested lambda in template)
My patch for 60092 failed to consider that we need to avoid obscuring a
capture proxy for 'this', not just 'this' itself.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit e40151a986a68e29c169913e36b76fa4310379d7
Author: Jason Merrill
Date: Tue May 13 12:52:37 2014 -0400
PR c++/61151
* semantics.c (is_this_parameter): Allow capture proxies too.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index d925f5c..583b870 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -8158,8 +8158,10 @@ maybe_initialize_constexpr_call_table (void)
bool
is_this_parameter (tree t)
{
- return (TREE_CODE (t) == PARM_DECL
- && DECL_NAME (t) == this_identifier);
+ if (!DECL_P (t) || DECL_NAME (t) != this_identifier)
+return false;
+ gcc_assert (TREE_CODE (t) == PARM_DECL || is_capture_proxy (t));
+ return true;
}
/* We have an expression tree T that represents a call, either CALL_EXPR
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this18.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this18.C
new file mode 100644
index 000..fec2da6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this18.C
@@ -0,0 +1,30 @@
+// PR c++/61151
+// { dg-do compile { target c++11 } }
+
+struct B
+{
+ void foo () {}
+};
+
+template
+struct A
+{
+ template void bar ();
+ B a;
+};
+
+template
+template
+void
+A::bar ()
+{
+ auto f = [this] () { auto g = [=] () { a.foo (); }; g (); };
+ f ();
+}
+
+int
+main ()
+{
+ A a;
+ a.bar ();
+}
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
Attached patch passes regression tests and benchmark test. OK for google-4_9?
Thanks,
Dehao
On Tue, May 13, 2014 at 10:43 AM, Dehao Chen wrote:
> As discussed offline, this is actually due to missing parts of the
> previous patch (some changes does not appear in the change log of
> r199154). I've updated the patch to include those missing pieces.
> Testing on going.
>
> Dehao
>
> On Tue, May 13, 2014 at 10:04 AM, Cary Coutant wrote:
>>> The problem is that linemap_location_from_macro_expansion_p will
>>> always return true if locus has discriminator. And in linemap_lookup,
>>> this will lead to call linemap_macro_map_lookup, in which there is an
>>> assertion:
>>>
>>> linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
>>>
>>> However, line is actually not a macro location.
>>
>> That sounds like we're leaking a discriminator location into the
>> linemap code. Before you can call
>> linemap_location_from_macro_expansion_p, you need to do this (as in
>> expand_location_1):
>>
>> /* If LOC describes a location with a discriminator, extract the
>> discriminator and map it to the real location. */
>> if (min_discriminator_location != UNKNOWN_LOCATION
>> && loc >= min_discriminator_location
>> && loc < next_discriminator_location)
>> loc = map_discriminator_location (loc);
>>
>> -cary
Index: gcc/diagnostic.c
===
--- gcc/diagnostic.c(revision 210386)
+++ gcc/diagnostic.c(working copy)
@@ -188,6 +188,7 @@ diagnostic_set_info_translated (diagnostic_info *d
va_list *args, location_t location,
diagnostic_t kind)
{
+ location = map_discriminator_location (location);
diagnostic->message.err_no = errno;
diagnostic->message.args_ptr = args;
diagnostic->message.format_spec = msg;
@@ -514,6 +515,9 @@ diagnostic_report_current_module (diagnostic_conte
if (where <= BUILTINS_LOCATION)
return;
+ if (has_discriminator (where))
+where = map_discriminator_location (where);
+
linemap_resolve_location (line_table, where,
LRK_MACRO_DEFINITION_LOCATION,
&map);
Index: gcc/tree-ssa-uninit.c
===
--- gcc/tree-ssa-uninit.c (revision 210386)
+++ gcc/tree-ssa-uninit.c (working copy)
@@ -149,6 +149,7 @@ warn_uninit (enum opt_code wc, tree t,
location = (context != NULL && gimple_has_location (context))
? gimple_location (context)
: DECL_SOURCE_LOCATION (var);
+ location = map_discriminator_location (location);
location = linemap_resolve_location (line_table, location,
LRK_SPELLING_LOCATION,
NULL);
Index: gcc/tree-diagnostic.c
===
--- gcc/tree-diagnostic.c (revision 210386)
+++ gcc/tree-diagnostic.c (working copy)
@@ -108,6 +108,8 @@ maybe_unwind_expanded_macro_loc (diagnostic_contex
unsigned ix;
loc_map_pair loc, *iter;
+ if (has_discriminator (where))
+where = map_discriminator_location (where);
map = linemap_lookup (line_table, where);
if (!linemap_macro_expansion_map_p (map))
return;
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 210386)
+++ gcc/builtins.c (working copy)
@@ -59,6 +59,7 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "ubsan.h"
#include "cilk.h"
+#include "input.h"
static tree do_mpc_arg1 (tree, tree, int (*)(mpc_ptr, mpc_srcptr, mpc_rnd_t));
@@ -12064,13 +12065,16 @@ fold_builtin_next_arg (tree exp, bool va_start_p)
tree fntype = TREE_TYPE (current_function_decl);
int nargs = call_expr_nargs (exp);
tree arg;
+ location_t loc = LOCATION_LOCUS (input_location);
+ if (has_discriminator (loc))
+loc = map_discriminator_location (loc);
+
/* There is good chance the current input_location points inside the
definition of the va_start macro (perhaps on the token for
builtin) in a system header, so warnings will not be emitted.
Use the location in real source code. */
source_location current_location =
-linemap_unwind_to_first_non_reserved_loc (line_table, input_location,
- NULL);
+linemap_unwind_to_first_non_reserved_loc (line_table, loc, NULL);
if (!stdarg_p (fntype))
{
Re: Ping^2 [patch, toplevel] configure nios2-elf libraries to build with -mno-gpopt
On 05/13/14 08:11, Sandra Loosemore wrote: On 05/05/2014 02:32 PM, Sandra Loosemore wrote: Ping! http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01618.html And ping again As a co-maintainer for the nios2 port, I think this would be something you could self-approve. Regardless, approved :-) jeff
RE: [Patch,Microblaze]: Added Break Handler Support
Hello Michael: Thanks for the comments on ChangeLog. Modified ChangeLog is inlined below. 2014-05-13 Ajit Agarwal * config/microblaze/microblaze.c (break_handler): New Declaration. (microblaze_break_function_p,microblaze_is_break_handler) : New function. (compute_frame_size): use of microblaze_break_function_p. Add the test of break_handler. (microblaze_function_prologue,microblaze_function_epilogue) : Add the test of variable break_handler. (microblaze_globalize_label) : Add the test of break_handler. * config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro * config/microblaze/microblaze.md : (*,_internal): Add microblaze_is_break_handler () test. (call_internal1,call_value_intern) : Use of microblaze_break_function_p. Use of SYMBOL_REF_DECL. * config/microblaze/microblaze-protos.h (microblaze_break_function_p,microblaze_is_break_handler) : New Declaration. * testsuite/gcc.target/microblaze/others/break_handler.c : New. Thanks & Regards Ajit -Original Message- From: Michael Eager [mailto:ea...@eagercon.com] Sent: Tuesday, May 13, 2014 10:30 PM To: Ajit Kumar Agarwal; gcc-patches@gcc.gnu.org Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,Microblaze]: Added Break Handler Support On 05/13/14 02:14, Ajit Kumar Agarwal wrote: > Hello Michael: > > The following patch is to handle Software and Hardware breaks in Microblaze > Architecture. > Deja GNU testcase does not have any regressions and the testcase attached > passes through. > Review comments are incorporated. > > Okay for trunk? Just saying OK would only be appropriate if you had write access to the repository. > Added Break Handler support to incorporate the hardware and software > break. The Break Handler routine will be generating the rtbd > instruction. At the call point where the software breaks are generated with > the instruction brki with register operand as r16. > > 2014-05-13 Ajit Agarwal > > * config/microblaze/microblaze.c > (microblaze_break_function_p,microblaze_is_break_handler) : New > > * config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro > > * config/microblaze/microblaze.md : >Extended support for generation of brki instruction and rtbd instruction. A better ChangeLog entry is * config/microblaze/microblaze.md (*,_internal): Add microblaze_is_break_handler () test. Give specifics, naming functions, rather than making general comments. As the ChangeLog standard says: It’s important to name the changed function or variable in full. Don’t abbreviate function or variable names, and don’t combine them. Subsequent maintainers will often search for a function name to find all the change log entries that pertain to it; if you abbreviate the name, they won’t find it when they search. Mention each place where there are changes. There should be a ChangeLog entry for each non-trivial change. Your patch made four significant changes to microblaze.md. There appear to be several changes in microblaze.c, not just the definition of the new functions as shown in your entry. > * config/microblaze/microblaze-protos.h > (microblaze_break_function_p,microblaze_is_break_handler) : New > Declaration. > > * testsuite/gcc.target/microblaze/others/break_handler.c : New. Thanks for the test case. As mentioned previously, add documentation for _break_handler. > diff --git a/gcc/config/microblaze/microblaze-protos.h > b/gcc/config/microblaze/microblaze-protos.h > index b03e9e1..f3cc099 100644 > --- a/gcc/config/microblaze/microblaze-protos.h > +++ b/gcc/config/microblaze/microblaze-protos.h Please include the patch only once, not both inline and again as an attachment. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [RFC] Using function clones for Pointer Bounds Checker
On 05/13/14 02:38, Ilya Enkovich wrote: propagate constant bounds value and remove checks in called function). So from a linking standpoint, presumably you have to mangle the instrumented caller/callee in some manner. Right? Or are you dynamically dispatching somehow? Originally the idea was o have instrumented clone to have the same assembler name as the original function. Since instrumented code is fully compatible with not instrumented code, we always emit only one version. Usage of the same assembler name allows instrumented and not instrumented calls to look similar in assembler. It worked fine until I tried it with LTO where assembler name is used as a unique identifier. With linker resolutions files it became even more harder to use such approach. To resolve these issues I started to use new assembler name with postfix, but linked with the original name using IDENTIFIER_TRANSPARENT_ALIAS. It gives different assembler names for clones and originals during compilation, but both clone and original functions have similar name in output assembler. OK. So if I read that correctly, it implies that the existence of bounds information does not change the signature of the callee. This is obviously important for C++. Sounds like I need to sit down with the branch and see how this works in the new scheme. jeff
Re: [PATCH, 1/2] shrink wrap a function with a single loop: copy propagation
On 05/09/14 01:30, Zhenqiang Chen wrote: So why restrict this to just cases where we have to propagate into a COMPARE at the end of a block? So in your example, assume the first block looks like prepare_shrink_wrap will move_insn_for_shrink_wrap in BB_INSNS_REVERSE order. Current prepare_shrink_wrap should handle the case since there is no data flow dependence. OK. My first thought here was that we must have some code which does 90% of what you need. Did you look at any of the existing RTL optimization infrastructure to see if there was code you could extend to handle this? Most the codes are from function copyprop_hardreg_forward_1 in "cprop_hardreg" pass. Previously I had a patch to reuse it: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00305.html. What do you think about the old patch? Utilizing routines from the hard register copy propagation pass seems wise. I'll take a look at the updated patch. jeff
Re: [Patch,Microblaze]: Added Break Handler Support
On 05/13/14 12:15, Ajit Kumar Agarwal wrote: Hello Michael: Thanks for the comments on ChangeLog. Modified ChangeLog is inlined below. Please resubmit the patch with documentation for _break_handler. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
[PATCH] Provide inlining context in strict-overflow warnings
Patterns that trigger the optimization and warning can form after
inlining, and it can be rather difficult to figure out what exactly is
causing the warning. The inlining context at least provides additional
hints, enabling developers to substitute the arguments and discover
what, precisely, is happening.
More context is provided with -g than without, but I think this is
acceptable.
I bootstrapped and tested the attached patch on x86_64-redhat-linux-gnu,
with no new regressions.
--
Florian Weimer / Red Hat Product Security Team
gcc/
2014-05-13 Florian Weimer
* fold-const.c (fold_undefer_overflow_warnings): Print notes with
inlining information.
gcc/testsuite/
2014-05-13 Florian Weimer
* c-c++-common/Wstrict-overflow-1.c: New test.
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 0fcb87f..5f13992 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -268,6 +268,12 @@ fold_undefer_overflow_warnings (bool issue, const_gimple stmt, int code)
else
locus = gimple_location (stmt);
warning_at (locus, OPT_Wstrict_overflow, "%s", warnmsg);
+
+ /* Print notes with inlining information. */
+ for (tree block = gimple_block (stmt); block && TREE_CODE (block) == BLOCK;
+ block = BLOCK_SUPERCONTEXT (block))
+if (BLOCK_SOURCE_LOCATION (block))
+ inform (BLOCK_SOURCE_LOCATION (block), "inlined from here");
}
/* Stop deferring overflow warnings, ignoring any deferred
diff --git a/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c b/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c
new file mode 100644
index 000..693c52b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -Wstrict-overflow" } */
+
+int f (void);
+
+static int g (int a, int b, int c)
+{
+ if (a + b < c) /* { dg-warning "assuming signed overflow" } */
+return -1;
+ return f ();
+}
+
+static int h (int a, int b)
+{
+ return g(a, 1, b); /* { dg-message "note: inlined from here" } */
+}
+
+int k (int a)
+{
+ return h(a, a); /* { dg-message "note: inlined from here" } */
+}
Re: [GOOGLE] Updates highest_location when updating next_discriminator_location
> Attached patch passes regression tests and benchmark test. OK for google-4_9? OK. Thanks again! -cary
[patch,mips] avoid invalid register for JALR
When I was trying to benchmark another patch (which I'll be sending
along shortly) with CSiBE for -mabi=64, I ran into an assembler error
like this:
/tmp/ccJv2faG.s: Assembler messages:
/tmp/ccJv2faG.s:1605: Error: a destination register must be supplied
`jalr $31'
Indeed, GCC is generating invalid code here; the single-operand JALR
instruction doesn't permit the use of $31 because it is already the
implicit destination register. The attached patch introduces a new
register class JALR_REGS to represent the valid set of registers for
this instruction, and modifies the "c" register constraint to use it.
I had some difficulty in regression-testing this patch because of
unrelated problems on trunk in the past week -- at first I was getting
ICEs due to a null pointer dereference in tree code, then when I tried
again a couple days later trunk was not even building. So I ended up
testing this patch on a more stable 4.9.0 checkout modified to support
Mentor's extended set of mips-sde-elf multilibs instead.
OK to commit?
-Sandra
2014-05-13 Sandra Loosemore
gcc/
* config/mips/mips.h (enum reg_class): Add JALR_REGS.
(REG_CLASS_NAMES): Add initializer for JALR_REGS.
(REG_CLASS_CONTENTS): Likewise.
* config/mips/constraints.md ("c"): Use JALR_REGS
instead of GR_REGS.
Index: gcc/config/mips/mips.h
===
--- gcc/config/mips/mips.h (revision 210372)
+++ gcc/config/mips/mips.h (working copy)
@@ -1840,6 +1840,7 @@ enum reg_class
PIC_FN_ADDR_REG, /* SVR4 PIC function address register */
V1_REG, /* Register $v1 ($3) used for TLS access. */
LEA_REGS, /* Every GPR except $25 */
+ JALR_REGS, /* integer registers except $31 */
GR_REGS, /* integer registers */
FP_REGS, /* floating point registers */
MD0_REG, /* first multiply/divide register */
@@ -1878,6 +1879,7 @@ enum reg_class
"PIC_FN_ADDR_REG", \
"V1_REG",\
"LEA_REGS",\
+ "JALR_REGS",\
"GR_REGS",\
"FP_REGS",\
"MD0_REG",\
@@ -1919,6 +1921,7 @@ enum reg_class
{ 0x0200, 0x, 0x, 0x, 0x, 0x }, /* PIC_FN_ADDR_REG */ \
{ 0x0008, 0x, 0x, 0x, 0x, 0x }, /* V1_REG */ \
{ 0xfdff, 0x, 0x, 0x, 0x, 0x }, /* LEA_REGS */ \
+ { 0x7fff, 0x, 0x, 0x, 0x, 0x }, /* JALR_REGS */ \
{ 0x, 0x, 0x, 0x, 0x, 0x }, /* GR_REGS */ \
{ 0x, 0x, 0x, 0x, 0x, 0x }, /* FP_REGS */ \
{ 0x, 0x, 0x0001, 0x, 0x, 0x }, /* MD0_REG */ \
Index: gcc/config/mips/constraints.md
===
--- gcc/config/mips/constraints.md (revision 210372)
+++ gcc/config/mips/constraints.md (working copy)
@@ -50,7 +50,7 @@
;; for details.
(define_register_constraint "c" "TARGET_MIPS16 ? M16_REGS
: TARGET_USE_PIC_FN_ADDR_REG ? PIC_FN_ADDR_REG
- : GR_REGS"
+ : JALR_REGS"
"A register suitable for use in an indirect jump. This will always be
@code{$25} for @option{-mabicalls}.")
[patch, mips] delete bit-rotten ADJUST_REG_ALLOC_ORDER definition
This patch is a follow-up to this thread from a few years ago:
https://gcc.gnu.org/ml/gcc/2011-01/msg00093.html
https://gcc.gnu.org/ml/gcc/2011-01/msg00158.html
As noted there, the current definition of ADJUST_REG_ALLOC_ORDER is
obsolete:
(1) This hook is a holdover from the old pre-IRA register allocator and
it's not clear it does anything useful with IRA.
(2) It's inconsistent with the current REG_ALLOC_ORDER definition which
is not just {0, 1, 2, ...} any more.
I considered re-working the hook to jiggle the $24 ordering for MIPS16
relative to the current REG_ALLOC_ORDER definition, but it wasn't clear
to me either where in the ordering it should go, or that it would be
worthwhile to try to tune this. Indeed, the CSiBE results for removing
it entirely are pretty much in the noise range, except that removing the
hack that is supposed to benefit MIPS16 resulted in more improvement for
that case than any of the others tested! Here are some numbers
comparing geomean for old and new with -Os for various combinations:
default: 1062.3,1062.28
-EL -mips16 -msoft-float: 758.624,758.551
-EL -mips16: 763.398,763.321
-EL -msoft-float: 1115.11,1115.09
-EL: 1062.22,1062.2
-EL -mabi=64: 1181.8,1181.81
-mabi=64: 1181.93,1181.94
-mips16 -msoft-float: 758.337,758.236
-mips16: 763.11,763.011
-EL -mabi=64 -msoft-float: 1218.11,1218.11
-mabi=64 -msoft-float: 1218.24,1218.25
-msoft-float: 1115.73,1115.71
-EL -mmicromips -msoft-float: 792.314,792.32
-EL -mnan=2008: 1062.22,1062.2
-mnan=2008: 1062.3,1062.28
As for the invalid JALR patch I posted just now
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01015.html
I had trouble regression-testing this patch on trunk due to various
other breakage in the past week, and ended up doing it on a 4.9.0
checkout modified to support Mentor's extended set of mips-sde-elf
multilibs instead. That's also the source of the CSiBE numbers above.
Also, those numbers do include the JALR patch in the baseline.
This patch did fix some of the dspr2-MULT.c and dspr2-MULTU.c test
failures that were reported in the original discussion, but I believe
the current XFAILs for those tests are still valid for the same reasons
listed in PR target/51729.
So, this patch should be considered more of a code cleanup thing, than
either an optimization improvement or a test regression fix. OK to commit?
-Sandra
2014-05-13 Catherine Moore
Sandra Loosemore
gcc/
* config/mips/mips.c (mips_order_regs_for_local_alloc): Delete.
* config/mips/mips.h (ADJUST_REG_ALLOC_ORDER): Delete.
* config/mips/mips-protos.h (mips_order_regs_for_local_alloc): Delete.
Index: gcc/config/mips/mips.c
===
--- gcc/config/mips/mips.c (revision 210372)
+++ gcc/config/mips/mips.c (working copy)
@@ -17509,28 +17509,6 @@ mips_conditional_register_usage (void)
}
}
-/* When generating MIPS16 code, we want to allocate $24 (T_REG) before
- other registers for instructions for which it is possible. This
- encourages the compiler to use CMP in cases where an XOR would
- require some register shuffling. */
-
-void
-mips_order_regs_for_local_alloc (void)
-{
- int i;
-
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-reg_alloc_order[i] = i;
-
- if (TARGET_MIPS16)
-{
- /* It really doesn't matter where we put register 0, since it is
- a fixed register anyhow. */
- reg_alloc_order[0] = 24;
- reg_alloc_order[24] = 0;
-}
-}
-
/* Implement EH_USES. */
bool
Index: gcc/config/mips/mips.h
===
--- gcc/config/mips/mips.h (revision 210372)
+++ gcc/config/mips/mips.h (working copy)
@@ -2006,13 +2009,6 @@ enum reg_class
182,183,184,185,186,187 \
}
-/* ADJUST_REG_ALLOC_ORDER is a macro which permits reg_alloc_order
- to be rearranged based on a particular function. On the mips16, we
- want to allocate $24 (T_REG) before other registers for
- instructions for which it is possible. */
-
-#define ADJUST_REG_ALLOC_ORDER mips_order_regs_for_local_alloc ()
-
/* True if VALUE is an unsigned 6-bit number. */
#define UIMM6_OPERAND(VALUE) \
Index: gcc/config/mips/mips-protos.h
===
--- gcc/config/mips/mips-protos.h (revision 210372)
+++ gcc/config/mips/mips-protos.h (working copy)
@@ -248,7 +248,6 @@ extern bool mips_expand_ext_as_unaligned
extern bool mips_expand_ins_as_unaligned_store (rtx, rtx, HOST_WIDE_INT,
HOST_WIDE_INT);
extern bool mips_mem_fits_mode_p (enum machine_mode mode, rtx x);
-extern void mips_order_regs_for_local_alloc (void);
extern HOST_WIDE_INT mips_debugger_offset (rtx, HOST_WIDE_INT);
extern void mips_push_asm_switch (struct mips_asm_switch *);
Re: wide-int, ada
> I don't think that the mechanical change in UI_From_gnu is correct, see the
> comment just above. The annotate_value change is very likely correct, but
> please double check and, upon positive outcome, remove the last sentence of
> the comment just above.
The annotate_value change was wrong, fixed thusly, applied on the mainline.
* gcc-interface/decl.c (annotate_value): Fix thinko in latest change.
--
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c (revision 210350)
+++ gcc-interface/decl.c (working copy)
@@ -7515,15 +7515,16 @@ annotate_value (tree gnu_size)
case BIT_AND_EXPR:
tcode = Bit_And_Expr;
- /* For negative values, build NEGATE_EXPR of the opposite. Such values
- appear in expressions containing aligning patterns. Note that, since
- sizetype is unsigned, we have to jump through some hoops. */
+ /* For negative values in sizetype, build NEGATE_EXPR of the opposite.
+ Such values appear in expressions with aligning patterns. Note that,
+ since sizetype is unsigned, we have to jump through some hoops. */
if (TREE_CODE (TREE_OPERAND (gnu_size, 1)) == INTEGER_CST)
{
tree op1 = TREE_OPERAND (gnu_size, 1);
- if (wi::neg_p (op1))
+ wide_int signed_op1 = wi::sext (op1, TYPE_PRECISION (sizetype));
+ if (wi::neg_p (signed_op1))
{
- op1 = wide_int_to_tree (sizetype, wi::neg (op1));
+ op1 = wide_int_to_tree (sizetype, wi::neg (signed_op1));
pre_op1 = annotate_value (build1 (NEGATE_EXPR, sizetype, op1));
}
}
Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)
On Tue, 2014-05-13 at 09:10 +0100, Joern Rennecke wrote: > On 12 May 2014 23:39, Oleg Endo wrote: > > > This is the same as changing/setting the FP modes (PR, SZ) on SH while > > preserving the other FPSCR bits, or did I miss something? > > It's more like if you have to control multiple bits at once to get a > specific mode. > Say you have to turn SZ off and PR on. You you knew that only one bit needs > changing, you can do with one less arithmetic operation. Right. I was thinking to add FPSCR.SZ mode switching to SH, in order to do float vector moves. For that SZ and PR need to be switched both at the same time (only SH4A has both, fpchg and fschg). So basically I'd add another mode entity, which would emit SZ mode changes in addition to the PR mode changes. But then adjacent FPSCR-changing insns could be combined ... any idea/suggestion how to accomplish that? Cheers, Oleg
Re: [patch,mips] avoid invalid register for JALR
Sandra Loosemore writes:
> When I was trying to benchmark another patch (which I'll be sending
> along shortly) with CSiBE for -mabi=64, I ran into an assembler error
> like this:
>
> /tmp/ccJv2faG.s: Assembler messages:
> /tmp/ccJv2faG.s:1605: Error: a destination register must be supplied
> `jalr $31'
JALR patterns should have an explicit clobber of $31, which I thought
was also supposed to stop $31 from being used as the call address. E.g.:
int foo (void)
{
typedef void (*type) (void);
register type fn asm ("$31");
asm ("foo %0" : "=r" (fn));
fn ();
return 1;
}
gives the expected shuffle:
#APP
# 5 "/tmp/foo.c" 1
foo $31
# 0 "" 2
#NO_APP
move$2,$31
jalr$2
If you change the asm to some other random register then the JALR
uses it directly. Do you have a testcase?
> Indeed, GCC is generating invalid code here; the single-operand JALR
> instruction doesn't permit the use of $31 because it is already the
> implicit destination register. The attached patch introduces a new
> register class JALR_REGS to represent the valid set of registers for
> this instruction, and modifies the "c" register constraint to use it.
>
> I had some difficulty in regression-testing this patch because of
> unrelated problems on trunk in the past week -- at first I was getting
> ICEs due to a null pointer dereference in tree code, then when I tried
> again a couple days later trunk was not even building.
Could you give more details?
Thanks,
Richard
RE: [Patch,Microblaze]: Added Break Handler Support
Hello Michael: Resubmitting the Patch with documentation for _break_handler in the config/microblaze/microblaze.h. Thanks & Regards Ajit -Original Message- From: Michael Eager [mailto:ea...@eagercon.com] Sent: Wednesday, May 14, 2014 12:55 AM To: Ajit Kumar Agarwal; gcc-patches@gcc.gnu.org Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,Microblaze]: Added Break Handler Support On 05/13/14 12:15, Ajit Kumar Agarwal wrote: > Hello Michael: > > Thanks for the comments on ChangeLog. Modified ChangeLog is inlined below. Please resubmit the patch with documentation for _break_handler. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 0001-Patch-microblaze-Added-Break-Handler-support.patch Description: 0001-Patch-microblaze-Added-Break-Handler-support.patch
Re: [Patch,Microblaze]: Added Break Handler Support
On 05/13/14 14:42, Ajit Kumar Agarwal wrote: Hello Michael: Resubmitting the Patch with documentation for _break_handler in the config/microblaze/microblaze.h. Please put everything together in one place. When you resubmit a patch, include the ChangeLog. I'm not sure what you changed, but there are no changes to gcc/doc/extend.texi in your patch. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] Fix issue in uninit analysis (PR middle-end/61112)
I have concerns with the proposed this patch:
1) not sharing cd_root may lead to difficulties in later predication
simplication
2) the change to check post-dom may also lead to incomplete predicate chain.
I think the right fix to the problem is to realize that BBs with the
following conditions y_8 !=0, p.0_10 !=0, and x_5 !=0 are actually
control equivalent. This fact allows simplifying the USE predicates
from (y_8 !=0 OR p.0_10 !=0 OR x_5 !=0) into just p.0_10 !=0 which is
the same as the condition for the definition.
David
On Tue, May 13, 2014 at 2:09 AM, Richard Biener
wrote:
> On Mon, May 12, 2014 at 5:42 AM, Patrick Palka wrote:
>> Hi,
>>
>> This patch fixes a bogus warning generated by -Wmaybe-uninitialized.
>> The problem is that we sometimes fail to acknowledge a defining edge
>> belonging to a control-dependence chain because we assume that each
>> defining edge shares the same control-dependence root. But this may not
>> be true if a defining edge flows into a PHI node that is different from
>> the root PHI node. This faulty assumption may result in an incomplete
>> control-dependency chain being computed, ultimately causing a
>> false-positive warning like in the test case.
>>
>> To fix this, this patch computes the control-dependency root on a
>> per-defining-edge basis, using the function nearest_common_dominator to
>> find a common dominator (i.e. a BB before which control flow is
>> irrelevant) between the control-dependency root of the root PHI node and
>> that of the edge's dest PHI node.
>>
>> However, that change alone is not enough. We must also tweak the
>> function collect_phi_def_edges to allow recursing on PHI nodes that are
>> not dominated by the root PHI node's control dependency root as we can
>> still figure out a control dependency chain in such cases as long the
>> PHI node postdominates the PHI argument, i.e. as long as the control
>> flow from the PHI argument edge down to the root PHI node is irrelevant.
>>
>> Both these changes are required to make the below test case compile
>> without emitting a bogus warning.
>>
>> I bootstrapped and regtested this change on x86_64-unknown-linux-gnu.
>> Is it OK for trunk?
>
> CCing the author of the code.
>
> Given the lengthy comment above I miss comments in the code
> that reflect the complexity of this issue and explains the implementation.
>
> Other than that I defer to David, but any improvement to this pass
> is welcome! Can you assess the effect on compile-time this patch has?
> (from an algorithmic standpoint?)
>
> Thanks,
> Richard.
>
>> 2014-05-10 Patrick Palka
>>
>> * tree-ssa-uninit.c (collect_phi_def_edges): Remove cd_root
>> parameter. Refactor to consolidate duplicate code. Tweak dump
>> message.
>> (find_def_preds): Add dump messages. Adjust call to
>> collect_phi_def_edges. Adjust the control dependency root on
>> a per-edge basis.
>> ---
>> gcc/testsuite/gcc.dg/uninit-pred-11.c | 20 ++
>> gcc/tree-ssa-uninit.c | 75
>> +++
>> 2 files changed, 60 insertions(+), 35 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/uninit-pred-11.c
>>
>> diff --git a/gcc/testsuite/gcc.dg/uninit-pred-11.c
>> b/gcc/testsuite/gcc.dg/uninit-pred-11.c
>> new file mode 100644
>> index 000..493be68
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/uninit-pred-11.c
>> @@ -0,0 +1,20 @@
>> +// PR middle-end/61112
>> +// { dg-options "-Wuninitialized -O2" }
>> +
>> +int p;
>> +
>> +void
>> +foo (int x, int y, int z)
>> +{
>> + int w;
>> + if (x)
>> +w = z;
>> + if (y)
>> +w = 10;
>> + if (p)
>> +w = 20;
>> +
>> + if (x || y || p)
>> +p = w; // { dg-bogus "uninitialized" }
>> +}
>> +
>> diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
>> index 8b298fa..472b8e5 100644
>> --- a/gcc/tree-ssa-uninit.c
>> +++ b/gcc/tree-ssa-uninit.c
>> @@ -641,13 +641,11 @@ find_predicates (pred_chain_union *preds,
>>
>> /* Computes the set of incoming edges of PHI that have non empty
>> definitions of a phi chain. The collection will be done
>> - recursively on operands that are defined by phis. CD_ROOT
>> - is the control dependence root. *EDGES holds the result, and
>> - VISITED_PHIS is a pointer set for detecting cycles. */
>> + recursively on operands that are defined by phis. *EDGES holds
>> + the result, and VISITED_PHIS is a pointer set for detecting cycles. */
>>
>> static void
>> -collect_phi_def_edges (gimple phi, basic_block cd_root,
>> - vec *edges,
>> +collect_phi_def_edges (gimple phi, vec *edges,
>> pointer_set_t *visited_phis)
>> {
>>size_t i, n;
>> @@ -663,34 +661,23 @@ collect_phi_def_edges (gimple phi, basic_block cd_root,
>>opnd_edge = gimple_phi_arg_edge (phi, i);
>>opnd = gimple_phi_arg_def (phi, i);
>>
>> - if (TREE_CODE (opnd) != SSA_NAME)
>> -{
>> - if (dump_file && (dump_fl
