Re: fix spurious error from switch-conversion on overaligned types
On Thu, Mar 9, 2017 at 10:24 PM, Olivier Hainque wrote: >> Yes, unconditionally. > > Here's an adjusted patch, pleasantly simpler indeed > (thanks again for the suggestion). > > Test passes fine as well. > > Is this one OK ? Ok. Richard. > Olivier > > * tree-switch-conversion (array_value_type): Start by > resetting candidate type to it's main variant. > > > > >
Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
On 10/03/17 06:24, Jakub Jelinek wrote:
On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
gcc/ChangeLog:
PR translation/79923
* auto-profile.c (get_combined_location): Convert leading
character of diagnostics to lower case and remove trailing period.
(read_profile): Likewise for various diagnostics.
* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
period from various diagnostics.
* config/arm/arm.c (arm_option_override): Likewise.
* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
(msp430_expand_delay_cycles): Likewise.
Mostly ok, but for
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
&& (imm < 0 || imm > 32))
{
if (fcode == ARM_BUILTIN_WRORHI)
- error ("the range of count should be in 0 to 32. please check the
intrinsic _mm_rori_pi16 in code.");
+ error ("the range of count should be in 0 to 32. please check the
intrinsic _mm_rori_pi16 in code");
I wonder if this shouldn't use a semicolon space in the middle
instead of dot space space (many times in the same file).
Is there a convention in GCC to use semicolons?
I'm okay with changing it to a semicolon (it's slightly better IMO) as long as
it's consistent
with the style GCC uses.
Also, for the benefit of translators, this might be better done as
error ("the range of count should be in 0 to 32; please check the
intrinsic %s in code",
"_mm_rori_pi16");
so that there are fewer of these messages.
Adding some ARM folks on this.
These iWMMXt builtins haven't been touched in ages and could do with some TLC
in general.
For example, I'm not a fan of having all these "please check the intrinsic ..."
messages.
If we've got a reference to the tree we're expanding, isn't there some kind of
error function
that will point to the location in the source that's causing the error? I'd
rather use that than
hardcoding the intrinsic names. This would also allow us to collapse all these
if (fcode == <...>)
error (...);
else if (fcode == <...>)
error (...);
else if ...
constructs.
While we're at it:
if (fcode == ARM_BUILTIN_WSRLHI)
- error ("the count should be no less than 0. please check the
intrinsic _mm_srli_pi16 in code.");
+ error ("the count should be no less than 0. please check the
intrinsic _mm_srli_pi16 in code");
Let's use "the count should be a non-negative integer" to be consistent with
the error reporting
for UInteger error messages.
Perhaps commit everything except arm-builtins.c separately and deal with
this part with the ARM folks?
I agree. David, if you want to clean up the error reporting in these intrinsics
as a separate patch I'd be grateful.
Otherwise, could you please open a bugzilla ticket so we can track this?
Thanks,
Kyrill
Jakub
Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
On Fri, Mar 10, 2017 at 09:24:18AM +, Kyrill Tkachov wrote:
>
> On 10/03/17 06:24, Jakub Jelinek wrote:
> > On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
> > > gcc/ChangeLog:
> > > PR translation/79923
> > > * auto-profile.c (get_combined_location): Convert leading
> > > character of diagnostics to lower case and remove trailing period.
> > > (read_profile): Likewise for various diagnostics.
> > > * config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
> > > period from various diagnostics.
> > > * config/arm/arm.c (arm_option_override): Likewise.
> > > * config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
> > > (msp430_expand_delay_cycles): Likewise.
> > Mostly ok, but for
> >
> > > --- a/gcc/config/arm/arm-builtins.c
> > > +++ b/gcc/config/arm/arm-builtins.c
> > > @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
> > > && (imm < 0 || imm > 32))
> > > {
> > > if (fcode == ARM_BUILTIN_WRORHI)
> > > - error ("the range of count should be in 0 to 32. please check
> > > the intrinsic _mm_rori_pi16 in code.");
> > > + error ("the range of count should be in 0 to 32. please check
> > > the intrinsic _mm_rori_pi16 in code");
> > I wonder if this shouldn't use a semicolon space in the middle
> > instead of dot space space (many times in the same file).
>
> Is there a convention in GCC to use semicolons?
> I'm okay with changing it to a semicolon (it's slightly better IMO) as long
> as it's consistent
> with the style GCC uses.
We have tons of messages like:
invalid --param name %qs; did you mean %qs?
Jakub
Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
On 10/03/17 09:30, Jakub Jelinek wrote:
On Fri, Mar 10, 2017 at 09:24:18AM +, Kyrill Tkachov wrote:
On 10/03/17 06:24, Jakub Jelinek wrote:
On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
gcc/ChangeLog:
PR translation/79923
* auto-profile.c (get_combined_location): Convert leading
character of diagnostics to lower case and remove trailing period.
(read_profile): Likewise for various diagnostics.
* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
period from various diagnostics.
* config/arm/arm.c (arm_option_override): Likewise.
* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
(msp430_expand_delay_cycles): Likewise.
Mostly ok, but for
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
&& (imm < 0 || imm > 32))
{
if (fcode == ARM_BUILTIN_WRORHI)
- error ("the range of count should be in 0 to 32. please check the
intrinsic _mm_rori_pi16 in code.");
+ error ("the range of count should be in 0 to 32. please check the
intrinsic _mm_rori_pi16 in code");
I wonder if this shouldn't use a semicolon space in the middle
instead of dot space space (many times in the same file).
Is there a convention in GCC to use semicolons?
I'm okay with changing it to a semicolon (it's slightly better IMO) as long as
it's consistent
with the style GCC uses.
We have tons of messages like:
invalid --param name %qs; did you mean %qs?
Thanks, then using a semicolon here is the right thing to do.
Kyrill
Jakub
Re: stabilize store merging
On Fri, Mar 10, 2017 at 12:02 AM, Alexandre Oliva wrote: > On Mar 8, 2017, Richard Biener wrote: > >> On Wed, Mar 8, 2017 at 12:58 AM, Alexandre Oliva wrote: >>> Don't let pointer randomization change the order in which we process >>> store chains. This may cause SSA_NAMEs to be released in different >>> order, and if they're reused later, they may cause differences in SSA >>> partitioning, leading to differences in expand, and ultimately to >>> different code. > >> Any reason you chose to maintain a new hash-map instead of at >> interesting places gather to-be-processed chains into a vector and >> sort that after IDs? > > Where would we get such ids? AFAICT base_addrs don't have to be decls, > they can be arbitrary expressions. > >> Traversing the hash-map still doesn't get you >> a reliable order but only one depending on the chain creation and >> thus stmt walk order > > True. That's what all we need in general: output depends on stable > inputs only (relative order of stmts within BBs), not on random flukes > like pointer values within the compiler process. > >> But it will for example still make testcase reduction fragile if >> shrinking the hash-map by removing unrelated code ends up processing >> things in different order. > > True, if you remove or move the stmts that initiate a chain within a BB, > you will change the processing order. There are many passes that will > process stmts or insns in the order they appear, so shuffling them will > yield different SSA version assignments, pseudo numbers and whatnot. > Why should this pass be held to a higher standard? > >> So - if we fix this can we fix it by properly sorting things? > > I don't see a way to do better, considering there's no real ID we could > use for sorting. Do you? Just use the ID you use but instead of maintaining a hash-map and traverse that collect work items from the other hash-table into a vec and then sort after the ID. I was just thinking that if we're going all the way of having IDs then walking a hash-map of those IDs is not as good as we can do with similar cost? Richard. > > Even if we were to use decl IDs rather than pointers in the tree > hashing, that would still make for impredictable ordering, because decl > ids are not necessarily stable across e.g. debug and nondebug > compilations. Their order is, but if they were to appear within more > complex exprs, as we may have in this pass, hash computation would not > ensure decl id ordering is preserved. > > -- > 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
Re: fix spurious error from switch-conversion on overaligned types
> On Mar 10, 2017, at 10:20 , Richard Biener wrote: > >> Is this one OK ? > > Ok. Committing, thanks!
[PATCH] Remove __gnu_pbds::detail::binary_heap::is_heap (PR libstdc++/62045)
Hi,
The ill-formed checking in binary_heap::push_heap has made it
O(n). Remove this checking.
Since assert_valid also checks if (*this) is a legal heap, we can
remove is_heap and the assertions using it completely.
Bootstrapped and tested on x86_64-linux-gnu.
2017-03-09 Xi Ruoyao
PR libstdc++/62045:
* include/ext/pb_ds/qdetail/binary_heap_/binary_heap_.hpp
(is_heap): Remove.
(push_heap): Remove the wrong checking using is_heap.
(make_heap): Remove the assertion using is_heap.
* include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
(modify): Ditto.
(resize_for_insert_if_needed): Add PB_DS_ASSERT_VALID after
calling make_heap.
---
.../ext/pb_ds/detail/binary_heap_/binary_heap_.hpp | 21 +++--
.../pb_ds/detail/binary_heap_/insert_fn_imps.hpp| 2 +-
2 files changed, 4 insertions(+), 19 deletions(-)
diff --git
a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
b/libstdc++-
v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
index 2755f06..f653a1e 100644
--- a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
+++ b/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
@@ -266,20 +266,14 @@ namespace __gnu_pbds
const entry_cmp& m_cmp = static_cast(*this);
entry_pointer end = m_a_entries + m_size;
std::make_heap(m_a_entries, end, m_cmp);
- _GLIBCXX_DEBUG_ASSERT(is_heap());
}
void
push_heap()
{
- if (!is_heap())
- make_heap();
- else
- {
- const entry_cmp& m_cmp = static_cast(*this);
- entry_pointer end = m_a_entries + m_size;
- std::push_heap(m_a_entries, end, m_cmp);
- }
+ const entry_cmp& m_cmp = static_cast(*this);
+ entry_pointer end = m_a_entries + m_size;
+ std::push_heap(m_a_entries, end, m_cmp);
}
void
@@ -290,15 +284,6 @@ namespace __gnu_pbds
std::pop_heap(m_a_entries, end, m_cmp);
}
- bool
- is_heap()
- {
- const entry_cmp& m_cmp = static_cast(*this);
- entry_pointer end = m_a_entries + m_size;
- bool p = std::__is_heap(m_a_entries, end, m_cmp);
- return p;
- }
-
#ifdef _GLIBCXX_DEBUG
void
assert_valid(const char*, int) const;
diff --git
a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
b/libstdc++-
v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
index f82dd52..3b63515 100644
--- a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
+++ b/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
@@ -92,6 +92,7 @@ resize_for_insert_if_needed()
m_actual_size = new_size;
m_a_entries = new_entries;
make_heap();
+ PB_DS_ASSERT_VALID((*this))
}
PB_DS_CLASS_T_DEC
@@ -103,7 +104,6 @@ modify(point_iterator it, const_reference r_new_val)
swap_value_imp(it.m_p_e, r_new_val, s_no_throw_copies_ind);
fix(it.m_p_e);
PB_DS_ASSERT_VALID((*this))
- _GLIBCXX_DEBUG_ASSERT(is_heap());
}
PB_DS_CLASS_T_DEC
--
Xi Ruoyao
School of Aerospace Science and Technology, Xidian University
Re: [PATCH] Remove __gnu_pbds::detail::binary_heap::is_heap (PR libstdc++/62045)
On Fri, Mar 10, 2017 at 07:18:24PM +0800, Xi Ruoyao wrote: > Hi, > > The ill-formed checking in binary_heap::push_heap has made it > O(n). Remove this checking. > > Since assert_valid also checks if (*this) is a legal heap, we can > remove is_heap and the assertions using it completely. I think this patch should also go to libstd...@gcc.gnu.org. Marek
[PATCH] Remove __gnu_pbds::detail::binary_heap::is_heap (PR libstdc++/62045) [resend]
Hi,
This was resent to be in both libstdc++ and gcc-patches list.
The ill-formed checking in binary_heap::push_heap has made it
O(n). Remove this checking.
Since assert_valid also checks if (*this) is a legal heap, we can
remove is_heap and the assertions using it completely.
Bootstrapped and tested on x86_64-linux-gnu.
2017-03-09 Xi Ruoyao
PR libstdc++/62045:
* include/ext/pb_ds/qdetail/binary_heap_/binary_heap_.hpp
(is_heap): Remove.
(push_heap): Remove the wrong checking using is_heap.
(make_heap): Remove the assertion using is_heap.
* include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
(modify): Ditto.
(resize_for_insert_if_needed): Add PB_DS_ASSERT_VALID after
calling make_heap.
---
.../ext/pb_ds/detail/binary_heap_/binary_heap_.hpp | 21 +++--
.../pb_ds/detail/binary_heap_/insert_fn_imps.hpp| 2 +-
2 files changed, 4 insertions(+), 19 deletions(-)
diff --git
a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
b/libstdc++-
v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
index 2755f06..f653a1e 100644
--- a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
+++ b/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/binary_heap_.hpp
@@ -266,20 +266,14 @@ namespace __gnu_pbds
const entry_cmp& m_cmp = static_cast(*this);
entry_pointer end = m_a_entries + m_size;
std::make_heap(m_a_entries, end, m_cmp);
- _GLIBCXX_DEBUG_ASSERT(is_heap());
}
void
push_heap()
{
- if (!is_heap())
- make_heap();
- else
- {
- const entry_cmp& m_cmp = static_cast(*this);
- entry_pointer end = m_a_entries + m_size;
- std::push_heap(m_a_entries, end, m_cmp);
- }
+ const entry_cmp& m_cmp = static_cast(*this);
+ entry_pointer end = m_a_entries + m_size;
+ std::push_heap(m_a_entries, end, m_cmp);
}
void
@@ -290,15 +284,6 @@ namespace __gnu_pbds
std::pop_heap(m_a_entries, end, m_cmp);
}
- bool
- is_heap()
- {
- const entry_cmp& m_cmp = static_cast(*this);
- entry_pointer end = m_a_entries + m_size;
- bool p = std::__is_heap(m_a_entries, end, m_cmp);
- return p;
- }
-
#ifdef _GLIBCXX_DEBUG
void
assert_valid(const char*, int) const;
diff --git
a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
b/libstdc++-
v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
index f82dd52..3b63515 100644
--- a/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
+++ b/libstdc++-v3/include/ext/pb_ds/detail/binary_heap_/insert_fn_imps.hpp
@@ -92,6 +92,7 @@ resize_for_insert_if_needed()
m_actual_size = new_size;
m_a_entries = new_entries;
make_heap();
+ PB_DS_ASSERT_VALID((*this))
}
PB_DS_CLASS_T_DEC
@@ -103,7 +104,6 @@ modify(point_iterator it, const_reference r_new_val)
swap_value_imp(it.m_p_e, r_new_val, s_no_throw_copies_ind);
fix(it.m_p_e);
PB_DS_ASSERT_VALID((*this))
- _GLIBCXX_DEBUG_ASSERT(is_heap());
}
PB_DS_CLASS_T_DEC
--
Xi Ruoyao
School of Aerospace Science and Technology, Xidian University
C++ PATCH to fix segv with [[noreturn]] (PR c++/79967)
Not sure of the validity of the test, but clang accepts it and so do we,
with this patch. We shouldn't attempt to dereference a pointer that
could be NULL without checking it first.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-03-10 Marek Polacek
PR c++/79967
* decl.c (grokdeclarator): Check ATTRLIST before dereferencing it.
* g++.dg/cpp0x/gen-attrs-63.C: New test.
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 3e7316f..b51ef8a 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -11402,7 +11402,8 @@ grokdeclarator (const cp_declarator *declarator,
if (declarator
&& declarator->kind == cdk_id
- && declarator->std_attributes)
+ && declarator->std_attributes
+ && attrlist != NULL)
/* [dcl.meaning]/1: The optional attribute-specifier-seq following
a declarator-id appertains to the entity that is declared. */
*attrlist = chainon (*attrlist, declarator->std_attributes);
diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
index e69de29..05f53e3 100644
--- gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
@@ -0,0 +1,12 @@
+// PR c++/79967
+// { dg-do compile { target c++11 } }
+
+template
+struct A
+{
+ int g () { f (); return 0; }
+};
+
+void f ();
+
+void g (A a) { a.g (); }
Marek
Re: [PATCH] Remove __gnu_pbds::detail::binary_heap::is_heap (PR libstdc++/62045) [resend]
On 10/03/17 19:36 +0800, Xi Ruoyao wrote: Hi, This was resent to be in both libstdc++ and gcc-patches list. Thanks. The ill-formed checking in binary_heap::push_heap has made it O(n). Remove this checking. The checking certainly looks redundant, I wouldn't say ill-formed though (that's a formal term in the C++ standard meaning the code isn't valid C++). Since assert_valid also checks if (*this) is a legal heap, we can remove is_heap and the assertions using it completely. The patch looks good, and since you're just removing code I don't think we need a copyright assignment. I'll get this applied to the active branches, thanks!
[libstdc++-v3] Fix detection of obsolete isnan
libstdc++-v3 configure checks whether old glibc inline definitions
of isnan would conflict with the libstdc++-v3 definitions and
works around them if so. But if g++ 6.x build A is used to build
another g++ 6.x B, the configure step for B will pick up the math.h
installed alongside A instead of the glibc version. configure will
then assume that the workaround isn't necessary, leaving B with a
broken cmath.
isinf already worked around this. This patch extends the same fix
to isnan. (Thanks to George for the fix.)
Tested on arch64-linux-gnu. OK for trunk and 6.x?
Thanks,
Richard
libstdc++-v3/
2017-03-10 George Lander
* acinclude.m4 (glibcxx_cv_obsolete_isnan): Define
_GLIBCXX_INCLUDE_NEXT_C_HEADERS before including math.h.
* configure: Regenerate.
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index d9859aa..5998fe6 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2297,7 +2297,8 @@ AC_DEFUN([GLIBCXX_CHECK_MATH11_PROTO], [
AC_MSG_CHECKING([for obsolete isnan function in ])
AC_CACHE_VAL(glibcxx_cv_obsolete_isnan, [
AC_COMPILE_IFELSE([AC_LANG_SOURCE(
-[#include
+[#define _GLIBCXX_INCLUDE_NEXT_C_HEADERS
+ #include
#undef isnan
namespace std {
using ::isnan;
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 9bb9862..29456c4 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -18390,6 +18390,7 @@ else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
+#define _GLIBCXX_INCLUDE_NEXT_C_HEADERS
#include
#undef isnan
namespace std {
Re: [libstdc++-v3] Fix detection of obsolete isnan
On 10/03/17 12:12 +, Richard Sandiford wrote: libstdc++-v3 configure checks whether old glibc inline definitions of isnan would conflict with the libstdc++-v3 definitions and works around them if so. But if g++ 6.x build A is used to build another g++ 6.x B, the configure step for B will pick up the math.h installed alongside A instead of the glibc version. configure will then assume that the workaround isn't necessary, leaving B with a broken cmath. isinf already worked around this. This patch extends the same fix to isnan. (Thanks to George for the fix.) Huh, I wonder why I only did it for one of them. Tested on arch64-linux-gnu. OK for trunk and 6.x? OK, thanks.
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
Hi Segher,
> On Fri, Feb 10, 2017 at 11:56:39AM +0100, Rainer Orth wrote:
>> Segher Boessenkool writes:
>>
>> > As stated in the PR, this test now passes on aarch64, ia64, powerpc,
>> > and s390x. This patch disables the xfails for those targets.
>>
>> As I'd mentioned in PR tree-optimization/78775, the test XPASSes on
>> SPARC, too.
>>
>> > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk?
>> [...]
>> > 2017-02-09 Segher Boessenkool
>> >
>> > gcc/testsuite/
>> >PR testsuite/79356
>> >* gcc.dg/attr-alloc_size-11.c: Don't xfail on aarch64, ia64, powerpc,
>> >or s390x.
>>
>> TBH, I'd strongly prefer to have a proper analysis instead of just
>> un-xfail-ing the test on an ever growing apparently random list of
>> targets.
>
> Yeah, I agree. I thought it was just another test that stopped failing,
> but it seems to fail in two ways now, making the testcase succeed?
> Lovely. Please consider this patch withdrawn.
I just noticed that nothing has happened at all in a month, so anything
is better than the tests XPASSing on a number of targets.
So the patch is ok for mainline with sparc*-*-* added to the target
lists and a reference to PR testsuite/79356 in the comment.
I'd still be very grateful if Martin could have a look what's really
going on here, though.
Thanks.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH] MPX: Fix option handling.
Hello.
This is follow-up patch which I agreed on with Jakub.
It enables CHKP with LSAN and majority of UBSAN options.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
>From a410d5e4e028d34dc00b4aa637cdcd3986b971d8 Mon Sep 17 00:00:00 2001
From: marxin
Date: Fri, 10 Mar 2017 11:05:27 +0100
Subject: [PATCH] MPX: Fix option handling.
gcc/ChangeLog:
2017-03-10 Martin Liska
PR target/65705
PR target/69804
* toplev.c (process_options): Enable MPX with LSAN and UBSAN
(except -fsanitize=bounds and -fsanitize=bounds-strict).
* tree-chkp.c (chkp_walk_pointer_assignments): Verify that
FIELD != NULL.
---
gcc/testsuite/gcc.target/i386/pr71458.c | 2 +-
gcc/toplev.c| 47 +++--
gcc/tree-chkp.c | 2 +-
3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/gcc/testsuite/gcc.target/i386/pr71458.c b/gcc/testsuite/gcc.target/i386/pr71458.c
index 2faf6bb9391..27e7764b5a0 100644
--- a/gcc/testsuite/gcc.target/i386/pr71458.c
+++ b/gcc/testsuite/gcc.target/i386/pr71458.c
@@ -1,6 +1,6 @@
/* { dg-do compile { target { ! x32 } } } */
/* { dg-options "-fcheck-pointer-bounds -mmpx -fsanitize=bounds" } */
-/* { dg-error "-fcheck-pointer-bounds is not supported with Undefined Behavior Sanitizer" "" { target *-*-* } 0 } */
+/* { dg-error "-fcheck-pointer-bounds is not supported with -fsanitize=bounds" "" { target *-*-* } 0 } */
enum {} a[0];
void fn1(int);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 6a7e4fbdffb..38bc33e007e 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1274,27 +1274,34 @@ process_options (void)
flag_check_pointer_bounds = 0;
}
- if (flag_sanitize)
+ if (flag_sanitize & SANITIZE_BOUNDS_STRICT)
{
- if (flag_sanitize & SANITIZE_ADDRESS)
- error_at (UNKNOWN_LOCATION,
- "-fcheck-pointer-bounds is not supported with "
- "Address Sanitizer");
-
- if (flag_sanitize & (SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))
- error_at (UNKNOWN_LOCATION,
- "-fcheck-pointer-bounds is not supported with "
- "Undefined Behavior Sanitizer");
-
- if (flag_sanitize & SANITIZE_LEAK)
- error_at (UNKNOWN_LOCATION,
- "-fcheck-pointer-bounds is not supported with "
- "Leak Sanitizer");
-
- if (flag_sanitize & SANITIZE_THREAD)
- error_at (UNKNOWN_LOCATION,
- "-fcheck-pointer-bounds is not supported with "
- "Thread Sanitizer");
+ error_at (UNKNOWN_LOCATION,
+ "-fcheck-pointer-bounds is not supported with "
+ "-fsanitize=bounds-strict");
+ flag_check_pointer_bounds = 0;
+ }
+ else if (flag_sanitize & SANITIZE_BOUNDS)
+ {
+ error_at (UNKNOWN_LOCATION,
+ "-fcheck-pointer-bounds is not supported with "
+ "-fsanitize=bounds");
+ flag_check_pointer_bounds = 0;
+ }
+
+ if (flag_sanitize & SANITIZE_ADDRESS)
+ {
+ error_at (UNKNOWN_LOCATION,
+ "-fcheck-pointer-bounds is not supported with "
+ "Address Sanitizer");
+ flag_check_pointer_bounds = 0;
+ }
+
+ if (flag_sanitize & SANITIZE_THREAD)
+ {
+ error_at (UNKNOWN_LOCATION,
+ "-fcheck-pointer-bounds is not supported with "
+ "Thread Sanitizer");
flag_check_pointer_bounds = 0;
}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index c057b977342..fdb95ee5cc6 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3804,7 +3804,7 @@ chkp_walk_pointer_assignments (tree lhs, tree rhs, void *arg,
FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (rhs), cnt, field, val)
{
- if (chkp_type_has_pointer (TREE_TYPE (field)))
+ if (field && chkp_type_has_pointer (TREE_TYPE (field)))
{
tree lhs_field = chkp_build_component_ref (lhs, field);
chkp_walk_pointer_assignments (lhs_field, val, arg, handler);
--
2.11.1
Re: [PATCH] MPX: Fix option handling.
On Fri, Mar 10, 2017 at 02:09:20PM +0100, Martin Liška wrote: > Hello. > > This is follow-up patch which I agreed on with Jakub. > It enables CHKP with LSAN and majority of UBSAN options. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Martin > >From a410d5e4e028d34dc00b4aa637cdcd3986b971d8 Mon Sep 17 00:00:00 2001 > From: marxin > Date: Fri, 10 Mar 2017 11:05:27 +0100 > Subject: [PATCH] MPX: Fix option handling. > > gcc/ChangeLog: > > 2017-03-10 Martin Liska > > PR target/65705 > PR target/69804 > * toplev.c (process_options): Enable MPX with LSAN and UBSAN > (except -fsanitize=bounds and -fsanitize=bounds-strict). Please avoid the ()s, that is confusing with ()s used to surround function etc. names. Just use UBSAN, except ... strict. > * tree-chkp.c (chkp_walk_pointer_assignments): Verify that > FIELD != NULL. > + error_at (UNKNOWN_LOCATION, > + "-fcheck-pointer-bounds is not supported with " > + "-fsanitize=bounds-strict"); > + flag_check_pointer_bounds = 0; Given the recent i18n discussions, perhaps this ought to be %<-fcheck-pointer-bounds%> and %<-fsanitize=bounds-strict%> and similarly elsewhere (of course not for Address/Thread Sanitizer words). Ok for trunk either way. Jakub
[PATCH] MPX: fix PR middle-end/79753
Hello.
Currently, __builtin_ia32_bndret is used for all functions that have non-void
return type. I think the right fix is to return bounds just for a bounded type.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
>From f4ad5003a42ca95187305a9b201656c7da2aaa01 Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 9 Mar 2017 15:42:49 +0100
Subject: [PATCH] MPX: fix PR middle-end/79753
gcc/ChangeLog:
2017-03-09 Martin Liska
PR middle-end/79753
* tree-chkp.c (chkp_build_returned_bound): Do not build
returned bounds for a LHS that's not a BOUNDED_P type.
gcc/testsuite/ChangeLog:
2017-03-10 Martin Liska
* gcc.target/i386/mpx/pr79753.c: New test.
---
gcc/testsuite/gcc.target/i386/mpx/pr79753.c | 14 ++
gcc/tree-chkp.c | 11 ++-
2 files changed, 20 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/mpx/pr79753.c
diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr79753.c b/gcc/testsuite/gcc.target/i386/mpx/pr79753.c
new file mode 100644
index 000..9b7bc52e1ed
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr79753.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx -O2" } */
+
+int
+foo (void)
+{
+ return 0;
+}
+
+void
+bar (int **p)
+{
+ *p = (int *) (__UINTPTR_TYPE__) foo ();
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index acd57eac5ef..c057b977342 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -2218,6 +2218,7 @@ chkp_build_returned_bound (gcall *call)
gimple *stmt;
tree fndecl = gimple_call_fndecl (call);
unsigned int retflags;
+ tree lhs = gimple_call_lhs (call);
/* To avoid fixing alloca expands in targets we handle
it separately. */
@@ -2227,9 +2228,8 @@ chkp_build_returned_bound (gcall *call)
|| DECL_FUNCTION_CODE (fndecl) == BUILT_IN_ALLOCA_WITH_ALIGN))
{
tree size = gimple_call_arg (call, 0);
- tree lb = gimple_call_lhs (call);
gimple_stmt_iterator iter = gsi_for_stmt (call);
- bounds = chkp_make_bounds (lb, size, &iter, true);
+ bounds = chkp_make_bounds (lhs, size, &iter, true);
}
/* We know bounds returned by set_bounds builtin call. */
else if (fndecl
@@ -2282,9 +2282,10 @@ chkp_build_returned_bound (gcall *call)
bounds = chkp_find_bounds (gimple_call_arg (call, argno), &iter);
}
- else if (chkp_call_returns_bounds_p (call))
+ else if (chkp_call_returns_bounds_p (call)
+ && BOUNDED_P (lhs))
{
- gcc_assert (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME);
+ gcc_assert (TREE_CODE (lhs) == SSA_NAME);
/* In general case build checker builtin call to
obtain returned bounds. */
@@ -2311,7 +2312,7 @@ chkp_build_returned_bound (gcall *call)
print_gimple_stmt (dump_file, call, 0, TDF_VOPS|TDF_MEMSYMS);
}
- bounds = chkp_maybe_copy_and_register_bounds (gimple_call_lhs (call), bounds);
+ bounds = chkp_maybe_copy_and_register_bounds (lhs, bounds);
return bounds;
}
--
2.11.1
Re: [PATCH] [PATCH, rs6000] Fix pr79941 (v2)
On Thu, 2017-03-09 at 17:24 -0600, Segher Boessenkool wrote:
> Hi Will,
>
> On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote:
> > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those
> > entries were added to the intrinsics-to-be-folded list where the generic
> > multiplies should have been instead. Test coverage in place was for the
> > generic multiplies, and this was missed by my testing.
> >
> > The mul[eo]* unsigned operations were missing entries in
> > builtin_function_type()
> > to indicate the overloaded arguments were unsigned. This is corrected here,
> > and those operations now fold accurately to the desired instruction.
> >
> > Testcases have been added for the mule/mulo operations, as well as a dg-do
> > run test based on the testcase in the PR. I'll note that the variables in
> > that
> > test case are now marked as volatile so they are not entirely optimized out
> > during
> > the compile.
> >
> > OK for trunk?
> > regtest is currently running on ppc64*.
>
>
>
> > PR target/79941
> > * config/rs6000/rs6000.c (builtin_function_type): Add VMUL*UH
> > entries to the case statement that marks unsigned arguments to
> > overloaded functions.
>
> UH and UB?
Yes. I've updated that to read "Add VMUL*U[HB]" in my local copy.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c
> > @@ -0,0 +1,38 @@
> > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with char
> > + inputs produce the right results. */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec -O2 " } */
>
> ^ this space looks off
It is. fixed locally in both *mule-char.c and *mule-short.c testcases.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c
> > @@ -0,0 +1,61 @@
> > +/* PR target/79941 */
> > +
> > +/* { dg-do run } */
> > +/* { dg-require-effective-target powerpc_vsx_ok } */
> > +/* { dg-options "-mvsx -O2 -save-temps" } */
>
> I think that -save-temps is a leftover from testing? It shouldn't be there?
Because this is a "dg-do run", and I am also doing a scan-assembler
stanza at the end, I needed that to preserve the intermediate. I can
rework things if this is not considered proper.. :-)
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c
> > @@ -0,0 +1,37 @@
> > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with short
> > + inputs produce the right results. */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec -O2 " } */
>
> ^ another
yup, got it updated locally now, thanks.
>
> Okay with those nits fixed, but let's hear if Mike remembers anything first.
yup.
>
> Thanks,
>
>
> Segher
>
Re: [PATCH rs6000] Fix PR79907
On 03/09/2017 04:26 PM, Segher Boessenkool wrote: > That looks correct. Okay for trunk, thanks! Does it need backporting > as well? The -mupper-regs-di support is new in GCC 7 and I verified the testcase compiles fine with GCC 5/6, so no backporting necessary. -Pat
RE: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
> > I just noticed that nothing has happened at all in a month, so anything > is better than the tests XPASSing on a number of targets. > > So the patch is ok for mainline with sparc*-*-* added to the target > lists and a reference to PR testsuite/79356 in the comment. > > I'd still be very grateful if Martin could have a look what's really > going on here, though. > > Thanks. > Rainer > Hi, This test has also been XPASS-ing for mips-mti-elf and mips-mti-linux-gnu, so I think you could also add mips*-*-* to the xfail targets. Regards, Toma
Re: [PATCH] [PATCH, rs6000] Fix pr79941 (v2)
On Fri, Mar 10, 2017 at 08:08:06AM -0600, Will Schmidt wrote:
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c
> > > @@ -0,0 +1,61 @@
> > > +/* PR target/79941 */
> > > +
> > > +/* { dg-do run } */
> > > +/* { dg-require-effective-target powerpc_vsx_ok } */
> > > +/* { dg-options "-mvsx -O2 -save-temps" } */
> >
> > I think that -save-temps is a leftover from testing? It shouldn't be there?
>
> Because this is a "dg-do run", and I am also doing a scan-assembler
> stanza at the end, I needed that to preserve the intermediate. I can
> rework things if this is not considered proper.. :-)
Oh I missed that. No, it is fine then.
Segher
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
Hi Toma, >> I just noticed that nothing has happened at all in a month, so anything >> is better than the tests XPASSing on a number of targets. >> >> So the patch is ok for mainline with sparc*-*-* added to the target >> lists and a reference to PR testsuite/79356 in the comment. >> >> I'd still be very grateful if Martin could have a look what's really >> going on here, though. > This test has also been XPASS-ing for mips-mti-elf and mips-mti-linux-gnu, > so I think you could also add mips*-*-* to the xfail targets. no argument from me. This just underlines that someone needs to investigate what's really going on here. Consider such a patch pre-approved. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] [PATCH, rs6000] Fix pr79941 (v2)
Jumping in so we can continue the Will/Bill confusion: ;)
The official prototypes from the original AltiVec PIM are:
vector unsigned short vec_mule (vector unsigned char, vector unsigned char);
vector signed short vec_mule (vector signed char, vector signed char);
vector unsigned int vec_mule (vector unsigned short, vector unsigned short);
vector signed int vec_mule (vector signed short, vector signed short);
These are still the only supported forms. For POWER9, we are adding similar
prototypes for 32-bit multiplies with a 64-bit result.
I do not know why we have this _UNS variant, but it seems to be something
we can do without. It does not appear in the built-in table in rs6000-c.c All
the entries in that built-in table are type-correct with respect to the above
definitions. We should delete that entry from rs6000-builtin.def.
-- Bill
Bill Schmidt, Ph.D.
GCC for Linux on Power
Linux on Power Toolchain
IBM Linux Technology Center
wschm...@linux.vnet.ibm.com
> On Mar 9, 2017, at 6:15 PM, Jakub Jelinek wrote:
>
> On Thu, Mar 09, 2017 at 07:01:06PM -0500, Michael Meissner wrote:
This looks good to me, but I'll defer the actual review to PowerPC
maintainers. Perhaps there was some hidden reason (xlC compatibility,
whatever) that said that vmuleub etc. should have signed vector arguments
and result.
Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc.
codes are for (the builtin doesn't seem to be user accessible).
>>>
>>> It used to be, but that was removed when mult-even was removed (which
>>> seems to be the only thing it was used for). Mike, do you remember?
>>
>> I don't recall. Perhaps it is related to:
>>
>> 2016-12-19 Will Schmidt
>>
>> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for
>> early expansion of vector multiply and subtract builtins.
>
> That added the folding. The questions are:
> 1) if it is intentional that ALTIVEC_BUILTIN_VMULEUH etc. used signed rather
> than unsigned vector types as arguments and return value (and if yes, why)?
> BU_ALTIVEC_2 (VMULEUH,"vmuleuh",CONST,
> vec_widen_umult_even_v8hi)
> BU_ALTIVEC_2 (VMULEUH_UNS,"vmuleuh_uns",CONST,
> vec_widen_umult_even_v8hi)
> and builtin_function_type only mentioning
> /* unsigned 2 argument functions. */
>case ALTIVEC_BUILTIN_VMULEUH_UNS:
> Back e.g. in 4.6 UNS was used in targetm.vectorize.builtin_mul_widen_even.
> Does the Altivec spec say that that vec_vmuleuh arguments should be
> vector signed short? For vec_mule it chooses {uh,ub,sh,sb} depending on
> whether the arguments are signed/unsigned and short/char vectors.
> 2) can we now remove ALTIVEC_BUILTIN_VMULEUH_UNS etc.?
>
> Jakub
>
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
On Fri, Mar 10, 2017 at 03:31:39PM +0100, Rainer Orth wrote: > Hi Toma, > > >> I just noticed that nothing has happened at all in a month, so anything > >> is better than the tests XPASSing on a number of targets. > >> > >> So the patch is ok for mainline with sparc*-*-* added to the target > >> lists and a reference to PR testsuite/79356 in the comment. > >> > >> I'd still be very grateful if Martin could have a look what's really > >> going on here, though. > > > This test has also been XPASS-ing for mips-mti-elf and mips-mti-linux-gnu, > > so I think you could also add mips*-*-* to the xfail targets. > > no argument from me. This just underlines that someone needs to > investigate what's really going on here. > > Consider such a patch pre-approved. gcc-testresults shows it passing on some other mips targets as well. I'll include it. Segher
[C++ Patch/RFC] PR 77752
Hi,
if you like, this ICE is closely related to c++/60848, but occurs when
we don't have (yet) a broken definition of std::initializer_list, only a
declaration, which is nonetheless able to cause an ICE at the beginning
of build_list_conv, as called by implicit_conversion for the testcase at
issue.
As such, the broken declaration cannot be rejected by the code we have
in finish_struct, something must happen earlier than that. It seems to
me that xref_tag_1 can be a good place, per the below patchlet, which
passes testing on x86_64-linux. I briefly wondered if making
is_std_init_list stricter would make sense instead, but I don't think so
(consistently with the trail of c++/60848 too): I believe that by the
time we use is_std_init_list in the internals we want something as
simple as possible, we are assuming that broken, fake,
std::initializer_list aren't around in the translation unit. In terms of
details, I also wondered if CLASSTYPE_IS_TEMPLATE would make for a
better check, but seems unnecessarily more complex. Also, in principle,
we may want to have an even stricter check at declaration time (how many
template parameters, etc) but that seems overkilling and I don't think
we are risking ICEs because of that.
Thanks, Paolo.
///
Index: cp/decl.c
===
--- cp/decl.c (revision 246023)
+++ cp/decl.c (working copy)
@@ -13645,6 +13645,13 @@ xref_tag_1 (enum tag_types tag_code, tree name,
in push_template_decl. */
CLASSTYPE_LAMBDA_EXPR (t) = error_mark_node;
t = pushtag (name, t, scope);
+ if (t != error_mark_node
+ && CP_TYPE_CONTEXT (t) == std_node
+ && strcmp (TYPE_NAME_STRING (t), "initializer_list") == 0
+ && !CLASSTYPE_TEMPLATE_INFO (t))
+ fatal_error (input_location,
+"declaration of std::initializer_list does not match "
+"#include , isn't a template");
}
}
else
Index: testsuite/g++.dg/cpp0x/initlist97.C
===
--- testsuite/g++.dg/cpp0x/initlist97.C (revision 0)
+++ testsuite/g++.dg/cpp0x/initlist97.C (working copy)
@@ -0,0 +1,9 @@
+// PR c++/77752
+// { dg-do compile { target c++11 } }
+
+namespace std {
+ class initializer_list; // { dg-message "initializer_list" }
+}
+void f(std::initializer_list l) { f({2}); }
+
+// { dg-prune-output "compilation terminated" }
[PR 77333] Fix fntypes of calls calling clones
Hi,
PR 77333 is a i686-windows target bug, which however has its root in
our general mechanism of adjusting gimple statements when redirecting
call graph edge. Basically, these three things trigger it:
1) IPA-CP figures out that the this parameter of a C++ class method is
unused and because the class is in an anonymous namespace, it can
be removed and all calls adjusted. That effectively changes a
normal method into a static method and so internally, its type
changes from METHOD_TYPE to FUNCTION_TYPE.
2) Since the fix of PR 57330, we do not update gimple_call_fntype to
match the new type, in fact we explicitely set it to the old, now
invalid, type (see redirect_call_stmt_to_callee in cgraph.c).
3) Function ix86_get_callcvt which decides on call ABI, ends with the
following condition:
if (ret != 0
|| is_stdarg
|| TREE_CODE (type) != METHOD_TYPE
|| ix86_function_type_abi (type) != MS_ABI)
return IX86_CALLCVT_CDECL | ret;
return IX86_CALLCVT_THISCALL;
...and since now the callee is no longer a METHOD_TYPE but callers
still think that they are, leading to calling convention mismatches
and subsequent crashes. It took me quite a lot of time to come up
with a small testcase (reproducible using wine) but eventually I
managed.
The fix is not to do 2) above, but doing so without re-introducing PR
57330, of course. There are two options. One is to use the
call_stmt_cannot_inline_p flag of call-graph edges and not do any
IPA-CP accross those edges. That is done in the patch below. The (so
far a bit hypothetical) problem with that approach is that the call
graph edge flag may not be 100% reliable in LTO, because incompatible
decls might get merged and then we wold re-introduce PR 57330 again -
only with on invalid code and with LTO but an ICE nevertheless.
So the alternative would be to re-check when doing the gimple
statement adjustment and if the types match, then set the correct new
gimple_fntype and if they don't... then we can either leave it be or
just run the same type transformation on it as we did on the callee,
though they would be bogus either way. That is implemented in the
attached patch.
I have successfully bootstrapped both patches on x86_64-linux and I
have also tested them both on a windows cross-compiler and wine (with
some noise but I believe it is just noise).
Honza, Richi, do you prefer any one approach over the other?
Actually, we can have both, would that be desirable?
Thanks,
Martin
2017-03-02 Martin Jambor
PR ipa/77333
* ipa-prop.h (ipa_node_params): New field call_stmt_type_mismatch.
(ipa_node_params::ipa_node_params): Initialize it to zero.
* cgraph.c (redirect_call_stmt_to_callee): Set gimple fntype to
the type of the new target.
* ipa-cp.c (propagate_constants_across_call): Set variable flag of
lattices and call_stmt_type_mismatch of the callee when
encountering an edge with mismatched types.
(estimate_local_effects): Do not clone for all contexts when
call_stmt_type_mismatch is set.
testsuite/
* g++.dg/ipa/pr77333.C: New test.
---
gcc/cgraph.c | 2 +-
gcc/ipa-cp.c | 11 ---
gcc/ipa-prop.h | 4 ++-
gcc/testsuite/g++.dg/ipa/pr77333.C | 65 ++
4 files changed, 76 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr77333.C
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 839388496ee..642ff0bcfc2 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1425,7 +1425,7 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
new_stmt = chkp_copy_call_skip_bounds (new_stmt);
gimple_call_set_fndecl (new_stmt, e->callee->decl);
- gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt));
+ gimple_call_set_fntype (new_stmt, TREE_TYPE (e->callee->decl));
if (gimple_vdef (new_stmt)
&& TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index aa3c9973a66..d27151ffade 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -2231,9 +2231,11 @@ propagate_constants_across_call (struct cgraph_edge *cs)
checking instrumentation_clone flag for chain source and target.
Going through instrumentation thunks we always have it changed
from 0 to 1 and all other nodes do not change it. */
- if (!cs->callee->instrumentation_clone
- && callee->instrumentation_clone)
+ if (cs->call_stmt_cannot_inline_p
+ || (!cs->callee->instrumentation_clone
+ && callee->instrumentation_clone))
{
+ callee_info->call_stmt_type_mismatch = true;
for (i = 0; i < parms_count; i++)
ret |= set_all_contains_variable (ipa_get_parm_lattices (callee_info,
i));
@@ -2841,8 +2843,9 @@ estimate_local_effects (struct cgraph_node *no
Re: C++ PATCH to fix segv with [[noreturn]] (PR c++/79967)
OK.
On Fri, Mar 10, 2017 at 6:48 AM, Marek Polacek wrote:
> Not sure of the validity of the test, but clang accepts it and so do we,
> with this patch. We shouldn't attempt to dereference a pointer that
> could be NULL without checking it first.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-03-10 Marek Polacek
>
> PR c++/79967
> * decl.c (grokdeclarator): Check ATTRLIST before dereferencing it.
>
> * g++.dg/cpp0x/gen-attrs-63.C: New test.
>
> diff --git gcc/cp/decl.c gcc/cp/decl.c
> index 3e7316f..b51ef8a 100644
> --- gcc/cp/decl.c
> +++ gcc/cp/decl.c
> @@ -11402,7 +11402,8 @@ grokdeclarator (const cp_declarator *declarator,
>
>if (declarator
>&& declarator->kind == cdk_id
> - && declarator->std_attributes)
> + && declarator->std_attributes
> + && attrlist != NULL)
> /* [dcl.meaning]/1: The optional attribute-specifier-seq following
> a declarator-id appertains to the entity that is declared. */
> *attrlist = chainon (*attrlist, declarator->std_attributes);
> diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
> gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
> index e69de29..05f53e3 100644
> --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-63.C
> @@ -0,0 +1,12 @@
> +// PR c++/79967
> +// { dg-do compile { target c++11 } }
> +
> +template
> +struct A
> +{
> + int g () { f (); return 0; }
> +};
> +
> +void f ();
> +
> +void g (A a) { a.g (); }
>
> Marek
Re: [C++ PATCH] Fix error-recovery for invalid enumerators (PR c++/79896)
OK.
On Tue, Mar 7, 2017 at 2:00 PM, Jakub Jelinek wrote:
> Hi!
>
> Doing copy_node on error_mark_node doesn't work too well, lots of spots
> in the compiler assume there is just a single error_mark_node and compare
> it using pointer comparison, rather than checking for TREE_CODE () ==
> ERROR_MARK. The testcase ICEs in particular during gimplification,
> the CONST_DECL's DECL_INITIAL is not error_mark_node, but some other
> ERROR_MARK and gimplify_expr doesn't like that.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2017-03-07 Jakub Jelinek
>
> PR c++/79896
> * decl.c (finish_enum_value_list): If value is error_mark_node,
> don't copy it and change its type.
> * init.c (constant_value_1): Return error_mark_node if DECL_INITIAL
> of CONST_DECL is error_mark_node.
>
> * g++.dg/ext/int128-5.C: New test.
>
> --- gcc/cp/decl.c.jj2017-03-01 09:31:48.0 +0100
> +++ gcc/cp/decl.c 2017-03-07 12:31:23.055294773 +0100
> @@ -14323,9 +14323,12 @@ finish_enum_value_list (tree enumtype)
>input_location = saved_location;
>
>/* Do not clobber shared ints. */
> - value = copy_node (value);
> + if (value != error_mark_node)
> + {
> + value = copy_node (value);
>
> - TREE_TYPE (value) = enumtype;
> + TREE_TYPE (value) = enumtype;
> + }
>DECL_INITIAL (decl) = value;
> }
>
> --- gcc/cp/init.c.jj2017-03-02 22:30:24.0 +0100
> +++ gcc/cp/init.c 2017-03-07 12:41:08.724540062 +0100
> @@ -2162,7 +2162,8 @@ constant_value_1 (tree decl, bool strict
>init = DECL_INITIAL (decl);
>if (init == error_mark_node)
> {
> - if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
> + if (TREE_CODE (decl) == CONST_DECL
> + || DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
> /* Treat the error as a constant to avoid cascading errors on
>excessively recursive template instantiation (c++/9335). */
> return init;
> --- gcc/testsuite/g++.dg/ext/int128-5.C.jj 2017-03-07 12:36:24.617301861
> +0100
> +++ gcc/testsuite/g++.dg/ext/int128-5.C 2017-03-07 12:36:49.388973865 +0100
> @@ -0,0 +1,10 @@
> +// PR c++/79896
> +// { dg-do compile { target { ilp32 && { ! int128 } } } }
> +// { dg-options "" }
> +
> +enum E
> +{
> + e1 = 0xULL,
> + e2, // { dg-error "overflow in enumeration values" }
> + e3
> +} e = e3;
>
> Jakub
Re: [PATCH] PR libstdc++/79789 fix non-reserved names in headers
On 09/03/17 19:46 +, Jonathan Wakely wrote: On 03/03/17 10:47 -0500, David Edelsohn wrote: This patch caused a new regression on AIX. I'm unable to bootstrap on either gcc111 or gcc119 so I can't test the fix. export CONFIG_SHELL=/usr/bin/bash PATH=/opt/freeware/bin:$PATH $gccsrcdir/configure ... --disable-werror --with-included-gettext --with-gmp=/opt/cfarm --with-libiconv-prefix=/opt/cfarm --disable-libstdcxx-pch Here's what I'm committing to trunk. Tested powerpc64le-linux and powerpc-ibm-aix7.2.0.0 commit 5e390a2874a9629c13eaddb76f82a66f0634a864 Author: Jonathan Wakely Date: Fri Mar 10 13:14:33 2017 + Fix libstdc++ reserved names test to pass on AIX * testsuite/17_intro/names.cc: Undefine macros that clash with identifiers in AIX system headers. diff --git a/libstdc++-v3/testsuite/17_intro/names.cc b/libstdc++-v3/testsuite/17_intro/names.cc index a7d9a6b..c525861 100644 --- a/libstdc++-v3/testsuite/17_intro/names.cc +++ b/libstdc++-v3/testsuite/17_intro/names.cc @@ -98,4 +98,13 @@ #define x ( #define y ( #define z ( + +#ifdef _AIX +// See https://gcc.gnu.org/ml/libstdc++/2017-03/msg00015.html +#undef f +#undef r +#undef x +#undef y +#endif + #include
Re: [C++ PATCH] Fix error-recovery in maybe_thunk_body (PR c++/79899)
OK.
On Tue, Mar 7, 2017 at 2:03 PM, Jakub Jelinek wrote:
> Hi!
>
> Apparently in error recovery, populate_clone_array can't return fns
> with all NULLs. maybe_thunk_body used to handle that fine, but
> the newly added ctor_omit_inherited_parms call ICEs.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2017-03-07 Jakub Jelinek
>
> PR c++/79899
> * optimize.c (maybe_thunk_body): Don't ICE if fns[0] is NULL.
> Use XALLOCAVEC macro.
>
> * g++.dg/other/friend7.C: New test.
>
> --- gcc/cp/optimize.c.jj2017-02-04 08:43:15.0 +0100
> +++ gcc/cp/optimize.c 2017-03-07 13:21:52.280184213 +0100
> @@ -262,7 +262,7 @@ maybe_thunk_body (tree fn, bool force)
>populate_clone_array (fn, fns);
>
>/* Don't use thunks if the base clone omits inherited parameters. */
> - if (ctor_omit_inherited_parms (fns[0]))
> + if (fns[0] && ctor_omit_inherited_parms (fns[0]))
> return 0;
>
>DECL_ABSTRACT_P (fn) = false;
> @@ -324,7 +324,7 @@ maybe_thunk_body (tree fn, bool force)
>if (length > max_parms)
> max_parms = length;
> }
> - args = (tree *) alloca (max_parms * sizeof (tree));
> + args = XALLOCAVEC (tree, max_parms);
>
>/* We know that any clones immediately follow FN in TYPE_METHODS. */
>FOR_EACH_CLONE (clone, fn)
> --- gcc/testsuite/g++.dg/other/friend7.C.jj 2017-03-07 13:22:44.814487619
> +0100
> +++ gcc/testsuite/g++.dg/other/friend7.C2017-03-07 13:23:16.793063590
> +0100
> @@ -0,0 +1,9 @@
> +// PR c++/79899
> +
> +// { dg-do compile }
> +// { dg-options "-Os" }
> +
> +struct A
> +{
> + friend A::~A() {} // { dg-error "implicitly friends of their class" }
> +};
>
> Jakub
Re: [v3 PATCH] Make optional's comparisons be two-parameter templates.
On 19/02/17 17:53 +0200, Ville Voutilainen wrote: This has not been adopted by LEWG/LWG yet, but was submitted as a proposed resolution for a new LWG issue; optional can't current be compared to a T, and an optional can't be compared to something non-T that is comparable to a T. This approach fixes both of those problems, allowing optional's comparisons to actually work like the comparisons of the underlying type. Tested on Linux-x64. Please mention LWG 2934 in the changelog. OK for trunk, thanks.
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
On Fri, Mar 10, 2017 at 01:57:31PM +0100, Rainer Orth wrote:
> I just noticed that nothing has happened at all in a month, so anything
> is better than the tests XPASSing on a number of targets.
>
> So the patch is ok for mainline with sparc*-*-* added to the target
> lists and a reference to PR testsuite/79356 in the comment.
>
> I'd still be very grateful if Martin could have a look what's really
> going on here, though.
Same here.
Committed as:
Subject: [PATCH] testsuite, 79356
As stated in the PR (and elsewhere), this test now passes on aarch64,
ia64, mips, powerpc, sparc, and s390x. This patch disables the xfails
for those targets.
gcc/testsuite/
PR testsuite/79356
* gcc.dg/attr-alloc_size-11.c: Don't xfail on aarch64, ia64, mips,
powerpc, sparc, or s390x.
---
gcc/testsuite/gcc.dg/attr-alloc_size-11.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-11.c
b/gcc/testsuite/gcc.dg/attr-alloc_size-11.c
index fac8b18..fe0cb5c 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-11.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-11.c
@@ -45,10 +45,10 @@ typedef __SIZE_TYPE__size_t;
return CAT (alloc_anti_range_, __LINE__)(n); \
} typedef void dummy /* Require a semicolon. */
-/* The following tests fail because of missing range information. */
-TEST (signed char, SCHAR_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1
range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info
for signed char" { xfail *-*-* } } */
-TEST (short, SHRT_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1 range
\\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info for
short" { xfail *-*-* } } */
-
+/* The following tests fail because of missing range information. The xfail
+ exclusions are PR79356. */
+TEST (signed char, SCHAR_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1
range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info
for signed char" { xfail { ! { aarch64*-*-* ia64-*-* mips*-*-* powerpc*-*-*
sparc*-*-* s390x-*-* } } } } */
+TEST (short, SHRT_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1 range
\\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info for
short" { xfail { ! { aarch64*-*-* ia64-*-* mips*-*-* powerpc*-*-* sparc*-*-*
s390x-*-* } } } } */
TEST (int, INT_MIN + 2, ALLOC_MAX);/* { dg-warning "argument 1 range
\\\[13, \[0-9\]+\\\] exceeds maximum object size 12" } */
TEST (int, -3, ALLOC_MAX); /* { dg-warning "argument 1 range
\\\[13, \[0-9\]+\\\] exceeds maximum object size 12" } */
TEST (int, -2, ALLOC_MAX); /* { dg-warning "argument 1 range
\\\[13, \[0-9\]+\\\] exceeds maximum object size 12" } */
--
1.9.3
Re: [C++ Patch/RFC] PR 77752
On Fri, Mar 10, 2017 at 9:58 AM, Paolo Carlini wrote: > As such, the broken declaration cannot be rejected by the code we have in > finish_struct, something must happen earlier than that. It seems to me that > xref_tag_1 can be a good place, per the below patchlet, which passes testing > on x86_64-linux. I briefly wondered if making is_std_init_list stricter > would make sense instead, but I don't think so (consistently with the trail > of c++/60848 too): I believe that by the time we use is_std_init_list in the > internals we want something as simple as possible, we are assuming that > broken, fake, std::initializer_list aren't around in the translation unit. > In terms of details, I also wondered if CLASSTYPE_IS_TEMPLATE would make for > a better check, but seems unnecessarily more complex. Also, in principle, we > may want to have an even stricter check at declaration time (how many > template parameters, etc) but that seems overkilling and I don't think we > are risking ICEs because of that. I agree with all of this. How about in pushtag_1 instead, where we can return error_mark_node instead of aborting? Jason
Re: [PATCH] PR libstdc++/79789 fix non-reserved names in headers
On Fri, Mar 10, 2017 at 10:20 AM, Jonathan Wakely wrote: > On 09/03/17 19:46 +, Jonathan Wakely wrote: >> >> On 03/03/17 10:47 -0500, David Edelsohn wrote: >>> >>> This patch caused a new regression on AIX. >> >> >> I'm unable to bootstrap on either gcc111 or gcc119 so I can't test >> the fix. > > > export CONFIG_SHELL=/usr/bin/bash > PATH=/opt/freeware/bin:$PATH > $gccsrcdir/configure ... --disable-werror --with-included-gettext > --with-gmp=/opt/cfarm --with-libiconv-prefix=/opt/cfarm > --disable-libstdcxx-pch > > Here's what I'm committing to trunk. > > Tested powerpc64le-linux and powerpc-ibm-aix7.2.0.0 Great! Thank you very much! - David
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
On 03/10/2017 05:57 AM, Rainer Orth wrote:
Hi Segher,
On Fri, Feb 10, 2017 at 11:56:39AM +0100, Rainer Orth wrote:
Segher Boessenkool writes:
As stated in the PR, this test now passes on aarch64, ia64, powerpc,
and s390x. This patch disables the xfails for those targets.
As I'd mentioned in PR tree-optimization/78775, the test XPASSes on
SPARC, too.
Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk?
[...]
2017-02-09 Segher Boessenkool
gcc/testsuite/
PR testsuite/79356
* gcc.dg/attr-alloc_size-11.c: Don't xfail on aarch64, ia64, powerpc,
or s390x.
TBH, I'd strongly prefer to have a proper analysis instead of just
un-xfail-ing the test on an ever growing apparently random list of
targets.
Yeah, I agree. I thought it was just another test that stopped failing,
but it seems to fail in two ways now, making the testcase succeed?
Lovely. Please consider this patch withdrawn.
I just noticed that nothing has happened at all in a month, so anything
is better than the tests XPASSing on a number of targets.
So the patch is ok for mainline with sparc*-*-* added to the target
lists and a reference to PR testsuite/79356 in the comment.
I'd still be very grateful if Martin could have a look what's really
going on here, though.
Sorry, I haven't had a chance to look more deeply into why these
assertions pass on some targets and fail on others. There is at
least one bug that tracks a VRP problem that manifests only on
some targets and not others (79054). I don't know if this is
a symptom of the same bug or something different. I'll see if
I can find some time to isolate it.
Martin
Re: [PING 6, PATCH] Remove xfail from thread_local-order2.C.
On 07/02/17 16:01, Mike Stump wrote:
On Feb 7, 2017, at 2:20 AM, Rainer Orth wrote:
No. In fact, I'd go for something like this:
2017-02-07 Dominik Vogt
Rainer Orth
* g++.dg/tls/thread_local-order2.C: Only xfail execution on
*-*-solaris*.
# HG changeset patch
# Parent 031bb7a327cc984d387a8ae64e7c65d4b8793731
Only xfail g++.dg/tls/thread_local-order2.C on Solaris
diff --git a/gcc/testsuite/g++.dg/tls/thread_local-order2.C
b/gcc/testsuite/g++.dg/tls/thread_local-order2.C
--- a/gcc/testsuite/g++.dg/tls/thread_local-order2.C
+++ b/gcc/testsuite/g++.dg/tls/thread_local-order2.C
@@ -2,10 +2,11 @@
// that isn't reverse order of construction. We need to move
// __cxa_thread_atexit into glibc to get this right.
-// { dg-do run { xfail *-*-* } }
+// { dg-do run }
// { dg-require-effective-target c++11 }
// { dg-add-options tls }
// { dg-require-effective-target tls_runtime }
+// { dg-xfail-run-if "" { *-*-solaris* } }
extern "C" void abort();
extern "C" int printf (const char *, ...);
This way one can easily add per-target PR references or explanations,
e.g. for darwin10 or others should they come up.
Tested on i386-pc-solaris2.12 and x86_64-pc-linux-gnu. Ok for mainline?
Ok.
I think that addresses most all known issues. I'll pre-appove any additional
targets people find as trivial. For example, if darwin10 doesn't pass, then
*-*-darwin10* would be fine to add if that fixes the issue. I don't happen to
have one that old to just test on.
I am seeing this failure on arm and aarch64 bare-metal environment where newlib
are used.
This patch also XFAIL this testcase on newlib.
OK for trunk?
Regards,
Jiong
gcc/testsuite/
2017-03-10 Jiong Wang
* g++.dg/tls/thread_local-order2.C: XFAIL on newlib.
diff --git a/gcc/testsuite/g++.dg/tls/thread_local-order2.C b/gcc/testsuite/g++.dg/tls/thread_local-order2.C
index 3cbd257b5fab05d9af7aeceb4f97e9a79d2a283e..d274e8c606542893f8a792469e075056793335ea 100644
--- a/gcc/testsuite/g++.dg/tls/thread_local-order2.C
+++ b/gcc/testsuite/g++.dg/tls/thread_local-order2.C
@@ -6,7 +6,7 @@
// { dg-require-effective-target c++11 }
// { dg-add-options tls }
// { dg-require-effective-target tls_runtime }
-// { dg-xfail-run-if "" { hppa*-*-hpux* *-*-solaris* } }
+// { dg-xfail-run-if "" { { hppa*-*-hpux* *-*-solaris* } || { newlib } } }
extern "C" void abort();
extern "C" int printf (const char *, ...);
Re: [PATCH] [PATCH, rs6000] Fix pr79941 (v2)
Just so there's no duplication of effort -- I'll follow up with a patch to
remove the
outdated built-in.
-- Bill
Bill Schmidt, Ph.D.
GCC for Linux on Power
Linux on Power Toolchain
IBM Linux Technology Center
wschm...@linux.vnet.ibm.com
> On Mar 10, 2017, at 8:24 AM, Bill Schmidt wrote:
>
> Jumping in so we can continue the Will/Bill confusion: ;)
>
> The official prototypes from the original AltiVec PIM are:
>
> vector unsigned short vec_mule (vector unsigned char, vector unsigned char);
> vector signed short vec_mule (vector signed char, vector signed char);
>
> vector unsigned int vec_mule (vector unsigned short, vector unsigned short);
> vector signed int vec_mule (vector signed short, vector signed short);
>
> These are still the only supported forms. For POWER9, we are adding similar
> prototypes for 32-bit multiplies with a 64-bit result.
>
> I do not know why we have this _UNS variant, but it seems to be something
> we can do without. It does not appear in the built-in table in rs6000-c.c
> All
> the entries in that built-in table are type-correct with respect to the above
> definitions. We should delete that entry from rs6000-builtin.def.
>
> -- Bill
>
> Bill Schmidt, Ph.D.
> GCC for Linux on Power
> Linux on Power Toolchain
> IBM Linux Technology Center
> wschm...@linux.vnet.ibm.com
>
>> On Mar 9, 2017, at 6:15 PM, Jakub Jelinek wrote:
>>
>> On Thu, Mar 09, 2017 at 07:01:06PM -0500, Michael Meissner wrote:
> This looks good to me, but I'll defer the actual review to PowerPC
> maintainers. Perhaps there was some hidden reason (xlC compatibility,
> whatever) that said that vmuleub etc. should have signed vector arguments
> and result.
>
> Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc.
> codes are for (the builtin doesn't seem to be user accessible).
It used to be, but that was removed when mult-even was removed (which
seems to be the only thing it was used for). Mike, do you remember?
>>>
>>> I don't recall. Perhaps it is related to:
>>>
>>> 2016-12-19 Will Schmidt
>>>
>>> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for
>>> early expansion of vector multiply and subtract builtins.
>>
>> That added the folding. The questions are:
>> 1) if it is intentional that ALTIVEC_BUILTIN_VMULEUH etc. used signed rather
>> than unsigned vector types as arguments and return value (and if yes, why)?
>> BU_ALTIVEC_2 (VMULEUH,"vmuleuh",CONST,
>> vec_widen_umult_even_v8hi)
>> BU_ALTIVEC_2 (VMULEUH_UNS,"vmuleuh_uns",CONST,
>> vec_widen_umult_even_v8hi)
>> and builtin_function_type only mentioning
>> /* unsigned 2 argument functions. */
>> case ALTIVEC_BUILTIN_VMULEUH_UNS:
>> Back e.g. in 4.6 UNS was used in targetm.vectorize.builtin_mul_widen_even.
>> Does the Altivec spec say that that vec_vmuleuh arguments should be
>> vector signed short? For vec_mule it chooses {uh,ub,sh,sb} depending on
>> whether the arguments are signed/unsigned and short/char vectors.
>> 2) can we now remove ALTIVEC_BUILTIN_VMULEUH_UNS etc.?
>>
>> Jakub
>>
>
Re: [PING 6, PATCH] Remove xfail from thread_local-order2.C.
On Mar 10, 2017, at 8:22 AM, Jiong Wang wrote: > > I am seeing this failure on arm and aarch64 bare-metal environment where > newlib are used. > > This patch also XFAIL this testcase on newlib. > > OK for trunk? That's fine, if you want. The other solution is to actually fix newlib, which would be a better solution, if you have the time.
C++ PATCH for c++/79960, partial ordering with alias template
The semantics of alias templates are still pretty unclear, but this
testcase looks like it ought to work. I decided to make it work by
treating partial ordering specially; while normally a use of a
non-transparent alias template compares as distinct from its
expansion, as necessary for proper redeclaration handling, when
comparing the result of deduction and substitution to our goal we
can't be so strict. In this testcase, we end up comparing
const __has_tuple_size>
to
const volatile __has_tuple_size
which are not the same for SFINAE, but let's treat them the same for ordering.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 2599ce3daf721917b52f4a7c6c6ee1098907a800
Author: Jason Merrill
Date: Fri Mar 10 00:57:04 2017 -0500
PR c++/79960 - alias templates and partial ordering
* pt.c (comp_template_args): Add partial_order parm.
(template_args_equal): Likewise.
(comp_template_args_porder): New.
(get_partial_spec_bindings): Use it.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 68f2722..5be5dfe 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6208,8 +6208,8 @@ extern int is_specialization_of (tree,
tree);
extern bool is_specialization_of_friend(tree, tree);
extern tree get_pattern_parm (tree, tree);
extern int comp_template_args (tree, tree, tree * = NULL,
-tree * = NULL);
-extern int template_args_equal (tree, tree);
+tree * = NULL, bool = false);
+extern int template_args_equal (tree, tree, bool = false);
extern tree maybe_process_partial_specialization (tree);
extern tree most_specialized_instantiation (tree);
extern void print_candidates (tree);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 416f132..b8ce9fe 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8240,7 +8240,7 @@ coerce_innermost_template_parms (tree parms,
/* Returns 1 if template args OT and NT are equivalent. */
int
-template_args_equal (tree ot, tree nt)
+template_args_equal (tree ot, tree nt, bool partial_order /* = false */)
{
if (nt == ot)
return 1;
@@ -8288,8 +8288,13 @@ template_args_equal (tree ot, tree nt)
template argument; we need them to be distinct so that we
substitute into the specialization arguments at instantiation
time. And aliases can't be equivalent without being ==, so
-we don't need to look any deeper. */
- if (TYPE_ALIAS_P (nt) || TYPE_ALIAS_P (ot))
+we don't need to look any deeper.
+
+ During partial ordering, however, we need to treat them normally so
+ that we can order uses of the same alias with different
+ cv-qualification (79960). */
+ if (!partial_order
+ && (TYPE_ALIAS_P (nt) || TYPE_ALIAS_P (ot)))
return false;
else
return same_type_p (ot, nt);
@@ -8321,7 +8326,8 @@ template_args_equal (tree ot, tree nt)
int
comp_template_args (tree oldargs, tree newargs,
- tree *oldarg_ptr, tree *newarg_ptr)
+ tree *oldarg_ptr, tree *newarg_ptr,
+ bool partial_order)
{
int i;
@@ -8339,7 +8345,7 @@ comp_template_args (tree oldargs, tree newargs,
tree nt = TREE_VEC_ELT (newargs, i);
tree ot = TREE_VEC_ELT (oldargs, i);
- if (! template_args_equal (ot, nt))
+ if (! template_args_equal (ot, nt, partial_order))
{
if (oldarg_ptr != NULL)
*oldarg_ptr = ot;
@@ -8351,6 +8357,12 @@ comp_template_args (tree oldargs, tree newargs,
return 1;
}
+inline bool
+comp_template_args_porder (tree oargs, tree nargs)
+{
+ return comp_template_args (oargs, nargs, NULL, NULL, true);
+}
+
static void
add_pending_template (tree d)
{
@@ -21584,8 +21596,8 @@ get_partial_spec_bindings (tree tmpl, tree spec_tmpl,
tree args)
if (spec_args == error_mark_node
/* We only need to check the innermost arguments; the other
arguments will always agree. */
- || !comp_template_args (INNERMOST_TEMPLATE_ARGS (spec_args),
- INNERMOST_TEMPLATE_ARGS (args)))
+ || !comp_template_args_porder (INNERMOST_TEMPLATE_ARGS (spec_args),
+INNERMOST_TEMPLATE_ARGS (args)))
return NULL_TREE;
/* Now that we have bindings for all of the template arguments,
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-57.C
b/gcc/testsuite/g++.dg/cpp0x/alias-decl-57.C
new file mode 100644
index 000..f257e62
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-57.C
@@ -0,0 +1,30 @@
+// PR c++/79960
+// { dg-do compile { target c++11 } }
+
+using size_t = decltype(sizeof(0));
+
+template struct tuple_size;
+
+template::value>
+ using __has_tuple_size = T;
+
+template struct tuple_size> {
+ static constexpr size_t value
[PATCH] [ADA] Fix bootstrap failure on mips64el-linux-gnuabi64
Hi, This patch fixes an error caused by my changing of the signal constants on MIPS in r244026. While that patch worked on mipsel, ada fails to bootstrap with it on mips64el with the error: s-osinte.ads:610:07: component "sa_flags" overlaps "sa_handler" at line 608 ../gcc-interface/Makefile:297: recipe for target 'a-dispat.o' failed make[9]: *** [a-dispat.o] Error 1 The fix is to adjust the size of sa_flags in struct_sigaction from an unsigned long to an int. I checked the glibc sources and sa_flags is an int on all Linux arches. Thanks, James gcc/ada/Changelog: 2017-03-10 James Cowgill * s-osinte-linux.ads (struct_sigaction): Use correct type for sa_flags. diff --git a/gcc/ada/s-osinte-linux.ads b/gcc/ada/s-osinte-linux.ads index ee1809e2ec1..b0ba2296398 100644 --- a/gcc/ada/s-osinte-linux.ads +++ b/gcc/ada/s-osinte-linux.ads @@ -182,7 +182,7 @@ package System.OS_Interface is type struct_sigaction is record sa_handler : System.Address; sa_mask : sigset_t; - sa_flags: Interfaces.C.unsigned_long; + sa_flags: int; sa_restorer : System.Address; end record; pragma Convention (C, struct_sigaction); @@ -607,8 +607,7 @@ private for struct_sigaction use record sa_handler at Linux.sa_handler_pos range 0 .. Standard'Address_Size - 1; sa_maskat Linux.sa_mask_posrange 0 .. 1023; - sa_flags at Linux.sa_flags_pos -range 0 .. Interfaces.C.unsigned_long'Size - 1; + sa_flags at Linux.sa_flags_pos range 0 .. int'Size - 1; end record; -- We intentionally leave sa_restorer unspecified and let the compiler -- append it after the last field, so disable corresponding warning. -- 2.11.0
[PATCH] Decrease compile time memory with heavy find_base_{value,term} on i?86/x86_64 (PR rtl-optimization/63191)
Hi!
On the testcase in the PR (too large for our testsuite) we waste several
gigabytes of memory from allocations in ix86_delegitimize_address called
from find_base_{value,term}.
E.g. for find_base_term
(plus:SI (value:SI 1:1 @0x2c60f50/0x2c50f40)
(const:SI (plus:SI (unspec:SI [
(symbol_ref:SI ("_ZL2Zs") [flags 0x2] )
] UNSPEC_GOTOFF)
(const_int 8 [0x8]
on which it returns
(const:SI (plus:SI (symbol_ref:SI ("_ZL2Zs") [flags 0x2] )
(const_int 8 [0x8])))
it needs to allocate the plus and const, but it is all wasted, the caller
for CONST just looks through it and for PLUS if the other operand is
CONST_INT also just recurses on the first operand.
The following patch fixes that by handling delegitimization slightly
differently when called from ix86_find_base_term - we don't allocate memory
that is known to be not useful to the caller.
The template is kind of forced inline, so that we don't slow down the normal
delegitimization (but if you prefer normal static inline with bool
base_term_p argument, it is easy to change that).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-10 Jakub Jelinek
PR rtl-optimization/63191
* config/i386/i386.c (ix86_delegitimize_address): Turn into small
wrapper function, moved the whole old content into ...
(ix86_delegitimize_address_tmpl): ... this. New template.
(ix86_find_base_term): Use ix86_delegitimize_address_tmpl
instead of ix86_delegitimize_address.
--- gcc/config/i386/i386.c.jj 2017-03-07 20:04:52.0 +0100
+++ gcc/config/i386/i386.c 2017-03-10 14:46:24.351775710 +0100
@@ -17255,10 +17255,17 @@ ix86_delegitimize_tls_address (rtx orig_
has a different PIC label for each routine but the DWARF debugging
information is not associated with any particular routine, so it's
necessary to remove references to the PIC label from RTL stored by
- the DWARF output code. */
+ the DWARF output code.
-static rtx
-ix86_delegitimize_address (rtx x)
+ This template is used in the normal ix86_delegitimize_address
+ entrypoint (e.g. used in the target delegitimization hook) and
+ in ix86_find_base_term. As compile time memory optimization, we
+ avoid allocating rtxes that will not change anything on the outcome
+ of the callers (find_base_value and find_base_term). */
+
+template
+static inline rtx
+ix86_delegitimize_address_tmpl (rtx x)
{
rtx orig_x = delegitimize_mem_from_attrs (x);
/* addend is NULL or some rtx if x is something+GOTOFF where
@@ -17285,6 +17292,10 @@ ix86_delegitimize_address (rtx x)
&& GET_CODE (XEXP (XEXP (x, 0), 0)) == UNSPEC
&& XINT (XEXP (XEXP (x, 0), 0), 1) == UNSPEC_PCREL)
{
+ /* find_base_{value,term} only care about MEMs with arg_pointer_rtx
+base. A CONST can't be arg_pointer_rtx based. */
+ if (base_term_p && MEM_P (orig_x))
+ return orig_x;
rtx x2 = XVECEXP (XEXP (XEXP (x, 0), 0), 0, 0);
x = gen_rtx_PLUS (Pmode, XEXP (XEXP (x, 0), 1), x2);
if (MEM_P (orig_x))
@@ -17361,7 +17372,9 @@ ix86_delegitimize_address (rtx x)
if (! result)
return ix86_delegitimize_tls_address (orig_x);
- if (const_addend)
+ /* For (PLUS something CONST_INT) both find_base_{value,term} just
+ recurse on the first operand. */
+ if (const_addend && !base_term_p)
result = gen_rtx_CONST (Pmode, gen_rtx_PLUS (Pmode, result, const_addend));
if (reg_addend)
result = gen_rtx_PLUS (Pmode, reg_addend, result);
@@ -17399,6 +17412,14 @@ ix86_delegitimize_address (rtx x)
return result;
}
+/* The normal instantiation of the above template. */
+
+static rtx
+ix86_delegitimize_address (rtx x)
+{
+ return ix86_delegitimize_address_tmpl (x);
+}
+
/* If X is a machine specific address (i.e. a symbol or label being
referenced as a displacement from the GOT implemented using an
UNSPEC), then return the base term. Otherwise return X. */
@@ -17424,7 +17445,7 @@ ix86_find_base_term (rtx x)
return XVECEXP (term, 0, 0);
}
- return ix86_delegitimize_address (x);
+ return ix86_delegitimize_address_tmpl (x);
}
static void
Jakub
[PATCH] rs6000: float128 on BE and 32-bit
This fixes float128 on BE and on 32-bit.
The configure tests need to use -mabi=altivec for 32-bit, since it is
not the default there. That also enables the "vector" keyword, used by
the tests. To do this it temporarily adds a few flags to the CFLAGS
variable.
It also fixes a syntax error in the libgcc_cv_powerpc_float128_hw test
(the function name was missing in the function declaration).
Regenerating config.in (via autoreconf) removed the duplicate definition
of HAVE_SOLARIS_CRTS.
Finally, this adds a "-mfloat128-hardware requires -m64" test to
rs6000.c: all the current patterns need 64-bit registers. Maybe we'll
want to add float128 hardware support to 32-bit some day, but certainly
not today.
Tested on powerpc64-linux {-m32,-m64}, powerpc64le-linux, and
powerpc-linux-paired. Committing to trunk.
Segher
2017-03-10 Segher Boessenkool
* config/rs6000/rs6000.c (rs6000_option_override_internal): Disallow
-mfloat128-hardware without -m64.
libgcc/
* configure.ac (test for libgcc_cv_powerpc_float128): Temporarily
modify CFLAGS. Add -mabi=altivec -mvsx -mfloat128.
(test for libgcc_cv_powerpc_float128_hw): Add -mpower9-vector and
-mfloat128-hardware to the CFLAGS. Fix syntax error in the C snippet.
* configure: Regenerate.
* config.in: Regenerate.
---
gcc/config/rs6000/rs6000.c | 8
libgcc/config.in | 3 ---
libgcc/configure | 12 +++-
libgcc/configure.ac| 12 +++-
4 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index aa7c8c3..9aa0d58 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4687,6 +4687,14 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
}
+ if (TARGET_FLOAT128_HW && !TARGET_64BIT)
+{
+ if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
+ error ("-mfloat128-hardware requires -m64");
+
+ rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
+}
+
if (TARGET_FLOAT128_HW && !TARGET_FLOAT128_KEYWORD
&& (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0
&& (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_KEYWORD) == 0)
diff --git a/libgcc/config.in b/libgcc/config.in
index 4d33411..25aa0d9 100644
--- a/libgcc/config.in
+++ b/libgcc/config.in
@@ -21,9 +21,6 @@
/* Define if the system-provided CRTs are present on Solaris. */
#undef HAVE_SOLARIS_CRTS
-/* Define if the system-provided CRTs are present on Solaris. */
-#undef HAVE_SOLARIS_CRTS
-
/* Define to 1 if you have the header file. */
#undef HAVE_STDINT_H
diff --git a/libgcc/configure b/libgcc/configure
index 5c900cc..45c4597 100644
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -4779,6 +4779,8 @@ case ${host} in
# software libraries, and whether the assembler can handle xsaddqp
# for hardware support.
powerpc*-*-linux*)
+ saved_CFLAGS="$CFLAGS"
+ CFLAGS="$CFLAGS -mabi=altivec -mvsx -mfloat128"
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC ISA 2.06 to
build __float128 libraries" >&5
$as_echo_n "checking for PowerPC ISA 2.06 to build __float128 libraries... "
>&6; }
if test "${libgcc_cv_powerpc_float128+set}" = set; then :
@@ -4786,8 +4788,7 @@ if test "${libgcc_cv_powerpc_float128+set}" = set; then :
else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-#pragma GCC target ("vsx")
- vector double dadd (vector double a, vector double b) { return a + b; }
+vector double dadd (vector double a, vector double b) { return a + b; }
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
libgcc_cv_powerpc_float128=yes
@@ -4799,6 +4800,7 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_powerpc_float128"
>&5
$as_echo "$libgcc_cv_powerpc_float128" >&6; }
+ CFLAGS="$CFLAGS -mpower9-vector -mfloat128-hardware"
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC ISA 3.0 to
build hardware __float128 libraries" >&5
$as_echo_n "checking for PowerPC ISA 3.0 to build hardware __float128
libraries... " >&6; }
if test "${libgcc_cv_powerpc_float128_hw+set}" = set; then :
@@ -4806,12 +4808,11 @@ if test "${libgcc_cv_powerpc_float128_hw+set}" = set;
then :
else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-#pragma GCC target ("vsx,power9-vector")
- #include
+#include
#ifndef AT_PLATFORM
#error "AT_PLATFORM is not defined"
#endif
- vector unsigned char (vector unsigned char a, vector unsigned char b)
+ vector unsigned char add (vector unsigned char a, vector unsigned char b)
{
vector unsigned char ret;
__asm__ ("xsaddqp %0,%1,%2" : "=v" (ret) : "v" (a), "v" (b));
@@ -4830,6 +4831,7 @@ rm -f core conftest.err conftest.$ac_objext
conftest.$ac_ext
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$libgcc_cv_powerpc_float128_hw" >&5
$as_echo
[PATCH] Improve VRP for IMAGPART_EXPR of ATOMIC_COMPARE_EXCHANGE ifn (PR tree-optimization/79981)
Hi!
The following patch teaches VRP that IMAGPART_EXPR of
ATOMIC_COMPARE_EXCHANGE ifn call is always 0 or 1. We cast it to bool
afterwards, but this hint allows VRP to:
--- pr79981.c.103t.vrp1_2017-03-10 15:39:29.0 +0100
+++ pr79981.c.103t.vrp1 2017-03-10 15:48:15.051608067 +0100
@@ -17,7 +17,7 @@ Value ranges after VRP:
_1: [0, +INF]
.MEM_2: VARYING
_8: VARYING
-_9: [0, +INF]
+_9: [0, 1]
csi (int * lock)
@@ -32,11 +32,11 @@ csi (int * lock)
# USE = nonlocal null
# CLB = nonlocal null
_8 = ATOMIC_COMPARE_EXCHANGE (lock_4(D), 0, 1, 260, 2, 0);
- # RANGE [0, 4294967295]
+ # RANGE [0, 1] NONZERO 1
_9 = IMAGPART_EXPR <_8>;
# RANGE [0, 1]
_1 = (_Bool) _9;
- if (_1 != 0)
+ if (_9 != 0)
goto ; [99.96%]
else
goto ; [0.04%]
which should be beneficial e.g. for s390x code generation.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-10 Jakub Jelinek
PR tree-optimization/79981
* tree-vrp.c (extract_range_basic): Handle IMAGPART_EXPR of
ATOMIC_COMPARE_EXCHANGE ifn result.
(stmt_interesting_for_vrp, vrp_visit_stmt): Handle
IFN_ATOMIC_COMPARE_EXCHANGE.
--- gcc/tree-vrp.c.jj 2017-02-22 18:15:48.0 +0100
+++ gcc/tree-vrp.c 2017-03-10 15:45:30.574778201 +0100
@@ -4107,7 +4107,7 @@ extract_range_basic (value_range *vr, gi
}
/* Handle extraction of the two results (result of arithmetics and
a flag whether arithmetics overflowed) from {ADD,SUB,MUL}_OVERFLOW
- internal function. */
+ internal function. Similarly from ATOMIC_COMPARE_EXCHANGE. */
else if (is_gimple_assign (stmt)
&& (gimple_assign_rhs_code (stmt) == REALPART_EXPR
|| gimple_assign_rhs_code (stmt) == IMAGPART_EXPR)
@@ -4132,6 +4132,16 @@ extract_range_basic (value_range *vr, gi
case IFN_MUL_OVERFLOW:
subcode = MULT_EXPR;
break;
+ case IFN_ATOMIC_COMPARE_EXCHANGE:
+ if (code == IMAGPART_EXPR)
+ {
+ /* This is the boolean return value whether compare and
+exchange changed anything or not. */
+ set_value_range (vr, VR_RANGE, build_int_cst (type, 0),
+ build_int_cst (type, 1), NULL);
+ return;
+ }
+ break;
default:
break;
}
@@ -7283,6 +7293,7 @@ stmt_interesting_for_vrp (gimple *stmt)
case IFN_ADD_OVERFLOW:
case IFN_SUB_OVERFLOW:
case IFN_MUL_OVERFLOW:
+ case IFN_ATOMIC_COMPARE_EXCHANGE:
/* These internal calls return _Complex integer type,
but are interesting to VRP nevertheless. */
if (lhs && TREE_CODE (lhs) == SSA_NAME)
@@ -8308,6 +8319,7 @@ vrp_visit_stmt (gimple *stmt, edge *take
case IFN_ADD_OVERFLOW:
case IFN_SUB_OVERFLOW:
case IFN_MUL_OVERFLOW:
+ case IFN_ATOMIC_COMPARE_EXCHANGE:
/* These internal calls return _Complex integer type,
which VRP does not track, but the immediate uses
thereof might be interesting. */
Jakub
[PATCH, rs6000] Remove orphaned VMUL*_UNS built-ins
Hi, Jakub observed that these built-ins are no longer reachable (and haven't been for quite a while). Time to chuck them out. Bootstrapped and tested on powerpc64le-unknown-linux-gnu with (surprise!) no regressions. Is this ok for trunk? Thanks, Bill 2017-03-10 Bill Schmidt * config/rs6000/rs6000-builtin.def (VMULEUB_UNS): Remove orphaned built-in. (VMULEUH_UNS): Likewise. (VMULOUB_UNS): Likewise. (VMULOUH_UNS): Likewise. * config/rs6000/rs6000.c (builtin_function_type): Remove references to ALTIVEC_BUILTIN_VMUL[EO]U[BH]_UNS. Index: gcc/config/rs6000/rs6000-builtin.def === --- gcc/config/rs6000/rs6000-builtin.def(revision 246040) +++ gcc/config/rs6000/rs6000-builtin.def(working copy) @@ -1059,16 +1059,12 @@ BU_ALTIVEC_2 (VMINUW, "vminuw", CONST, umin BU_ALTIVEC_2 (VMINSW,"vminsw", CONST, sminv4si3) BU_ALTIVEC_2 (VMINFP,"vminfp", CONST, sminv4sf3) BU_ALTIVEC_2 (VMULEUB, "vmuleub",CONST, vec_widen_umult_even_v16qi) -BU_ALTIVEC_2 (VMULEUB_UNS,"vmuleub_uns", CONST, vec_widen_umult_even_v16qi) BU_ALTIVEC_2 (VMULESB, "vmulesb",CONST, vec_widen_smult_even_v16qi) BU_ALTIVEC_2 (VMULEUH, "vmuleuh",CONST, vec_widen_umult_even_v8hi) -BU_ALTIVEC_2 (VMULEUH_UNS,"vmuleuh_uns", CONST, vec_widen_umult_even_v8hi) BU_ALTIVEC_2 (VMULESH, "vmulesh",CONST, vec_widen_smult_even_v8hi) BU_ALTIVEC_2 (VMULOUB, "vmuloub",CONST, vec_widen_umult_odd_v16qi) -BU_ALTIVEC_2 (VMULOUB_UNS,"vmuloub_uns", CONST, vec_widen_umult_odd_v16qi) BU_ALTIVEC_2 (VMULOSB, "vmulosb",CONST, vec_widen_smult_odd_v16qi) BU_ALTIVEC_2 (VMULOUH, "vmulouh",CONST, vec_widen_umult_odd_v8hi) -BU_ALTIVEC_2 (VMULOUH_UNS,"vmulouh_uns", CONST, vec_widen_umult_odd_v8hi) BU_ALTIVEC_2 (VMULOSH, "vmulosh",CONST, vec_widen_smult_odd_v8hi) BU_ALTIVEC_2 (VNOR, "vnor", CONST, norv4si3) BU_ALTIVEC_2 (VOR, "vor",CONST, iorv4si3) Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 246040) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -18526,10 +18526,6 @@ builtin_function_type (machine_mode mode_ret, mach break; /* unsigned 2 argument functions. */ -case ALTIVEC_BUILTIN_VMULEUB_UNS: -case ALTIVEC_BUILTIN_VMULEUH_UNS: -case ALTIVEC_BUILTIN_VMULOUB_UNS: -case ALTIVEC_BUILTIN_VMULOUH_UNS: case ALTIVEC_BUILTIN_VMULEUB: case ALTIVEC_BUILTIN_VMULEUH: case ALTIVEC_BUILTIN_VMULOUB:
[patch, libgfortran] PR78854 [F03] DTIO namelist output not working on internal unit
Hi all,
The attached patch fixes this PR by properly stashing the internal unit created
by parent so that it may be correctly accessed by the child DTIO procedure.
Note the included test case. The Fortran Standard requires that the iotype be
passed to the child routine so that it is aware of what the intended purpose is.
In the case of namelist I/O the iotype is set to "NAMELIST". It is up to the
user to program the child procedure to look for that and do the right thing for
namelists to work correctly. If a user chooses to ignore this feature, so be
it, but tough luck if things don't work as "expected".
There are some other DTIO bugs related to this one. Once I get this patch in I
will be able to address those more specifically.
Regression tested on x86_64.
OK for trunk?
Regards,
Jerry
2017-03-10 Jerry DeLisle
PR libgfortran/78854
* io/list_read.c (nml_get_obj_data): Stash internal unit for
later use by child procedures.
* io/write.c (nml_write_obj): Likewise.
* io/tranfer.c (data_transfer_init): Minor whitespace.
* io/unit.c (set_internal_uit): Look for the stashed internal
unit and use it if found.
2017-03-10 Jerry DeLisle
PR libgfortran/78854
* gfortran.dg/dtio_25.f90: New test.
diff --git a/gcc/testsuite/gfortran.dg/dtio_25.f90 b/gcc/testsuite/gfortran.dg/dtio_25.f90
new file mode 100644
index ..fc049cd3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/dtio_25.f90
@@ -0,0 +1,41 @@
+! { dg-do run }
+! PR78854 namelist write to internal unit.
+module m
+ implicit none
+ type :: t
+character :: c
+integer :: k
+ contains
+procedure :: write_formatted
+generic :: write(formatted) => write_formatted
+ end type
+contains
+ subroutine write_formatted(dtv, unit, iotype, v_list, iostat, iomsg)
+class(t), intent(in) :: dtv
+integer, intent(in) :: unit
+character(*), intent(in) :: iotype
+integer, intent(in) :: v_list(:)
+integer, intent(out) :: iostat
+character(*), intent(inout) :: iomsg
+if (iotype.eq."NAMELIST") then
+ write (unit, '(a,a,a,a,i5)') 'x%c="',dtv%c,'",','x%k=', dtv%k
+else
+ write (unit,*) dtv%c, dtv%k
+end if
+ end subroutine
+end module
+
+program p
+ use m
+ implicit none
+ character(len=50) :: buffer
+ type(t) :: x
+ namelist /nml/ x
+ x = t('a', 5)
+ write (buffer, nml)
+ if (buffer.ne.'&NML x%c="a",x%k=5 /') call abort
+ x = t('x', 0)
+ read (buffer, nml)
+ if (x%c.ne.'a'.or. x%k.ne.5) call abort
+end
+
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index dd4ab72e..7f57ff1a 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -3301,6 +3301,11 @@ get_name:
child_iomsg_len = IOMSG_LEN;
}
+ /* If reading from an internal unit, stash it to allow
+ the child procedure to access it. */
+ if (is_internal_unit (dtp))
+ stash_internal_unit (dtp);
+
/* Call the user defined formatted READ procedure. */
dtp->u.p.current_unit->child_dtio++;
dtio_ptr ((void *)&list_obj, &unit, iotype, &vlist,
diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c
index 36786c03..fc22d802 100644
--- a/libgfortran/io/transfer.c
+++ b/libgfortran/io/transfer.c
@@ -2822,6 +2822,7 @@ data_transfer_init (st_parameter_dt *dtp, int read_flag)
return;
}
}
+
/* Process the ADVANCE option. */
dtp->u.p.advance_status
diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c
index ed3bc323..b733b939 100644
--- a/libgfortran/io/unit.c
+++ b/libgfortran/io/unit.c
@@ -461,6 +461,7 @@ set_internal_unit (st_parameter_dt *dtp, gfc_unit *iunit, int kind)
{
gfc_offset start_record = 0;
+ iunit->unit_number = dtp->common.unit;
iunit->recl = dtp->internal_unit_len;
iunit->internal_unit = dtp->internal_unit;
iunit->internal_unit_len = dtp->internal_unit_len;
@@ -598,15 +599,28 @@ get_unit (st_parameter_dt *dtp, int do_create)
return unit;
}
}
+
+ /* If an internal unit number is passed from the parent to the child
+ it should have been stashed on the newunit_stack ready to be used.
+ Check for it now and return the internal unit if found. */
+ if (newunit_tos && (dtp->common.unit <= NEWUNIT_START)
+ && (dtp->common.unit == newunit_stack[newunit_tos].unit_number))
+{
+ unit = newunit_stack[newunit_tos--].unit;
+ return unit;
+}
+
/* Has to be an external unit. */
dtp->u.p.unit_is_internal = 0;
dtp->internal_unit = NULL;
dtp->internal_unit_desc = NULL;
+
/* For an external unit with unit number < 0 creating it on the fly
is not allowed, such units must be created with
OPEN(NEWUNIT=...). */
if (dtp->common.unit < 0)
return get_gfc_unit (dtp->common.unit, 0);
+
return get_gfc_unit (dtp->common.unit, do_create);
}
diff --git a/libgfortran/io/write.c b/libgfortran/io/write.c
index 47970d42..f03929e4 100644
--- a/libgfortran/io/write.c
+++ b
Re: [PATCH] Decrease compile time memory with heavy find_base_{value,term} on i?86/x86_64 (PR rtl-optimization/63191)
On 03/10/2017 06:53 PM, Jakub Jelinek wrote:
+
+template
+static inline rtx
+ix86_delegitimize_address_tmpl (rtx x)
{
Why is this a template and not a function arg?
Bernd
Re: [PATCH] Decrease compile time memory with heavy find_base_{value,term} on i?86/x86_64 (PR rtl-optimization/63191)
On Fri, Mar 10, 2017 at 07:52:37PM +0100, Bernd Schmidt wrote:
> On 03/10/2017 06:53 PM, Jakub Jelinek wrote:
> > +
> > +template
> > +static inline rtx
> > +ix86_delegitimize_address_tmpl (rtx x)
> > {
>
> Why is this a template and not a function arg?
That was just an attempt to ensure that the compiler actually
either inlines it, or doesn't share code between the two versions, so the
base_term_p argument isn't checked at runtime.
But, as I said, I have no problem changing that, i.e.
remove the template line, s/_tmpl/_1/, add , bool base_term_p
and tweak both callers.
Jakub
Re: [PATCH] Fix out-of-bounds write in RTL function reader (PR bootstrap/79952)
On Fri, 2017-03-10 at 00:36 +0100, Bernd Schmidt wrote: > On 03/09/2017 08:28 PM, David Malcolm wrote: > > The root cause is an out-of-bounds memory write in the RTL dump > > reader when handling SYMBOL_REFs with SYMBOL_FLAG_HAS_BLOCK_INFO > > set. > > > > Such SYMBOL_REFs are normally created by > > varasm.c:create_block_symbol, > > which has: > > Hmm, I don't actually recall seeing this stuff. It's for section > anchors > apparently. > > > OK for trunk in stage 4? > > > > gcc/ChangeLog: > > PR bootstrap/79952 > > * read-rtl-function.c (function_reader::read_rtx_operand): > > Update > > x with result of extra_parsing_for_operand_code_0. > > (function_reader::extra_parsing_for_operand_code_0): Convert > > return type from void to rtx, returning x. When reading > > SYMBOL_REF with SYMBOL_FLAG_HAS_BLOCK_INFO, reallocate x to the > > larger size containing struct block_symbol. > > Looks OK for now, but longer term I think we should make it possible > to > reconstruct this data. Thanks; fix committed to trunk as r246044. I'm also not very familiar with this part of RTL. print-rtl.c:rtx_writer::print_rtx_operand_code_0 has some special -casing for SYMBOL_REF, but if I'm reading things right we don't yet dump SYMBOL_REF_BLOCK and SYMBOL_REF_BLOCK_OFFSET, so we'd need to dump these somehow.
Re: [PATCH] Fix out-of-bounds write in RTL function reader (PR bootstrap/79952)
On 03/10/2017 08:03 PM, David Malcolm wrote: print-rtl.c:rtx_writer::print_rtx_operand_code_0 has some special -casing for SYMBOL_REF, but if I'm reading things right we don't yet dump SYMBOL_REF_BLOCK and SYMBOL_REF_BLOCK_OFFSET, so we'd need to dump these somehow. Yeah. Perhaps as an extra table or so to show the various blocks and the symbols in them sorted by offset - could be useful information for RTL dumps too. Bernd
Re: [PATCH, rs6000] Remove orphaned VMUL*_UNS built-ins
On Fri, Mar 10, 2017 at 12:16:31PM -0600, Bill Schmidt wrote: > Jakub observed that these built-ins are no longer reachable (and > haven't been for quite a while). Time to chuck them out. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with > (surprise!) no regressions. Is this ok for trunk? Yes please. Thanks! Segher > 2017-03-10 Bill Schmidt > > * config/rs6000/rs6000-builtin.def (VMULEUB_UNS): Remove orphaned > built-in. > (VMULEUH_UNS): Likewise. > (VMULOUB_UNS): Likewise. > (VMULOUH_UNS): Likewise. > * config/rs6000/rs6000.c (builtin_function_type): Remove > references to ALTIVEC_BUILTIN_VMUL[EO]U[BH]_UNS.
Re: [PATCH, rs6000] Remove orphaned VMUL*_UNS built-ins
Thanks! Committed in revision 246046. > On Mar 10, 2017, at 1:14 PM, Segher Boessenkool > wrote: > > On Fri, Mar 10, 2017 at 12:16:31PM -0600, Bill Schmidt wrote: >> Jakub observed that these built-ins are no longer reachable (and >> haven't been for quite a while). Time to chuck them out. >> >> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with >> (surprise!) no regressions. Is this ok for trunk? > > Yes please. Thanks! > > > Segher > > >> 2017-03-10 Bill Schmidt >> >> * config/rs6000/rs6000-builtin.def (VMULEUB_UNS): Remove orphaned >> built-in. >> (VMULEUH_UNS): Likewise. >> (VMULOUB_UNS): Likewise. >> (VMULOUH_UNS): Likewise. >> * config/rs6000/rs6000.c (builtin_function_type): Remove >> references to ALTIVEC_BUILTIN_VMUL[EO]U[BH]_UNS. >
[PATCH] Build crt*vr.S with AltiVec enabled
These files won't build on targets that do not have AltiVec enabled,
breaking the build, unless we tell GAS that Altivec insns are fine.
The alternative is to not build these files in that case, which is much
more complicated.
Tested on powerpc64-linux {-m64,-m32}, powerpc64le-linux, and on
powerpc-linux-paired. Committing to trunk.
Segher
2017-03-10 Segher Boessenkool
libgcc/
* config/rs6000/crtrestvr.s: Use .machine altivec.
* config/rs6000/crtsavevr.s: Ditto.
---
libgcc/config/rs6000/crtrestvr.S | 1 +
libgcc/config/rs6000/crtsavevr.S | 1 +
2 files changed, 2 insertions(+)
diff --git a/libgcc/config/rs6000/crtrestvr.S b/libgcc/config/rs6000/crtrestvr.S
index 592a2b4..a44ab89 100644
--- a/libgcc/config/rs6000/crtrestvr.S
+++ b/libgcc/config/rs6000/crtrestvr.S
@@ -31,6 +31,7 @@
/* Called with r0 pointing just beyond the end of the vector save area. */
+ .machine altivec
.section ".text"
CFI_STARTPROC
HIDDEN_FUNC(_restvr_20)
diff --git a/libgcc/config/rs6000/crtsavevr.S b/libgcc/config/rs6000/crtsavevr.S
index 2fd54c4..bc02019 100644
--- a/libgcc/config/rs6000/crtsavevr.S
+++ b/libgcc/config/rs6000/crtsavevr.S
@@ -31,6 +31,7 @@
/* Called with r0 pointing just beyond the end of the vector save area. */
+ .machine altivec
.section ".text"
CFI_STARTPROC
HIDDEN_FUNC(_savevr_20)
--
1.9.3
Re: [PATCH] Improve VRP for IMAGPART_EXPR of ATOMIC_COMPARE_EXCHANGE ifn (PR tree-optimization/79981)
On March 10, 2017 6:56:40 PM GMT+01:00, Jakub Jelinek wrote:
>Hi!
>
>The following patch teaches VRP that IMAGPART_EXPR of
>ATOMIC_COMPARE_EXCHANGE ifn call is always 0 or 1. We cast it to bool
>afterwards, but this hint allows VRP to:
>--- pr79981.c.103t.vrp1_ 2017-03-10 15:39:29.0 +0100
>+++ pr79981.c.103t.vrp12017-03-10 15:48:15.051608067 +0100
>@@ -17,7 +17,7 @@ Value ranges after VRP:
> _1: [0, +INF]
> .MEM_2: VARYING
> _8: VARYING
>-_9: [0, +INF]
>+_9: [0, 1]
>
>
> csi (int * lock)
>@@ -32,11 +32,11 @@ csi (int * lock)
> # USE = nonlocal null
> # CLB = nonlocal null
> _8 = ATOMIC_COMPARE_EXCHANGE (lock_4(D), 0, 1, 260, 2, 0);
>- # RANGE [0, 4294967295]
>+ # RANGE [0, 1] NONZERO 1
> _9 = IMAGPART_EXPR <_8>;
> # RANGE [0, 1]
> _1 = (_Bool) _9;
>- if (_1 != 0)
>+ if (_9 != 0)
> goto ; [99.96%]
> else
> goto ; [0.04%]
>which should be beneficial e.g. for s390x code generation.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>2017-03-10 Jakub Jelinek
>
> PR tree-optimization/79981
> * tree-vrp.c (extract_range_basic): Handle IMAGPART_EXPR of
> ATOMIC_COMPARE_EXCHANGE ifn result.
> (stmt_interesting_for_vrp, vrp_visit_stmt): Handle
> IFN_ATOMIC_COMPARE_EXCHANGE.
>
>--- gcc/tree-vrp.c.jj 2017-02-22 18:15:48.0 +0100
>+++ gcc/tree-vrp.c 2017-03-10 15:45:30.574778201 +0100
>@@ -4107,7 +4107,7 @@ extract_range_basic (value_range *vr, gi
> }
> /* Handle extraction of the two results (result of arithmetics and
> a flag whether arithmetics overflowed) from {ADD,SUB,MUL}_OVERFLOW
>- internal function. */
>+ internal function. Similarly from ATOMIC_COMPARE_EXCHANGE. */
> else if (is_gimple_assign (stmt)
> && (gimple_assign_rhs_code (stmt) == REALPART_EXPR
> || gimple_assign_rhs_code (stmt) == IMAGPART_EXPR)
>@@ -4132,6 +4132,16 @@ extract_range_basic (value_range *vr, gi
> case IFN_MUL_OVERFLOW:
> subcode = MULT_EXPR;
> break;
>+ case IFN_ATOMIC_COMPARE_EXCHANGE:
>+if (code == IMAGPART_EXPR)
>+ {
>+/* This is the boolean return value whether compare and
>+ exchange changed anything or not. */
>+set_value_range (vr, VR_RANGE, build_int_cst (type, 0),
>+ build_int_cst (type, 1), NULL);
>+return;
>+ }
>+break;
> default:
> break;
> }
>@@ -7283,6 +7293,7 @@ stmt_interesting_for_vrp (gimple *stmt)
> case IFN_ADD_OVERFLOW:
> case IFN_SUB_OVERFLOW:
> case IFN_MUL_OVERFLOW:
>+case IFN_ATOMIC_COMPARE_EXCHANGE:
> /* These internal calls return _Complex integer type,
> but are interesting to VRP nevertheless. */
> if (lhs && TREE_CODE (lhs) == SSA_NAME)
>@@ -8308,6 +8319,7 @@ vrp_visit_stmt (gimple *stmt, edge *take
> case IFN_ADD_OVERFLOW:
> case IFN_SUB_OVERFLOW:
> case IFN_MUL_OVERFLOW:
>+ case IFN_ATOMIC_COMPARE_EXCHANGE:
> /* These internal calls return _Complex integer type,
> which VRP does not track, but the immediate uses
> thereof might be interesting. */
>
> Jakub
Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)
On Thu, 2017-03-09 at 21:12 -0700, Martin Sebor wrote:
> On 03/09/2017 10:45 AM, David Malcolm wrote:
> > gcc/ChangeLog:
> > PR driver/79875
> > * opts.c (parse_sanitizer_options): Add missing question mark
> > to
> > "did you mean" message.
> > ---
> > gcc/opts.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/gcc/opts.c b/gcc/opts.c
> > index 8274fab..6ea57af 100644
> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -1640,7 +1640,7 @@ parse_sanitizer_options (const char *p,
> > location_t loc, int scode,
> > if (hint)
> > error_at (loc,
> > "unrecognized argument to -f%ssanitize%s=
> > option: %q.*s;"
> > - " did you mean %qs",
> > + " did you mean %qs?",
>
> I have just an observation/question here for future consideration.
> If this sort of diagnostic is common (I count 23 instances of it)
> or if it is expected to become common, would it make sense to add
> a directive for it to the pretty printer to ensure consistency?
> I.e., to automatically prepend "; did you mean" and append the
> "?" to the new directive?
Interesting idea. The "did you mean" messages tend to be of the form:
if (hint)
error ("SOME MESSAGE; did you mean SOME_FORMAT_ARG?",
ARGS, TO, MESSAGE, HINT_THAT_MATCHES_FORMAT_ARG);
else
error ("SOME MESSAGE",
ARGS, TO, MESSAGE);
which is kind of a pain due to the duplication. It might be nicer to
eliminate the repetition by making the hint optional, replacing the
conditional with a new directive (e.g. %H):
error ("SOME MESSAGE%H",
ARGS, TO, MESSAGE, HINT);
where %H (or whatever) prepends the "; did you mean ?"
if HINT is non-NULL, and doesn't if HINT is NULL.
Is this amenable to translation?
(I'm not sure if %H is already in use).
That said, I don't know how many more of these hints we'll add; I think
we've implemented all of the obvious ones now, so this might be over
-engineering at this point.
Some warts here, in addition to the ones you note below:
* it's usually %qs for the hint (11 places), but it's sometimes %qE (3
places), and in one place (gcc.c) it's "%<-%s%>".
> My search shows the following forms:
>
> 1) Space before "?"
> gcc/c-family/c-common.c: "%<<<%> in boolean context, did you mean
> %<<%> ?");
That extra space feels like an error: the punctuation is quoted, so
it's not ambiguous.
> 2) No question mark at the end:
> gcc/git/gcc/opts.c: "unrecognized argument to -f%ssanitize%s= option:
> %q.*s;"
> " did you mean %qs",
> value ? "" : "no-",
Fixed by the patch.
> 3) Missing space after semicolon:
>
> gcc/c/c-decl.c: G_("implicit declaration of function %qE;did you mean
> %qs?"),
I'd view that as a bug.
> 4) Explicit quotes:
> gcc/gensupport.c: message_at (loc, "(did you mean \"%s\"?)",
>
> (though this won't be fixed by the suggested directive).
Indeed, this one ultimately uses vfprintf.
> Martin
[PATCH] gcov: Mark BBs that do not correspond to a line in source code (PR gcov-profile/79891).
Hello.
As briefly discussed in the PR, there are BB that do not correspond to a real
line in source
code. profile.c emits locations for all BBs that have a gimple statement
belonging to a line.
I hope these should be marked in gcov utility and not added in --all-block mode
to counts of lines.
Patch survives make check RUNTESTFLAGS="gcov.exp".
Thanks for review and feedback.
Martin
>From cc8738a287d5b0b98d61013ee065c96ed3e3cefa Mon Sep 17 00:00:00 2001
From: marxin
Date: Fri, 10 Mar 2017 20:01:06 +0100
Subject: [PATCH] gcov: Mark BBs that do not correspond to a line in source
code (PR gcov-profile/79891).
gcc/ChangeLog:
2017-03-10 Martin Liska
PR gcov-profile/79891
* gcov.c (read_graph_file): Mark BB that correspond to a real
line in source code.
(accumulate_line_counts): Do not sum these BBs.
gcc/testsuite/ChangeLog:
2017-03-10 Martin Liska
PR gcov-profile/79891
* gcc.misc-tests/gcov-17.c: New test.
---
gcc/gcov.c | 12 +---
gcc/testsuite/gcc.misc-tests/gcov-17.c | 30 ++
2 files changed, 39 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-17.c
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 4b5043c2f9f..10209b4c560 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -141,6 +141,7 @@ typedef struct block_info
/* Block is a landing pad for longjmp or throw. */
unsigned is_nonlocal_return : 1;
+ unsigned has_line_assigned: 1;
union
{
@@ -1448,6 +1449,8 @@ read_graph_file (void)
fn->num_blocks = num_blocks;
fn->blocks = XCNEWVEC (block_t, fn->num_blocks);
+ fn->blocks[ENTRY_BLOCK].has_line_assigned = 1;
+ fn->blocks[EXIT_BLOCK].has_line_assigned = 1;
for (ix = 0; ix != num_blocks; ix++)
fn->blocks[ix].flags = gcov_read_unsigned ();
}
@@ -1529,6 +1532,7 @@ read_graph_file (void)
else if (fn && tag == GCOV_TAG_LINES)
{
unsigned blockno = gcov_read_unsigned ();
+ fn->blocks[blockno].has_line_assigned = 1;
unsigned *line_nos = XCNEWVEC (unsigned, length - 1);
if (blockno >= fn->num_blocks || fn->blocks[blockno].u.line.encoding)
@@ -2458,9 +2462,11 @@ accumulate_line_counts (source_t *src)
/* Cycle detection. */
for (block = line->u.blocks; block; block = block->chain)
{
- for (arc_t *arc = block->pred; arc; arc = arc->pred_next)
- if (!line->has_block (arc->src))
- count += arc->count;
+ if (block->has_line_assigned)
+ for (arc_t *arc = block->pred; arc; arc = arc->pred_next)
+ if (!line->has_block (arc->src))
+ count += arc->count;
+
for (arc_t *arc = block->succ; arc; arc = arc->succ_next)
arc->cs_count = arc->count;
}
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-17.c b/gcc/testsuite/gcc.misc-tests/gcov-17.c
new file mode 100644
index 000..1cff708be9b
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-17.c
@@ -0,0 +1,30 @@
+/* Test gcov block mode. */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+unsigned int
+UuT (void)
+{
+ unsigned int true_var = 1;
+ unsigned int false_var = 0;
+ unsigned int ret = 0;
+
+ if (true_var) /* count(1) */
+{
+ if (false_var) /* count(1) */
+ ret = 111; /* count(#) */
+}
+ else
+ret = 999; /* count(#) */
+ return ret;
+}
+
+int
+main (int argc, char **argv)
+{
+ UuT ();
+ return 0;
+}
+
+/* { dg-final { run-gcov { -a gcov-17.c } } } */
--
2.11.1
Re: stabilize store merging
On Mar 10, 2017, Richard Biener wrote: >>> So - if we fix this can we fix it by properly sorting things? >> I don't see a way to do better, considering there's no real ID we could >> use for sorting. Do you? > Just use the ID you use but instead of maintaining a hash-map and traverse > that collect work items from the other hash-table into a vec and then sort > after the ID. Why is that better? You wrote 'hash-map' again, so I won't assume it was a mistake any more. I'm using a map for the seqno-to-chain mapping; that's a lot more predictable performance- and size-wise. Iterating over a hashmap requires one to go over all of the holes, and you get shuffled results that you then have to sort every time you'll iterate over the entries. You may have to do so multiple times, and if the number of chains is large (is that relevant in practice?) you may have to resize it multiple times. Whereas with the map I proposed, we always insert at the end, we iterate efficiently without having to do additional sorting, we most often remove from the head, and in the exceptional aliasing cases in which we remove from the middle, we do so without much regard to the element count. Now, if there's something you dislike about maps, we could make it a doubly-linked list, or maybe even a singly-linked list. I could replace seqno with a pointer to the next entry in the seq list. We'd just have to keep a pointer to the first entry instead of the newly-added map. We don't even need a pointer to the last element: we could make it work like a stack, rather than a queue, since the sequence of elements doesn't matter much, as long as we make it consistent. That would amount to more boilerplace list-maintenance code, but it's certainly doable, it costs just a pointer vs an int in the chains and an additional pointer over your suggestion, but it saves the shuffling and sorting. > I was just thinking that if we're going all the way of having IDs then > walking a hash-map of those IDs is not as good as we can do > with similar cost? You seem to consider walking a hash_map (which we did before) pretty costly, but then I'm not proposin us to do so; the performance features of a map are not the same as those of a hash_map, and I was proposing a map. Please reassess under this light, and considering the revised suggestion above. -- 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
Re: Fix IRA issue, PR79728
Ping (minus the require-effective-target line, as Uros pointed out). Bernd On 03/03/2017 02:51 PM, Bernd Schmidt wrote: This is an ICE where setup_pressure_classes fails if xmm0 is a global reg. Instead of GENERAL/FLOAT/SSE/MMX_REGS, it computes only SSE_FIRST_REG as the third register class. The problem is that the costs for moving between SSE_FIRST_REG and SSE_REGS are inflated because we think we have no available registers in SSE_FIRST_REG (since the only register in that class is global), and that somewhat confuses the algorithm. The following fixes it by tweaking contains_regs_of_mode. Out of caution, I've retained the old meaning for code in reload which uses this. Bootstrapped and tested on x86_64-linux. Ok? Bernd
Re: stabilize store merging
On Mar 10, 2017, Alexandre Oliva wrote:
> Now, if there's something you dislike about maps, we could make it a
> doubly-linked list, or maybe even a singly-linked list.
Scratch that, there's a use that may remove after a lookup by base_addr,
so a singly-linked list would require a linear walk of the list in this
case. So, doubly-linked it is, which means...
> it costs just a pointer vs an int in the chains and an additional
> pointer over your suggestion, but it saves the shuffling and sorting.
... make it two pointers instead.
How's this? Regstrapping now... Ok if it passes?
Don't let pointer randomization change the order in which we process
store chains. This may cause SSA_NAMEs to be released in different
order, and if they're reused later, they may cause differences in SSA
partitioning, leading to differences in expand, and ultimately to
different code.
bootstrap-debug-lean (-fcompare-debug) on i686-linux-gnu has failed in
haifa-sched.c since r245196 exposed the latent ordering problem in
store merging. In this case, the IR differences (different SSA names
selected for copies in out-of-SSA, resulting in some off-by-one
differences in pseudos) were not significant enough to be visible in
the compiler output.
for gcc/ChangeLog
* gimple-ssa-store-merging.c (struct imm_store_chain_info):
Add linked-list forward and backlinks. Insert on
construction, remove on destruction.
(class pass_store_merging): Add m_stores_head field.
(pass_store_merging::terminate_and_process_all_chains):
Iterate over m_stores_head list.
(pass_store_merging::terminate_all_aliasing_chains):
Likewise.
(pass_store_merging::execute): Check for debug stmts first.
Push new chains onto the m_stores_head stack.
---
gcc/gimple-ssa-store-merging.c | 65 ++--
1 file changed, 49 insertions(+), 16 deletions(-)
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 17ac94a..5bdb459 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -675,11 +675,34 @@ merged_store_group::apply_stores ()
struct imm_store_chain_info
{
+ /* Doubly-linked list that imposes an order on chain processing.
+ PNXP (prev's next pointer) points to the head of a list, or to
+ the next field in the previous chain in the list.
+ See pass_store_merging::m_stores_head for more rationale. */
+ imm_store_chain_info *next, **pnxp;
tree base_addr;
auto_vec m_store_info;
auto_vec m_merged_store_groups;
- imm_store_chain_info (tree b_a) : base_addr (b_a) {}
+ imm_store_chain_info (imm_store_chain_info *&inspt, tree b_a)
+ : next (inspt), pnxp (&inspt), base_addr (b_a)
+ {
+inspt = this;
+if (next)
+ {
+ gcc_checking_assert (pnxp == next->pnxp);
+ next->pnxp = &next;
+ }
+ }
+ ~imm_store_chain_info ()
+ {
+*pnxp = next;
+if (next)
+ {
+ gcc_checking_assert (&next == next->pnxp);
+ next->pnxp = pnxp;
+ }
+ }
bool terminate_and_process_chain ();
bool coalesce_immediate_stores ();
bool output_merged_store (merged_store_group *);
@@ -718,6 +741,17 @@ public:
private:
hash_map m_stores;
+ /* Form a doubly-linked stack of the elements of m_stores, so that
+ we can iterate over them in a predictable way. Using this order
+ avoids extraneous differences in the compiler output just because
+ of tree pointer variations (e.g. different chains end up in
+ different positions of m_stores, so they are handled in different
+ orders, so they allocate or release SSA names in different
+ orders, and when they get reused, subsequent passes end up
+ getting different SSA names, which may ultimately change
+ decisions when going out of SSA). */
+ imm_store_chain_info *m_stores_head;
+
bool terminate_and_process_all_chains ();
bool terminate_all_aliasing_chains (imm_store_chain_info **,
bool, gimple *);
@@ -730,11 +764,11 @@ private:
bool
pass_store_merging::terminate_and_process_all_chains ()
{
- hash_map::iterator iter
-= m_stores.begin ();
bool ret = false;
- for (; iter != m_stores.end (); ++iter)
-ret |= terminate_and_release_chain ((*iter).second);
+ while (m_stores_head)
+ret |= terminate_and_release_chain (m_stores_head);
+ gcc_assert (m_stores.elements () == 0);
+ gcc_assert (m_stores_head == NULL);
return ret;
}
@@ -799,15 +833,14 @@ pass_store_merging::terminate_all_aliasing_chains
(imm_store_chain_info
}
}
- hash_map::iterator iter
-= m_stores.begin ();
-
/* Check for aliasing with all other store chains. */
- for (; iter != m_stores.end (); ++iter)
+ for (imm_store_chain_info *next = m_stores_head, *cur = next; cur; cur =
next)
{
+ next = cur->next;
+
/* We already checked all the stores in chain_info and terminated the
chain
Re: [C++ Patch/RFC] PR 77752
Hi,
On 10/03/2017 16:57, Jason Merrill wrote:
On Fri, Mar 10, 2017 at 9:58 AM, Paolo Carlini wrote:
As such, the broken declaration cannot be rejected by the code we have in
finish_struct, something must happen earlier than that. It seems to me that
xref_tag_1 can be a good place, per the below patchlet, which passes testing
on x86_64-linux. I briefly wondered if making is_std_init_list stricter
would make sense instead, but I don't think so (consistently with the trail
of c++/60848 too): I believe that by the time we use is_std_init_list in the
internals we want something as simple as possible, we are assuming that
broken, fake, std::initializer_list aren't around in the translation unit.
In terms of details, I also wondered if CLASSTYPE_IS_TEMPLATE would make for
a better check, but seems unnecessarily more complex. Also, in principle, we
may want to have an even stricter check at declaration time (how many
template parameters, etc) but that seems overkilling and I don't think we
are risking ICEs because of that.
I agree with all of this. How about in pushtag_1 instead, where we
can return error_mark_node instead of aborting?
Sure. In that case the testcase for that older issue also requires
adjusting. The below passes testing, anyway.
Thanks,
Paolo.
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 246023)
+++ cp/name-lookup.c(working copy)
@@ -6207,6 +6207,15 @@ pushtag_1 (tree name, tree type, tag_scope scope)
decl = pushdecl_with_scope_1 (decl, b, /*is_friend=*/false);
if (decl == error_mark_node)
return decl;
+
+ if (DECL_CONTEXT (decl) == std_node
+ && strcmp (TYPE_NAME_STRING (type), "initializer_list") == 0
+ && !CLASSTYPE_TEMPLATE_INFO (type))
+ {
+ error ("declaration of std::initializer_list does not match "
+"#include , isn't a template");
+ return error_mark_node;
+ }
}
if (! in_class)
Index: testsuite/g++.dg/cpp0x/initlist85.C
===
--- testsuite/g++.dg/cpp0x/initlist85.C (revision 246023)
+++ testsuite/g++.dg/cpp0x/initlist85.C (working copy)
@@ -3,7 +3,7 @@
namespace std
{
- struct initializer_list {}; // { dg-message "initializer_list" }
+ struct initializer_list {}; // { dg-error "declaration" }
}
void foo(std::initializer_list &);
@@ -10,7 +10,5 @@ void foo(std::initializer_list &);
void f()
{
- foo({1, 2});
+ foo({1, 2}); // { dg-error "invalid initialization" }
}
-
-// { dg-prune-output "compilation terminated" }
Index: testsuite/g++.dg/cpp0x/initlist97.C
===
--- testsuite/g++.dg/cpp0x/initlist97.C (revision 0)
+++ testsuite/g++.dg/cpp0x/initlist97.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/77752
+// { dg-do compile { target c++11 } }
+
+namespace std {
+ class initializer_list; // { dg-error "declaration" }
+}
+void f(std::initializer_list l) { f({2}); } // { dg-error "incomplete type" }
Combiner fix for PR79910
In this PR, we have a few insns involved in two instruction combinations:
insn 16: set r100
insn 27: some calculation
insn 28: some calculation
insn 32: using r100
insn 33: using r100
insn 35: some calculation
Then we combine insns 27, 28 and 33, producing two output insns, As a
result, insn 28 (i2) now obtains a use of r100. But insn 32, which is
not involved in this combination, has the LOG_LINKS entry for that
register, and we don't know that we need to update it. As a result, the
second combination, involving regs 16, 32 and 35 (based on the remaining
LOG_LINK for r100), produces incorrect code, as we don't realize there's
a use of r100 before insn 32.
The following fixes it. Bootstrapped and tested on x86_64-linux, ok (on
all branches)?
Bernd
PR rtl-optimization/79910
* combine.c (record_used_regs): New static function.
(try_combine): Handle situations where there is an additional
instruction between I2 and I3 which needs to have a LOG_LINK
updated.
PR rtl-optimization/79910
* gcc.dg/torture/pr79910.c: New test.
Index: gcc/combine.c
===
--- gcc/combine.c (revision 245685)
+++ gcc/combine.c (working copy)
@@ -2559,6 +2560,57 @@ can_split_parallel_of_n_reg_sets (rtx_in
return true;
}
+/* Set up a set of registers used in an insn. Called through note_uses,
+ arguments as described for that function. */
+
+static void
+record_used_regs (rtx *xptr, void *data)
+{
+ bitmap set = (bitmap)data;
+ int i, j;
+ enum rtx_code code;
+ const char *fmt;
+ rtx x = *xptr;
+
+ /* repeat is used to turn tail-recursion into iteration since GCC
+ can't do it when there's no return value. */
+ repeat:
+ if (x == 0)
+return;
+
+ code = GET_CODE (x);
+ if (REG_P (x))
+{
+ unsigned regno = REGNO (x);
+ unsigned end_regno = END_REGNO (x);
+ while (regno < end_regno)
+ bitmap_set_bit (set, regno++);
+ return;
+}
+
+ /* Recursively scan the operands of this expression. */
+
+ for (i = GET_RTX_LENGTH (code) - 1, fmt = GET_RTX_FORMAT (code); i >= 0; i--)
+{
+ if (fmt[i] == 'e')
+ {
+ /* If we are about to do the last recursive call
+ needed at this level, change it into iteration.
+ This function is called enough to be worth it. */
+ if (i == 0)
+ {
+ x = XEXP (x, 0);
+ goto repeat;
+ }
+
+ record_used_regs (&XEXP (x, i), data);
+ }
+ else if (fmt[i] == 'E')
+ for (j = 0; j < XVECLEN (x, i); j++)
+ record_used_regs (&XVECEXP (x, i, j), data);
+}
+}
+
/* Try to combine the insns I0, I1 and I2 into I3.
Here I0, I1 and I2 appear earlier than I3.
I0 and I1 can be zero; then we combine just I2 into I3, or I1 and I2 into
@@ -2742,6 +2794,21 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
added_links_insn = 0;
+ auto_bitmap i2_regset, i3_regset, links_regset;
+ note_uses (&PATTERN (i2), record_used_regs, (bitmap)i2_regset);
+ note_uses (&PATTERN (i3), record_used_regs, (bitmap)i3_regset);
+ insn_link *ll;
+ FOR_EACH_LOG_LINK (ll, i3)
+bitmap_set_bit (links_regset, ll->regno);
+ FOR_EACH_LOG_LINK (ll, i2)
+bitmap_set_bit (links_regset, ll->regno);
+ if (i1)
+ FOR_EACH_LOG_LINK (ll, i1)
+ bitmap_set_bit (links_regset, ll->regno);
+ if (i0)
+FOR_EACH_LOG_LINK (ll, i0)
+ bitmap_set_bit (links_regset, ll->regno);
+
/* First check for one important special case that the code below will
not handle. Namely, the case where I1 is zero, I2 is a PARALLEL
and I3 is a SET whose SET_SRC is a SET_DEST in I2. In that case,
@@ -4051,6 +4124,33 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
return 0;
}
+ auto_bitmap new_regs_in_i2;
+ if (newi2pat)
+{
+ /* We need to discover situations where we introduce a use of a
+ register into I2, where none of the existing LOG_LINKS contain
+ a reference to it. This can happen if previously I3 referenced
+ the reg, and there is an additional use between I2 and I3. We
+ must remove the LOG_LINKS entry from that additional use and
+ distribute it along with our own ones. */
+ note_uses (&newi2pat, record_used_regs, (bitmap)new_regs_in_i2);
+ bitmap_and_compl_into (new_regs_in_i2, i2_regset);
+ bitmap_and_compl_into (new_regs_in_i2, links_regset);
+
+ /* Here, we first look for situations where a hard register use
+ moved, and just give up. This should happen approximately
+ never, and it's not worth it to deal with possibilities like
+ multi-word registers. Later, when fixing up LOG_LINKS, we
+ deal with the case where a pseudo use moved. */
+ if (prev_nonnote_insn (i3) != i2)
+ for (unsigned r = 0; r < FIRST_PSEUDO_REGISTER; r++)
+ if (bitmap_bit_p (new_regs_in_i2, r))
+ {
+ undo_all ();
+ return 0;
+ }
+}
+
if (MAY_HAVE_DEBUG_INSNS)
{
struct undo *undo;
@@ -4494,6 +4594,45 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
NULL_RTX, NULL_RTX, NULL_RTX);
}
+if (ne
Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
On Fri, 2017-03-10 at 09:24 +, Kyrill Tkachov wrote:
> On 10/03/17 06:24, Jakub Jelinek wrote:
> > On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
> > > gcc/ChangeLog:
> > > PR translation/79923
> > > * auto-profile.c (get_combined_location): Convert leading
> > > character of diagnostics to lower case and remove trailing
> > > period.
> > > (read_profile): Likewise for various diagnostics.
> > > * config/arm/arm-builtins.c (arm_expand_builtin): Remove
> > > trailing
> > > period from various diagnostics.
> > > * config/arm/arm.c (arm_option_override): Likewise.
> > > * config/msp430/msp430.c (msp430_expand_delay_cycles):
> > > Likewise.
> > > (msp430_expand_delay_cycles): Likewise.
> > Mostly ok, but for
> >
> > > --- a/gcc/config/arm/arm-builtins.c
> > > +++ b/gcc/config/arm/arm-builtins.c
> > > @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
> > > && (imm < 0 || imm > 32))
> > > {
> > > if (fcode == ARM_BUILTIN_WRORHI)
> > > - error ("the range of count should be in 0 to 32.
> > > please check the intrinsic _mm_rori_pi16 in code.");
> > > + error ("the range of count should be in 0 to 32.
> > > please check the intrinsic _mm_rori_pi16 in code");
> > I wonder if this shouldn't use a semicolon space in the middle
> > instead of dot space space (many times in the same file).
>
> Is there a convention in GCC to use semicolons?
> I'm okay with changing it to a semicolon (it's slightly better IMO)
> as long as it's consistent
> with the style GCC uses.
>
> > Also, for the benefit of translators, this might be better done as
> > error ("the range of count should be in 0 to 32; please
> > check the intrinsic %s in code",
> >"_mm_rori_pi16");
> > so that there are fewer of these messages.
> > Adding some ARM folks on this.
>
> These iWMMXt builtins haven't been touched in ages and could do with
> some TLC in general.
> For example, I'm not a fan of having all these "please check the
> intrinsic ..." messages.
> If we've got a reference to the tree we're expanding, isn't there
> some kind of error function
> that will point to the location in the source that's causing the
> error? I'd rather use that than
> hardcoding the intrinsic names. This would also allow us to collapse
> all these
> if (fcode == <...>)
>error (...);
> else if (fcode == <...>)
>error (...);
> else if ...
>
> constructs.
>
> While we're at it:
>
> if (fcode == ARM_BUILTIN_WSRLHI)
> - error ("the count should be no less than 0. please
> check the intrinsic _mm_srli_pi16 in code.");
> + error ("the count should be no less than 0. please
> check the intrinsic _mm_srli_pi16 in code");
>
>
> Let's use "the count should be a non-negative integer" to be
> consistent with the error reporting
> for UInteger error messages.
>
> > Perhaps commit everything except arm-builtins.c separately and deal
> > with
> > this part with the ARM folks?
>
> I agree. David, if you want to clean up the error reporting in these
> intrinsics as a separate patch I'd be grateful.
> Otherwise, could you please open a bugzilla ticket so we can track
> this?
I started looking at this, but realized I don't have the arm ISA
expertise to touch this code (sorry), so I filed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79995
instead.
Re: C PATCH to tidy implicit_decl_warning
On Mon, 6 Mar 2017, Marek Polacek wrote: > I didn't like that the function implicit_decl_warning is > a) missing a comment, > b) missing braces in an if were confusing, > b) wrongly formatted, >else > if () >is ugly. > > This patches fixes the above. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2017-03-06 Marek Polacek > > * c-decl.c (implicit_decl_warning): Add a comment. Fix formatting. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 0/7] Various i18n fixes (and questions)
On Thu, 9 Mar 2017, David Malcolm wrote: > However, we're deep in stage 4 of the development cycle. Looking at > our development plan > https://gcc.gnu.org/develop.html > it's not clear to me how such changes fit into our schedule: the plan > seems to make no mention of how i18n and translation fit in to the > stages (it talks about bugs and documentation, but not about translatable > strings). > > Do we have a "string freeze" in our schedule? i.e. is there a point > at which we avoid changing strings to avoid disrupting things for > translators? We don't have a string freeze; these strings are functionally considered much like documentation. > Also, from a developer POV, when should we regenerate and check-in > the .pot files? The rules for submitting patches: > https://gcc.gnu.org/contribute.html > and for committing them: > https://gcc.gnu.org/svnwrite.html > seem to make no mention of gettext and .pot files. > > Looking in libcpp/po/ChangeLog and gcc/po/ChangeLog I see that > Joseph [CCed] seems to regularly commit regenerated copies of the > .pot files; do we have a policy about this? See translation.html. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 2/7] aarch64.c: tweaks to quoting in error messages (PR target/79925)
On Thu, 9 Mar 2017, David Malcolm wrote: > gcc/ChangeLog: > PR target/79925 > * config/aarch64/aarch64.c (aarch64_validate_mcpu): Quote the > full command-line argument, rather than just "str". > (aarch64_validate_march): Likewise. > (aarch64_validate_mtune): Likewise. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, doc] Revise GCC manual section 6.11, Additional Floating Types
On Thu, 9 Mar 2017, Bill Schmidt wrote:
> The one statement I've made that I'm not quite confident is that
> __float128 is an alias for _Float64x on hppa HP-UX. This was not
> clear in the original text, so I'd appreciate confirmation or
> correction on this point.
It can't simultaneously be an alias for _Float128 and _Float64x (those are
always distinct types, which may or may not be ABI-equivalent).
Actually, in pa/pa.c, __float128 is defined as an alias for long double
(not _Float128 or _Float64x). On x86/x86_64, and powerpc (when
supported), and non-HP-UX ia64, it's an alias for _Float128; on ia64-hpux
it's an alias for long double.
> +@itemize @bullet
> +@item @code{__float128} is available on i386, x86_64, IA-64, and
> +hppa HP-UX, as well as on PowerPC 64-bit Linux targets that enable
"GNU/Linux".
> +@code{_Float64} and @code{Float32x} types are supported on all systems
@code{_Float32x} (pre-existing issue).
> +@code{TFmode} maps to a 128-bit floating-point type, which is usually
> +@code{__float128}. On PowerPC, where a transition is underway for
> +@code{long double} from the @code{__ibm128} type to @code{__float128}
> +in future releases, @code{TFmode} refers to the type that represents
> +@code{long double} during compilation. @code{KFmode} is always
> +@code{__float128}, @code{IFmode} is always @code{__ibm128}, and @code{TFmode}
> +will always be one or the other.
Machine modes are largely an implementation detail (only user-visible
using the mode attribute); I'm not convinced this documentation belongs
here at all (although a limited amount of documentation of declaring with
mode attributes is needed to describe how to declare the complex ibm128
type).
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH 6/7 v2] i386.c: make "sorry" message more amenable to translation (PR target/79926)
On Fri, 2017-03-10 at 07:33 +0100, Jakub Jelinek wrote:
> On Thu, Mar 09, 2017 at 12:45:28PM -0500, David Malcolm wrote:
> > PR target/79926 notes that in:
> >
> > sorry ("%s instructions aren't allowed in %s service routine",
> >isa, (cfun->machine->func_type == TYPE_EXCEPTION
> > ? "exception" : "interrupt"));
> >
> > the text from the second %s won't be translated, but should be.
> >
> > This patch reworks the diagnostic by breaking it out into two
> > messages
> > and marking them with G_() so they're seen by xgettext; a test run
> > of
> > xgettext confirms that both messages make it into po/gcc.pot (in an
> > earlier version of the patch I attempted to do it without
> > introducing
> > a local, but xgettext only picked up on one of the strings).
> >
> > gcc/ChangeLog:
> > PR target/79926
> > * config/i386/i386.c (ix86_set_current_function): Make "sorry"
> > message more amenable to translation.
> > ---
> > gcc/config/i386/i386.c | 12 +---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index e705a3e..b8688e3 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -7271,9 +7271,15 @@ ix86_set_current_function (tree fndecl)
> >if (isa != NULL)
> > {
> > if (cfun->machine->func_type != TYPE_NORMAL)
> > - sorry ("%s instructions aren't allowed in %s service
> > routine",
> > - isa, (cfun->machine->func_type ==
> > TYPE_EXCEPTION
> > -? "exception" : "interrupt"));
> > + {
> > + const char *msgid
> > + = (cfun->machine->func_type == TYPE_EXCEPTION
> > + ? G_("%s instructions aren't allowed in"
> > + " exception service routine")
> > + : G_("%s instructions aren't allowed in"
> > + " interrupt service routine"));
> > + sorry (msgid, isa);
>
> 1) aren't should be actually aren%'t
>(we should probably look through gcc.pot and patch all spots that
>have n't instead of n%'t in them and are in the gcc-internal
> -format)
> 2) I think it should be better to do:
> sorry (cfun->machine->func_type == TYPE_EXCEPTION
>? G_("%s instructions aren%'t allowed in exception
> "
> "service routine")
>: G_("%s instructions aren%'t allowed in interrupt
> "
> "service routine"));
>That way, you don't introduce another -Wformat-security issue
> Ok for trunk with those changes.
Thanks.
Martin pointed out that the wording of these messages could be improved
by adding an article, and by adding quotes to "no_caller_saved_registers"
Here's a revised version that makes those changes (in addition to
the ones you suggested).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/ChangeLog:
PR target/79926
* config/i386/i386.c (ix86_set_current_function): Make "sorry"
messages more amenable to translation, and improve wording.
gcc/testsuite/ChangeLog:
PR target/79926
* gcc.target/i386/interrupt-387-err-1.c: Update expected message.
* gcc.target/i386/interrupt-387-err-2.c: Likewise.
* gcc.target/i386/interrupt-bnd-err-1.c: Likewise.
* gcc.target/i386/interrupt-bnd-err-2.c: Likewise.
* gcc.target/i386/interrupt-mmx-err-1.c: Likewise.
* gcc.target/i386/interrupt-mmx-err-2.c: Likewise.
---
gcc/config/i386/i386.c | 13 -
gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c | 4 ++--
gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c | 2 +-
gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c | 4 ++--
gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c | 2 +-
gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c | 4 ++--
gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c | 2 +-
7 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e705a3e..9fbf8d0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -7271,12 +7271,15 @@ ix86_set_current_function (tree fndecl)
if (isa != NULL)
{
if (cfun->machine->func_type != TYPE_NORMAL)
- sorry ("%s instructions aren't allowed in %s service routine",
- isa, (cfun->machine->func_type == TYPE_EXCEPTION
-? "exception" : "interrupt"));
+ sorry (cfun->machine->func_type == TYPE_EXCEPTION
+ ? G_("%s instructions aren%'t allowed in an"
+ " exception service routine")
+ : G_("%s instructions aren%'t allowed in an"
+ " interrupt service routine"),
+ isa);
else
- sorry ("%s instructions aren't allowed in function with "
- "no_caller_saved_registers attribute
Re: [PATCH 1/5] testsuite: attr-alloc_size-11.c (PR79356)
On 03/10/2017 09:20 AM, Martin Sebor wrote:
On 03/10/2017 05:57 AM, Rainer Orth wrote:
Hi Segher,
On Fri, Feb 10, 2017 at 11:56:39AM +0100, Rainer Orth wrote:
Segher Boessenkool writes:
As stated in the PR, this test now passes on aarch64, ia64, powerpc,
and s390x. This patch disables the xfails for those targets.
As I'd mentioned in PR tree-optimization/78775, the test XPASSes on
SPARC, too.
Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk?
[...]
2017-02-09 Segher Boessenkool
gcc/testsuite/
PR testsuite/79356
* gcc.dg/attr-alloc_size-11.c: Don't xfail on aarch64, ia64,
powerpc,
or s390x.
TBH, I'd strongly prefer to have a proper analysis instead of just
un-xfail-ing the test on an ever growing apparently random list of
targets.
Yeah, I agree. I thought it was just another test that stopped failing,
but it seems to fail in two ways now, making the testcase succeed?
Lovely. Please consider this patch withdrawn.
I just noticed that nothing has happened at all in a month, so anything
is better than the tests XPASSing on a number of targets.
So the patch is ok for mainline with sparc*-*-* added to the target
lists and a reference to PR testsuite/79356 in the comment.
I'd still be very grateful if Martin could have a look what's really
going on here, though.
Sorry, I haven't had a chance to look more deeply into why these
assertions pass on some targets and fail on others. There is at
least one bug that tracks a VRP problem that manifests only on
some targets and not others (79054). I don't know if this is
a symptom of the same bug or something different. I'll see if
I can find some time to isolate it.
It could well be a BRANCH_COST effect. BRANCH_COST is probably the most
annoying target property that bleeds into the tree/gimple world. From
looking at the gimple in the BZ that could well be the case.
See logical_op_short_circuit for how this is often dealt with in the
testsuite.
jeff
