Re: [Patch, libstdc++/65420] Use constexpr variables as regex_constans flags
2015-03-15 9:09 GMT+01:00 Tim Shen timshe...@gmail.com: Did a little bit refectoring on regex_constants, so that users won't be polluted too much (they are not enum types anymore). Your implementation choice is an interesting approach. I believe that a strict reading of the library specification does not allow that form, because you are using as bitmask type a class type that is not std::bitset: [bitmask.types]: Each bitmask type can be implemented as an enumerated type that overloads certain operators, as an integer type, or as a bitset (20.6). One could argue that this restriction to this specific set of types could be considered as a Standard Library defect, but no-one has yet submitted one ;-), so it depends on the decision of the maintainers here whether it would be better to follow that strict reading of the Standard or not. What I would consider as more controversial are the following things: a) The class takes advantage of a deprecated rule to implicitly declare a copy-assignment operator (because it has a user-declared copy-constructor). I suggest to fix that and add a defaulted one before at some future point this bitmask type is no longer copy-assignable. b) Basically no operation is marked as noexcept. This is user-observable and seems like an unfortunate outcome. c) The presence of a non-explicit conversion to bool allows users to unintentionally write code like std::regex_constants::syntax_option_type a = ..; [..] int x = a + 3; // Oops The presence of this function also *seemingly* allows to implicitly convert syntax_option_type to any integer type: int x = a; // Oops I suggest to make this conversion function explicit. d) One could consider to mark the mutable operations (syntax_option_type operator=(syntax_option_type)) as constexpr in C++14 mode, but that is just an idea, not a required functionality. - Daniel
Re: [Patch, libstdc++/65420] Use constexpr variables as regex_constans flags
On 15 March 2015 at 08:09, Tim Shen wrote:
Did a little bit refectoring on regex_constants, so that users won't
be polluted too much (they are not enum types anymore).
I think this is overengineered and unnecessary.
All it needs is something like:
enum syntax_option_type : unsigned { };
constexpr syntax_option_type icase = ...;
[Patch, libstdc++/65420] Use constexpr variables as regex_constans flags
Did a little bit refectoring on regex_constants, so that users won't
be polluted too much (they are not enum types anymore).
Bootstrapped and tested.
Thanks!
--
Regards,
Tim Shen
commit 5cd86b408ef0a9cba1a21a3018c797d9e245d158
Author: Tim Shen tims...@google.com
Date: Sat Mar 14 23:05:05 2015 -0700
PR libstdc++/65420
* include/bits/regex.h: Add definition for static constexpr variables.
* include/bits/regex_constants.h: Change flags to constexpr variables.
diff --git a/libstdc++-v3/include/bits/regex.h
b/libstdc++-v3/include/bits/regex.h
index a23c2c9..99ac973 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -782,6 +782,46 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_AutomatonPtr _M_automaton;
};
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::icase;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::nosubs;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::optimize;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::collate;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::ECMAScript;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::basic;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::extended;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::awk;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::grep;
+
+ templatetypename _Ch_type, typename _Rx_traits
+constexpr regex_constants::syntax_option_type
+basic_regex_Ch_type, _Rx_traits::egrep;
+
/** @brief Standard regular expressions. */
typedef basic_regexcharregex;
diff --git a/libstdc++-v3/include/bits/regex_constants.h
b/libstdc++-v3/include/bits/regex_constants.h
index 1ef5a43..1514a50 100644
--- a/libstdc++-v3/include/bits/regex_constants.h
+++ b/libstdc++-v3/include/bits/regex_constants.h
@@ -51,19 +51,92 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* @name 5.1 Regular Expression Syntax Options
*/
//@{
- enum __syntax_option
+
+ class syntax_option_type
{
-_S_icase,
-_S_nosubs,
-_S_optimize,
-_S_collate,
-_S_ECMAScript,
-_S_basic,
-_S_extended,
-_S_awk,
-_S_grep,
-_S_egrep,
-_S_syntax_last
+ private:
+enum __syntax_option
+{
+ _S_op_icase,
+ _S_op_nosubs,
+ _S_op_optimize,
+ _S_op_collate,
+ _S_op_ECMAScript,
+ _S_op_basic,
+ _S_op_extended,
+ _S_op_awk,
+ _S_op_grep,
+ _S_op_egrep,
+ _S_op_syntax_last
+};
+
+constexpr
+syntax_option_type(unsigned int __flag) : _M_flag(__flag) { }
+
+ public:
+enum _Flag : unsigned int
+{
+ _S_icase = 1 _S_op_icase,
+ _S_nosubs = 1 _S_op_nosubs,
+ _S_optimize = 1 _S_op_optimize,
+ _S_collate = 1 _S_op_collate,
+ _S_ECMAScript = 1 _S_op_ECMAScript,
+ _S_basic = 1 _S_op_basic,
+ _S_extended = 1 _S_op_extended,
+ _S_awk = 1 _S_op_awk,
+ _S_grep = 1 _S_op_grep,
+ _S_egrep = 1 _S_op_egrep,
+};
+
+constexpr
+syntax_option_type(_Flag __flag = _S_ECMAScript) : _M_flag(__flag) { }
+
+constexpr
+syntax_option_type(const syntax_option_type) = default;
+
+constexpr syntax_option_type
+operator(syntax_option_type __rhs) const
+{ return _M_flag __rhs._M_flag; }
+
+constexpr syntax_option_type
+operator|(syntax_option_type __rhs) const
+{ return _M_flag | __rhs._M_flag; }
+
+constexpr syntax_option_type
+operator^(syntax_option_type __rhs) const
+{ return _M_flag ^ __rhs._M_flag; }
+
+constexpr syntax_option_type
+operator~() const
+{ return ~_M_flag; }
+
+syntax_option_type
+operator=(syntax_option_type __rhs)
+{
+ _M_flag = __rhs._M_flag;
+ return *this;
+}
+
+syntax_option_type
+operator|=(syntax_option_type __rhs)
+{
+ _M_flag |= __rhs._M_flag;
+ return *this;
+}
+
+syntax_option_type
+operator^=(syntax_option_type __rhs)
+{
+ _M_flag ^= __rhs._M_flag;
+ return *this;
+}
+
+constexpr
+operator bool() const
+{ return _M_flag; }
+
+ private:
+unsigned int _M_flag;
};
/**
@@ -77,125 +150,86 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* elements @c ECMAScript,
Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386
Ilya Enkovich enkovich@gmail.com writes: This patch allows propagation of loop invariants for i386 if propagated value is a constant to be used in address operand. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk or stage 1? Is it necessary for this to be a target hook? The concept doesn't seem particularly target-specific. We should only propagate into the address if the new cost is no greater than the old cost, but if the address meets that condition and if propagating at this point in the pipeline is a win on x86, then wouldn't it be a win for other targets too? Thanks, Richard
Re: [PATCH PR64820] Fix ASan UAR detection fails on 32-bit targets if SSP is enabled.
Maxim Ostapenko m.ostape...@partner.samsung.com writes:
@@ -293,17 +302,15 @@ alloc_stack_frame_space (HOST_WIDE_INT size, unsigned
HOST_WIDE_INT align)
new_frame_offset = frame_offset;
Think this assignment is dead after your change.
if (FRAME_GROWS_DOWNWARD)
{
- new_frame_offset -= size + frame_phase;
- new_frame_offset = -align;
- new_frame_offset += frame_phase;
+ new_frame_offset
+ = align_base (frame_offset - frame_phase - size,
+ align, false) + frame_phase;
offset = new_frame_offset;
}
else
{
- new_frame_offset -= frame_phase;
- new_frame_offset += align - 1;
- new_frame_offset = -align;
- new_frame_offset += frame_phase;
+ new_frame_offset
+ = align_base (frame_offset - frame_phase, align, true) + frame_phase;
offset = new_frame_offset;
new_frame_offset += size;
}
(Patch looks good to me otherwise FWIW.)
Thanks,
Richard
[Patch, fortran] PR59198 - [4.8/4.9/5 Regression] ICE on cyclically dependent polymorphic types
Dear All,
As will be apparent from the PR, I have spent a silly amount of time
on this one :-( Once I became 'de-obsessed' with the fact that the
reduced testcase worked, when 'rng' was made a pointer and
concentrated on the procedure pointer component 'obs1_int', finding
the problem was rather more straightforward, even if not 'obvious'.
The ChangeLog says it all. If the check is not done for components
that are not procedure pointers,
typebound_operator_9.f03 breaks. I am not entirely sure why this is
the case but the fix works fine.
Bootstraps and regtests on FC21/x86_64 - OK for 4.8, 4.9 and 5.0?
Paul
2014-03-15 Paul Thomas pa...@gcc.gnu.org
PR fortran/59198
* trans-types.c (gfc_get_derived_type): If an abstract derived
type with procedure pointer components has no other type of
component, return the backend_decl directly. Otherwise build
the components.
2014-03-15 Paul Thomas pa...@gcc.gnu.org
PR fortran/59198
* gfortran.dg/proc_ptr_comp_44.f90 : New test
* gfortran.dg/proc_ptr_comp_45.f90 : New test
Index: gcc/fortran/trans-types.c
===
*** gcc/fortran/trans-types.c (revision 221333)
--- gcc/fortran/trans-types.c (working copy)
*** gfc_get_derived_type (gfc_symbol * deriv
*** 2448,2456
/* Its components' backend_decl have been built or we are
seeing recursion through the formal arglist of a procedure
pointer component. */
! if (TYPE_FIELDS (derived-backend_decl)
! || derived-attr.proc_pointer_comp)
return derived-backend_decl;
else
typenode = derived-backend_decl;
}
--- 2448,2469
/* Its components' backend_decl have been built or we are
seeing recursion through the formal arglist of a procedure
pointer component. */
! if (TYPE_FIELDS (derived-backend_decl))
return derived-backend_decl;
+ else if (derived-attr.proc_pointer_comp derived-attr.abstract)
+ {
+ /* If an abstract derived type with procedure pointer components
+has no other type of component, return the backend_decl.
+Otherwise build the components. */
+ for (c = derived-components; c; c = c-next)
+ {
+ if (!c-attr.proc_pointer)
+ break;
+ else if (c-next == NULL)
+ return derived-backend_decl;
+ }
+ typenode = derived-backend_decl;
+ }
else
typenode = derived-backend_decl;
}
Index: gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90
===
*** gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90 (working copy)
***
*** 0
--- 1,71
+ ! { dg-do compile }
+ ! Test the fix for PR59198, where the field for the component 'term' in
+ ! the derived type 'decay_gen_t' was not being built.
+ !
+ ! Contributed by Juergen Reuter juergen.reu...@desy.de
+ !
+ module decays
+ abstract interface
+ function obs_unary_int ()
+ end function obs_unary_int
+ end interface
+
+ type, abstract :: any_config_t
+contains
+ procedure (any_config_final), deferred :: final
+ end type any_config_t
+
+ type :: decay_term_t
+ type(unstable_t), dimension(:), pointer :: unstable_product = null ()
+ end type decay_term_t
+
+ type, abstract :: decay_gen_t
+ type(decay_term_t), dimension(:), allocatable :: term
+ procedure(obs_unary_int), nopass, pointer :: obs1_int = null ()
+ end type decay_gen_t
+
+ type, extends (decay_gen_t) :: decay_root_t
+contains
+ procedure :: final = decay_root_final
+ end type decay_root_t
+
+ type, abstract :: rng_t
+ end type rng_t
+
+ type, extends (decay_gen_t) :: decay_t
+ class(rng_t), allocatable :: rng
+contains
+ procedure :: final = decay_final
+ end type decay_t
+
+ type, extends (any_config_t) :: unstable_config_t
+contains
+ procedure :: final = unstable_config_final
+ end type unstable_config_t
+
+ type :: unstable_t
+ type(unstable_config_t), pointer :: config = null ()
+ type(decay_t), dimension(:), allocatable :: decay
+ end type unstable_t
+
+ interface
+ subroutine any_config_final (object)
+import
+class(any_config_t), intent(inout) :: object
+ end subroutine any_config_final
+ end interface
+
+ contains
+ subroutine decay_root_final (object)
+ class(decay_root_t), intent(inout) :: object
+ end subroutine decay_root_final
+
+ recursive subroutine decay_final (object)
+ class(decay_t), intent(inout) :: object
+ end subroutine decay_final
+
+ recursive subroutine unstable_config_final (object)
+ class(unstable_config_t), intent(inout) :: object
+ end subroutine unstable_config_final
+
+ end module
Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
On 03/14/2015 08:34 AM, Segher Boessenkool wrote:
On Fri, Mar 13, 2015 at 03:54:57PM -0600, Martin Sebor wrote:
Attached is a patch that eliminates the unused stack frame
allocated by gcc 5 with -pg -mprofile-kernel on powepc64le
and brings the code into parity with previous gcc versions.
The patch doesn't do anything to change the emitted code
when -mprofile-kernel is used without -pg. Since the former
option isn't fully documented (as noted in pr 65372) it's
unclear what effect it should be expected to have without
-pg.
-mprofile-kernel does nothing without profiling enabled. Maybe it
should just have been called -pk or something horrid like that.
The effect it should have is to do what the only user of the option
(the 64-bit PowerPC Linux kernel) wants. The effect it does have
is to make the 64-bit ABI more like the 32-bit ABI for mcount.
Thanks for the review and the clarification. FWIW, I mentioned
-pg because the reporter had noted that in prior versions of
GCC specifying -pg in addition to -mprofile-kernel wasn't
necessary to get the expected effect.
2015-03-13 Anton Blanchard an...@samba.org
PR target/63354
* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
^ typo
* cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New
^ typo^ typo
It shouldn't have gcc/ in the path names at all, actually.
Sorry, I must have mangled the ChangeLog sopmehow while copying
it from one terminal to another. I fixed it in the new patch
(attached) along with the other issues you pointed out.
I tested the changes in powerpc64*-linux-* native builds and on
an x86_64 host in a build for the powerpc-unknown-linux-gnu and
powerpc64-apple-darwin targets. Of these, the -mprofile-kernel
option is only accepted for powerpc64*-linux-* (which was also
confirmed by inspecting the sources) so I adjusted the test
target accordingly and kept the body of
rs6000_keep_leaf_when_profiled you suggested.
Martin
+/* -mprofile-kernel code calls mcount before the function prolog,
prologue.
+ so a profiled leaf function should stay a leaf function. */
+
+static bool
+rs6000_keep_leaf_when_profiled (void)
+{
+ return TARGET_PROFILE_KERNEL;
+}
Something like
switch (DEFAULT_ABI)
{
case ABI_AIX:
case ABI_ELFv2:
return TARGET_PROFILE_KERNEL;
default:
return true;
}
although I'm not sure about Darwin here. More conservative is to
return false for anything untested, of course.
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-options -O2 -pg -mprofile-kernel } */
+
+int foo (void)
+{
+ return 1;
+}
+
+/* { dg-final { scan-assembler bl _mcount } } */
+/* { dg-final { scan-assembler-not \(addi|stdu\) 1, } } */
Either you should run this only on AIX/ELFv2 ABIs, or you want to
test for stwu as well. Bare 1 does not work for all assemblers
(only Darwin again?)
Segher
2015-03-13 Anton Blanchard an...@samba.org
PR target/63354
* config/rs6000/linux64.h (TARGET_KEEP_LEAF_WHEN_PROFILED): Define.
* config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled): New
function.
2015-03-13 Martin Sebor mse...@redhat.com
PR target/63354
* gcc.target/powerpc/pr63354.c: New test.
diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
index 0879e7e..f51e892 100644
--- a/gcc/config/rs6000/linux64.h
+++ b/gcc/config/rs6000/linux64.h
@@ -59,6 +59,9 @@ extern int dot_symbols;
#define TARGET_PROFILE_KERNEL profile_kernel
+#undef TARGET_KEEP_LEAF_WHEN_PROFILED
+#define TARGET_KEEP_LEAF_WHEN_PROFILED rs6000_keep_leaf_when_profiled
+
#define TARGET_USES_LINUX64_OPT 1
#ifdef HAVE_LD_LARGE_TOC
#undef TARGET_CMODEL
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 31b46ea..9bad535 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24397,6 +24397,23 @@ rs6000_output_function_prologue (FILE *file,
rs6000_pic_labelno++;
}
+/* -mprofile-kernel code calls mcount before the function prologue,
+ so a profiled leaf function should stay a leaf function. */
+
+static bool
+rs6000_keep_leaf_when_profiled (void)
+{
+ switch (DEFAULT_ABI)
+{
+ case ABI_AIX:
+ case ABI_ELFv2:
+ return TARGET_PROFILE_KERNEL;
+
+ default:
+ return true;
+}
+}
+
/* Non-zero if vmx regs are restored before the frame pop, zero if
we restore after the pop when possible. */
#define ALWAYS_RESTORE_ALTIVEC_BEFORE_POP 0
diff --git a/gcc/testsuite/gcc.target/powerpc/pr63354.c b/gcc/testsuite/gcc.target/powerpc/pr63354.c
new file mode 100644
index 000..d95f1eb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc64*-linux* } } } */
+/* { dg-options -O2 -pg -mprofile-kernel
[committed] Fix gcc.dg/torture/pr65270-1.c an gcc.dg/torture/pr65270-2.c on hppa*-*-hpux*
The attach change adds the -fno-common option on hppa*-*-hpux*. This provides
necessary alignment.
Tested on hppa2.0w-hp-hpux11.11. Committed to trunk.
Dave
--
John David Anglin dave.ang...@bell.net
2015-03-15 John David Anglin dang...@gcc.gnu.org
* gcc.dg/torture/pr65270-1.c: Add -fno-common to dg-options on
hppa*-*-hpux*.
* gcc.dg/torture/pr65270-2.c: Likewise.
Index: gcc.dg/torture/pr65270-1.c
===
--- gcc.dg/torture/pr65270-1.c (revision 221432)
+++ gcc.dg/torture/pr65270-1.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-options -fno-common { target hppa*-*-hpux* } } */
struct a
{
Index: gcc.dg/torture/pr65270-2.c
===
--- gcc.dg/torture/pr65270-2.c (revision 221432)
+++ gcc.dg/torture/pr65270-2.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-options -fno-common { target hppa*-*-hpux* } } */
struct a
{
RE: [PATCH][ARM] New testcase to check parameter passing bug
Hi,
I have modified and moved the testcase following your comments.
(from gcc.target/arm to gcc.dg)
Please let me know if there's still something to fix more.
I appreciate all your comments.
Honggyu
---
gcc/testsuite/ChangeLog|4
gcc/testsuite/gcc.dg/pr65358.c | 33 +
2 files changed, 37 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr65358.c
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5302dbd..5dcf2cf 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2015-03-16 Honggyu Kim hong.gyu@lge.com
+
+ * gcc.dg/pr65358.c: New test.
+
2015-03-15 John David Anglin dang...@gcc.gnu.org
* gcc.dg/torture/pr65270-1.c: Add -fno-common to dg-options on
diff --git a/gcc/testsuite/gcc.dg/pr65358.c b/gcc/testsuite/gcc.dg/pr65358.c
new file mode 100644
index 000..3790764
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr65358.c
@@ -0,0 +1,33 @@
+/* { dg-do run */
+/* { dg-options -O2 } */
+
+struct pack
+{
+ int fine;
+ int victim;
+ int killer;
+};
+
+int __attribute__ ((__noinline__, __noclone__))
+bar (int a, int b, struct pack p)
+{
+ if (a != 20 || b != 30)
+__builtin_abort ();
+ if (p.fine != 40 || p.victim != 50 || p.killer != 60)
+__builtin_abort ();
+ return 0;
+}
+
+int __attribute__ ((__noinline__, __noclone__))
+foo (int arg1, int arg2, int arg3, struct pack p)
+{
+ return bar (arg2, arg3, p);
+}
+
+int main (void)
+{
+ struct pack p = { 40, 50, 60 };
+
+ (void) foo (10, 20, 30, p);
+ return 0;
+}
--
1.7.9.5
Re: [RS6000] bswapdi2 pattern, reload and lra
On Sun, Mar 15, 2015 at 10:51:02PM -0400, David Edelsohn wrote: On Tue, Dec 17, 2013 at 6:50 AM, Alan Modra amo...@gmail.com wrote: PR target/61350 gcc/ * config/rs6000/rs6000.md (bswapdi2): Remove one scratch reg. Modify Z-r bswapdi splitter to use dest in place of scratch. In r-Z and Z-r bswapdi splitter rename word_high, word_low to word1, word2 and rearrange logic to suit. (bswapdi2_64bit): Remove early clobber on Z-r alternative. (bswapdi2_ldbrx): Likewise. Remove '??' on r-r. (bswapdi2_32bit): Remove early clobber on Z-r alternative. Add one '?' on r-r. Modify Z-r splitter to avoid need for early clobber. gcc/testsuite/ * gcc.target/powerpc/pr53199.c: Add extra functions. Okay. Committed revision 221445. I'll leave it a few days before applying to gcc-4.9. -- Alan Modra Australia Development Lab, IBM
Re: [RS6000] bswapdi2 pattern, reload and lra
On Tue, Dec 17, 2013 at 6:50 AM, Alan Modra amo...@gmail.com wrote:
This patch is aimed at fixing test failures introduced by my
2013-12-07 change to bswapdi2_32bit:
FAIL: gcc.target/powerpc/pr53199.c scan-assembler-times lwbrx 6
FAIL: gcc.target/powerpc/pr53199.c scan-assembler-times stwbrx 6
The 2013-12-07 change was necessary to make -m32 -mlra generate good
code for pr53199.c:reg_reverse. Too many '?'s on the r-r alternative
result in lra never choosing that option. Instead we get Z-r,
ie. storing the input reg to a stack temp then using lwbrx from there.
That means we have a load-hit-store flush with a severe slowdown.
(See http://gcc.gnu.org/ml/gcc-patches/2013-12/msg2.html for the
corresponding -m64 result, a 4x slowdown.)
A similar problem occurs with -m64 -mcpu=power7 -mlra due to
bswapdi2_ldbrx having two '?'s on r-r.
To fix this I ran into a whole lot of pain. reload and lra are quite
different in their selection of insn alternatives. I could not find
any combination of '?'s that generated the best code for both reload
an lra on pr53199.c. To see why, it's necessary to look (from a great
height) at how reload chooses amongst alternatives. A particular
alternative gets a loser score, with the lowest loser being
chosen. loser is incremented
a) when an operand does not match its constraint.
b) when an alternative has a '?' in the constraint.
c) when a scratch register is required.
d) when an early clobber output clashes with one of the inputs.
a) is fairly obvious. For example, if we have a MEM when the operand
alternative needs a reg, then we'll require a reload.
b) is also quite obvious. Multiple '?'s accumulate.
c) is a little more subtle. It bites you when alternatives require
differing numbers of scratch registers. Take for example
bswapdi2_64bit, which before this patch has three alternatives
Z-r with 3 scratch regs, (Z is a subset of m)
r-Z with 2 scratch regs,
r-r with 3 scratch regs.
All other things being equal, with reload you could correct this
disparity by adding a '?' to the r-Z alternative. We might want
to do that so that Z-r and r-r are the same distance apart
as r-Z is from r-r. With lra it seems that scratch regs are
weighted differently..
d) is also tricky, and a trap for anyone optimizing insn selection for
functions like some in pr53199.c that have just one rtl insn with
early clobbers. PowerPC generally returns function results in the
same register as the first argument, so these hit the early
clobber. Code elsewhere in larger functions probably won't.
lra penalizes early clobbers differently to reload (a lot less).
So, putting this all together.. Point (d) test implication is covered
by the additional functions in pr53199.c. Avoiding early clobbers
where possible is good since it reduces differences between reload and
lra insn alternative costing. We also generate better code. I
managed to do this for all the Z-r bswapdi patterns, but stopped
short of changing r-r as well since at that point everything looked
OK! Avoiding extra scratch registers for one alternative is also good.
The op4 r-r scratch wasn't even used, and op4 for Z-r fairly easy to
do without. Renaming word_high/low to word1/2 was to make it a little
easier to track lifetime of addr1/2.
Bootstrapped and regression tested powerpc64-linux. Output of
pr53199.c inspected for sanity with -mcpu=power{6,7} -m{32,64} and
{-mlra,}. OK to apply?
gcc/
* config/rs6000/rs6000.md (bswapdi2): Remove one scratch reg.
Modify Z-r bswapdi splitter to use dest in place of scratch.
In r-Z and Z-r bswapdi splitter rename word_high, word_low
to word1, word2 and rearrange logic to suit.
(bswapdi2_64bit): Remove early clobber on Z-r alternative.
(bswapdi2_ldbrx): Likewise. Remove '??' on r-r.
(bswapdi2_32bit): Remove early clobber on Z-r alternative.
Add one '?' on r-r. Modify Z-r splitter to avoid need for
early clobber.
gcc/testsuite/
* gcc.target/powerpc/pr53199.c: Add extra functions.
Okay.
Thanks, David
Re: PR c++/64626 - C++14 single quote should not always be a digit separator
OK,
Here is a new try at PR c++/64626 - C++14 single quote should not always
be a digit separator.
I decided to look for multiple terminating single quotes.
The test cases have bee adjusted and debugged.
Built and tested on x86_64-linux.
Ed
libcpp/
2015-03-16 Edward Smith-Rowland 3dw...@verizon.net
PR c++/64626
* lex.c (lex_number): If a number ends with digit-seps (') skip back
and let lex_string take them.
gcc/testsuite/
2015-03-16 Edward Smith-Rowland 3dw...@verizon.net
PR c++/64626
g++.dg/cpp1y/pr64626-1.C: New.
g++.dg/cpp1y/pr64626-2.C: New.
g++.dg/cpp1y/digit-sep-neg.C: Adjust errors and warnings.
Index: libcpp/lex.c
===
--- libcpp/lex.c(revision 221440)
+++ libcpp/lex.c(working copy)
@@ -1400,6 +1400,9 @@
NORMALIZE_STATE_UPDATE_IDNUM (nst, *cur);
cur++;
}
+ /* A number can't end with a digit separator. */
+ while (cur pfile-buffer-cur DIGIT_SEP (cur[-1]))
+ --cur;
pfile-buffer-cur = cur;
}
Index: gcc/testsuite/g++.dg/cpp1y/pr64626-1.C
===
--- gcc/testsuite/g++.dg/cpp1y/pr64626-1.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp1y/pr64626-1.C (working copy)
@@ -0,0 +1,20 @@
+// PR c++/64626
+// { dg-do compile { target c++14 } }
+
+#define STR(s) #s
+int
+main()
+{
+ int i = 1'2;
+ const char *s[3]
+ {
+STR(1' '),
+STR(..),
+STR(%:%),
+ };
+}
+#if 0
+1' '
+..
+%:%
+#endif
Index: gcc/testsuite/g++.dg/cpp1y/pr64626-2.C
===
--- gcc/testsuite/g++.dg/cpp1y/pr64626-2.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp1y/pr64626-2.C (working copy)
@@ -0,0 +1,11 @@
+// PR c++/64626
+// { dg-do compile { target c++14 } }
+
+0''; // { dg-error empty character constant }
+
+123'''; // { dg-error empty character constant }
+
+// { dg-error expected unqualified-id expected unqualified-id { target
*-*-* } 4 }
+
+// { dg-error missing terminating missing terminating { target *-*-* } 6 }
+// { dg-error expected unqualified-id expected unqualified-id { target
*-*-* } 6 }
Index: gcc/testsuite/g++.dg/cpp1y/digit-sep-neg.C
===
--- gcc/testsuite/g++.dg/cpp1y/digit-sep-neg.C (revision 221440)
+++ gcc/testsuite/g++.dg/cpp1y/digit-sep-neg.C (working copy)
@@ -10,7 +10,7 @@
i = 0004''000'000; // { dg-error adjacent digit separators }
i = 0B1'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0'0; // OK
i = 0b'0001'''''; // { dg-error digit separator after
base indicator }
- i = 0b0001''''''; // { dg-error digit separator outside
digit sequence }
+ i = 0b0001''''''; // { dg-error missing terminating }
unsigned u = 0b0001''''''U; // { dg-error digit
separator outside digit sequence }
double d = 0.0;
@@ -18,9 +18,13 @@
d = 1.'602'176'565e-19; // { dg-error digit separator adjacent to decimal
point }
d = 1.602''176'565e-19; // { dg-error adjacent digit separators }
d = 1.602'176'565'e-19; // { dg-error digit separator adjacent to exponent
}
- d = 1.602'176'565e'-19; // { dg-error digit separator adjacent to exponent
}
+ d = 1.602'176'565e'-19; // { dg-error missing terminating }
d = 1.602'176'565e-'19; // { dg-error digit separator adjacent to exponent
}
d = 1.602'176'565e-1'9; // OK
- d = 1.602'176'565e-19'; // { dg-error digit separator outside digit
sequence }
+ d = 1.602'176'565e-19'; // { dg-error missing terminating }
float f = 1.602'176'565e-19'F; // { dg-error digit separator outside digit
sequence }
}
+
+// { dg-error exponent has no digits exponent has no digits { target *-*-*
} 21 }
+// { dg-error expected ';' before expected ';' before { target *-*-* } 14 }
+// { dg-error expected ';' before expected ';' before { target *-*-* } 25 }
[AArch64][PR65375] Fix RTX cost for vector SET
AArch64 RTX cost for vector SET is causing PR65375. Lower subreg is
using this rtx_cost to compute the cost of moves, and splitting anything
larger than word size, 64-bits in this case. The aarch64 rtx_costs is
returning 2 * COST_N_INSNS(1) for vector moves, so they get split.
Attach patch fixes this.
With the patch the testcase in the PR:
#include arm_neon.h
void hello_vst2(float* fout, float *fin)
{
float32x4x2_t a;
a = vld2q_f32 (fin);
vst2q_f32 (fout, a);
}
Changes to:
hello_vst2:
- ld2 {v0.4s - v1.4s}, [x1]
- sub sp, sp, #32
- umovx1, v0.d[0]
- umovx2, v0.d[1]
- str q1, [sp, 16]
- mov x5, x1
- stp x5, x2, [sp]
- ld1 {v0.16b - v1.16b}, [sp]
+ ld2 {v2.4s - v3.4s}, [x1]
+ orr v0.16b, v2.16b, v2.16b
+ orr v1.16b, v3.16b, v3.16b
st2 {v0.4s - v1.4s}, [x0]
- add sp, sp, 32
ret
lower-subreg.c:compute_costs() only cares about the cost of a (set (reg)
(const_int )) move but I think the intention, at least for now, is to
return extra_cost-vect.alu for all the vector operations.
Regression tested on aarch64-linux-gnu with no new regression. Is this
OK for trunk?
Thanks,
Kugan
gcc/ChangeLog:
2015-03-16 Kugan Vivekanandarajah kug...@linaro.org
Jim Wilson jim.wil...@linaro.org
PR target/65375
* config/aarch64/aarch64.c (aarch64_rtx_costs): Return
extra_cost-vect.alu for SET.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index cba3c1a..db69979 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5517,6 +5517,13 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
op0 = SET_DEST (x);
op1 = SET_SRC (x);
+ /* Sets don't have a mode, so we must recompute this here. */
+ if (VECTOR_MODE_P (GET_MODE (op0)))
+ {
+ *cost += extra_cost-vect.alu;
+ return true;
+ }
+
switch (GET_CODE (op0))
{
case MEM:
