Re: [PING v3] Unreviewed GCC-6 patches
On Mon, 29 Aug 2016, Jakub Sejdak wrote: > Hi! > > I would like to ping a couple of unreviewed patches for GCC-6 branch > (they are already in trunk): > > - Backport new Phoenix-RTOS OS name to config.sub > https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01441.html > > - Backport support for Phoenix-RTOS targets in GCC's config for ARM platform. > https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01442.html > > - Backport support for Phoenix-RTOS targets in libgcc's config for ARM > platform. > https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01440.html Ok. Thanks, Richard. > Thanks, > Kuba Sejdak > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
[v3 PATCH, RFC] Implement LWG 2729 for tuple
First, pardons all around if I'm completely repeating what Marc
already tried to do.
I think I'm taking a different approach.
I'm not adding any defaulted or deleted functions, so I don't think I'm changing
triviality. Chances are I need to actually delete the copy operations I want to
get rid of, though; currently they are defined with a conditional
parameter type.
That leaves open the possibility of a move operation being implicitly defaulted
rather than suppressed, which can make them trivial where they weren't before.
I'm not sure whether this has abi impact. In the sense that this is making
previously-defined move operations suppressed, maybe. I guess I could
alternatively try making them conditional, and making the inverse of the
conditional private; that should keep the traits working and retain abi
compatibility, because I don't think our toolchains at least on non-Windows
platforms include access levels in mangling.
Tested for tuple on Linux-x64. Thoughts?
2016-08-30 Ville Voutilainen
Implement LWG 2729 for tuple.
* include/std/tuple (_Tuple_impl(_Tuple_impl&&)):
Suppress conditionally.
(_Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&&)): Likewise.
(__is_tuple_impl_trait_impl, __is_tuple_impl_trait): New.
(_Tuple_impl(const _Head&)): Constrain.
(_Tuple_impl(_UHead&&)): Likewise.
(_Tuple_impl(_Tuple_impl&&)): Suppress conditionally.
(_Tuple_impl(const _Tuple_impl<_Idx, _UHead>&)): Constrain.
(_Tuple_impl(_Tuple_impl<_Idx, _UHead>&&)): Likewise.
(operator=(const tuple&)): Enable conditionally.
(operator=(tuple&&)): Suppress conditionally.
(operator=(const tuple<_UElements...>&)): Constrain.
(operator=(tuple<_UElements...>&&)): Likewise.
(operator=(const tuple&)): Enable conditionally (2-param tuple).
(operator=(tuple&&)): Suppress conditionally (2-param tuple).
(operator=(const tuple<_U1, _U2>&)): Constrain.
(operator=(tuple<_U1, _U2>&&)): Likewise.
(operator=(const pair<_U1, _U2>&)): Likewise.
(operator=(pair<_U1, _U2>&&)): Likewise.
* testsuite/20_util/tuple/element_access/get_neg.cc: Adjust.
* testsuite/20_util/tuple/tuple_traits.cc: New.
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index c06a040..9f43732 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -220,8 +220,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr _Tuple_impl(const _Tuple_impl&) = default;
constexpr
- _Tuple_impl(_Tuple_impl&& __in)
- noexcept(__and_,
+ _Tuple_impl(typename conditional<
+ __and_,
+is_move_constructible<_Inherited>>::value,
+ _Tuple_impl&&, __nonesuch&&>::type __in)
+ noexcept(__and_,
is_nothrow_move_constructible<_Inherited>>::value)
: _Inherited(std::move(_M_tail(__in))),
_Base(std::forward<_Head>(_M_head(__in))) { }
@@ -232,7 +235,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Base(_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
template
-constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
+constexpr _Tuple_impl(typename conditional<
+ __and_,
+ is_move_constructible<_Inherited>>::value,
+ _Tuple_impl<_Idx, _UHead, _UTails...>&&,
+ __nonesuch&&>::type __in)
: _Inherited(std::move
(_Tuple_impl<_Idx, _UHead, _UTails...>::_M_tail(__in))),
_Base(std::forward<_UHead>
@@ -338,6 +345,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};
+ template
+struct __is_tuple_impl_trait_impl : false_type
+ { };
+
+ template
+struct __is_tuple_impl_trait_impl<_Tuple_impl<_Idx, _Tp...>> : true_type
+ { };
+
+ template
+struct __is_tuple_impl_trait : public __is_tuple_impl_trait_impl<_Tp>
+ { };
+
// Basis case of inheritance recursion.
template
struct _Tuple_impl<_Idx, _Head>
@@ -356,11 +375,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr _Tuple_impl()
: _Base() { }
+ template::value,
+ bool>::type=true>
explicit
constexpr _Tuple_impl(const _Head& __head)
: _Base(__head) { }
- template
+ template,
+__not_<__is_tuple_impl_trait<
+ typename
+remove_reference<_UHead>::type>>
+>::value,
+ bool>::type = true>
explicit
constexpr _Tuple_impl(_UHead&& __head)
: _Base(std::forward<_UHead>(__head)) { }
@@ -368,15 +396,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr _Tuple_impl(const _Tuple_impl&) = default;
constexpr
- _Tuple_impl(_Tuple_impl&& __in)
+ _Tuple_impl(typename conditional<
+
Re: [PATCHv2, PING 3][ARM] -mpure-code option for ARM
On 11/08/16 15:13, Andre Vieira (lists) wrote: > On 25/07/16 11:52, Andre Vieira (lists) wrote: >> On 11/07/16 17:56, Andre Vieira (lists) wrote: >>> On 07/07/16 13:30, mickael guene wrote: Hi Andre, Another feedback on your purecode patch. You have to disable casesi pattern since then it will generate wrong code with -mpure-code option. Indeed it will generate an 'adr rx, .Lx' (aka 'subs rx, PC, #offset') which will not work in our case since 'Lx' label is put in an .rodata section. So offset value is unknown and can be impossible to encode correctly. Regards Mickael On 06/30/2016 04:32 PM, Andre Vieira (lists) wrote: > Hello, > > This patch adds the -mpure-code option for ARM. This option ensures > functions are put into sections that contain only code and no data. To > ensure this throughout compilation we give these sections the ARM > processor-specific ELF section attribute "SHF_ARM_PURECODE". This option > is only supported for non-pic code for armv7-m targets. > > This patch introduces a new target hook 'TARGET_ASM_ELF_FLAGS_NUMERIC'. > This target hook enables a target to use the numeric value for elf > section attributes rather than their alphabetical representation. If > TARGET_ASM_ELF_FLAGS_NUMERIC returns TRUE, the existing > 'default_elf_asm_named_section', will print the numeric value of the > section attributes for the current section. This target hook has two > parameters: > unsigned int FLAGS, the input parameter that tells the function the > current section's attributes; > unsigned int *NUM, used to pass down the numerical representation of the > section's attributes. > > The default implementation for TARGET_ASM_ELF_FLAGS_NUMERIC will return > false, so existing behavior is not changed. > > Bootstrapped and tested for arm-none-linux-gnueabihf. Further tested for > arm-none-eabi with a Cortex-M3 target. > > > gcc/ChangeLog: > 2016-06-30 Andre Vieira > Terry Guo > > * target.def (elf_flags_numeric): New target hook. > * targhooks.h (default_asm_elf_flags_numeric): New. > * varasm.c (default_asm_elf_flags_numeric): New. > (default_elf_asm_named_section): Use new target hook. > * config/arm/arm.opt (mpure-code): New. > * config/arm/arm.h (SECTION_ARM_PURECODE): New. > * config/arm/arm.c (arm_asm_init_sections): Add section > attribute to default text section if -mpure-code. > (arm_option_check_internal): Diagnose use of option with > non supported targets and/or options. > (arm_asm_elf_flags_numeric): New. > (arm_function_section): New. > (arm_elf_section_type_flags): New. > * config/arm/elf.h (JUMP_TABLES_IN_TEXT_SECTION): Disable > for -mpure-code. > * gcc/doc/texi (TARGET_ASM_ELF_FLAGS_NUMERIC): New. > * gcc/doc/texi.in (TARGET_ASM_ELF_FLAGS_NUMERIC): Likewise. > > > > gcc/testsuite/ChangeLog: > 2016-06-30 Andre Vieira > Terry Guo > > * gcc.target/arm/pure-code/ffunction-sections.c: New. > * gcc.target/arm/pure-code/no-literal-pool.c: New. > * gcc.target/arm/pure-code/pure-code.exp: New. > >>> Hi Sandra, Mickael, >>> >>> Thank you for your comments. I changed the description of -mpure-code in >>> invoke.texi to better reflect the error message you get wrt supported >>> targets. >>> >>> As for the target hook description, I hope the text is clearer now. Let >>> me know if you think it needs further explanation. >>> >>> I also fixed the double '%' in the text string for unnamed text sections >>> and disabled the casesi pattern. >>> >>> I duplicated the original casesi test >>> 'gcc/testsuite/gcc.c-torture/compile/pr46934.c' for pure-code to make >>> sure the casesi was disabled and other patterns were selected instead. >>> >>> Reran regressions for pure-code.exp for Cortex-M3. >>> >>> Cheers, >>> Andre >>> >>> >>> gcc/ChangeLog: >>> 2016-07-11 Andre Vieira >>> Terry Guo >>> >>> * target.def (elf_flags_numeric): New target hook. >>> * hooks.c (hook_uint_uintp_false): New generic hook. >>> * varasm.c (default_elf_asm_named_section): Use new target hook. >>> * config/arm/arm.opt (mpure-code): New. >>> * config/arm/arm.h (SECTION_ARM_PURECODE): New. >>> * config/arm/arm.c (arm_asm_init_sections): Add section >>> attribute to default text section if -mpure-code. >>> (arm_option_check_internal): Diagnose use of option with >>> non supported targets and/or options. >>> (arm_asm_elf_flags_numeric): New. >>> (arm_function_section): New. >>> (arm_elf_section_type_flags):
Re: [PING v3] Unreviewed GCC-6 patches
> Ok. > > Thanks, > Richard. Sorry for silly question, but does it mean that I have a green light to commit it to SVN or did you just acknowledge the fact and will take a look at this later? For future: I'm planning to commit it to GCC-5 also later. Do I have to send another patch or maybe it is good enough to commit to both GCC-6 and GCC-5 at the same time? Thanks for quick response, Kuba -- Jakub Sejdak Software Engineer Phoenix Systems (www.phoesys.com) +48 608 050 163
Re: [PATCH] Fix PR64078
On 29/08/16 18:43, Bernd Edlinger wrote:
Thanks!
Actually my patch missed to fix one combination: -m32 with -fpic
make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts
'-m32 -fpic'"
FAIL: c-c++-common/ubsan/object-size-9.c -O2 execution test
FAIL: c-c++-common/ubsan/object-size-9.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution test
The problem here is that the functions f2 and f3 access a stack-
based object out of bounds and that is inlined in main and
therefore smashes the return address of main in this case.
A possible fix could look like follows:
Index: object-size-9.c
===
--- object-size-9.c (revision 239794)
+++ object-size-9.c (working copy)
@@ -93,5 +93,9 @@
#endif
f4 (12);
f5 (12);
+#ifdef __cplusplus
+ /* Stack may be smashed by f2/f3 above. */
+ __builtin_exit (0);
+#endif
return 0;
}
Do you think that this should be fixed too?
I think it should be fixed. Ideally, we'd prevent the out-of-bounds
writes to have harmful effects, but I'm not sure how to enforce that.
This works for me:
...
diff --git a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
index 46f1fb9..fec920d 100644
--- a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
+++ b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
@@ -31,6 +31,7 @@ static struct C
f2 (int i)
{
struct C x;
+ struct C x2;
x.d[i] = 'z';
return x;
}
@@ -45,6 +46,7 @@ static struct C
f3 (int i)
{
struct C x;
+ struct C x2;
char *p = x.d;
p += i;
*p = 'z';
...
But I have no idea how stable this solution is.
Thanks,
- Tom
Re: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems
Bump From: gcc-patches-ow...@gcc.gnu.org on behalf of Tamar Christina Sent: Thursday, August 18, 2016 10:15:12 AM To: GCC Patches Cc: James Greenhalgh; Richard Earnshaw; Marcus Shawcroft; nd Subject: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems Hi all, This fixes a bug in the vector load functions in which they load the vector in the wrong order for big endian systems. This patch flips the order conditionally in the vec_concats. No testcase given because plenty of existing tests for vld functions. Ran regression tests on aarch64_be-none-elf and aarch64-none-elf. Vldx tests now pass on aarch64_be-none-elf and no regressions on both. Ok for trunk? I do not have commit rights so if ok can someone apply it for me? Thanks, Tamar gcc/ 2016-08-16 Tamar Christina * gcc/config/aarch64/aarch64-simd.md (aarch64_ld2_dreg_le): New. (aarch64_ld2_dreg_be): New. (aarch64_ld2_dreg): Removed. (aarch64_ld3_dreg_le): New. (aarch64_ld3_dreg_be): New. (aarch64_ld3_dreg): Removed. (aarch64_ld4_dreg_le): New. (aarch64_ld4_dreg_be): New. (aarch64_ld4_dreg): Removed. (aarch64_ld): Wrapper around _le, _be.
Re: [PATCH] Fix PR64078
On 29/08/16 18:43, Bernd Edlinger wrote: make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts '-m32 -fpic'" FAIL: c-c++-common/ubsan/object-size-9.c -O2 execution test FAIL: c-c++-common/ubsan/object-size-9.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test Filed PR77411 - object-size-9.c -fpic -m32 failure Thanks, - Tom
Re: Implement -Wimplicit-fallthrough (version 7)
On Mon, Aug 29, 2016 at 09:32:04AM -0400, Eric Gallager wrote: > I tried v6 on my binutils-gdb fork, and it printed A LOT of > warnings... BTW, why is that so? Does binutils-gdb not use various FALLTHRU comments? Marek
[RFC PATCH, alpha]: Disable _FloatN on alpha
Hello!
The _FloatN patch uncovers a problem on alpha OSF/1 ABI when _Float32
variable arguments are passed to a function. As seen from the source,
32bit _Float32 arguments (SFmode) are *not* promoted to 64bit DFmode
when passing to as variable arguments, and this uncovers following
limitation:
Values, passed in FP registers are pushed to a pretend-args area of
called va-arg function in their 64-bit full word mode, so they are
stored as their bit-exact copy from the reg to the memory using STT
instruction. However, when this memory is later accessed as SFmode
value using LDS instruction, this doesn't load SFmode value (as it was
passed in the FP register), since LDS reorders bits on the way from/to
memory. To make things worse, the value can be moved from pretend-args
area as a DImode value and stored as a SImode subreg of this value.
Float arguments, passed to va-arg function work OK, since default c
promoting rules always promote float variable arguments to double.
This is not the case with _Float32.
To illustrate the problem, consider following source:
--cut here--
volatile _Float32 a = 1.0f32, b = 2.5F32, c = -2.5f32;
_Float32
vafn (_Float32 arg1, ...)
{
__builtin_va_list ap;
_Float32 ret;
__builtin_va_start (ap, arg1);
ret = arg1 + __builtin_va_arg (ap, _Float32);
__builtin_va_end (ap);
return ret;
}
int
main (void)
{
volatile _Float32 r;
r = vafn (a, c);
if (r != -1.5f32)
__builtin_abort ();
return 0;
}
--cut here--
Please note the lack of promotion in __builtin_va_arg.
This is compiled to following assembly:
vafn:
...
stt $f17,24($30) <- store DFmode to pretend args area and ...
ldq $1,24($30) <- ... load it as DImode value
...
stl $1,16($30) <- store SImode subreg to stack and ...
lds $f10,16($30) <- ... load it as SFmode value
stq $1,40($30) <- (dead DImode store)
adds $f16,$f10,$f0 <- use SFmode value in $f10
...
main:
...
lda $1,a
lds $f16,0($1)
lda $1,c
lds $f17,0($1)
jsr $26,vafn
...
SFmode values are stored in the FP registers in canonical form, as
subset of DFmode values, with 11-bit exponents restricted to the
corresponding single-precision range, and with the 29 low-order
fraction bits restricted to be all zero. IOW, it is not possible to
load SFmode value from the location where the full 64bit value was
stored from the FP register.
I didn't find a way around this limitation, so I propose to disable
_FloatN on alpha with the attached patch.
Uros.
diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c
index 702cd27..1bf27ca 100644
--- a/gcc/config/alpha/alpha.c
+++ b/gcc/config/alpha/alpha.c
@@ -736,6 +736,20 @@ alpha_vector_mode_supported_p (machine_mode mode)
return mode == V8QImode || mode == V4HImode || mode == V2SImode;
}
+/* Target hook for floatn_mode. */
+
+static machine_mode
+alpha_floatn_mode (int n ATTRIBUTE_UNUSED, bool extended ATTRIBUTE_UNUSED)
+{
+ /* Alpha can't pass _Float32 variable arguments. The function that is
+ receiving a variable number of arguments pushes floating-point values
+ in the remaining incoming registers to the stack as DFmode 64-bit values.
+ These stores are not compatible with SFmode loads due to implicit
+ reordering of bits during SFmode load. Disable all _Floatn types. */
+
+ return VOIDmode;
+}
+
/* Return 1 if this function can directly return via $26. */
int
@@ -9978,8 +9992,6 @@ alpha_atomic_assign_expand_fenv (tree *hold, tree *clear,
tree *update)
#undef TARGET_PROMOTE_FUNCTION_MODE
#define TARGET_PROMOTE_FUNCTION_MODE
default_promote_function_mode_always_promote
-#undef TARGET_PROMOTE_PROTOTYPES
-#define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_false
#undef TARGET_FUNCTION_VALUE
#define TARGET_FUNCTION_VALUE alpha_function_value
@@ -10020,6 +10032,8 @@ alpha_atomic_assign_expand_fenv (tree *hold, tree
*clear, tree *update)
#define TARGET_SCALAR_MODE_SUPPORTED_P alpha_scalar_mode_supported_p
#undef TARGET_VECTOR_MODE_SUPPORTED_P
#define TARGET_VECTOR_MODE_SUPPORTED_P alpha_vector_mode_supported_p
+#undef TARGET_FLOATN_MODE
+#define TARGET_FLOATN_MODE alpha_floatn_mode
#undef TARGET_BUILD_BUILTIN_VA_LIST
#define TARGET_BUILD_BUILTIN_VA_LIST alpha_build_builtin_va_list
Re: [PING v3] Unreviewed GCC-6 patches
On Tue, 30 Aug 2016, Jakub Sejdak wrote: > > Ok. > > > > Thanks, > > Richard. > > Sorry for silly question, but does it mean that I have a green light > to commit it to SVN or did you just acknowledge the fact and will take > a look at this later? It was an approval for the backport. > For future: I'm planning to commit it to GCC-5 also later. Do I have > to send another patch or maybe it is good enough to commit to both > GCC-6 and GCC-5 at the same time? Go ahead for both branches. Richard. > Thanks for quick response, > Kuba > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Fix PR69047
On Mon, 29 Aug 2016, Andreas Schwab wrote:
> On Aug 26 2016, Richard Biener wrote:
>
> > Index: gcc/testsuite/gcc.dg/pr69047.c
> > ===
> > --- gcc/testsuite/gcc.dg/pr69047.c (revision 0)
> > +++ gcc/testsuite/gcc.dg/pr69047.c (working copy)
> > @@ -0,0 +1,18 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O -fdump-tree-cddce1" } */
> > +
> > +__UINT8_TYPE__
> > +f(__UINT16_TYPE__ b)
> > +{
> > + __UINT8_TYPE__ a;
> > +#if __BYTE_ORDER == __LITTLE_ENDIAN
> > + __builtin_memcpy(&a, &b, sizeof a);
> > +#elif __BYTE_ORDER == __BIG_ENDIAN
> > + __builtin_memcpy(&a, (char *)&b + sizeof a, sizeof a);
> > +#else
> > + a = b;
> > +#endif
> > + return a;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump "_\[0-9\]+ = \\(\[^)\]+\\) b" "cddce1" } }
> > */
> >
>
> On m68k:
>
> FAIL: gcc.dg/pr69047.c scan-tree-dump cddce1 "_[0-9]+ = \\([^)]+\\) b"
>
> $ cat pr69047.c.037t.cddce1
>
> ;; Function f (f, funcdef_no=0, decl_uid=1432, cgraph_uid=0, symbol_order=0)
>
> f (short unsigned int b)
> {
> unsigned char a;
> unsigned char _2;
>
> :
> _2 = BIT_FIELD_REF ;
> return _2;
>
> }
>
>
> Andreas.
Ah, forgot to re-write to use GCC internal macros.
Tested on m68k, applied.
Richard.
2016-08-30 Richard Biener
PR tree-optimization/69047
* gcc.dg/pr69047.c: Fix byte-order check.
Index: gcc/testsuite/gcc.dg/pr69047.c
===
--- gcc/testsuite/gcc.dg/pr69047.c (revision 239856)
+++ gcc/testsuite/gcc.dg/pr69047.c (working copy)
@@ -5,9 +5,9 @@ __UINT8_TYPE__
f(__UINT16_TYPE__ b)
{
__UINT8_TYPE__ a;
-#if __BYTE_ORDER == __LITTLE_ENDIAN
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
__builtin_memcpy(&a, &b, sizeof a);
-#elif __BYTE_ORDER == __BIG_ENDIAN
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
__builtin_memcpy(&a, (char *)&b + sizeof a, sizeof a);
#else
a = b;
Re: [PATCH] Fix abi_tag23* test failure (PR c++/77379)
On 29 August 2016 at 22:10, Jakub Jelinek wrote:
> On Mon, Aug 29, 2016 at 12:42:28PM -0400, Jason Merrill wrote:
>> Another missing ABI tag, sigh.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>
>> commit 1337a943a2d3926537b63d6e1f0d7f46ef10a06d
>> Author: Jason Merrill
>> Date: Fri Aug 26 15:12:52 2016 -0400
>>
>> PR c++/77379 - ABI tag on thunk
>>
>> * mangle.c (maybe_check_abi_tags): Add version parm, handle thunks.
>> (mangle_thunk): Add thunk parameter.
>> * method.c (finish_thunk): Pass it.
>> * cp-tree.h: Declare it.
>
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/abi/abi-tag23.C
>> +// { dg-final { scan-assembler "_ZThn16_N7Derived7get_fooB3barEv" } }
>
> This unfortunately fails e.g. on i686-linux, because the symbol is
It also fails on arm* targets.
> _ZThn8_N7Derived7get_fooB3barEv instead of
> _ZThn16_N7Derived7get_fooB3barEv
>
> The following patch accepts any negative offsets. Tested on x86_64-linux
> and i686-linux, ok for trunk?
>
> 2016-08-29 Jakub Jelinek
>
> PR c++/77379
> * g++.dg/abi/abi-tag23.C: Adjust scan-assembler regex for differing
> thunk offsets.
> * g++.dg/abi/abi-tag23a.C: Likewise.
>
> --- gcc/g++.dg/abi/abi-tag23.C.jj 2016-08-29 19:34:12.0 +0200
> +++ gcc/g++.dg/abi/abi-tag23.C 2016-08-29 22:04:16.328873328 +0200
> @@ -32,4 +32,4 @@ int main()
>Final().get_foo();
> }
>
> -// { dg-final { scan-assembler "_ZThn16_N7Derived7get_fooB3barEv" } }
> +// { dg-final { scan-assembler "_ZThn\[0-9]+_N7Derived7get_fooB3barEv" } }
> --- gcc/g++.dg/abi/abi-tag23a.C.jj 2016-08-29 19:34:12.0 +0200
> +++ gcc/g++.dg/abi/abi-tag23a.C 2016-08-29 22:04:55.053398520 +0200
> @@ -32,4 +32,4 @@ int main()
>Final().get_foo();
> }
>
> -// { dg-final { scan-assembler "_ZThn16_N7Derived7get_fooEv" } }
> +// { dg-final { scan-assembler "_ZThn\[0-9]+_N7Derived7get_fooEv" } }
>
>
> Jakub
Re: [PATCH] Fix PR64078
On 08/30/16 10:21, Tom de Vries wrote:
> On 29/08/16 18:43, Bernd Edlinger wrote:
>> Thanks!
>>
>> Actually my patch missed to fix one combination: -m32 with -fpic
>>
>> make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts
>> '-m32 -fpic'"
>>
>> FAIL: c-c++-common/ubsan/object-size-9.c -O2 execution test
>> FAIL: c-c++-common/ubsan/object-size-9.c -O2 -flto
>> -fno-use-linker-plugin -flto-partition=none execution test
>>
>> The problem here is that the functions f2 and f3 access a stack-
>> based object out of bounds and that is inlined in main and
>> therefore smashes the return address of main in this case.
>>
>> A possible fix could look like follows:
>>
>> Index: object-size-9.c
>> ===
>> --- object-size-9.c(revision 239794)
>> +++ object-size-9.c(working copy)
>> @@ -93,5 +93,9 @@
>> #endif
>> f4 (12);
>> f5 (12);
>> +#ifdef __cplusplus
>> + /* Stack may be smashed by f2/f3 above. */
>> + __builtin_exit (0);
>> +#endif
>> return 0;
>> }
>>
>>
>> Do you think that this should be fixed too?
>
> I think it should be fixed. Ideally, we'd prevent the out-of-bounds
> writes to have harmful effects, but I'm not sure how to enforce that.
>
> This works for me:
> ...
> diff --git a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
> b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
> index 46f1fb9..fec920d 100644
> --- a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
> +++ b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
> @@ -31,6 +31,7 @@ static struct C
> f2 (int i)
> {
> struct C x;
> + struct C x2;
> x.d[i] = 'z';
> return x;
> }
> @@ -45,6 +46,7 @@ static struct C
> f3 (int i)
> {
> struct C x;
> + struct C x2;
> char *p = x.d;
> p += i;
> *p = 'z';
> ...
>
> But I have no idea how stable this solution is.
>
At least x2 could not be opimized away, as it is no POD,
but there is no guarantee, that x2 comes after x on the stack.
Another possibility, which seems to work as well:
Index: gcc/testsuite/c-c++-common/ubsan/object-size-9.c
===
--- gcc/testsuite/c-c++-common/ubsan/object-size-9.c(revision 239794)
+++ gcc/testsuite/c-c++-common/ubsan/object-size-9.c(working copy)
@@ -30,7 +30,7 @@ f1 (struct T x, int i)
static struct C
f2 (int i)
{
- struct C x;
+ struct C x __attribute__ ((aligned(16)));
x.d[i] = 'z';
return x;
}
@@ -44,7 +44,7 @@ f2 (int i)
static struct C
f3 (int i)
{
- struct C x;
+ struct C x __attribute ((aligned(16)));
char *p = x.d;
p += i;
*p = 'z';
Thanks
Bernd.
Fix ICE in RTL PRE on abnormal edges
We ran into an ICE at -O3 on x86-64/Darwin for the attached testcase with our
GCC 6 compiler:
+===GNAT BUG DETECTED==+
| Pro 17.0w (20160830-62) (x86_64-apple-darwin14.5.0) GCC error: |
| in rtl_split_edge, at cfgrtl.c:1887 |
| Error detected around p.adb:87:7
It doesn't reproduce with mainline, but the issue is latent.
postreload-gcse.c:eliminate_partially_redundant_loads uses an ad-hoc predicate
to detect problematic patterns and consequently punt:
/* Don't try anything on basic blocks with strange predecessors. */
if (! bb_has_well_behaved_predecessors (bb))
continue;
This predicate has for abnormal edges:
if ((pred->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (pred))
return false;
which makes sense since rtl_split_edge aborts on abnormal edges (it's the ICE)
so any abnormal critical edge cannot be used to create a full redundancy by
PRE. Now PRE actually uses commit_edge_insertions to insert intructions on
edges and commit_one_edge_insertion has an additional restriction:
/* If the source has one successor and the edge is not abnormal,
insert there. Except for the entry block.
Don't do this if the predecessor ends in a jump other than
unconditional simple jump. E.g. for asm goto that points all
its labels at the fallthru basic block, we can't insert instructions
before the asm goto, as the asm goto can have various of side effects,
and can't emit instructions after the asm goto, as it must end
the basic block. */
else if ((e->flags & EDGE_ABNORMAL) == 0
&& single_succ_p (e->src)
That is to say, even an abnormal edge whose source has a single successor is
rejected here and leads to the tentative splitting and the ICE. So the patch
makes bb_has_well_behaved_predecessors match what commit_one_edge_insertion
supports for abnormal edges with an explicit reference in the comment.
Tested on x86_64-suse-linux, applied on the mainline.
2016-08-30 Eric Botcazou
* postreload-gcse.c (bb_has_well_behaved_predecessors): Tweak
criterion used for abnormal egdes.
2016-08-30 Eric Botcazou
* gnat.dg/opt57.ad[sb]: New test.
* gnat.dg/opt57_pkg.ads: New helper.
--
Eric Botcazoucommit 5a1da5d39381c87a5a680cc886ea2f6c0cadf306
Author: Eric Botcazou
Date: Tue Jul 26 00:05:15 2016 +0200
Fix for P725-013 (build failure of CodePeer on x86-64/Darwin with GCC 6).
diff --git a/gcc/postreload-gcse.c b/gcc/postreload-gcse.c
index 566e875..da04fb7 100644
--- a/gcc/postreload-gcse.c
+++ b/gcc/postreload-gcse.c
@@ -962,7 +962,9 @@ bb_has_well_behaved_predecessors (basic_block bb)
FOR_EACH_EDGE (pred, ei, bb->preds)
{
- if ((pred->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (pred))
+ /* commit_one_edge_insertion refuses to insert on abnormal edges even if
+ the source has only one successor so EDGE_CRITICAL_P is too weak. */
+ if ((pred->flags & EDGE_ABNORMAL) && !single_pred_p (pred->dest))
return false;
if ((pred->flags & EDGE_ABNORMAL_CALL) && cfun->has_nonlocal_label)
package body Opt57 is
type Phase_Enum is (None_Phase, FE_Init_Phase, FE_Phase);
type Message_State is (No_Messages, Some_Messages);
type Module_List_Array is array (Phase_Enum, Message_State) of List;
type Private_Module_Factory is limited record
Module_Lists : Module_List_Array;
end record;
type Element_Array is array (Positive range <>) of Module_Factory_Ptr;
type Hash_Table is array (Positive range <>) of aliased Module_Factory_Ptr;
type Heap_Data_Rec (Table_Last : Positive) is limited record
Number_Of_Elements : Positive;
Table : Hash_Table (1 .. Table_Last);
end record;
type Heap_Data_Ptr is access Heap_Data_Rec;
type Table is limited record
Data : Heap_Data_Ptr;
end record;
function All_Elements (M : Table) return Element_Array is
Result : Element_Array (1 .. Natural (M.Data.Number_Of_Elements));
Last : Natural := 0;
begin
for H in M.Data.Table'Range loop
Last := Last + 1;
Result (Last) := M.Data.Table(H);
end loop;
return Result;
end;
The_Factories : Table;
subtype Language_Array is Element_Array;
type Language_Array_Ptr is access Language_Array;
All_Languages : Language_Array_Ptr := null;
procedure Init is
begin
if All_Languages = null then
All_Languages := new Language_Array'(All_Elements (The_Factories));
end if;
end;
function Is_Empty (L : List) return Boolean is
begin
return Link_Constant (L.Next) = L'Unchecked_Access;
end;
function First (L : List) return Linkable_Ptr is
begin
return Links_Type (L.Next.all).C
Re: Strip NOP_EXPR before building MEM
On Sun, Aug 28, 2016 at 7:59 PM, Jan Hubicka wrote:
> Hi,
> I have noticed that ivopts build MEM_REF (NOP_EXPR (address))
> where NOP_EXPR converts one pointer type to another. Later it tries
> to expand it that on strict alignment targets punt on determining
> the alignment and winds expensive memory reference.
>
> It seems to make no sense to have NOP_EXPR here, but in addition
> I think we want to somehow preserve alignment from original MEM_REF?
As you noticed the type carries semantics thus if you want to fix it this
way then you need to make build_simple_mem_ref strip the nops _after_
remembering the type to use for the MEM_REF (and the aliasing).
After all you are changing *(int *)&large-struct to *&large-struct with
your patch - certainly not what you intended?
Sth like
@@ -4618,6 +4623,7 @@ build_simple_mem_ref_loc (location_t loc
HOST_WIDE_INT offset = 0;
tree ptype = TREE_TYPE (ptr);
tree tem;
+ STRIP_NOPS (ptr);
/* For convenience allow addresses that collapse to a simple base
and offset. */
if (TREE_CODE (ptr) == ADDR_EXPR
Richard.
> Honza
>
> * tree-ssa-lop-ivopts.c (get_computation_cost_at):
> strip nops before building mem ref
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 62ba71b..bd92031 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -5058,7 +5058,10 @@ fallback:
> return infinite_cost;
>
>if (address_p)
> -comp = build_simple_mem_ref (comp);
> +{
> + STRIP_NOPS (comp);
> + comp = build_simple_mem_ref (comp);
> +}
>
>cost = comp_cost (computation_cost (comp, speed), 0);
>
Re: [v3 PATCH] PR libstdc++/77395
diff --git a/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
b/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
index 1525fef..7543397 100644
--- a/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
+++ b/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
@@ -52,7 +52,7 @@ std::pair v0{1,2};
std::pair v1{1,2};
-std::pair v2 = {1,2}; // { dg-error "explicit" }
+std::pair v2 = {1,2}; // { dg-error "explicit" }
std::pair v3{std::pair{1,2}};
This, and ...
diff --git
a/libstdc++-v3/testsuite/28_regex/iterators/regex_token_iterator/ctors/char/dr2332_neg.cc
b/libstdc++-v3/testsuite/28_regex/iterators/regex_token_iterator/ctors/char/dr2332_neg.cc
index b15f83e..b86c0bf 100644
---
a/libstdc++-v3/testsuite/28_regex/iterators/regex_token_iterator/ctors/char/dr2332_neg.cc
+++
b/libstdc++-v3/testsuite/28_regex/iterators/regex_token_iterator/ctors/char/dr2332_neg.cc
@@ -34,5 +34,5 @@ test01()
iter_type(s, s, std::regex{}, il);// { dg-error "deleted" }
const int i[2] = { };
- iter_type(s, s, std::regex{}, i);// { dg-error "deleted" }
+ iter_type(s, s, std::regex{}, i); // { dg-error "deleted" }
}
... this are not necessary.
A clean build will get -fno-show-column in scripts/testsuite_flags
which makes such voodoo unnecessary.
OK without those two whitespace changes.
Re: Ping : [Patch, fortran] PR48298 - [F03] User-Defined Derived-Type IO (DTIO)
Dear All, Janne's proposed change to namelist transfer has been implemented. This avoids ABI brekage. Please find the ChangeLogs below and the new patch attached. Bootstraps and regtests on FC21/x86_64. I will commit tomorrow morning if there are no objections in the meantime. Best regards Paul 2016-08-23 Paul Thomas Jerry DeLisle PR fortran/48298 * decl.c (access_attr_decl): Include case INTERFACE_DTIO as appropriate. * gfortran.h : Add INTRINSIC_FORMATTED and INTRINSIC_UNFORMATTED to gfc_intrinsic_op. Add INTERFACE_DTIO to interface type. Add new enum 'dtio_codes'. Add bitfield 'has_dtio_procs' to symbol_attr. Add prototypes 'gfc_check_dtio_interfaces' and 'gfc_find_specific_dtio_proc'. * interface.c (dtio_op): New function. (gfc_match_generic_spec): Match generic DTIO interfaces. (gfc_match_interface): Treat DTIO interfaces in the same way as (gfc_current_interface_head): Add INTERFACE_DTIO appropriately. (check_dtio_arg_TKR_intent): New function. (check_dtio_interface1): New function. (gfc_check_dtio_interfaces): New function. (gfc_find_specific_dtio_proc): New function. * io.c : Add FMT_DT to format_token. (format_lex): Handle DTIO formatting. * match.c (gfc_op2string): Add DTIO operators. * resolve.c (derived_inaccessible): Ignore pointer components to enclosing derived type. (resolve_transfer): Resolve transfers that involve DTIO. procedures. Find the specific subroutine for the transfer and use its existence to over-ride some of the constraints on derived types. If the transfer is recursive, require that the subroutine be so qualified. (dtio_procs_present): New function. (resolve_fl_namelist): Remove inhibition of polymorphic objects in namelists if DTIO read and write subroutines exist. Likewise for derived types. (resolve_types): Invoke 'gfc_verify_dtio_procedures'. * symbol.c : Set 'dtio_procs' using 'minit'. * trans-decl.c (gfc_finish_var_decl): If a derived-type/class object is associated with DTIO procedures, make it TREE_STATIC. * trans-expr.c (gfc_get_vptr_from_expr): If the expression drills down to a PARM_DECL, extract the vptr correctly. (gfc_conv_derived_to_class): Check 'info' in the test for 'useflags'. If the se expression exists and is a pointer, use it as the class _data. * trans-io.c : Add IOCALL_X_DERIVED to iocall and the function prototype. Likewise for IOCALL_SET_NML_DTIO_VAL. (set_parameter_tree): Renamed from 'set_parameter_const', now returns void and has new tree argument. Calls modified to match new interface. (transfer_namelist_element): Transfer DTIO procedure pointer and vpointer using the new function IOCALL_SET_NML_DTIO_VAL. (get_dtio_proc): New function. (transfer_expr): Add new argument for the vptr field of class objects. Add the code to call the specific DTIO proc, convert derived types to class and call IOCALL_X_DERIVED. (trans_transfer): Add BT_CLASS to structures for treatment by the scalarizer. Obtain the vptr for the dynamic type, both for scalar and array transfer. 2016-08-23 Jerry DeLisle Paul Thomas PR libgfortran/48298 * gfortran.map : Flag _st_set_nml_dtio_var and _gfortran_transfer_derived. * io/format.c (format_lex): Detect DTIO formatting. (parse_format_list): Parse the DTIO format. (next_format): Include FMT_DT. * io/format.h : Likewise. Add structure 'udf' to structure 'fnode' to carry the IOTYPE string and the 'vlist'. * io/io.h : Add prototypes for the two types of DTIO subroutine and a typedef for gfc_class. Also, add to 'namelist_type' fields for the pointer to the DTIO procedure and the vtable. Add fields to struct st_parameter_dt for pointers to the two types of DTIO subroutine. Add to gfc_unit DTIO specific fields. (internal_proto): Add prototype for 'read_user_defined' and 'write_user_defined'. * io/list_read.c (check_buffers): Use the 'current_unit' field. (unget_char): Likewise. (eat_spaces): Likewise. (list_formatted_read_scalar): For case BT_CLASS, call the DTIO procedure. (nml_get_obj_data): Likewise when DTIO procedure is present,. * io/transfer.c : Export prototypes for 'transfer_derived' and 'transfer_derived_write'. (unformatted_read): For case BT_CLASS, call the DTIO procedure. (unformatted_write): Likewise. (formatted_transfer_scalar_read): Likewise. (formatted_transfer_scalar_write: Likewise. (transfer_derived): New function. (data_transfer_init): Set last_char if no child_dtio. (finalize_transfer): Return if child_dtio set. (st_write_done): Add condition for child_dtio not set. Add extra arguments for st_set_nml_var prototype. (set_nml_var): New function that contains the contents of the old version of st_set_nml_var. Also sets the 'dtio_sub' and 'vtable
[PATCH] rs6000: Don't emit a use of LR in returns and sibcalls
The exit block (to which every return artificially jumps) already has
a use of LR. The LR use in all returns and sibcalls is an anachronism,
probably made unnecessary by the dataflow merge. The simple_returns
that shrink-wrapping generates also do not have such a use. Newer
backends do not do this either it seems.
With this use removed, a normal return is no longer a parallel but just
a return insn, and cfgcleanup then can transform conditional jumps to
those into conditional returns.
This splits the return emission code with restoring_FPRs_inline from
that without it; this is simpler code, fewer lines, and less indentation.
The return_internal_ pattern can now be deleted since nothing uses
it anymore.
Tested on powerpc64-linux (-m32,-m64); will test on powerpc64le-linux
as well. David and Iain, can you please test on AIX and Darwin?
Or is this okay for trunk without testing? ;-)
Segher
2016-08-30 Segher Boessenkool
* config/rs6000/rs6000.c (rs6000_emit_epilogue): Do not emit
USEs of LR_REGNO in returns and sibcalls.
(rs6000_output_mi_thunk): Similar.
(rs6000_sibcall_aix): Similar.
* config/rs6000/rs6000.md (sibcall, sibcall_value, sibcall_local32,
sibcall_local64, sibcall_value_local32, sibcall_value_local64,
sibcall_nonlocal_sysv, sibcall_value_nonlocal_sysv):
Remove the USE of LR_REGNO from the patterns as well. Delete an
obsolete comment.
(return_internal_): Delete.
---
gcc/config/rs6000/rs6000.c | 122 ++--
gcc/config/rs6000/rs6000.md | 19 ---
2 files changed, 51 insertions(+), 90 deletions(-)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 4de70ea..2f15a05 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -28277,7 +28277,6 @@ rs6000_emit_epilogue (int sibcall)
longer necessary. */
p = rtvec_alloc (9
- + 1
+ 32 - info->first_gp_reg_save
+ LAST_ALTIVEC_REGNO + 1 - info->first_altivec_reg_save
+ 63 + 1 - info->first_fp_reg_save);
@@ -28288,9 +28287,6 @@ rs6000_emit_epilogue (int sibcall)
j = 0;
RTVEC_ELT (p, j++) = ret_rtx;
- RTVEC_ELT (p, j++) = gen_rtx_USE (VOIDmode,
- gen_rtx_REG (Pmode,
-LR_REGNO));
RTVEC_ELT (p, j++)
= gen_rtx_USE (VOIDmode, gen_rtx_SYMBOL_REF (Pmode, alloc_rname));
/* The instruction pattern requires a clobber here;
@@ -29013,73 +29009,63 @@ rs6000_emit_epilogue (int sibcall)
emit_insn (gen_add3_insn (sp_reg_rtx, sp_reg_rtx, sa));
}
- if (!sibcall)
+ if (!sibcall && restoring_FPRs_inline)
+{
+ if (cfa_restores)
+ {
+ /* We can't hang the cfa_restores off a simple return,
+since the shrink-wrap code sometimes uses an existing
+return. This means there might be a path from
+pre-prologue code to this return, and dwarf2cfi code
+wants the eh_frame unwinder state to be the same on
+all paths to any point. So we need to emit the
+cfa_restores before the return. For -m64 we really
+don't need epilogue cfa_restores at all, except for
+this irritating dwarf2cfi with shrink-wrap
+requirement; The stack red-zone means eh_frame info
+from the prologue telling the unwinder to restore
+from the stack is perfectly good right to the end of
+the function. */
+ emit_insn (gen_blockage ());
+ emit_cfa_restores (cfa_restores);
+ cfa_restores = NULL_RTX;
+ }
+
+ emit_jump_insn (targetm.gen_simple_return ());
+}
+
+ if (!sibcall && !restoring_FPRs_inline)
{
- rtvec p;
bool lr = (strategy & REST_NOINLINE_FPRS_DOESNT_RESTORE_LR) == 0;
- if (! restoring_FPRs_inline)
- {
- p = rtvec_alloc (4 + 64 - info->first_fp_reg_save);
- RTVEC_ELT (p, 0) = ret_rtx;
- }
- else
- {
- if (cfa_restores)
- {
- /* We can't hang the cfa_restores off a simple return,
-since the shrink-wrap code sometimes uses an existing
-return. This means there might be a path from
-pre-prologue code to this return, and dwarf2cfi code
-wants the eh_frame unwinder state to be the same on
-all paths to any point. So we need to emit the
-cfa_restores before the return. For -m64 we really
-don't need epilogue cfa_restores at all, except for
-this irritating dwarf2cfi with shrink-wrap
-requirement; The stack red-zone means eh_frame info
-from the prologue telling the unwinder to restore
-from the stack is perfectly good
[PATCH, libstdc++]: Fix testsuite FAILs related to missing de_DE.UTF-8 locale
Hello!
There are several libstdc++ testsuite FAILs on systems where
de_DE.UTF-8 locale is missing.
Apparently, following "dg-do run" directive overrides
dg-require-namedlocale requirement.
Attached patch fixes testsuite failures by moving
dg-require-namedlocale directive after dg-do run.
2016-08-30 Uros Bizjak
* testsuite/22_locale/time_get/get/char/2.cc: Move dg-do run
directive above dg-require-namedlocale directive.
* testsuite/22_locale/time_get/get/wchar_t/2.cc: Ditto.
* testsuite/27_io/manipulators/extended/get_time/char/2.cc: Ditto.
* testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc: Ditto.
* testsuite/27_io/manipulators/extended/put_time/char/2.cc: Ditto.
* testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc: Ditto.
Tested on x86_64-linux-gnu {,-m32} CentOS 5.11.
OK for mainline?
Uros.
diff --git a/libstdc++-v3/testsuite/22_locale/time_get/get/char/2.cc
b/libstdc++-v3/testsuite/22_locale/time_get/get/char/2.cc
index 35ec627..a6eed49 100644
--- a/libstdc++-v3/testsuite/22_locale/time_get/get/char/2.cc
+++ b/libstdc++-v3/testsuite/22_locale/time_get/get/char/2.cc
@@ -1,5 +1,5 @@
+// { dg-do-run { target c++11 } }
// { dg-require-namedlocale "de_DE.UTF-8" }
-// { dg-do run { target c++11 } }
// 2014-04-14 Rüdiger Sonderfeld
diff --git a/libstdc++-v3/testsuite/22_locale/time_get/get/wchar_t/2.cc
b/libstdc++-v3/testsuite/22_locale/time_get/get/wchar_t/2.cc
index 63b70d8..48ddb39 100644
--- a/libstdc++-v3/testsuite/22_locale/time_get/get/wchar_t/2.cc
+++ b/libstdc++-v3/testsuite/22_locale/time_get/get/wchar_t/2.cc
@@ -1,5 +1,5 @@
-// { dg-require-namedlocale "de_DE.UTF-8" }
// { dg-do run { target c++11 } }
+// { dg-require-namedlocale "de_DE.UTF-8" }
// 2014-04-14 Rüdiger Sonderfeld
diff --git
a/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/char/2.cc
b/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/char/2.cc
index 3ba6017..42168f2 100644
--- a/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/char/2.cc
+++ b/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/char/2.cc
@@ -1,5 +1,5 @@
-// { dg-require-namedlocale "de_DE.UTF-8" }
// { dg-do run { target c++11 } }
+// { dg-require-namedlocale "de_DE.UTF-8" }
// 2014-04-14 Rüdiger Sonderfeld
diff --git
a/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc
b/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc
index 4adba53..a465650 100644
--- a/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc
+++ b/libstdc++-v3/testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc
@@ -1,5 +1,5 @@
-// { dg-require-namedlocale "de_DE.UTF-8" }
// { dg-do run { target c++11 } }
+// { dg-require-namedlocale "de_DE.UTF-8" }
// 2014-04-14 Rüdiger Sonderfeld
diff --git
a/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/char/2.cc
b/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/char/2.cc
index 3a1d2c4..ac74472 100644
--- a/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/char/2.cc
+++ b/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/char/2.cc
@@ -1,5 +1,5 @@
-// { dg-require-namedlocale "de_DE.UTF-8" }
// { dg-do run { target c++11 } }
+// { dg-require-namedlocale "de_DE.UTF-8" }
// 2014-04-14 Rüdiger Sonderfeld
diff --git
a/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc
b/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc
index c0d5869..4c32b067 100644
--- a/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc
+++ b/libstdc++-v3/testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc
@@ -1,5 +1,5 @@
-// { dg-require-namedlocale "de_DE.UTF-8" }
// { dg-do run { target c++11 } }
+// { dg-require-namedlocale "de_DE.UTF-8" }
// 2014-04-14 Rüdiger Sonderfeld
Re: [PATCH, libstdc++]: Fix testsuite FAILs related to missing de_DE.UTF-8 locale
Hi Uros,
> There are several libstdc++ testsuite FAILs on systems where
> de_DE.UTF-8 locale is missing.
>
> Apparently, following "dg-do run" directive overrides
> dg-require-namedlocale requirement.
>
> Attached patch fixes testsuite failures by moving
> dg-require-namedlocale directive after dg-do run.
>
> 2016-08-30 Uros Bizjak
>
> * testsuite/22_locale/time_get/get/char/2.cc: Move dg-do run
> directive above dg-require-namedlocale directive.
> * testsuite/22_locale/time_get/get/wchar_t/2.cc: Ditto.
> * testsuite/27_io/manipulators/extended/get_time/char/2.cc: Ditto.
> * testsuite/27_io/manipulators/extended/get_time/wchar_t/2.cc: Ditto.
> * testsuite/27_io/manipulators/extended/put_time/char/2.cc: Ditto.
> * testsuite/27_io/manipulators/extended/put_time/wchar_t/2.cc: Ditto.
>
> Tested on x86_64-linux-gnu {,-m32} CentOS 5.11.
>
> OK for mainline?
ok, thanks for fixing this.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Implement -Wimplicit-fallthrough (version 7)
On 8/30/16, Marek Polacek wrote: > On Mon, Aug 29, 2016 at 09:32:04AM -0400, Eric Gallager wrote: >> I tried v6 on my binutils-gdb fork, and it printed A LOT of >> warnings... > > BTW, why is that so? Does binutils-gdb not use various FALLTHRU comments? > > Marek > There are a lot of comments mentioning fallthroughs, but they mostly need to be rewritten to be recognized properly. There's just so many different ways of writing them. For example, one I've seen a lot writes it as "Drop through" instead of "Fall through" or something. Other examples of comments mentioning fallthroughs: /* else fall through */ /* else fallthrough to: */ /* FALL THRU into number case. */ /* ObjC NSString constant: fall through and parse like STRING. */ /* Fall through, pretend it is global. */ /* Fall through into normal member function. */ /* fall in for static symbols that do NOT start with '.' */ /* >>> !! else fall through !! <<< */ /* ... fall through for unsigned ints ... */ /* fall thru to manual case */ Also, at second glance, it's actually not quite as many warnings as I thought it was at first, it mostly just looked that way since each -Wimplicit-fallthrough warning takes up 12 lines due to the double-fixits. FWIW, Aldy's -Walloca-larger-than patch actually was noisier in the GDB portion at least. (The Binutils portion was already clean on its alloca usage since it already used the -Wstack-usage warning.) Oh, and one other thing I discovered: __attribute__((fallthrough)) triggers -Wdeclaration-after-statement: In file included from ./defs.h:124:0, from ./cli/cli-setshow.c:20: ./cli/cli-setshow.c: In function ‘do_setshow_command’: ./../include/ansidecl.h:505:33: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] # define ATTRIBUTE_FALLTHROUGH __attribute__((fallthrough)) ^ ./cli/cli-setshow.c:359:4: note: in expansion of macro ‘ATTRIBUTE_FALLTHROUGH’ ATTRIBUTE_FALLTHROUGH; ^ Which doesn't really make sense to me.
Re: C++ PATCH for c++/57728 (explicit instantiation and defaulted functions)
Hi Jason,
> The testcase in 57728 demonstrates that we have been treating
> functions explicitly defaulted in the class body inconsistently with
> explicit instantiation: an extern instantiation causes them not to be
> generated, but a normal explicit instantiation doesn't cause them to
> be emitted, leading to link errors.
>
> After discussing this issue with other vendors (who had the same bug),
> we have decided to treat these functions like implicitly declared
> members, so explicit instantiation consistently doesn't affect them.
>
> The second patch addresses a FIXME I noticed while looking at this:
> there's no reason for us to be calling a trivial default constructor
> in the first place.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
[...]
> diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> b/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> new file mode 100644
> index 000..5c14c01
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> @@ -0,0 +1,17 @@
> +// PR c++/57728
> +// { dg-do link { target c++11 } }
[...]
> diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> b/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> index 5c14c01..912d507 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/explicit12.C
> @@ -15,3 +15,5 @@ int main()
> {
>A a;
> }
> +
> +// { dg-final { scan-assembler-not "_ZN1AIiEC1Ev" } }
>
This test currently shows up as UNRESOLVED, and g++.log has
g++.dg/cpp0x/explicit12.C -std=c++11 : output file does not exist
Was this meant as a compile instead of link test, like the companion
explicit11.C?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: PR35503 - warn for restrict pointer
On 30 August 2016 at 05:34, David Malcolm wrote:
> On Mon, 2016-08-29 at 20:01 -0400, David Malcolm wrote:
>> On Mon, 2016-08-29 at 19:55 -0400, David Malcolm wrote:
>> [...]
>> > Assuming you have the location_t values available, you can create a
>> > rich_location for the primary range, and then add secondary ranges
>> > like
>> > this:
>> >
>> > rich_location richloc (loc_of_arg1);
>>
>> Oops, the above should be:
>>
>> rich_location richloc (line_table, loc_of_arg1);
>>
>> or:
>>
>> gcc_rich_location (loc_of_arg1);
> and this should be:
>
> gcc_rich_location richloc (loc_of_arg1);
>> which does the same thing (#include "gcc-rich-location.h").
>
> Clearly I need to sleep :)
Hi David,
Thanks for the suggestions. I can now see multiple source ranges for
pr35503-2.c (included in patch).
Output shows: http://pastebin.com/FNAVDU8A
(Posted pastebin link to avoid mangling by the mailer)
However the test for underline fails:
FAIL: c-c++-common/pr35503-2.c -Wc++-compat expected multiline
pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
&alpha\);.*\n \^~ ~~ ~~ .*\n"
I have attached gcc.log for the test-case. Presumably I have written
the test-case incorrectly.
Could you please have a look at it ?
Thanks,
Prathamesh
>
>> > richloc.add_range (loc_of_arg3, false); /* false here = don't
>> > draw
>> > a
>> > caret, just the underline */
>> > richloc.add_range (loc_of_arg4, false);
>> > warning_at_rich_loc (&richloc, OPT_Wrestrict, etc...
>> >
>> > See line-map.h for more information on rich_location.
>>
>> [...]
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..4239cef 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "substring-locations.h"
#include "spellcheck.h"
+#include "gcc-rich-location.h"
cpp_reader *parse_in; /* Declared in c-pragma.h. */
@@ -13057,4 +13058,86 @@ diagnose_mismatched_attributes (tree olddecl, tree
newdecl)
return warned;
}
+/* Warn if an argument at position param_pos is passed to a
+ restrict-qualified param, and it aliases with another argument. */
+
+void
+warn_for_restrict (unsigned param_pos, vec *args)
+{
+ tree arg = (*args)[param_pos];
+ if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node, 0))
+return;
+
+ location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
+ gcc_rich_location richloc (loc);
+
+ unsigned i;
+ tree current_arg;
+ auto_vec arg_positions;
+
+ FOR_EACH_VEC_ELT (*args, i, current_arg)
+{
+ if (i == param_pos)
+ continue;
+
+ tree current_arg = (*args)[i];
+ if (operand_equal_p (arg, current_arg, 0))
+ {
+ TREE_VISITED (current_arg) = 1;
+ arg_positions.safe_push (i);
+ }
+}
+
+ if (arg_positions.is_empty ())
+return;
+
+ struct obstack fmt_obstack;
+ gcc_obstack_init (&fmt_obstack);
+ char *fmt = (char *) obstack_alloc (&fmt_obstack, 0);
+
+ char num[32];
+ sprintf (num, "%u", param_pos + 1);
+
+ obstack_grow (&fmt_obstack, "passing argument ",
+ strlen ("passing argument "));
+ obstack_grow (&fmt_obstack, num, strlen (num));
+ obstack_grow (&fmt_obstack,
+ " to restrict-qualified parameter aliases with argument",
+ strlen (" to restrict-qualified parameter "
+ "aliases with argument"));
+
+ /* make argument plural and append space. */
+ if (arg_positions.length () > 1)
+obstack_1grow (&fmt_obstack, 's');
+ obstack_1grow (&fmt_obstack, ' ');
+
+ unsigned pos;
+ unsigned ranges_added = 1;
+
+ FOR_EACH_VEC_ELT (arg_positions, i, pos)
+{
+ /* FIXME: Fow now, only 3 ranges can be added. Remove this once
+the restriction is lifted. */
+ if (ranges_added < 3)
+ {
+ tree arg = (*args)[pos];
+ if (EXPR_HAS_LOCATION (arg))
+ {
+ richloc.add_range (EXPR_LOCATION (arg), false);
+ ranges_added++;
+ }
+ }
+
+ sprintf (num, "%u", pos + 1);
+ obstack_grow (&fmt_obstack, num, strlen (num));
+
+ if (i < arg_positions.length () - 1)
+ obstack_grow (&fmt_obstack, ", ", strlen (", "));
+}
+
+ obstack_1grow (&fmt_obstack, 0);
+ warning_at_rich_loc (&richloc, OPT_Wrestrict, fmt);
+ obstack_free (&fmt_obstack, fmt);
+}
+
#include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index bc22baa..cdb762e 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -920,6 +920,7 @@ extern void c_parse_final_cleanups (void);
extern void warn_for_omitted_condop (location_t, tree);
extern void warn_for_memset (location_t, tree, tree, int);
+extern void warn_for_restrict (unsigned, vec *);
/* These macros provide convenient access to the various _STMT nodes. */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
in
Re: PR35503 - warn for restrict pointer (-Wrestrict)
On 8/29/16, Jason Merrill wrote: > On Mon, Aug 29, 2016 at 10:28 AM, Marek Polacek wrote: >> On Mon, Aug 29, 2016 at 09:20:53AM -0400, Eric Gallager wrote: >>> I tried this patch on my fork of gdb-binutils and got a few warnings >>> from it. Would it be possible to have the caret point to the argument >>> mentioned, instead of the function name? And also print the option >>> name? E.g., instead of the current: >>> >>> or32-opc.c: In function ‘or32_print_register’: >>> or32-opc.c:956:3: warning: passing argument 1 to restrict qualified >>> parameter aliases with argument 3 >>>sprintf (disassembled, "%sr%d", disassembled, regnum); >>>^~~ >>> >>> could it look like: >>> >>> or32-opc.c: In function ‘or32_print_register’: >>> or32-opc.c:956:3: warning: passing argument 1 to restrict qualified >>> parameter aliases with argument 3 [-Wrestrict] >>>sprintf (disassembled, "%sr%d", disassembled, regnum); >>> ^~~~ >>> >>> instead? >> >> I didn't try to implement it, but I think this should be fairly easy to >> achieve in the C FE, because c_parser_postfix_expression_after_primary >> has arg_loc, which is a vector of parameter locations. > > The C++ FE doesn't have this currently, but it could be added without > too much trouble: in cp_parser_parenthesized_expression_list, extract > the locations from the cp_expr return value of > cp_parser_assignment_expression, and then pass the locations back up > to cp_parser_postfix_expression. > > Jason > On the topic of how to get this warning working with various frontends, is there any reason why the Objective C frontend doesn't handle -Wrestrict? Currently when trying to use it, it just says: cc1obj: warning: command line option '-Wrestrict' is valid for C/C++ but not for ObjC
Re: [PATCH, libstdc++]: Fix testsuite FAILs related to missing de_DE.UTF-8 locale
On 30/08/16 13:19 +0200, Uros Bizjak wrote:
Hello!
There are several libstdc++ testsuite FAILs on systems where
de_DE.UTF-8 locale is missing.
Apparently, following "dg-do run" directive overrides
dg-require-namedlocale requirement.
Yes, a dg-do with a target selector will decide whether to run the
test or not based on the target, and ignore any previous dg-require-*
This is documented at
https://gcc.gnu.org/onlinedocs/gccint/Directives.html and I noticed
some incorrect tests while adding the { target c++11 } selectors but
didn't get around to fixing them.
Thanks for fixing these.
Re: PR35503 - warn for restrict pointer (-Wrestrict)
On 30 August 2016 at 17:11, Eric Gallager wrote: > On 8/29/16, Jason Merrill wrote: >> On Mon, Aug 29, 2016 at 10:28 AM, Marek Polacek wrote: >>> On Mon, Aug 29, 2016 at 09:20:53AM -0400, Eric Gallager wrote: I tried this patch on my fork of gdb-binutils and got a few warnings from it. Would it be possible to have the caret point to the argument mentioned, instead of the function name? And also print the option name? E.g., instead of the current: or32-opc.c: In function ‘or32_print_register’: or32-opc.c:956:3: warning: passing argument 1 to restrict qualified parameter aliases with argument 3 sprintf (disassembled, "%sr%d", disassembled, regnum); ^~~ could it look like: or32-opc.c: In function ‘or32_print_register’: or32-opc.c:956:3: warning: passing argument 1 to restrict qualified parameter aliases with argument 3 [-Wrestrict] sprintf (disassembled, "%sr%d", disassembled, regnum); ^~~~ instead? >>> >>> I didn't try to implement it, but I think this should be fairly easy to >>> achieve in the C FE, because c_parser_postfix_expression_after_primary >>> has arg_loc, which is a vector of parameter locations. >> >> The C++ FE doesn't have this currently, but it could be added without >> too much trouble: in cp_parser_parenthesized_expression_list, extract >> the locations from the cp_expr return value of >> cp_parser_assignment_expression, and then pass the locations back up >> to cp_parser_postfix_expression. >> >> Jason >> > > > On the topic of how to get this warning working with various > frontends, is there any reason why the Objective C frontend doesn't > handle -Wrestrict? Currently when trying to use it, it just says: > > cc1obj: warning: command line option '-Wrestrict' is valid for C/C++ > but not for ObjC Hi Eric, I am not sure if restrict is valid for ObjC/Obj-C++ and hence didn't add the option for these front-ends. If it is valid, I will enable the option for ObjC and Obj-C++. Thanks, Prathamesh
Re: [RFC PATCH, alpha]: Disable _FloatN on alpha
On Tue, 30 Aug 2016, Uros Bizjak wrote: > I didn't find a way around this limitation, so I propose to disable > _FloatN on alpha with the attached patch. If your ABI requires binary32 values to be promoted in variable arguments, I'd think you could do that with a new target hook that specifies a type to promote to in variable arguments (when such promotion otherwise would not occur), affecting code generation for both function calls and va_arg (but not for unprototyped function calls, only for variable arguments). (TS 18661 allows conversion to the same type to act as a convertFormat operation, so it's allowed for such argument passing to convert sNaNs to qNaNs, which would be the effect of having such conversions to and from binary64 in the call path.) (Ideally of course such a hook would take effect *after* the front end, but various existing such hooks operate in the front end so I don't think it's a problem for a new hook to do so as well.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v2 0/9] Separate shrink-wrapping
Hi,
On Fri, 26 Aug 2016, Bernd Schmidt wrote:
> And that comment puzzles me. Surely prologue and epilogue are executed only
> once currently, so how does frequency come into it? Again - please provide an
> example.
int some_global;
int foo (void) {
if (!some_global) {
call_this();
call_that();
x = some + large / computation;
while (x--) { much_more_code(); }
some_global = 1;
}
return some_global;
}
Now you need the full pro/epilogue (saving/restoring all call clobbered
regs presumably used by the large computation and loop) for only one call
of that function (the first), and then never again. But you do need a
part of it always assuming the architecture is right, and this is a shared
library, namely the setup of the PIC pointer for the access to
some_global.
A prologue does many things, and in some cases many of them aren't
necessary for all calls (indeed that's often the case for functions
containing early-outs), so much so that the full prologue including
unnecessary parts needs more time than the functional body of a functions
particular call. It's obvious that it would be better to move those parts
of the prologue to places where they actually are needed:
int f2 (void) {
setup_pic_reg();
if (!some_global) {
save_call_clobbers();
code_from_above();
restore_call_clobbers();
}
retreg = some_global;
restore_pic_reg();
}
That includes moving parts of the prologue into loops:
int g() {
int sum = 0;
for (int i = 0; i < NUM; i++) {
sum += i;
if (sum >= CUTOFF) {
some_long_winded_expression();
that_eventually_calls_abort();
}
}
return sum;
}
Here all parts of the prologue that somehow make it possible to call other
functions are necessary only when the program will abort eventually: hence
is necessary only at one call of g() at most. Again it's sensible to move
those parts inside the unlikely condition, even though it's inside a loop.
Ciao,
Michael.
Re: [PATCH] rs6000: Don't emit a use of LR in returns and sibcalls
On Tue, Aug 30, 2016 at 7:17 AM, Segher Boessenkool wrote: > The exit block (to which every return artificially jumps) already has > a use of LR. The LR use in all returns and sibcalls is an anachronism, > probably made unnecessary by the dataflow merge. The simple_returns > that shrink-wrapping generates also do not have such a use. Newer > backends do not do this either it seems. > > With this use removed, a normal return is no longer a parallel but just > a return insn, and cfgcleanup then can transform conditional jumps to > those into conditional returns. > > This splits the return emission code with restoring_FPRs_inline from > that without it; this is simpler code, fewer lines, and less indentation. > > The return_internal_ pattern can now be deleted since nothing uses > it anymore. > > Tested on powerpc64-linux (-m32,-m64); will test on powerpc64le-linux > as well. David and Iain, can you please test on AIX and Darwin? > > Or is this okay for trunk without testing? ;-) > > > Segher > > > 2016-08-30 Segher Boessenkool > > * config/rs6000/rs6000.c (rs6000_emit_epilogue): Do not emit > USEs of LR_REGNO in returns and sibcalls. > (rs6000_output_mi_thunk): Similar. > (rs6000_sibcall_aix): Similar. > * config/rs6000/rs6000.md (sibcall, sibcall_value, sibcall_local32, > sibcall_local64, sibcall_value_local32, sibcall_value_local64, > sibcall_nonlocal_sysv, sibcall_value_nonlocal_sysv): > Remove the USE of LR_REGNO from the patterns as well. Delete an > obsolete comment. > (return_internal_): Delete. This is okay. PPC64 BE Linux uses ABI_AIX, so that AIX logic should be okay. Thanks, David
Re: PR35503 - warn for restrict pointer
On Tue, 2016-08-30 at 17:08 +0530, Prathamesh Kulkarni wrote:
> On 30 August 2016 at 05:34, David Malcolm
> wrote:
> > On Mon, 2016-08-29 at 20:01 -0400, David Malcolm wrote:
> > > On Mon, 2016-08-29 at 19:55 -0400, David Malcolm wrote:
> > > [...]
> > > > Assuming you have the location_t values available, you can
> > > > create a
> > > > rich_location for the primary range, and then add secondary
> > > > ranges
> > > > like
> > > > this:
> > > >
> > > > rich_location richloc (loc_of_arg1);
> > >
> > > Oops, the above should be:
> > >
> > > rich_location richloc (line_table, loc_of_arg1);
> > >
> > > or:
> > >
> > > gcc_rich_location (loc_of_arg1);
> > and this should be:
> >
> > gcc_rich_location richloc (loc_of_arg1);
> > > which does the same thing (#include "gcc-rich-location.h").
> >
> > Clearly I need to sleep :)
> Hi David,
> Thanks for the suggestions. I can now see multiple source ranges for
> pr35503-2.c (included in patch).
> Output shows: http://pastebin.com/FNAVDU8A
> (Posted pastebin link to avoid mangling by the mailer)
The underlines look great, thanks for working on this.
> However the test for underline fails:
> FAIL: c-c++-common/pr35503-2.c -Wc++-compat expected multiline
> pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
> &alpha\);.*\n \^~ ~~ ~~ .*\n"
> I have attached gcc.log for the test-case. Presumably I have written
> the test-case incorrectly.
> Could you please have a look at it ?
(I hope this doesn't get too badly mangled by Evolution...)
I think you have an extra trailing space on the line containing the
expected underlines within the multiline directive:
+/* { dg-begin-multiline-output "" }
+ f (&alpha, &beta, &alpha, &alpha);
+ ^~ ~~ ~~
^ EXTRA SPACE HERE
+ { dg-end-multiline-output "" } */
+}
as the actual output is:
f (&alpha, &beta, &alpha, &alpha);
^~ ~~ ~~
^ LINE ENDS HERE, with no trailing
space present
This space shows up in the error here:
FAIL: c-c++-common/pr35503-2.c -Wc++-compat expected multiline
pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
&alpha\);.*\n \^~ ~~ ~~ .*\n"
^ EXTRA SPACE
BTW, the .* at the end of the pattern means it's ok to have additional
material in the actual output that isn't matched (e.g. for comments
containing dg- directives [1] ) but it doesn't work the other way
around: all of the text within the dg-begin/end-multiline directives
has to be in the actual output.
[1] so you can have e.g.:
f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to
restrict-qualified parameter aliases with arguments 3, 4" } */
and:
/* { dg-begin-multiline-output "" }
f (&alpha, &beta, &alpha, &alpha);
^~ ~~ ~~
{ dg-end-multiline-output "" } */
where the actual output will look like:
pr35503-2.c:8:6: warning: passing argument 1 to restrict-qualified parameter
aliases with arguments 3, 4 [-Wrestrict]
f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to
restrict-qualified parameter aliases with arguments 3, 4" } */
^~ ~~ ~~
and you can omit the copy of the dg-warning directive in the expected
multiline output (which would otherwise totally confuse DejaGnu).
Doing so avoids having to specify the line number.
> Thanks,
> Prathamesh
> >
> > > > richloc.add_range (loc_of_arg3, false); /* false here =
> > > > don't
> > > > draw
> > > > a
> > > > caret, just the underline */
> > > > richloc.add_range (loc_of_arg4, false);
> > > > warning_at_rich_loc (&richloc, OPT_Wrestrict, etc...
> > > >
> > > > See line-map.h for more information on rich_location.
> > >
> > > [...]
Re: [RFC PATCH, alpha]: Disable _FloatN on alpha
On Tue, Aug 30, 2016 at 2:09 PM, Joseph Myers wrote:
> On Tue, 30 Aug 2016, Uros Bizjak wrote:
>
>> I didn't find a way around this limitation, so I propose to disable
>> _FloatN on alpha with the attached patch.
>
> If your ABI requires binary32 values to be promoted in variable arguments,
> I'd think you could do that with a new target hook that specifies a type
> to promote to in variable arguments (when such promotion otherwise would
> not occur), affecting code generation for both function calls and va_arg
> (but not for unprototyped function calls, only for variable arguments).
> (TS 18661 allows conversion to the same type to act as a convertFormat
> operation, so it's allowed for such argument passing to convert sNaNs to
> qNaNs, which would be the effect of having such conversions to and from
> binary64 in the call path.)
>
> (Ideally of course such a hook would take effect *after* the front end,
> but various existing such hooks operate in the front end so I don't think
> it's a problem for a new hook to do so as well.)
IIUC, trying to solve this issue in the front-end would bring us to
float->double promotion rules for variable arguments. The compiler
doesn't convert va-args automatically, it only warns for undefined
situation, e.g.:
--cut here--
#include
float sum (float a, ...)
{
va_list arg;
float acc = a;
va_start (arg, a);
acc += va_arg (arg, float);
va_end (arg);
return acc;
}
--cut here--
$ gcc -O2 va.c
va.c: In function ‘sum’:
va.c:9: warning: ‘float’ is promoted to ‘double’ when passed through ‘...’
va.c:9: warning: (so you should pass ‘double’ not ‘float’ to ‘va_arg’)
va.c:9: note: if this code is reached, the program will abort
In a similar way, if we require that _Float32 gets promoted to
_Float64 in va-arg, all the compiler can do is to generate a
target-dependant warning, so user can fix the program. I don't think
this is desirable, as we would have something like:
#ifdef __alpha__
acc += va_arg (arg, _Float64);
#else
acc += va_arg (arg, _Float32);
#endif
Maybe better way forward would be to introduce a backend hook to
generate target-dependant load of the argument from the save area.
This way, it would be possible to emit some kind of builtin that
expands to some RTX sequence.
Uros.
Re: [PATCH][AArch64] Add legitimize_address_displacement hook
On 10/08/16 17:31, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>> OK. But please enhance the comment with some explanation as to WHY
>> you've chosen to use just two base pairings rather than separate bases
>> for each access size.
>
> OK here is the updated patch which also handles unaligned accesses
> which further improves the benefit:
>
> This patch adds legitimize_address_displacement hook so that stack accesses
> with large offsets are split into a more efficient sequence. Unaligned and
> TI/TFmode use a 256-byte range, byte and halfword accesses use a 4KB range,
> wider accesses use a 16KB range to maximise the available addressing range
> and increase opportunities to share the base address.
>
> int f(int x)
> {
> int arr[8192];
> arr[4096] = 0;
> arr[6000] = 0;
> arr[7000] = 0;
> arr[8191] = 0;
> return arr[x];
> }
>
> Now generates:
>
> sub sp, sp, #32768
> add x1, sp, 16384
> str wzr, [x1]
> str wzr, [x1, 7616]
> str wzr, [x1, 11616]
> str wzr, [x1, 16380]
> ldr w0, [sp, w0, sxtw 2]
> add sp, sp, 32768
> ret
>
> instead of:
>
> sub sp, sp, #32768
> mov x2, 28000
> add x1, sp, 16384
> mov x3, 32764
> str wzr, [x1]
> mov x1, 24000
> add x1, sp, x1
> str wzr, [x1]
> add x1, sp, x2
> str wzr, [x1]
> add x1, sp, x3
> str wzr, [x1]
> ldr w0, [sp, w0, sxtw 2]
> add sp, sp, 32768
> ret
>
> Bootstrap, GCC regression OK.
>
> ChangeLog:
> 2016-08-10 Wilco Dijkstra
>
> gcc/
> * config/aarch64/aarch64.c (aarch64_legitimize_address_displacement):
> New function.
> (TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): Define.
OK.
R.
> --
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 9a5fc199128b1326d0fb2afe0833aa6a5ce62ddf..b8536175a84b76f8c2939e61f1379ae279b20d43
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -4173,6 +4173,24 @@ aarch64_legitimate_address_p (machine_mode mode, rtx x,
>return aarch64_classify_address (&addr, x, mode, outer_code, strict_p);
> }
>
> +/* Split an out-of-range address displacement into a base and offset.
> + Use 4KB range for 1- and 2-byte accesses and a 16KB range otherwise
> + to increase opportunities for sharing the base address of different sizes.
> + For TI/TFmode and unaligned accesses use a 256-byte range. */
> +static bool
> +aarch64_legitimize_address_displacement (rtx *disp, rtx *off, machine_mode
> mode)
> +{
> + HOST_WIDE_INT mask = GET_MODE_SIZE (mode) < 4 ? 0xfff : 0x3fff;
> +
> + if (mode == TImode || mode == TFmode ||
> + (INTVAL (*disp) & (GET_MODE_SIZE (mode) - 1)) != 0)
> + mask = 0xff;
> +
> + *off = GEN_INT (INTVAL (*disp) & ~mask);
> + *disp = GEN_INT (INTVAL (*disp) & mask);
> + return true;
> +}
> +
> /* Return TRUE if rtx X is immediate constant 0.0 */
> bool
> aarch64_float_const_zero_rtx_p (rtx x)
> @@ -14137,6 +14155,10 @@ aarch64_optab_supported_p (int op, machine_mode
> mode1, machine_mode,
> #undef TARGET_LEGITIMATE_CONSTANT_P
> #define TARGET_LEGITIMATE_CONSTANT_P aarch64_legitimate_constant_p
>
> +#undef TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT
> +#define TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT \
> + aarch64_legitimize_address_displacement
> +
> #undef TARGET_LIBGCC_CMP_RETURN_MODE
> #define TARGET_LIBGCC_CMP_RETURN_MODE aarch64_libgcc_cmp_return_mode
>
>
Re: [ARM][PR target/77281] Fix an invalid check for vectors of, the same floating-point constants.
Ping. On 19/08/16 15:47, Richard Earnshaw (lists) wrote: On 19/08/16 15:06, Matthew Wahab wrote: On 19/08/16 14:30, Richard Earnshaw (lists) wrote: On 19/08/16 12:48, Matthew Wahab wrote: 2016-08-19 Matthew Wahab PR target/77281 * config/arm/arm.c (neon_valid_immediate): Delete declaration. Use const_vec_duplicate to check for duplicated elements. Ok for trunk? OK. Thanks. R. Is this ok to backport to gcc-6? Matthew I believe we're in a release process, so backporting needs RM approval. R. Is this ok to backport to gcc-6? I've tested for arm-none-linux-gnueabihf with native bootstrap and check-gcc. Matthew
[committed] rich_location: add convenience overloads for adding fix-it hints
Adding a fix-it hint to a diagnostic usually follows one of these
patterns:
(a) an insertion fix-its, with the insertion at the primary caret location
(b) a removals/replacements, affecting the range of the primary location
(other cases are possible, e.g. multiple fix-its, and affecting other
locations, but these are the common ones)
Given these common cases, this patch adds overloads of the rich_location
methods for adding fix-it hints, so that the location information can
be omitted if it matches that of the primary location within the
rich_location.
Similarly when adding "remove" and "replace" fix-it hints to a diagnostic,
it's tedious to have to extract the source_range from a location_t
(aka source_location). To make this more convenient, this patch
adds overload of the rich_location::add_fixit_remove/replace methods,
accepting a source_location directly.
The patch updates the various in-tree users of fix-it hints to use
the new simpler API where appropriate. I didn't touch the case where
there are multiple fix-its in one rich_location, as it seems better to
be more explicit about locations for this case (adding a pair of parens
in warn_logical_not_parentheses).
The above makes the gcc_rich_location::add_fixit_misspelled_id overload
taking a const char * rather redundant, so I eliminated it.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r239861.
gcc/c/ChangeLog:
* c-decl.c (implicit_decl_warning): Use add_fixit_replace
rather than add_fixit_misspelled_id.
(undeclared_variable): Likewise.
* c-parser.c (c_parser_declaration_or_fndef): Likewise. Remove
now-redundant "here" params from add_fixit_insert method calls.
(c_parser_parameter_declaration): Likewise.
* c-typeck.c (build_component_ref): Remove now-redundant range
param from add_fixit_replace method calls.
gcc/cp/ChangeLog:
* name-lookup.c (suggest_alternatives_for): Use add_fixit_replace
rather than add_fixit_misspelled_id.
* parser.c (cp_parser_diagnose_invalid_type_name): Likewise.
gcc/ChangeLog:
* diagnostic-show-locus.c (test_one_liner_fixit_insert): Remove
redundant location param.
(test_one_liner_fixit_remove): Likewise.
(test_one_liner_fixit_replace): Likewise.
(test_one_liner_fixit_replace_equal_secondary_range): Likewise.
* gcc-rich-location.c
(gcc_rich_location::add_fixit_misspelled_id): Eliminate call to
get_range_from_loc. Drop overload taking a const char *.
* gcc-rich-location.h
(gcc_rich_location::add_fixit_misspelled_id): Drop overload taking
a const char *.
libcpp/ChangeLog:
* include/line-map.h (rich_location::add_fixit_insert): Add
comments. Add overload omitting the source_location param.
(rich_location::add_fixit_remove): Add comments. Add overloads
omitting the range, and accepting a source_location.
(rich_location::add_fixit_replace): Likewise.
* line-map.c (rich_location::add_fixit_insert): Add comments. Add
overload omitting the source_location param.
(rich_location::add_fixit_remove): Add comments. Add overloads
omitting the range, and accepting a source_location.
(rich_location::add_fixit_replace): Likewise.
---
gcc/c/c-decl.c | 8 +++
gcc/c/c-parser.c| 10 -
gcc/c/c-typeck.c| 2 +-
gcc/cp/name-lookup.c| 2 +-
gcc/cp/parser.c | 2 +-
gcc/diagnostic-show-locus.c | 17 --
gcc/gcc-rich-location.c | 17 +-
gcc/gcc-rich-location.h | 2 --
libcpp/include/line-map.h | 34
libcpp/line-map.c | 54 +
10 files changed, 105 insertions(+), 43 deletions(-)
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 0c52a64..8f49c35 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3096,7 +3096,7 @@ implicit_decl_warning (location_t loc, tree id, tree
olddecl)
if (hint)
{
gcc_rich_location richloc (loc);
- richloc.add_fixit_misspelled_id (loc, hint);
+ richloc.add_fixit_replace (hint);
warned = pedwarn_at_rich_loc
(&richloc, OPT_Wimplicit_function_declaration,
"implicit declaration of function %qE; did you mean %qs?",
@@ -3109,7 +3109,7 @@ implicit_decl_warning (location_t loc, tree id, tree
olddecl)
if (hint)
{
gcc_rich_location richloc (loc);
- richloc.add_fixit_misspelled_id (loc, hint);
+ richloc.add_fixit_replace (hint);
warned = warning_at_rich_loc
(&richloc, OPT_Wimplicit_function_declaration,
G_("implicit declaration of function %qE;did you mean %qs?"),
@@ -3437,7 +3437,7 @@ undeclared_variable (location_t loc, tree id)
if (guessed_id)
{
Re: [RFC PATCH, alpha]: Disable _FloatN on alpha
On Tue, 30 Aug 2016, Uros Bizjak wrote: > IIUC, trying to solve this issue in the front-end would bring us to > float->double promotion rules for variable arguments. The compiler > doesn't convert va-args automatically, it only warns for undefined > situation, e.g.: No, those would be unchanged. The hook would mean two things: * When generating the call, a promotion is inserted for _Float32 like it would be for float (if that's the appropriate ABI for this case on this architecture). * When expanding va_arg (ap, _Float32) - which is perfectly valid, unlike va_arg (ap, float) - the front end changes the call to look like (_Float32) va_arg (ap, _Float64). > Maybe better way forward would be to introduce a backend hook to > generate target-dependant load of the argument from the save area. > This way, it would be possible to emit some kind of builtin that > expands to some RTX sequence. Sure, if you can make the back end do the right thing so that plain __builtin_va_arg (ap, _Float32) works without the front end needing to change the type and insert a promotion at the call site and a conversion back to _Float32 at the va_arg site, that's even better. -- Joseph S. Myers jos...@codesourcery.com
[PATCH] C: fixits for modernizing structure member designators
This patch adds fix-it hints to our warning for old-style structure
member designators, showing how to modernize them to C99 form.
For example:
modernize-named-inits.c:19:5: warning: obsolete use of designated initializer
with ‘:’ [-Wpedantic]
foo: 1,
^
. =
In conjunction with the not-yet-in-trunk -fdiagnostics-generate-patch,
this can generate patches like this:
--- modernize-named-inits.c
+++ modernize-named-inits.c
@@ -16,7 +16,7 @@
/* Old-style named initializers. */
struct foo old_style_f = {
- foo: 1,
- bar: 2,
+ .foo= 1,
+ .bar= 2,
};
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c/ChangeLog:
* c-parser.c (c_parser_initelt): Provide fix-it hints for
modernizing old-style structure member designator to C99 style.
gcc/testsuite/ChangeLog:
* gcc.dg/modernize-named-inits.c: New test case.
---
gcc/c/c-parser.c | 17 +++-
gcc/testsuite/gcc.dg/modernize-named-inits.c | 39
2 files changed, 50 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/modernize-named-inits.c
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index c245e70..a6281fc 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4459,13 +4459,18 @@ c_parser_initelt (c_parser *parser, struct obstack *
braced_init_obstack)
&& c_parser_peek_2nd_token (parser)->type == CPP_COLON)
{
/* Old-style structure member designator. */
- set_init_label (c_parser_peek_token (parser)->location,
- c_parser_peek_token (parser)->value,
- c_parser_peek_token (parser)->location,
- braced_init_obstack);
+ location_t name_loc = c_parser_peek_token (parser)->location;
+ tree fieldname = c_parser_peek_token (parser)->value;
+ location_t colon_loc = c_parser_peek_2nd_token (parser)->location;
+ set_init_label (name_loc, fieldname, name_loc, braced_init_obstack);
/* Use the colon as the error location. */
- pedwarn (c_parser_peek_2nd_token (parser)->location, OPT_Wpedantic,
- "obsolete use of designated initializer with %<:%>");
+ rich_location richloc (line_table, colon_loc);
+ /* Provide fix-it hints for converting to C99 style i.e.
+from "NAME:" to ".NAME =". */
+ richloc.add_fixit_insert (name_loc, ".");
+ richloc.add_fixit_replace (colon_loc, "=");
+ pedwarn_at_rich_loc (&richloc, OPT_Wpedantic,
+ "obsolete use of designated initializer with %<:%>");
c_parser_consume_token (parser);
c_parser_consume_token (parser);
}
diff --git a/gcc/testsuite/gcc.dg/modernize-named-inits.c
b/gcc/testsuite/gcc.dg/modernize-named-inits.c
new file mode 100644
index 000..4e16494
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/modernize-named-inits.c
@@ -0,0 +1,39 @@
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-show-caret -std=c99 -pedantic" } */
+
+struct foo
+{
+ int foo;
+ int bar;
+};
+
+union u
+{
+ int color;
+ int shape;
+};
+
+/* Old-style named initializers. */
+
+struct foo old_style_f = {
+ foo: 1, /* { dg-warning "5: obsolete use of designated initializer with ':'"
} */
+/* { dg-begin-multiline-output "" }
+ foo: 1,
+ ^
+ . =
+ { dg-end-multiline-output "" } */
+
+ bar: 1, /* { dg-warning "9: obsolete use of designated initializer with
':'" } */
+ /* { dg-begin-multiline-output "" }
+ bar: 1,
+ ^
+ . =
+{ dg-end-multiline-output "" } */
+};
+
+union u old_style_u = { color: 3 }; /* { dg-warning "30: obsolete use of
designated initializer with ':'" } */
+/* { dg-begin-multiline-output "" }
+ union u old_style_u = { color: 3 };
+ ^
+ .=
+ { dg-end-multiline-output "" } */
--
1.8.5.3
[PATCH] C++: add fixit for '>>' template error
This patch adds fix-it hints to our warning for >>
within a nested template argument list (for -std=c++98).
For example:
double-greater-than-fixit.C:5:12: error: ‘>>’ should be ‘> >’ within a nested
template argument list
foo> i;
^~
> >
In conjunction with the not-yet-in-trunk -fdiagnostics-generate-patch,
this can generate patches like this:
--- double-greater-than-fixit.C
+++ double-greater-than-fixit.C
@@ -4,3 +4,3 @@
-foo> i;
+foo > i;
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
* parser.c (cp_parser_enclosed_template_argument_list): Add fix-it
hint to ">>" within nested template argument list error.
gcc/testsuite/ChangeLog:
* g++.dg/template/double-greater-than-fixit.C: New test case.
---
gcc/cp/parser.c | 6 --
gcc/testsuite/g++.dg/template/double-greater-than-fixit.C | 10 ++
2 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 48dbca1..ca9f8b9 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -26342,8 +26342,10 @@ cp_parser_enclosed_template_argument_list (cp_parser*
parser)
global source location is still on the token before the
'>>', so we need to say explicitly where we want it. */
cp_token *token = cp_lexer_peek_token (parser->lexer);
- error_at (token->location, "%<>>%> should be %<> >%> "
- "within a nested template argument list");
+ gcc_rich_location richloc (token->location);
+ richloc.add_fixit_replace ("> >");
+ error_at_rich_loc (&richloc, "%<>>%> should be %<> >%> "
+"within a nested template argument list");
token->type = CPP_GREATER;
}
diff --git a/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
b/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
new file mode 100644
index 000..f0de4ec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
@@ -0,0 +1,10 @@
+/* { dg-options "-fdiagnostics-show-caret -std=c++98" } */
+template
+struct foo {};
+
+foo> i; // { dg-error "12: .>>. should be .> >. within a nested
template argument list" }
+/* { dg-begin-multiline-output "" }
+ foo> i;
+^~
+> >
+ { dg-end-multiline-output "" } */
--
1.8.5.3
Re: Implement -Wimplicit-fallthrough (version 7)
On Mon, Aug 29, 2016 at 04:02:33PM -0400, Jason Merrill wrote:
> On Mon, Aug 29, 2016 at 7:58 AM, Marek Polacek wrote:
> > --- gcc/gcc/cp/parser.c
> > +++ gcc/gcc/cp/parser.c
> > @@ -5135,6 +5135,31 @@ cp_parser_primary_expression (cp_parser *parser,
> > case RID_AT_SELECTOR:
> > return cp_parser_objc_expression (parser);
> >
> > + case RID_ATTRIBUTE:
> > + {
> > + /* This might be __attribute__((fallthrough));. */
> > + tree attr = cp_parser_gnu_attributes_opt (parser);
> > + if (attr != NULL_TREE
> > + && is_attribute_p ("fallthrough", get_attribute_name
> > (attr)))
> > + {
> > + tree fn = build_call_expr_internal_loc (token->location,
> > + IFN_FALLTHROUGH,
> > + void_type_node, 0);
> > + return cp_expr (fn, token->location);
> > + }
> > + else if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
> > + {
> > + cp_parser_error (parser, "only attribute % "
> > +"can be used");
>
> Let's say "can be applied to a null statement".
Fixed.
> > @@ -10585,15 +10610,26 @@ cp_parser_statement (cp_parser* parser, tree
> > in_statement_expr,
> > }
> >/* Look for an expression-statement instead. */
> >statement = cp_parser_expression_statement (parser,
> > in_statement_expr);
> > +
> > + /* Handle [[fallthrough]];. */
> > + if (std_attrs != NULL_TREE
> > + && is_attribute_p ("fallthrough", get_attribute_name (std_attrs))
> > + && statement == NULL_TREE)
> > + {
> > + tree fn = build_call_expr_internal_loc (statement_location,
> > + IFN_FALLTHROUGH,
> > + void_type_node, 0);
>
> Let's use the 'statement' variable rather than a new variable fn so
> that we set the call's location below. Let's also warn here about
> [[fallthrough]] on non-null expression statements.
Done. But I couldn't figure out a testcase where the warning would trigger, so
not sure how important that is.
> > + finish_expr_stmt (fn);
>
> Let's clear std_attrs here...
>
> > + }
>
> >/* Set the line number for the statement. */
> >if (statement && STATEMENT_CODE_P (TREE_CODE (statement)))
> > SET_EXPR_LOCATION (statement, statement_location);
> >
> > - /* Note that for now, we don't do anything with c++11 statements
> > - parsed at this level. */
> > - if (std_attrs != NULL_TREE)
> > + /* Allow "[[fallthrough]];", but warn otherwise. */
> > + if (std_attrs != NULL_TREE
> > + && !is_attribute_p ("fallthrough", get_attribute_name (std_attrs)))
>
> ...so we don't need to check for fallthrough here.
Ok, done.
> > @@ -24114,6 +24164,16 @@ cp_parser_std_attribute (cp_parser *parser)
> > " use %");
> > TREE_PURPOSE (TREE_PURPOSE (attribute)) = get_identifier ("gnu");
> > }
> > + /* C++17 fallthrough attribute is equivalent to GNU's. */
> > + else if (cxx_dialect >= cxx11
>
> There's no point checking this, here or for attribute deprecated; if
> we're in this function, we must be in C++11 or above. Let's drop the
> check in both places.
Makes sense, fixed.
Thanks for looking into this. I'll post another version once it passes
testing.
Marek
[committed] Small documentation fix to STRUCTURE
Committed as obvious as r239862, this fixes a "rebase-o" ("typo during
rebase") that found its way into the documentation on STRUCTURE which
is clearly incorrect:
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi (revision 239861)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -2127,9 +2127,10 @@ be disabled using -std=legacy.
@cindex @code{RECORD}
Record structures are a pre-Fortran-90 vendor extension to create
-user-defined aggregate data types. GNU Fortran does not support
-record structures, only Fortran 90's ``derived types'', which have
-a different syntax.
+user-defined aggregate data types. Support for record structures in GNU
+Fortran can be enabled with the @option{-fdec-structure} compile flag.
+If you have a choice, you should instead use Fortran 90's ``derived types'',
+which have a different syntax.
In many cases, record structures can easily be converted to derived types.
To convert, replace @code{STRUCTURE /}@var{structure-name}@code{/}
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (revision 239861)
+++ gcc/fortran/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2016-08-30 Fritz Reese
+
+ * gfortran.texi: Fix typo in STRUCTURE documentation.
+
2016-08-29 Fritz Reese
Fix, reorganize, and clarify comparisons of anonymous types/components.
---
Fritz Reese
Re: [PATCH] C: fixits for modernizing structure member designators
On 08/30/2016 04:38 PM, David Malcolm wrote:
In conjunction with the not-yet-in-trunk -fdiagnostics-generate-patch,
this can generate patches like this:
--- modernize-named-inits.c
+++ modernize-named-inits.c
@@ -16,7 +16,7 @@
/* Old-style named initializers. */
struct foo old_style_f = {
- foo: 1,
- bar: 2,
+ .foo= 1,
+ .bar= 2,
};
What happens if there are newlines in between any of the tokens?
Bernd
Re: [PATCH] C: fixits for modernizing structure member designators
On Tue, Aug 30, 2016 at 04:40:57PM +0200, Bernd Schmidt wrote:
> On 08/30/2016 04:38 PM, David Malcolm wrote:
>
> > In conjunction with the not-yet-in-trunk -fdiagnostics-generate-patch,
> > this can generate patches like this:
> >
> > --- modernize-named-inits.c
> > +++ modernize-named-inits.c
> > @@ -16,7 +16,7 @@
> > /* Old-style named initializers. */
> >
> > struct foo old_style_f = {
> > - foo: 1,
> > - bar: 2,
> > + .foo= 1,
> > + .bar= 2,
> > };
>
> What happens if there are newlines in between any of the tokens?
It's easy to check for yourself: when the identifier and colon are not
on the same line, we print something like
w.c: In function ‘main’:
w.c:7:1: warning: obsolete use of designated initializer with ‘:’ [-Wpedantic]
:
^
=
which I don't think is desirable -- giving up on the fix-it hint in such case
could be appropriate.
I admit I dislike the lack of a space before = in ".bar= 2", but using
richloc.add_fixit_replace (colon_loc, " =");
wouldn't work for "foo : 1" I think.
Marek
Re: PR35503 - warn for restrict pointer
On 26/08/16 13:39, Prathamesh Kulkarni wrote:
Hi,
The following patch adds option -Wrestrict that warns when an argument
is passed to a restrict qualified parameter and it aliases with
another argument.
eg:
int foo (const char *__restrict buf, const char *__restrict fmt, ...);
void f(void)
{
char buf[100] = "hello";
foo (buf, "%s-%s", buf, "world");
}
Does -Wrestrict generate a warning for this example?
...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
int i;
for (i = 0; i < n; i++)
p[i] = q[i] + r[i];
}
h (100, a, b, b)
...
[ Note that this is valid C, and does not violate the restrict definition. ]
If -Wrestrict indeed generates a warning, then we should be explicit in
the documentation that while the warning triggers on this type of
example, the code is correct.
Thanks,
- Tom
[gomp4] runtime default compute dimensions
This patch interrogates the target device to determine default gemotry at
runtime. This has the greatest difference on gang partitioning, where there's a
noticeable sawtooth in the relationship between number of gangs and execution
time. Picking the number of gangs as an exact multiple of number of physical
multi-cpus gets the best performance. Picking one more than that gives a step
increase in execution time. The sawtooth gets blunter as the multiplication
factor increases, as one might expect when scheduling smaller and smaller
parcels of work onto a limited set of physical cpus.
nathan
2016-08-30 Nathan Sidwell
* plugin/plugin-nvptx.c (nvptx_exec): Interrogate board attributes
to determine default geometry.
Index: plugin/plugin-nvptx.c
===
--- plugin/plugin-nvptx.c (revision 239862)
+++ plugin/plugin-nvptx.c (working copy)
@@ -938,14 +938,42 @@ nvptx_exec (void (*fn), size_t mapnum, v
}
}
- /* Do some sanity checking. The CUDA API doesn't appear to
- provide queries to determine these limits. */
+ int warp_size, block_size, dev_size, cpu_size;
+ CUdevice dev = nvptx_thread()->ptx_dev->dev;
+ /* 32 is the default for known hardware. */
+ int gang = 0, worker = 32, vector = 32;
+
+ if (CUDA_SUCCESS == cuDeviceGetAttribute
+ (&block_size, CU_DEVICE_ATTRIBUTE_MAX_THREADS_PER_BLOCK, dev)
+ && CUDA_SUCCESS == cuDeviceGetAttribute
+ (&warp_size, CU_DEVICE_ATTRIBUTE_WARP_SIZE, dev)
+ && CUDA_SUCCESS == cuDeviceGetAttribute
+ (&dev_size, CU_DEVICE_ATTRIBUTE_MULTIPROCESSOR_COUNT, dev)
+ && CUDA_SUCCESS == cuDeviceGetAttribute
+ (&cpu_size, CU_DEVICE_ATTRIBUTE_MAX_THREADS_PER_MULTIPROCESSOR, dev))
+ {
+ GOMP_PLUGIN_debug (0, " warp_size=%d, block_size=%d,"
+ " dev_size=%d, cpu_size=%d\n",
+ warp_size, block_size, dev_size, cpu_size);
+ gang = (cpu_size / block_size) * dev_size;
+ worker = block_size / warp_size;
+ vector = warp_size;
+ }
+
+ /* There is no upper bound on the gang size. The best size
+ matches the hardware configuration. Logical gangs are
+ scheduled onto physical hardware. To maximize usage, we
+ should guess a large number. */
if (default_dims[GOMP_DIM_GANG] < 1)
- default_dims[GOMP_DIM_GANG] = 32;
+ default_dims[GOMP_DIM_GANG] = gang ? gang : 1024;
+ /* The worker size must not exceed the hardware. */
if (default_dims[GOMP_DIM_WORKER] < 1
- || default_dims[GOMP_DIM_WORKER] > 32)
- default_dims[GOMP_DIM_WORKER] = 32;
- default_dims[GOMP_DIM_VECTOR] = 32;
+ || (default_dims[GOMP_DIM_WORKER] > worker && gang))
+ default_dims[GOMP_DIM_WORKER] = worker;
+ /* The vector size must exactly match the hardware. */
+ if (default_dims[GOMP_DIM_VECTOR] < 1
+ || (default_dims[GOMP_DIM_VECTOR] != vector && gang))
+ default_dims[GOMP_DIM_VECTOR] = vector;
GOMP_PLUGIN_debug (0, " default dimensions [%d,%d,%d]\n",
default_dims[GOMP_DIM_GANG],
Re: Ping : [Patch, fortran] PR48298 - [F03] User-Defined Derived-Type IO (DTIO)
On Mon, Aug 29, 2016 at 1:15 PM, Paul Richard Thomas wrote: > Hi Janne, Andre, Jerry and All, > > I am perfectly happy to adopt Janne's suggestion for > st_set_(dtio_)nml_var. Do the changes to the IO structures have any > impact? These are in: > > fnode, st_parameter_dt & gfc_unit > > I don't think that these should be visible but I want expert opinion > before making that assumption :-) fnode and gfc_unit are private to libgfortran, so these can be changed in any way you see fit. st_parameter_dt, however, is more complex. It is allocated by the frontend on the stack, with some empty space to be used as scratch space by the library (the st_parameter_dt.u member). So you're free to do whatever you want with the private part, as long as you keep the size of the u.p struct smaller than u.pad (which should happen automagically unless you have mangled check_st_parameter_dt somehow). Otherwise I don't think you should go and touch st_parameter_dt. This design is somewhat unfortunate, in that it quite severely restricts the recursion depth of any function potentially doing I/O. Say, you have a recursive function that writes something if some condition is met. Any function potentially doing I/O bumps up the stack by almost 1kB.. I'm thinking a simpler design for handling the large number of optional I/O specifiers could be something like having 3 arguments, an array of keys, an array of values, and a scalar telling how many elements the key/value arrays have. That way only the specifiers that are actually specified would need to have space allocated and be transferred. And then instead of a having the frontend allocate a large scratch space on the stack, the library could use a TLS variable to look it up. But anyway, talk is cheap, and I'm unlikely to have time to implement the above any time soon... :( > > Cheers > > > Paul > > On 29 August 2016 at 10:46, Janne Blomqvist wrote: >> On Mon, Aug 29, 2016 at 11:15 AM, Andre Vehreschild wrote: >>> Hi all, >>> Anyway, a small nit I found was the function st_set_nml_var in libgfortran. This is an exported function, and thus part of the ABI. So you cannot add arguments to it, as that would break backwards compatibility. >>> >>> Please explain the above. I was of the opinion, that when we change >>> something significantly the global ABI version gets bumped. Given that >>> we are doing gcc-7 currently and there have been some changes, that ABI >>> version should have been bumped already with respect to gcc-6. >> >> We strive very(?) hard to retain ABI compatibility for libgfortran, as >> having to recompile everything is a huge PITA for our users. As a >> result we have been able avoid bumping the SO major version number >> since GCC 4.3 IIRC. >> >> There is a long laundry-list of cleanups that could be done once we do >> bump the SO major version: >> https://gcc.gnu.org/wiki/LibgfortranAbiCleanup >> >> Probably when (if?) the new array descriptor is merged we have to do >> said bump, as that one is used everywhere and retaining compatibility >> with the old descriptor seems to be a huge undertaking. >> >>> I am asking that stupid question mostly, because during extending >>> coarray stuff to support allocatable components in derived typed >>> coarrays the API of the caf-library has to be modified and carrying >>> along the old signatures just causes useless garbage being carried >>> forward. (Opencoarrays is working on supporting the same renovated API. >>> Other users of that API are not known to me.) So what is the best way >>> to resolve this? >> >> I haven't involved myself in the coarray stuff, but AFAIU the corray >> lib hasn't been considered stable, in order that the developers can >> more quickly iterate on the design without having to be bogged down by >> ABI compatibility considerations. >> >> -- >> Janne Blomqvist > > > > -- > The difference between genius and stupidity is; genius has its limits. > > Albert Einstein -- Janne Blomqvist
Re: [patch] Some testsuite cleanup
On 01/08/16 17:31 +0100, Jonathan Wakely wrote:
On 01/08/16 09:23 -0700, Mike Stump wrote:
On Jul 31, 2016, at 1:30 PM, Jonathan Wakely wrote:
-fno-show-column
is a good general option. If you guys want to add column number test cases,
they can avoid it, and test down to the column. Most people don't care, and
most test aren't interested in column testing anyway. But, if you want to do a
sea change, you can add column numbers to every expected line and that way,
they all will fail, if any of the numbers go wrong. If someone wants to sign
up for that, it is slightly better, but requires that someone do all the work.
OK, thanks Mike.
I plan to make this change then (rather than adding it to individual
tests as and when we find one that needs it).
--- a/libstdc++-v3/scripts/testsuite_flags.in
+++ b/libstdc++-v3/scripts/testsuite_flags.in
@@ -56,7 +56,7 @@ case ${query} in
echo ${CC}
;;
--cxxflags)
- CXXFLAGS_default="-D_GLIBCXX_ASSERT -fmessage-length=0"
+ CXXFLAGS_default="-D_GLIBCXX_ASSERT -fmessage-length=0 -fno-show-column"
CXXFLAGS_config="@SECTION_FLAGS@ @EXTRA_CXX_FLAGS@"
echo ${CXXFLAGS_default} ${CXXFLAGS_config} ;;
This adds it to the default flags used for the entire libstdc++
testsuite.
I've also added -fno-show-column on the gcc-6-branch, with this patch.
This makes it easier to backport patches from trunk.
commit aa3fdea285ce29a6444efd0fd87008bbaf960332
Author: redi
Date: Tue Aug 2 19:34:15 2016 +
Add -fno-show-column to libstdc++ test flags
Backport from mainline
2016-08-02 Jonathan Wakely
* scripts/testsuite_flags.in: Add -fno-show-column to cxxflags.
Backport from mainline
2016-07-27 Jonathan Wakely
* testsuite/20_util/forward/1_neg.cc: Move dg-error to right line.
diff --git a/libstdc++-v3/scripts/testsuite_flags.in b/libstdc++-v3/scripts/testsuite_flags.in
index ee59167..ea1a464 100755
--- a/libstdc++-v3/scripts/testsuite_flags.in
+++ b/libstdc++-v3/scripts/testsuite_flags.in
@@ -56,7 +56,7 @@ case ${query} in
echo ${CC}
;;
--cxxflags)
- CXXFLAGS_default="-D_GLIBCXX_ASSERT -fmessage-length=0"
+ CXXFLAGS_default="-D_GLIBCXX_ASSERT -fmessage-length=0 -fno-show-column"
CXXFLAGS_config="@SECTION_FLAGS@ @EXTRA_CXX_FLAGS@"
echo ${CXXFLAGS_default} ${CXXFLAGS_config}
;;
diff --git a/libstdc++-v3/testsuite/20_util/forward/1_neg.cc b/libstdc++-v3/testsuite/20_util/forward/1_neg.cc
index d2f3477..46ba96c 100644
--- a/libstdc++-v3/testsuite/20_util/forward/1_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/forward/1_neg.cc
@@ -27,8 +27,8 @@ template
std::shared_ptr
factory(A1&& a1, A2&& a2)
{
-return std::shared_ptr(new T(std::forward(a1),
-std::forward(a2))); // { dg-error "rvalue" }
+return std::shared_ptr(new T(std::forward(a1), // { dg-error "rvalue" }
+std::forward(a2)));
}
struct A
Re: [PATCH] C: fixits for modernizing structure member designators
On Tue, 2016-08-30 at 16:50 +0200, Marek Polacek wrote:
> On Tue, Aug 30, 2016 at 04:40:57PM +0200, Bernd Schmidt wrote:
> > On 08/30/2016 04:38 PM, David Malcolm wrote:
> >
> > > In conjunction with the not-yet-in-trunk -fdiagnostics-generate
> > > -patch,
> > > this can generate patches like this:
> > >
> > > --- modernize-named-inits.c
> > > +++ modernize-named-inits.c
> > > @@ -16,7 +16,7 @@
> > > /* Old-style named initializers. */
> > >
> > > struct foo old_style_f = {
> > > - foo: 1,
> > > - bar: 2,
> > > + .foo= 1,
> > > + .bar= 2,
> > > };
> >
> > What happens if there are newlines in between any of the tokens?
>
> It's easy to check for yourself: when the identifier and colon are
> not
> on the same line, we print something like
>
> w.c: In function ‘main’:
> w.c:7:1: warning: obsolete use of designated initializer with ‘:’ [
> -Wpedantic]
> :
> ^
> =
>
> which I don't think is desirable -- giving up on the fix-it hint in
> such case
> could be appropriate.
I think that's a bug in diagnostic-show-locus.c; currently it only
prints lines and fixits for the lines references in the ranges within
the rich_location. I'll try to fix that.
> I admit I dislike the lack of a space before = in ".bar= 2", but
> using
>
> richloc.add_fixit_replace (colon_loc, " =");
>
> wouldn't work for "foo : 1" I think.
I actually tried that first, but I didn't like the way it was printed
e.g.:
w.c: In function ‘main’:> w.c:7:1: warning: obsolete use of designated
initializer with ‘:’ [-Wpedantic]>
foo: 1,
^
. =
I'm looking at rewriting how fixits get printed, to print the affected
range of the affected lines (using the edit-context.c work posted last
week), so this would appear as:
foo: 1,
^
.foo =
Also, there may be a case for adding some smarts to gcc_rich_location for
adding fixits in a formatting-aware way, by looking at the neighboring
whitespace (this might help for the issue with adding "break;" etc in the
fallthru patch kit).
Dave
Re: PR35503 - warn for restrict pointer
On 30 August 2016 at 20:24, Tom de Vries wrote:
> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>
>> Hi,
>> The following patch adds option -Wrestrict that warns when an argument
>> is passed to a restrict qualified parameter and it aliases with
>> another argument.
>>
>> eg:
>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>
>> void f(void)
>> {
>> char buf[100] = "hello";
>> foo (buf, "%s-%s", buf, "world");
>> }
>
>
> Does -Wrestrict generate a warning for this example?
>
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
> int i;
> for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
>
> h (100, a, b, b)
> ...
>
> [ Note that this is valid C, and does not violate the restrict definition. ]
>
> If -Wrestrict indeed generates a warning, then we should be explicit in the
> documentation that while the warning triggers on this type of example, the
> code is correct.
I am afraid it would warn for the above case. The patch just checks if
the parameter is qualified
with restrict, and if the corresponding argument has aliases in the
call (by calling operand_equal_p).
Is such code common in practice ? If it is, I wonder if we should keep
the warning in -Wall ?
To avoid such false positives, we would need to track which arguments
are modified by the call.
I suppose that could be done with ipa mod/ref analysis (and moving the
warning to middle-end) ?
I tried looking into gcc sources but couldn't find where it's implemented.
Thanks,
Prathamesh
>
> Thanks,
> - Tom
Re: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems
On Thu, Aug 18, 2016 at 10:15:12AM +0100, Tamar Christina wrote: > Hi all, > > This fixes a bug in the vector load functions in which they load the > vector in the wrong order for big endian systems. This patch flips the > order conditionally in the vec_concats. > > No testcase given because plenty of existing tests for vld functions. > Ran regression tests on aarch64_be-none-elf and aarch64-none-elf. > Vldx tests now pass on aarch64_be-none-elf and no regressions on both. > > Ok for trunk? > > I do not have commit rights so if ok can someone apply it for me? Thanks for the patch. I committed it as revision 239865, with a minor fix-up to your code formatting. > @@ -5008,7 +5146,10 @@ >rtx mem = gen_rtx_MEM (BLKmode, operands[1]); >set_mem_size (mem, * 8); > > - emit_insn (gen_aarch64_ld_dreg (operands[0], > mem)); > + if (BYTES_BIG_ENDIAN) > + emit_insn (gen_aarch64_ld_dreg_be > (operands[0], mem)); > + else > + emit_insn (gen_aarch64_ld_dreg_le > (operands[0], mem)); >DONE; These lines exceed 80 characters and the indentation is wrong. I re-indented them to 4 spaces rather than a tab, and broke the line on the , - as so: @@ -5048,7 +5186,12 @@ rtx mem = gen_rtx_MEM (BLKmode, operands[1]); set_mem_size (mem, * 8); - emit_insn (gen_aarch64_ld_dreg (operands[0], mem)); + if (BYTES_BIG_ENDIAN) +emit_insn (gen_aarch64_ld_dreg_be (operands[0], + mem)); + else +emit_insn (gen_aarch64_ld_dreg_le (operands[0], + mem)); DONE; Thanks, James > gcc/ > 2016-08-16 Tamar Christina > > * gcc/config/aarch64/aarch64-simd.md > (aarch64_ld2_dreg_le): New. > (aarch64_ld2_dreg_be): New. > (aarch64_ld2_dreg): Removed. > (aarch64_ld3_dreg_le): New. > (aarch64_ld3_dreg_be): New. > (aarch64_ld3_dreg): Removed. > (aarch64_ld4_dreg_le): New. > (aarch64_ld4_dreg_be): New. > (aarch64_ld4_dreg): Removed. > (aarch64_ld): Wrapper around _le, _be.
Re: Implement -Wimplicit-fallthrough (version 7)
On Tue, Aug 30, 2016 at 07:38:18AM -0400, Eric Gallager wrote: > On 8/30/16, Marek Polacek wrote: > > On Mon, Aug 29, 2016 at 09:32:04AM -0400, Eric Gallager wrote: > >> I tried v6 on my binutils-gdb fork, and it printed A LOT of > >> warnings... > > > > BTW, why is that so? Does binutils-gdb not use various FALLTHRU comments? > > > > Marek > > > > > There are a lot of comments mentioning fallthroughs, but they mostly > need to be rewritten to be recognized properly. There's just so many > different ways of writing them. For example, one I've seen a lot > writes it as "Drop through" instead of "Fall through" or something. > Other examples of comments mentioning fallthroughs: > > /* else fall through */ > /* else fallthrough to: */ > /* FALL THRU into number case. */ > /* ObjC NSString constant: fall through and parse like STRING. */ > /* Fall through, pretend it is global. */ > /* Fall through into normal member function. */ > /* fall in for static symbols that do NOT start with '.' */ > /* >>> !! else fall through !! <<< */ > /* ... fall through for unsigned ints ... */ > /* fall thru to manual case */ Thanks, this is interesting. I wonder if we should also recognize "else fall through"; I've seen that in our codebase a few times. But why would anyone write something as ugly as ">>> !! else fall through !! <<<" is beyond me. > Also, at second glance, it's actually not quite as many warnings as I > thought it was at first, it mostly just looked that way since each > -Wimplicit-fallthrough warning takes up 12 lines due to the > double-fixits. FWIW, Aldy's -Walloca-larger-than patch actually was > noisier in the GDB portion at least. (The Binutils portion was already > clean on its alloca usage since it already used the -Wstack-usage > warning.) That's nice to hear. The latest version of the patch doesn't have the fix-it hints, it just says in an inform note where a statement falls through to, so it's going to be less verbose. > Oh, and one other thing I discovered: > __attribute__((fallthrough)) triggers -Wdeclaration-after-statement: > > In file included from ./defs.h:124:0, > from ./cli/cli-setshow.c:20: > ./cli/cli-setshow.c: In function ‘do_setshow_command’: > ./../include/ansidecl.h:505:33: warning: ISO C90 forbids mixed > declarations and code [-Wdeclaration-after-statement] > # define ATTRIBUTE_FALLTHROUGH __attribute__((fallthrough)) > ^ > ./cli/cli-setshow.c:359:4: note: in expansion of macro ‘ATTRIBUTE_FALLTHROUGH’ > ATTRIBUTE_FALLTHROUGH; > ^ > > Which doesn't really make sense to me. Yes, I think this is a bug. I'm pondering how to fix this. Thanks a lot Eric! Marek
Re: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems
On 18 August 2016 at 11:15, Tamar Christina wrote: > Hi all, > > This fixes a bug in the vector load functions in which they load the > vector in the wrong order for big endian systems. This patch flips the > order conditionally in the vec_concats. > > No testcase given because plenty of existing tests for vld functions. > Ran regression tests on aarch64_be-none-elf and aarch64-none-elf. > Vldx tests now pass on aarch64_be-none-elf and no regressions on both. > Before your patch, I can see aarch64/vldN_1.c and aarch64/advsimd-intrinsics/vldX_lane.c failing. Do you know why aarch64/advsimd-intrinsics/vldX.c and vldX_dup are not failing? Thanks Christophe > Ok for trunk? > > I do not have commit rights so if ok can someone apply it for me? > > Thanks, > Tamar > > gcc/ > 2016-08-16 Tamar Christina > > * gcc/config/aarch64/aarch64-simd.md > (aarch64_ld2_dreg_le): New. > (aarch64_ld2_dreg_be): New. > (aarch64_ld2_dreg): Removed. > (aarch64_ld3_dreg_le): New. > (aarch64_ld3_dreg_be): New. > (aarch64_ld3_dreg): Removed. > (aarch64_ld4_dreg_le): New. > (aarch64_ld4_dreg_be): New. > (aarch64_ld4_dreg): Removed. > (aarch64_ld): Wrapper around _le, _be.
[PATCH v3 1/2] Temporary remove "at least 8 byte alignment" code from x86
This change drops forced alignment to 8 if requested alignment is higher
than 8: before the patch, -falign-functions=9 was generating
.p2align 4,,8
.p2align 3
which means: "align to 16 if the skip is 8 bytes or less; else align to 8".
After this change, ".p2align 3" is not emitted.
This behavior will be implemented differently by the next patch.
2016-08-30 Denys Vlasenko
* config/i386/freebsd.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Remove "If N
is large, do at least 8 byte alignment" code.
* config/i386/gnu-user.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/iamcu.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/openbsdelf.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/x86-64.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
Index: gcc/config/i386/freebsd.h
===
--- gcc/config/i386/freebsd.h (revision 239390)
+++ gcc/config/i386/freebsd.h (working copy)
@@ -92,25 +92,17 @@ along with GCC; see the file COPYING3. If not see
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
#endif
Index: gcc/config/i386/gnu-user.h
===
--- gcc/config/i386/gnu-user.h (revision 239390)
+++ gcc/config/i386/gnu-user.h (working copy)
@@ -94,24 +94,16 @@ along with GCC; see the file COPYING3. If not see
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
#endif
Index: gcc/config/i386/iamcu.h
===
--- gcc/config/i386/iamcu.h (revision 239390)
+++ gcc/config/i386/iamcu.h (working copy)
@@ -62,23 +62,15 @@ see the files COPYING3 and COPYING.RUNTIME respect
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
Index: gcc/config/i386/openbsdelf.h
===
--- gcc/config/i386/openbsdelf.h(revision 239390)
+++ gcc/config/i386/openbsdelf.h(working copy)
@@ -63,24 +63,16 @@ along with GCC; see the file COPYING3. If not see
/* A C statement to output to the stdio stream FILE an assembler
command to advance the l
[PATCH v3 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]
falign-functions=N is too simplistic. Ingo Molnar ran some tests and it seems that on latest x86 CPUs, 64-byte alignment of functions runs fastest (he tried many other possibilites): this way, after a call CPU can fetch a lot of insns in the first cacheline fill. However, developers are less than thrilled by the idea of a slam-dunk 64-byte aligning everything. Too much waste: On 05/20/2015 02:47 AM, Linus Torvalds wrote: > At the same time, I have to admit that I abhor a 64-byte function > alignment, when we have a fair number of functions that are (much) > smaller than that. > > Is there some way to get gcc to take the size of the function into > account? Because aligning a 16-byte or 32-byte function on a 64-byte > alignment is just criminally nasty and wasteful. This change makes it possible to align functions to 64-byte boundaries *if* this does not introduce huge amount of padding. Example syntax is -falign-functions=64,9: "align to 64 by skipping up to 9 bytes (not inclusive)". IOW: "after a call insn, CPU will always be able to fetch at least 9 bytes of insns". x86 had a tweak: -falign-functions=N with N > 8 was adding secondary alignment. For example, falign-functions=10 was emitting this before every function: .p2align 4,,9 .p2align 3 This tweak was removed by the previous patch. Now it is reinstated by the logic that if falign-functions=N[,M] is specified and N > 8, then default value of N2 is 8, not 0. But now this can be suppressed by falign-functions=N,M,0 - which wasn't possible before. In general, optional N2,M2 pair can be used to generate any secondary alignment user wants. Testing: Tested that with -falign-functions=N (tried 8, 15, 16, 17...) the alignment directives are the same before and after the patch. Tested that -falign-functions=N,N (two equal parameters) works exactly like -falign-functions=N. No change from past behavior: Tested that "-falign-functions" uses an arch-dependent alignment. Tested that "-O2" uses an arch-dependent alignment. Tested that "-O2 -falign-functions=N" uses explicitly given alignment. 2016-08-30 Denys Vlasenko * common.opt (-falign-functions=): Accept a string instead of integer. (-falign-jumps=): Likewise. (-falign-labels=): Likewise. (-falign-loops=): Likewise. * flags.h (struct target_flag_state): Revamp how alignment data is stored: for each of four alignment types, store two pairs of log/maxskip values. * toplev.c (read_uint): New function. (read_log_maxskip): New function. (parse_N_M): New function. (init_alignments): Set align_FOO[0/1].log/maxskip from specified falign-FOO=N[,M[,N[,M]]] options. * varasm.c (assemble_start_function): Call two ASM_OUTPUT_MAX_SKIP_ALIGN macros, first for N,M and second time for N2,M2 from falign-functions=N,M,N2,M2. This generates 0, 1, or 2 align directives. * config/aarch64/aarch64-protos.h (struct tune_params): Change foo_align members from integers to strings. * config/i386/i386.c (struct ptt): Likewise. * config/aarch64/aarch64.c (_tunings): Change foo_align field values from integers to strings. * config/i386/i386.c (processor_target_table): Likewise. * config/arm/arm.c (arm_override_options_after_change_1): Fix if() condition to detect that -falign-functions is specified. * config/i386/i386.c (ix86_default_align): Likewise. * common/config/i386/i386-common.c (ix86_handle_option): Remove conversion of malign_foo's to falign_foo's. The warning is still emitted. * doc/invoke.texi: Update option documentation. * testsuite/gcc.target/i386/falign-functions.c: New file. Index: gcc/common/config/i386/i386-common.c === --- gcc/common/config/i386/i386-common.c(revision 239860) +++ gcc/common/config/i386/i386-common.c(working copy) @@ -998,36 +998,17 @@ ix86_handle_option (struct gcc_options *opts, } return true; - - /* Comes from final.c -- no real reason to change it. */ -#define MAX_CODE_ALIGN 16 - case OPT_malign_loops_: warning_at (loc, 0, "-malign-loops is obsolete, use -falign-loops"); - if (value > MAX_CODE_ALIGN) - error_at (loc, "-malign-loops=%d is not between 0 and %d", - value, MAX_CODE_ALIGN); - else - opts->x_align_loops = 1 << value; return true; case OPT_malign_jumps_: warning_at (loc, 0, "-malign-jumps is obsolete, use -falign-jumps"); - if (value > MAX_CODE_ALIGN) - error_at (loc, "-malign-jumps=%d is not between 0 and %d", - value, MAX_CODE_ALIGN); - else - opts->x_align_jumps = 1 << value; return true; case OPT_malign_functions_: warning_at (loc, 0, "-malign-functions is obsolete, use -falign-functions"); - if (value > MAX_CODE_ALIGN) - error_at
Re: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems
On 30/08/16 17:11, Christophe Lyon wrote: On 18 August 2016 at 11:15, Tamar Christina wrote: Hi all, This fixes a bug in the vector load functions in which they load the vector in the wrong order for big endian systems. This patch flips the order conditionally in the vec_concats. No testcase given because plenty of existing tests for vld functions. Ran regression tests on aarch64_be-none-elf and aarch64-none-elf. Vldx tests now pass on aarch64_be-none-elf and no regressions on both. Before your patch, I can see aarch64/vldN_1.c and aarch64/advsimd-intrinsics/vldX_lane.c failing. Do you know why aarch64/advsimd-intrinsics/vldX.c and vldX_dup are not failing? That's weird, on my clean build and our nightlies I see aarch64/advsimd-intrinsics/vldX.c, aarch64/advsimd-intrinsics/vtbX.c and aarch64/vldN_1.c failing but aarch64/advsimd-intrinsics/vldX_lane.c is passing. I don't know why the vldX_lane was passing, but I'll look into the discrepancy. Cheers, Tamar Thanks Christophe Ok for trunk? I do not have commit rights so if ok can someone apply it for me? Thanks, Tamar gcc/ 2016-08-16 Tamar Christina * gcc/config/aarch64/aarch64-simd.md (aarch64_ld2_dreg_le): New. (aarch64_ld2_dreg_be): New. (aarch64_ld2_dreg): Removed. (aarch64_ld3_dreg_le): New. (aarch64_ld3_dreg_be): New. (aarch64_ld3_dreg): Removed. (aarch64_ld4_dreg_le): New. (aarch64_ld4_dreg_be): New. (aarch64_ld4_dreg): Removed. (aarch64_ld): Wrapper around _le, _be.
Re: [PATCH 3/4] BRIG (HSAIL) frontend: libhsail-rt
Updated patch attached. This one was updated with a new ultra simple fiber implementation that calls ucontext.h API directly instead of requiring Pth in between. Plus cleanups. On Mon, Aug 15, 2016 at 4:22 PM, Pekka Jääskeläinen wrote: > Updated patch attached. > Mostly comment cleanups and renamed the builtins. > > On Mon, May 16, 2016 at 8:26 PM, Pekka Jääskeläinen > wrote: >> The libhsail runtime library. >> >> -- >> Pekka Jääskeläinen >> Parmance 003-brig-fe-libhsail-rt.patch.gz Description: GNU Zip compressed data
Re: Implement -Wimplicit-fallthrough (version 7)
On Tue, Aug 30, 2016 at 10:25 AM, Marek Polacek wrote:
>> > @@ -10585,15 +10610,26 @@ cp_parser_statement (cp_parser* parser, tree
>> > in_statement_expr,
>> > }
>> >/* Look for an expression-statement instead. */
>> >statement = cp_parser_expression_statement (parser,
>> > in_statement_expr);
>> > +
>> > + /* Handle [[fallthrough]];. */
>> > + if (std_attrs != NULL_TREE
>> > + && is_attribute_p ("fallthrough", get_attribute_name (std_attrs))
>> > + && statement == NULL_TREE)
>> > + {
>> > + tree fn = build_call_expr_internal_loc (statement_location,
>> > + IFN_FALLTHROUGH,
>> > + void_type_node, 0);
>>
>> Let's use the 'statement' variable rather than a new variable fn so
>> that we set the call's location below. Let's also warn here about
>> [[fallthrough]] on non-null expression statements.
>
> Done. But I couldn't figure out a testcase where the warning would trigger,
> so
> not sure how important that is.
What happens for
[[fallthrough]] 42;
? What ought to happen is a warning that the attribute only applies
to null statements.
Jason
Re: Ping : [Patch, fortran] PR48298 - [F03] User-Defined Derived-Type IO (DTIO)
On 08/30/2016 08:00 AM, Janne Blomqvist wrote: > On Mon, Aug 29, 2016 at 1:15 PM, Paul Richard Thomas > wrote: >> Hi Janne, Andre, Jerry and All, >> >> I am perfectly happy to adopt Janne's suggestion for >> st_set_(dtio_)nml_var. Do the changes to the IO structures have any >> impact? These are in: >> >> fnode, st_parameter_dt & gfc_unit >> >> I don't think that these should be visible but I want expert opinion >> before making that assumption :-) > > fnode and gfc_unit are private to libgfortran, so these can be changed > in any way you see fit. > > st_parameter_dt, however, is more complex. It is allocated by the > frontend on the stack, with some empty space to be used as scratch > space by the library (the st_parameter_dt.u member). So you're free to > do whatever you want with the private part, as long as you keep the > size of the u.p struct smaller than u.pad (which should happen > automagically unless you have mangled check_st_parameter_dt somehow). > Otherwise I don't think you should go and touch st_parameter_dt. > > This design is somewhat unfortunate, in that it quite severely > restricts the recursion depth of any function potentially doing I/O. > Say, you have a recursive function that writes something if some > condition is met. Any function potentially doing I/O bumps up the > stack by almost 1kB.. I'm thinking a simpler design for handling the > large number of optional I/O specifiers could be something like having > 3 arguments, an array of keys, an array of values, and a scalar > telling how many elements the key/value arrays have. That way only the > specifiers that are actually specified would need to have space > allocated and be transferred. And then instead of a having the > frontend allocate a large scratch space on the stack, the library > could use a TLS variable to look it up. > > But anyway, talk is cheap, and I'm unlikely to have time to implement > the above any time soon... :( > Yes, and others have talked about this. I am not ready to make the plunge there yet either. So, we have touched sr_parameter_dt carefully and not to exceed pad. I have been down this road many times. Jerry
Re: PR35503 - warn for restrict pointer
On 30/08/16 17:34, Prathamesh Kulkarni wrote:
On 30 August 2016 at 20:24, Tom de Vries wrote:
On 26/08/16 13:39, Prathamesh Kulkarni wrote:
Hi,
The following patch adds option -Wrestrict that warns when an argument
is passed to a restrict qualified parameter and it aliases with
another argument.
eg:
int foo (const char *__restrict buf, const char *__restrict fmt, ...);
void f(void)
{
char buf[100] = "hello";
foo (buf, "%s-%s", buf, "world");
}
Does -Wrestrict generate a warning for this example?
...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
int i;
for (i = 0; i < n; i++)
p[i] = q[i] + r[i];
}
h (100, a, b, b)
...
[ Note that this is valid C, and does not violate the restrict definition. ]
If -Wrestrict indeed generates a warning, then we should be explicit in the
documentation that while the warning triggers on this type of example, the
code is correct.
I am afraid it would warn for the above case. The patch just checks if
the parameter is qualified
with restrict, and if the corresponding argument has aliases in the
call (by calling operand_equal_p).
Is such code common in practice ?
[ The example is from the definition of restrict in the c99 standard. ]
According to the definition of restrict, only the restrict on p is
required to know that p doesn't alias with q and that p doesn't alias
with r.
AFAIK the current implementation of gcc already generates optimal code
if restrict is only on p. But earlier versions (and possibly other
compilers as well?) need the restrict on q and r as well.
So I expect this code to occur.
If it is, I wonder if we should keep
the warning in -Wall ?
Hmm, I wonder if we can use the const keyword to silence the warning.
So if we generate a warning for:
...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
int i;
for (i = 0; i < n; i++)
p[i] = q[i] + r[i];
}
h (100, a, b, b)
...
but not for:
...
void h(int n, int * restrict p, const int * restrict q, const int *
restrict r)
{
int i;
for (i = 0; i < n; i++)
p[i] = q[i] + r[i];
}
h (100, a, b, b)
...
Then there's an easy way to rewrite the example such that the warning
doesn't trigger, without having to remove the restrict.
Thanks,
- Tom
Re: PR35503 - warn for restrict pointer
On 30 August 2016 at 22:58, Tom de Vries wrote:
> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
>>
>> On 30 August 2016 at 20:24, Tom de Vries wrote:
>>>
>>> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
Hi,
The following patch adds option -Wrestrict that warns when an argument
is passed to a restrict qualified parameter and it aliases with
another argument.
eg:
int foo (const char *__restrict buf, const char *__restrict fmt, ...);
void f(void)
{
char buf[100] = "hello";
foo (buf, "%s-%s", buf, "world");
}
>>>
>>>
>>>
>>> Does -Wrestrict generate a warning for this example?
>>>
>>> ...
>>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>>> {
>>> int i;
>>> for (i = 0; i < n; i++)
>>> p[i] = q[i] + r[i];
>>> }
>>>
>>> h (100, a, b, b)
>>> ...
>>>
>>> [ Note that this is valid C, and does not violate the restrict
>>> definition. ]
>>>
>>> If -Wrestrict indeed generates a warning, then we should be explicit in
>>> the
>>> documentation that while the warning triggers on this type of example,
>>> the
>>> code is correct.
>>
>> I am afraid it would warn for the above case. The patch just checks if
>> the parameter is qualified
>> with restrict, and if the corresponding argument has aliases in the
>> call (by calling operand_equal_p).
>
>
>> Is such code common in practice ?
>
>
> [ The example is from the definition of restrict in the c99 standard. ]
>
> According to the definition of restrict, only the restrict on p is required
> to know that p doesn't alias with q and that p doesn't alias with r.
>
> AFAIK the current implementation of gcc already generates optimal code if
> restrict is only on p. But earlier versions (and possibly other compilers as
> well?) need the restrict on q and r as well.
>
> So I expect this code to occur.
>
>> If it is, I wonder if we should keep
>> the warning in -Wall ?
>>
>
> Hmm, I wonder if we can use the const keyword to silence the warning.
>
> So if we generate a warning for:
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
> int i;
> for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
>
> but not for:
> ...
> void h(int n, int * restrict p, const int * restrict q, const int * restrict
> r)
> {
> int i;
> for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
>
> Then there's an easy way to rewrite the example such that the warning
> doesn't trigger, without having to remove the restrict.
The attached (untested) patch silences the warning if parameter is
const qualified.
I will give it some testing after resolving a different issue.
Thanks,
Prathamesh
>
> Thanks,
> - Tom
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..4239cef 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "substring-locations.h"
#include "spellcheck.h"
+#include "gcc-rich-location.h"
cpp_reader *parse_in; /* Declared in c-pragma.h. */
@@ -13057,4 +13058,86 @@ diagnose_mismatched_attributes (tree olddecl, tree
newdecl)
return warned;
}
+/* Warn if an argument at position param_pos is passed to a
+ restrict-qualified param, and it aliases with another argument. */
+
+void
+warn_for_restrict (unsigned param_pos, vec *args)
+{
+ tree arg = (*args)[param_pos];
+ if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node, 0))
+return;
+
+ location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
+ gcc_rich_location richloc (loc);
+
+ unsigned i;
+ tree current_arg;
+ auto_vec arg_positions;
+
+ FOR_EACH_VEC_ELT (*args, i, current_arg)
+{
+ if (i == param_pos)
+ continue;
+
+ tree current_arg = (*args)[i];
+ if (operand_equal_p (arg, current_arg, 0))
+ {
+ TREE_VISITED (current_arg) = 1;
+ arg_positions.safe_push (i);
+ }
+}
+
+ if (arg_positions.is_empty ())
+return;
+
+ struct obstack fmt_obstack;
+ gcc_obstack_init (&fmt_obstack);
+ char *fmt = (char *) obstack_alloc (&fmt_obstack, 0);
+
+ char num[32];
+ sprintf (num, "%u", param_pos + 1);
+
+ obstack_grow (&fmt_obstack, "passing argument ",
+ strlen ("passing argument "));
+ obstack_grow (&fmt_obstack, num, strlen (num));
+ obstack_grow (&fmt_obstack,
+ " to restrict-qualified parameter aliases with argument",
+ strlen (" to restrict-qualified parameter "
+ "aliases with argument"));
+
+ /* make argument plural and append space. */
+ if (arg_positions.length () > 1)
+obstack_1grow (&fmt_obstack, 's');
+ obstack_1grow (&fmt_obstack, ' ');
+
+ unsigned pos;
+ unsigned ranges_added = 1;
+
+ FOR_EACH_VEC_ELT (arg_positions, i, pos)
+{
+ /* FIXME: Fow now, only 3 ranges can be added. Remove this once
+the restrictio
Re: [RFC][IPA-VRP] Add support for IPA VRP in ipa-cp/ipa-prop
On 30 August 2016 at 10:50, Kugan Vivekanandarajah
wrote:
> Hi Honza,
>
> Here is a re-based version which also addresses the review comments.
>
> On 21/07/16 22:54, Jan Hubicka wrote:
>>> Maybe it is better to separate value range and alignment summary
>>> writing/reading to different functions. Here is another updated
>>> version which does this.
>>
>> Makes sense to me. Note that the alignment summary propagation can be either
>> handled by doing bitwise constant propagation and/or extending our value
>> ranges
>> by stride (as described in
>> http://www.lighterra.com/papers/valuerangeprop/Patterson1995-ValueRangeProp.pdf
>> I would like it to go eventually away in favour of more generic solution.
>>
>>> -/* If DEST_PLATS already has aggregate items, check that aggs_by_ref
>>> matches
>>> +/* Propagate value range across jump function JFUNC that is associated with
>>> + edge CS and update DEST_PLATS accordingly. */
>>> +
>>> +static bool
>>> +propagate_vr_accross_jump_function (cgraph_edge *cs,
>>> +ipa_jump_func *jfunc,
>>> +struct ipcp_param_lattices *dest_plats)
>>> +{
>>> + struct ipcp_param_lattices *src_lats;
>>> + ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range;
>>> +
>>> + if (dest_lat->bottom_p ())
>>> +return false;
>>> +
>>> + if (jfunc->type == IPA_JF_PASS_THROUGH)
>>> +{
>>> + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
>>> + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
>>> + src_lats = ipa_get_parm_lattices (caller_info, src_idx);
>>> +
>>> + if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
>>> + return dest_lat->meet_with (src_lats->m_value_range);
>>
>> Clearly we can propagate thorugh expressions here (PLUS_EXPR). I have run
>> into similar issue in loop code that builds simple generic expresisons
>> (like (int)ssa_name+10) and it would be nice to have easy way to deterine
>> their value range based on the knowledge of SSA_NAME's valur range.
>>
>> Bit this is fine for initial implementaiotn for sure.
>
> Indeed. I will do this as a follow up.
>
>>>
>>> +/* Look up all VR information that we have discovered and copy it over
>>> + to the transformation summary. */
>>> +
>>> +static void
>>> +ipcp_store_vr_results (void)
>>> +{
>>> + cgraph_node *node;
>>> +
>>> + FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
>>> + {
>>> +ipa_node_params *info = IPA_NODE_REF (node);
>>> +bool found_useful_result = false;
>>> +
>>> +if (!opt_for_fn (node->decl, flag_ipa_vrp))
>>> + {
>>> + if (dump_file)
>>> + fprintf (dump_file, "Not considering %s for VR discovery "
>>> + "and propagate; -fipa-ipa-vrp: disabled.\n",
>>> + node->name ());
>>> + continue;
>>
>> I belive you need to also prevent propagation through functions copmiled with
>> -fno-ipa-vrp, not only prevent any transformations.
>
> Do you mean the following, I was following other implementations.
>
> @@ -2264,6 +2430,11 @@ propagate_constants_accross_call (struct
> cgraph_edge *cs)
> &dest_plats->bits_lattice);
>ret |= propagate_aggs_accross_jump_function (cs, jump_func,
> dest_plats);
> + if (opt_for_fn (callee->decl, flag_ipa_vrp))
> +ret |= propagate_vr_accross_jump_function (cs,
> + jump_func, dest_plats);
> + else
> +ret |= dest_plats->m_value_range.set_to_bottom ();
>
>>> +/* Update value range of formal parameters as described in
>>> + ipcp_transformation_summary. */
>>> +
>>> +static void
>>> +ipcp_update_vr (struct cgraph_node *node)
>>> +{
>>> + tree fndecl = node->decl;
>>> + tree parm = DECL_ARGUMENTS (fndecl);
>>> + tree next_parm = parm;
>>> + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
>>> + if (!ts || vec_safe_length (ts->m_vr) == 0)
>>> +return;
>>> + const vec &vr = *ts->m_vr;
>>> + unsigned count = vr.length ();
>>> +
>>> + for (unsigned i = 0; i < count; ++i, parm = next_parm)
>>> +{
>>> + if (node->clone.combined_args_to_skip
>>> + && bitmap_bit_p (node->clone.combined_args_to_skip, i))
>>> + continue;
>>> + gcc_checking_assert (parm);
>>> + next_parm = DECL_CHAIN (parm);
>>> + tree ddef = ssa_default_def (DECL_STRUCT_FUNCTION (node->decl),
>>> parm);
>>> +
>>> + if (!ddef || !is_gimple_reg (parm))
>>> + continue;
>>> +
>>> + if (cgraph_local_p (node)
>> The test of cgraph_local_p seems redundant here. The analysis phase should
>> not determine
>> anything if function is reachable non-locally.
>
> Removed it.
>
>>> +/* Info about value ranges. */
>>> +
>>> +struct GTY(()) ipa_vr
>>> +{
>>> + /* The data fields below are valid only if known is true. */
>>> + bool known;
>>> + enum value_range_type type;
>>> + tree min;
>>> + tree max;
>> What is the point of representing range as trees rather than wide ints. Can
>> they
>> be non-constant integer?
>
> Changed to wide_int after adding that support.
>
> LTO Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressi
[gomp4] default to runtime gang size
This patch changes the default for gang partitioned size to be determined at
runtime (and thus interrogate the hardware).
The auto-loop test is designed for num_gangs 32. The fortran nested function
test appears to also require that, but my be hiding another defect. Its use of
gang(static:1) seems a little strange and not justified by the comments
discussing its use.
nathan
2016-08-28 Nathan Sidwell
gcc/
* config/nvptx/nvptx.c (PTX_GANG_DEFAULT): Set to zero.
libgomp/
* testsuite/libgomp.oacc-c-c++-common/loop-auto-1.c: Set gang
dimension.
* testsuite/libgomp.oacc-fortran/nested-function-1.f90: Likewise.
Index: gcc/config/nvptx/nvptx.c
===
--- gcc/config/nvptx/nvptx.c (revision 239868)
+++ gcc/config/nvptx/nvptx.c (working copy)
@@ -4157,7 +4157,7 @@ nvptx_expand_builtin (tree exp, rtx targ
/* Define dimension sizes for known hardware. */
#define PTX_VECTOR_LENGTH 32
#define PTX_WORKER_LENGTH 32
-#define PTX_GANG_DEFAULT 32
+#define PTX_GANG_DEFAULT 0 /* Defer to runtime. */
/* Validate compute dimensions of an OpenACC offload or routine, fill
in non-unity defaults. FN_LEVEL indicates the level at which a
Index: libgomp/testsuite/libgomp.oacc-c-c++-common/loop-auto-1.c
===
--- libgomp/testsuite/libgomp.oacc-c-c++-common/loop-auto-1.c (revision 239868)
+++ libgomp/testsuite/libgomp.oacc-c-c++-common/loop-auto-1.c (working copy)
@@ -2,6 +2,8 @@
not optimized away at -O0, and then confuses the target assembler.
{ dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fopenacc-dim=32" } */
+
#include
#include
@@ -151,8 +153,7 @@ int gang_1 (int *ary, int size)
{
clear (ary, size);
-#pragma acc parallel num_gangs (32) num_workers (32) vector_length(32) copy(ary[0:size]) firstprivate (size)
- /* { dg-warning "region is vector partitioned but does not contain vector partitioned code" "vector" { target *-*-* } 154 } */
+#pragma acc parallel num_gangs (32) num_workers (32) vector_length(32) copy(ary[0:size]) firstprivate (size)/* { dg-warning "region is vector partitioned but does not contain vector partitioned code" "vector" { target *-*-* } } */
{
#pragma acc loop auto
for (int jx = 0; jx < size / 64; jx++)
Index: libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90
===
--- libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90 (revision 239868)
+++ libgomp/testsuite/libgomp.oacc-fortran/nested-function-1.f90 (working copy)
@@ -1,6 +1,7 @@
! Exercise nested function decomposition, gcc/tree-nested.c.
! { dg-do run }
+! { dg-additional-options "-fopenacc-dim=32" }
program collapse2
call test1
Re: [MPX] Fix for PR77267
Would something like that count?
I did not do the warning thing, cause the problem only appears when
you provide the -Wl,-as-needed option to the linker.
(In all other cases warning would be redundant). Are we able to check
that on runtime?
diff --git a/gcc/config.in b/gcc/config.in
index fc3321c..a736de3 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1538,6 +1538,12 @@
#endif
+/* Define if your linker supports --push-state/--pop-state */
+#ifndef USED_FOR_TARGET
+#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
+#endif
+
+
/* Define if your linker links a mix of read-only and read-write sections into
a read-write section. */
#ifndef USED_FOR_TARGET
diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
index 4b9910f..6aa195d 100644
--- a/gcc/config/i386/linux-common.h
+++ b/gcc/config/i386/linux-common.h
@@ -79,13 +79,23 @@ along with GCC; see the file COPYING3. If not see
#endif
#endif
+#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
+#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
+#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
+#else
+#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
+#define MPX_LD_AS_NEEDED_GUARD_POP ""
+#endif
+
#ifndef LIBMPX_SPEC
#if defined(HAVE_LD_STATIC_DYNAMIC)
#define LIBMPX_SPEC "\
%{mmpx:%{fcheck-pointer-bounds:\
%{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
%{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
--lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
+" MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpx " MPX_LD_AS_NEEDED_GUARD_POP "\
+%{static-libmpx:--no-whole-archive "\
+LD_DYNAMIC_OPTION \
LIBMPX_LIBS ""
#else
#define LIBMPX_SPEC "\
@@ -99,7 +109,8 @@ along with GCC; see the file COPYING3. If not see
%{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
%{static:-lmpxwrappers}\
%{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
--lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
+" MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpxwrappers "
MPX_LD_AS_NEEDED_GUARD_POP "\
+%{static-libmpxwrappers:--no-whole-archive "\
LD_DYNAMIC_OPTION "}"
#else
#define LIBMPXWRAPPERS_SPEC "\
diff --git a/gcc/configure b/gcc/configure
index 871ed0c..094 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -29609,6 +29609,30 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
$as_echo "$ld_bndplt_support" >&6; }
+# Check linker supports '--push-state'/'--pop-state'
+ld_pushpopstate_support=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
--push-state/--pop-state options" >&5
+$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
+if test x"$ld_is_gold" = xno; then
+ if test $in_tree_ld = yes ; then
+if test "$gcc_cv_gld_major_version" -eq 2 -a
"$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
2; then
+ ld_pushpopstate_support=yes
+fi
+ elif test x$gcc_cv_ld != x; then
+# Check if linker supports --push-state/--pop-state options
+if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
+ ld_pushpopstate_support=yes
+fi
+ fi
+fi
+if test x"$ld_pushpopstate_support" = xyes; then
+
+$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" >&5
+$as_echo "$ld_pushpopstate_support" >&6; }
+
# Configure the subdirectories
# AC_CONFIG_SUBDIRS($subdirs)
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 241e82d..93af766 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -6237,6 +6237,27 @@ if test x"$ld_bndplt_support" = xyes; then
fi
AC_MSG_RESULT($ld_bndplt_support)
+# Check linker supports '--push-state'/'--pop-state'
+ld_pushpopstate_support=no
+AC_MSG_CHECKING(linker --push-state/--pop-state options)
+if test x"$ld_is_gold" = xno; then
+ if test $in_tree_ld = yes ; then
+if test "$gcc_cv_gld_major_version" -eq 2 -a
"$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
2; then
+ ld_pushpopstate_support=yes
+fi
+ elif test x$gcc_cv_ld != x; then
+# Check if linker supports --push-state/--pop-state options
+if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
+ ld_pushpopstate_support=yes
+fi
+ fi
+fi
+if test x"$ld_pushpopstate_support" = xyes; then
+ AC_DEFINE(HAVE_LD_PUSHPOPSTATE_SUPPORT, 1,
+ [Define if your linker supports --push-state/--pop-state])
+fi
+AC_MSG_RESULT($ld_pushpopstate_support)
+
# Configure the subdirectories
# AC_CONFIG_SUBDIRS($subdirs)
2016-08-29 13:09 GMT+03:00 Ilya Enkovich :
> 2016-08-25 12:27 GMT+03:00 Alexander Ivchenko :
>> The attached patched fixes the usage of MPX in presence of
>> "-Wl,-as-needed" option. 'make checked' on MPX-enabled machine.
>>
>> "--push-state" and "--pop-state" are not supported by gold at the
>> moment. But that's OK because using MPX with gold only recommended in
>> stat
Re: [MPX] Fix for PR77267
On Tue, Aug 30, 2016 at 11:53 AM, Alexander Ivchenko wrote: > Would something like that count? > > I did not do the warning thing, cause the problem only appears when > you provide the -Wl,-as-needed option to the linker. > (In all other cases warning would be redundant). Are we able to check > that on runtime? > > Is this possible to add -lmpx after -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed -- H.J.
Re: [RFC][IPA-VRP] Add support for IPA VRP in ipa-cp/ipa-prop
Hi, Just noticed a small nit - why not reuse ipa_vr in ipa_jump_func instead of adding vr_known and m_vr ? This is because we want to reuse vrp_intersect_ranges and vrp_meet which are not trivial. On the other hand, in ipa_jump_func, since we stream the data we have a simpler data structure with wide_int. Thanks, Kugan
Re: [PATCH] C++: add fixit for '>>' template error
OK.
On Tue, Aug 30, 2016 at 10:43 AM, David Malcolm wrote:
> This patch adds fix-it hints to our warning for >>
> within a nested template argument list (for -std=c++98).
>
> For example:
>
> double-greater-than-fixit.C:5:12: error: ‘>>’ should be ‘> >’ within a nested
> template argument list
> foo> i;
> ^~
> > >
>
> In conjunction with the not-yet-in-trunk -fdiagnostics-generate-patch,
> this can generate patches like this:
>
> --- double-greater-than-fixit.C
> +++ double-greater-than-fixit.C
> @@ -4,3 +4,3 @@
>
> -foo> i;
> +foo > i;
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> * parser.c (cp_parser_enclosed_template_argument_list): Add fix-it
> hint to ">>" within nested template argument list error.
>
> gcc/testsuite/ChangeLog:
> * g++.dg/template/double-greater-than-fixit.C: New test case.
> ---
> gcc/cp/parser.c | 6 --
> gcc/testsuite/g++.dg/template/double-greater-than-fixit.C | 10 ++
> 2 files changed, 14 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48dbca1..ca9f8b9 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -26342,8 +26342,10 @@ cp_parser_enclosed_template_argument_list
> (cp_parser* parser)
> global source location is still on the token before the
> '>>', so we need to say explicitly where we want it. */
> cp_token *token = cp_lexer_peek_token (parser->lexer);
> - error_at (token->location, "%<>>%> should be %<> >%> "
> - "within a nested template argument list");
> + gcc_rich_location richloc (token->location);
> + richloc.add_fixit_replace ("> >");
> + error_at_rich_loc (&richloc, "%<>>%> should be %<> >%> "
> +"within a nested template argument list");
>
> token->type = CPP_GREATER;
> }
> diff --git a/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
> b/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
> new file mode 100644
> index 000..f0de4ec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/double-greater-than-fixit.C
> @@ -0,0 +1,10 @@
> +/* { dg-options "-fdiagnostics-show-caret -std=c++98" } */
> +template
> +struct foo {};
> +
> +foo> i; // { dg-error "12: .>>. should be .> >. within a nested
> template argument list" }
> +/* { dg-begin-multiline-output "" }
> + foo> i;
> +^~
> +> >
> + { dg-end-multiline-output "" } */
> --
> 1.8.5.3
>
Fwd: libgo patch committed: Use -fgo-c-header to share between Go and C
Resending without the attachment as it was blocked as spam. The actual change be seen in Subversion or at https://groups.google.com/forum/?pli=1#!topic/gofrontend-dev/fT2-PlvVZ9o . Ian -- Forwarded message -- From: Ian Lance Taylor Date: Tue, Aug 30, 2016 at 2:07 PM Subject: libgo patch committed: Use -fgo-c-header to share between Go and C To: gcc-patches , "gofrontend-...@googlegroups.com" This patch to libgo uses the new -fgo-c-header option to share the definitions of data structures and constants between Go and C. Data structures are defined in new files in libgo/go/runtime. The -fgo-c-header option is used to generate a C header file. That header file is #include'd by libgo/runtime/runtime.h and used when building the code in libgo/runtime. The C versions of the data structures are removed. This is a step toward converting much of the libgo runtime to Go without breaking things as we go. Putting the structs in Go means using the naming conventions of the Go runtime package, so many of the constants pick up a leading underscore and all other underscores are dropped. The C code is adjusted accordingly. This also picks up a change made a while back to the gc runtime: instead of using two different thread local variables, g and m, the m variable is removed and the value is now always accessed as g->m. This required a few additions to the C code to make sure that g->m is always set correctly. This change also passes the new -fgo-compiling-runtime option when compiling the runtime package, although the option doesn't do anything yet. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu and sparc-sun-solaris11. Committed to mainline. Ian
[gomp4] fix an ICE involving assumed-size arrays
Usually a data clause would would have OMP_CLAUSE_SIZE set, but not all do. In the latter case, lower_omp_target falls back to using size of the type of the variable specified in the data clause. However, in the case of assumed-size arrays, the size of the type may be NULL because its undefined. My fix for this solution is to set the size to one byte if the size of the type is NULL. This solution at least allows the runtime the opportunity to remap any data already present on the accelerator. However, if the data isn't present on the accelerator, this will likely result in some sort of segmentation fault on the accelerator. The OpenACC spec is not clear how the compiler should handle assumed-sized arrays when the user does not provide an explicit data clause with a proper subarray. It was tempting to make such implicit variables errors, but arguably that would affect usability. Perhaps I should a warning for implicitly used assumed-sizes arrays? I've applied this patch to gomp-4_0-branch. It looks like OpenMP has a similar problem. Cesar 2016-08-30 Cesar Philippidis gcc/ * omp-low.c (lower_omp_target): Handle NULL-sized types for assumed-sized arrays. libgomp/ * testsuite/libgomp.oacc-fortran/assumed-size.f90: New test. diff --git a/gcc/omp-low.c b/gcc/omp-low.c index b314523..0faf6c3 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -16534,6 +16534,12 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) s = OMP_CLAUSE_SIZE (c); if (s == NULL_TREE) s = TYPE_SIZE_UNIT (TREE_TYPE (ovar)); + /* Fortran assumed-size arrays have zero size because the + type is incomplete. Set the size to one to allow the + runtime to remap any existing data that is already + present on the accelerator. */ + if (s == NULL_TREE) + s = integer_one_node; s = fold_convert (size_type_node, s); purpose = size_int (map_idx++); CONSTRUCTOR_APPEND_ELT (vsize, purpose, s); diff --git a/libgomp/testsuite/libgomp.oacc-fortran/assumed-size.f90 b/libgomp/testsuite/libgomp.oacc-fortran/assumed-size.f90 new file mode 100644 index 000..79de675 --- /dev/null +++ b/libgomp/testsuite/libgomp.oacc-fortran/assumed-size.f90 @@ -0,0 +1,31 @@ +! Test if implicitly determined data clauses work with an +! assumed-sized array variable. Note that the array variable, 'a', +! has been explicitly copied in and out via acc enter data and acc +! exit data, respectively. + +program test + implicit none + + integer, parameter :: n = 100 + integer a(n), i + + call dtest (a, n) + + do i = 1, n + if (a(i) /= i) call abort + end do +end program test + +subroutine dtest (a, n) + integer i, n + integer a(*) + + !$acc enter data copyin(a(1:n)) + + !$acc parallel loop + do i = 1, n + a(i) = i + end do + + !$acc exit data copyout(a(1:n)) +end subroutine dtest
Re: PR35503 - warn for restrict pointer (-Wrestrict)
On Aug 30, 2016, at 4:57 AM, Prathamesh Kulkarni wrote: > > On 30 August 2016 at 17:11, Eric Gallager wrote: >> On 8/29/16, Jason Merrill wrote: >>> On Mon, Aug 29, 2016 at 10:28 AM, Marek Polacek wrote: On Mon, Aug 29, 2016 at 09:20:53AM -0400, Eric Gallager wrote: > I tried this patch on my fork of gdb-binutils and got a few warnings > from it. Would it be possible to have the caret point to the argument > mentioned, instead of the function name? And also print the option > name? E.g., instead of the current: > > or32-opc.c: In function ‘or32_print_register’: > or32-opc.c:956:3: warning: passing argument 1 to restrict qualified > parameter aliases with argument 3 > sprintf (disassembled, "%sr%d", disassembled, regnum); > ^~~ > > could it look like: > > or32-opc.c: In function ‘or32_print_register’: > or32-opc.c:956:3: warning: passing argument 1 to restrict qualified > parameter aliases with argument 3 [-Wrestrict] > sprintf (disassembled, "%sr%d", disassembled, regnum); >^~~~ > > instead? I didn't try to implement it, but I think this should be fairly easy to achieve in the C FE, because c_parser_postfix_expression_after_primary has arg_loc, which is a vector of parameter locations. >>> >>> The C++ FE doesn't have this currently, but it could be added without >>> too much trouble: in cp_parser_parenthesized_expression_list, extract >>> the locations from the cp_expr return value of >>> cp_parser_assignment_expression, and then pass the locations back up >>> to cp_parser_postfix_expression. >>> >>> Jason >>> >> >> >> On the topic of how to get this warning working with various >> frontends, is there any reason why the Objective C frontend doesn't >> handle -Wrestrict? Currently when trying to use it, it just says: >> >> cc1obj: warning: command line option '-Wrestrict' is valid for C/C++ >> but not for ObjC > Hi Eric, > I am not sure if restrict is valid for ObjC/Obj-C++ and hence didn't > add the option for these front-ends. > If it is valid, I will enable the option for ObjC and Obj-C++. This is wrong, C/C++ options should always be ObjC/ObjC++ options.
Re: [PR59319] output friends in debug info
On Aug 26, 2016, Jason Merrill wrote: > I suppose we can teach dwz to use the maximal set of friends... *nod* > This seems unnecessary; there is no semantic difference for a > particular specialization depending on whether it became a friend > directly or from its template. 'k, I took that out. If we change our minds, it's easy enough to put it back in. > But looking more closely I see that for functions, it is only > maintained before the function template is defined. That should be > simple enough to change. Done. >> - I haven't used local_specializations, should I? I was a bit >> confused about the apparently unused local_specialization_stack, >> too. > No, local_specializations is just for function-local decls. Thanks. Here's the updated patch. Handling non-template friends is kind of easy, but it required a bit of infrastructure in dwarf2out to avoid (i) forcing debug info for unused types or functions: DW_TAG_friend DIEs are only emitted if their DW_AT_friend DIE is emitted, and (ii) creating DIEs for such types or functions just to have them discarded at the end. To this end, I introduced a list (vec, actually) of types with friends, processed at the end of the translation unit, and a list of DW_TAG_friend DIEs that, when we're pruning unused types, reference DIEs that are still not known to be used, revisited after we finish deciding all other DIEs, so that we prune DIEs that would have referenced pruned types or functions. Handling template friends turned out to be trickier: there's no representation in DWARF for templates. I decided to give debuggers as much information as possible, enumerating all specializations of friend templates and outputting DW_TAG_friend DIEs referencing them as well. I considered marking those as DW_AT_artificial, to indicate they're not explicitly stated in the source code, but in the end we decided that was not useful. The greatest challenge was to enumerate all specializations of a template. It looked trivial at first, given DECL_TEMPLATE_INSTANTIATIONS, but it won't list specializations of class-scoped functions and of nested templates. For other templates, I ended up writing code to look for specializations in the hashtables of decl or type specializations. That's not exactly efficient, but it gets the job done. for gcc/ChangeLog PR debug/59319 * dwarf2out.c (class_types_with_friends): New. (gen_friend_tags_for_type, gen_friend_tags): New. (gen_member_die): Record class types with friends. (deferred_marks): New. (prune_unused_types_defer_undecided_mark_p): New. (prune_unused_types_defer_mark): New. (prune_unused_types_deferred_walk): New. (prune_unused_types_walk): Defer DW_TAG_friend. (prune_unused_types): Check deferred marks is empty on entry, empty it after processing. (dwarf2out_finish): Generate friend tags. (dwarf2out_early_finish): Likewise. * langhooks-def.h (LANG_HOOKS_GET_FRIENDS): New. (LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it. * langhooks.h (lang_hooks_for_types): Add get_friends. for gcc/cp/ChangeLog PR debug/59319 * cp-objcp-common.c (cp_get_friends): New. * cp-objcp-common.h (cp_get_friends): Declare. (LANG_HOOKS_GET_FRIENDS): Override. * cp-tree.h (enumerate_friend_specializations): Declare. * pt.c (optimize_friend_specialization_lookup_p): New. (retrieve_friend_specialization): New. (enumerate_friend_specializations): New. (register_specialization): Update DECL_TEMPLATE_INSTANTIATIONS for functions, even after definition, if we are emitting debug info. for gcc/testsuite/ChangeLog PR debug/59319 * g++.dg/debug/dwarf2/friend-1.C: New. * g++.dg/debug/dwarf2/friend-2.C: New. * g++.dg/debug/dwarf2/friend-3.C: New. * g++.dg/debug/dwarf2/friend-4.C: New. * g++.dg/debug/dwarf2/friend-5.C: New. * g++.dg/debug/dwarf2/friend-6.C: New. * g++.dg/debug/dwarf2/friend-7.C: New. * g++.dg/debug/dwarf2/friend-8.C: New. * g++.dg/debug/dwarf2/friend-9.C: New. * g++.dg/debug/dwarf2/friend-10.C: New. * g++.dg/debug/dwarf2/friend-11.C: New. * g++.dg/debug/dwarf2/friend-12.C: New. * g++.dg/debug/dwarf2/friend-13.C: New. * g++.dg/debug/dwarf2/friend-14.C: New. * g++.dg/debug/dwarf2/friend-15.C: New. * g++.dg/debug/dwarf2/friend-16.C: New. * g++.dg/debug/dwarf2/friend-17.C: New. * g++.dg/debug/dwarf2/friend-18.C: New. --- gcc/cp/cp-objcp-common.c | 106 ++ gcc/cp/cp-objcp-common.h |3 gcc/cp/cp-tree.h |1 gcc/cp/pt.c | 194 + gcc/dwarf2out.c | 165 + gcc/langhooks-def.h
Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
On Aug 24, 2016, Richard Biener wrote: > The issue with LTO / early debug is that we can't prune DIEs that refer > to optimized out entities from early debug output thus we have to create > those DIEs late only (in the above case, as tmpl_value_param_die_table > is not streamed, not at all for LTO). I don't fully understand the problem you describe, but I wonder if the infrastructure I wrote for the PR59319 patch, to revisit DIEs and prune them if they reference other pruned DIEs, would help. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
[PATCH] selftest.c: avoid explicit "selftest::" qualifiers
On Mon, 2016-08-29 at 23:53 +0200, Bernd Schmidt wrote:
> On 08/25/2016 03:13 AM, David Malcolm wrote:
> > Split out a new base class for temp_source_file, named_temp_file,
> > moving the deletion to the base class dtor, so that we can write
> > out temporary files in other ways in selftests.
> >
> > gcc/ChangeLog:
> > * selftest.c (selftest::named_temp_file::named_temp_file): New
> > ctor.
> > (selftest::temp_source_file::~temp_source_file): Move to...
> > (selftest::named_temp_file::~named_temp_file): ...here.
> > (selftest::test_named_temp_file): New function.
> > (selftest::selftest_c_tests): Call test_named_temp_file.
> > * selftest.h (class named_temp_file): New class.
> > (class temp_source_file): Convert to a subclass of
> > named_temp_file.
>
> Ok.
>
> > +selftest::named_temp_file::named_temp_file (const char *suffix)
>
> Any reason these aren't inside namespace selftest to shorten these
> declarations?
I was being consistent with the rest of the file, but I no longer
remember why I used explicit namespace selftest:: prefixes there.
The following follow-up patch removes them, moving the
"namespace selftest {" to the top of the file so it covers
everything.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/ChangeLog:
* selftest.c: Move "namespace selftest {" to top of file,
removing explicit "selftest::" qualifiers throughout.
---
gcc/selftest.c | 78 +++---
1 file changed, 36 insertions(+), 42 deletions(-)
diff --git a/gcc/selftest.c b/gcc/selftest.c
index e6c9510..69d9931 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -24,12 +24,14 @@ along with GCC; see the file COPYING3. If not see
#if CHECKING_P
-int selftest::num_passes;
+namespace selftest {
+
+int num_passes;
/* Record the successful outcome of some aspect of a test. */
void
-selftest::pass (const location &/*loc*/, const char */*msg*/)
+pass (const location &/*loc*/, const char */*msg*/)
{
num_passes++;
}
@@ -37,7 +39,7 @@ selftest::pass (const location &/*loc*/, const char */*msg*/)
/* Report the failed outcome of some aspect of a test and abort. */
void
-selftest::fail (const location &loc, const char *msg)
+fail (const location &loc, const char *msg)
{
fprintf (stderr,"%s:%i: %s: FAIL: %s\n", loc.m_file, loc.m_line,
loc.m_function, msg);
@@ -47,7 +49,7 @@ selftest::fail (const location &loc, const char *msg)
/* As "fail", but using printf-style formatted output. */
void
-selftest::fail_formatted (const location &loc, const char *fmt, ...)
+fail_formatted (const location &loc, const char *fmt, ...)
{
va_list ap;
@@ -65,26 +67,23 @@ selftest::fail_formatted (const location &loc, const char
*fmt, ...)
to be non-NULL; fail gracefully if either are NULL. */
void
-selftest::assert_streq (const location &loc,
- const char *desc_expected, const char *desc_actual,
- const char *val_expected, const char *val_actual)
+assert_streq (const location &loc,
+ const char *desc_expected, const char *desc_actual,
+ const char *val_expected, const char *val_actual)
{
/* If val_expected is NULL, the test is buggy. Fail gracefully. */
if (val_expected == NULL)
-::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=NULL",
-desc_expected, desc_actual);
+fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=NULL",
+ desc_expected, desc_actual);
/* If val_actual is NULL, fail with a custom error message. */
if (val_actual == NULL)
-::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=NULL",
-desc_expected, desc_actual, val_expected);
+fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=NULL",
+ desc_expected, desc_actual, val_expected);
if (0 == strcmp (val_expected, val_actual))
-::selftest::pass (loc, "ASSERT_STREQ");
+pass (loc, "ASSERT_STREQ");
else
-::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
-desc_expected, desc_actual, val_expected, val_actual);
+fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
+ desc_expected, desc_actual, val_expected, val_actual);
}
/* Implementation detail of ASSERT_STR_CONTAINS.
@@ -93,36 +92,35 @@ selftest::assert_streq (const location &loc,
::selftest::fail if it is not found. */
void
-selftest::assert_str_contains (const location &loc,
- const char *desc_haystack,
- const char *desc_needle,
- const char *val_haystack,
- const char *val_needle)
+assert_str_contains (const location &loc,
+const char *desc_haystack,
+const char *des
[committed] Remove arbitrary limits from rich_location
This patch eliminates the hard-coded limits within rich_location
(up to 3 ranges, up to 2 fixits). The common case is still
handled by embedding the values inside rich_location - it only
uses dynamic allocation if these limits are exceeded, so
creation of rich_location instances on the stack should still
be fast. This is implemented via a new container class,
semi_embedded_vec .
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Successful stage 1 selftest on powerpc-ibm-aix7.1.3.0.
Committed to trunk as r239879.
gcc/ChangeLog:
* diagnostic-show-locus.c (colorizer::begin_state): Support more
than 3 ranges per diagnostic by alternating between color 1 and
color 2.
(layout::layout): Replace use of rich_location::MAX_RANGES
with richloc->get_num_locations ().
(layout::calculate_line_spans): Replace use of
rich_location::MAX_RANGES with m_layout_ranges.length ().
(layout::print_annotation_line): Handle arbitrary numbers of
ranges in caret-printing by defaulting to '^'.
(selftest::test_one_liner_many_fixits): New function.
(test_diagnostic_show_locus_one_liner): Call it.
* diagnostic.c (diagnostic_initialize): Update for renaming
of rich_location::MAX_RANGES to
rich_location::STATICALLY_ALLOCATED_RANGES.
* diagnostic.h (struct diagnostic_context): Likewise.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-show-locus-bw.c
(test_many_nested_locations): New function.
* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
(test_show_locus): Handle "test_many_nested_locations".
libcpp/ChangeLog:
* include/line-map.h (class semi_embedded_vec): New class.
(semi_embedded_vec::semi_embedded_vec): New ctor.
(semi_embedded_vec::~semi_embedded_vec): New
dtor.
(semi_embedded_vec::operator[]): New methods.
(semi_embedded_vec::push): New method.
(semi_embedded_vec::truncate): New method.
(rich_location::get_num_locations): Reimplement in terms of
m_ranges.
(rich_location::get_range): Make non-inline.
(rich_location::get_num_fixit_hints): Reimplement in terms of
m_fixit_hints.
(rich_location::add_fixit): New function.
(rich_location::MAX_RANGES): Rename to...
(rich_location::STATICALLY_ALLOCATED_RANGES): ...this.
(rich_location::MAX_FIXIT_HINTS): Rename to...
(rich_location::STATICALLY_ALLOCATED_RANGES): ...this, and make
private.
(rich_location::m_num_ranges): Eliminate in favor of...
(rich_location::m_ranges): ...this, converting from a fixed-size
array to a semi_embedded_vec.
(rich_location::m_num_fixit_hints): Eliminate in favor of...
(rich_location::m_fixit_hints): ...this, converting from a
fixed-size array to a semi_embedded_vec.
* line-map.c (rich_location::rich_location): Update for above
changes.
(rich_location::~rich_location): Likewise.
(rich_location::get_loc): Likewise.
(rich_location::get_range): New methods.
(rich_location::add_range): Update for above changes.
(rich_location::set_range): Likewise.
(rich_location::add_fixit_insert): Likewise.
(rich_location::add_fixit_replace): Likewise.
(rich_location::get_last_fixit_hint): Likewise.
(rich_location::reject_impossible_fixit): Likewise.
(rich_location::add_fixit): New method.
---
gcc/diagnostic-show-locus.c| 64 -
gcc/diagnostic.c | 2 +-
gcc/diagnostic.h | 2 +-
.../gcc.dg/plugin/diagnostic-test-show-locus-bw.c | 44 +++
.../plugin/diagnostic_plugin_test_show_locus.c | 53
libcpp/include/line-map.h | 145 +++--
libcpp/line-map.c | 92 +++--
7 files changed, 341 insertions(+), 61 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 60f6820..a22a660 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -317,8 +317,12 @@ colorizer::begin_state (int state)
break;
default:
- /* We don't expect more than 3 ranges per diagnostic. */
- gcc_unreachable ();
+ /* For ranges beyond 2, alternate between color 1 and color 2. */
+ {
+ gcc_assert (state > 2);
+ pp_string (m_context->printer,
+ state % 2 ? m_range1 : m_range2);
+ }
break;
}
}
@@ -720,8 +724,8 @@ layout::layout (diagnostic_context * context,
m_exploc (richloc->get_expanded_location (0)),
m_colorizer (context, diagnostic_kind),
m_colorize_source_p (context->colorize_source_p),
- m_layout_ranges (rich_location::MAX_RANGES),
- m_line_spans (1 + rich_location::MAX_RANGES),
+ m_layout_ranges (richloc->get_nu
Re: PR35503 - warn for restrict pointer
On Tue, 2016-08-30 at 23:26 +0530, Prathamesh Kulkarni wrote: [...] > The attached (untested) patch silences the warning if parameter is > const qualified. > I will give it some testing after resolving a different issue. I've now removed the hard-coded limit of 3 ranges per rich_location, so you should now be able to add an arbitrary number of underlined ranges to the diagnostic (r239879 removes the limit). Dave
[PATCH, rs6000] Fix PR72827 (ada bootstrap failure)
Hi,
The ada bootstrap failure reported in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72827
occurs because of a latent bug in the powerpc back end. The immediate cause is
dead store
elimination removing two stores relative to the frame pointer that are not
dead; however,
DSE is tricked into doing this because we have temporarily adjusted the frame
pointer prior
to performing the loads. DSE relies on the frame pointer remaining constant to
be able to
infer stack stores that are dead.
The code responsible is in rs6000_split_multireg_move, which wants to form new
addresses
after reload/lra to use for the individual stores, but has to use only existing
hard
registers. Currently that code always modifies the register holding the base
pointer,
which is a bad idea when the base pointer is a register expected to remain
fixed. This
patch identifies those situations and causes the index register to be modified
instead.
(If the index register number is zero, indicating constant zero, we fall
through to the
existing code; it's ok to add zero to the fixed registers...)
The diffs are a little munged due to indentation and line length adjustments;
the change
is just to add the code block labeled with commentary.
We might someday want to rip up this logic and instead change secondary reload
to allocate
an extra register to be used during expansion. But to clear the bootstrap
failure, a
simple patch like this seems best.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with the usual
languages plus ada,
with no regressions. Gnattools now builds properly during bootstrap.
Is this ok for trunk, and eventually for backport to the 5 and 6 branches?
Thanks,
Bill
2016-08-30 Bill Schmidt
PR target/72827
* config/rs6000/rs6000.c (rs6000_split_multireg_move): Never
modify the frame pointer or the stack pointer; instead, use
the index register for temporary address generation.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 239871)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -24506,15 +24506,31 @@ rs6000_split_multireg_move (rtx dst, rtx src)
&& REG_P (basereg)
&& REG_P (offsetreg)
&& REGNO (basereg) != REGNO (offsetreg));
- if (REGNO (basereg) == 0)
+ /* We mustn't modify the stack pointer or frame pointer
+as this will confuse dead store elimination. */
+ if ((REGNO (basereg) == STACK_POINTER_REGNUM
+ || REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
+ && REGNO (offsetreg) != 0)
{
- rtx tmp = offsetreg;
- offsetreg = basereg;
- basereg = tmp;
+ emit_insn (gen_add3_insn (offsetreg, basereg,
+ offsetreg));
+ restore_basereg = gen_sub3_insn (offsetreg, offsetreg,
+ basereg);
+ dst = replace_equiv_address (dst, offsetreg);
}
- emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
- restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg);
- dst = replace_equiv_address (dst, basereg);
+ else
+ {
+ if (REGNO (basereg) == 0)
+ {
+ rtx tmp = offsetreg;
+ offsetreg = basereg;
+ basereg = tmp;
+ }
+ emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
+ restore_basereg = gen_sub3_insn (basereg, basereg,
+ offsetreg);
+ dst = replace_equiv_address (dst, basereg);
+ }
}
}
else if (GET_CODE (XEXP (dst, 0)) != LO_SUM)
[patch, libgfortran] PR77393 [7 Regression] Revision r237735 changed the behavior of F0.0
Hi all,
The attached patch fixes the problem by adding a new helper function to
determine the buffer size needed for F0 editing depending on the kind. In this
new function there are some constants presented which document the limits needed
for each kind type.
As can be seen, the required buffers are fixed on stack at 256 bytes which will
handle almost all cases unless a user is doing something with unusually wide
formats. The buffer is malloc'ed if a larger size is needed.
I have not changed the buffering mechanism, only the method of determining the
needed size.
Regression tested on x86-linux. New test case provided.
OK for trunk?
Regards,
Jerry
2016-08-31 Jerry DeLisle
PR libgfortran/77393
* io/write.c (kind_from_size): New function to calculate required buffer
size based on kind type. (select_buffer, select_string): Use new
function. (write_float_0, write_real, write_real_g0, write_complex):
Adjust calls to pass parameters needed by new function.
diff --git a/libgfortran/io/write.c b/libgfortran/io/write.c
index db27f2d..0e4ce0b 100644
--- a/libgfortran/io/write.c
+++ b/libgfortran/io/write.c
@@ -1357,11 +1357,52 @@ get_precision (st_parameter_dt *dtp, const fnode *f, const char *source, int kin
return determine_en_precision (dtp, f, source, kind);
}
+/* 4932 is the maximum exponent of long double and quad precision, 3
+ extra characters for the sign, the decimal point, and the
+ trailing null. Extra digits are added by the calling functions for
+ requested precision. Likewise for float and double. F0 editing produces
+ full precision output. */
+static int
+size_from_kind (st_parameter_dt *dtp, const fnode *f, int kind)
+{
+ int size;
+
+ if (f->format == FMT_F && f->u.real.w == 0)
+{
+ switch (kind)
+ {
+ case 4:
+ size = 38 + 3; /* These constants shown for clarity. */
+ break;
+ case 8:
+ size = 308 + 3;
+ break;
+ case 10:
+ size = 4932 + 3;
+ break;
+ case 16:
+ size = 4932 + 3;
+ break;
+ default:
+ internal_error (&dtp->common, "bad real kind");
+ break;
+ }
+}
+ else
+size = f->u.real.w + 1; /* One byte for a NULL character. */
+
+ return size;
+}
+
static char *
-select_buffer (int precision, char *buf, size_t *size)
+select_buffer (st_parameter_dt *dtp, const fnode *f, int precision,
+ char *buf, size_t *size, int kind)
{
char *result;
- *size = BUF_STACK_SZ / 2 + precision;
+
+ /* The buffer needs at least one more byte to allow room for normalizing. */
+ *size = size_from_kind (dtp, f, kind) + precision + 1;
+
if (*size > BUF_STACK_SZ)
result = xmalloc (*size);
else
@@ -1370,10 +1411,11 @@ select_buffer (int precision, char *buf, size_t *size)
}
static char *
-select_string (const fnode *f, char *buf, size_t *size)
+select_string (st_parameter_dt *dtp, const fnode *f, char *buf, size_t *size,
+ int kind)
{
char *result;
- *size = f->u.real.w + 1;
+ *size = size_from_kind (dtp, f, kind) + f->u.real.d;
if (*size > BUF_STACK_SZ)
result = xmalloc (*size);
else
@@ -1397,6 +1439,7 @@ write_float_string (st_parameter_dt *dtp, char *fstr, size_t len)
memcpy (p, fstr, len);
}
+
static void
write_float_0 (st_parameter_dt *dtp, const fnode *f, const char *source, int kind)
{
@@ -1409,9 +1452,9 @@ write_float_0 (st_parameter_dt *dtp, const fnode *f, const char *source, int kin
int precision = get_precision (dtp, f, source, kind);
/* String buffer to hold final result. */
- result = select_string (f, str_buf, &res_len);
+ result = select_string (dtp, f, str_buf, &res_len, kind);
- buffer = select_buffer (precision, buf_stack, &buf_size);
+ buffer = select_buffer (dtp, f, precision, buf_stack, &buf_size, kind);
get_float_string (dtp, f, source , kind, 0, buffer,
precision, buf_size, result, &res_len);
@@ -1527,10 +1570,10 @@ write_real (st_parameter_dt *dtp, const char *source, int kind)
int precision = get_precision (dtp, &f, source, kind);
/* String buffer to hold final result. */
- result = select_string (&f, str_buf, &res_len);
+ result = select_string (dtp, &f, str_buf, &res_len, kind);
- /* scratch buffer to hold final result. */
- buffer = select_buffer (precision, buf_stack, &buf_size);
+ /* Scratch buffer to hold final result. */
+ buffer = select_buffer (dtp, &f, precision, buf_stack, &buf_size, kind);
get_float_string (dtp, &f, source , kind, 1, buffer,
precision, buf_size, result, &res_len);
@@ -1572,9 +1615,9 @@ write_real_g0 (st_parameter_dt *dtp, const char *source, int kind, int d)
int precision = get_precision (dtp, &f, source, kind);
/* String buffer to hold final result. */
- result = select_string (&f, str_buf, &res_len);
+ result = select_string (dtp, &f, str_buf, &res_len, kind);
- buffer = select_buffer (precision, buf_stack, &buf_size);
+ buffer = select_buffer (dtp,
[PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem
PR77289 exposes a latent problem with LRA constraint matching. In the buggy
test cases, LRA performs a speculative register elimination before checking
operands for matching constraints. With the elimination, the operands
appear to match. However, when we call check_rtl() which attempts to
recognize the instruction, the reg above it not eliminated leading to a
"insn does not satisfy its constraints" ICE.
This patch fixes the problem by not performing register elimination during
constraint matching. operands_match_p() uses get_hard_reg() to grab a
REG's hard reg number, so I have removed get_hard_reg()'s call to
get_final_hard_regno() which performs the register elimination, which
fixes the bug. uses_hard_regs_p() is the other caller of get_hard_regno()
and it still needs register elimination to be called, so I have changed
it to call get_final_hard_regno() instead. Since uses_hard_regs_p()
may call get_final_hard_regno() with a pseudo, I have added support
for mapping those to hard reg numbers before performing the register
elimination.
This has passed bootstrap and regtesting with no regressions.
Ok for mainline?
Peter
gcc/
PR rtl-optimization/77289
* lra-constraints.c (get_final_hard_regno): Add support for non hard
register numbers. Remove support for subregs.
(get_hard_regno): Use SUBREG_P. Don't call get_final_hard_regno().
(get_reg_class): Delete removed get_final_hard_regno() argument.
(uses_hard_regs_p): Call get_final_hard_regno().
gcc/testsuite/
PR rtl-optimization/77289
* gcc.target/powerpc/pr77289.c: New test.
Index: gcc/lra-constraints.c
===
--- gcc/lra-constraints.c (revision 239866)
+++ gcc/lra-constraints.c (working copy)
@@ -182,21 +182,22 @@ get_try_hard_regno (int regno)
return ira_class_hard_regs[rclass][0];
}
-/* Return final hard regno (plus offset) which will be after
- elimination. We do this for matching constraints because the final
- hard regno could have a different class. */
+/* Return the final hard regno which will be after elimination.
+ We do this because the final hard regno could have a different class. */
static int
-get_final_hard_regno (int hard_regno, int offset)
+get_final_hard_regno (int regno)
{
- if (hard_regno < 0)
-return hard_regno;
- hard_regno = lra_get_elimination_hard_regno (hard_regno);
- return hard_regno + offset;
+ if (! HARD_REGISTER_NUM_P (regno))
+regno = lra_get_regno_hard_regno (regno);
+ if (regno < 0)
+return regno;
+ return lra_get_elimination_hard_regno (regno);
}
-/* Return hard regno of X after removing subreg and making
- elimination. If X is not a register or subreg of register, return
- -1. For pseudo use its assignment. */
+/* Return the hard regno of X after removing its subreg. If X is not
+ a register or a subreg of a register, return -1. If X is a pseudo,
+ use its assignment. We do not process register eliminiations while
+ matching constraints. See PR77289. */
static int
get_hard_regno (rtx x)
{
@@ -204,19 +205,19 @@ get_hard_regno (rtx x)
int offset, hard_regno;
reg = x;
- if (GET_CODE (x) == SUBREG)
+ if (SUBREG_P (x))
reg = SUBREG_REG (x);
if (! REG_P (reg))
return -1;
- if ((hard_regno = REGNO (reg)) >= FIRST_PSEUDO_REGISTER)
+ if (! HARD_REGISTER_NUM_P (hard_regno = REGNO (reg)))
hard_regno = lra_get_regno_hard_regno (hard_regno);
if (hard_regno < 0)
return -1;
offset = 0;
- if (GET_CODE (x) == SUBREG)
+ if (SUBREG_P (x))
offset += subreg_regno_offset (hard_regno, GET_MODE (reg),
SUBREG_BYTE (x), GET_MODE (x));
- return get_final_hard_regno (hard_regno, offset);
+ return hard_regno + offset;
}
/* If REGNO is a hard register or has been allocated a hard register,
@@ -232,7 +233,7 @@ get_reg_class (int regno)
hard_regno = lra_get_regno_hard_regno (regno);
if (hard_regno >= 0)
{
- hard_regno = get_final_hard_regno (hard_regno, 0);
+ hard_regno = get_final_hard_regno (hard_regno);
return REGNO_REG_CLASS (hard_regno);
}
if (regno >= new_regno_start)
@@ -1712,7 +1713,7 @@ uses_hard_regs_p (rtx x, HARD_REG_SET se
if (REG_P (x))
{
- x_hard_regno = get_hard_regno (x);
+ x_hard_regno = get_final_hard_regno (REGNO (x));
return (x_hard_regno >= 0
&& overlaps_hard_reg_set_p (set, mode, x_hard_regno));
}
Index: gcc/testsuite/gcc.target/powerpc/pr77289.c
===
--- gcc/testsuite/gcc.target/powerpc/pr77289.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr77289.c (working copy)
@@ -0,0 +1,31 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-skip-if "do not overr
[Patch, fortran] PR48298 - [F03] User-Defined Derived-Type IO (DTIO)
Committed to trunk as revision 239880. Many thanks to all those who tested, reviewed or commented upon the patch. Paul and Jerry
Re: [PATCH] Fix PR64078
On 30/08/16 11:38, Bernd Edlinger wrote:
On 08/30/16 10:21, Tom de Vries wrote:
On 29/08/16 18:43, Bernd Edlinger wrote:
Thanks!
Actually my patch missed to fix one combination: -m32 with -fpic
make check-gcc-c++ RUNTESTFLAGS="ubsan.exp=object-size-9.c --tool_opts
'-m32 -fpic'"
FAIL: c-c++-common/ubsan/object-size-9.c -O2 execution test
FAIL: c-c++-common/ubsan/object-size-9.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution test
The problem here is that the functions f2 and f3 access a stack-
based object out of bounds and that is inlined in main and
therefore smashes the return address of main in this case.
A possible fix could look like follows:
Index: object-size-9.c
===
--- object-size-9.c(revision 239794)
+++ object-size-9.c(working copy)
@@ -93,5 +93,9 @@
#endif
f4 (12);
f5 (12);
+#ifdef __cplusplus
+ /* Stack may be smashed by f2/f3 above. */
+ __builtin_exit (0);
+#endif
return 0;
}
Do you think that this should be fixed too?
I think it should be fixed. Ideally, we'd prevent the out-of-bounds
writes to have harmful effects, but I'm not sure how to enforce that.
This works for me:
...
diff --git a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
index 46f1fb9..fec920d 100644
--- a/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
+++ b/gcc/testsuite/c-c++-common/ubsan/object-size-9.c
@@ -31,6 +31,7 @@ static struct C
f2 (int i)
{
struct C x;
+ struct C x2;
x.d[i] = 'z';
return x;
}
@@ -45,6 +46,7 @@ static struct C
f3 (int i)
{
struct C x;
+ struct C x2;
char *p = x.d;
p += i;
*p = 'z';
...
But I have no idea how stable this solution is.
At least x2 could not be opimized away, as it is no POD,
but there is no guarantee, that x2 comes after x on the stack.
Another possibility, which seems to work as well:
Index: gcc/testsuite/c-c++-common/ubsan/object-size-9.c
===
--- gcc/testsuite/c-c++-common/ubsan/object-size-9.c(revision 239794)
+++ gcc/testsuite/c-c++-common/ubsan/object-size-9.c(working copy)
@@ -30,7 +30,7 @@ f1 (struct T x, int i)
static struct C
f2 (int i)
{
- struct C x;
+ struct C x __attribute__ ((aligned(16)));
x.d[i] = 'z';
return x;
}
@@ -44,7 +44,7 @@ f2 (int i)
static struct C
f3 (int i)
{
- struct C x;
+ struct C x __attribute ((aligned(16)));
char *p = x.d;
p += i;
*p = 'z';
Works for me.
Thanks,
- Tom
Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)
Hi Bill,
On Tue, Aug 30, 2016 at 08:23:46PM -0500, Bill Schmidt wrote:
> The ada bootstrap failure reported in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72827
> occurs because of a latent bug in the powerpc back end. The immediate cause
> is dead store
> elimination removing two stores relative to the frame pointer that are not
> dead; however,
> DSE is tricked into doing this because we have temporarily adjusted the frame
> pointer prior
> to performing the loads. DSE relies on the frame pointer remaining constant
> to be able to
> infer stack stores that are dead.
DSE should really detect this is happening and not do the wrong thing.
Maybe add an assert somewhere? Much easier to debug, that way.
> Is this ok for trunk, and eventually for backport to the 5 and 6 branches?
Will it backport without changes?
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c(revision 239871)
> +++ gcc/config/rs6000/rs6000.c(working copy)
> @@ -24506,15 +24506,31 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> && REG_P (basereg)
> && REG_P (offsetreg)
> && REGNO (basereg) != REGNO (offsetreg));
> - if (REGNO (basereg) == 0)
> + /* We mustn't modify the stack pointer or frame pointer
> + as this will confuse dead store elimination. */
> + if ((REGNO (basereg) == STACK_POINTER_REGNUM
> +|| REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
> + && REGNO (offsetreg) != 0)
This should only check for HARD_FRAME_POINTER_REGNUM if there *is* a
frame pointer?
> {
> - rtx tmp = offsetreg;
> - offsetreg = basereg;
> - basereg = tmp;
std::swap (basereg, offsetreg);
> + emit_insn (gen_add3_insn (offsetreg, basereg,
> + offsetreg));
> + restore_basereg = gen_sub3_insn (offsetreg, offsetreg,
> +basereg);
> + dst = replace_equiv_address (dst, offsetreg);
> }
> - emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
> - restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg);
> - dst = replace_equiv_address (dst, basereg);
> + else
> + {
> + if (REGNO (basereg) == 0)
> + {
> + rtx tmp = offsetreg;
> + offsetreg = basereg;
> + basereg = tmp;
> + }
> + emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
> + restore_basereg = gen_sub3_insn (basereg, basereg,
> +offsetreg);
> + dst = replace_equiv_address (dst, basereg);
> + }
> }
> }
> else if (GET_CODE (XEXP (dst, 0)) != LO_SUM)
If (say) base=r1 offset=r0 this will now adjust r1? That cannot be good.
Segher
