Re: [PATCH] Fix PR69951
On Thu, 3 Mar 2016, Christophe Lyon wrote: > On 2 March 2016 at 10:49, Christophe Lyon wrote: > > On 2 March 2016 at 10:16, James Greenhalgh wrote: > >> On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote: > >>> On 1 March 2016 at 10:51, James Greenhalgh > >>> wrote: > >>> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote: > >>> >> On Mon, 29 Feb 2016, James Greenhalgh wrote: > >>> >> > >>> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote: > >>> >> > > > >>> >> > > The following fixes PR69951, hopefully the last case of decl alias > >>> >> > > issues with alias analysis. This time it's points-to and the > >>> >> > > DECL_UIDs > >>> >> > > used in points-to sets not being canonicalized. > >>> >> > > > >>> >> > > The simplest (and cheapest) fix is to make aliases refer to the > >>> >> > > ultimate alias target via their DECL_PT_UID which we conveniently > >>> >> > > have available. > >>> >> > > > >>> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to > >>> >> > > trunk. > >>> >> > > > >>> >> > > Richard. > >>> >> > > > >>> >> > > 2016-02-26 Richard Biener > >>> >> > > > >>> >> > > PR tree-optimization/69551 > >>> >> > > * tree-ssa-structalias.c (get_constraint_for_ssa_var): When > >>> >> > > looking through aliases adjust DECL_PT_UID to refer to the > >>> >> > > ultimate alias target. > >>> >> > > > >>> >> > > * gcc.dg/torture/pr69951.c: New testcase. > >>> >> > > >>> >> > I see this new testcase failing on an ARM target as so: > >>> >> > > >>> >> > /tmp/ccChjoFc.s: Assembler messages: > >>> >> > /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a > >>> >> > symbol match an ARM instruction: b > >>> >> > > >>> >> > FAIL: gcc.dg/torture/pr69951.c -O0 (test for excess errors) > >>> >> > > >>> >> > But I haven't managed to reproduce it outside of the test > >>> >> > environment. > >>> >> > > >>> >> > The fix looks trivial, rename b to anything else you fancy (well... > >>> >> > stay > >>> >> > clear of add and ldr). I'll put a fix in myself if I can manage to > >>> >> > get > >>> >> > this to reproduce - though if anyone else wants to do it I won't be > >>> >> > offended :-). > >>> >> > >>> >> Huh, I wonder what's the use of such warning. After all 'ldr' is a > >>> >> valid > >>> >> C symbol name, too. In fact my cross arm as doesn't report this > >>> >> warning (binutils 2.25.0) > >>> >> > >>> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms > >>> >> Assembler messages: > >>> >> Error: unrecognized option -mwarn-syms > >>> > > >>> > Right, I've figured out the set of conditions... You need to be > >>> > targeting > >>> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition > >>> > from config/arm/linux-elf.h is pulled in. This causes us to emit: > >>> > > >>> > b = a > >>> > > >>> > Rather than > >>> > > >>> > .setb,a > >>> > > >>> > Writing it as "b = a" causes the warning added to resolve binutils > >>> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have > >>> > backported > >>> > that patch). > >>> > > >>> James, > >>> > >>> What happens for you on arm-none-eabi configurations? > >>> I'm using binutils-2.25, so I can't see this warning whatever the target. > >>> However, I'm using qemu-arm and this test fails on arm-none-eabi, > >>> because argc is set to 0 during the startup-code. > >>> > >>> As I understand it, qemu-arm considers the code page as readonly, > >>> and thus the GetCmdLine semi hosting call cannot write argc/argv > >>> back to memory in the same code page (I'm using libgloss' crt0). > >>> > >>> I tried to play with qemu-system-arm, but then libgloss' crt0 does not > >>> set the reset vector and the simulation does random things, starting at > >>> address 0. > >>> > >>> Am I missing some newlib/libgloss configuration bits, or should I > >>> send a newlib patch to address this? > >> > >> Hi Christophe, > >> > >> I didn't get this running under arm-none-eabi. The testcase does have > >> undefined behaviour (too few arguments to main), but I'd be surprised if > >> that was the issue... I'm sure the testcase could be rewritten to avoid > >> the dependence on argc if this proves an issue for other bare-metal > >> testers running under an emulator. > >> > > > > Indeed, I'm wondering why the testcase depends on argc beeing non-zero? > > > > To avoid modifying the testcase too much, I propose to replace > if (argc) > by > if (argc >= 0) > as in the attached patch. > This does make the trick on arm-non-eabi. > > OK? Ok. Richard. > Christophe. > > > >> Thanks, > >> James > >> > >>> > Resolving it by hacking the testcase would be one fix, but I wonder why > >>> > the > >>> > ARM port prefers to emit "b = a" in a linux environment if .set does the > >>> > same thing and always avoids the warning? Maybe > >>> > Ramana/Richard/Kyrill/Nick > >>> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't > >>> > have the PR1
Re: [PATCH] Fix PR70054
On Fri, 4 Mar 2016, Jeff Law wrote:
> On 03/03/2016 05:35 AM, Richard Biener wrote:
> >
> > The following patch adjusts strict_aliasing_warning to use
> > proper alias_set_subset_of instead of relying on alias_sets_conflict_p
> > as after the PR66110 fix aggregates with a char[] member do not
> > automatically behave like having alias-set zero. As a side-effect
> > the test will be somewhat stricter as well accessing a 'long' with
> > a struct { int i; long l; } * will now warn while it previously
> > didn't:
> >
> > struct S { int i; long l; };
> > long x;
> > struct S foo () { return *(struct S *)&x; }
> >
> > now warns:
> >
> > t.c: In function ‘foo’:
> > t.c:3:35: warning: dereferencing type-punned pointer will break
> > strict-aliasing rules [-Wstrict-aliasing]
> > struct S foo () { return *(struct S *)&x; }
> > ^
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?
> >
> > Thanks,
> > Richard.
> >
> > 2016-03-03 Richard Biener
> >
> > PR c++/70054
> > * c-common.c (strict_aliasing_warning): Use alias_set_subset_of
> > instead of alias_sets_conflict_p.
> >
> > * g++.dg/warn/Wstrict-aliasing-bogus-union-2.C: New testcase.
> > * gcc.dg/Wstrict-aliasing-struct-member.c: New testcase.
> The PR doesn't have a regression marker, but from reading the PR it's clearly
> a regression.
Yes, it is - sorry for not updating the bugzilla appropriately.
> OK for the trunk,
Thanks,
Richard.
[testsuite] Skip gcc.dg/Wno-frame-address.c on IA-64
Same reason as for the other architectures.
Tested on ia64-suse-linux, applied on the mainline as obvious.
2016-03-04 Eric Botcazou
* gcc.dg/Wno-frame-address.c: Skip on IA-64.
--
Eric BotcazouIndex: gcc.dg/Wno-frame-address.c
===
--- gcc.dg/Wno-frame-address.c (revision 233957)
+++ gcc.dg/Wno-frame-address.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-skip-if "Cannot access arbitrary stack frames" { arm*-*-* hppa*-*-* visium-*-* } } */
+/* { dg-skip-if "Cannot access arbitrary stack frames" { arm*-*-* hppa*-*-* ia64-*-* visium-*-* } } */
/* { dg-options "-Werror" } */
/* { dg-additional-options "-mbackchain" { target { s390*-*-* } } } */
Re: [PATCH]Replace -shared with -r -nostdlib in gcc.dg/lto/pr61526 pr54709 pr64415 test cases.
On Thu, Mar 3, 2016 at 6:53 PM, Renlin Li wrote:
> Hi Richard,
>
>
> On 03/03/16 12:47, Richard Biener wrote:
>>
>> On Thu, Mar 3, 2016 at 1:07 PM, Renlin Li wrote:
>>>
>>> Hi Richard,
>>>
>>>
>>> On 03/03/16 10:13, Richard Biener wrote:
On Wed, Mar 2, 2016 at 5:12 PM, Renlin Li
wrote:
>
> Hi Richard,
>
>
> On 02/03/16 13:35, Richard Biener wrote:
>>
>> On Tue, Mar 1, 2016 at 4:56 PM, Renlin Li
>> wrote:
>>>
>>> Hi Richard,
>>>
>>>
>>> On 01/03/16 09:16, Richard Biener wrote:
On Mon, Feb 29, 2016 at 5:13 PM, Renlin Li
wrote:
>
> Hi all,
>
> The gcc.dg/lto/pr54709, pr61526, pr64415 linking testcases keep
> failing
> on
> arm/aarch64 bare-metal target.
>
> It's because statically built newlib library is used to link with
> shared
> object.
> And the linker complains about relocations which cannot be used in
> shared object.
>
> For example, the following errors are produced:
>
> crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol'
> can
> not
> be
> used when making a shared object; recompile with -fPIC
>
> crtbegin.o: relocation R_ARM_THM_MOVW_ABS_NC against `a local
> symbol'
> can
> not
> be used when making a shared object; recompile with -fPIC
>
> librdimon.a(rdimon-syscalls.o): relocation
> R_AARCH64_ADR_PREL_PG_HI21
> against
> external symbol `_impure_ptr' can not be used when making a shared
> object;
> recompile with -fPIC
>
> Presumably, bare-metal toolchain for other architecture have those
> test
> case
> failures as well?
>
> In this patch, -shared option is replace by -r -nostdlib. So that
> the
> standard
> system startup files or libraries are not used when linking.
Note that -shared is not equivalent to -r -nostdlib so please verify
that
the
original issue can be still reproduced with its fix reverted but -r
-nostdlib
used with the new -r -nostdlib handling on trunk.
>>>
>>>
>>> pr54709_0.c: Cannot be reproduced with even -shared. The error
>>> message
>>> is
>>> the same as shown above.
>>> pr64415_0.c: Reproduced with "-r -nostdlib".
>>> pr61526_0.c: Reproduced with "-r -nostdlib".
>>>
>>> By the way, those linking test cases all pass for linux toolchain.
>>> Only
>>> fail
>>> for aarch64/arm baremetal toolchain.
>>>
>>> Andrew, I saw you have done similar things in r153555
>>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=153555
>>>
>>> Do you have any thoughts?
>>>
>>> And also here, the last comments in this ticket suggests to add
>>> check_effective_target_shared to the exp file to limit it to linux
>>> targets
>>> only:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61526
>>
>> As said LTO testcases tend to be somewhat fragile so limiting them to
>> targets known to work might be the best option.
>>
>> Richard.
>
>
> Forgot the mention that, by purely adding "-nostdlib" option (instead
> of
> replacing -shared)
> fixes the failures as well.
>
> I test those test cases again with fix reverted, keep "-shared" option,
> add
> "-nostdlib" option.
Ok, so I discovered we have a "shared" target which means if a target
doesn't
support shared libs we can guard against it with using
/* { dg-require-effective-target shared } */
does adding that to the three testcases fix the issue for you?
>>>
>>> By adding this target check
>>> /* { dg-require-effective-target shared } */
>>>
>>> Those test cases aredeemed to be unsupported, and thus skipped for
>>> aarch64-none-elf target.
>>>
>>> However, it's a little bit tricky for arm bare-metal target.
>>>
>>> The shared option check actually successes for arm-none-eabi toolchain.
>>> This is because the default cpu for arm-none-eabi toolchain is arm7tdmi.
>>> And
>>> the start file crtbegin.o doesn't contains any modifications not allowed
>>> in
>>> shared object.
>>>
>>> arm-none-eabi is built with multilib. When running this testcase, it's
>>> compiled with "-march=armv7-a".
>>> The crtbegin.o for this architecture version contains relocations which
>>> cannot be used in shared object.
>>> That's why they fails to linking test.
>>
>> For -shared it should provide a crtbeginS.o then. Why not fix it
>> properly?
>>
>> Richard.
>
>
> That's the case for linux toolchain. Multiple versions of startfile are
> generated.
> crtbegin.o, crtbeginS.o, crtbeginT.o etc.
>
> If I understand it correctly, this is not applicable to bare-metal
> t
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Thu, Mar 3, 2016 at 9:16 PM, Jakub Jelinek wrote:
> Hi!
>
> Before my recent decide_alg change, *dynamic_check == -1 was indeed
> guaranteed, because any_alg_usable_p doesn't depend on the arguments of
> decide_alg that might change during recursive call, so we'd only recurse if
> it wouldn't set *dynamic_check. But, if we give up because we'd otherwise
> recurse infinitely, we can set *dynamic_check to 128.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2016-03-03 Jakub Jelinek
>
> PR target/70062
> * config/i386/i386.c (decide_alg): If
> TARGET_INLINE_STRINGOPS_DYNAMICALLY, allow *dynamic_check to be also
> 128 from the recursive call.
>
> * gcc.target/i386/pr70062.c: New test.
I don't like the fact that *dynamic_check is set to max (which is 0
with your testcase) when recursion avoidance code already set it to
"something reasonable", together with loop_1_byte alg. What do you
think about attached (lightly tested) patch?
Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8a026ae..ded9951 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT
expected_size,
}
alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
zero_memset, have_as, dynamic_check, noalign);
- gcc_assert (*dynamic_check == -1);
+
if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
- *dynamic_check = max;
+ {
+ /* *dynamic_check could be set to 128 above because we avoided
+infinite recursion. */
+ if (*dynamic_check == 128)
+ gcc_assert (alg == loop_1_byte);
+ else
+ {
+ gcc_assert (*dynamic_check == -1);
+ *dynamic_check = max;
+ }
+ }
else
- gcc_assert (alg != libcall);
+ gcc_assert (alg != libcall && *dynamic_check == -1);
+
return alg;
}
return (alg_usable_p (algs->unknown_size, memset, have_as)
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
> I don't like the fact that *dynamic_check is set to max (which is 0
> with your testcase) when recursion avoidance code already set it to
> "something reasonable", together with loop_1_byte alg. What do you
> think about attached (lightly tested) patch?
But that can still set *dynamic_check to 0 if the recursive call has
not set *dynamic_check.
So, perhaps we want *dynamic_check = max ? max : 128;
?
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 8a026ae..ded9951 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT
> expected_size,
> }
>alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
> zero_memset, have_as, dynamic_check, noalign);
> - gcc_assert (*dynamic_check == -1);
> +
>if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> - *dynamic_check = max;
> + {
> + /* *dynamic_check could be set to 128 above because we avoided
> + infinite recursion. */
> + if (*dynamic_check == 128)
> + gcc_assert (alg == loop_1_byte);
> + else
> + {
> + gcc_assert (*dynamic_check == -1);
> + *dynamic_check = max;
> + }
> + }
>else
> - gcc_assert (alg != libcall);
> + gcc_assert (alg != libcall && *dynamic_check == -1);
> +
>return alg;
> }
>return (alg_usable_p (algs->unknown_size, memset, have_as)
Jakub
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> I don't like the fact that *dynamic_check is set to max (which is 0
>> with your testcase) when recursion avoidance code already set it to
>> "something reasonable", together with loop_1_byte alg. What do you
>> think about attached (lightly tested) patch?
>
> But that can still set *dynamic_check to 0 if the recursive call has
> not set *dynamic_check.
> So, perhaps we want *dynamic_check = max ? max : 128;
> ?
I believe that recursive call set *dynamic_check to zero for a reason.
The sent patch deals with recursion breaking issues only, leaving all
other functionality as it was before. So, your issue is IMO orthogonal
to the PR70062 and should be fixed (if at all) in a separate patch.
Uros.
>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 8a026ae..ded9951 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT
>> expected_size,
>> }
>>alg = decide_alg (count, new_expected_size, min_size, max_size,
>> memset,
>> zero_memset, have_as, dynamic_check, noalign);
>> - gcc_assert (*dynamic_check == -1);
>> +
>>if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
>> - *dynamic_check = max;
>> + {
>> + /* *dynamic_check could be set to 128 above because we avoided
>> + infinite recursion. */
>> + if (*dynamic_check == 128)
>> + gcc_assert (alg == loop_1_byte);
>> + else
>> + {
>> + gcc_assert (*dynamic_check == -1);
>> + *dynamic_check = max;
>> + }
>> + }
>>else
>> - gcc_assert (alg != libcall);
>> + gcc_assert (alg != libcall && *dynamic_check == -1);
>> +
>>return alg;
>> }
>>return (alg_usable_p (algs->unknown_size, memset, have_as)
>
>
> Jakub
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote: > On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek wrote: > > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote: > >> I don't like the fact that *dynamic_check is set to max (which is 0 > >> with your testcase) when recursion avoidance code already set it to > >> "something reasonable", together with loop_1_byte alg. What do you > >> think about attached (lightly tested) patch? > > > > But that can still set *dynamic_check to 0 if the recursive call has > > not set *dynamic_check. > > So, perhaps we want *dynamic_check = max ? max : 128; > > ? > > I believe that recursive call set *dynamic_check to zero for a reason. > The sent patch deals with recursion breaking issues only, leaving all > other functionality as it was before. So, your issue is IMO orthogonal > to the PR70062 and should be fixed (if at all) in a separate patch. The recursive call should never set *dynamic_check to 0, only to -1 or 128 (the last one newly, since my fix). And, my issue is IMO not orghogonal to that, either *dynamic_check == 0 is ok, or it is not. Perhaps better would be to add an extra argument to decide_alg, false for toplevel invocation, true for recursive invocation, and if recursive call, just never try to recurse again (thus we could revert the previous change) and don't set *dynamic_check to anything in that case during the recursion. And then at the caller side decide what we want for max == -1 and what for max == 0 with *dynamic_check. Jakub
Re: [PATCH][AArch64][testsuite] PR target/70004: Remove check using undefined behaviour
On 02/03/16 16:17, Jakub Jelinek wrote:
On Tue, Mar 01, 2016 at 02:29:19PM +, Kyrill Tkachov wrote:
This test was reported as starting to fail a scan-assembler check with
-mtune=thunderx.
The compiler decided to move the result back into the integer registers and
perform the shift there
rather than do it on the SIMD side.
However, the C code is undefined because the shift is wider than the type. GCC
even warns for it, but the
test catches it with dg-warning.
I don't think it's correct for the test to rely on undefined code, so the
simplest thing to do is to delete
the undefined code.
Ok for trunk?
Thanks,
Kyrill
2016-03-01 Kyrylo Tkachov
PR target/70004
* gcc.target/aarch64/scalar_shift_1.c: (test_corners_sisd_di):
Delete.
(test_corners_sisd_si): Likewise.
(main): Remove checks of the above.
I think you should extend this patch, by adding the removed functions
into another, dg-do compile only test without any scan-assembler stuff in
there, just to make sure you don't ICE on it and emit the warnings that
should be emitted, and possibly by keeping the well defined corner case
stuff in there instead of removing the functions altogether. b >> 63 and
b >> 0 are both well defined, similarly b >> 31 and b >> 0 in the other
function.
Can do. I've moved the dodgy functions into their own separate compile test.
The test passes.
Ok?
Thanks,
Kyrill
2016-03-04 Kyrylo Tkachov
PR target/70004
* gcc.target/aarch64/scalar_shift_1.c: (test_corners_sisd_di):
Delete.
(test_corners_sisd_si): Likewise.
(main): Remove checks of the above.
* gcc.target/aarch64/shift_wide_invalid_1.c: New test.
diff --git a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
index
8465c896705dbfd4c76b0815511ea7b4b034e095..7be1b12a75bf9f201644aef471c5edb99979c0c5
100644
--- a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
@@ -181,34 +181,6 @@ test_ashift_right_int_si (Int32x1 b, Int32x1 c)
/* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ 4" } } */
/* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ w\[0-9\]+" } } */
-Int64x1
-test_corners_sisd_di (Int64x1 b)
-{
- force_simd_di (b);
- b = b >> 63;
- force_simd_di (b);
- b = b >> 0;
- b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
-
- return b;
-}
-/* { dg-final { scan-assembler "sshr\td\[0-9\]+,\ d\[0-9\]+,\ 63" } } */
-
-Int32x1
-test_corners_sisd_si (Int32x1 b)
-{
- force_simd_si (b);
- b = b >> 31;
- force_simd_si (b);
- b = b >> 0;
- b += b >> 33; /* { dg-warning "right shift count >= width of type" } */
-
- return b;
-}
-/* { dg-final { scan-assembler "sshr\tv\[0-9\]+\.2s,\ v\[0-9\]+\.2s,\ 31" } }
*/
-
-
-
#define CHECK(var,val) \
do \
{\
@@ -236,8 +208,6 @@ main ()
CHECK (x, 0x21524110ull);
x = test_ashift_right_sisd_di (x, 8);
CHECK (x, 0x2152ull);
- x = test_corners_sisd_di (x);
- CHECK (x, 0xfffeull);
y = test_lshift_left_sisd_si (y, 4);
CHECK (y, 0xadbeef00);
@@ -252,8 +222,6 @@ main ()
CHECK (y, 0x5241);
y = test_ashift_right_sisd_si (y, 4);
CHECK (y, 0xff52);
- y = test_corners_sisd_si (y);
- CHECK (y, 0xfffe);
return 0;
}
Jakub
diff --git a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
index 8465c896705dbfd4c76b0815511ea7b4b034e095..7be1b12a75bf9f201644aef471c5edb99979c0c5 100644
--- a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
@@ -181,34 +181,6 @@ test_ashift_right_int_si (Int32x1 b, Int32x1 c)
/* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ 4" } } */
/* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ w\[0-9\]+" } } */
-Int64x1
-test_corners_sisd_di (Int64x1 b)
-{
- force_simd_di (b);
- b = b >> 63;
- force_simd_di (b);
- b = b >> 0;
- b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
-
- return b;
-}
-/* { dg-final { scan-assembler "sshr\td\[0-9\]+,\ d\[0-9\]+,\ 63" } } */
-
-Int32x1
-test_corners_sisd_si (Int32x1 b)
-{
- force_simd_si (b);
- b = b >> 31;
- force_simd_si (b);
- b = b >> 0;
- b += b >> 33; /* { dg-warning "right shift count >= width of type" } */
-
- return b;
-}
-/* { dg-final { scan-assembler "sshr\tv\[0-9\]+\.2s,\ v\[0-9\]+\.2s,\ 31" } } */
-
-
-
#define CHECK(var,val) \
do \
{\
@@ -236,8 +208,6 @@ main ()
CHECK (x, 0x21524110ull);
x = test_ashift_right_sisd_di (x, 8);
CHECK (x, 0x2152ull);
- x = test_corners_sisd_di (x);
- CHECK (x, 0xfffeull);
y = test_lshift_left_sisd_si (y, 4);
CHECK (y, 0xadbeef00);
@@ -252,8 +222,6 @@ main ()
CHECK (y, 0x5241);
y = test_ashift_right_sisd_si
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek wrote: > On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote: >> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek wrote: >> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote: >> >> I don't like the fact that *dynamic_check is set to max (which is 0 >> >> with your testcase) when recursion avoidance code already set it to >> >> "something reasonable", together with loop_1_byte alg. What do you >> >> think about attached (lightly tested) patch? >> > >> > But that can still set *dynamic_check to 0 if the recursive call has >> > not set *dynamic_check. >> > So, perhaps we want *dynamic_check = max ? max : 128; >> > ? >> >> I believe that recursive call set *dynamic_check to zero for a reason. >> The sent patch deals with recursion breaking issues only, leaving all >> other functionality as it was before. So, your issue is IMO orthogonal >> to the PR70062 and should be fixed (if at all) in a separate patch. > > The recursive call should never set *dynamic_check to 0, only to > -1 or 128 (the last one newly, since my fix). > And, my issue is IMO not orghogonal to that, either *dynamic_check == 0 > is ok, or it is not. > Perhaps better would be to add an extra argument to decide_alg, false > for toplevel invocation, true for recursive invocation, and if recursive > call, just never try to recurse again (thus we could revert the previous > change) and don't set *dynamic_check to anything in that case during the > recursion. > And then at the caller side decide what we want for max == -1 and what > for max == 0 with *dynamic_check. I think that would work, and considering that we have only one non-recursive callsite of decide_alg, it looks like the cleanest solution. Uros.
Re: [PATCH][AArch64][testsuite] PR target/70004: Remove check using undefined behaviour
On Fri, Mar 04, 2016 at 10:41:04AM +, Kyrill Tkachov wrote: > Can do. I've moved the dodgy functions into their own separate compile test. > The test passes. > Ok? LGTM. Jakub
Re: [PATCH][AArch64][testsuite] PR target/70004: Remove check using undefined behaviour
On Fri, Mar 04, 2016 at 12:02:59PM +0100, Jakub Jelinek wrote: > On Fri, Mar 04, 2016 at 10:41:04AM +, Kyrill Tkachov wrote: > > Can do. I've moved the dodgy functions into their own separate compile test. > > The test passes. > > Ok? > > LGTM. OK from me too. James
[PATCH PR69052]Set higher precedence for CONST_WIDE_INT than CONST_INT when canonicalizing addr expr
Hi, A address canonicalization interface was introduced by my original patch to PR69052. The interface sorts sub-parts in an address expression based on precedences defined by function commutative_operand_precedence. It also assumes that all CONST_INT sub-parts are at the end of vector after sorting. But this is not always true because commutative_operand_precedence sets the same precedence to both CONST_INT and CONST_WIDE_INT. The patch could be broken in cases which have CONST_WIDE_INT sorted after CONST_INT. Even though I don't think we can really run into such complicated case, I worked out a patch forcing CONST_INT to lower precedence than CONST_WIDE_INT, so that for sure it will be sorted after all other kinds sub-parts. This is an obvious change. Bootstrap&test on x86_64, bootstrap&test on AArch64. Is it OK for this stage? Thanks, bin 2016-03-04 Bin Cheng PR rtl-optimization/69052 * loop-invariant.c (compare_address_parts): Force CONST_INT to lower precedence than CONST_WIDE_INT. diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c index dcbe932..27a1815 100644 --- a/gcc/loop-invariant.c +++ b/gcc/loop-invariant.c @@ -820,6 +820,12 @@ compare_address_parts (const void *x, const void *y) int px = commutative_operand_precedence (*rx); int py = commutative_operand_precedence (*ry); + /* Put CONST_INT behind CONST_WIDE_INT. */ + if (CONST_INT_P (*rx) && CONST_WIDE_INT_P (*ry)) +return 1; + else if (CONST_WIDE_INT_P (*rx) && CONST_INT_P (*ry)) +return -1; + return (py - px); }
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 04, 2016 at 11:42:34AM +0100, Uros Bizjak wrote:
> On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek wrote:
> > On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
> >> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek wrote:
> >> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
> >> >> I don't like the fact that *dynamic_check is set to max (which is 0
> >> >> with your testcase) when recursion avoidance code already set it to
> >> >> "something reasonable", together with loop_1_byte alg. What do you
> >> >> think about attached (lightly tested) patch?
> >> >
> >> > But that can still set *dynamic_check to 0 if the recursive call has
> >> > not set *dynamic_check.
> >> > So, perhaps we want *dynamic_check = max ? max : 128;
> >> > ?
> >>
> >> I believe that recursive call set *dynamic_check to zero for a reason.
> >> The sent patch deals with recursion breaking issues only, leaving all
> >> other functionality as it was before. So, your issue is IMO orthogonal
> >> to the PR70062 and should be fixed (if at all) in a separate patch.
> >
> > The recursive call should never set *dynamic_check to 0, only to
> > -1 or 128 (the last one newly, since my fix).
> > And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
> > is ok, or it is not.
> > Perhaps better would be to add an extra argument to decide_alg, false
> > for toplevel invocation, true for recursive invocation, and if recursive
> > call, just never try to recurse again (thus we could revert the previous
> > change) and don't set *dynamic_check to anything in that case during the
> > recursion.
> > And then at the caller side decide what we want for max == -1 and what
> > for max == 0 with *dynamic_check.
>
> I think that would work, and considering that we have only one
> non-recursive callsite of decide_alg, it looks like the cleanest
> solution.
So like this?
2016-03-03 Jakub Jelinek
PR target/70062
* config/i386/i386.c (decide_alg): Add RECUR argument. Revert
2016-02-22 changes, instead don't recurse if RECUR is already true.
Don't change *dynamic_check if RECUR. Adjust recursive caller
to pass true to the new argument.
(ix86_expand_set_or_movmem): Adjust decide_alg caller.
* gcc.target/i386/pr70062.c: New test.
--- gcc/config/i386/i386.c.jj 2016-03-03 21:49:38.535744904 +0100
+++ gcc/config/i386/i386.c 2016-03-04 12:06:40.075300426 +0100
@@ -26032,14 +26032,13 @@ static enum stringop_alg
decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
unsigned HOST_WIDE_INT min_size, unsigned HOST_WIDE_INT max_size,
bool memset, bool zero_memset, bool have_as,
- int *dynamic_check, bool *noalign)
+ int *dynamic_check, bool *noalign, bool recur)
{
const struct stringop_algs *algs;
bool optimize_for_speed;
int max = 0;
const struct processor_costs *cost;
int i;
- HOST_WIDE_INT orig_expected_size = expected_size;
bool any_alg_usable_p = false;
*noalign = false;
@@ -26157,19 +26156,18 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
enum stringop_alg alg;
HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
- /* If there aren't any usable algorithms or if recursing with the
-same arguments as before, then recursing on smaller sizes or
-same size isn't going to find anything. Just return the simple
-byte-at-a-time copy loop. */
- if (!any_alg_usable_p || orig_expected_size == new_expected_size)
-{
- /* Pick something reasonable. */
- if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
-*dynamic_check = 128;
- return loop_1_byte;
-}
+ /* If there aren't any usable algorithms or if recursing already,
+then recursing on smaller sizes or same size isn't going to
+find anything. Just return the simple byte-at-a-time copy loop. */
+ if (!any_alg_usable_p || recur)
+ {
+ /* Pick something reasonable. */
+ if (TARGET_INLINE_STRINGOPS_DYNAMICALLY && !recur)
+ *dynamic_check = 128;
+ return loop_1_byte;
+ }
alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
- zero_memset, have_as, dynamic_check, noalign);
+ zero_memset, have_as, dynamic_check, noalign, true);
gcc_assert (*dynamic_check == -1);
if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
*dynamic_check = max;
@@ -26430,7 +26428,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx
alg = decide_alg (count, expected_size, min_size, probable_max_size,
issetmem,
issetmem && val_exp == const0_rtx, have_as,
- &dynamic_check, &noalign);
+ &dynamic_check, &noalign, false);
if (alg == libcall)
return false;
gcc_assert (alg != no_stringop);
--- gcc/testsuite/gcc.target/i386/pr70062.c.jj 2016-03
Re: [PATCH] Another fix for decide_alg (PR target/70062)
On Fri, Mar 4, 2016 at 12:11 PM, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 11:42:34AM +0100, Uros Bizjak wrote:
>> On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek wrote:
>> > On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
>> >> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek wrote:
>> >> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> >> >> I don't like the fact that *dynamic_check is set to max (which is 0
>> >> >> with your testcase) when recursion avoidance code already set it to
>> >> >> "something reasonable", together with loop_1_byte alg. What do you
>> >> >> think about attached (lightly tested) patch?
>> >> >
>> >> > But that can still set *dynamic_check to 0 if the recursive call has
>> >> > not set *dynamic_check.
>> >> > So, perhaps we want *dynamic_check = max ? max : 128;
>> >> > ?
>> >>
>> >> I believe that recursive call set *dynamic_check to zero for a reason.
>> >> The sent patch deals with recursion breaking issues only, leaving all
>> >> other functionality as it was before. So, your issue is IMO orthogonal
>> >> to the PR70062 and should be fixed (if at all) in a separate patch.
>> >
>> > The recursive call should never set *dynamic_check to 0, only to
>> > -1 or 128 (the last one newly, since my fix).
>> > And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
>> > is ok, or it is not.
>> > Perhaps better would be to add an extra argument to decide_alg, false
>> > for toplevel invocation, true for recursive invocation, and if recursive
>> > call, just never try to recurse again (thus we could revert the previous
>> > change) and don't set *dynamic_check to anything in that case during the
>> > recursion.
>> > And then at the caller side decide what we want for max == -1 and what
>> > for max == 0 with *dynamic_check.
>>
>> I think that would work, and considering that we have only one
>> non-recursive callsite of decide_alg, it looks like the cleanest
>> solution.
>
> So like this?
Yes, this looks like much better and cleaner solution.
> 2016-03-03 Jakub Jelinek
>
> PR target/70062
> * config/i386/i386.c (decide_alg): Add RECUR argument. Revert
> 2016-02-22 changes, instead don't recurse if RECUR is already true.
> Don't change *dynamic_check if RECUR. Adjust recursive caller
> to pass true to the new argument.
> (ix86_expand_set_or_movmem): Adjust decide_alg caller.
>
> * gcc.target/i386/pr70062.c: New test.
OK.
Thanks,
Uros.
> --- gcc/config/i386/i386.c.jj 2016-03-03 21:49:38.535744904 +0100
> +++ gcc/config/i386/i386.c 2016-03-04 12:06:40.075300426 +0100
> @@ -26032,14 +26032,13 @@ static enum stringop_alg
> decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
> unsigned HOST_WIDE_INT min_size, unsigned HOST_WIDE_INT max_size,
> bool memset, bool zero_memset, bool have_as,
> - int *dynamic_check, bool *noalign)
> + int *dynamic_check, bool *noalign, bool recur)
> {
>const struct stringop_algs *algs;
>bool optimize_for_speed;
>int max = 0;
>const struct processor_costs *cost;
>int i;
> - HOST_WIDE_INT orig_expected_size = expected_size;
>bool any_alg_usable_p = false;
>
>*noalign = false;
> @@ -26157,19 +26156,18 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
>enum stringop_alg alg;
>HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
>
> - /* If there aren't any usable algorithms or if recursing with the
> -same arguments as before, then recursing on smaller sizes or
> -same size isn't going to find anything. Just return the simple
> -byte-at-a-time copy loop. */
> - if (!any_alg_usable_p || orig_expected_size == new_expected_size)
> -{
> - /* Pick something reasonable. */
> - if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> -*dynamic_check = 128;
> - return loop_1_byte;
> -}
> + /* If there aren't any usable algorithms or if recursing already,
> +then recursing on smaller sizes or same size isn't going to
> +find anything. Just return the simple byte-at-a-time copy loop. */
> + if (!any_alg_usable_p || recur)
> + {
> + /* Pick something reasonable. */
> + if (TARGET_INLINE_STRINGOPS_DYNAMICALLY && !recur)
> + *dynamic_check = 128;
> + return loop_1_byte;
> + }
>alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
> - zero_memset, have_as, dynamic_check, noalign);
> + zero_memset, have_as, dynamic_check, noalign, true);
>gcc_assert (*dynamic_check == -1);
>if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> *dynamic_check = max;
> @@ -26430,7 +26428,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx
>alg = decide_alg (count, expected_size, min_size, probable_max_size,
> issetmem,
>
Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)
On Fri, Mar 04, 2016 at 07:27:51AM +0100, Jakub Jelinek wrote:
> The difference is that c_parser_cast_expression can parse (type)...,
> ++..., --..., &..., +..., -..., *..., ~..., !..., &&...,
> sizeof/alignof/__extension__/__real/__imag/__transaction_{atomic,relaxed}...
> Perhaps even safer version would be:
> if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
> && c_token_starts_typename (c_parser_peek_2nd_token (parser)))
> {
> ... error handling ...
> }
> else
> ... c_parser_postfix_expression (parser);
This occurred to me too, but I found that too ugly, and I'm not even sure it
prevents other possible ICEs. Plus we'd need to put this new check into three
places, so I dropped this idea.
> But, even c_parser_postfix_expression parses tons of expressions that
> aren't allowed there, so I think it is overkill. It needs to check
> afterwards, and from what I can see that happens in cilk_set_spawn_marker.
> Thus I think c_parser_cast_expression is safe, it parses perhaps more
> unexpected expressions, but will reject them all anyway, because they aren't
> CALL_EXPR after parsing.
Yeah, that's right.
Marek
Re: [PATCH, PR70026] Fix boolean comparison processing in masks conversion patterns
On Wed, Mar 2, 2016 at 5:14 PM, Ilya Enkovich wrote:
> Hi,
>
> This patch fixes mask type determination for boolean values comparison. That
> avoid incorrect application of masks conversion pattern. Bootstrapped and
> tested on x86_64-pc-linux-gnu. OK for trunk?
Ok.
Richard.
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-03-02 Ilya Enkovich
>
> * tree-vect-patterns.c (search_type_for_mask): Handle
> comparison of booleans.
>
>
> gcc/testsuite/
>
> 2016-03-02 Ilya Enkovich
>
> * gcc.dg/pr70026.c: New test.
>
>
> diff --git a/gcc/testsuite/gcc.dg/pr70026.c b/gcc/testsuite/gcc.dg/pr70026.c
> new file mode 100644
> index 000..32f59e2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr70026.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +unsigned int a[64], b[64], c[64], d[64], e[64];
> +
> +void
> +foo ()
> +{
> + int i;
> + for (i = 0; i < 64; i++)
> +{
> + d[i] = a[i];
> + e[i] = ((b[i] < e[i]) != !c[i]) && !a[i];
> +}
> +}
> diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
> index 95ce38d..7037e04 100644
> --- a/gcc/tree-vect-patterns.c
> +++ b/gcc/tree-vect-patterns.c
> @@ -3219,6 +3219,15 @@ search_type_for_mask (tree var, vec_info *vinfo)
> {
> tree comp_vectype, mask_type;
>
> + if (TREE_CODE (TREE_TYPE (rhs1)) == BOOLEAN_TYPE)
> + {
> + res = search_type_for_mask (rhs1, vinfo);
> + res2 = search_type_for_mask (gimple_assign_rhs2 (def_stmt),
> vinfo);
> + if (!res || (res2 && TYPE_PRECISION (res) > TYPE_PRECISION
> (res2)))
> + res = res2;
> + break;
> + }
> +
> comp_vectype = get_vectype_for_scalar_type (TREE_TYPE (rhs1));
> if (comp_vectype == NULL_TREE)
> return NULL_TREE;
Re: [PATCH PR69052]Set higher precedence for CONST_WIDE_INT than CONST_INT when canonicalizing addr expr
On Fri, Mar 4, 2016 at 12:07 PM, Bin Cheng wrote: > Hi, > A address canonicalization interface was introduced by my original patch to > PR69052. The interface sorts sub-parts in an address expression based on > precedences defined by function commutative_operand_precedence. It also > assumes that all CONST_INT sub-parts are at the end of vector after sorting. > But this is not always true because commutative_operand_precedence sets the > same precedence to both CONST_INT and CONST_WIDE_INT. The patch could be > broken in cases which have CONST_WIDE_INT sorted after CONST_INT. Even > though I don't think we can really run into such complicated case, I worked > out a patch forcing CONST_INT to lower precedence than CONST_WIDE_INT, so > that for sure it will be sorted after all other kinds sub-parts. > > This is an obvious change. Bootstrap&test on x86_64, bootstrap&test on > AArch64. Is it OK for this stage? I believe the obvious change would be to change commutative_operand_precedence to pre-CONST_WIDE_INT behavior, namely giving CONST_WIDE_INT the same precedence as CONST_DOUBLE. Richard. > Thanks, > bin > > 2016-03-04 Bin Cheng > > PR rtl-optimization/69052 > * loop-invariant.c (compare_address_parts): Force CONST_INT to lower > precedence than CONST_WIDE_INT.
Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)
On Thu, Mar 03, 2016 at 11:31:08PM -0700, Jeff Law wrote: > Do we need to do anything for the call into c_parser_postfix_expression that > occurs between the two you changed in this patch. > > I think you can get into that code with something like > > _Cilk_spawn _Cilk_spawn ()); Absolutely. Dunno how I missed that. Will fix in a new version of the patch... > For the non-error case (the second one you changed), I think we do want to > verify that the expression we got back was a function call and issue an > appropriate error otherwise. You could make an argument that we should > issue a diagnostic for the other cases as well, but it's less obviously > correct. As noted elsewhere, cilk_set_spawn_marker rejects everything except CALL_EXPR. Marek
Re: [PATCH 2/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #1)
On 03/03/2016 06:15 PM, Patrick Palka wrote: Cool, this also fixes the false-positives seen in bdwgc, whose coding style suggests indenting things inside an #ifdef as if it were an if(), e.g.: if (a) foo (); # ifndef A bar (); # endif ... Once again I find myself thinking this is not a false positive, but terrible code we should warn for. Bernd
Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)
On Thu, Mar 03, 2016 at 10:51:14PM -0700, Jeff Law wrote:
> On 03/03/2016 07:15 AM, Marek Polacek wrote:
> >This is ICE on invalid Cilk+ code. cilk_spawn expects a function call, so
> >e.g.
> >_Cilk_spawn (void) is invalid. The function call after the cilk_spawn
> >keyword
> >is parsed using recursive call in c_parser_postfix_expression (case
> >RID_CILK_SPAWN). Now, c_parser_postfix_expression sees '(' followed by a
> >typename, so it thinks we're inside a compound literal, which means it
> >expects
> >'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
> >gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
> >But as the comment in c_parser_postfix_expression says, the code for parsing
> >a compound literal here is likely dead. I made an experiment and added
> >gcc_unreachable () in that block, ran regtest, and there were no failures.
> >Thus it should be safe to just remove the code, which also fixes this ICE;
> >with
> >the patch we just give a proper error and don't crash anymore.
> >
> >Bootstrapped/regtested on x86_64-linux, ok for trunk? I'm actually slightly
> >nervous about the change, so maybe better table until gcc7?
> >
> >2016-03-03 Marek Polacek
> >
> > PR c/69798
> > * c-parser.c (c_parser_postfix_expression): Remove code dealing with
> > compound literals.
> >
> > * gcc.dg/cilk-plus/pr69798-1.c: New test.
> > * gcc.dg/cilk-plus/pr69798-2.c: New test.
> FWIW, I don't particularly like this version. My worry about just removing
> this code is that I expect our testsuite doesn't cover parsing issues all
> that well.
>
> I'd feel better if it'd been run through one of the commercial suites. Even
> then those suites don't cover the GNU extensions, but at least the scope of
> things that might go wrong is significantly diminished.
I agree but unfortunately I don't have the means to run such a suite. Compound
literals are ISO C99 thing, so hopefully they are well-covered in those test
suites.
Ultimately I want to either do away with the code, or add tests that exercise
the codepath.
Marek
Re: [PATCH 2/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #1)
On Fri, Mar 04, 2016 at 01:53:20PM +0100, Bernd Schmidt wrote: > On 03/03/2016 06:15 PM, Patrick Palka wrote: > >Cool, this also fixes the false-positives seen in bdwgc, whose coding > >style suggests indenting things inside an #ifdef as if it were an > >if(), e.g.: > > > > if (a) > > foo (); > ># ifndef A > > bar (); > ># endif > > ... > > Once again I find myself thinking this is not a false positive, but terrible > code we should warn for. I agree. It seems entirely sane to warn here. Marek
Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798) (take 3)
On Fri, Mar 04, 2016 at 07:41:10AM +0100, Jakub Jelinek wrote:
> On Thu, Mar 03, 2016 at 11:31:08PM -0700, Jeff Law wrote:
> > >2016-03-03 Marek Polacek
> > >
> > > PR c/69798
> > > * c-parser.c (c_parser_postfix_expression): Call
> > > c_parser_cast_expression instead of c_parser_postfix_expression.
> > >
> > > * gcc.dg/cilk-plus/pr69798-1.c: New test.
> > > * gcc.dg/cilk-plus/pr69798-2.c: New test.
> > Do we need to do anything for the call into c_parser_postfix_expression that
> > occurs between the two you changed in this patch.
> >
> > I think you can get into that code with something like
> >
> > _Cilk_spawn _Cilk_spawn ());
>
> The _Cilk_spawn _Cilk_spawn (struct S);
> case needs the same treatment as the other 2 spots, sure, and should be also
> covered by a testcase. Perhaps another testcase should test the various
> other cases of the postfix vs. cast expression I've mentioned in the other
> mail, just make sure the diagnostics is reasonable and we don't ICE.
Okay, so the following is the latest version. I extended the tests and I also
changed the case for consecutive _Cilk_spawn keywords. And some whitespace
fixes while at it.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-03-04 Marek Polacek
PR c/69798
* c-parser.c (c_parser_postfix_expression): Call
c_parser_cast_expression rather than c_parser_postfix_expression.
* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index bb508b7..a7d5827 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -8024,8 +8024,8 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc, "-fcilkplus must be enabled to use "
"%<_Cilk_spawn%>");
- expr = c_parser_postfix_expression (parser);
- expr.value = error_mark_node;
+ expr = c_parser_cast_expression (parser, NULL);
+ expr.value = error_mark_node;
}
else if (c_parser_peek_token (parser)->keyword == RID_CILK_SPAWN)
{
@@ -8034,14 +8034,14 @@ c_parser_postfix_expression (c_parser *parser)
/* Now flush out all the _Cilk_spawns. */
while (c_parser_peek_token (parser)->keyword == RID_CILK_SPAWN)
c_parser_consume_token (parser);
- expr = c_parser_postfix_expression (parser);
+ expr = c_parser_cast_expression (parser, NULL);
}
else
{
- expr = c_parser_postfix_expression (parser);
+ expr = c_parser_cast_expression (parser, NULL);
expr.value = build_cilk_spawn (loc, expr.value);
}
- break;
+ break;
default:
c_parser_error (parser, "expected expression");
expr.value = error_mark_node;
diff --git gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
index e69de29..c5a37a8 100644
--- gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
+++ gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
@@ -0,0 +1,73 @@
+/* PR c/69798 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+extern int foo (void);
+
+void
+fn1 (int i, int *p)
+{
+l:
+ _Cilk_spawn (void); /* { dg-error "expected expression" } */
+ _Cilk_spawn (char []); /* { dg-error "expected expression" } */
+ _Cilk_spawn (int *); /* { dg-error "expected expression" } */
+ _Cilk_spawn (int) 1; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn ({}); /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn ++i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn i++; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn --i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn i--; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn &i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn +i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn -i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn ~i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn !i; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn *p; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn &&l; /* { dg-error "only function calls can be spawned" } */
+ _Cilk_spawn sizeof (i); /* { dg-error "only function calls can be spawned" }
*/
+ _Cilk_spawn sizeof (short); /* { dg-error "only function calls can be
spawned" } */
+ _Cilk_spawn __alignof__ (i); /* { dg-error "only function calls can be
spawned" } */
+ _Cilk_spawn __alignof__ (short); /* { dg-error "only function calls can be
spawned" } */
+ _Cilk_spawn __extension__ i; /* { dg-error "only function calls can be
spawned" } */
+ _Cilk_spawn __func__; /* { dg-error "only function calls can be spawned" } */
Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798) (take 3)
On Fri, Mar 04, 2016 at 02:12:33PM +0100, Marek Polacek wrote: > Okay, so the following is the latest version. I extended the tests and I also > changed the case for consecutive _Cilk_spawn keywords. And some whitespace > fixes while at it. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? Ok, thanks > 2016-03-04 Marek Polacek > > PR c/69798 > * c-parser.c (c_parser_postfix_expression): Call > c_parser_cast_expression rather than c_parser_postfix_expression. > > * gcc.dg/cilk-plus/pr69798-1.c: New test. > * gcc.dg/cilk-plus/pr69798-2.c: New test. Jakub
[PATCH 03/10] gcc/arc: generate jump tables in code section for nps400
When code runs from section loaded into fast memory we do not want it to use rodata section from a slow memory for any jump tables. This commit turns on CASE_VECTOR_PC_RELATIVE by default for NPS400 targets, which in turn turns on JUMP_TABLES_IN_TEXT_SECTION, which will place the jump tables into the code section. As a later optimisation we could be smarter about this, only turning on inline jump tables when the code section is not the default code section (.text), which we assume is not loaded into fast memory. gcc/ChangeLog: * config/arc/arc.opt (TARGET_CASE_VECTOR_PC_RELATIVE): Default on for NPS400. --- gcc/ChangeLog.NPS400 | 7 +++ gcc/config/arc/arc.opt | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400 index 0281640..716e413 100644 --- a/gcc/ChangeLog.NPS400 +++ b/gcc/ChangeLog.NPS400 @@ -1,3 +1,10 @@ +2015-09-08 Andrew Burgess + Joern Rennecke + Noam Camus + + * config/arc/arc.opt (TARGET_CASE_VECTOR_PC_RELATIVE): Default on + for NPS400. + 2016-02-02 Joern Rennecke Andrew Burgess diff --git a/gcc/config/arc/arc.opt b/gcc/config/arc/arc.opt index f8e062c..55a5b32 100644 --- a/gcc/config/arc/arc.opt +++ b/gcc/config/arc/arc.opt @@ -283,7 +283,7 @@ Target Var(TARGET_BBIT_PEEPHOLE) Enable bbit peephole2. mcase-vector-pcrel -Target Var(TARGET_CASE_VECTOR_PC_RELATIVE) +Target Var(TARGET_CASE_VECTOR_PC_RELATIVE) Init(ARC_NPS400 != 0) Use pc-relative switch case tables - this enables case table shortening. mcompact-casesi -- 2.6.4
[PATCH 01/10] gcc: Add support for mellanox nps400 arc variant
This commit adds support for the mellanox nps400 arc variant. Nothing
much actually changes with this commit other than adding support for
'arc*-mellanox-*' targets at configuration time.
I've added a new makefile fragment for the mellanox variant, right now
there's not much in here, I'll be adding some content in a later commit,
but adding the file now lets me get all of the build
infrastructure (config.gcc) changes done in a single commit.
gcc/ChangeLog:
* config.gcc: Add support for arc*-mellanox-* nps400 targets.
* config/arc/t-nps400: New file.
---
gcc/ChangeLog.NPS400| 5 +
gcc/config.gcc | 10 ++
gcc/config/arc/t-nps400 | 21 +
3 files changed, 36 insertions(+)
create mode 100644 gcc/config/arc/t-nps400
diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400
index 4e68491..286f2dd 100644
--- a/gcc/ChangeLog.NPS400
+++ b/gcc/ChangeLog.NPS400
@@ -1,3 +1,8 @@
+2016-02-01 Andrew Burgess
+
+ * config.gcc: Add support for arc*-mellanox-* nps400 targets.
+ * config/arc/t-nps400: New file.
+
2016-02-02 Andrew Burgess
* common.opt (ffat-lto-objects): Make this flag default on.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 6722260..9a48dca 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1015,6 +1015,11 @@ arc*-*-elf*)
case ${with_endian} in
big*)
tm_defines="DRIVER_ENDIAN_SELF_SPECS=\\\"%{!EL:%{!mlittle-endian:-mbig-endian}}\\\"
${tm_defines}"
esac
+ case ${target} in
+ arc*-mellanox-*)
+ tm_defines="${tm_defines} ARC_NPS400=1"
+ tmake_file="${tmake_file} arc/t-nps400";;
+ esac
;;
arc*-*-linux-uclibc*)
extra_headers="arc-simd.h"
@@ -1040,6 +1045,11 @@ arc*-*-linux-uclibc*)
case ${with_endian} in
big*)
tm_defines="DRIVER_ENDIAN_SELF_SPECS=\\\"%{!EL:%{!mlittle-endian:-mbig-endian}}\\\"
${tm_defines}"
esac
+ case ${target} in
+ arc*-mellanox-*)
+ tm_defines="${tm_defines} ARC_NPS400=1"
+ tmake_file="${tmake_file} arc/t-nps400";;
+ esac
;;
arm-wrs-vxworks)
tm_file="elfos.h arm/elf.h arm/aout.h ${tm_file} vx-common.h vxworks.h
arm/vxworks.h"
diff --git a/gcc/config/arc/t-nps400 b/gcc/config/arc/t-nps400
new file mode 100644
index 000..e332e24
--- /dev/null
+++ b/gcc/config/arc/t-nps400
@@ -0,0 +1,21 @@
+# GCC Makefile fragment for Mellanox NPS400 variant of Synopsys
+# DesignWare ARC
+
+# Copyright (C) 2016 Free Software Foundation, Inc.
+
+# This file is part of GCC.
+
+# GCC 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.
+
+# GCC 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 GCC; see the file COPYING3. If not see
+#
[PATCH 04/10] gcc/arc: Replace rI constraint with r & Cm2 for ld and update insns
In the load*_update instructions the constraint 'rI' was being used, which would accept either a register or a signed 12 bit constant. The problem is that the 32-bit form of ld with update only takes a signed 9-bit immediate. As such, some ld instructions could be generated that would, when assembled be 64-bit long, however, GCC believed them to be 32-bit long. This error in the length would cause problems during branch shortening. The store*_update have the same restrictions on immediate size, however, the patterns for these instructions already only accept 9-bit immediates, and so should be safe. gcc/ChangeLog: * config/arc/arc.md (*loadqi_update): Replace use of 'rI' constraint with separate 'r' and 'Cm2' constraints. (*load_zeroextendqisi_update): Likewise. (*load_signextendqisi_update): Likewise. (*loadhi_update): Likewise. (*load_zeroextendhisi_update): Likewise. (*load_signextendhisi_update): Likewise. (*loadsi_update): Likewise. (*loadsf_update): Likewise. --- gcc/ChangeLog.NPS400 | 12 +++ gcc/config/arc/arc.md | 96 +-- 2 files changed, 60 insertions(+), 48 deletions(-) diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400 index 716e413..71463df 100644 --- a/gcc/ChangeLog.NPS400 +++ b/gcc/ChangeLog.NPS400 @@ -1,3 +1,15 @@ +2016-02-01 Andrew Burgess + + * config/arc/arc.md (*loadqi_update): Replace use of 'rI' + constraint with separate 'r' and 'Cm2' constraints. + (*load_zeroextendqisi_update): Likewise. + (*load_signextendqisi_update): Likewise. + (*loadhi_update): Likewise. + (*load_zeroextendhisi_update): Likewise. + (*load_signextendhisi_update): Likewise. + (*loadsi_update): Likewise. + (*loadsf_update): Likewise. + 2015-09-08 Andrew Burgess Joern Rennecke Noam Camus diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index 4193d26..99e8e30 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -1151,40 +1151,40 @@ ;; Note: loadqi_update has no 16-bit variant (define_insn "*loadqi_update" - [(set (match_operand:QI 3 "dest_reg_operand" "=r,r") + [(set (match_operand:QI 3 "dest_reg_operand" "=r,r,r") (match_operator:QI 4 "any_mem_operand" - [(plus:SI (match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))])) - (set (match_operand:SI 0 "dest_reg_operand" "=r,r") + [(plus:SI (match_operand:SI 1 "register_operand" "0,0,0") + (match_operand:SI 2 "nonmemory_operand" "r,Cm2,Cal"))])) + (set (match_operand:SI 0 "dest_reg_operand" "=r,r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" "ldb.a%V4 %3,[%0,%S2]" - [(set_attr "type" "load,load") - (set_attr "length" "4,8")]) + [(set_attr "type" "load,load,load") + (set_attr "length" "4,4,8")]) (define_insn "*load_zeroextendqisi_update" - [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") + [(set (match_operand:SI 3 "dest_reg_operand" "=r,r,r") (zero_extend:SI (match_operator:QI 4 "any_mem_operand" -[(plus:SI (match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) - (set (match_operand:SI 0 "dest_reg_operand" "=r,r") +[(plus:SI (match_operand:SI 1 "register_operand" "0,0,0") + (match_operand:SI 2 "nonmemory_operand" "r,Cm2,Cal"))]))) + (set (match_operand:SI 0 "dest_reg_operand" "=r,r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" "ldb.a%V4 %3,[%0,%S2]" - [(set_attr "type" "load,load") - (set_attr "length" "4,8")]) + [(set_attr "type" "load,load,load") + (set_attr "length" "4,4,8")]) (define_insn "*load_signextendqisi_update" - [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") + [(set (match_operand:SI 3 "dest_reg_operand" "=r,r,r") (sign_extend:SI (match_operator:QI 4 "any_mem_operand" -[(plus:SI (match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) - (set (match_operand:SI 0 "dest_reg_operand" "=r,r") +[(plus:SI (match_operand:SI 1 "register_operand" "0,0,0") + (match_operand:SI 2 "nonmemory_operand" "r,Cm2,Cal"))]))) + (set (match_operand:SI 0 "dest_reg_operand" "=r,r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" "ldb.x.a%V4 %3,[%0,%S2]" - [(set_attr "type" "load,load") - (set_attr "length" "4,8")]) + [(set_attr "type" "load,load,load") + (set_attr "length" "4,4,8")]) (define_insn "*storeqi_update" [(set (match_operator:QI 4 "any_mem_operand" @@ -1201,41 +1201,41 @@ ;; ??? pattern may have to be re-written ;; Note: no 16-bit variant for this pattern (define_insn "*loadhi_updat
[PATCH 00/10] ARC: Add support for NPS400 variant
The NPS400 is an ARC700 variant from Mellanox (formally EZChip). This patch series adds a new GCC build target with the mellanox vendor string, that configures the ARC backend to support NPS400. This patch series is intended for GCC 7, Stage 1, once it reopens. I am posting early in the hope that I could get an early review, especially on patch #1, the build infrastructure, then if I need to rework anything I can get started on it sooner. I've run regression tests against a standard arc-elf target, and the results look good. All feedback appreciated. Thanks, Andrew --- Andrew Burgess (10): gcc: Add support for mellanox nps400 arc variant gcc/arc: Add -munaligned-access option for nps400 gcc/arc: generate jump tables in code section for nps400 gcc/arc: Replace rI constraint with r & Cm2 for ld and update insns gcc/arc: convert some constraints to define_constraint gcc/arc: Add support for nps400 cmem xld/xst instructions gcc/arc: Add nps400 bitops support gcc/arc: Mask integer 'L' operands to 32-bit gcc/arc: Add an nps400 specific testcase gcc/arc: Add __NPS400__ define for nps400 targets gcc/ChangeLog.NPS400 | 122 +++ gcc/config.gcc| 10 + gcc/config/arc/arc.c | 67 +++- gcc/config/arc/arc.h | 35 +- gcc/config/arc/arc.md | 567 +++--- gcc/config/arc/arc.opt| 14 +- gcc/config/arc/constraints.md | 86 - gcc/config/arc/predicates.md | 19 + gcc/config/arc/t-nps400 | 21 ++ gcc/testsuite/ChangeLog.NPS400| 44 +++ gcc/testsuite/gcc.target/arc/cmem-1.c | 10 + gcc/testsuite/gcc.target/arc/cmem-2.c | 10 + gcc/testsuite/gcc.target/arc/cmem-3.c | 10 + gcc/testsuite/gcc.target/arc/cmem-4.c | 10 + gcc/testsuite/gcc.target/arc/cmem-5.c | 10 + gcc/testsuite/gcc.target/arc/cmem-6.c | 10 + gcc/testsuite/gcc.target/arc/cmem-7.c | 26 ++ gcc/testsuite/gcc.target/arc/cmem-ld.inc | 16 + gcc/testsuite/gcc.target/arc/cmem-st.inc | 18 + gcc/testsuite/gcc.target/arc/extzv-1.c| 11 + gcc/testsuite/gcc.target/arc/insv-1.c | 21 ++ gcc/testsuite/gcc.target/arc/insv-2.c | 18 + gcc/testsuite/gcc.target/arc/movb-1.c | 13 + gcc/testsuite/gcc.target/arc/movb-2.c | 13 + gcc/testsuite/gcc.target/arc/movb-3.c | 13 + gcc/testsuite/gcc.target/arc/movb-4.c | 13 + gcc/testsuite/gcc.target/arc/movb-5.c | 13 + gcc/testsuite/gcc.target/arc/movb_cl-1.c | 9 + gcc/testsuite/gcc.target/arc/movb_cl-2.c | 11 + gcc/testsuite/gcc.target/arc/movbi_cl-1.c | 9 + gcc/testsuite/gcc.target/arc/movh_cl-1.c | 27 ++ gcc/testsuite/gcc.target/arc/movl-1.c | 17 + gcc/testsuite/gcc.target/arc/mrgb-1.c | 14 + gcc/testsuite/gcc.target/arc/nps400-1.c | 23 ++ gcc/testsuite/gcc.target/arc/setmem-1.c | 13 + gcc/testsuite/gcc.target/arc/setmem-2.c | 18 + gcc/testsuite/gcc.target/arc/setmem-3.c | 13 + gcc/testsuite/gcc.target/arc/setmem-4.c | 18 + 38 files changed, 1231 insertions(+), 161 deletions(-) create mode 100644 gcc/config/arc/t-nps400 create mode 100644 gcc/testsuite/ChangeLog.NPS400 create mode 100644 gcc/testsuite/gcc.target/arc/cmem-1.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-2.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-3.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-4.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-5.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-6.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-7.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-ld.inc create mode 100644 gcc/testsuite/gcc.target/arc/cmem-st.inc create mode 100644 gcc/testsuite/gcc.target/arc/extzv-1.c create mode 100644 gcc/testsuite/gcc.target/arc/insv-1.c create mode 100644 gcc/testsuite/gcc.target/arc/insv-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-3.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-4.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-5.c create mode 100644 gcc/testsuite/gcc.target/arc/movb_cl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movb_cl-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movbi_cl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movh_cl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/mrgb-1.c create mode 100644 gcc/testsuite/gcc.target/arc/nps400-1.c create mode 100644 gcc/testsuite/gcc.target/arc/setmem-1.c create mode 100644 gcc/testsuite/gcc.target/arc/setmem-2.c create mode 100644 gcc/testsuite/gcc.target/arc/setmem-3.c create mode 100644 gcc/testsuite/gcc.target/arc/setmem-4.c -- 2.6.4
[PATCH 05/10] gcc/arc: convert some constraints to define_constraint
The define_memory_constraint allows for the address operand to be reloaded into a base register. However, for the constraints 'Us<' and 'Us>', which are used for matching 'push' and 'pop' instructions moving the address into a base register is not helpful. The constraints then should be define_constraint, not define_memory_constraint. Similarly the Usd constraint, used for generating small data area memory accesses, can't have its operand loaded into a register as the relocation for small data area symbols only works within ld/st instructions. gcc/ChangeLog: * config/arc/constraints.md (Usd): Convert to define_constraint. (Us<): Likewise. (Us>): Likewise. --- gcc/ChangeLog.NPS400 | 7 +++ gcc/config/arc/constraints.md | 18 +++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400 index 71463df..5d1533c 100644 --- a/gcc/ChangeLog.NPS400 +++ b/gcc/ChangeLog.NPS400 @@ -1,3 +1,10 @@ +2016-03-01 Joern Rennecke + Andrew Burgess + + * config/arc/constraints.md (Usd): Convert to define_constraint. + (Us<): Likewise. + (Us>): Likewise. + 2016-02-01 Andrew Burgess * config/arc/arc.md (*loadqi_update): Replace use of 'rI' diff --git a/gcc/config/arc/constraints.md b/gcc/config/arc/constraints.md index 668b60a..b6954ad 100644 --- a/gcc/config/arc/constraints.md +++ b/gcc/config/arc/constraints.md @@ -269,11 +269,15 @@ (and (match_code "mem") (match_test "compact_store_memory_operand (op, VOIDmode)"))) -(define_memory_constraint "Usd" - "@internal - A valid _small-data_ memory operand for ARCompact instructions" - (and (match_code "mem") - (match_test "compact_sda_memory_operand (op, VOIDmode)"))) +; Don't use define_memory_constraint here as the relocation patching +; for small data symbols only works within a ld/st instruction and +; define_memory_constraint may result in the address being calculated +; into a register first. +(define_constraint "Usd" + "@internal +A valid _small-data_ memory operand for ARCompact instructions" + (and (match_code "mem") +(match_test "compact_sda_memory_operand (op, VOIDmode)"))) (define_memory_constraint "Usc" "@internal @@ -283,7 +287,7 @@ ;; ??? the assembler rejects stores of immediates to small data. (match_test "!compact_sda_memory_operand (op, VOIDmode)"))) -(define_memory_constraint "Us<" +(define_constraint "Us<" "@internal Stack pre-decrement" (and (match_code "mem") @@ -291,7 +295,7 @@ (match_test "REG_P (XEXP (XEXP (op, 0), 0))") (match_test "REGNO (XEXP (XEXP (op, 0), 0)) == SP_REG"))) -(define_memory_constraint "Us>" +(define_constraint "Us>" "@internal Stack post-increment" (and (match_code "mem") -- 2.6.4
[PATCH 02/10] gcc/arc: Add -munaligned-access option for nps400
New option for nps400 arc (-munaligned-access) that allows GCC to
generate unaligned accesses, the option is off by default. Turning this
option on will update the value for STRICT_ALIGNMENT.
gcc/ChangeLog:
* config/arc/arc.h (ARC_NPS400): Define if not already defined.
(UNALIGNED_ACCESS_DEFAULT): Define, if not already defined.
(STRICT_ALIGNMENT): Make use of unaligned_access var.
* config/arc/arc.c (arc_expand_movmem): Take STRICT_ALIGNMENT into
account.
* config/arc/arc.opt: (munaligned-access): New option.
* config/arc/t-nps400: Add munaligned-access to the multilib list.
gcc/testsuite/ChangeLog:
* gcc.target/arc/setmem-1.c: New file.
* gcc.target/arc/setmem-2.c: New file.
* gcc.target/arc/setmem-3.c: New file.
* gcc.target/arc/setmem-4.c: New file.
---
gcc/ChangeLog.NPS400| 11 +++
gcc/config/arc/arc.c| 4 ++--
gcc/config/arc/arc.h| 19 +++
gcc/config/arc/arc.opt | 4
gcc/config/arc/t-nps400 | 2 +-
gcc/testsuite/ChangeLog.NPS400 | 6 ++
gcc/testsuite/gcc.target/arc/setmem-1.c | 13 +
gcc/testsuite/gcc.target/arc/setmem-2.c | 18 ++
gcc/testsuite/gcc.target/arc/setmem-3.c | 13 +
gcc/testsuite/gcc.target/arc/setmem-4.c | 18 ++
10 files changed, 101 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/ChangeLog.NPS400
create mode 100644 gcc/testsuite/gcc.target/arc/setmem-1.c
create mode 100644 gcc/testsuite/gcc.target/arc/setmem-2.c
create mode 100644 gcc/testsuite/gcc.target/arc/setmem-3.c
create mode 100644 gcc/testsuite/gcc.target/arc/setmem-4.c
diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400
index 286f2dd..0281640 100644
--- a/gcc/ChangeLog.NPS400
+++ b/gcc/ChangeLog.NPS400
@@ -1,3 +1,14 @@
+2016-02-02 Joern Rennecke
+ Andrew Burgess
+
+ * config/arc/arc.h (ARC_NPS400): Define if not already defined.
+ (UNALIGNED_ACCESS_DEFAULT): Define, if not already defined.
+ (STRICT_ALIGNMENT): Make use of unaligned_access var.
+ * config/arc/arc.c (arc_expand_movmem): Take STRICT_ALIGNMENT into
+ account.
+ * config/arc/arc.opt: (munaligned-access): New option.
+ * config/arc/t-nps400: Add munaligned-access to the multilib list.
+
2016-02-01 Andrew Burgess
* config.gcc: Add support for arc*-mellanox-* nps400 targets.
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index d60db50..35bb44a 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -7137,7 +7137,7 @@ arc_expand_movmem (rtx *operands)
HOST_WIDE_INT size;
int align = INTVAL (operands[3]);
unsigned n_pieces;
- int piece = align;
+ int piece = STRICT_ALIGNMENT ? align : 4;
rtx store[2];
rtx tmpx[2];
int i;
@@ -7146,7 +7146,7 @@ arc_expand_movmem (rtx *operands)
return false;
size = INTVAL (operands[2]);
/* move_by_pieces_ninsns is static, so we can't use it. */
- if (align >= 4)
+ if (align >= 4 || !STRICT_ALIGNMENT)
{
if (TARGET_LL64)
n_pieces = (size + 4) / 8U + ((size >> 1) & 1) + (size & 1);
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index 21c049f..1cb59ec 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -62,6 +62,10 @@ along with GCC; see the file COPYING3. If not see
#undef ASM_APP_OFF
#undef CC1_SPEC
+#ifndef ARC_NPS400
+#define ARC_NPS400 0
+#endif
+
/* Names to predefine in the preprocessor for this target machine. */
#define TARGET_CPU_CPP_BUILTINS() \
do { \
@@ -309,6 +313,10 @@ along with GCC; see the file COPYING3. If not see
#define MULTILIB_DEFAULTS { "mARC700" }
#endif
+#ifndef UNALIGNED_ACCESS_DEFAULT
+#define UNALIGNED_ACCESS_DEFAULT 0
+#endif
+
/* Target machine storage layout. */
/* We want zero_extract to mean the same
@@ -416,10 +424,13 @@ if (GET_MODE_CLASS (MODE) == MODE_INT \
/* Set this nonzero if move instructions will actually fail to work
when given unaligned data. */
-/* On the ARC the lower address bits are masked to 0 as necessary. The chip
- won't croak when given an unaligned address, but the insn will still fail
- to produce the correct result. */
-#define STRICT_ALIGNMENT 1
+/* On most ARC cores the lower address bits are masked to 0 as necessary,
+ the chip won't croak when given an unaligned address, but the insn will
+ still fail to produce the correct result. */
+/* The NPS400 ARC variant supports unaligned access. Although not without
+ cost, this is still fast enough that we can justify keeping
+ SLOW_UNALIGNED_ACCESS off. */
+#define STRICT_ALIGNMENT (!unaligned_access)
/* Layout of source language data types. */
diff --git a/gcc/config/arc/arc.opt b/gcc/config/arc/arc.opt
index 2227b75..f8e062c 100644
--- a/gcc/config/arc/arc.opt
+++
[PATCH 09/10] gcc/arc: Add an nps400 specific testcase
This test case triggered a bug caused by VOIDmode not being handled in
proper_comparison_operator, this problem was fixed with a commit on
2016-01-27 by Claudiu Zissulescu, adding this test case for coverage.
gcc/testsuite/ChangeLog:
* gcc.target/arc/nps400-1.c: New file.
---
gcc/testsuite/ChangeLog.NPS400 | 4
gcc/testsuite/gcc.target/arc/nps400-1.c | 23 +++
2 files changed, 27 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/arc/nps400-1.c
diff --git a/gcc/testsuite/ChangeLog.NPS400 b/gcc/testsuite/ChangeLog.NPS400
index d658bd9..20f88d0 100644
--- a/gcc/testsuite/ChangeLog.NPS400
+++ b/gcc/testsuite/ChangeLog.NPS400
@@ -1,3 +1,7 @@
+2016-02-06 Andrew Burgess
+
+ * gcc.target/arc/nps400-1.c: New file.
+
2016-01-19 Andrew Burgess
* gcc.target/arc/movh_cl-1.c: New file.
diff --git a/gcc/testsuite/gcc.target/arc/nps400-1.c
b/gcc/testsuite/gcc.target/arc/nps400-1.c
new file mode 100644
index 000..57d6800
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arc/nps400-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile { target arc*-mellanox-* } } */
+/* { dg-options "-mq-class -mbitops -munaligned-access -mcmem -O2
-fno-strict-aliasing" } */
+
+enum npsdp_mem_space_type {
+ NPSDP_EXTERNAL_MS = 1
+};
+struct npsdp_ext_addr {
+ struct {
+struct {
+ enum npsdp_mem_space_type mem_type : 1;
+ unsigned msid : 5;
+};
+ };
+ char user_space[];
+} a;
+char b;
+void fn1() {
+ ((struct npsdp_ext_addr *)a.user_space)->mem_type = NPSDP_EXTERNAL_MS;
+ ((struct npsdp_ext_addr *)a.user_space)->msid =
+ ((struct npsdp_ext_addr *)a.user_space)->mem_type ? 1 : 10;
+ while (b)
+;
+}
--
2.6.4
[PATCH 08/10] gcc/arc: Mask integer 'L' operands to 32-bit
When formatting 'L' operands (least significant word) only print
32-bits, don't sign extend to 64-bits.
This commit could really be applied directly to the current GCC trunk,
however, the only test I have for this issue right now relies on the
nps400 bitops support.
gcc/ChangeLog:
* config/arc/arc.c (arc_print_operand): Print integer 'L' operands
as 32-bits.
gcc/testsuite/ChangeLog:
* gcc.target/arc/movh_cl-1.c: New file.
---
gcc/ChangeLog.NPS400 | 6 ++
gcc/config/arc/arc.c | 10 --
gcc/testsuite/ChangeLog.NPS400 | 4
gcc/testsuite/gcc.target/arc/movh_cl-1.c | 27 +++
4 files changed, 41 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arc/movh_cl-1.c
diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400
index 8229d67..146370c 100644
--- a/gcc/ChangeLog.NPS400
+++ b/gcc/ChangeLog.NPS400
@@ -1,3 +1,9 @@
+2016-01-19 Joern Rennecke
+ Andrew Burgess
+
+ * config/arc/arc.c (arc_print_operand): Print integer 'L' operands
+ as 32-bits.
+
2013-02-19 Joern Rennecke
Andrew Burgess
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index a75f200..dc885d3 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -3176,18 +3176,16 @@ arc_print_operand (FILE *file, rtx x, int code)
else if (GET_CODE (x) == CONST_INT
|| GET_CODE (x) == CONST_DOUBLE)
{
- rtx first, second;
+ rtx first, second, word;
split_double (x, &first, &second);
if((WORDS_BIG_ENDIAN) == 0)
- fprintf (file, "0x%08" PRIx64,
- code == 'L' ? INTVAL (first) : INTVAL (second));
+ word = (code == 'L' ? first : second);
else
- fprintf (file, "0x%08" PRIx64,
- code == 'L' ? INTVAL (second) : INTVAL (first));
-
+ word = (code == 'L' ? second : first);
+ fprintf (file, "0x%08" PRIx32, ((uint32_t) INTVAL (word)));
}
else
output_operand_lossage ("invalid operand to %%H/%%L code");
diff --git a/gcc/testsuite/ChangeLog.NPS400 b/gcc/testsuite/ChangeLog.NPS400
index 22dec32..d658bd9 100644
--- a/gcc/testsuite/ChangeLog.NPS400
+++ b/gcc/testsuite/ChangeLog.NPS400
@@ -1,3 +1,7 @@
+2016-01-19 Andrew Burgess
+
+ * gcc.target/arc/movh_cl-1.c: New file.
+
2013-02-19 Joern Rennecke
Andrew Burgess
diff --git a/gcc/testsuite/gcc.target/arc/movh_cl-1.c
b/gcc/testsuite/gcc.target/arc/movh_cl-1.c
new file mode 100644
index 000..8fbea7e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arc/movh_cl-1.c
@@ -0,0 +1,27 @@
+/* { dg-do compile { target arc*-mellanox-* } } */
+/* { dg-options "-O2 -mbitops" } */
+
+struct thing
+{
+ union
+ {
+int raw;
+struct
+{
+ unsigned a : 1;
+ unsigned b : 1;
+};
+ };
+};
+
+extern void func (int);
+
+void
+blah ()
+{
+ struct thing xx;
+ xx.a = xx.b = 1;
+ func (xx.raw);
+}
+
+/* { dg-final { scan-assembler "movh\.cl r\[0-9\]+,0xc000>>16" } } */
--
2.6.4
[PATCH 06/10] gcc/arc: Add support for nps400 cmem xld/xst instructions
This commit adds support for NPS400 cmem memory sections. Data to be placed into cmem memory is placed into a section ".cmem", ".cmem_shared", or ".cmem_private". There are restrictions on how instructions can be used to operate on data held in cmem memory, this is reflected by the introduction of new operand constraints (Uex/Ucm), and modifications to some instructions to make use of these constraints. gcc/ChangeLog: * config/arc/arc.h (SYMBOL_FLAG_CMEM): Define. (TARGET_NPS400_CMEM_DEFAULT): Provide default definition. * config/arc/arc.c (arc_address_cost): Return 0 for cmem_address. (arc_encode_section_info): Set SYMBOL_FLAG_CMEM where indicated. * config/arc/arc.opt (mcmem): New option. * config/arc/arc.md (*extendqihi2_i): Add r/Uex alternative, supply length for r/m alternative. (*extendqisi2_ac): Likewise. (*extendhisi2_i): Add r/Uex alternative, supply length for r/m and r/Uex alternative. (movqi_insn): Add r/Ucm and Ucm/?Rac alternatives. (movhi_insn): Likewise. (movsi_insn): Add r/Ucm,Ucm/w alternatives. (*zero_extendqihi2_i): Add r/Ucm alternative. (*zero_extendqisi2_ac): Likewise. (*zero_extendhisi2_i): Likewise. * config/arc/constraints.md (Uex): New memory constraint. (Ucm): New define_constraint. * config/arc/predicates.md (long_immediate_loadstore_operand): Return 0 for MEM with cmem_address address. (cmem_address_0): New predicates. (cmem_address_1): Likewise. (cmem_address_2): Likewise. (cmem_address): Likewise. gcc/testsuite/ChangeLog: * gcc.target/arc/cmem-1.c: New file. * gcc.target/arc/cmem-2.c: New file. * gcc.target/arc/cmem-3.c: New file. * gcc.target/arc/cmem-4.c: New file. * gcc.target/arc/cmem-5.c: New file. * gcc.target/arc/cmem-6.c: New file. * gcc.target/arc/cmem-7.c: New file. * gcc.target/arc/cmem-ld.inc: New file. * gcc.target/arc/cmem-st.inc: New file. --- gcc/ChangeLog.NPS400 | 28 gcc/config/arc/arc.c | 20 ++ gcc/config/arc/arc.h | 5 ++ gcc/config/arc/arc.md| 115 +-- gcc/config/arc/arc.opt | 4 ++ gcc/config/arc/constraints.md| 14 +++- gcc/config/arc/predicates.md | 19 + gcc/testsuite/ChangeLog.NPS400 | 13 gcc/testsuite/gcc.target/arc/cmem-1.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-2.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-3.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-4.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-5.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-6.c| 10 +++ gcc/testsuite/gcc.target/arc/cmem-7.c| 26 +++ gcc/testsuite/gcc.target/arc/cmem-ld.inc | 16 + gcc/testsuite/gcc.target/arc/cmem-st.inc | 18 + 17 files changed, 287 insertions(+), 51 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/cmem-1.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-2.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-3.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-4.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-5.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-6.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-7.c create mode 100644 gcc/testsuite/gcc.target/arc/cmem-ld.inc create mode 100644 gcc/testsuite/gcc.target/arc/cmem-st.inc diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400 index 5d1533c..2a0f820 100644 --- a/gcc/ChangeLog.NPS400 +++ b/gcc/ChangeLog.NPS400 @@ -1,3 +1,31 @@ +2013-08-31 Joern Rennecke + Andrew Burgess + + * config/arc/arc.h (SYMBOL_FLAG_CMEM): Define. + (TARGET_NPS400_CMEM_DEFAULT): Provide default definition. + * config/arc/arc.c (arc_address_cost): Return 0 for cmem_address. + (arc_encode_section_info): Set SYMBOL_FLAG_CMEM where indicated. + * config/arc/arc.opt (mcmem): New option. + * config/arc/arc.md (*extendqihi2_i): Add r/Uex alternative, + supply length for r/m alternative. + (*extendqisi2_ac): Likewise. + (*extendhisi2_i): Add r/Uex alternative, supply length for r/m and + r/Uex alternative. + (movqi_insn): Add r/Ucm and Ucm/?Rac alternatives. + (movhi_insn): Likewise. + (movsi_insn): Add r/Ucm,Ucm/w alternatives. + (*zero_extendqihi2_i): Add r/Ucm alternative. + (*zero_extendqisi2_ac): Likewise. + (*zero_extendhisi2_i): Likewise. + * config/arc/constraints.md (Uex): New memory constraint. + (Ucm): New define_constraint. + * config/arc/predicates.md (long_immediate_loadstore_operand): + Return 0 for MEM with cmem_address address. + (cmem_address_0): New predicates. + (cmem_address_1): Likewise. + (cmem_address
[PATCH 10/10] gcc/arc: Add __NPS400__ define for nps400 targets
Arrange to have the define __NPS400__ defined when compiling code for
Mellanox NPS400 ARC variant.
gcc/ChangeLog:
* conig/arc/arc.h (TARGET_CPU_CPP_BUILTINS): Add __NPS400__.
---
gcc/ChangeLog.NPS400 | 4
gcc/config/arc/arc.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400
index 146370c..e1889b9 100644
--- a/gcc/ChangeLog.NPS400
+++ b/gcc/ChangeLog.NPS400
@@ -1,3 +1,7 @@
+2016-02-08 Andrew Burgess
+
+ * conig/arc/arc.h (TARGET_CPU_CPP_BUILTINS): Add __NPS400__.
+
2016-01-19 Joern Rennecke
Andrew Burgess
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index f278bf5..0330ab4 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -120,6 +120,8 @@ along with GCC; see the file COPYING3. If not see
? "__BIG_ENDIAN__" : "__LITTLE_ENDIAN__"); \
if (TARGET_BIG_ENDIAN) \
builtin_define ("__big_endian__"); \
+if (ARC_NPS400)\
+ builtin_define ("__NPS400__"); \
} while(0)
#if DEFAULT_LIBC == LIBC_UCLIBC
--
2.6.4
Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
On Thu, Feb 25, 2016 at 7:00 PM, Alan Lawrence wrote:
> On 22/02/16 12:03, Jakub Jelinek wrote:
>> (f) A global command-line option, which we check alongside DECL_COMMON and
>> further tests (basically, we want only DECL_COMMON decls that either have
>> ARRAY_TYPE, or some other aggregate type with flexible array member or some
>> other trailing array in the struct), and use the resulting predicate in
>> places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
>> predicate returns true, we assume the DECL_SIZE is
>> "variable"/"unlimited"/whatever.
>> The two spots known to me that would need to use this new predicate would
>> be:
>> tree.c (array_at_struct_end_p):
> [...]
>> tree-dfa.c (get_ref_base_and_extent):
> [...]
>
> Close enough to what I meant by (a)/(b), but never mind that, and I hadn't
> looked at where the change for aggressive loop opts needed to be. However...
> Well you are essentially writing the patch for me at this point (so, thanks!),
> but here's a patch that puts that under a global -funknown-commons flag.
> Testcase as before.
>
> Bootstrapped (with flag overridden to true) on x86_64 and aarch64, check-gcc,
> check-fortran, and tested 416.gamess on AArch64, where this patch enables
> running *without* the -fno-aggressive-loop-opts previously required.
>
> In the gcc testsuite, these fail with the option turned on:
> gcc.dg/pr53265.c (test for warnings, line 137)
> gcc.dg/pr53265.c (test for warnings, line 138)
> gcc.dg/pr64277.c scan-tree-dump cunroll ... (*2)
> gcc.dg/tree-ssa/cunroll-{1,2,3,4,9,10,11}.c scan-tree-dump
> cunroll/cunrolli/ivcanon (various)
> gcc.dg/tree-ssa/loop-38.c scan-tree-dump cunrolli "Loop 1 iterates at most 11
> times"
> ...which all appear harmless.
>
> Thomas Koenig suggests
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69368#c80)
> that this be combined with some flag fiddling and warnings in the Fortran
> front-end; this patch doesn't do that, as I'm not very familiar with the
> frontends, but that can follow in a separate patch. (Thomas?)
>
> OK for trunk?
Comments below.
> Cheers, Alan
>
> gcc/ChangeLog:
>
> DATE Alan Lawrence
> Jakub Jelinek
>
> * common.opt (funknown-commons, flag_unknown_commons): New.
> * tree.c (array_at_struct_end_p): Do not limit to size of decl for
> DECL_COMMONS if flag_unknown_commons is set.
> * tree-dfa.c (get_ref_base_and_extent): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/unknown_commons.f: New.
> ---
> gcc/common.opt | 4
> gcc/testsuite/gfortran.dg/unknown_commons.f | 20
> gcc/tree-dfa.c | 15 ++-
> gcc/tree.c | 6 --
> 4 files changed, 42 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gfortran.dg/unknown_commons.f
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 520fa9c..b5b1282 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2455,6 +2455,10 @@ funit-at-a-time
> Common Report Var(flag_unit_at_a_time) Init(1)
> Compile whole compilation unit at a time.
>
> +funknown-commons
> +Common Var(flag_unknown_commons)
> +Assume common declarations may be overridden with unknown larger sizes
I think to make it work with LTO you need to mark it 'Optimization'.
Also it's about
arrays so maybe
'Assume common declarations may be overridden with ones with a larger
trailing array'
also if we document it here we should eventually document it in invoke.texi.
Not sure if "unknown commons" is a good term, maybe "unconstrained
commons" instead?
Otherwise looks ok to me.
Richard.
> +
> funroll-loops
> Common Report Var(flag_unroll_loops) Optimization
> Perform loop unrolling when iteration count is known.
> diff --git a/gcc/testsuite/gfortran.dg/unknown_commons.f
> b/gcc/testsuite/gfortran.dg/unknown_commons.f
> new file mode 100644
> index 000..97e3ce3
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/unknown_commons.f
> @@ -0,0 +1,20 @@
> +! { dg-do compile }
> +! { dg-options "-O3 -funknown-commons -fdump-tree-dom2-details" }
> +
> +! Test for PR69368: a single-element array in a common block, which will be
> +! overridden with a larger size at link time (contrary to language spec).
> +! Dominator opts considers accesses to differently-computed elements of X as
> +! equivalent, unless -funknown-commons is passed in.
> + SUBROUTINE FOO
> + IMPLICIT DOUBLE PRECISION (X)
> + INTEGER J
> + COMMON /MYCOMMON / X(1)
> + DO 10 J=1,1024
> + X(J+1)=X(J+7)
> + 10 CONTINUE
> + RETURN
> + END
> +! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
> +! We should retain both a read and write of mycommon.x.
> +! { dg-final { scan-tree-dump-times " _\[0-9\]+ =
> mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
> +! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] =
> _\[0-9\]+;" 1 "dom2" } }
> diff --git a/gcc/tree
[PATCH 07/10] gcc/arc: Add nps400 bitops support
Add support for nps400 bit operation instructions. There's a new flag -mbitops that turns this feature on. There are new instructions, some changes to existing instructions, a new register class to support the new instructions, and some new expand and peephole optimisations. gcc/ChangeLog: * config/arc/arc.c (arc_conditional_register_usage): Take TARGET_RRQ_CLASS into account. (arc_print_operand): Support printing 'p' and 's' operands. * config/arc/arc.h (TARGET_NPS400_BITOPS_DEFAULT): Provide default as 0. (TARGET_RRQ_CLASS): Define. (IS_POWEROF2_OR_0_P): Define. * config/arc/arc.md (*movsi_insn): Add w/Clo, w/Chi, and w/Cbi alternatives. (*tst_movb): New define_insn. (*tst): Avoid recognition if it could prevent '*tst_movb' combination; replace c/CnL with c/Chs alternative. (*tst_bitfield_tst): New define_insn. (*tst_bitfield_asr): New define_insn. (*tst_bitfield): New define_insn. (andsi3_i): Add Rrq variant. (extzv): New define_expand. (insv): New define_expand. (*insv_i): New define_insn. (*movb): New define_insn. (*movb_signed): New define_insn. (*movb_high): New define_insn. (*movb_high_signed): New define_insn. (*movb_high_signed + 1): New define_split pattern. (*mrgb): New define_insn. (*mrgb + 1): New define_peephole2 pattern. (*mrgb + 2): New define_peephole2 pattern. * config/arc/arc.opt (mbitops): New option for nps400, uses TARGET_NPS400_BITOPS_DEFAULT. * config/arc/constraints.md (q): Make register class conditional. (Rrq): New register constraint. (Chs): New constraint. (Clo): New constraint. (Chi): New constraint. (Cbf): New constraint. (Cbn): New constraint. (C18): New constraint. (Cbi): New constraint. gcc/testsuite/ChangeLog: * gcc.target/arc/extzv-1.c: New file. * gcc.target/arc/insv-1.c: New file. * gcc.target/arc/insv-2.c: New file. * gcc.target/arc/movb-1.c: New file. * gcc.target/arc/movb-2.c: New file. * gcc.target/arc/movb-3.c: New file. * gcc.target/arc/movb-4.c: New file. * gcc.target/arc/movb-5.c: New file. * gcc.target/arc/movb_cl-1.c: New file. * gcc.target/arc/movb_cl-2.c: New file. * gcc.target/arc/movbi_cl-1.c: New file. * gcc.target/arc/movl-1.c: New file. * gcc.target/arc/mrgb-1.c: New file. --- gcc/ChangeLog.NPS400 | 42 gcc/config/arc/arc.c | 33 ++- gcc/config/arc/arc.h | 9 + gcc/config/arc/arc.md | 382 ++ gcc/config/arc/arc.opt| 4 + gcc/config/arc/constraints.md | 58 - gcc/testsuite/ChangeLog.NPS400| 17 ++ gcc/testsuite/gcc.target/arc/extzv-1.c| 11 + gcc/testsuite/gcc.target/arc/insv-1.c | 21 ++ gcc/testsuite/gcc.target/arc/insv-2.c | 18 ++ gcc/testsuite/gcc.target/arc/movb-1.c | 13 + gcc/testsuite/gcc.target/arc/movb-2.c | 13 + gcc/testsuite/gcc.target/arc/movb-3.c | 13 + gcc/testsuite/gcc.target/arc/movb-4.c | 13 + gcc/testsuite/gcc.target/arc/movb-5.c | 13 + gcc/testsuite/gcc.target/arc/movb_cl-1.c | 9 + gcc/testsuite/gcc.target/arc/movb_cl-2.c | 11 + gcc/testsuite/gcc.target/arc/movbi_cl-1.c | 9 + gcc/testsuite/gcc.target/arc/movl-1.c | 17 ++ gcc/testsuite/gcc.target/arc/mrgb-1.c | 14 ++ 20 files changed, 663 insertions(+), 57 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/extzv-1.c create mode 100644 gcc/testsuite/gcc.target/arc/insv-1.c create mode 100644 gcc/testsuite/gcc.target/arc/insv-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-3.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-4.c create mode 100644 gcc/testsuite/gcc.target/arc/movb-5.c create mode 100644 gcc/testsuite/gcc.target/arc/movb_cl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movb_cl-2.c create mode 100644 gcc/testsuite/gcc.target/arc/movbi_cl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/movl-1.c create mode 100644 gcc/testsuite/gcc.target/arc/mrgb-1.c diff --git a/gcc/ChangeLog.NPS400 b/gcc/ChangeLog.NPS400 index 2a0f820..8229d67 100644 --- a/gcc/ChangeLog.NPS400 +++ b/gcc/ChangeLog.NPS400 @@ -1,3 +1,45 @@ +2013-02-19 Joern Rennecke + Andrew Burgess + + * config/arc/arc.c (arc_conditional_register_usage): Take + TARGET_RRQ_CLASS into account. + (arc_print_operand): Support printing 'p' and 's' operands. + * config/arc/arc.h (TARGET_NPS400_BITOPS_DEFAULT): Provide default + as 0. + (TARGET_RRQ_CLASS): Define. + (IS_POW
Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
On Fri, Mar 04, 2016 at 02:27:46PM +0100, Richard Biener wrote: > > +funknown-commons > > +Common Var(flag_unknown_commons) > > +Assume common declarations may be overridden with unknown larger sizes > > I think to make it work with LTO you need to mark it 'Optimization'. > Also it's about > arrays so maybe > > 'Assume common declarations may be overridden with ones with a larger > trailing array' > > also if we document it here we should eventually document it in invoke.texi. Also, isn't the *.opt description line supposed to end with a full stop? Jakub
Re: [PING] genattrab.c generate switch
On Thu, 2016-03-03 at 17:36 -0500, Patrick Palka wrote:
> On Thu, Mar 3, 2016 at 4:29 PM, Jesper Broge Jørgensen
> wrote:
> >
> > On 18/02/16 13:22, Bernd Schmidt wrote:
> > >
> > > On 01/19/2016 12:47 PM, Jesper Broge Jørgensen wrote:
> > > >
> > > > Here is the reformatted patch:
> > >
> > >
> > > This will probably have to wait until stage1.
> > >
> > > > + const int code = GET_CODE (op2);
> > > > + if (code != IOR)
> > > > +{
> > > > + if (code == EQ_ATTR)
> > >
> > >
> > > All the formatting still looks completely mangled. This was
> > > probably done
> > > by your mailer. Please try attaching the diff as text/plain.
> > >
> > >
> > > Bernd
> > >
> > Hi i send the patch back as an attatchment as requested about two
> > weeks ago
> > (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01256.html) but i
> > have not
> > received any response.
> >
> > If it has to wait for stage 1 are there anything else i can do for
> > the patch
> > untill then?
>
> I still suggest to try making write_test_expr() avoid emitting
> redundant parentheses for chains of || or &&, which would fix the
> original issue all the same. Previously you claimed that such a
> change would not be simpler than your current patch, but I gave it a
> quick try and ended up with a much smaller patch:
>
> gcc/genattrtab.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
Patrick, did you forget to attach the patch? I see the diffstat, but
no patch.
RFA: PR 70044: Catch a second call to aarch64_override_options_after_change
Hi Markus, Hi Richard,
PR 70044 reports a problem with the AArch64 backend. With LTO enabled
the function aarch64_override_options_after_change can be called more
than once for the same function. If only leaf frame pointers were
being omitted originally, then the first call will set
flag_omit_frame_pointer to true. Then the second call will set
flag_omit_leaf_frame_pointer to false, thus forcing the frame pointer
to always be omitted, contrary to the original settings of these
flags.
The attached patch fixes this problem by setting
flag_omit_frame_pointer to true, but using a special value of 2 to do
so. Then when the second call occurs we can detect this case and
ensure that we do not set flag_omit_leaf_frame_pointer to false.
Tested with no regressions on an aarch64-elf toolchain.
OK to apply ?
Cheers
Nick
gcc/ChangeLog
2016-03-04 Nick Clifton
PR target/7044
* config/aarch64/aarch64.c
(aarch64_override_options_after_change_1): When forcing
flag_omit_frame_pointer to be true, use a special value that can
be detected if this function is called again, thus preventing
flag_omit_leaf_frame_pointer from being forced to be false.
Index: gcc/config/aarch64/aarch64.c
===
--- gcc/config/aarch64/aarch64.c (revision 233960)
+++ gcc/config/aarch64/aarch64.c (working copy)
@@ -8110,6 +8110,21 @@
static void
aarch64_override_options_after_change_1 (struct gcc_options *opts)
{
+ /* The logic here is that if we are disabling all frame pointer generation
+ then we do not need to disable leaf frame pointer generation as a
+ separate operation. But if we are *only* disabling leaf frame pointer
+ generation then we set flag_omit_frame_pointer to true, but in
+ aarch64_frame_pointer_required we return false only for leaf functions.
+
+ PR 70044: We have to be careful about being called multiple times for the
+ same function. Once we have decided to set flag_omit_frame_pointer just
+ so that we can omit leaf frame pointers, we must then not interpret a
+ second call as meaning that all frame pointer generation should be
+ omitted. We do this by setting flag_omit_frame_pointer to a special,
+ non-zero value. */
+ if (opts->x_flag_omit_frame_pointer == 2)
+opts->x_flag_omit_frame_pointer = 0;
+
if (opts->x_flag_omit_frame_pointer)
opts->x_flag_omit_leaf_frame_pointer = false;
else if (opts->x_flag_omit_leaf_frame_pointer)
Re: RFA: PR 70044: Catch a second call to aarch64_override_options_after_change
Hi Nick, On 04/03/16 13:44, Nick Clifton wrote: Hi Markus, Hi Richard, PR 70044 reports a problem with the AArch64 backend. With LTO enabled the function aarch64_override_options_after_change can be called more than once for the same function. If only leaf frame pointers were being omitted originally, then the first call will set flag_omit_frame_pointer to true. Then the second call will set flag_omit_leaf_frame_pointer to false, thus forcing the frame pointer to always be omitted, contrary to the original settings of these flags. The attached patch fixes this problem by setting flag_omit_frame_pointer to true, but using a special value of 2 to do so. Then when the second call occurs we can detect this case and ensure that we do not set flag_omit_leaf_frame_pointer to false. Tested with no regressions on an aarch64-elf toolchain. OK to apply ? Thanks for looking at this. Cheers Nick gcc/ChangeLog 2016-03-04 Nick Clifton PR target/7044 * config/aarch64/aarch64.c (aarch64_override_options_after_change_1): When forcing flag_omit_frame_pointer to be true, use a special value that can be detected if this function is called again, thus preventing flag_omit_leaf_frame_pointer from being forced to be false. + /* The logic here is that if we are disabling all frame pointer generation + then we do not need to disable leaf frame pointer generation as a + separate operation. But if we are*only* disabling leaf frame pointer + generation then we set flag_omit_frame_pointer to true, but in + aarch64_frame_pointer_required we return false only for leaf functions. + + PR 70044: We have to be careful about being called multiple times for the + same function. Once we have decided to set flag_omit_frame_pointer just + so that we can omit leaf frame pointers, we must then not interpret a + second call as meaning that all frame pointer generation should be + omitted. We do this by setting flag_omit_frame_pointer to a special, + non-zero value. */ + if (opts->x_flag_omit_frame_pointer == 2) +opts->x_flag_omit_frame_pointer = 0; + if (opts->x_flag_omit_frame_pointer) opts->x_flag_omit_leaf_frame_pointer = false; else if (opts->x_flag_omit_leaf_frame_pointer) This is missing a second hunk from the patch you attached in the PR that I think is necessary for this to work (setting to x_flag_omit_frame_pointer)... I've been testing a very similar patch that just changes that logic to: if (opts->x_flag_omit_frame_pointer == 1) opts->x_flag_omit_leaf_frame_pointer = 0; else if (opts->x_flag_omit_leaf_frame_pointer) opts->x_flag_omit_frame_pointer = 2; Note that this patch would expose a bug in common/config/aarch64/aarch64-common.c where there's a thinko in the handling of OPT_momit_leaf_frame_pointer. That's my bad and I'll propose a patch for it soon. Also, is there a way to create a testcase for the testuite? i.e. is there a simple way to scan the assembly generated after the final LTO processing for the presence of the frame pointer? Thanks, Kyrill
Re: [PING] genattrab.c generate switch
On Fri, 4 Mar 2016, David Malcolm wrote:
On Thu, 2016-03-03 at 17:36 -0500, Patrick Palka wrote:
On Thu, Mar 3, 2016 at 4:29 PM, Jesper Broge Jørgensen
wrote:
On 18/02/16 13:22, Bernd Schmidt wrote:
On 01/19/2016 12:47 PM, Jesper Broge Jørgensen wrote:
Here is the reformatted patch:
This will probably have to wait until stage1.
+ const int code = GET_CODE (op2);
+ if (code != IOR)
+{
+ if (code == EQ_ATTR)
All the formatting still looks completely mangled. This was
probably done
by your mailer. Please try attaching the diff as text/plain.
Bernd
Hi i send the patch back as an attatchment as requested about two
weeks ago
(https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01256.html) but i
have not
received any response.
If it has to wait for stage 1 are there anything else i can do for
the patch
untill then?
I still suggest to try making write_test_expr() avoid emitting
redundant parentheses for chains of || or &&, which would fix the
original issue all the same. Previously you claimed that such a
change would not be simpler than your current patch, but I gave it a
quick try and ended up with a much smaller patch:
gcc/genattrtab.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
Patrick, did you forget to attach the patch? I see the diffstat, but
no patch.
Here it is:
-- >8 --
Subject: [PATCH] Reduce nesting of parentheses in conditionals generated by
genattrtab
gcc/ChangeLog:
* genattrtab.c (write_test_expr): New parameter EMIT_PARENS
which defaults to true. Emit an outer pair of parentheses only if
EMIT_PARENS. When continuing a chain of && or ||, don't emit
parentheses for the right-hand operand.
---
gcc/genattrtab.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index b64d8b9..e2ccf1f 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -3432,16 +3432,16 @@ find_attrs_to_cache (rtx exp, bool create)
#define FLG_OUTSIDE_AND8
static unsigned int
-write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags)
+write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
+bool emit_parens = true)
{
int comparison_operator = 0;
RTX_CODE code;
struct attr_desc *attr;
- /* In order not to worry about operator precedence, surround our part of
- the expression with parentheses. */
+ if (emit_parens)
+fprintf (outf, "(");
- fprintf (outf, "(");
code = GET_CODE (exp);
switch (code)
{
@@ -3575,8 +3575,18 @@ write_test_expr (FILE *outf, rtx exp, unsigned int
attrs_cached, int flags)
|| GET_CODE (XEXP (exp, 1)) == EQ_ATTR
|| (GET_CODE (XEXP (exp, 1)) == NOT
&& GET_CODE (XEXP (XEXP (exp, 1), 0)) == EQ_ATTR)))
- attrs_cached
- = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags);
+ {
+ bool need_parens = true;
+
+ /* No need to emit parentheses around the right-hand operand if we are
+continuing a chain of && or ||. */
+ if (GET_CODE (XEXP (exp, 1)) == code)
+ need_parens = false;
+
+ attrs_cached
+ = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags,
+ need_parens);
+ }
else
write_test_expr (outf, XEXP (exp, 1), attrs_cached,
flags | comparison_operator);
@@ -3794,7 +3804,9 @@ write_test_expr (FILE *outf, rtx exp, unsigned int
attrs_cached, int flags)
GET_RTX_NAME (code));
}
- fprintf (outf, ")");
+ if (emit_parens)
+fprintf (outf, ")");
+
return attrs_cached;
}
--
2.8.0.rc0.11.g9bfbc33
Re: [PATCH] Fix vec_set_hi* patterns (PR target/70059)
On 03 Mar 21:17, Jakub Jelinek wrote: > On Thu, Mar 03, 2016 at 01:08:41PM +0100, Jakub Jelinek wrote: > > Fixed thusly, unfortunately I don't have access to avx512f (and not even to > > avx512dq) hw, so while I will bootstrap/regtest it on Haswell-E, can't test > > the tests if they now work at runtime (they link and the assembly of the foo > > routine has changed and looks good to me). Can somebody test this please > > on real hw or emulator? > > Ok for trunk if it passes? This is definetely copy-and-paste issue. OK for trunk and branches (although in 4_9 only 1 pattern affected). Thanks for catching this! > FYI, my bootstrap/regtest on Haswell-E (but without trying to run any > AVX512-* code, just link it at most) passed on both x86_64-linux and > i686-linux. Checked on skylake-avx512 simulator: $ ./*-ref/src/gcc/contrib/compare_tests *-ref/bld/ *-exp/bld # Comparing directories ## Dir1=31153-pr70059-ref/bld/: 3 sum files ## Dir2=15951-pr70059-exp/bld: 3 sum files # Comparing 3 common sum files ## /bin/sh ./31153-pr70059-ref/src/gcc/contrib/compare_tests /tmp/gxx-sum1.21498 /tmp/gxx-sum2.21498 New tests that PASS: gcc.target/i386/avx512dq-pr70059.c (test for excess errors) gcc.target/i386/avx512dq-pr70059.c execution test gcc.target/i386/avx512f-pr70059.c (test for excess errors) gcc.target/i386/avx512f-pr70059.c execution test # No differences found in 3 common sum files > > > 2016-03-03 Jakub Jelinek > > > > PR target/70059 > > * config/i386/sse.md (vec_set_lo_, > > _vinsert_mask): Formatting > > fixes. > > (vec_set_hi_): Likewise. Swap VEC_CONCAT operands. > > > > * gcc.target/i386/avx512f-pr70059.c: New test. > > * gcc.target/i386/avx512dq-pr70059.c: New test. > > Jakub -- K
[RFC] PR69195, Reload confused by invalid reg equivs
This is a fix for two testcases that show reload replacing pseudos
that don't get hard regs, with their equivalent mem initialization,
but failing to address the mem properly. The short story is that ira
analysis creates reg equivalence info for use by reload, based on
register lifetimes that are invalidated by ira itself deleting dead
insns.
My first attempt to fix this problem was to just run
delete_trivially_dead_insns early in ira. That's enough to cure the
testcase failures. However, ira also deletes unreachable blocks (that
are only recognized as such after ira reg equivalence analysis), and
I figure it is possible that deleting those insns may similarly affect
register lifetimes.
So what this patch does is revalidate the reg equivalences after insns
have been deleted, recreating them as if insns had been deleted before
any analysis occurred. The patch has been bootstrapped and regression
tested on powerpc64le-linux. Do we go with this approach, or just run
simple delete_trivially_dead_insns early? Or something else?
gcc/
PR rtl-optimization/69195
* ira.c (pdx_subregs): New static, extracted from update_equiv_regs.
(set_paradoxical_subreg): Delete pdx_subregs param.
(add_store_equivs): New function, extracted from..
(update_equiv_regs): ..here. Don't create and free pdx_subregs or
reg_equiv.
(validate_equiv_regs): New function.
(ira): Create and free pdx_subregs and reg_equiv. After deleting
insns, call validate_equiv_regs. Call setup_reg_equiv later.
(setup_reg_equiv): Protect against deleted insns.
gcc/testsuite/
* gcc.dg/pr69195.c: New.
* gcc.dg/pr69238.c: New.
diff --git a/gcc/ira.c b/gcc/ira.c
index 0973258..047e164 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -3284,11 +3284,15 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSED,
}
}
+/* Use pdx_subregs to show whether a reg is used in a paradoxical
+ subreg. */
+static bool *pdx_subregs;
+
/* Check whether the SUBREG is a paradoxical subreg and set the result
in PDX_SUBREGS. */
static void
-set_paradoxical_subreg (rtx_insn *insn, bool *pdx_subregs)
+set_paradoxical_subreg (rtx_insn *insn)
{
subrtx_iterator::array_type array;
FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
@@ -3319,6 +3323,100 @@ adjust_cleared_regs (rtx loc, const_rtx old_rtx
ATTRIBUTE_UNUSED, void *data)
return NULL_RTX;
}
+/* For insns that set a MEM to the contents of a REG that is only used
+ in a single basic block, see if the register is always equivalent
+ to that memory location and if moving the store from INSN to the
+ insn that sets REG is safe. If so, put a REG_EQUIV note on the
+ initializing insn.
+
+ Don't add a REG_EQUIV note if the insn already has one. The
+ existing REG_EQUIV is likely more useful than the one we are
+ adding.
+
+ If one of the regs in the address has reg_equiv[REGNO].replace set,
+ then we can't add this REG_EQUIV note. The reg_equiv[REGNO].replace
+ optimization may move the set of this register immediately before
+ insn, which puts it after reg_equiv[REGNO].init_insns, and hence the
+ mention in the REG_EQUIV note would be to an uninitialized pseudo. */
+static void
+add_store_equivs (void)
+{
+ for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
+{
+ if (! INSN_P (insn))
+ continue;
+
+ rtx set = single_set (insn);
+ if (! set)
+ continue;
+
+ rtx dest = SET_DEST (set);
+ rtx src = SET_SRC (set);
+ unsigned int regno;
+ rtx_insn *init_insn;
+
+ if (MEM_P (dest)
+ && REG_P (src)
+ && (regno = REGNO (src)) >= FIRST_PSEUDO_REGISTER
+ && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
+ && DF_REG_DEF_COUNT (regno) == 1
+ && ! pdx_subregs[regno]
+ && reg_equiv[regno].init_insns != 0
+ && (init_insn = reg_equiv[regno].init_insns->insn ()) != 0
+ && ! find_reg_note (init_insn, REG_EQUIV, NULL_RTX)
+ && ! contains_replace_regs (XEXP (dest, 0))
+ && validate_equiv_mem (init_insn, src, dest)
+ && ! memref_used_between_p (dest, init_insn, insn)
+ /* Attaching a REG_EQUIV note will fail if INIT_INSN has
+multiple sets. */
+ && set_unique_reg_note (init_insn, REG_EQUIV, copy_rtx (dest)))
+ {
+ /* This insn makes the equivalence, not the one initializing
+the register. */
+ ira_reg_equiv[regno].init_insns
+ = gen_rtx_INSN_LIST (VOIDmode, insn, NULL_RTX);
+ df_notes_rescan (init_insn);
+ }
+}
+}
+
+/* After deleting dead insns, check that REG_EQUIV notes are still
+ valid. */
+static void
+validate_equiv_regs (void)
+{
+ bool done_something = false;
+ unsigned int max = vec_safe_length (reg_equivs);
+
+ for (unsigned int regno = FIRST_PSEUDO_REGISTER; regno < max; regno++)
+{
+ for (rtx_insn_list *elem = reg_equiv[regn
Re: [PING] genattrab.c generate switch
On 03/04/2016 03:27 PM, Patrick Palka wrote:
I still suggest to try making write_test_expr() avoid emitting
redundant parentheses for chains of || or &&, which would fix the
original issue all the same. Previously you claimed that such a
change would not be simpler than your current patch, but I gave it a
quick try and ended up with a much smaller patch:
This looks like a reasonable stopgap if a release manager thinks this is
important enough to fix for gcc-6. Some comments below for that case.
Longer term I'm not sure - in theory maybe the switch would allow us to
generate better code, but I tried it and code size actually seems to go
up (could be jump tables however). I also noticed that in the version
with the switch we still have cases of
switch (cached_type)
{
default:
switch (cached_type)
{
}
}
so that might be a point where the patch could be improved to see if we
can get better code generation by collapsing these switches. Might also
be worth trying to optimize this pattern in gcc.
static unsigned int
-write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int
flags)
+write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int
flags,
+ bool emit_parens = true)
Indentation looks wrong, but could be mailer damage. You should also
update the function comment.
+ attrs_cached
+= write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags,
+ need_parens);
Same here, watch indentation.
Bernd
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
Hi, On Tue, Mar 01, 2016 at 11:06:43PM +0100, Jakub Jelinek wrote: > On Tue, Mar 01, 2016 at 10:47:46PM +0100, Martin Jambor wrote: > > well, exactly what I wrote in the original email and what you have > > quoted (and me as well) above. But let me quote the dejagnu source > > comment of dg-runtest, which is perhaps more clear: > > > > # FLAGS is a set of options to always pass. > > # DEFAULT_EXTRA_FLAGS is a set of options to pass if the testcase > > # doesn't > > # specify any (with dg-option). > > > > So if I changed DEFAULT_EXTRA_FLAGS rather than FLAGS, I'd have to go > > through all testcases specifying dg-options and add -Wno-hsa there > > too. Moreover, we'd have to add -Wno-hsa to all appropriate future > > testcases if they specify their own dg-options. > > Ah, ok; what about adding > # Disable HSA warnings by default. > lappend ALWAYS_CFLAGS "additional_flags=-Wno-hsa" > in libgomp/testsuite/lib/libgomp.exp (next to e.g. > -fno-diagnostics-show-caret)? > That works nicely (though I have to override it explicitely in the libgomp.hsa.c directory with another -Whsa, but I guess we can live with that). So I will use the above for the libgomp case. I have tried to come up with a similar alternative for gcc.dg/gomp/gomp.exp, g++.dg/gomp/gomp.exp and gfortran/gomp/gomp.exp but so far I have not achieved to make the C++ and Fortran cases work in any other way but pass -Wno-hsa in FLAGS (and thus change the name). For C, adding the following before the main loop works, even though it looks too much like a hack to me: global TEST_ALWAYS_FLAGS set TEST_ALWAYS_FLAGS [concat $TEST_ALWAYS_FLAGS "-Wno-hsa"] However, the C++ and Fortran cases use gfortran-dg-runtest to cycle through a set of torture options and I have not yet discovered the right magic variable to set (for example, adding -Wno-hsa to DG_TORTURE_OPTIONS elements does not work). I'm afraid I have spent way too much time on this already, so unless someone has any ideas, I'd suggest that we use the (already approved) name-changing gomp patch as it is. Or at least for C++ and Fortran. Thanks, Martin
Re: [PATCH] Fix detection of setrlimit in libstdc++ testsuite
On 02/03/16 09:38 -0800, Mike Stump wrote: On Mar 2, 2016, at 2:08 AM, Maxim Kuvyrkov wrote: PING ^ 2. The patch is sitting without comments for 4+ months. So the libstdc++ people are usually pretty active and responsive, I usually let them review these sorts of patches as domain experts. I only kick in if they are unreasonably absent. Reviewing the case, I don’t see any hint that the libstdc++ list was ever sent an email. You will want to include it in any patches and pings. Thanks for forwarding this, Mike. Libstdc++ patches need to be sent to the libstdc++ list, see both https://gcc.gnu.org/lists.html and https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_contributing.html#list.patches I don't comment on patches I'm not aware of :-) I did look at the patch, and it does seem reasonable. Given that the libstdc+ folks never saw it, I’lll defer to them. You can ping patches once a week, if you would like. Paolo and I are both attending the WG21 C++ meeting this week. I'm busy and have not had much time to keep up with email, but I'll review the patch ASAP.
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
On Fri, Mar 04, 2016 at 04:27:11PM +0100, Martin Jambor wrote: > > Ah, ok; what about adding > > # Disable HSA warnings by default. > > lappend ALWAYS_CFLAGS "additional_flags=-Wno-hsa" > > in libgomp/testsuite/lib/libgomp.exp (next to e.g. > > -fno-diagnostics-show-caret)? > > > > That works nicely (though I have to override it explicitely in the > libgomp.hsa.c directory with another -Whsa, but I guess we can live > with that). So I will use the above for the libgomp case. Ok. > I have tried to come up with a similar alternative for > gcc.dg/gomp/gomp.exp, g++.dg/gomp/gomp.exp and gfortran/gomp/gomp.exp > but so far I have not achieved to make the C++ and Fortran cases work > in any other way but pass -Wno-hsa in FLAGS (and thus change the > name). For C, adding the following before the main loop works, even > though it looks too much like a hack to me: > > global TEST_ALWAYS_FLAGS > set TEST_ALWAYS_FLAGS [concat $TEST_ALWAYS_FLAGS "-Wno-hsa"] Doesn't this also cause the -Wno-hsa option on all further tests executed by other *.exp after gomp.exp by the same runtest invocation? > However, the C++ and Fortran cases use gfortran-dg-runtest to cycle > through a set of torture options and I have not yet discovered the > right magic variable to set (for example, adding -Wno-hsa to > DG_TORTURE_OPTIONS elements does not work). > > I'm afraid I have spent way too much time on this already, so unless > someone has any ideas, I'd suggest that we use the (already approved) > name-changing gomp patch as it is. Or at least for C++ and Fortran. Do you have URL for what you refer to? Jakub
Re: [PING] genattrab.c generate switch
On Fri, Mar 04, 2016 at 04:13:41PM +0100, Bernd Schmidt wrote: > On 03/04/2016 03:27 PM, Patrick Palka wrote: > >>>I still suggest to try making write_test_expr() avoid emitting > >>>redundant parentheses for chains of || or &&, which would fix the > >>>original issue all the same. Previously you claimed that such a > >>>change would not be simpler than your current patch, but I gave it a > >>>quick try and ended up with a much smaller patch: > > This looks like a reasonable stopgap if a release manager thinks this is > important enough to fix for gcc-6. Some comments below for that case. Longer I think it is important for gcc-6, because one compiler for weirdo reasons imposes unreasonably small restrictions on the () nesting and some people use it as stage1 compiler. So, with the formatting nits fixed, if it got appropriately tested, I think we want it for stage4. Jakub
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
Hi, On Fri, Mar 04, 2016 at 04:31:29PM +0100, Jakub Jelinek wrote: > On Fri, Mar 04, 2016 at 04:27:11PM +0100, Martin Jambor wrote: > > > Ah, ok; what about adding > > > # Disable HSA warnings by default. > > > lappend ALWAYS_CFLAGS "additional_flags=-Wno-hsa" > > > in libgomp/testsuite/lib/libgomp.exp (next to e.g. > > > -fno-diagnostics-show-caret)? > > > > > > > That works nicely (though I have to override it explicitely in the > > libgomp.hsa.c directory with another -Whsa, but I guess we can live > > with that). So I will use the above for the libgomp case. > > Ok. > > > I have tried to come up with a similar alternative for > > gcc.dg/gomp/gomp.exp, g++.dg/gomp/gomp.exp and gfortran/gomp/gomp.exp > > but so far I have not achieved to make the C++ and Fortran cases work > > in any other way but pass -Wno-hsa in FLAGS (and thus change the > > name). For C, adding the following before the main loop works, even > > though it looks too much like a hack to me: > > > > global TEST_ALWAYS_FLAGS > > set TEST_ALWAYS_FLAGS [concat $TEST_ALWAYS_FLAGS "-Wno-hsa"] > > Doesn't this also cause the -Wno-hsa option on all further tests executed by > other *.exp after gomp.exp by the same runtest invocation? > Not in the limited runs that I experimented with so far, but I certainly kept this possibility in mind too. If so, I would either set it back before invoking dg-finish or dismiss the whole idea. > > However, the C++ and Fortran cases use gfortran-dg-runtest to cycle > > through a set of torture options and I have not yet discovered the > > right magic variable to set (for example, adding -Wno-hsa to > > DG_TORTURE_OPTIONS elements does not work). > > > > I'm afraid I have spent way too much time on this already, so unless > > someone has any ideas, I'd suggest that we use the (already approved) > > name-changing gomp patch as it is. Or at least for C++ and Fortran. > > Do you have URL for what you refer to? > Sure, the patch has been posted here: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00071.html and approved here: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00074.html Martin
Re: [PING] genattrab.c generate switch
On 04/03/16 16:13, Bernd Schmidt wrote:
On 03/04/2016 03:27 PM, Patrick Palka wrote:
I still suggest to try making write_test_expr() avoid emitting
redundant parentheses for chains of || or &&, which would fix the
original issue all the same. Previously you claimed that such a
change would not be simpler than your current patch, but I gave it a
quick try and ended up with a much smaller patch:
This looks like a reasonable stopgap if a release manager thinks this
is important enough to fix for gcc-6. Some comments below for that
case. Longer term I'm not sure - in theory maybe the switch would
allow us to generate better code, but I tried it and code size
actually seems to go up (could be jump tables however). I also noticed
that in the version with the switch we still have cases of
switch (cached_type)
{
default:
switch (cached_type)
{
}
}
so that might be a point where the patch could be improved to see if
we can get better code generation by collapsing these switches. Might
also be worth trying to optimize this pattern in gcc.
Bernd
I can look into that if you deem it worth it, or would you rather just
go with Patriks suppressed parenthesis?
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
On Fri, Mar 04, 2016 at 05:01:34PM +0100, Martin Jambor wrote: > Not in the limited runs that I experimented with so far, but I > certainly kept this possibility in mind too. If so, I would either > set it back before invoking dg-finish or dismiss the whole idea. > > > > However, the C++ and Fortran cases use gfortran-dg-runtest to cycle > > > through a set of torture options and I have not yet discovered the > > > right magic variable to set (for example, adding -Wno-hsa to > > > DG_TORTURE_OPTIONS elements does not work). > > > > > > I'm afraid I have spent way too much time on this already, so unless > > > someone has any ideas, I'd suggest that we use the (already approved) > > > name-changing gomp patch as it is. Or at least for C++ and Fortran. > > > > Do you have URL for what you refer to? > > > > Sure, the patch has been posted here: > > https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00071.html For the g*.dg/gomp/, if you'd only move -Wno-hsa into the last argument next to -fopenmp, how many tests would be affected? If not really many, perhaps those could be changed to use dg-additional-options instead of dg-options. Jakub
C++ PATCH for c++/70067 (ICE with typename typedef)
On this testcase, when strip_typedefs rebuilds a TYPENAME_TYPE to remove
the typedef, in this case it looks up the same typedef and we then abort
because we still have a typedef. In that case, strip the typedef
explicitly.
Tested x86_64-pc-linux-gnu, applying to trunk and 5.
commit 3f1401348184108dd03d3b0d76e51d5166ca95bf
Author: Jason Merrill
Date: Thu Mar 3 22:40:40 2016 -0500
PR c++/70067
* tree.c (strip_typedefs): Handle TYPENAME_TYPE lookup finding the
same type.
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 0b7b144..aaf9a4f 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1437,6 +1437,9 @@ strip_typedefs (tree t, bool *remove_attributes)
result = make_typename_type (strip_typedefs (TYPE_CONTEXT (t),
remove_attributes),
fullname, typename_type, tf_none);
+ /* Handle 'typedef typename A::N N;' */
+ if (typedef_variant_p (result))
+ result = TYPE_MAIN_VARIANT (DECL_ORIGINAL_TYPE (TYPE_NAME (result)));
}
break;
case DECLTYPE_TYPE:
diff --git a/gcc/testsuite/g++.dg/template/typename21.C b/gcc/testsuite/g++.dg/template/typename21.C
new file mode 100644
index 000..e5e59b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/typename21.C
@@ -0,0 +1,11 @@
+// PR c++/70067
+// { dg-do compile { target c++98 } }
+
+template struct A;
+template struct B { struct N { }; };
+template struct D: B {
+ typedef typename D::N N;
+ A *a;
+};
+
+D d;
Re: [PING] genattrab.c generate switch
On Fri, 4 Mar 2016, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 04:13:41PM +0100, Bernd Schmidt wrote:
> > On 03/04/2016 03:27 PM, Patrick Palka wrote:
> > >>>I still suggest to try making write_test_expr() avoid emitting
> > >>>redundant parentheses for chains of || or &&, which would fix the
> > >>>original issue all the same. Previously you claimed that such a
> > >>>change would not be simpler than your current patch, but I gave it a
> > >>>quick try and ended up with a much smaller patch:
> >
> > This looks like a reasonable stopgap if a release manager thinks this is
> > important enough to fix for gcc-6. Some comments below for that case. Longer
>
> I think it is important for gcc-6, because one compiler for
> weirdo reasons imposes unreasonably small restrictions on the () nesting
> and some people use it as stage1 compiler.
>
> So, with the formatting nits fixed, if it got appropriately tested, I think
> we want it for stage4.
I updated the function comment and made sure that the indentation is
correct. Earlier I successfully bootstrapped + tested this patch on
x86_64-pc-linux-gnu, but I will do so again to make sure. This patch
reduces the maximum parentheses nesting level in insn-attrtab.c on
x86_64 from about 31 to about 6.
OK to commit after testing?
-- >8 --
Subject: [PATCH] Reduce nesting of parentheses in conditionals generated by
genattrtab
gcc/ChangeLog:
* genattrtab.c (write_test_expr): New parameter EMIT_PARENS
which defaults to true. Emit an outer pair of parentheses only if
EMIT_PARENS. When continuing a chain of && or ||, don't emit
parentheses for the right-hand operand.
---
gcc/genattrtab.c | 31 +++
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index b64d8b9..5974f3e 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -3416,7 +3416,10 @@ find_attrs_to_cache (rtx exp, bool create)
/* Given a piece of RTX, print a C expression to test its truth value to OUTF.
We use AND and IOR both for logical and bit-wise operations, so
- interpret them as logical unless they are inside a comparison expression.
*/
+ interpret them as logical unless they are inside a comparison expression.
+
+ An outermost pair of parentheses is emitted around this C expression unless
+ EMIT_PARENS is false. */
/* Interpret AND/IOR as bit-wise operations instead of logical. */
#define FLG_BITWISE1
@@ -3432,16 +3435,16 @@ find_attrs_to_cache (rtx exp, bool create)
#define FLG_OUTSIDE_AND8
static unsigned int
-write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags)
+write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
+bool emit_parens = true)
{
int comparison_operator = 0;
RTX_CODE code;
struct attr_desc *attr;
- /* In order not to worry about operator precedence, surround our part of
- the expression with parentheses. */
+ if (emit_parens)
+fprintf (outf, "(");
- fprintf (outf, "(");
code = GET_CODE (exp);
switch (code)
{
@@ -3575,8 +3578,18 @@ write_test_expr (FILE *outf, rtx exp, unsigned int
attrs_cached, int flags)
|| GET_CODE (XEXP (exp, 1)) == EQ_ATTR
|| (GET_CODE (XEXP (exp, 1)) == NOT
&& GET_CODE (XEXP (XEXP (exp, 1), 0)) == EQ_ATTR)))
- attrs_cached
- = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags);
+ {
+ bool need_parens = true;
+
+ /* No need to emit parentheses around the right-hand operand if we are
+continuing a chain of && or ||. */
+ if (GET_CODE (XEXP (exp, 1)) == code)
+ need_parens = false;
+
+ attrs_cached
+ = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags,
+ need_parens);
+ }
else
write_test_expr (outf, XEXP (exp, 1), attrs_cached,
flags | comparison_operator);
@@ -3794,7 +3807,9 @@ write_test_expr (FILE *outf, rtx exp, unsigned int
attrs_cached, int flags)
GET_RTX_NAME (code));
}
- fprintf (outf, ")");
+ if (emit_parens)
+fprintf (outf, ")");
+
return attrs_cached;
}
--
2.8.0.rc0.11.g9bfbc33
Re: [PATCH PR69052]Set higher precedence for CONST_WIDE_INT than CONST_INT when canonicalizing addr expr
On Fri, Mar 4, 2016 at 11:57 AM, Richard Biener wrote: > On Fri, Mar 4, 2016 at 12:07 PM, Bin Cheng wrote: >> Hi, >> A address canonicalization interface was introduced by my original patch to >> PR69052. The interface sorts sub-parts in an address expression based on >> precedences defined by function commutative_operand_precedence. It also >> assumes that all CONST_INT sub-parts are at the end of vector after sorting. >> But this is not always true because commutative_operand_precedence sets the >> same precedence to both CONST_INT and CONST_WIDE_INT. The patch could be >> broken in cases which have CONST_WIDE_INT sorted after CONST_INT. Even >> though I don't think we can really run into such complicated case, I worked >> out a patch forcing CONST_INT to lower precedence than CONST_WIDE_INT, so >> that for sure it will be sorted after all other kinds sub-parts. >> >> This is an obvious change. Bootstrap&test on x86_64, bootstrap&test on >> AArch64. Is it OK for this stage? > > I believe the obvious change would be to change > commutative_operand_precedence to pre-CONST_WIDE_INT behavior, namely > giving CONST_WIDE_INT the same precedence as CONST_DOUBLE. Yes, but I am not sure what else this change will affect, so I made the smallest change in the original code. I am testing this now. It would be great if anyone describes it a bit. Thanks, bin > > Richard. > > > >> Thanks, >> bin >> >> 2016-03-04 Bin Cheng >> >> PR rtl-optimization/69052 >> * loop-invariant.c (compare_address_parts): Force CONST_INT to lower >> precedence than CONST_WIDE_INT.
[Patch, fortran] Initializing components of derived type variables
Greetings, Here I propose a patch to gfortran which allows initializing components of derived type variables with a new compile flag. Currently the options -finit-integer=, -finit-real=, -finit-logical=, and -finit-character= are avaialable to initialize variables of each respective type; however, there is no way (other than explicitly in source) to initialize derived type variables. In this patch I add the flag -finit-derived. This flag allows components of derived type variables to be initialized as with local variables. This flag obeys the values given to the other -finit-*= flags; with -finit-local-zero derived type variables are initialized to zero. The brunt of the work happens in expr.c (gfc_default_initializer). I add a boolean parameter to gfc_default_initializer which indicates whether or not to generate initializers for components that do not already have them. If true, an expression is generated for each component which does not already have an explicit initializer. This parameter is passed potentially as true in two places in resolve.c (apply_default_init and resolve_fl_variable_derived). Generation is guarded by can_create_initializer, which ensures similar conditions as in resolve.c (build_default_init_expr). To do the generation, gfc_default_initializer calls upon a new function component_init in expr.c. In order to use the -finit-* flags this calls upon the behavior formerly implemented in resolve.c (build_default_init_expr); I moved this behavior to the public function gfc_build_default_init_expr in expr.c; build_default_init_expr in resolve.c now simply wraps this function to protect it from being called on invalid symbols. I wrestled with whether to change the interface for gfc_default_initializer or create an entirely new function (like gfc_generate_derived_initializer). I decided to change the old function because their behaviors would be almost identical, and there are only a few calls to the former. The patch is based on trunk. It builds and passes all regression tests on x86-64-gnu-linux. --- Fritz Reese From c116c13e03b61deefd41ad548b019af467e91506 Mon Sep 17 00:00:00 2001 From: Fritz O. Reese Date: Fri, 17 Oct 2014 15:46:05 -0400 Subject: [PATCH] 2016-03-02 Fritz Reese gcc/fortran/ * lang.opt, invoke.texi, options.c: New option -finit-derived. * gfortran.h (gfc_option): New option -finit-derived. (gfc_default_initializer): Update prototype. (gfc_build_default_init_expr, gfc_apply_init): New prototypes. * expr.c (gfc_build_default_init_expr, gfc_apply_init, component_initializer): New functions. (gfc_default_initializer): Allow generation of initializers. * decl.c (build_struct): Use new function gfc_apply_init. (variable_decl): Use new interface for gfc_default_initializer. (gfc_match_derived_decl): Likewise. * trans-openmp.c (gfc_trans_omp_array_reduction_or_udr): Likewise. * trans-decl.c (gfc_generate_function_code): Likewise. * class.c (gfc_find_derived_vtab, find_intrinsic_vtab): Likewise. * resolve.c (resolve_allocate_expr): Likewise. (build_default_init_expr): Use new function gfc_build_default_init_expr. (can_generate_init): New function. (apply_default_init, resolve_fl_variable_derived): Use gfc_default_initializer to generate initializers. gcc/testsuite/gfortran.dg/ * assert.inc: Runtime assertions. * init_flag_13.f90, init_flag_14.f90: New testcases for -finit-derived. --- gcc/fortran/class.c|7 +- gcc/fortran/decl.c | 50 +- gcc/fortran/expr.c | 267 ++-- gcc/fortran/gfortran.h |6 +- gcc/fortran/invoke.texi|9 +- gcc/fortran/lang.opt |4 + gcc/fortran/options.c |5 + gcc/fortran/resolve.c | 194 + gcc/fortran/trans-decl.c |3 +- gcc/fortran/trans-openmp.c |2 +- gcc/testsuite/gfortran.dg/assert.inc | 62 +++ gcc/testsuite/gfortran.dg/init_flag_13.f90 | 36 gcc/testsuite/gfortran.dg/init_flag_14.f90 | 36 13 files changed, 460 insertions(+), 221 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/assert.inc create mode 100644 gcc/testsuite/gfortran.dg/init_flag_13.f90 create mode 100644 gcc/testsuite/gfortran.dg/init_flag_14.f90 diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c index 6a7339f..cfcf4f6 100644 --- a/gcc/fortran/class.c +++ b/gcc/fortran/class.c @@ -2325,7 +2325,8 @@ gfc_find_derived_vtab (gfc_symbol *derived) gfc_set_sym_referenced (def_init); def_init->ts.type = BT_DERIVED; def_init->ts.u.derived = derived; - def_init->value = gfc_default_initializer (&def_init->ts); + def_init->value = gfc_default_initializer (&def_init->ts, + false); c->initializer = gfc_lval_expr_from_sym (def_init); } @@ -2412,7 +2413
Re: [PING] genattrab.c generate switch
On Fri, Mar 04, 2016 at 11:34:06AM -0500, Patrick Palka wrote: > * genattrtab.c (write_test_expr): New parameter EMIT_PARENS > which defaults to true. Emit an outer pair of parentheses only if > EMIT_PARENS. When continuing a chain of && or ||, don't emit > parentheses for the right-hand operand. Can you additionally to bootstrap/regtest on x86_64-linux compare insn-attrtab.s (compiled with -g0 and optimizations on) if it is bitwise identical without/with the patch (that should be the case, right), and also try say cross-compiler to armv7hl-linux-gnueabi (where I believe the nesting depth is significantly larger) and compare insn-attrtab.s there as well (no need to build arm binutils, just configure cross-compiler and let the build die after it builds cc1/cc1plus or even far before; all we care is that insn-attrtab.s is bitwise the same)? Jakub
Re: [PING] genattrab.c generate switch
On Fri, Mar 4, 2016 at 11:42 AM, Jakub Jelinek wrote: > On Fri, Mar 04, 2016 at 11:34:06AM -0500, Patrick Palka wrote: >> * genattrtab.c (write_test_expr): New parameter EMIT_PARENS >> which defaults to true. Emit an outer pair of parentheses only if >> EMIT_PARENS. When continuing a chain of && or ||, don't emit >> parentheses for the right-hand operand. > > Can you additionally to bootstrap/regtest on x86_64-linux compare > insn-attrtab.s (compiled with -g0 and optimizations on) if it is bitwise > identical without/with the patch (that should be the case, right), > and also try say cross-compiler to armv7hl-linux-gnueabi (where I believe > the nesting depth is significantly larger) and compare insn-attrtab.s > there as well (no need to build arm binutils, just configure cross-compiler > and let the build die after it builds cc1/cc1plus or even far before; all > we care is that insn-attrtab.s is bitwise the same)? I just quickly tested building the generated insn-attrtab.c with and without the patch using my host gcc 5.3 compiler and the .s output is not the same. So the patch may be wrong, or the removal of parentheses is functionally harmless but subtly affects code generation -- which is plausible && and || naturally associate left-to-right but the explicit parentheses emitted by write_test_expr effectively made chains of && or || to associate right-to-left. This change in associativity could be affecting the behavior of optimization passes, in theory. Or the change could just be wrong, although the earlier successful bootstrap + regtest suggests not.
Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute
On 26/02/16 14:52, James Greenhalgh wrote:
gcc/ChangeLog:
* gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment):
Rewrite, looking one level down for records and arrays.
---
gcc/config/aarch64/aarch64.c | 31 ---
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9142ac0..b084f83 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t
pcum_v, machine_mode mode,
static unsigned int
aarch64_function_arg_alignment (machine_mode mode, const_tree type)
{
- unsigned int alignment;
+ if (!type)
+return GET_MODE_ALIGNMENT (mode);
+ if (integer_zerop (TYPE_SIZE (type)))
+return 0;
- if (type)
-{
- if (!integer_zerop (TYPE_SIZE (type)))
- {
- if (TYPE_MODE (type) == mode)
- alignment = TYPE_ALIGN (type);
- else
- alignment = GET_MODE_ALIGNMENT (mode);
- }
- else
- alignment = 0;
-}
- else
-alignment = GET_MODE_ALIGNMENT (mode);
+ gcc_assert (TYPE_MODE (type) == mode);
+
+ if (!AGGREGATE_TYPE_P (type))
+return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
+
+ if (TREE_CODE (type) == ARRAY_TYPE)
+return TYPE_ALIGN (TREE_TYPE (type));
+
+ unsigned int alignment = 0;
+
+ for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+alignment = std::max (alignment, DECL_ALIGN (field));
return alignment;
}
Ping.
If this is not appropriate for GCC6, then is it OK for Stage 1 of GCC 7?
I think this needs to be a GCC 7 fix at this point.
Additionally, I'd like to fully understand PR69841 before we take this
patch.
In particular, under what circumstances can the C++ front end set DECL_ALIGN
of a type to be wider than we expect. Can we ever end up with 128-bit
alignment of a template parameter when we were expecting 32- or 64-bit
alignment, and if we can what are the implications on this patch?
OK, so IIUC, we *should* be able to rely on DECL_ALIGN for the AAPCS64, as
PR/69841 occurred on gcc-5-branch because a C++ frontend change had not been
backported.
I'm not proposing to backport these AArch64 changes, hence:
Ping^2.
(For gcc 7 ?)
Also tests https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01073.html .
Thanks,
Alan
Re: [PING] genattrab.c generate switch
On 03/04/2016 06:14 PM, Patrick Palka wrote:
I just quickly tested building the generated insn-attrtab.c with and
without the patch using my host gcc 5.3 compiler and the .s output is
not the same.
Hmm, looking at the 003t.original dump it looks like there are
differences in SAVE_EXPRs. Indeed we seem to generate different code for
int at;
int foo ()
{
if (at == 2 || at == 4 || at == 7)
return 1;
return 0;
}
int bar ()
{
if (at == 2 || (at == 4 || at == 7))
return 1;
return 0;
}
That's probably something we want to fix.
Bernd
Re: [PING] genattrab.c generate switch
On 03/04/2016 05:03 PM, Jesper Broge Jørgensen wrote: I can look into that if you deem it worth it, or would you rather just go with Patriks suppressed parenthesis? For the moment (in stage4) we'll at most go with Patrick's patch. Whether we do anything beyond that depends on whether we can demonstrate a benefit - it might be worth investigating. Maybe more interesting than fixing genattrtab would be an optimization to identify switch patterns like the one I posted. Bernd
[C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
This is not a regression but I thought I'd post this anyway. Martin reported
that we generate -Wunused-value warnings on the attached testcase, which
arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
for __atomic_load/store/compare.)
Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
2016-03-04 Marek Polacek
PR c/69407
* c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
operations.
* gcc.dg/atomic-op-6.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 965cf49..25afa9c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree
function,
&& orig_code != BUILT_IN_ATOMIC_STORE_N)
result = sync_resolve_return (first_param, result, orig_format);
+ if (fetch_op)
+ /* Prevent -Wunused-value warning. */
+ TREE_USED (result) = true;
+
/* If new_return is set, assign function to that expr and cast the
result to void since the generic interface returned void. */
if (new_return)
diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
index e69de29..c8d93a4 100644
--- gcc/testsuite/gcc.dg/atomic-op-6.c
+++ gcc/testsuite/gcc.dg/atomic-op-6.c
@@ -0,0 +1,11 @@
+/* Test we don't generate bogus warnings. */
+/* PR c/69407 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+void
+foo (int *p, int a)
+{
+ __atomic_fetch_add (&p, a, 0);
+ __atomic_add_fetch (&p, a, 0);
+}
Marek
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
On Fri, Mar 04, 2016 at 05:04:31PM +0100, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 05:01:34PM +0100, Martin Jambor wrote:
> > Not in the limited runs that I experimented with so far, but I
> > certainly kept this possibility in mind too. If so, I would either
> > set it back before invoking dg-finish or dismiss the whole idea.
> >
> > > > However, the C++ and Fortran cases use gfortran-dg-runtest to cycle
> > > > through a set of torture options and I have not yet discovered the
> > > > right magic variable to set (for example, adding -Wno-hsa to
> > > > DG_TORTURE_OPTIONS elements does not work).
> > > >
> > > > I'm afraid I have spent way too much time on this already, so unless
> > > > someone has any ideas, I'd suggest that we use the (already approved)
> > > > name-changing gomp patch as it is. Or at least for C++ and Fortran.
> > >
> > > Do you have URL for what you refer to?
> > >
> >
> > Sure, the patch has been posted here:
> >
> > https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00071.html
>
> For the g*.dg/gomp/, if you'd only move -Wno-hsa into the last argument
> next to -fopenmp, how many tests would be affected?
Out of 287 files that have dg-options with them in the gomp
directories, only 9 generate hsa warnings:
c-c++-common/gomp/clauses-1.c:/* { dg-options "-fopenmp" } */
c-c++-common/gomp/if-1.c:/* { dg-options "-fopenmp" } */
c-c++-common/gomp/pr61486-2.c:/* { dg-options "-fopenmp" } */
c-c++-common/gomp/target-teams-1.c:/* { dg-options "-fopenmp
-fdump-tree-gimple" } */
g++.dg/gomp/target-teams-1.C:// { dg-options "-fopenmp -fdump-tree-gimple" }
gcc.dg/gomp/pr68128-2.c:/* { dg-options "-O2 -fopenmp -fdump-tree-omplower" }
*/
gfortran.dg/gomp/target1.f90:! { dg-options "-fopenmp" }
gfortran.dg/gomp/target2.f90:! { dg-options "-fopenmp -ffree-line-length-160"
}
gfortran.dg/gomp/target3.f90:! { dg-options "-fopenmp" }
> If not really many,
> perhaps those could be changed to use dg-additional-options instead of
> dg-options.
I do not know what -ffree-line-length-160 is, but probably all of
them, even though putting -O2 in gcc.dg/gomp/pr68128-2.c to
"additional" flags feels just wrong.
However, the real question is: Would such a solution really be much
better than the first version of the patch
(https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01813.html)? After
all, in comparison it would only avoid touching two tests and it will
not avoid issues with tests added in future if they use dg-options.
Martin
Re: [RFC] PR69195, Reload confused by invalid reg equivs
On 03/04/2016 03:54 PM, Alan Modra wrote: This is a fix for two testcases that show reload replacing pseudos that don't get hard regs, with their equivalent mem initialization, but failing to address the mem properly. The short story is that ira analysis creates reg equivalence info for use by reload, based on register lifetimes that are invalidated by ira itself deleting dead insns. My first attempt to fix this problem was to just run delete_trivially_dead_insns early in ira. That's enough to cure the testcase failures. However, ira also deletes unreachable blocks (that are only recognized as such after ira reg equivalence analysis), and I figure it is possible that deleting those insns may similarly affect register lifetimes. So what this patch does is revalidate the reg equivalences after insns have been deleted, recreating them as if insns had been deleted before any analysis occurred. The patch has been bootstrapped and regression tested on powerpc64le-linux. Do we go with this approach, or just run simple delete_trivially_dead_insns early? Or something else? I've managed to reproduce this, and I think your analysis of the problem is correct. So the patch is probably ok from the point of you of "will it work". I can probably be convinced to approve it as-is, but I wonder if you'd be prepared to try something else first. This whole area in IRA is turning into spaghetti a little bit. I would prefer that we schedule a new small optimization pass beforehand (either as a real pass or just a function call) that does only the label-substituting trick from update_equiv_regs, then removes dead code as necessary. When we then get to IRA and setting up equiv regs, we should no longer get to a point where we need to reverify equivalences or update regstat. Bernd
Re: [hsa, testsuite] Suppress hsa warnings in libgomp tests
On Fri, Mar 04, 2016 at 06:12:09PM +0100, Martin Jambor wrote:
> > For the g*.dg/gomp/, if you'd only move -Wno-hsa into the last argument
> > next to -fopenmp, how many tests would be affected?
>
> Out of 287 files that have dg-options with them in the gomp
> directories, only 9 generate hsa warnings:
>
> c-c++-common/gomp/clauses-1.c:/* { dg-options "-fopenmp" } */
> c-c++-common/gomp/if-1.c:/* { dg-options "-fopenmp" } */
> c-c++-common/gomp/pr61486-2.c:/* { dg-options "-fopenmp" } */
> c-c++-common/gomp/target-teams-1.c:/* { dg-options "-fopenmp
> -fdump-tree-gimple" } */
> g++.dg/gomp/target-teams-1.C:// { dg-options "-fopenmp -fdump-tree-gimple" }
> gcc.dg/gomp/pr68128-2.c:/* { dg-options "-O2 -fopenmp -fdump-tree-omplower"
> } */
> gfortran.dg/gomp/target1.f90:! { dg-options "-fopenmp" }
> gfortran.dg/gomp/target2.f90:! { dg-options "-fopenmp
> -ffree-line-length-160" }
> gfortran.dg/gomp/target3.f90:! { dg-options "-fopenmp" }
>
> > If not really many,
> > perhaps those could be changed to use dg-additional-options instead of
> > dg-options.
>
> I do not know what -ffree-line-length-160 is, but probably all of
> them, even though putting -O2 in gcc.dg/gomp/pr68128-2.c to
> "additional" flags feels just wrong.
{ dg-options "-fopenmp" }
can go, that should be the default.
And, I'm fine with moving the -fdump-tree-*, -O2 and -ffree-line-length-160
to dg-additional-options.
I think I prefer it that way.
Jakub
Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
On Fri, Mar 04, 2016 at 06:30:40PM +0100, Marek Polacek wrote:
> This is not a regression but I thought I'd post this anyway. Martin reported
> that we generate -Wunused-value warnings on the attached testcase, which
> arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since
> we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g.
> for __atomic_load/store/compare.)
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7?
I'm ok with it for gcc6.
> diff --git gcc/testsuite/gcc.dg/atomic-op-6.c
> gcc/testsuite/gcc.dg/atomic-op-6.c
> index e69de29..c8d93a4 100644
> --- gcc/testsuite/gcc.dg/atomic-op-6.c
> +++ gcc/testsuite/gcc.dg/atomic-op-6.c
> @@ -0,0 +1,11 @@
> +/* Test we don't generate bogus warnings. */
> +/* PR c/69407 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -Wextra" } */
> +
> +void
> +foo (int *p, int a)
> +{
> + __atomic_fetch_add (&p, a, 0);
> + __atomic_add_fetch (&p, a, 0);
> +}
But IMHO you should add dg-bogus directives here.
Jakub
Re: [PING] genattrab.c generate switch
On 03/04/2016 06:27 PM, Bernd Schmidt wrote:
On 03/04/2016 06:14 PM, Patrick Palka wrote:
I just quickly tested building the generated insn-attrtab.c with and
without the patch using my host gcc 5.3 compiler and the .s output is
not the same.
Hmm, looking at the 003t.original dump it looks like there are
differences in SAVE_EXPRs. Indeed we seem to generate different code for
int at;
int foo ()
{
if (at == 2 || at == 4 || at == 7)
return 1;
return 0;
}
int bar ()
{
if (at == 2 || (at == 4 || at == 7))
return 1;
return 0;
}
Ahh... it's not just different placement of SAVE_EXPRs, it's actually a
case of TRUTH_ORIF_EXPR vs. TRUTH_OR_EXPR (the distinction is invisible
in the dumps), the latter being created by fold_range_test. That's a bit
of a broken optimization what with its inability to see more than two
comparisons at a time... we convert one ORIF per function, but a
different one.
Bernd
Re: [PING] genattrab.c generate switch
On Fri, Mar 04, 2016 at 06:49:58PM +0100, Bernd Schmidt wrote:
> On 03/04/2016 06:27 PM, Bernd Schmidt wrote:
> >On 03/04/2016 06:14 PM, Patrick Palka wrote:
> >
> >>I just quickly tested building the generated insn-attrtab.c with and
> >>without the patch using my host gcc 5.3 compiler and the .s output is
> >>not the same.
> >
> >Hmm, looking at the 003t.original dump it looks like there are
> >differences in SAVE_EXPRs. Indeed we seem to generate different code for
> >
> >int at;
> >
> >int foo ()
> >{
> > if (at == 2 || at == 4 || at == 7)
> > return 1;
> > return 0;
> >}
> >
> >int bar ()
> >{
> > if (at == 2 || (at == 4 || at == 7))
> > return 1;
> > return 0;
> >}
>
> Ahh... it's not just different placement of SAVE_EXPRs, it's actually a case
> of TRUTH_ORIF_EXPR vs. TRUTH_OR_EXPR (the distinction is invisible in the
> dumps), the latter being created by fold_range_test. That's a bit of a
> broken optimization what with its inability to see more than two comparisons
> at a time... we convert one ORIF per function, but a different one.
I think we don't need to guarantee identical assembly, the reason I've
suggested that was if it passed, it would be much easier to verify.
Without that, I think it should be bootstrapped at least on one other
target. Note the cases you remove the parens aren't just || and &&, but
most likely also | and & (at least there is some flag whether to print those
as && or &). And there is code for the caching of the attributes where the
result is still usable, I believe the patch doesn't break that, but it
wouldn't hurt to verify that.
Jakub
Re: [PING] genattrab.c generate switch
On 03/04/2016 06:56 PM, Jakub Jelinek wrote: I think we don't need to guarantee identical assembly, the reason I've suggested that was if it passed, it would be much easier to verify. Without that, I think it should be bootstrapped at least on one other target. Note the cases you remove the parens aren't just || and &&, but most likely also | and & (at least there is some flag whether to print those as && or &). And there is code for the caching of the attributes where the result is still usable, I believe the patch doesn't break that, but it wouldn't hurt to verify that. Let's just defer it IMO. What do we care if other compilers are terminally broken? Let's use it as marketing material :) Bernd
Re: [PING] genattrab.c generate switch
On Fri, Mar 04, 2016 at 07:01:10PM +0100, Bernd Schmidt wrote: > On 03/04/2016 06:56 PM, Jakub Jelinek wrote: > >I think we don't need to guarantee identical assembly, the reason I've > >suggested that was if it passed, it would be much easier to verify. > >Without that, I think it should be bootstrapped at least on one other > >target. Note the cases you remove the parens aren't just || and &&, but > >most likely also | and & (at least there is some flag whether to print those > >as && or &). And there is code for the caching of the attributes where the > >result is still usable, I believe the patch doesn't break that, but it > >wouldn't hurt to verify that. > > Let's just defer it IMO. What do we care if other compilers are terminally > broken? Let's use it as marketing material :) Ok. Jakub
Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
On Fri, Mar 04, 2016 at 06:41:26PM +0100, Jakub Jelinek wrote:
> I'm ok with it for gcc6.
Cool.
> But IMHO you should add dg-bogus directives here.
Ok, version with dg-bogus:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-03-04 Marek Polacek
PR c/69407
* c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch
operations.
* gcc.dg/atomic-op-6.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 965cf49..25afa9c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree
function,
&& orig_code != BUILT_IN_ATOMIC_STORE_N)
result = sync_resolve_return (first_param, result, orig_format);
+ if (fetch_op)
+ /* Prevent -Wunused-value warning. */
+ TREE_USED (result) = true;
+
/* If new_return is set, assign function to that expr and cast the
result to void since the generic interface returned void. */
if (new_return)
diff --git gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-op-6.c
index e69de29..f88c293 100644
--- gcc/testsuite/gcc.dg/atomic-op-6.c
+++ gcc/testsuite/gcc.dg/atomic-op-6.c
@@ -0,0 +1,11 @@
+/* Test we don't generate bogus warnings. */
+/* PR c/69407 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+void
+foo (int *p, int a)
+{
+ __atomic_fetch_add (&p, a, 0); /* { dg-bogus "value computed is not used" }
*/
+ __atomic_add_fetch (&p, a, 0); /* { dg-bogus "value computed is not used" }
*/
+}
Marek
Re: [PATCH PR69052]Set higher precedence for CONST_WIDE_INT than CONST_INT when canonicalizing addr expr
On March 4, 2016 5:35:13 PM GMT+01:00, "Bin.Cheng" wrote: >On Fri, Mar 4, 2016 at 11:57 AM, Richard Biener > wrote: >> On Fri, Mar 4, 2016 at 12:07 PM, Bin Cheng wrote: >>> Hi, >>> A address canonicalization interface was introduced by my original >patch to PR69052. The interface sorts sub-parts in an address >expression based on precedences defined by function >commutative_operand_precedence. It also assumes that all CONST_INT >sub-parts are at the end of vector after sorting. But this is not >always true because commutative_operand_precedence sets the same >precedence to both CONST_INT and CONST_WIDE_INT. The patch could be >broken in cases which have CONST_WIDE_INT sorted after CONST_INT. Even >though I don't think we can really run into such complicated case, I >worked out a patch forcing CONST_INT to lower precedence than >CONST_WIDE_INT, so that for sure it will be sorted after all other >kinds sub-parts. >>> >>> This is an obvious change. Bootstrap&test on x86_64, bootstrap&test >on AArch64. Is it OK for this stage? >> >> I believe the obvious change would be to change >> commutative_operand_precedence to pre-CONST_WIDE_INT behavior, namely >> giving CONST_WIDE_INT the same precedence as CONST_DOUBLE. >Yes, but I am not sure what else this change will affect, so I made >the smallest change in the original code. I don't like going with this kind of weirdo changes out of caution. If you think it's too dangerous the push it back to gcc 7 and consider backporting for 6.2 I am testing this now. It >would be great if anyone describes it a bit. Thanks, Richard. > >Thanks, >bin >> >> Richard. >> >> >> >>> Thanks, >>> bin >>> >>> 2016-03-04 Bin Cheng >>> >>> PR rtl-optimization/69052 >>> * loop-invariant.c (compare_address_parts): Force CONST_INT >to lower >>> precedence than CONST_WIDE_INT.
Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
Hello! > This is not a regression but I thought I'd post this anyway. Martin reported > that we generate -Wunused-value warnings on the attached testcase, which > arguable doesn't make sense. Setting TREE_USED suppresses the warning. Since > we already compute 'fetch_op' I used that. (This warning doesn't trigger e.g. > for __atomic_load/store/compare.) > > Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc7? > > 2016-03-04 Marek Polacek > > PR c/69407 > * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch > operations. > > * gcc.dg/atomic-op-6.c: New test. You can probably revert my workaround [1] that suppressed these warnings in libsupc++/guard.cc. [1] https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00023.html Uros.
Re: [C PATCH] Prevent -Wunused-value warning with __atomic_fetch_* (PR c/69407)
On Fri, Mar 04, 2016 at 07:03:09PM +0100, Marek Polacek wrote: > Ok, version with dg-bogus: > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2016-03-04 Marek Polacek > > PR c/69407 > * c-common.c (resolve_overloaded_builtin): Set TREE_USED for the fetch > operations. > > * gcc.dg/atomic-op-6.c: New test. > diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c > index 965cf49..25afa9c 100644 > --- gcc/c-family/c-common.c > +++ gcc/c-family/c-common.c > @@ -11443,6 +11443,10 @@ resolve_overloaded_builtin (location_t loc, tree > function, > && orig_code != BUILT_IN_ATOMIC_STORE_N) > result = sync_resolve_return (first_param, result, orig_format); > > + if (fetch_op) > + /* Prevent -Wunused-value warning. */ > + TREE_USED (result) = true; > + Can't sync_resolve_return return error_mark_node? If it could, perhaps it would be saver to move this a few lines above the sync_resolve_return call, where we've already checked that result is not error_mark_node. Otherwise LGTM. Jakub
Backport fix of PR 69666 and PR 69920 to gcc-5 branch
Hi,
a week has passed with PR 69920 fix in and it seems to have fixed all
issues caused by the fix to PR 69666, which I have reverted on the
gcc-5 branch.
So I am going to un-do that revert and backport the PR 69920 fix in
one commit to the branch, after final bootstrap and testing runs
finish (actually, it has passed successfully on x86_64-linux, there is
one on i686 that is still running).
Thanks,
Martin
2016-03-03 Martin Jambor
PR tree-optimization/69666
PR middle-end/69920
* tree-sra.c (sra_modify_assign): Do not attempt to create
default_def replacements for unscalarizable regions. Do not
remove loads of uninitialized aggregates to SSA_NAMEs.
testsuite/
* gcc.dg/torture/pr69932.c: New test.
* gcc.dg/torture/pr69936.c: Likewise.
diff --git a/gcc/testsuite/gcc.dg/torture/pr69932.c
b/gcc/testsuite/gcc.dg/torture/pr69932.c
new file mode 100644
index 000..4b82130
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr69932.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+
+int a;
+void fn1() {
+ int b = 4;
+ short c[4];
+ c[b] = c[a];
+ if (c[2]) {}
+
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr69936.c
b/gcc/testsuite/gcc.dg/torture/pr69936.c
new file mode 100644
index 000..3023bbb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr69936.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+
+int a;
+char b;
+void fn1(int p1) {}
+
+int fn2() { return 5; }
+
+void fn3() {
+ if (fn2())
+;
+ else {
+char c[5];
+c[0] = 5;
+ lbl_608:
+fn1(c[9]);
+int d = c[9];
+c[2] | a;
+d = c[b];
+ }
+ goto lbl_608;
+}
+
+int main() { return 0; }
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 145a07c..3457aac 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -3242,6 +3242,7 @@ sra_modify_assign (gimple stmt, gimple_stmt_iterator *gsi)
}
else if (racc
&& !racc->grp_unscalarized_data
+ && !racc->grp_unscalarizable_region
&& TREE_CODE (lhs) == SSA_NAME
&& !access_has_replacements_p (racc))
{
@@ -3405,7 +3406,8 @@ sra_modify_assign (gimple stmt, gimple_stmt_iterator *gsi)
else
{
if (access_has_children_p (racc)
- && !racc->grp_unscalarized_data)
+ && !racc->grp_unscalarized_data
+ && TREE_CODE (lhs) != SSA_NAME)
{
if (dump_file)
{
Re: [pr/69916] ICE with empty loops
On 02/24/2016 09:07 PM, Nathan Sidwell wrote: this patch fixes the ICE reported in pr69916 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69916) The loop is lowered at omp-lowering, but subsequently determined to be dead before we get to oacc-target-lower. The loop CF is removed along with the (pure) IFN_OACC_LOOP function calls inserted during lowering. However the IFN_UNIQUE loop head & tail calls remain (because they are not pure). Thus in the oacc-target-lower pass we rediscover the loop structure. @@ -20726,10 +20742,12 @@ oacc_loop_xform_head_tail (gcall *from, determined partitioning mask and chunking argument. */ static void -oacc_loop_xform_loop (gcall *end_marker, tree mask_arg, tree chunk_arg) +oacc_loop_xform_loop (gcall *end_marker, unsigned ifns, + tree mask_arg, tree chunk_arg) Document the new arg. + gcc_checking_assert (ifns); I prefer "ifns != 0" and "ifns == 0" for non-booleans. I don't think it's a requirement, so your call. - /* If we didn't see LOOP_BOUND, it should be in the single -successor block. */ + /* The LOOP_BOUND ifn, could be in the single successor +block. */ Lose the comma? Ok with these changes. Bernd
Re: C++ PATCH for c++/70067 (ICE with typename typedef)
On Fri, Mar 4, 2016 at 8:06 AM, Jason Merrill wrote:
> On this testcase, when strip_typedefs rebuilds a TYPENAME_TYPE to remove the
> typedef, in this case it looks up the same typedef and we then abort because
> we still have a typedef. In that case, strip the typedef explicitly.
>
> Tested x86_64-pc-linux-gnu, applying to trunk and 5.
I checked this into trunk for:
ERROR: g++.dg/template/typename21.C -std=c++11: syntax error in
target selector "target c++98" for " dg-do 2 compile { target c++98 }
"
ERROR: g++.dg/template/typename21.C -std=c++14: syntax error in
target selector "target c++98" for " dg-do 2 compile { target c++98 }
"
ERROR: g++.dg/template/typename21.C -std=c++98: syntax error in
target selector "target c++98" for " dg-do 2 compile { target c++98 }
"
Will backport to 5 if needed.
--
H.J.
Index: ChangeLog
===
--- ChangeLog (revision 233974)
+++ ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2016-03-04 H.J. Lu
+
+ * g++.dg/template/typename21.C: Replace c++98 with c++98_only.
+
2016-03-04 David Malcolm
PR c/68187
Index: g++.dg/template/typename21.C
===
--- g++.dg/template/typename21.C (revision 233974)
+++ g++.dg/template/typename21.C (working copy)
@@ -1,5 +1,5 @@
// PR c++/70067
-// { dg-do compile { target c++98 } }
+// { dg-do compile { target c++98_only } }
template struct A;
template struct B { struct N { }; };
Re: C++ PATCH for c++/70067 (ICE with typename typedef)
c++98_only is wrong, we should just remove the target specifier. Jason
Re: [PATCH] Handle oacc region in oacc routine
On Wed, Mar 02, 2016 at 12:03:12PM -0500, Tom de Vries wrote: > 2015-10-16 Tom de Vries Please adjust the date ;) > * omp-low.c (check_omp_nesting_restrictions): Check for non-oacc > construct in oacc routine. Check for oacc region in oacc routine. > > * c-c++-common/goacc/nesting-fail-1.c (f_acc_routine): New function. > * c-c++-common/goacc-gomp/nesting-fail-1.c (f_acc_routine): New > function. Ok for trunk. Jakub
Re: C++ PATCH for c++/70067 (ICE with typename typedef)
On Fri, Mar 4, 2016 at 11:47 AM, Jason Merrill wrote: > c++98_only is wrong, we should just remove the target specifier. > > Jason Tested with trunk and 5. Done. -- H.J.
Re: [PING] genattrab.c generate switch
On Fri, Mar 4, 2016 at 12:49 PM, Bernd Schmidt wrote:
> On 03/04/2016 06:27 PM, Bernd Schmidt wrote:
>>
>> On 03/04/2016 06:14 PM, Patrick Palka wrote:
>>
>>> I just quickly tested building the generated insn-attrtab.c with and
>>> without the patch using my host gcc 5.3 compiler and the .s output is
>>> not the same.
>>
>>
>> Hmm, looking at the 003t.original dump it looks like there are
>> differences in SAVE_EXPRs. Indeed we seem to generate different code for
>>
>> int at;
>>
>> int foo ()
>> {
>>if (at == 2 || at == 4 || at == 7)
>> return 1;
>>return 0;
>> }
>>
>> int bar ()
>> {
>>if (at == 2 || (at == 4 || at == 7))
>> return 1;
>>return 0;
>> }
>
>
> Ahh... it's not just different placement of SAVE_EXPRs, it's actually a case
> of TRUTH_ORIF_EXPR vs. TRUTH_OR_EXPR (the distinction is invisible in the
> dumps), the latter being created by fold_range_test. That's a bit of a
> broken optimization what with its inability to see more than two comparisons
> at a time... we convert one ORIF per function, but a different one.
>
>
> Bernd
I've filed PR c/70087, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70087
[C++ PATCH] Improve -fsanitize=vptr (PR c++/70035)
Hi!
-fsanitize=vptr library side of instrumentation assumes that the vtable
pointers in objects are either NULL, or some vtable pointer, if it is random
garbage, it might crash.
The following patch attempts to clear the vtable pointers in objects next to
the spot where -flifetime-dse=2 clobbers the object.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-03-04 Jakub Jelinek
PR c++/70035
* cp-tree.h (cp_ubsan_maybe_initialize_vtbl_ptrs): New prototype.
* decl.c (start_preparsed_function): Call
cp_ubsan_maybe_initialize_vtbl_ptrs if needed.
* cp-ubsan.c (cp_ubsan_dfs_initialize_vtbl_ptrs,
cp_ubsan_maybe_initialize_vtbl_ptrs): New functions.
* g++.dg/ubsan/pr70035.C: New test.
--- gcc/cp/cp-tree.h.jj 2016-02-26 08:57:10.0 +0100
+++ gcc/cp/cp-tree.h2016-03-04 14:07:51.045471741 +0100
@@ -6940,6 +6940,7 @@ extern void cp_ubsan_maybe_instrument_me
extern void cp_ubsan_instrument_member_accesses (tree *);
extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree, tree);
extern tree cp_ubsan_maybe_instrument_cast_to_vbase (location_t, tree, tree);
+extern void cp_ubsan_maybe_initialize_vtbl_ptrs (tree);
/* -- end of C++ */
--- gcc/cp/decl.c.jj2016-02-24 22:33:35.0 +0100
+++ gcc/cp/decl.c 2016-03-04 14:53:54.283672876 +0100
@@ -14136,6 +14136,13 @@ start_preparsed_function (tree decl1, tr
finish_expr_stmt (exprstmt);
}
+ if (!processing_template_decl
+ && DECL_CONSTRUCTOR_P (decl1)
+ && (flag_sanitize & SANITIZE_VPTR)
+ && !DECL_CLONED_FUNCTION_P (decl1)
+ && !implicit_default_ctor_p (decl1))
+cp_ubsan_maybe_initialize_vtbl_ptrs (current_class_ptr);
+
return true;
}
--- gcc/cp/cp-ubsan.c.jj2016-02-24 23:00:44.0 +0100
+++ gcc/cp/cp-ubsan.c 2016-03-04 14:07:24.107840416 +0100
@@ -272,3 +272,55 @@ cp_ubsan_maybe_instrument_cast_to_vbase
return cp_ubsan_maybe_instrument_vptr (loc, op, type, true,
UBSAN_CAST_TO_VBASE);
}
+
+/* Called from initialize_vtbl_ptrs via dfs_walk. BINFO is the base
+ which we want to initialize the vtable pointer for, DATA is
+ TREE_LIST whose TREE_VALUE is the this ptr expression. */
+
+static tree
+cp_ubsan_dfs_initialize_vtbl_ptrs (tree binfo, void *data)
+{
+ if (!TYPE_CONTAINS_VPTR_P (BINFO_TYPE (binfo)))
+return dfs_skip_bases;
+
+ if (!BINFO_PRIMARY_P (binfo) || BINFO_VIRTUAL_P (binfo))
+{
+ tree base_ptr = TREE_VALUE ((tree) data);
+
+ base_ptr = build_base_path (PLUS_EXPR, base_ptr, binfo, /*nonnull=*/1,
+ tf_warning_or_error);
+
+ /* Compute the location of the vtpr. */
+ tree vtbl_ptr
+ = build_vfield_ref (cp_build_indirect_ref (base_ptr, RO_NULL,
+ tf_warning_or_error),
+ TREE_TYPE (binfo));
+ gcc_assert (vtbl_ptr != error_mark_node);
+
+ /* Assign NULL to the vptr. */
+ tree vtbl = build_zero_cst (TREE_TYPE (vtbl_ptr));
+ finish_expr_stmt (cp_build_modify_expr (vtbl_ptr, NOP_EXPR, vtbl,
+ tf_warning_or_error));
+}
+
+ return NULL_TREE;
+}
+
+/* Initialize all the vtable pointers in the object pointed to by
+ ADDR to NULL, so that we catch invalid calls to methods before
+ mem-initializers are completed. */
+
+void
+cp_ubsan_maybe_initialize_vtbl_ptrs (tree addr)
+{
+ if (!cp_ubsan_instrument_vptr_p (NULL_TREE))
+return;
+
+ tree type = TREE_TYPE (TREE_TYPE (addr));
+ tree list = build_tree_list (type, addr);
+
+ /* Walk through the hierarchy, initializing the vptr in each base
+ class to NULL. */
+ dfs_walk_once (TYPE_BINFO (type), cp_ubsan_dfs_initialize_vtbl_ptrs,
+NULL, list);
+}
--- gcc/testsuite/g++.dg/ubsan/pr70035.C.jj 2016-03-04 14:20:39.234960689
+0100
+++ gcc/testsuite/g++.dg/ubsan/pr70035.C2016-03-04 14:55:36.988268869
+0100
@@ -0,0 +1,26 @@
+// PR c++/70035
+// { dg-do run }
+// { dg-shouldfail "ubsan" }
+// { dg-options "-fsanitize=vptr -fno-sanitize-recover=undefined" }
+
+struct A {
+ A (int) {}
+ virtual int foo () { return 1; }
+};
+struct B : public A {
+ using A::foo;
+ B (int x) : A (foo (x)) {}
+ int foo (int x) { return x * 2; }
+};
+
+int
+main ()
+{
+ B b (20);
+}
+
+// { dg-output "\[^\n\r]*pr70035.C:12:\[0-9]*: runtime error: member call on
address 0x\[0-9a-fA-F]* which does not point to an object of type
'B'(\n|\r\n|\r)" }
+// { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
+// { dg-output " ?.. .. .. .. ?.. .. .. .. ?.. .. .. ..
\[^\n\r]*(\n|\r\n|\r)" }
+// { dg-output " ?\\^~~\[^\n\r]*(\n|\r\n|\r)" }
+// { dg-output " ?invalid vptr" }
Jakub
[PATCH] Fix va_arg ((ap), ...) on s390* with C++14 (PR c++/70084)
Hi!
As mentioned in the PR, on the following testcase we emit invalid
error on s390* in the va_arg ((ap), int) case, va_arg (ap, int) is fine
(and the former is fine in -std=c++11 or -std=c++98 too).
The bug only triggers in constructors (or destructors), and happens because
build_va_arg creates ADDR_EXPR that has slightly different type, and has
INDIRECT_REF inside of it. copy_tree_body_r notices this and cancels the
two, but unlike e.g. build_fold_addr_expr_with_type_loc
if (TREE_CODE (t) == INDIRECT_REF)
{
t = TREE_OPERAND (t, 0);
if (TREE_TYPE (t) != ptrtype)
t = build1_loc (loc, NOP_EXPR, ptrtype, t); <<-- this
}
it doesn't attempt to handle the case where ADDR_EXPR contained different
type from the type of INDIRECT_REF's operand.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
bootstrap/regtest on s390{,x}-linux pending. Ok for trunk if it passes
even there?
2016-03-04 Jakub Jelinek
PR c++/70084
* tree-inline.c (copy_tree_body_r): When cancelling ADDR_EXPR
of INDIRECT_REF and ADDR_EXPR changed type, fold_convert it
to the right type.
* g++.dg/expr/stdarg3.C: New test.
--- gcc/tree-inline.c.jj2016-02-12 00:50:55.0 +0100
+++ gcc/tree-inline.c 2016-03-04 18:35:28.079458603 +0100
@@ -1266,7 +1266,12 @@ copy_tree_body_r (tree *tp, int *walk_su
/* Handle the case where we substituted an INDIRECT_REF
into the operand of the ADDR_EXPR. */
if (TREE_CODE (TREE_OPERAND (*tp, 0)) == INDIRECT_REF)
- *tp = TREE_OPERAND (TREE_OPERAND (*tp, 0), 0);
+ {
+ tree t = TREE_OPERAND (TREE_OPERAND (*tp, 0), 0);
+ if (TREE_TYPE (t) != TREE_TYPE (*tp))
+ t = fold_convert (remap_type (TREE_TYPE (*tp), id), t);
+ *tp = t;
+ }
else
recompute_tree_invariant_for_addr_expr (*tp);
--- gcc/testsuite/g++.dg/expr/stdarg3.C.jj 2016-03-04 18:08:22.029669394
+0100
+++ gcc/testsuite/g++.dg/expr/stdarg3.C 2016-03-04 18:06:50.0 +0100
@@ -0,0 +1,18 @@
+// PR c++/70084
+// { dg-do compile }
+
+#include
+
+struct A
+{
+ A (const char *f, ...);
+};
+
+A::A (const char *f, ...)
+{
+ va_list ap;
+ va_start (ap, f);
+ int i = va_arg (ap, int);// { dg-bogus "first argument to 'va_arg' not
of type 'va_list'" }
+ int j = va_arg ((ap), int); // { dg-bogus "first argument to 'va_arg' not
of type 'va_list'" }
+ va_end (ap);
+}
Jakub
[C++ PATCH] Fix up -flifetime-dse=2 clobbers
Hi! While working on PR70035, I've noticed that the clobbers at the start of (certain) constructors are strangely emitted twice instead of just once. The problem is that start_preparsed_function is first called on the (abstract) constructor and then again on the cloned constructors (base or complete ctor). And in each case the clobber is emitted. Fixed by only emitting it on the (abstract) constructor, not on the clones. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2016-03-04 Jakub Jelinek * decl.c (start_preparsed_function): Don't emit start clobber at the start of constructor clones. --- gcc/decl.c.jj 2016-03-04 19:35:09.0 +0100 +++ gcc/decl.c 2016-03-04 19:38:17.040994718 +0100 @@ -14120,6 +14120,7 @@ start_preparsed_function (tree decl1, tr if (!processing_template_decl && (flag_lifetime_dse > 1) && DECL_CONSTRUCTOR_P (decl1) + && !DECL_CLONED_FUNCTION_P (decl1) /* We can't clobber safely for an implicitly-defined default constructor because part of the initialization might happen before we enter the constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */ Jakub
Re: [C++ PATCH] Fix up -flifetime-dse=2 clobbers
OK. Jason
Re: [C++ PATCH] Improve -fsanitize=vptr (PR c++/70035)
OK. Jason
C++ PATCH to constexpr loops and SAVE_EXPR
This patch addresses a significant wrong-code issue with constexpr
loops: the value of a SAVE_EXPR was being cached across iterations, so
an increment would only happen once.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit b4fbc938b1ce031d9b082f2d3e062976ac431977
Author: Jason Merrill
Date: Wed Mar 2 16:10:07 2016 -0500
Fix constexpr handling of SAVE_EXPR in loops.
* constexpr.c (struct constexpr_ctx): Add save_exprs field.
(cxx_eval_loop_expr): Discard SAVE_EXPR values before looping.
(cxx_eval_constant_expression) [SAVE_EXPR]: Add it to the set.
(cxx_eval_outermost_constant_expr, is_sub_constant_expr): Initialize.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 5a81469..4fadc0f 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -897,6 +897,9 @@ struct constexpr_ctx {
/* Values for any temporaries or local variables within the
constant-expression. */
hash_map *values;
+ /* SAVE_EXPRs that we've seen within the current LOOP_EXPR. NULL if we
+ aren't inside a loop. */
+ hash_set *save_exprs;
/* The CONSTRUCTOR we're currently building up for an aggregate
initializer. */
tree ctor;
@@ -3161,16 +3164,27 @@ cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
bool *non_constant_p, bool *overflow_p,
tree *jump_target)
{
+ constexpr_ctx new_ctx = *ctx;
+
tree body = TREE_OPERAND (t, 0);
while (true)
{
- cxx_eval_statement_list (ctx, body,
+ hash_set save_exprs;
+ new_ctx.save_exprs = &save_exprs;
+
+ cxx_eval_statement_list (&new_ctx, body,
non_constant_p, overflow_p, jump_target);
if (returns (jump_target) || breaks (jump_target) || *non_constant_p)
break;
+
+ /* Forget saved values of SAVE_EXPRs. */
+ for (hash_set::iterator iter = save_exprs.begin();
+ iter != save_exprs.end(); ++iter)
+ new_ctx.values->remove (*iter);
}
if (breaks (jump_target))
*jump_target = NULL_TREE;
+
return NULL_TREE;
}
@@ -3452,6 +3466,8 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), false,
non_constant_p, overflow_p);
ctx->values->put (t, r);
+ if (ctx->save_exprs)
+ ctx->save_exprs->add (t);
}
break;
@@ -3875,7 +3891,10 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
bool non_constant_p = false;
bool overflow_p = false;
hash_map map;
- constexpr_ctx ctx = { NULL, &map, NULL, NULL, allow_non_constant, strict };
+
+ constexpr_ctx ctx = { NULL, &map, NULL, NULL, NULL,
+ allow_non_constant, strict };
+
tree type = initialized_type (t);
tree r = t;
if (AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type))
@@ -3983,7 +4002,9 @@ is_sub_constant_expr (tree t)
bool non_constant_p = false;
bool overflow_p = false;
hash_map map;
- constexpr_ctx ctx = { NULL, &map, NULL, NULL, true, true };
+
+ constexpr_ctx ctx = { NULL, &map, NULL, NULL, NULL, true, true };
+
cxx_eval_constant_expression (&ctx, t, false, &non_constant_p,
&overflow_p);
return !non_constant_p && !overflow_p;
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-loop3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-loop3.C
new file mode 100644
index 000..5e7c3c9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-loop3.C
@@ -0,0 +1,23 @@
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+ int i;
+};
+
+constexpr bool f()
+{
+ A ar[4] = { 1, 2, 3, 4 };
+ A *ap = ar;
+ int i = 0;
+ do
+*ap++ = A{i};
+ while (++i < 3);
+ return (ar[0].i == 0
+ && ar[1].i == 1
+ && ar[2].i == 2
+ && ar[3].i == 4);
+}
+
+#define SA(X) static_assert((X),#X)
+SA(f());
Re: C++ PATCH for c++/67364 (constexpr vs. empty class)
On 03/03/2016 05:37 PM, Jason Merrill wrote:
On 02/25/2016 09:08 AM, Jason Merrill wrote:
We don't bother evaluating a store to an empty class member, and we
shouldn't complain about accesses either.
This needs to use really_empty_class, since that's what
expand_aggr_init_1 uses.
And even calling build_value_init is wrong for these classes, as it
might not be well-formed. Just build a value directly.
Tested x86_64-pc-linux-gnu, applying to trunk and 5.
commit 6207dfd6bce685275831f468e9954bec71d39430
Author: Jason Merrill
Date: Fri Mar 4 15:40:16 2016 -0500
PR c++/67364
* constexpr.c (cxx_eval_component_reference): Further tweak.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 4fadc0f..d308175 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1990,13 +1990,16 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
return t;
}
- if (CONSTRUCTOR_NO_IMPLICIT_ZERO (whole)
- && !is_really_empty_class (TREE_TYPE (t)))
+ /* We only create a CONSTRUCTOR for a subobject when we modify it, so empty
+ classes never get represented; throw together a value now. */
+ if (is_really_empty_class (TREE_TYPE (t)))
+return build_constructor (TREE_TYPE (t), NULL);
+
+ if (CONSTRUCTOR_NO_IMPLICIT_ZERO (whole))
{
/* 'whole' is part of the aggregate initializer we're currently
building; if there's no initializer for this member yet, that's an
- error. But expand_aggr_init_1 doesn't bother to initialize really
- empty classes, so ignore them here, too. */
+ error. */
if (!ctx->quiet)
error ("accessing uninitialized member %qD", part);
*non_constant_p = true;
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-empty2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-empty2.C
new file mode 100644
index 000..2acfa98
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-empty2.C
@@ -0,0 +1,27 @@
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+ constexpr A(int) { }
+};
+
+struct B: A {
+ constexpr B(int i): A(i) { }
+ constexpr B(const B& b): A(b) { }
+};
+
+struct C {
+ B b;
+ constexpr C(int i): b(i) { }
+ constexpr C(const C&c): b(c.b) {}
+};
+
+constexpr int f()
+{
+ C b1{42};
+ C b2{b1};
+ b2.b;
+ return 42;
+}
+
+constexpr int i = f();
[commit] Sync include/plugin-api.h with binutils
I'm committing the attached patch to sync include/plugin-api.h with binutils. -cary 2016-03-03 Than McIntosh * plugin-api.h: Add new hooks to the plugin transfer vector to to support querying section alignment and section size. (ld_plugin_get_input_section_alignment): New hook. (ld_plugin_get_input_section_size): New hook. (ld_plugin_tag): Add LDPT_GET_INPUT_SECTION_ALIGNMENT and LDPT_GET_INPUT_SECTION_SIZE. (ld_plugin_tv): Add tv_get_input_section_alignment and tv_get_input_section_size. 2016-03-03 Evgenii Stepanov * plugin-api.h (enum ld_plugin_tag): Add LDPT_GET_SYMBOLS_V3. plugin.patch Description: Binary data
Re: [PATCH] Fix va_arg ((ap), ...) on s390* with C++14 (PR c++/70084)
On 03/04/2016 01:37 PM, Jakub Jelinek wrote:
Hi!
As mentioned in the PR, on the following testcase we emit invalid
error on s390* in the va_arg ((ap), int) case, va_arg (ap, int) is fine
(and the former is fine in -std=c++11 or -std=c++98 too).
The bug only triggers in constructors (or destructors), and happens because
build_va_arg creates ADDR_EXPR that has slightly different type, and has
INDIRECT_REF inside of it. copy_tree_body_r notices this and cancels the
two, but unlike e.g. build_fold_addr_expr_with_type_loc
if (TREE_CODE (t) == INDIRECT_REF)
{
t = TREE_OPERAND (t, 0);
if (TREE_TYPE (t) != ptrtype)
t = build1_loc (loc, NOP_EXPR, ptrtype, t); <<-- this
}
it doesn't attempt to handle the case where ADDR_EXPR contained different
type from the type of INDIRECT_REF's operand.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
bootstrap/regtest on s390{,x}-linux pending. Ok for trunk if it passes
even there?
2016-03-04 Jakub Jelinek
PR c++/70084
* tree-inline.c (copy_tree_body_r): When cancelling ADDR_EXPR
of INDIRECT_REF and ADDR_EXPR changed type, fold_convert it
to the right type.
* g++.dg/expr/stdarg3.C: New test.
OK.
jeff
C++ PATCH for c++/69203 (ICE with delete[])
The representation of delete[] causes the constexpr machinery to ICE.
That's a separate bug worth fixing, but for GCC 6 it's simpler to stop
when we see the wrapper, and also gives a better diagnostic.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 3cf04bc44a26e84e8c88231572baadb465c70ed3
Author: Jason Merrill
Date: Tue Mar 1 09:08:37 2016 -0500
PR c++/69203
* cp-tree.h (COND_EXPR_IS_VEC_DELETE): New.
* init.c (build_vec_delete_1): Set it.
* constexpr.c (potential_constant_expression_1) [COND_EXPR]: Check it.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index d308175..c9f9c47 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4885,8 +4885,16 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict,
return false;
return true;
+case COND_EXPR:
+ if (COND_EXPR_IS_VEC_DELETE (t))
+ {
+ if (flags & tf_error)
+ error_at (location_of (t),
+ "% is not a constant-expression");
+ return false;
+ }
+ /* Fall through. */
case IF_STMT:
-case COND_EXPR:
case VEC_COND_EXPR:
/* If the condition is a known constant, we know which of the legs we
care about; otherwise we only require that the condition and
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b1dc23c..9c7f0cc 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -107,6 +107,7 @@ operator == (const cp_expr &lhs, tree rhs)
/* Usage of TREE_LANG_FLAG_?:
0: IDENTIFIER_MARKED (IDENTIFIER_NODEs)
NEW_EXPR_USE_GLOBAL (in NEW_EXPR).
+ COND_EXPR_IS_VEC_DELETE (in COND_EXPR).
DELETE_EXPR_USE_GLOBAL (in DELETE_EXPR).
COMPOUND_EXPR_OVERLOADED (in COMPOUND_EXPR).
CLEANUP_P (in TRY_BLOCK)
@@ -404,6 +405,9 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
#define STMT_EXPR_NO_SCOPE(NODE) \
TREE_LANG_FLAG_0 (STMT_EXPR_CHECK (NODE))
+#define COND_EXPR_IS_VEC_DELETE(NODE) \
+ TREE_LANG_FLAG_0 (COND_EXPR_CHECK (NODE))
+
/* Returns nonzero iff TYPE1 and TYPE2 are the same type, in the usual
sense of `same'. */
#define same_type_p(TYPE1, TYPE2) \
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 43f854c..1ba3c59 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3685,6 +3685,7 @@ build_vec_delete_1 (tree base, tree maxindex, tree type,
TREE_NO_WARNING (cond) = 1;
body = build3_loc (input_location, COND_EXPR, void_type_node,
cond, body, integer_zero_node);
+ COND_EXPR_IS_VEC_DELETE (body) = true;
body = build1 (NOP_EXPR, void_type_node, body);
if (controller)
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-delete2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-delete2.C
new file mode 100644
index 000..4a453a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-delete2.C
@@ -0,0 +1,12 @@
+// PR c++/69203
+// { dg-do compile { target c++11 } }
+
+struct A { ~A(); };
+constexpr int f(int i) { return i; }
+constexpr int g(A* ap)
+{
+ return f((delete[] ap, 42)); // { dg-message "" }
+}
+
+A a;
+constexpr int i = g(&a); // { dg-error "" }
Re: [PATCH] Fix PR c++/66786 (ICE with nested lambdas in variable template)
OK. Jason
Fix PR69824, crash in C frontend
This is a crash encountered with an implicit declaration inside an
argument list. In the PR, Jakub identified a place where we change the
DECL_CONTEXT for such decls, and I think I agree with him that this
seems wrong. The following patch ensures they aren't placed on the list
in the first place.
Bootstrapped and tested on x86_64-linux, ok?
Bernd
c/
PR c/69824
* c-decl.c (get_parm_info): Don't queue implicit function declarations
for later.
testsuite/
PR c/69824
* gcc.dg/pr69824.c: New test.
Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c (revision 233451)
+++ gcc/c/c-decl.c (working copy)
@@ -7050,25 +7050,28 @@ get_parm_info (bool ellipsis, tree expr)
vec_safe_push (tags, tag);
break;
+ case FUNCTION_DECL:
+ /* FUNCTION_DECLs appear when there is an implicit function
+ declaration in the parameter list. */
+ gcc_assert (b->nested);
+ goto set_shadowed;
+
case CONST_DECL:
case TYPE_DECL:
- case FUNCTION_DECL:
/* CONST_DECLs appear here when we have an embedded enum,
and TYPE_DECLs appear here when we have an embedded struct
or union. No warnings for this - we already warned about the
- type itself. FUNCTION_DECLs appear when there is an implicit
- function declaration in the parameter list. */
+ type itself. */
/* When we reinsert this decl in the function body, we need
to reconstruct whether it was marked as nested. */
- gcc_assert (TREE_CODE (decl) == FUNCTION_DECL
- ? b->nested
- : !b->nested);
+ gcc_assert (!b->nested);
DECL_CHAIN (decl) = others;
others = decl;
/* fall through */
case ERROR_MARK:
+ set_shadowed:
/* error_mark_node appears here when we have an undeclared
variable. Just throw it away. */
if (b->id)
Index: gcc/testsuite/gcc.dg/pr69824.c
===
--- gcc/testsuite/gcc.dg/pr69824.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr69824.c (working copy)
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-w" } */
+int bar() { return foo(); }
+void baz(int c[foo()]) { return; }
Limit vector alignments (PR 69973)
This is a problem I ran into before, although I can't remember the
details. The problem here is that a user program requests an
unreasonably large vector, the default_vector_alignment returns an
unreasonably large alignment in a HOST_WIDE_INT, which then gets stored
int TYPE_ALIGN, which is an int. Therefore TYPE_ALIGN becomes zero,
smaller than the type size, and we abort.
It seems misguided not to restrict alignments much more drastically in
this hook, but for now I chose what I think is a conservative fix:
limiting alignments to MAX_OFILE_ALIGNMENT.
Bootstrapped and tested on x86_64-linux. Ok?
Bernd
PR c/69973
* targhooks.c (default_vector_alignment): Limit to MAX_OFILE_ALIGNMENT.
testsuite/
PR c/69973
* gcc.dg/pr69973.c: New test.
Index: gcc/targhooks.c
===
--- gcc/targhooks.c (revision 233451)
+++ gcc/targhooks.c (working copy)
@@ -1031,7 +1031,10 @@ tree default_mangle_decl_assembler_name
HOST_WIDE_INT
default_vector_alignment (const_tree type)
{
- return tree_to_shwi (TYPE_SIZE (type));
+ HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type));
+ if (align > MAX_OFILE_ALIGNMENT)
+align = MAX_OFILE_ALIGNMENT;
+ return align;
}
bool
Index: gcc/testsuite/gcc.dg/pr69973.c
===
--- gcc/testsuite/gcc.dg/pr69973.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr69973.c (working copy)
@@ -0,0 +1,2 @@
+/* { dg-do compile } */
+typedef int v4si __attribute__ ((vector_size (1 << 29)));
Re: Proposed Patch for Bug 69687
On Thu, 3 Mar 2016, Mike Stump wrote: > On Mar 3, 2016, at 6:21 AM, Bernd Schmidt wrote: > > What C standard can we assume for libiberty? I was looking at patching this > > and discovered that SIZE_MAX is defined only for C99, so I'm leaning > > towards retaining the ints and using INT_MAX. > > As long as you don’t need a constant… you can also do something like: > > #ifndef SIZE_MAX > #define SIZE_MAX (sizeof (size_t) == sizeof (int) ? INT_MAX : sizeof > (size_t) == sizeof (long) ? LONG_MAX : (abort (), 0)) > #endif If you don't require usability in #if (so can use casts), ((size_t) -1) suffices as a value of SIZE_MAX (size_t is always unsigned). -- Joseph S. Myers jos...@codesourcery.com
Fix postreload_combine miscompilation (PR 69941)
This is a transformation which looks for
(set reg1 const)
(set reg2 (plus reg2 reg1))
and tries to replace all further uses of reg2 with (plus reg2 reg1). The
problem here is that one of the uses is in a narrower mode, inside a
zero_extend, so we produce wrong results.
The fix seems rather straightforward, verifying all uses have a mode
matching the set. Bootstrapped and tested on x86_64-linux, ok?
Bernd
PR rtl-optimization/69941
* postreload.c (reload_combine_recognize_pattern): Ensure all uses of
the reg share its mode.
testsuite/
PR rtl-optimization/69941
* gcc.dg/torture/pr69941.c: New test.
Index: gcc/postreload.c
===
--- gcc/postreload.c (revision 233451)
+++ gcc/postreload.c (working copy)
@@ -1057,7 +1057,6 @@ static bool
reload_combine_recognize_pattern (rtx_insn *insn)
{
rtx set, reg, src;
- unsigned int regno;
set = single_set (insn);
if (set == NULL_RTX)
@@ -1068,7 +1067,20 @@ reload_combine_recognize_pattern (rtx_in
if (!REG_P (reg) || REG_NREGS (reg) != 1)
return false;
- regno = REGNO (reg);
+ unsigned int regno = REGNO (reg);
+ machine_mode mode = GET_MODE (reg);
+
+ if (reg_state[regno].use_index < 0
+ || reg_state[regno].use_index >= RELOAD_COMBINE_MAX_USES)
+return false;
+
+ for (int i = reg_state[regno].use_index;
+ i < RELOAD_COMBINE_MAX_USES; i++)
+{
+ struct reg_use *use = reg_state[regno].reg_use + i;
+ if (GET_MODE (*use->usep) != mode)
+ return false;
+}
/* Look for (set (REGX) (CONST_INT))
(set (REGX) (PLUS (REGX) (REGY)))
@@ -1090,8 +1102,6 @@ reload_combine_recognize_pattern (rtx_in
&& REG_P (XEXP (src, 1))
&& rtx_equal_p (XEXP (src, 0), reg)
&& !rtx_equal_p (XEXP (src, 1), reg)
- && reg_state[regno].use_index >= 0
- && reg_state[regno].use_index < RELOAD_COMBINE_MAX_USES
&& last_label_ruid < reg_state[regno].use_ruid)
{
rtx base = XEXP (src, 1);
Index: gcc/testsuite/gcc.dg/torture/pr69941.c
===
--- gcc/testsuite/gcc.dg/torture/pr69941.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr69941.c (working copy)
@@ -0,0 +1,30 @@
+/* { dg-do run } */
+
+int a = 0;
+int b = 0;
+int c = 0;
+int e = 0;
+int f = 0;
+int *g = &e;
+
+int fn1() { return b ? a : b; }
+
+int main() {
+ int h = fn1() <= 0x8000ULL; // h = 1;
+
+ int k = f; // k = 0;
+
+ long i = h ? k : k / h; // i = 0;
+
+ long l = (unsigned short)(i - 0x1800); // l = 0xe800
+
+ i = l ? l : c; // i = 0xe800;
+
+ *g = i; // *g = 0xe800; e = 0xe800;
+
+ unsigned char result = e >> 9; // result = 0x74;
+
+ if ((int)result != 0x74)
+__builtin_abort ();
+ return 0;
+}
Re: Fix PR69824, crash in C frontend
On Sat, 5 Mar 2016, Bernd Schmidt wrote: > This is a crash encountered with an implicit declaration inside an argument > list. In the PR, Jakub identified a place where we change the DECL_CONTEXT for > such decls, and I think I agree with him that this seems wrong. The following > patch ensures they aren't placed on the list in the first place. > > Bootstrapped and tested on x86_64-linux, ok? OK. -- Joseph S. Myers jos...@codesourcery.com
[PATCH] [PR tree-optimization/69196] Consider two anonymous SSA_NAMEs unassociated
As pointed out by Richi, the code did not precisely match the comment in the case of two anonymous SSA_NAMEs. In that case we don't have enough information to determine if the names are associated. Until we do something like build partitions (similar to what's done in tree-ssa-coalesce), it seems best to consider two anonymous SSA_NAMEs to be unassociated and count the PHI against the statement count for threading. Bootstrapped and regression tested on x86-64. Installed on the trunk. Jeff commit 2ef58c4014fc23573d8ff10e50381c6cbdcba6e6 Author: law Date: Sat Mar 5 05:10:58 2016 + PR tree-optimization/69196 * tree-ssa-threadbackward.c (fsm_find_control_statement_thread_paths): If the both SSA_NAMEs are anonymous, then consider them unassociated and include the PHI in the statement count. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@233999 138bc75d-0d04-0410-961f-82ee72b054a4 diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5c23836..09a2714 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2016-03-04 Jeff Law + + PR tree-optimization/69196 + * tree-ssa-threadbackward.c (fsm_find_control_statement_thread_paths): + If the both SSA_NAMEs are anonymous, then consider them unassociated + and include the PHI in the statement count. + 2016-03-05 Tom de Vries * omp-low.c (check_omp_nesting_restrictions): Check for non-oacc diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c index 747296b..6f1b757 100644 --- a/gcc/tree-ssa-threadbackward.c +++ b/gcc/tree-ssa-threadbackward.c @@ -311,7 +311,11 @@ fsm_find_control_statement_thread_paths (tree name, gphi *phi = gsip.phi (); tree dst = gimple_phi_result (phi); - if (SSA_NAME_VAR (dst) != SSA_NAME_VAR (name) + /* Note that if both NAME and DST are anonymous +SSA_NAMEs, then we do not have enough information +to consider them associated. */ + if ((SSA_NAME_VAR (dst) != SSA_NAME_VAR (name) + || !SSA_NAME_VAR (dst)) && !virtual_operand_p (dst)) ++n_insns; }
Re: Fix PR69824, crash in C frontend
On 03/04/2016 05:08 PM, Joseph Myers wrote: On Sat, 5 Mar 2016, Bernd Schmidt wrote: This is a crash encountered with an implicit declaration inside an argument list. In the PR, Jakub identified a place where we change the DECL_CONTEXT for such decls, and I think I agree with him that this seems wrong. The following patch ensures they aren't placed on the list in the first place. Bootstrapped and tested on x86_64-linux, ok? OK. I went ahead and committed this to the trunk and removed the the gcc-6 regression marker in the BZ. Bernd's call if/when to backport to the existing release branches. jeff
Re: Fix postreload_combine miscompilation (PR 69941)
On 03/04/2016 04:57 PM, Bernd Schmidt wrote: This is a transformation which looks for (set reg1 const) (set reg2 (plus reg2 reg1)) and tries to replace all further uses of reg2 with (plus reg2 reg1). The problem here is that one of the uses is in a narrower mode, inside a zero_extend, so we produce wrong results. The fix seems rather straightforward, verifying all uses have a mode matching the set. Bootstrapped and tested on x86_64-linux, ok? Bernd rcom-modes.diff PR rtl-optimization/69941 * postreload.c (reload_combine_recognize_pattern): Ensure all uses of the reg share its mode. testsuite/ PR rtl-optimization/69941 * gcc.dg/torture/pr69941.c: New test. OK. FWIW, based on my reading of the BZ, I think this counts as a regression -- it just happens to come and go. I strongly suspect that's due to changing register assignments or some such. I'm going to go ahead and push this to the trunk. Jeff
Re: Limit vector alignments (PR 69973)
On 03/04/2016 04:55 PM, Bernd Schmidt wrote: This is a problem I ran into before, although I can't remember the details. The problem here is that a user program requests an unreasonably large vector, the default_vector_alignment returns an unreasonably large alignment in a HOST_WIDE_INT, which then gets stored int TYPE_ALIGN, which is an int. Therefore TYPE_ALIGN becomes zero, smaller than the type size, and we abort. It seems misguided not to restrict alignments much more drastically in this hook, but for now I chose what I think is a conservative fix: limiting alignments to MAX_OFILE_ALIGNMENT. Bootstrapped and tested on x86_64-linux. Ok? Bernd valign.diff PR c/69973 * targhooks.c (default_vector_alignment): Limit to MAX_OFILE_ALIGNMENT. testsuite/ PR c/69973 * gcc.dg/pr69973.c: New test. This is safe enough that even though it's not a regression I think it's fine for gcc-6. I'll commit to the trunk momentarily. jeff
