Re: please approval my patch - add new logical traits to type_traits for logical completeness
(removing gcc-testresults@ which is for (automated) results of running the testsuite, not for patch submission) On Sat, 9 Feb 2019, 李苏旺 wrote: I have a patch about libstdc++ include/std/type_traits , testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc testsuite/20_util/logical_traits/requirements/typedefs.cc testsuite/20_util/logical_traits/value.cc, the patch want to add new logical traits , such as exclusive_or , not_exclusive_or, not_conjunction and not_disjunction for logical completeness , the detail patch as below: Hello, could you say where in the C++ standard these are specified? What paper added them, and what is the corresponding feature macro? If those are traits that you invented yourself, you will need to get them into the standard before we can add them to libstdc++. -- Marc Glisse
please approval my patch - add new logical traits to type_traits for logical completeness
hi all,
I have a patch about libstdc++
include/std/type_traits ,
testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc
testsuite/20_util/logical_traits/requirements/typedefs.cc
testsuite/20_util/logical_traits/value.cc,
the patch want to add new logical traits , such as exclusive_or ,
not_exclusive_or, not_conjunction and not_disjunction for logical
completeness , the detail patch as below:
Index: ChangeLog
===
--- ChangeLog (revision 268714)
+++ ChangeLog (working copy)
@@ -1,3 +1,12 @@
+2019-02-09 lisuwang
+ * include/std/type_traits(__xor_<..>, excluesive_or<...>,
not_exclusive_or<...>, not_conjunction<...>, not_disjunction<...>): Define.
+ * testsuite/20_util/logical_traits/value.cc: Add
+ additional cases for std::not_conjunction, std::not_disjunction,
std::excluesive_or and std::not_exclusive_or.
+ *
testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc: Add
+ explicit_instantiation of std::not_conjunction, std::not_disjunction,
std::excluesive_or and std::not_exclusive_or.
+ * testsuite/20_util/logical_traits/requirements/typedefs.cc: Add
+ the additional test cases to verify the typedefs.
+
2019-02-09 Jonathan Wakely
PR libstdc++/71044
Index: include/std/type_traits
===
--- include/std/type_traits (revision 268714)
+++ include/std/type_traits (working copy)
@@ -142,6 +142,29 @@
: public __bool_constant
{ };
+ template
+struct __xor_;
+
+ template<>
+ struct __xor_<>
+ : public true_type
+ { };
+
+ template
+struct __xor_<_B1>
+ : public _B1
+ { };
+
+ template
+struct __xor_<_B1, _B2>
+ : public __and_<__or_<_B1, _B2>,__not_<__and_<_B1, _B2>>>
+ { };
+
+ template
+struct __xor_<_B1, _B2,_B3,_Bn...>
+ : public __and_<__or_<_B1,__xor_<_B2,_B3,_Bn...>>,
__not_<__and_<_B1,__xor_<_B2,_B3,_Bn...
+ { };
+
#if __cplusplus >= 201703L
template
@@ -148,6 +171,10 @@
inline constexpr bool __or_v = __or_<_Bn...>::value;
template
inline constexpr bool __and_v = __and_<_Bn...>::value;
+ template
+inline constexpr bool __not_v = __not_<_Bn...>::value;
+ template
+inline constexpr bool __xor_v = __xor_<_Bn...>::value;
#define __cpp_lib_logical_traits 201510
@@ -165,8 +192,27 @@
struct negation
: __not_<_Pp>
{ };
-
template
+struct exclusive_or
+ : __xor_<_Bn...>
+ { };
+
+ template
+struct not_conjunction
+ : __not_<__and_<_Bn...>
+ { };
+
+ template
+struct not_disjunction
+ : __not_<__or_<_Bn...>>
+ { };
+
+ template
+struct not_exclusive_or
+ : __not_<__xor_<_Bn...>>
+ { };
+
+ template
inline constexpr bool conjunction_v = conjunction<_Bn...>::value;
template
@@ -174,6 +220,18 @@
template
inline constexpr bool negation_v = negation<_Pp>::value;
+
+ template
+inline constexpr bool exclusive_or_v = exclusive_or<_Bn...>::value;
+
+ template
+inline constexpr bool not_conjunction_v =
not_conjunction<_Bn...>::value;
+
+ template
+inline constexpr bool not_disjunction_v =
not_disjunction<_Bn...>::value;
+
+ template
+inline constexpr bool not_exclusive_or_v =
not_exclusive_or<_Bn...>::value;
#endif // C++17
Index:
testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc
===
--- testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc
(revision
268714)
+++ testsuite/20_util/logical_traits/requirements/explicit_instantiation.cc
(working
copy)
@@ -27,4 +27,8 @@
template struct conjunction;
template struct disjunction;
template struct negation;
+ template struct excluesive_or;
+ template struct not_exclusive_or;
+ template struct not_conjunction;
+ template struct not_disjunction;
}
Index: testsuite/20_util/logical_traits/requirements/typedefs.cc
===
--- testsuite/20_util/logical_traits/requirements/typedefs.cc (revision
268714)
+++ testsuite/20_util/logical_traits/requirements/typedefs.cc (working copy)
@@ -53,3 +53,43 @@
typedef test_type::type::value_type type_value_type;
typedef test_type::type::type type_type;
}
+
+void test04()
+{
+ // Check for required typedefs
+ typedef std::not_conjunction
test_type;
+ typedef test_type::value_type value_type;
+ typedef test_type::type type;
+ typedef test_type::type::value_type type_value_type;
+ typedef test_type::type::type type_type;
+}
+
+void test05()
+{
+ // Check for required typedefs
+ typedef std::not_disjunction
test_type;
+ typedef test_type::value_type value_type;
+ typedef test_type::type type;
+ typedef test_type::type::value_type type_value_type;
+ typedef test_type::type::type type_type;
+}
+
+void test06()
+{
+ // Check
[PATCH] PR libstdc++/88066 use <> for includes not ""
Using #include "..." to include a header in the same directory fails if
the user compiles with -I-, so always use something like for
internal headers.
I haven't added tests for this, because dg-options adds options to the
end, and the position of -I- matters (if it's at the end then the tests
won't find any headers in the build tree, as they're specified by -I
options earlier in the flags). It's been manually tested though.
PR libstdc++/88066
* include/bits/locale_conv.h: Use <> for includes not "".
* include/ext/random: Likewise.
* include/ext/vstring.h: Likewise.
Apparently I never sent this to the mailing lists. It was committed to
trunk a month ago, and now to gcc-8-branch too.
commit 9441a76de071a91258f5eee27b55779822ab842b
Author: redi
Date: Sat Feb 9 00:40:31 2019 +
PR libstdc++/88066 use <> for includes not ""
Using #include "..." to include a header in the same directory fails if
the user compiles with -I-, so always use something like for
internal headers.
I haven't added tests for this, because dg-options adds options to the
end, and the position of -I- matters (if it's at the end then the tests
won't find any headers in the build tree, as they're specified by -I
options earlier in the flags). It's been manually tested though.
PR libstdc++/88066
* include/bits/locale_conv.h: Use <> for includes not "".
* include/ext/random: Likewise.
* include/ext/vstring.h: Likewise.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-8-branch@268714
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/libstdc++-v3/include/bits/locale_conv.h
b/libstdc++-v3/include/bits/locale_conv.h
index bc5669f2521..72680b94030 100644
--- a/libstdc++-v3/include/bits/locale_conv.h
+++ b/libstdc++-v3/include/bits/locale_conv.h
@@ -35,10 +35,10 @@
#else
#include
-#include "stringfwd.h"
-#include "allocator.h"
-#include "codecvt.h"
-#include "unique_ptr.h"
+#include
+#include
+#include
+#include
namespace std _GLIBCXX_VISIBILITY(default)
{
diff --git a/libstdc++-v3/include/ext/random b/libstdc++-v3/include/ext/random
index 0a98b350925..8ae3bde777b 100644
--- a/libstdc++-v3/include/ext/random
+++ b/libstdc++-v3/include/ext/random
@@ -3778,8 +3778,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace __gnu_cxx
-#include "ext/opt_random.h"
-#include "random.tcc"
+#include
+#include
#endif // _GLIBCXX_USE_C99_STDINT_TR1 && UINT32_C
diff --git a/libstdc++-v3/include/ext/vstring.h
b/libstdc++-v3/include/ext/vstring.h
index 605311e9a28..e45e774e825 100644
--- a/libstdc++-v3/include/ext/vstring.h
+++ b/libstdc++-v3/include/ext/vstring.h
@@ -2962,6 +2962,6 @@ _GLIBCXX_END_NAMESPACE_VERSION
#endif // C++11
-#include "vstring.tcc"
+#include
#endif /* _VSTRING_H */
[PATCH] i386: Use EXT_REX_SSE_REG_P in *movoi_internal_avx/movti_internal
On Fri, Feb 8, 2019 at 3:28 AM H.J. Lu wrote:
>
> On Fri, Feb 8, 2019 at 1:51 AM Uros Bizjak wrote:
> >
> > On Thu, Feb 7, 2019 at 10:11 PM H.J. Lu wrote:
> > >
> > > OImode and TImode moves must be done in XImode to access upper 16
> > > vector registers without AVX512VL. With AVX512VL, we can access
> > > upper 16 vector registers in OImode and TImode.
> > >
> > > PR target/89229
> > > * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for
> > > upper 16 vector registers without TARGET_AVX512VL.
> > > (*movti_internal): Likewise.
> >
> > Please use (not (match_test "...")) instead of (match_test "!...") and
> > put the new test as the first argument of the AND rtx.
> >
> > LGTM with the above change.
>
> This is the patch I am checking in.
>
> Thanks.
>
> H.J.
> ---
> OImode and TImode moves must be done in XImode to access upper 16
> vector registers without AVX512VL. With AVX512VL, we can access
> upper 16 vector registers in OImode and TImode.
>
> PR target/89229
> * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for
> upper 16 vector registers without TARGET_AVX512VL.
> (*movti_internal): Likewise.
> ---
> gcc/config/i386/i386.md | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index c1492363bca..3d9141ae450 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -1933,8 +1933,9 @@
> (set_attr "type" "sselog1,sselog1,ssemov,ssemov")
> (set_attr "prefix" "vex")
> (set (attr "mode")
> - (cond [(ior (match_operand 0 "ext_sse_reg_operand")
> - (match_operand 1 "ext_sse_reg_operand"))
> + (cond [(and (not (match_test "TARGET_AVX512VL"))
> + (ior (match_operand 0 "ext_sse_reg_operand")
> + (match_operand 1 "ext_sse_reg_operand")))
> (const_string "XI")
> (and (eq_attr "alternative" "1")
> (match_test "TARGET_AVX512VL"))
> @@ -2012,8 +2013,9 @@
> (set (attr "mode")
> (cond [(eq_attr "alternative" "0,1")
> (const_string "DI")
> -(ior (match_operand 0 "ext_sse_reg_operand")
> - (match_operand 1 "ext_sse_reg_operand"))
> +(and (not (match_test "TARGET_AVX512VL"))
> + (ior (match_operand 0 "ext_sse_reg_operand")
> + (match_operand 1 "ext_sse_reg_operand")))
> (const_string "XI")
> (and (eq_attr "alternative" "3")
> (match_test "TARGET_AVX512VL"))
> --
Also need this patch since we no longer set MODE_XI for
AVX512VL.
--
H.J.
From 4d15af334ccde161863ff136e76ccbbd0243082c Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Fri, 8 Feb 2019 16:20:49 -0800
Subject: [PATCH] i386: Use EXT_REX_SSE_REG_P in
*movoi_internal_avx/movti_internal
We should use EXT_REX_SSE_REG_P to check upper 16 vector registers.
PR target/89229
* config/i386/i386.md (*movoi_internal_avx): Check
EXT_REX_SSE_REG_P instead of MODE_XI for upper 16 vector
registers.
(*movti_internal): Likewise.
---
gcc/config/i386/i386.md | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index df0e9c0a1a7..cda973c0fbf 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1923,7 +1923,8 @@
{
if (get_attr_mode (insn) == MODE_V8SF)
return "vmovups\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqu32\t{%1, %0|%0, %1}";
else
return "vmovdqu\t{%1, %0|%0, %1}";
@@ -1932,7 +1933,8 @@
{
if (get_attr_mode (insn) == MODE_V8SF)
return "vmovaps\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqa32\t{%1, %0|%0, %1}";
else
return "vmovdqa\t{%1, %0|%0, %1}";
@@ -1986,7 +1988,8 @@
{
if (get_attr_mode (insn) == MODE_V4SF)
return "%vmovups\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqu32\t{%1, %0|%0, %1}";
else
return "%vmovdqu\t{%1, %0|%0, %1}";
@@ -1995,7 +1998,8 @@
{
if (get_attr_mode (insn) == MODE_V4SF)
return "%vmovaps\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqa32\t{%1, %0|%0, %1}";
else
return "%vmovdqa\t{%1, %0|%0, %1}";
--
2.20.1
[PATCH] Add noexcept to filesystem::path query functions
In the standard these member functions are specified in terms of the
potentially-throwing path decompositions functions, but we implement
them without constructing any new paths or doing anything else that can
throw.
PR libstdc++/71044
* include/bits/fs_path.h (path::has_root_name)
(path::has_root_directory, path::has_root_path)
(path::has_relative_path, path::has_parent_path)
(path::has_filename, path::has_stem, path::has_extension)
(path::is_absolute, path::is_relative, path::_M_find_extension): Add
noexcept.
* src/c++17/fs_path.cc (path::has_root_name)
(path::has_root_directory, path::has_root_path)
(path::has_relative_path, path::has_parent_path)
(path::has_filename, path::_M_find_extension): Add noexcept.
Tested powerpc64le-linux, committed to trunk.
commit 9c708e630603652958c5866c58d90bf01b2795ac
Author: Jonathan Wakely
Date: Fri Feb 8 23:55:34 2019 +
Add noexcept to filesystem::path query functions
In the standard these member functions are specified in terms of the
potentially-throwing path decompositions functions, but we implement
them without constructing any new paths or doing anything else that can
throw.
PR libstdc++/71044
* include/bits/fs_path.h (path::has_root_name)
(path::has_root_directory, path::has_root_path)
(path::has_relative_path, path::has_parent_path)
(path::has_filename, path::has_stem, path::has_extension)
(path::is_absolute, path::is_relative, path::_M_find_extension): Add
noexcept.
* src/c++17/fs_path.cc (path::has_root_name)
(path::has_root_directory, path::has_root_path)
(path::has_relative_path, path::has_parent_path)
(path::has_filename, path::_M_find_extension): Add noexcept.
diff --git a/libstdc++-v3/include/bits/fs_path.h
b/libstdc++-v3/include/bits/fs_path.h
index 37dcfc16703..98b8dc08a6e 100644
--- a/libstdc++-v3/include/bits/fs_path.h
+++ b/libstdc++-v3/include/bits/fs_path.h
@@ -359,16 +359,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
// query
[[nodiscard]] bool empty() const noexcept { return _M_pathname.empty(); }
-bool has_root_name() const;
-bool has_root_directory() const;
-bool has_root_path() const;
-bool has_relative_path() const;
-bool has_parent_path() const;
-bool has_filename() const;
-bool has_stem() const;
-bool has_extension() const;
-bool is_absolute() const;
-bool is_relative() const { return !is_absolute(); }
+bool has_root_name() const noexcept;
+bool has_root_directory() const noexcept;
+bool has_root_path() const noexcept;
+bool has_relative_path() const noexcept;
+bool has_parent_path() const noexcept;
+bool has_filename() const noexcept;
+bool has_stem() const noexcept;
+bool has_extension() const noexcept;
+bool is_absolute() const noexcept;
+bool is_relative() const noexcept { return !is_absolute(); }
// generation
path lexically_normal() const;
@@ -433,7 +433,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
void _M_append(basic_string_view);
void _M_concat(basic_string_view);
-pair _M_find_extension() const;
+pair _M_find_extension() const noexcept;
template
struct _Cvt;
@@ -1102,21 +1102,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
}
inline bool
- path::has_stem() const
+ path::has_stem() const noexcept
{
auto ext = _M_find_extension();
return ext.first && ext.second != 0;
}
inline bool
- path::has_extension() const
+ path::has_extension() const noexcept
{
auto ext = _M_find_extension();
return ext.first && ext.second != string_type::npos;
}
inline bool
- path::is_absolute() const
+ path::is_absolute() const noexcept
{
#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
return has_root_name() && has_root_directory();
diff --git a/libstdc++-v3/src/c++17/fs_path.cc
b/libstdc++-v3/src/c++17/fs_path.cc
index db6a1cb29d8..268b5621509 100644
--- a/libstdc++-v3/src/c++17/fs_path.cc
+++ b/libstdc++-v3/src/c++17/fs_path.cc
@@ -1530,7 +1530,7 @@ path::parent_path() const
}
bool
-path::has_root_name() const
+path::has_root_name() const noexcept
{
if (_M_type() == _Type::_Root_name)
return true;
@@ -1540,7 +1540,7 @@ path::has_root_name() const
}
bool
-path::has_root_directory() const
+path::has_root_directory() const noexcept
{
if (_M_type() == _Type::_Root_dir)
return true;
@@ -1556,7 +1556,7 @@ path::has_root_directory() const
}
bool
-path::has_root_path() const
+path::has_root_path() const noexcept
{
if (_M_type() == _Type::_Root_name || _M_type() == _Type::_Root_dir)
return true;
@@ -1570,7 +1570,7 @@ path::has_root_path() const
}
bool
-path::has_relative_path() const
+path::has_relative_path() const noexcept
{
if (_M_type() == _Type::_Filename && !_M_pathname.empty())
return
[C++ PATCH] Fix std::is_constant_evaluated() in non-type template parameters (PR c++/88977)
Hi!
Non-type template arguments are constant-expression in the grammar and thus
manifestly constant-evaluated.
For e.g. class templates, convert_nontype_argument is called with
tf_warning_or_error and so while we called in the below spots
maybe_constant_value without manifestly_const_eval=true, there is a
if (TREE_CODE (expr) != INTEGER_CST
&& !value_dependent_expression_p (expr))
{
if (complain & tf_error)
{
int errs = errorcount, warns = warningcount + werrorcount;
if (!require_potential_constant_expression (expr))
expr = error_mark_node;
else
expr = cxx_constant_value (expr);
later on and cxx_constant_value will do the manifestly_const_eval=true.
On the testcase below with function template, complain is tf_none and
so we only call that maybe_constant_value and not cxx_constant_value.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2019-02-08 Jakub Jelinek
PR c++/88977
* pt.c (convert_nontype_argument): Pass true as manifestly_const_eval
to maybe_constant_value calls.
* g++.dg/cpp2a/is-constant-evaluated7.C: New test.
--- gcc/cp/pt.c.jj 2019-02-05 10:04:19.038028029 +0100
+++ gcc/cp/pt.c 2019-02-08 09:46:23.370257199 +0100
@@ -6821,12 +6821,14 @@ convert_nontype_argument (tree type, tre
/* Make sure we return NULL_TREE only if we have really issued
an error, as described above. */
return (complain & tf_error) ? NULL_TREE : error_mark_node;
- expr = maybe_constant_value (expr);
+ expr = maybe_constant_value (expr, NULL_TREE,
+ /*manifestly_const_eval=*/true);
expr = convert_from_reference (expr);
}
else if (TYPE_PTR_OR_PTRMEM_P (type))
{
- tree folded = maybe_constant_value (expr);
+ tree folded = maybe_constant_value (expr, NULL_TREE,
+ /*manifestly_const_eval=*/true);
if (TYPE_PTR_P (type) ? integer_zerop (folded)
: null_member_pointer_value_p (folded))
expr = folded;
--- gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated7.C.jj 2019-02-08
09:53:47.255935430 +0100
+++ gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated7.C 2019-02-08
09:49:45.159957823 +0100
@@ -0,0 +1,18 @@
+// P0595R2
+// PR c++/88977
+// { dg-do compile { target c++11 } }
+
+namespace std {
+ constexpr inline bool
+ is_constant_evaluated () noexcept
+ {
+return __builtin_is_constant_evaluated ();
+ }
+}
+
+template constexpr bool foo () { return B; }
+
+constexpr bool x = foo ();
+constexpr bool y = foo<__builtin_is_constant_evaluated ()> ();
+static_assert (x, "");
+static_assert (y, "");
Jakub
Re: [PATCH] driver: Also prune joined switches with negation
On Fri, Feb 8, 2019 at 3:02 PM H.J. Lu wrote:
>
> When -march=native is passed to host_detect_local_cpu to the backend,
> it overrides all command lines after it. That means
>
> $ gcc -march=native -march=skylake-avx512
>
> is the treated as
>
> $ gcc -march=skylake-avx512 -march=native
>
> Prune joined switches with negation to allow -march=skylake-avx512 to
> override previous -march=native on command-line.
>
> PR driver/69471
> * opts-common.c (prune_options): Also prune joined switches
> with negation.
> * config/i386/i386.opt (march=): Add Negative(march=).
> (mtune=): Add Negative(mtune=).
Here is the updated patch.
--
H.J.
From a5a453e48f8560955cfa2e2db3b8f4f22b9cf9b1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Fri, 8 Feb 2019 14:52:57 -0800
Subject: [PATCH] driver: Also prune joined switches with negation
When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it. That means
$ gcc -march=native -march=skylake-avx512
is the treated as
$ gcc -march=skylake-avx512 -march=native
Prune joined switches with negation to allow -march=skylake-avx512 to
override previous -march=native on command-line.
PR driver/69471
* opts-common.c (prune_options): Also prune joined switches
with negation.
* config/i386/i386.opt (march=): Add Negative(march=).
(mtune=): Add Negative(mtune=).
---
gcc/config/i386/i386.opt | 4 ++--
gcc/opts-common.c| 6 --
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
Generate code for given CPU.
masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
Use direct references against %gs when accessing tls data.
mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
Schedule code for given CPU.
mtune-ctrl=
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..7204bb5a8fa 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1014,10 +1014,6 @@ prune_options (struct cl_decoded_option **decoded_options,
if (option->neg_index < 0)
goto keep;
- /* Skip joined switches. */
- if ((option->flags & CL_JOINED))
- goto keep;
-
for (j = i + 1; j < old_decoded_options_count; j++)
{
if (old_decoded_options[j].errors & ~CL_ERR_WRONG_LANG)
@@ -1027,8 +1023,6 @@ prune_options (struct cl_decoded_option **decoded_options,
continue;
if (cl_options[next_opt_idx].neg_index < 0)
continue;
- if ((cl_options[next_opt_idx].flags & CL_JOINED))
- continue;
if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
break;
}
--
2.20.1
Re: [PATCH] Updated patches for the port of gccgo to GNU/Hurd
On 07.02.19 06:04, Ian Lance Taylor wrote: > On Thu, Jan 31, 2019 at 7:40 AM Svante Signell > wrote: >> >> As advised by the Debian gcc maintainer Matthias Klose and golang >> developer Ian Lance Taylor I'm re-submitting the patches for >> the port of gccgo to GNU/Hurd again. Now GOOS value is changed from gnu >> to hurd as requested. >> >> The 12 patches are: >> src_libgo_build.diff >> src_libgo_runtime.diff >> src_libgo_go_crypto.diff >> src_libgo_go_internal.diff >> src_libgo_go_net.diff >> src_libgo_go_os.diff >> src_libgo_go_runtime.diff >> src_libgo_go_syscall.diff >> src_libgo_go_test.diff >> >> src_libgo_testsuite_gotest.diff >> add-hurd-to-libgo-headers.diff >> add-hurd-to-libgo-test-headers.diff > > Thanks. I've committed versions of all of these patches other than > src_libgo_testsuite_gotest.diff. I omitted that one because as far as > I can tell it won't work. While the original code may not run on the > Hurd, the modified version won't work. > > I made various changes, and I'm sure I broke some things. Take a look > at GCC trunk and see how it seems. libtool: compile: /<>/build/./gcc/gccgo -B/<>/build/./gcc/ -B/usr/i686-gnu/bin/ -B/usr/i6 86-gnu/lib/ -isystem /usr/i686-gnu/include -isystem /usr/i686-gnu/sys-include -isystem /<>/build/sys-in clude -fchecking=1 -minline-all-stringops -O2 -g -I . -c -fgo-pkgpath=syscall -fPIC -o .libs/syscall.o gccgo: fatal error: no input files compilation terminated. Makefile:2844: recipe for target 'syscall.lo' failed make[6]: *** [syscall.lo] Error 1 make[6]: Leaving directory '/<>/build/i686-gnu/libgo' Makefile:2242: recipe for target 'all-recursive' failed make[5]: *** [all-recursive] Error 1 make[5]: Leaving directory '/<>/build/i686-gnu/libgo' Makefile:1167: recipe for target 'all' failed make[4]: *** [all] Error 2 make[4]: Leaving directory '/<>/build/i686-gnu/libgo' Makefile:20078: recipe for target 'all-target-libgo' failed make[3]: *** [all-target-libgo] Error 2 make[3]: Leaving directory '/<>/build' Makefile:24129: recipe for target 'bootstrap' failed make[2]: *** [bootstrap] Error 2
[PATCH] driver: Also prune joined switches with negation
When -march=native is passed to host_detect_local_cpu to the backend, it overrides all command lines after it. That means $ gcc -march=native -march=skylake-avx512 is the treated as $ gcc -march=skylake-avx512 -march=native Prune joined switches with negation to allow -march=skylake-avx512 to override previous -march=native on command-line. PR driver/69471 * opts-common.c (prune_options): Also prune joined switches with negation. * config/i386/i386.opt (march=): Add Negative(march=). (mtune=): Add Negative(mtune=). --- gcc/config/i386/i386.opt | 4 ++-- gcc/opts-common.c| 7 --- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index 9b93241f790..b7998ee7363 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -253,7 +253,7 @@ EnumValue Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline) march= -Target RejectNegative Joined Var(ix86_arch_string) +Target RejectNegative Negative(march=) Joined Var(ix86_arch_string) Generate code for given CPU. masm= @@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS) Use direct references against %gs when accessing tls data. mtune= -Target RejectNegative Joined Var(ix86_tune_string) +Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string) Schedule code for given CPU. mtune-ctrl= diff --git a/gcc/opts-common.c b/gcc/opts-common.c index ee8898b22ec..ba9eb144078 100644 --- a/gcc/opts-common.c +++ b/gcc/opts-common.c @@ -1014,8 +1014,8 @@ prune_options (struct cl_decoded_option **decoded_options, if (option->neg_index < 0) goto keep; - /* Skip joined switches. */ - if ((option->flags & CL_JOINED)) + /* Skip joined switches if there is no negation. */ + if ((option->flags & CL_JOINED) && option->neg_index < 0) goto keep; for (j = i + 1; j < old_decoded_options_count; j++) @@ -1027,7 +1027,8 @@ prune_options (struct cl_decoded_option **decoded_options, continue; if (cl_options[next_opt_idx].neg_index < 0) continue; - if ((cl_options[next_opt_idx].flags & CL_JOINED)) + if ((cl_options[next_opt_idx].flags & CL_JOINED) + && cl_options[next_opt_idx].neg_index < 0) continue; if (cancel_option (opt_idx, next_opt_idx, next_opt_idx)) break; -- 2.20.1
Re: [RS6000] Don't support inline PLT for ABI_V4 bss-plt
Hi Alan,
On Fri, Feb 08, 2019 at 11:05:57AM +1030, Alan Modra wrote:
> Inline PLT calls need PLT to be an array of addresses. bss-plt works
> differently.
>
> Bootstrap and regression test on powerpc64-linux biarch in progress.
> OK assuming no regressions?
>
> * config/rs6000/rs6000.c (rs6000_longcall_ref): Don't use
> inline plt for ABI_V4 bss-plt.
> (rs6000_call_sysv): Likewise.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 711278c7422..cced90bb518 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -32819,7 +32819,8 @@ rs6000_longcall_ref (rtx call_ref, rtx arg)
> }
>
>if (HAVE_AS_PLTSEQ
> - && (DEFAULT_ABI == ABI_ELFv2 || DEFAULT_ABI == ABI_V4))
> + && (DEFAULT_ABI == ABI_ELFv2
> + || (DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT)))
> {
>rtx base = const0_rtx;
>int regno;
> @@ -37981,7 +37982,7 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx
> tlsarg, rtx cookie)
>func = rs6000_longcall_ref (func_desc, tlsarg);
>/* If the longcall was implemented using PLT16 relocs, then r11
>needs to be valid at the call for lazy linking. */
This comment could use some work.
> - if (HAVE_AS_PLTSEQ)
> + if (HAVE_AS_PLTSEQ && REGNO (func) == 11)
This (and all occurrences below) need some explanation. It is probably
best if you factor out
bool something = (REGNO (func) == 11));
(with a descriptive name) and use that?
The patch is okay (for all branches) with those things improved. Thanks!
Segher
Re: C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function
On 2/8/19 12:21 PM, Marek Polacek wrote:
r256999 removed early bailout for pointer-to-member-function types, so we
now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.
That's fine but the problem here is that we end up converting a null pointer
to pointer-to-member-function type and that crashes in fold_convert:
16035 case INTEGER_CST:
16039 {
16040 /* Instantiate any typedefs in the type. */
16041 tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
16042 r = fold_convert (type, t);
It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
that then ICEs too, the infamous "canonical types differ for identical types":
type is
struct
{
void A:: (struct A *) * __pfn;
long int __delta;
}
and the type of "0" is "void A:: (struct A *) *". These types are
structurally equivalent but have different canonical types. (What's up
with that, anyway? It seems OK that the canonical type of the struct is
the struct itself and that the canonical type of the pointer is the pointer
itself.)
That could be handled in cp_fold_convert: add code to convert an integer_zerop
to
TYPE_PTRMEMFUNC_P. Unfortunately the 0 is not null_ptr_cst_p because it's got
a pointer type.
Or just don't bother substituting null_member_pointer_value_p and avoid the
above.
Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?
2019-02-08 Marek Polacek
PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
* pt.c (tsubst_copy_and_build) : Return early for
null member pointer value.
* g++.dg/cpp0x/nullptr40.C: New test.
diff --git gcc/cp/pt.c gcc/cp/pt.c
index b8fbf4046f0..acc2d8f1feb 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
looked up by digest_init. */
process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
+ if (null_member_pointer_value_p (t))
+ RETURN (t);
I would expect this to do the wrong thing if type is different from
TREE_TYPE (t). Can we get here for a dependent PMF type like T
(A::*)()? If not, let's assert that they're the same. Otherwise, maybe
cp_convert (type, nullptr_node)?
Jason
Re: [C++PATCH] [PR87322] move cp_evaluated up to tsubst all lambda parms
On 2/8/19 1:58 AM, Alexandre Oliva wrote:
On Feb 7, 2019, Jason Merrill wrote:
+ PR c++/86322. */
Wrong PR number.
Thanks
+ if (local_specializations)
+if (tree r = retrieve_local_specialization (t))
+ return r;
Hmm, I would expect this to do the wrong thing for pack expansion of a
lambda, giving us only one rather than one per element of the
expansion.
Oooh. Indeed, I hadn't realized this was possible.
I think we should separate local_specializations arising from different
pack expansion bindings, considering they won't always appear in tsubst
args:
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0177d7f77891..9f954698f454 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11698,6 +11698,10 @@ gen_elem_of_pack_expansion_instantiation (tree pattern,
ARGUMENT_PACK_SELECT_INDEX (aps) = index;
}
+ // Any local specialization bindings arising from this substitution
+ // cannot be reused for a different INDEX.
+ local_specialization_stack lss (lss_copy);
+
/* Substitute into the PATTERN with the (possibly altered)
arguments. */
if (pattern == in_decl)
For instance, in lambda-variadic5.C we should get three
instantiations of the "print" function template; can you add that
check to the test?
This causes lambda-variadic5.C to get 6 separate lambdas, instead of 2,
so it passes after the following patchlet:
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-variadic5.C
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-variadic5.C
index 97f64cd761a7..1f931757b72f 100644
--- a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-variadic5.C
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-variadic5.C
@@ -1,5 +1,7 @@
// PR c++/47226
// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+// { dg-final { scan-tree-dump-times ":: \\(null\\)" 6
"original" } }
template
void print(const T&) {}
Now, should I adjust lambda-variadic5.C to check for the presence of the
expencted count of lambdas in compiler dumps?
Sounds good.
Thinking of how to test for it drew my attention to issues about lambda
naming, that may have implications on ABI and duplicate code removal.
There are two issues I'm concerned with:
- we increment lambda_cnt when introducing a lambda within a generic,
and then increment it again as we specialize, partially or fully, each
lambda expr/object/type set. This makes for gaps in numbering of full
specializations and, being a global counter, does not ensure the
assignment of the same symbol name to corresponding instantiations in
separate translation units
We shouldn't use that name in any symbol shared between translation units.
- we don't seem to attempt to reuse lambdas across different
specializations of argument pack expansions.
Right, we shouldn't reuse them (other than via identical code folding
optimizations).
I guess that's ok, we
don't attempt to reuse them across different specializations of the
enclosing template function or class either, but, unlike enclosing
contexts, different argument pack indices do not mandate different
mangled names. This came up because I was thinking of having a
sub-map in local_specializations, so that the lambda would map to
another map in which we could look up template arguments to find a
specific instance. That wouldn't work, in part because args does not
provide the entire set of substitutions, and in another part because
we don't seem to be able to identify, in patterns undergoing
substitution but before actually performing the substitution, the set
of (implied?) template arguments that might affect the substitution
result.
Indeed. When I was fixing lambdas in pack expansions I thought about
making lambdas in pack expansions implicit templates with the parameters
matching the packs in the pack expansion, but the current implementation
matches the specification better I think.
Here's the patch I'm testing. OK if it passes?
OK.
Jason
Re: [C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE
On 2/8/19 4:07 AM, Alexandre Oliva wrote: On Feb 7, 2019, Jason Merrill wrote: In protected_accessible_p and shared_member_p, if we're left with a USING_DECL after strip_using_decl, we can't give a meaningful answer, and should probably abort; we shouldn't get here with a dependent expression. In g++.dg/lookup/using39.C, shared_member_p is called by cp_parser_primary_expression -> finish_id_expression -> finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using A::f' in B. Then I think finish_qualified_id_expr should check type_dependent_expression_p first. Jason
Re: arm access to stack slot out of allocated area
Hi Olivier, > Sorry, I had -mapcs-frame in mind. That's identical to -mapcs, and equally deprecated. It was superceded 2 decades ago. -mpcs-frame bugs have been reported multiple times, including on VxWorks. For example https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379 suggests VxWorks doesn't need -mapcs-frame. And that was in 2014! So I think we need to push much harder on getting rid of obsolete stuff and avoid people encountering these nasty issues. Cheers, Wilco
Re: [PATCH] Add testcase for PR tree-optimization/88739
On Fri, 8 Feb 2019 at 20:00, Richard Biener wrote:
>
> On February 8, 2019 7:22:48 PM GMT+01:00, Jakub Jelinek
> wrote:
> >Hi!
> >
> >The following testcase distilled from
> >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88739#c0
> >aborts on s390x-linux when compiled with trunk -O2 with r268332
> >reverted (or
> >e.g. with -O2 and gcc 7.x) and succeeds with trunk -O2, or -O0 with any
> >of
> >those compilers. Tested also on x86_64-linux with -m32/-m64 make
> >check.
> >
> >Ok for trunk?
>
> OK and thanks for the testcase!
>
> Richard.
>
> >2019-02-08 Jakub Jelinek
> >
> > PR tree-optimization/88739
> > * gcc.c-torture/execute/pr88739.c: New test.
> >
> >--- gcc/testsuite/gcc.c-torture/execute/pr88739.c.jj 2019-01-27
> >12:44:12.526219828 +0100
> >+++ gcc/testsuite/gcc.c-torture/execute/pr88739.c 2019-02-08
> >18:48:25.880556579 +0100
> >@@ -0,0 +1,59 @@
> >+/* PR tree-optimization/88739 */
> >+#if __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __CHAR_BIT__ == 8
> >+struct A
> >+{
> >+ unsigned int a, b, c;
> >+ unsigned int d : 30;
> >+ unsigned int e : 2;
> >+};
> >+
> >+union U
> >+{
> >+ struct A f;
> >+ unsigned int g[4];
> >+ unsigned short h[8];
> >+ unsigned char i[16];
> >+};
> >+volatile union U v = { .f.d = 0x4089 };
> >+
> >+__attribute__((noipa)) void
> >+bar (int x)
> >+{
> >+ static int i;
> >+ switch (i++)
> >+{
> >+case 0: if (x != v.f.d) __builtin_abort (); break;
> >+case 1: if (x != v.f.e) __builtin_abort (); break;
> >+case 2: if (x != v.g[3]) __builtin_abort (); break;
> >+case 3: if (x != v.h[6]) __builtin_abort (); break;
> >+case 4: if (x != v.h[7]) __builtin_abort (); break;
> >+default: __builtin_abort (); break;
> >+}
> >+}
> >+
> >+void
> >+foo (unsigned int x)
> >+{
> >+ union U u;
> >+ u.f.d = x >> 2;
> >+ u.f.e = 0;
> >+ bar (u.f.d);
> >+ bar (u.f.e);
> >+ bar (u.g[3]);
> >+ bar (u.h[6]);
> >+ bar (u.h[7]);
> >+}
> >+
> >+int
> >+main ()
> >+{
> >+ foo (0x10224);
> >+ return 0;
> >+}
> >+#else
> >+int
> >+main ()
> >+{
> >+ return 0;
> >+}
> >+#endif
> >
> > Jakub
>
Looks more elaborate than what I was deriving from #c0. I'll let you
know if I see some problems on aarch64_be or armeb.
Thanks,
Christophe
[PR fortran/89077, patch, part 2] - ICE using * as len specifier for character parameter
The attached patch attempts a substring length simplification
so that more complex expressions are handled in initialization
expressions. Thanks to Thomas König for the suggestion.
Regtested on x86_64-pc-linux-gnu.
(The PR still has other wrong-code issue to be addressed separately.)
OK for trunk? And for backports to 8/7?
Thanks,
Harald
2019-02-08 Harald Anlauf
PR fortran/89077
* resolve.c (gfc_resolve_substring_charlen): Check substring
length for constantness prior to general calculation of length.
2019-02-08 Harald Anlauf
PR fortran/89077
* gfortran.dg/substr_simplify.f90: New test.
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (revision 268658)
+++ gcc/fortran/resolve.c (working copy)
@@ -4965,6 +4965,7 @@
gfc_ref *char_ref;
gfc_expr *start, *end;
gfc_typespec *ts = NULL;
+ mpz_t diff;
for (char_ref = e->ref; char_ref; char_ref = char_ref->next)
{
@@ -5016,12 +5017,25 @@
return;
}
- /* Length = (end - start + 1). */
- e->ts.u.cl->length = gfc_subtract (end, start);
- e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
- gfc_get_int_expr (gfc_charlen_int_kind,
- NULL, 1));
+ /* Length = (end - start + 1).
+ Check first whether it has a constant length. */
+ if (gfc_dep_difference (end, start, ))
+{
+ gfc_expr *len = gfc_get_constant_expr (BT_INTEGER, gfc_charlen_int_kind,
+>where);
+ mpz_add_ui (len->value.integer, diff, 1);
+ mpz_clear (diff);
+ e->ts.u.cl->length = len;
+}
+ else
+{
+ e->ts.u.cl->length = gfc_subtract (end, start);
+ e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
+ gfc_get_int_expr (gfc_charlen_int_kind,
+ NULL, 1));
+}
+
/* F2008, 6.4.1: Both the starting point and the ending point shall
be within the range 1, 2, ..., n unless the starting point exceeds
the ending point, in which case the substring has length zero. */
Index: gcc/testsuite/gfortran.dg/substr_simplify.f90
===
--- gcc/testsuite/gfortran.dg/substr_simplify.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/substr_simplify.f90 (working copy)
@@ -0,0 +1,20 @@
+! { dg-do run }
+!
+! Test fixes for substring simplications derived from
+! PR fortran/89077 - ICE using * as len specifier for character parameter
+
+program test
+ implicit none
+ integer :: i
+ character(*), parameter :: s = 'abcdef', y = 'efcdab'
+ character(6), save :: t = transfer ([(s(i:i), i=1,len(s) )], s)
+ character(*), parameter :: u = transfer ([(s(i:i+2),i=1,len(s),3)], s)
+ character(6), save :: v = transfer ([(s(i:i+2),i=1,len(s),3)], s)
+ character(*), parameter :: w = transfer ([(y(i:i+1),i=len(s)-1,1,-2)], s)
+ character(6), save :: x = transfer ([(y(i:i+1),i=len(s)-1,1,-2)], s)
+ if (len (t) /= len (s) .or. t /= s) stop 1
+ if (len (u) /= len (s) .or. u /= s) stop 2
+ if (len (v) /= len (s) .or. v /= s) stop 3
+ if (len (w) /= len (s) .or. w /= s) stop 4
+ if (len (x) /= len (s) .or. x /= s) stop 5
+end
Re: [Patch]Bug 84762 - GCC for PowerPC32 violates the SysV ABI spec for small struct returns
Hi Lokesh, Sorry for not getting back to you earlier. On Thu, Jan 10, 2019 at 05:57:52PM +0530, Lokesh Janghel wrote: > Find the attached patch for the subjected issue. > Please let me know your thoughts and comments on the same. > > >Do you have a copyright assignment with the FSF? > We don't have a copyright assignment with FSF. Then please arrange that, first? Segher
[poweprc] RFA: patch changing expected code generation for test vsx-simode2.c
Recently I committed a patch solving
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88560
The patch resulted in test vsx-simode2.c failure. Here is the
difference in generated code:
@@ -13,9 +13,8 @@ foo:
.LFB0:
.cfi_startproc
std 3,-16(1)
- ori 2,2,0
- lwz 9,-12(1)
- mtvsrwz 32,9
+ addi 9,1,-12
+ lxsiwzx 32,0,9
The new version is one insn less. So I propose the following patch
changing the expected code generation.
Is it ok to commit it?
Index: ChangeLog
===
--- ChangeLog (revision 268581)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2019-02-08 Vladimir Makarov
+
+ * gcc.target/powerpc/vsx-simode2.c: Expect lxsiwzx instead of
+ mtvsrwz.
+
2019-02-06 Richard Biener
PR tree-optimization/89182
Index: gcc.target/powerpc/vsx-simode2.c
===
--- gcc.target/powerpc/vsx-simode2.c(revision 268581)
+++ gcc.target/powerpc/vsx-simode2.c(working copy)
@@ -11,5 +11,5 @@ unsigned int foo (unsigned int u)
return ret;
}
-/* { dg-final { scan-assembler "mtvsrwz" } } */
+/* { dg-final { scan-assembler "lxsiwzx" } } */
/* { dg-final { scan-assembler "mfvsrwz" } } */
patch to fix PR88560
The following patch fixes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88560
The patch was bootstrapped and tested on x86-64 and ppc64. It was
also tested on ARM by Tamar Christina. The patch changes expected
generated code for one test on ppc64 but in a better way. I'll send a
patch fixing the test later.
Committed as rev. 268705
Index: ChangeLog
===
--- ChangeLog (revision 268704)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2019-02-08 Vladimir Makarov
+
+ PR middle-end/88560
+ * lra-constraints.c (process_alt_operands): Don't increase reject
+ for memory when offset memory is required.
+
2019-02-08 Robin Dapp
* config/s390/vector.md: Implement vector copysign.
Index: lra-constraints.c
===
--- lra-constraints.c (revision 268704)
+++ lra-constraints.c (working copy)
@@ -2796,29 +2796,32 @@ process_alt_operands (int only_alternati
(GET_MODE (op), this_alternative, cl)
losers++;
- /* Input reloads can be inherited more often than output
- reloads can be removed, so penalize output
- reloads. */
- if (!REG_P (op) || curr_static_id->operand[nop].type != OP_IN)
- {
- if (lra_dump_file != NULL)
- fprintf
- (lra_dump_file,
- "%d Non input pseudo reload: reject++\n",
- nop);
- reject++;
- }
-
if (MEM_P (op) && offmemok)
addr_losers++;
- else if (curr_static_id->operand[nop].type == OP_INOUT)
+ else
{
- if (lra_dump_file != NULL)
- fprintf
- (lra_dump_file,
- "%d Input/Output reload: reject+=%d\n",
- nop, LRA_LOSER_COST_FACTOR);
- reject += LRA_LOSER_COST_FACTOR;
+ /* Input reloads can be inherited more often than
+ output reloads can be removed, so penalize output
+ reloads. */
+ if (!REG_P (op) || curr_static_id->operand[nop].type != OP_IN)
+ {
+ if (lra_dump_file != NULL)
+ fprintf
+ (lra_dump_file,
+ "%d Non input pseudo reload: reject++\n",
+ nop);
+ reject++;
+ }
+
+ if (curr_static_id->operand[nop].type == OP_INOUT)
+ {
+ if (lra_dump_file != NULL)
+ fprintf
+ (lra_dump_file,
+ "%d Input/Output reload: reject+=%d\n",
+ nop, LRA_LOSER_COST_FACTOR);
+ reject += LRA_LOSER_COST_FACTOR;
+ }
}
}
Re: [PATCH] Add testcase for PR tree-optimization/88739
On February 8, 2019 7:22:48 PM GMT+01:00, Jakub Jelinek
wrote:
>Hi!
>
>The following testcase distilled from
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88739#c0
>aborts on s390x-linux when compiled with trunk -O2 with r268332
>reverted (or
>e.g. with -O2 and gcc 7.x) and succeeds with trunk -O2, or -O0 with any
>of
>those compilers. Tested also on x86_64-linux with -m32/-m64 make
>check.
>
>Ok for trunk?
OK and thanks for the testcase!
Richard.
>2019-02-08 Jakub Jelinek
>
> PR tree-optimization/88739
> * gcc.c-torture/execute/pr88739.c: New test.
>
>--- gcc/testsuite/gcc.c-torture/execute/pr88739.c.jj 2019-01-27
>12:44:12.526219828 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr88739.c 2019-02-08
>18:48:25.880556579 +0100
>@@ -0,0 +1,59 @@
>+/* PR tree-optimization/88739 */
>+#if __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __CHAR_BIT__ == 8
>+struct A
>+{
>+ unsigned int a, b, c;
>+ unsigned int d : 30;
>+ unsigned int e : 2;
>+};
>+
>+union U
>+{
>+ struct A f;
>+ unsigned int g[4];
>+ unsigned short h[8];
>+ unsigned char i[16];
>+};
>+volatile union U v = { .f.d = 0x4089 };
>+
>+__attribute__((noipa)) void
>+bar (int x)
>+{
>+ static int i;
>+ switch (i++)
>+{
>+case 0: if (x != v.f.d) __builtin_abort (); break;
>+case 1: if (x != v.f.e) __builtin_abort (); break;
>+case 2: if (x != v.g[3]) __builtin_abort (); break;
>+case 3: if (x != v.h[6]) __builtin_abort (); break;
>+case 4: if (x != v.h[7]) __builtin_abort (); break;
>+default: __builtin_abort (); break;
>+}
>+}
>+
>+void
>+foo (unsigned int x)
>+{
>+ union U u;
>+ u.f.d = x >> 2;
>+ u.f.e = 0;
>+ bar (u.f.d);
>+ bar (u.f.e);
>+ bar (u.g[3]);
>+ bar (u.h[6]);
>+ bar (u.h[7]);
>+}
>+
>+int
>+main ()
>+{
>+ foo (0x10224);
>+ return 0;
>+}
>+#else
>+int
>+main ()
>+{
>+ return 0;
>+}
>+#endif
>
> Jakub
Re: [EXT] Re: [PATCH, fortran ieee]: PR 88678, Many gfortran.dg/ieee/ieee_X.f90 test cases fail starting with r267465
On Fri, 2019-02-08 at 10:42 +0100, Uros Bizjak wrote:
> so, the reverted patch neglected this assumption. Ignoring this, we
> can use
>
> --cut here--
> Index: libgfortran/config/fpu-glibc.h
> ===
> --- libgfortran/config/fpu-glibc.h (revision 268424)
> +++ libgfortran/config/fpu-glibc.h (working copy)
> @@ -129,6 +129,10 @@
> int
> support_fpu_trap (int flag)
> {
> +#if defined(__arm__) || defined(__aarch64__)
> + return 0;
> +#endif
> +
>return support_fpu_flag (flag);
> }
>
> --cut here--
>
> which would in effect revert to the previous status on arm/aarch64
> targets, while using correct setting for other targets, where
> exception support is assumed.
>
> Uros.
I am not sure we want to disable this for all arm/Aarch64 chips.
ieee_6.f90 is failing on my T88 ThunderX box with a 4.13 kernel and
passing on my T99 Thunderx2 box with a 4.15 kernel. I don't know
if it is the hardware or the kernel that is making the difference,
I am still trying to figure that out. I guess returning zero would be
a 'safe' choice in that it would say we do not support this on some
machines where it actually is supported but it would no longer claim
support on a machine where it is not supported.
Steve Ellcey
sell...@marvell.com
[PATCH] Add testcase for PR tree-optimization/88739
Hi!
The following testcase distilled from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88739#c0
aborts on s390x-linux when compiled with trunk -O2 with r268332 reverted (or
e.g. with -O2 and gcc 7.x) and succeeds with trunk -O2, or -O0 with any of
those compilers. Tested also on x86_64-linux with -m32/-m64 make check.
Ok for trunk?
2019-02-08 Jakub Jelinek
PR tree-optimization/88739
* gcc.c-torture/execute/pr88739.c: New test.
--- gcc/testsuite/gcc.c-torture/execute/pr88739.c.jj2019-01-27
12:44:12.526219828 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr88739.c 2019-02-08
18:48:25.880556579 +0100
@@ -0,0 +1,59 @@
+/* PR tree-optimization/88739 */
+#if __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __CHAR_BIT__ == 8
+struct A
+{
+ unsigned int a, b, c;
+ unsigned int d : 30;
+ unsigned int e : 2;
+};
+
+union U
+{
+ struct A f;
+ unsigned int g[4];
+ unsigned short h[8];
+ unsigned char i[16];
+};
+volatile union U v = { .f.d = 0x4089 };
+
+__attribute__((noipa)) void
+bar (int x)
+{
+ static int i;
+ switch (i++)
+{
+case 0: if (x != v.f.d) __builtin_abort (); break;
+case 1: if (x != v.f.e) __builtin_abort (); break;
+case 2: if (x != v.g[3]) __builtin_abort (); break;
+case 3: if (x != v.h[6]) __builtin_abort (); break;
+case 4: if (x != v.h[7]) __builtin_abort (); break;
+default: __builtin_abort (); break;
+}
+}
+
+void
+foo (unsigned int x)
+{
+ union U u;
+ u.f.d = x >> 2;
+ u.f.e = 0;
+ bar (u.f.d);
+ bar (u.f.e);
+ bar (u.g[3]);
+ bar (u.h[6]);
+ bar (u.h[7]);
+}
+
+int
+main ()
+{
+ foo (0x10224);
+ return 0;
+}
+#else
+int
+main ()
+{
+ return 0;
+}
+#endif
Jakub
Re: [PATCH][libbacktrace] Declare external backtrace fns noinline
Hi Tom!
On Fri, 8 Feb 2019 10:41:47 +0100, Tom de Vries wrote:
> The backtrace functions backtrace_full, backtrace_print and backtrace_simple
> walk the call stack, but make sure to skip the first entry, in order to skip
> over the functions themselves, and start the backtrace at the caller of the
> functions.
>
> When compiling with -flto, the functions may be inlined, causing them to skip
> over the caller instead.
So, when recently working on the OpenACC Profiling Interface
implementation in libgomp, where I'm using libbacktrace to figure out the
caller of certain libgomp functions, I recently wondered about the very
same issue, that we reliably have to skip a few initial frames.
So, "noinline" is how to do that reliably... ;-/ That might be
non-obvious for the casual reader, so they might not understand...
> Fix this by declaring the functions with __attribute__((noinline)).
... this alone.
I'd suggest to have a common "#define LIBBACKTRACE_NOINLINE [...]" (or
similar), together with the explanatory comment given above, and use that
at the respective definition (or declaration?) sites. Can that go into
the public libbacktrace "*.h" file, so that it can also be used
elsewhere, as described above?
If you agree, want me to prepare a patch?
Grüße
Thomas
> [libbacktrace] Declare external backtrace fns noinline
>
> 2019-02-08 Tom de Vries
>
> * backtrace.c (backtrace_full): Declare with __attribute__((noinline)).
> * print.c (backtrace_print): Same.
> * simple.c (backtrace_simple): Same.
>
> ---
> libbacktrace/backtrace.c | 2 +-
> libbacktrace/print.c | 2 +-
> libbacktrace/simple.c| 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libbacktrace/backtrace.c b/libbacktrace/backtrace.c
> index 29204c63313..c579e803825 100644
> --- a/libbacktrace/backtrace.c
> +++ b/libbacktrace/backtrace.c
> @@ -98,7 +98,7 @@ unwind (struct _Unwind_Context *context, void *vdata)
>
> /* Get a stack backtrace. */
>
> -int
> +int __attribute__((noinline))
> backtrace_full (struct backtrace_state *state, int skip,
> backtrace_full_callback callback,
> backtrace_error_callback error_callback, void *data)
> diff --git a/libbacktrace/print.c b/libbacktrace/print.c
> index b2f45446443..0767facecae 100644
> --- a/libbacktrace/print.c
> +++ b/libbacktrace/print.c
> @@ -80,7 +80,7 @@ error_callback (void *data, const char *msg, int errnum)
>
> /* Print a backtrace. */
>
> -void
> +void __attribute__((noinline))
> backtrace_print (struct backtrace_state *state, int skip, FILE *f)
> {
>struct print_data data;
> diff --git a/libbacktrace/simple.c b/libbacktrace/simple.c
> index d439fcee8e2..118936397da 100644
> --- a/libbacktrace/simple.c
> +++ b/libbacktrace/simple.c
> @@ -90,7 +90,7 @@ simple_unwind (struct _Unwind_Context *context, void *vdata)
>
> /* Get a simple stack backtrace. */
>
> -int
> +int __attribute__((noinline))
> backtrace_simple (struct backtrace_state *state, int skip,
> backtrace_simple_callback callback,
> backtrace_error_callback error_callback, void *data)
C++ PATCH for c++/89212 - ICE converting nullptr to pointer-to-member-function
r256999 removed early bailout for pointer-to-member-function types, so we
now try to tsubst each element of a pointer-to-member-function CONSTRUCTOR.
That's fine but the problem here is that we end up converting a null pointer
to pointer-to-member-function type and that crashes in fold_convert:
16035 case INTEGER_CST:
16039 {
16040 /* Instantiate any typedefs in the type. */
16041 tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
16042 r = fold_convert (type, t);
It seems obvious to use cp_fold_convert which handles TYPE_PTRMEM_P, but
that then ICEs too, the infamous "canonical types differ for identical types":
type is
struct
{
void A:: (struct A *) * __pfn;
long int __delta;
}
and the type of "0" is "void A:: (struct A *) *". These types are
structurally equivalent but have different canonical types. (What's up
with that, anyway? It seems OK that the canonical type of the struct is
the struct itself and that the canonical type of the pointer is the pointer
itself.)
That could be handled in cp_fold_convert: add code to convert an integer_zerop
to
TYPE_PTRMEMFUNC_P. Unfortunately the 0 is not null_ptr_cst_p because it's got
a pointer type.
Or just don't bother substituting null_member_pointer_value_p and avoid the
above.
Bootstrapped/regtested on x86_64-linux, ok for trunk and 8?
2019-02-08 Marek Polacek
PR c++/89212 - ICE converting nullptr to pointer-to-member-function.
* pt.c (tsubst_copy_and_build) : Return early for
null member pointer value.
* g++.dg/cpp0x/nullptr40.C: New test.
diff --git gcc/cp/pt.c gcc/cp/pt.c
index b8fbf4046f0..acc2d8f1feb 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -19251,6 +19251,9 @@ tsubst_copy_and_build (tree t,
looked up by digest_init. */
process_index_p = !(type && MAYBE_CLASS_TYPE_P (type));
+ if (null_member_pointer_value_p (t))
+ RETURN (t);
+
n = vec_safe_copy (CONSTRUCTOR_ELTS (t));
newlen = vec_safe_length (n);
FOR_EACH_VEC_SAFE_ELT (n, idx, ce)
diff --git gcc/testsuite/g++.dg/cpp0x/nullptr40.C
gcc/testsuite/g++.dg/cpp0x/nullptr40.C
new file mode 100644
index 000..21c188bdb5e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/nullptr40.C
@@ -0,0 +1,19 @@
+// PR c++/89212
+// { dg-do compile { target c++11 } }
+
+template using enable_if_t = int;
+
+template
+struct p
+{
+template>
+p(T) { }
+p() = default;
+};
+
+struct A
+{
+p i = 1;
+void bar();
+p j;
+};
PING Re: [PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
Ping
On Fri, 2019-01-25 at 15:02 -0500, David Malcolm wrote:
> On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
> > On 1/25/19 8:48 AM, David Malcolm wrote:
> > > PR c++/89036 reports an ICE due to this assertion failing
> > >
> > > 1136/* A class should never have more than one
> > > destructor. */
> > > 1137gcc_assert (!current_fns || via_using ||
> > > !DECL_DESTRUCTOR_P (method));
> > >
> > > on this template with a pair of dtors, with
> > > mutually exclusive "requires" clauses:
> > >
> > > template
> > > struct Y {
> > > ~Y() requires(true) = default;
> > > ~Y() requires(false) {}
> > > };
> > >
> > > (is this valid? my knowledge of this part of C++ is fairly hazy)
> >
> > Yes. A more sensible example would have 'true' and 'false' replaced
> > by
> > something determined from the template parms.
> >
> > >
> > > Nathan introduced this assertion as part of:
> > >
> > >ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> > > 2017-08-24 Nathan Sidwell
> > > Conversion operators kept on single overload set
> >
> > I'd just drop the assert at this point.
> >
> > nathan
>
> Thanks. Here's a version of the patch which drops the assertion.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/89036
> * class.c (add_method): Drop destructor assertion.
>
> gcc/testsuite/ChangeLog:
> PR c++/89036
> * g++.dg/concepts/pr89036.C: New test.
> ---
> gcc/cp/class.c | 3 ---
> gcc/testsuite/g++.dg/concepts/pr89036.C | 8
> 2 files changed, 8 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
>
> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> index e8773c2..6e9dac9 100644
> --- a/gcc/cp/class.c
> +++ b/gcc/cp/class.c
> @@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool
> via_using)
> }
> }
>
> - /* A class should never have more than one destructor. */
> - gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P
> (method));
> -
>current_fns = ovl_insert (method, current_fns, via_using);
>
>if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
> diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
> b/gcc/testsuite/g++.dg/concepts/pr89036.C
> new file mode 100644
> index 000..f83ef8b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
> @@ -0,0 +1,8 @@
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-fconcepts" }
> +
> +template
> +struct Y {
> + ~Y() requires(true) = default;
> + ~Y() requires(false) {}
> +};
Re: arm access to stack slot out of allocated area
On 08/02/2019 16:19, Olivier Hainque wrote: > Hi Wilco, > >> On 8 Feb 2019, at 15:49, Wilco Dijkstra wrote: >> >> Hi Olivier, >> >>> Below is a description of a very annoying bug we are witnessing >>> on ARM. >> ... >>> compiled with -Og -mapcs >> >> Do you know -mapcs has been deprecated for more than 4 years now? >> Is there a reason you are still using it? It was deprecated since -mapcs >> is both extremely inefficient and buggy. I would strongly suggest to stop >> using this option completely in all your software. > > Sorry, I had -mapcs-frame in mind. The investigation I described was > conducted with the default settings of the VxWorks port (no particular option) > and thought it might be of greater potential interest if exposed with a bare > eabi compiler. The conditions of interest are guarded with > > IS_NESTED (arm_current_func_type ()) > && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) > > Turns out that the arm-eabi code with -mapcs-frame is very slightly > different than the vxworks one, which I missed on the first reading. > > Let me double check again ... > >> It's useful to create a bug report with the details, however it might not be >> worth fixing if the issue cannot be reproduced without -mapcs. -mapcs >> was never meant to support a static chain, especially not spills of the >> static chain. > > Understood. I'll start by examining the differences > between the vxworks and the eabi+aapcs-frame configurations > more closely, and I'll report back. Olivier, while you are here could you also document the choices made by the vxworks port in terms of the ABI and how it differs from EABI ? It would certainly help with patch review. regards Ramana > > Thanks for your feedback! >
[committed][PATCH] Fix PR testsuite/89258
pr80887.c expects int size to be at least 32-bits, added the corresponding
require-effective-target directive.
Committed.
>From b8a747181ed83adfb0ff5f42ba74f1bc239620d8 Mon Sep 17 00:00:00 2001
From: jozefl
Date: Fri, 8 Feb 2019 16:47:28 +
Subject: [PATCH] 2019-02-08 Jozef Lawrynowicz
PR testsuite/89258
* gcc.dg/tree-ssa/pr80887.c: Require int32plus.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@268704 138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/testsuite/ChangeLog | 5 +
gcc/testsuite/gcc.dg/tree-ssa/pr80887.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5cbb14f5486..3269078fa36 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-08 Jozef Lawrynowicz
+
+ PR testsuite/89258
+ * gcc.dg/tree-ssa/pr80887.c: Require int32plus.
+
2019-02-08 Robin Dapp
* gcc.target/s390/vector/vec-copysign-execute.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80887.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80887.c
index df7a9d96eeb..9021b52c20e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr80887.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80887.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-fgimple -O" } */
+/* { dg-require-effective-target int32plus } */
int pos;
void __GIMPLE (startwith("fre"))
--
2.17.1
Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 04:18:52PM +, Iain Sandoe wrote: > > > On 8 Feb 2019, at 16:16, Segher Boessenkool > > wrote: > > > > On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote: > >> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in > >> simplifying the ABI_V4 case. This one passes regression testing. > >> OK to apply? > > > > I think this is correct. Thanks! Okay for trunk. Does it need backports? > > Yes, we reverted the original fix on 7 and 8. Then I hope we can make a working 8.3 :-/ Alan, to increase the chances of that, please backport it to 8 as soon as you are confident trunk works? Have tested it, I mean :-) Segher
Re: arm access to stack slot out of allocated area
Hi Wilco, > On 8 Feb 2019, at 15:49, Wilco Dijkstra wrote: > > Hi Olivier, > >> Below is a description of a very annoying bug we are witnessing >> on ARM. > ... >> compiled with -Og -mapcs > > Do you know -mapcs has been deprecated for more than 4 years now? > Is there a reason you are still using it? It was deprecated since -mapcs > is both extremely inefficient and buggy. I would strongly suggest to stop > using this option completely in all your software. Sorry, I had -mapcs-frame in mind. The investigation I described was conducted with the default settings of the VxWorks port (no particular option) and thought it might be of greater potential interest if exposed with a bare eabi compiler. The conditions of interest are guarded with IS_NESTED (arm_current_func_type ()) && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) Turns out that the arm-eabi code with -mapcs-frame is very slightly different than the vxworks one, which I missed on the first reading. Let me double check again ... > It's useful to create a bug report with the details, however it might not be > worth fixing if the issue cannot be reproduced without -mapcs. -mapcs > was never meant to support a static chain, especially not spills of the > static chain. Understood. I'll start by examining the differences between the vxworks and the eabi+aapcs-frame configurations more closely, and I'll report back. Thanks for your feedback!
Re: [RS6000] Correct save_reg_p
> On 8 Feb 2019, at 16:16, Segher Boessenkool
> wrote:
>
> On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote:
>> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
>> simplifying the ABI_V4 case. This one passes regression testing.
>> OK to apply?
>
> I think this is correct. Thanks! Okay for trunk. Does it need backports?
Yes, we reverted the original fix on 7 and 8.
Iain
>
>
> Segher
>
>
>> PR target/88343
>> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
>> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
>> setup.
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index cced90bb518..b4908c4f795 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
>> static bool
>> save_reg_p (int reg)
>> {
>> - /* We need to mark the PIC offset register live for the same conditions
>> - as it is set up, or otherwise it won't be saved before we clobber it.
>> */
>> -
>> if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
>> {
>> /* When calling eh_return, we must return true for all the cases
>> where conditional_register_usage marks the PIC offset reg
>> - call used. */
>> + call used or fixed. */
>> + if (crtl->calls_eh_return
>> + && ((DEFAULT_ABI == ABI_V4 && flag_pic)
>> + || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
>> + || (TARGET_TOC && TARGET_MINIMAL_TOC)))
>> +return true;
>> +
>> + /* We need to mark the PIC offset register live for the same
>> + conditions as it is set up in rs6000_emit_prologue, or
>> + otherwise it won't be saved before we clobber it. */
>> if (TARGET_TOC && TARGET_MINIMAL_TOC
>> - && (crtl->calls_eh_return
>> - || df_regs_ever_live_p (reg)
>> - || !constant_pool_empty_p ()))
>> + && !constant_pool_empty_p ())
>> +return true;
>> +
>> + if (DEFAULT_ABI == ABI_V4
>> + && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
>> + && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
>> return true;
>>
>> - if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
>> + if (DEFAULT_ABI == ABI_DARWIN
>>&& flag_pic && crtl->uses_pic_offset_table)
>> return true;
>> }
Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote:
> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
> simplifying the ABI_V4 case. This one passes regression testing.
> OK to apply?
I think this is correct. Thanks! Okay for trunk. Does it need backports?
Segher
> PR target/88343
> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
> setup.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index cced90bb518..b4908c4f795 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> static bool
> save_reg_p (int reg)
> {
> - /* We need to mark the PIC offset register live for the same conditions
> - as it is set up, or otherwise it won't be saved before we clobber it.
> */
> -
>if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
> {
>/* When calling eh_return, we must return true for all the cases
>where conditional_register_usage marks the PIC offset reg
> - call used. */
> + call used or fixed. */
> + if (crtl->calls_eh_return
> + && ((DEFAULT_ABI == ABI_V4 && flag_pic)
> + || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
> + || (TARGET_TOC && TARGET_MINIMAL_TOC)))
> + return true;
> +
> + /* We need to mark the PIC offset register live for the same
> + conditions as it is set up in rs6000_emit_prologue, or
> + otherwise it won't be saved before we clobber it. */
>if (TARGET_TOC && TARGET_MINIMAL_TOC
> - && (crtl->calls_eh_return
> - || df_regs_ever_live_p (reg)
> - || !constant_pool_empty_p ()))
> + && !constant_pool_empty_p ())
> + return true;
> +
> + if (DEFAULT_ABI == ABI_V4
> + && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
> + && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
> return true;
>
> - if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
> + if (DEFAULT_ABI == ABI_DARWIN
> && flag_pic && crtl->uses_pic_offset_table)
> return true;
> }
Re: [rs6000] 64-bit integer loads/stores and FP instructions
On Fri, Feb 08, 2019 at 11:46:37AM +0100, Eric Botcazou wrote: > > Backporting this is okay. (It was not done because it does not affect > > correctness). What is the "almost", btw? > > The predicate of operand #0 of movdi_internal32 is > rs6000_nonimmediate_operand > on the 7 branch and nonimmediate_operand on the 8 branch and later. Ah k, that is fine of course. > > (In https://gcc.gnu.org/ml/gcc-patches/2016-11/txt4j9hLRLCzf.txt and you > > presumably mean the *movdi_internal32 hunk, and the first line of it: > > > > - "=Y,r, r, ?m,?*d,?*d, > > + "=Y,r, r, ^m,^d, ^d, > > > > where the last three are mem<-reg, reg<-mem, reg<-reg, for fp regs). > > > > This patch was a year before we switched to LRA, and for non-LRA ports * > > does not do any register preferencing. > > Are you sure? No, I checked the docs so I wouldn't spout nonsense, but then I misread. Oops. > > For 6xx/7xx people *wanted* to use FP loads and stores whenever possible, > > because you get twice the bandwidth with them. > > > > If you do not want the FP registers used (or FP insns used), use > > -msoft-float, that's what it's for. > > No disagreement, but you can't force people to use it when they use FP code > in > other parts of their software. Believe me, we (and probably others) tried... Of course. And we try to use integer registers only in normal cases, these days. > > Patches are welcome, but complicating this already very complex code by > > introducing an arbitrary distinction between "embedded and other" (or > > actually, "server and other") isn't the way to go. Sorry. > > Not clear what you mean by complicating here... the approach is rather > simple > and precisely aimed at not disturbing the common path, i.e. server processors, > while preserving the historical path for other processors. AFAIK nobody has > evaluated the effects of the original change beyond POWER 7/8/9. You make an arbitrary distinction between certain CPUs and duplicate patterns based on that. > > Could you open a PR please? > > Sure, but about what now? ;-) About the bug: it should behave the same as before, use GPRs only in your testcase. Segher
Re: [PATCH, rs6000] PR target/89112 put branch probabilities on branches generated by inline expansion
Missed two more conditional branches created by inline expansion that should
have had
branch probability notes.
2019-02-08 Aaron Sawdey
* config/rs6000/rs6000-string.c (expand_compare_loop,
expand_block_compare): Insert REG_BR_PROB notes in inline expansion of
memcmp/strncmp.
Index: gcc/config/rs6000/rs6000-string.c
===
--- gcc/config/rs6000/rs6000-string.c (revision 268547)
+++ gcc/config/rs6000/rs6000-string.c (working copy)
@@ -1525,6 +1525,7 @@
else
j = emit_jump_insn (gen_bdnztf_si (fc_loop, ctr, ctr,
eqrtx, cond));
+ add_reg_br_prob_note (j, profile_probability::likely ());
JUMP_LABEL (j) = fc_loop;
LABEL_NUSES (fc_loop) += 1;
@@ -1897,6 +1898,7 @@
rtx ifelse = gen_rtx_IF_THEN_ELSE (VOIDmode, ne_rtx,
cvt_ref, pc_rtx);
rtx j = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
+ add_reg_br_prob_note (j, profile_probability::likely ());
JUMP_LABEL (j) = convert_label;
LABEL_NUSES (convert_label) += 1;
}
Pre-approved by Segher for trunk and backport to 8, will commit after regtest
completes.
Aaron
On 2/4/19 1:06 PM, Aaron Sawdey wrote:
> This is the second part of the fix for 89112, fixing the conditions that
> caused it to happen.
> This patch adds REG_BR_PROB notes to the branches generated by inline
> expansion of memcmp
> and strncmp. This prevents any of the code from being marked as cold and
> moved to the end
> of the function, which is what caused the long branches in 89112. With this
> patch, the test
> case for 89112 does not have any long branches within the expansion of
> memcmp, and the code
> for each memcmp is contiguous.
>
> OK for trunk and 8 backport if bootstrap/regtest passes?
>
> Thanks!
>
>Aaron
>
> 2019-02-04 Aaron Sawdey
>
> * config/rs6000/rs6000-string.c (do_ifelse, expand_cmp_vec_sequence,
> expand_compare_loop, expand_block_compare_gpr,
> expand_strncmp_align_check, expand_strncmp_gpr_sequence): add branch
> probability.
>
>
> Index: gcc/config/rs6000/rs6000-string.c
> ===
> --- gcc/config/rs6000/rs6000-string.c (revision 268522)
> +++ gcc/config/rs6000/rs6000-string.c (working copy)
> @@ -35,6 +35,8 @@
> #include "expr.h"
> #include "output.h"
> #include "target.h"
> +#include "profile-count.h"
> +#include "predict.h"
>
> /* Expand a block clear operation, and return 1 if successful. Return 0
> if we should let the compiler generate normal code.
> @@ -369,6 +371,7 @@
> B is the second thing to be compared.
> CR is the condition code reg input, or NULL_RTX.
> TRUE_LABEL is the label to branch to if the condition is true.
> + P is the estimated branch probability for the branch.
>
> The return value is the CR used for the comparison.
> If CR is null_rtx, then a new register of CMPMODE is generated.
> @@ -377,7 +380,7 @@
>
> static void
> do_ifelse (machine_mode cmpmode, rtx_code comparison,
> -rtx a, rtx b, rtx cr, rtx true_label)
> +rtx a, rtx b, rtx cr, rtx true_label, profile_probability p)
> {
>gcc_assert ((a == NULL_RTX && b == NULL_RTX && cr != NULL_RTX)
> || (a != NULL_RTX && b != NULL_RTX));
> @@ -395,7 +398,8 @@
>rtx cmp_rtx = gen_rtx_fmt_ee (comparison, VOIDmode, cr, const0_rtx);
>
>rtx ifelse = gen_rtx_IF_THEN_ELSE (VOIDmode, cmp_rtx, label_ref, pc_rtx);
> - rtx j = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
> + rtx_insn *j = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
> + add_reg_br_prob_note (j, p);
>JUMP_LABEL (j) = true_label;
>LABEL_NUSES (true_label) += 1;
> }
> @@ -781,7 +785,8 @@
>rtx lab_ref = gen_rtx_LABEL_REF (VOIDmode, dst_label);
>rtx ifelse = gen_rtx_IF_THEN_ELSE (VOIDmode, cmp_rtx,
>lab_ref, pc_rtx);
> - rtx j2 = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
> + rtx_insn *j2 = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
> + add_reg_br_prob_note (j2, profile_probability::likely ());
>JUMP_LABEL (j2) = dst_label;
>LABEL_NUSES (dst_label) += 1;
>
> @@ -1036,7 +1041,7 @@
>
>/* Difference found is stored here before jump to diff_label. */
>rtx diff = gen_reg_rtx (word_mode);
> - rtx j;
> + rtx_insn *j;
>
>/* Example of generated code for 35 bytes aligned 1 byte.
>
> @@ -1120,11 +1125,11 @@
>/* Check for > max_bytes bytes. We want to bail out as quickly as
>possible if we have to go over to memcmp. */
>do_ifelse (CCmode, GT, bytes_rtx, GEN_INT (max_bytes),
> - NULL_RTX, library_call_label);
> + NULL_RTX, library_call_label, profile_probability::even ());
>
>/* Check
Re: [PATCH] Fix PR89150, GC of tree-form bitmaps
On Fri, Feb 08, 2019 at 10:02:27AM -0500, Michael Ploujnikov wrote: > On 2019-02-07 3:09 p.m., Jakub Jelinek wrote: > > On Thu, Feb 07, 2019 at 03:04:21PM -0500, Michael Ploujnikov wrote: > >> 2019-02-07 Michael Ploujnikov > >> > >>PR middle-end/89150 > >>* bitmap.c (test_bitmap_tree_marking): New test. > >>(NOT_NULL_OR_GARBAGE): For shortening > >>test_bitmap_tree_marking. > >>(bitmap_c_tests): Add test_bitmap_tree_marking. > > > > Could you do that instead in a plugin in the testsuite? > > I mean, the patch is adding garbage collection roots, so it will not affect > > just -fself-tests run, but also any time the compiler will do GC. > > > > Jakub > > > > I'm not sure what I would need to do to get gengtype to process a test > plugin source file and I can't find examples of this. > > Instead, shouldn't I just do something like what's at the bottom of > gcc/ggc-tests.c and not worry about the extra root added to > gt-bitmap.h? No, instead ggc-tests.c should be moved into a plugin too. You can run gengtype from within *.exp and the additional advantage is that you test how plugins work better. Jakub
Re: [PATCH] Fix PR89150, GC of tree-form bitmaps
On 2019-02-07 3:09 p.m., Jakub Jelinek wrote: > On Thu, Feb 07, 2019 at 03:04:21PM -0500, Michael Ploujnikov wrote: >> 2019-02-07 Michael Ploujnikov >> >> PR middle-end/89150 >> * bitmap.c (test_bitmap_tree_marking): New test. >> (NOT_NULL_OR_GARBAGE): For shortening >> test_bitmap_tree_marking. >> (bitmap_c_tests): Add test_bitmap_tree_marking. > > Could you do that instead in a plugin in the testsuite? > I mean, the patch is adding garbage collection roots, so it will not affect > just -fself-tests run, but also any time the compiler will do GC. > > Jakub > I'm not sure what I would need to do to get gengtype to process a test plugin source file and I can't find examples of this. Instead, shouldn't I just do something like what's at the bottom of gcc/ggc-tests.c and not worry about the extra root added to gt-bitmap.h? - Michael signature.asc Description: OpenPGP digital signature
Re: arm access to stack slot out of allocated area
Hi Olivier, > Below is a description of a very annoying bug we are witnessing > on ARM. ... > compiled with -Og -mapcs Do you know -mapcs has been deprecated for more than 4 years now? Is there a reason you are still using it? It was deprecated since -mapcs is both extremely inefficient and buggy. I would strongly suggest to stop using this option completely in all your software. It's useful to create a bug report with the details, however it might not be worth fixing if the issue cannot be reproduced without -mapcs. -mapcs was never meant to support a static chain, especially not spills of the static chain. Cheers, Wilco
Re: [REVISED PATCH 5/9]: C++ P0482R5 char8_t: Standard library support
On 07/02/19 23:35 -0500, Tom Honermann wrote:
On 2/7/19 4:44 AM, Jonathan Wakely wrote:
On 23/12/18 21:27 -0500, Tom Honermann wrote:
Attached is a revised patch that addresses changes in P0482R6.
Changes from the prior patch include:
- Updated the value of the __cpp_char8_t feature test macro to 201811.
Tested on x86_64-linux.
Thanks, Tom, this is great work!
The front-end changes for char8_t went in recently, and I'm finally
ready to commit the library parts.
Great!
There's one big problem I found in
this patch, which is that the new numeric_limits
specialization uses constexpr unconditionally. That fails if
is compiled using options like -std=c++98 -fno-char8_t because the
specialization will be used, but the constexpr keyword isn't allowed.
That's easily fixed by replacing the keyword with _GLIBCXX_CONSTEXPR.
Hmm, the code for the char8_t specialization was copied from the
char16_t specialization which also uses constexpr unconditionally (but
is guarded by a C++11+ requirement).
That can use it unconditionally, because there's no -fchar16_t switch
to enable char16_t prior to C++11.
The char8_t specialization must
be elided when the compiler is invoked with -std=c++98 -fno-char8_t
(since the char8_t type doesn't exist then). The _GLIBCXX_USE_CHAR8_T
guard doesn't suffice for this? _GLIBCXX_USE_CHAR8_T should only be
defined if __cpp_char8_t is defined; and that should only be defined
if -fchar8_t or -std=c++2a is specified. Or perhaps you intended
-std=c++98 -fchar8_t? I agree in that case that use of
_GLIBCXX_CONSTEXPR is necessary.
Yes sorry, that's a typo above, I meant -std=c++98 -fchar8_t.
The -std=c++98 -fno-char8_t case works fine, as expected (because
-fno-char8_t is the default for -std=c++98 anyway).
The other way to solve that problem would be for the compiler to give
an error if -fchar8_t is used with C++98, but I see no fundamental
reason that combination of options shouldn't be allowed. We can
support it in the library by using the macro.
Agreed.
As discussed in San Diego, the other change needed is to add the
abi_tag attribute to the new versions of path::u8string and
path::generic_u8string, so that the mangling is different when its
return type is different:
#ifdef _GLIBCXX_USE_CHAR8_T
__attribute__((__abi_tag__("__u8")))
std::u8string u8string() const;
#else
std::string u8string() const;
#endif // _GLIBCXX_USE_CHAR8_T
Otherwise we get ODR violations when linking objects compiled
with -fchar8_t enabled to objects with it disabled (e.g. linking
-std=c++17 objects to -std=c++2a objects, which needs to work).
Are ODR violations bad? :)
Only when they make people send us bug reports ;-)
I suggest "__u8" as the name of the ABI tag, but I'm open to other
suggestions. "__char8_t" is a bit long and verbose. "__cxx20" would be
consistent with "__cxx11" used for the new ABI introduced in GCC 5 but
it regularly confuses people who think it is coupled to the -std=c++11
option (and so don't understand why they still see it for -std=c++14).
I have no preference or alternative suggestions here. Had I
recognized the issue, I would have asked you what to do about it :)
Also, I see that you've made changes to (to
add the experimental::u8string_view typedef) and to
std::experimental::path (to change the return type of u8string and
generic_u8string).
The former change is fairly harmless; it only adds a typedef, albeit
one which is not a reserved name in C++14/C++17 and so should be
available for users to define as a macro. Maybe prior to C++2a we
should only define it when GNU extensions are enabled (i.e. when using
-std=gnu++14 not -std=c++14):
#if defined _GLIBCXX_USE_CHAR8_T \
&& (__cplusplus > 201703L || !defined __STRICT_ANSI__)
using u8string_view = basic_string_view;
#endif
That makes sense.
Actually I was thinking about this further, and if somebody explicitly
uses -fchar8_t then they're asking for a non-standard dialect of C++
anyway, and so they can't complain about some extra non-standard
names. So I think it's fine to declare std::u8string_view whenever
char8_t is enabled.
Changing the return type of experimental::path members concerns me
more. That's a published TS which is not going to be revised, and it's
not obvious to me that users would want the change in semantics. If
somebody is still using the Filesystem TS in C++2a code, they're
probably not expecting it to change. If they need to update their code
for C++2a they might as well just use std::filesystem, and so having
char8_t support in std::experimental::filesystem isn't clearly useful.
I agree. I added the support to the experimental implementations more
out of a desire to be complete and to remove any potential barriers to
use of -fchar8_t than because I felt the changes were really
necessary. I would be perfectly fine with skipping the updates to the
experimental libraries completely.
OK, let's leave them alone.
[PATCH] expr.c: Correct indentations in expand_constructor
expand_constructor has
if ((TREE_STATIC (exp)
&& ((mode == BLKmode
&& ! (target != 0 && safe_from_p (target, exp, 1)))
|| TREE_ADDRESSABLE (exp)
|| (tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))
&& (! can_move_by_pieces
(tree_to_uhwi (TYPE_SIZE_UNIT (type)),
TYPE_ALIGN (type)))
&& ! mostly_zeros_p (exp
|| ((modifier == EXPAND_INITIALIZER || modifier == EXPAND_CONST_ADDRESS)
&& TREE_CONSTANT (exp)))
But indentations are misleading. It shuld be
if ((TREE_STATIC (exp)
&& ((mode == BLKmode
&& ! (target != 0 && safe_from_p (target, exp, 1)))
|| TREE_ADDRESSABLE (exp)
|| (tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))
&& (! can_move_by_pieces
(tree_to_uhwi (TYPE_SIZE_UNIT (type)),
TYPE_ALIGN (type)))
&& ! mostly_zeros_p (exp
|| ((modifier == EXPAND_INITIALIZER || modifier == EXPAND_CONST_ADDRESS)
&& TREE_CONSTANT (exp)))
* expr.c (expand_constructor): Correct indentations.
---
gcc/expr.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/gcc/expr.c b/gcc/expr.c
index af5bbeeb2f9..5d5d318a1a2 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -8137,12 +8137,12 @@ expand_constructor (tree exp, rtx target, enum
expand_modifier modifier,
if ((TREE_STATIC (exp)
&& ((mode == BLKmode
&& ! (target != 0 && safe_from_p (target, exp, 1)))
- || TREE_ADDRESSABLE (exp)
- || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))
- && (! can_move_by_pieces
-(tree_to_uhwi (TYPE_SIZE_UNIT (type)),
- TYPE_ALIGN (type)))
- && ! mostly_zeros_p (exp
+ || TREE_ADDRESSABLE (exp)
+ || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))
+ && (! can_move_by_pieces
+ (tree_to_uhwi (TYPE_SIZE_UNIT (type)),
+ TYPE_ALIGN (type)))
+ && ! mostly_zeros_p (exp
|| ((modifier == EXPAND_INITIALIZER || modifier == EXPAND_CONST_ADDRESS)
&& TREE_CONSTANT (exp)))
{
--
2.20.1
[PATCH] Fix PR89247
The following fixes LOOP_VECTORIZED IFNs made useless by CFG cleanup
after if-conversion by re-verifying the mentioned loops still exist.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2019-02-08 Richard Biener
PR tree-optimization/89247
* tree-if-conv.c: Include tree-cfgcleanup.h.
(version_loop_for_if_conversion): Record LOOP_VECTORIZED call.
(tree_if_conversion): Pass through predicate vector.
(pass_if_conversion::execute): Do CFG cleanup and SSA update
inline, see if any if-converted loops we refrece in
LOOP_VECTORIZED calls vanished and fixup.
* tree-if-conv.h (tree_if_conversion): Adjust prototype.
* gcc.dg/torture/pr89247.c: New testcase.
Index: gcc/tree-if-conv.c
===
--- gcc/tree-if-conv.c (revision 268668)
+++ gcc/tree-if-conv.c (working copy)
@@ -119,6 +119,7 @@ along with GCC; see the file COPYING3.
#include "internal-fn.h"
#include "fold-const.h"
#include "tree-ssa-sccvn.h"
+#include "tree-cfgcleanup.h"
/* Only handle PHIs with no more arguments unless we are asked to by
simd pragma. */
@@ -2719,7 +2720,7 @@ combine_blocks (struct loop *loop)
consistent after the condition is folded in the vectorizer. */
static struct loop *
-version_loop_for_if_conversion (struct loop *loop)
+version_loop_for_if_conversion (struct loop *loop, vec *preds)
{
basic_block cond_bb;
tree cond = make_ssa_name (boolean_type_node);
@@ -2759,6 +2760,7 @@ version_loop_for_if_conversion (struct l
new_loop->force_vectorize = false;
gsi = gsi_last_bb (cond_bb);
gimple_call_set_arg (g, 1, build_int_cst (integer_type_node, new_loop->num));
+ preds->safe_push (g);
gsi_insert_before (, g, GSI_SAME_STMT);
update_ssa (TODO_update_ssa);
return new_loop;
@@ -2979,7 +2981,7 @@ ifcvt_local_dce (basic_block bb)
changed. */
unsigned int
-tree_if_conversion (struct loop *loop)
+tree_if_conversion (struct loop *loop, vec *preds)
{
unsigned int todo = 0;
bool aggressive_if_conv;
@@ -3027,7 +3029,7 @@ tree_if_conversion (struct loop *loop)
struct loop *vloop
= (versionable_outer_loop_p (loop_outer (loop))
? loop_outer (loop) : loop);
- struct loop *nloop = version_loop_for_if_conversion (vloop);
+ struct loop *nloop = version_loop_for_if_conversion (vloop, preds);
if (nloop == NULL)
goto cleanup;
if (vloop != loop)
@@ -3139,11 +3141,12 @@ pass_if_conversion::execute (function *f
if (number_of_loops (fun) <= 1)
return 0;
+ auto_vec preds;
FOR_EACH_LOOP (loop, 0)
if (flag_tree_loop_if_convert == 1
|| ((flag_tree_loop_vectorize || loop->force_vectorize)
&& !loop->dont_vectorize))
- todo |= tree_if_conversion (loop);
+ todo |= tree_if_conversion (loop, );
if (todo)
{
@@ -3158,7 +3161,30 @@ pass_if_conversion::execute (function *f
gcc_assert (!bb->aux);
}
- return todo;
+ /* Perform IL update now, it might elide some loops. */
+ if (todo & TODO_cleanup_cfg)
+{
+ cleanup_tree_cfg ();
+ if (need_ssa_update_p (fun))
+ todo |= TODO_update_ssa;
+}
+ if (todo & TODO_update_ssa_any)
+update_ssa (todo & TODO_update_ssa_any);
+
+ /* If if-conversion elided the loop fall back to the original one. */
+ for (unsigned i = 0; i < preds.length (); ++i)
+{
+ gimple *g = preds[i];
+ unsigned ifcvt_loop = tree_to_uhwi (gimple_call_arg (g, 0));
+ if (!get_loop (fun, ifcvt_loop))
+ {
+ if (dump_file)
+ fprintf (dump_file, "If-converted loop vanished\n");
+ fold_loop_internal_call (g, boolean_false_node);
+ }
+}
+
+ return 0;
}
} // anon namespace
Index: gcc/tree-if-conv.h
===
--- gcc/tree-if-conv.h (revision 268668)
+++ gcc/tree-if-conv.h (working copy)
@@ -19,6 +19,6 @@ along with GCC; see the file COPYING3.
#ifndef GCC_TREE_IF_CONV_H
#define GCC_TREE_IF_CONV_H
-unsigned int tree_if_conversion (struct loop *);
+unsigned int tree_if_conversion (struct loop *, vec * = NULL);
#endif /* GCC_TREE_IF_CONV_H */
Index: gcc/testsuite/gcc.dg/torture/pr89247.c
===
--- gcc/testsuite/gcc.dg/torture/pr89247.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr89247.c (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+
+int *a;
+void b()
+{
+ void *c = &
+ for (;;)
+d:
+ if (*a)
+ ;
+ else
+ *a = ({ 0 < b; });
+}
[PATCH, i386]: Fix PR89221, --enable-frame-pointer does not work as intended
Hello!
Attached patch fixes --enable-frame-pointer handling, and this way
makes a couple of defines in config/i386/sol2.h obsolete.
2019-02-08 Uroš Bizjak
PR target/89221
* config.gcc (i[34567]86-*-*, x86_64-*-*): Move tests for enable_cld
and enable_frame_pointer ...
* configure.ac: ... here. Update help strings for --enable-cld and
--enable-frame-pointer.
(x86_64_*-*): Use USE_X86_64_FRAME_POINTER define.
* configure: Regenerate.
* config/i386/sol2.h (USE_IX86_FRAME_POINTER): Remove.
(USE_X86_64_FRAME_POINTER): Ditto.
Bootstrapped non x86_64-linux-gnu. Rainer, can you please test the
patch on solaris2 target?
Uros.
Index: config/i386/sol2.h
===
--- config/i386/sol2.h (revision 268670)
+++ config/i386/sol2.h (working copy)
@@ -248,9 +248,6 @@
#define ASAN_REJECT_SPEC \
DEF_ARCH64_SPEC("%e:-fsanitize=address is not supported in this
configuration")
-#define USE_IX86_FRAME_POINTER 1
-#define USE_X86_64_FRAME_POINTER 1
-
#undef NO_PROFILE_COUNTERS
#undef MCOUNT_NAME
Index: config.gcc
===
--- config.gcc (revision 268670)
+++ config.gcc (working copy)
@@ -604,12 +604,6 @@
echo "This target does not support --with-abi."
exit 1
fi
- if test "x$enable_cld" = xyes; then
- tm_defines="${tm_defines} USE_IX86_CLD=1"
- fi
- if test "x$enable_frame_pointer" = xyes; then
- tm_defines="${tm_defines} USE_IX86_FRAME_POINTER=1"
- fi
;;
x86_64-*-*)
case ${with_abi} in
@@ -630,12 +624,6 @@
echo "Unknown ABI used in --with-abi=$with_abi"
exit 1
esac
- if test "x$enable_cld" = xyes; then
- tm_defines="${tm_defines} USE_IX86_CLD=1"
- fi
- if test "x$enable_frame_pointer" = xyes; then
- tm_defines="${tm_defines} USE_IX86_FRAME_POINTER=1"
- fi
;;
arm*-*-*)
tm_p_file="arm/arm-flags.h ${tm_p_file} arm/aarch-common-protos.h"
Index: configure
===
--- configure (revision 268670)
+++ configure (working copy)
@@ -1687,9 +1687,8 @@
Link mingw executables with --large-address-aware
--enable-leading-mingw64-underscores
enable leading underscores on 64 bit mingw targets
- --enable-cldenable -mcld by default for 32bit x86
- --enable-frame-pointer enable -fno-omit-frame-pointer by default for 32bit
- x86
+ --enable-cldenable -mcld by default for x86
+ --enable-frame-pointer enable -fno-omit-frame-pointer by default for x86
--disable-win32-registry
disable lookup of installation paths in the Registry
on Windows hosts
@@ -12199,8 +12198,7 @@
case $target_os in
linux* | darwin[8912]*)
- # Enable -fomit-frame-pointer by default for Linux and Darwin with
- # DWARF2.
+ # Enable -fomit-frame-pointer by default for Linux and Darwin with DWARF2.
enable_frame_pointer=no
;;
*)
@@ -12211,6 +12209,25 @@
fi
+case $target in
+i[34567]86-*-*)
+ if test "x$enable_cld" = xyes; then
+ tm_defines="${tm_defines} USE_IX86_CLD=1"
+ fi
+ if test "x$enable_frame_pointer" = xyes; then
+ tm_defines="${tm_defines} USE_IX86_FRAME_POINTER=1"
+ fi
+ ;;
+x86_64-*-*)
+ if test "x$enable_cld" = xyes; then
+ tm_defines="${tm_defines} USE_IX86_CLD=1"
+ fi
+ if test "x$enable_frame_pointer" = xyes; then
+ tm_defines="${tm_defines} USE_X86_64_FRAME_POINTER=1"
+ fi
+ ;;
+esac
+
# Windows32 Registry support for specifying GCC installation paths.
# Check whether --enable-win32-registry was given.
if test "${enable_win32_registry+set}" = set; then :
@@ -18646,7 +18663,7 @@
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18640 "configure"
+#line 18666 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -18752,7 +18769,7 @@
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18746 "configure"
+#line 18772 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -25141,7 +25158,7 @@
no)
;;
*)
-as_fn_error "'$enableval' is an invalid value for
--enable-standard-branch-protection.\
+as_fn_error $? "'$enableval' is an invalid value for
--enable-standard-branch-protection.\
Valid choices are 'yes' and 'no'." "$LINENO" 5
;;
esac
Index: configure.ac
===
--- configure.ac(revision 268670)
+++ configure.ac
Re: [PATCH] S/390: Introduce jdd constraint
On 07.02.19 11:09, Ilya Leoshkevich wrote: > Bootstrapped and regtested on s390x-redhat-linux. > > Implementation of section anchors in S/390 back-end added in r266741 > broke jump labels in S/390 Linux kernel [1]. Currently jump labels > pass global variable addresses to .quad directive in inline assembly > using "X" constraint. In the past this used to produce regular symbol > references, however, after r266741 we sometimes get values like > (plus (reg) (const_int)), where (reg) points to a section anchor. > Strictly speaking, this is still correct, since "X" accepts anything. > Thus, now we need another way to support jump labels. > > The existing "i" constraint cannot be used, since with -fPIC it must > not accept non-local symbols, however, jump labels do require that, > e.g. __tracepoint_xdp_exception from kernel proper might be referenced > from kernel modules. > > The existing "ZL" constraint cannot be used for the same reason. > > The existing "b" constraint cannot be used because of the way > expand_asm_stmt works. It deduces whether the constraint allows > regs, subregs or mems, and processes asm operands differently based on > that. "b" is supposed to accept values like (mem (symbol_ref)), and > there appears to be no way to explain to expand_asm_stmt that for "b" > mem's address must not be in a register. > > This patch introduces the new machine-specific constraint named "jdd" - > "j" prefix is already used for constants, and "d" stands for "data". > It accepts anything that fits into the data section, whether or not > this might require a relocation, that is, anything that passes > CONSTANT_P check. > > [1] https://lkml.org/lkml/2019/1/23/346 > > gcc/ChangeLog: > > 2019-02-06 Ilya Leoshkevich > > * config/s390/constraints.md (jdd): New constraint. > > gcc/testsuite/ChangeLog: > > 2019-02-06 Ilya Leoshkevich > > * gcc.target/s390/jump-label.c: New test. Ok. Thanks! Andreas
Re: [PATCH] S/390: Implement vectory copysign
On 07.02.19 10:28, Robin Dapp wrote: > Hi, > > this patch implements vector copysign using vector select on S/390. > > Regtested and bootstrapped on s390x. > > Regards > Robin > > -- > > gcc/ChangeLog: > > 2019-02-07 Robin Dapp > > * config/s390/vector.md: Implement vector copysign. > > gcc/testsuite/ChangeLog: > > 2019-02-07 Robin Dapp > > * gcc.target/s390/vector/vec-copysign-execute.c: New test. > * gcc.target/s390/vector/vec-copysign.c: New test. > Ok. Thanks! Andreas
arm access to stack slot out of allocated area
Hello,
Below is a description of a very annoying bug we are witnessing
on ARM. I have ideas of possible ways to address it, but don't yet
have a patch.
The problem touches the synchronization of register elimination,
dataflow analysis and arm_expand_prologue. The overall logic is pretty
intricate, so I would appreciate at least insights from maintainers on
what would seem the most plausible direction.
If someone wants to take it, even better. I can certainly open a PR :)
The Ada testcase below, compiled with -Og -mapcs by an arm-eabi
compiler or simply with -Og by an arm-wrs-vxworks compiler (apcs
by default), produces code which uses a stack slot beyond the
allocated stack area for the "for_each" nested function.
The slot at hand is used as the stack home for the static chain,
which gets clobbered on the first call to Read and then bad things
happen. The actual manifestation we observed with the real code
was "fun" but I'll save that for the time being - that message is
long enough :)
We started observing this with a gcc-8 compiler and I could reproduce
with a pristine mainline early this week.
Here's a example compilation and the output we get, with notes
describing what register points where as the code proceeds:
$ arm-wrs-vxworks-gcc -S --RTS=kernel p.adb -Og -gnatn -o - |& grep -A 30
"for_each.*:"
p__compute_blocks__for_each_item$2970:
.LFB5:
.cfi_startproc
@ Nested: function declared inside another function.
@ args = 0, pretend = 0, frame = 36
@ frame_needed = 1, uses_anonymous_args = 0
str ip, [sp, #-4]! -- push ip
add ip, sp, #4 -- ip = entry_sp
push{r4, r5, r6, r7, r8, r9, r10, fp, ip, lr, pc} -- push 11 regs,
44 bytes
sub fp, ip, #8 -- fp = ip - 8, that is, entry_sp - 8,
-- first registers save slot.
ldr ip, [fp, #4]
sub sp, sp, #32 -- allocate 32 more bytes. sp is entry_sp - 80 here
-- (push ip = 4 bytes, multi push = 44 bytes, then
-- allocate 32)
mov r8, r0
str r1, [fp, #-68]
str ip, [fp, #-76] -- store ip at fp - 76, that is, entry_sp - 84,
-- 4 bytes beyond the end of the allocated frame.
The problematic store is initially emitted as
p.adb.235r.expand:(insn 3 2 4 2 (set (reg/f:SI 111 [ CHAIN$45 ])
p.adb.235r.expand-(reg:SI 12 ip [ CHAIN$45 ])) "p.adb":24:7 -1
p.adb.235r.expand- (nil))
by expand_function_start, then becomes
p.adb.279r.reload:(insn 3 320 4 2 (set (mem/c:SI (plus:SI (reg/f:SI 11 fp)
p.adb.279r.reload-(const_int -76 [0xffb4])) [19
%sfp+-36 S4 A32])
p.adb.279r.reload-(reg:SI 12 ip [ CHAIN$47 ])) "p.adb":29:7 179
{*arm_movsi_insn}
p.adb.279r.reload- (nil))
The core issue is a disagreement between register elimination
(which turns %sfp-36 into %fp-76) and prologue expansion regarding
static_chain_stack_bytes(), used both in arm_compute_frame_layout()
and arm_expand_prologue().
arm_compute_static_chain_stack_bytes() returns 0 during the whole
of register allocation, up to the end of register elimination and
reload_completed flipped to true.
The key factor is arm_r3_live_at_start_p() returning false all along.
Then we very quickly get to:
if (optimize)
df_analyze ();
in do_reload(), and arm_r3_live_at_start_p () starts returning true.
arm_expand_prologue proceeds on this basis and decides to
use a slot just ahead of "the stack frame" to temporarily store
IP.
Even though this store isn't really part of the "register save"
operations per se, the code sort of pretends it is with:
gcc_assert(arm_compute_static_chain_stack_bytes() == 4);
saved_regs += 4;
which later on shortens the amount of allocated stack by:
if (offsets->outgoing_args != offsets->saved_args + saved_regs)
{ ...
amount = GEN_INT (offsets->saved_args + saved_regs
- offsets->outgoing_args);
All in all, expand_prologue operates from a view of the stack
layout which doesn't match what register elimination has seen,
and the sfp->fp elimination is off by 4.
The first idea that came to mind was that relying on
r3_live_at_start_p during register elimination seems
fragile, and maybe the prologue expansion should
use a different spot for the ip temp storage.
The prologue logic is complex though, so I'm not 100% sure
this is a reasonable track.
Another idea was to detect the perception mismatch and
either skip the "use slot above frame" option in this case,
or arrange to adjust the final stack pointer update accordingly.
The first of these last two options seems cleaner as it
goes towards a more precise synchronisation between the layout
assumed by register elimination and the layout actually setup
by the prologue expander.
But I haven't yet experimented with it and I'm not certain
how viable
Re: [PATCH] Narrow ARRAY*_REF indexes to sizetype for gimple (PR tree-optimization/89223)
On Fri, Feb 08, 2019 at 12:47:08PM +0100, Richard Biener wrote:
> My worry is that while we know the IV i doesn't wrap we
> don't know the same for (sizetype)i in
>
> for (__int128 i = 0; i < n; ++i)
> a[i] = 0.;
First of all, for normal targets I think it is very rare that people use
__int128 array indexes (or long long on 32-bit targets).
I agree it can happen on weird targets more often.
> and thus vectorization doesn't work. Similar for
>
> void foo(long n, int *a)
> {
> for (long i = 0; i < n; ++i)
> a[i] = 1;
> }
Well, we should be able to improve the number of iterations analysis here
for the > sizetype ones, no valid array should have more than PTRDIFF_MAX
or SIZE_MAX-1 elements (well, all objects should have computable sizeof and
pointer subtraction needs to work, so it is even divided by the element type)
and thus we could derive some reasonable range for the loops with > sizetype
IVs where we use that.
Jakub
[PATCH] Un-XFAIL gcc.dg/vect/vect-24.c on vect_condition targets
Tested on x86_64-unknown-linux-gnu, applied.
Richard.
2019-02-08 Richard Biener
PR testsuite/89250
* gcc.dg/vect/vect-24.c: Remove XFAIL on vect_condition targets.
Index: gcc/testsuite/gcc.dg/vect/vect-24.c
===
--- gcc/testsuite/gcc.dg/vect/vect-24.c (revision 268668)
+++ gcc/testsuite/gcc.dg/vect/vect-24.c (working copy)
@@ -122,5 +122,7 @@ int main (void)
return main1 ();
}
-/* { dg-final { scan-tree-dump-times "vectorized 3 loops" 1 "vect" { xfail { {
! aarch64*-*-* } && { ! arm-*-* } } } } } */
+/* The short-cutting || is if-converted using COND_EXPRs rather than
+ bitwise or. */
+/* { dg-final { scan-tree-dump-times "vectorized 3 loops" 1 "vect" { xfail { !
vect_condition } } } } */
/* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0
"vect" { xfail { ! vect_align_stack_vars } } } } */
Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 11:13:50AM +1030, Alan Modra wrote:
> PR target/88343
> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
> setup, simplifying ABI_V4 case to df_regs_ever_live_p.
That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
simplifying the ABI_V4 case. This one passes regression testing.
OK to apply?
PR target/88343
* config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
case. Match logic in rs6000_emit_prologue emitting pic_offset_table
setup.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index cced90bb518..b4908c4f795 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
static bool
save_reg_p (int reg)
{
- /* We need to mark the PIC offset register live for the same conditions
- as it is set up, or otherwise it won't be saved before we clobber it. */
-
if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
{
/* When calling eh_return, we must return true for all the cases
where conditional_register_usage marks the PIC offset reg
-call used. */
+call used or fixed. */
+ if (crtl->calls_eh_return
+ && ((DEFAULT_ABI == ABI_V4 && flag_pic)
+ || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
+ || (TARGET_TOC && TARGET_MINIMAL_TOC)))
+ return true;
+
+ /* We need to mark the PIC offset register live for the same
+conditions as it is set up in rs6000_emit_prologue, or
+otherwise it won't be saved before we clobber it. */
if (TARGET_TOC && TARGET_MINIMAL_TOC
- && (crtl->calls_eh_return
- || df_regs_ever_live_p (reg)
- || !constant_pool_empty_p ()))
+ && !constant_pool_empty_p ())
+ return true;
+
+ if (DEFAULT_ABI == ABI_V4
+ && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
+ && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
return true;
- if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
+ if (DEFAULT_ABI == ABI_DARWIN
&& flag_pic && crtl->uses_pic_offset_table)
return true;
}
--
Alan Modra
Australia Development Lab, IBM
Re: [PATCH] Narrow ARRAY*_REF indexes to sizetype for gimple (PR tree-optimization/89223)
On Fri, 8 Feb 2019, Jakub Jelinek wrote:
> On Fri, Feb 08, 2019 at 10:48:30AM +0100, Richard Biener wrote:
> > > --- gcc/gimplify.c.jj 2019-01-28 23:30:53.199762928 +0100
> > > +++ gcc/gimplify.c2019-02-06 17:15:35.368976785 +0100
> > > @@ -2977,6 +2977,12 @@ gimplify_compound_lval (tree *expr_p, gi
> > >
> > >if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF)
> > > {
> > > + /* Expansion will cast the index to sizetype, so any excess bits
> > > + aren't useful for optimizations. */
> > > + if (!error_operand_p (TREE_OPERAND (t, 1))
> > > + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (t, 1)))
> > > + > TYPE_PRECISION (sizetype)))
> > > + TREE_OPERAND (t, 1) = fold_convert (sizetype, TREE_OPERAND (t, 1));
> >
> > Shouldn't we use ssizetype in case the index was signed (or always)?
>
> As get_inner_reference uses sizetype, I think it is best to do what will it
> do in the end. We use sizetype for POINTER_PLUS_EXPR as well and SCEV needs
> to handle that too. If we changed POINTER_PLUS_EXPR to use ssizetype, using
> ssizetype for the indexes would make sense too and we could even do the
> casts if it is sizetype originally. That said, adding conversions to either
> sizetype or ssizetype if the index is narrowed is IMHO harmful.
>
> > I wonder what the effects are on SCEV-analyzability when the IV is, say
> > [unsigned] __int128 and the uses we analyze are then ([s]sizetype) IV
> > vs. plain IV. That is, I'm not sure exposing the casting on GIMPLE
> > is always a good idea. Did you look at how SCEV behaves with the change?
>
> Don't see it would change any SCEV decisions, there are minor differences
> like
> (instantiate_scev
>(instantiate_below = 2 -> 3)
>(evolution_loop = 1)
> - (chrec = {0x40004, +, 0xfffe}_1)
> - (res = {0x40004, +, 0xfffe}_1))
> + (chrec = {4, +, 18446744073709551615}_1)
> + (res = {4, +, 18446744073709551615}_1))
> in -fdump-tree-all-scev, but the optimizations are pretty much same,
> initially the IL is tiny bit larger (because of the extra converts), soon it
> is smaller:
> - _15 = a[0x80008];
> - a[0x40004] = _15;
> - _23 = a[0x60006];
> - a[0x30003] = _23;
> - _31 = a[0x40004];
> - a[0x20002] = _31;
> - a[0x10001] = _31;
> + _5 = a[8];
> + a[4] = _5;
> + _26 = a[6];
> + a[3] = _26;
> + a[2] = _5;
> + a[1] = _5;
> in *.optimized. By exposing the casts, guess e.g. GIMPLE opts could find
> out that a[2] and a[0x20002] is the same thing etc. (rather than
> (sometimes?) discovering that only during RTL optimizations when it is
> expanded that way).
My worry is that while we know the IV i doesn't wrap we
don't know the same for (sizetype)i in
for (__int128 i = 0; i < n; ++i)
a[i] = 0.;
and thus vectorization doesn't work. Similar for
void foo(long n, int *a)
{
for (long i = 0; i < n; ++i)
a[i] = 1;
}
where if you cast i to int or unsigned int in the array reference
SCEV says
Creating dr for *_4
analyze_innermost: t.c:4:15: missed: failed: evolution of base is not
affine.
So the important thing for GIMPLE is to ensure to _not_ have
narrowing of IVs. Yes, sizetype is probably not too bad here
on normal targets but some embedded ones have sizetype 16bits
but pointers 24bits and if people write long or int IVs they
get bitten. Btw, pure sign-changes seem fine here (even to
unsigned).
One thing I don't like too much is exposing more 'sizetype' to the
IL before we "fixed" POINTER_PLUS_EXPR et al to take signed sizetype
args...
> > Otherwise I agree. Not sure if we want to do this during stage4 though.
>
> If you want to defer for stage1, I can certainly wait with that.
Definitely.
Richard.
> Jakub
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On Fri, Feb 08, 2019 at 11:29:10AM +, Matthew Malcomson wrote:
> I'm pretty sure there's no difference between the iwmmxt target and
> others so believe your simpler fix of just using 'q' is a good idea.
> (there's no difference in gas and no documentation I have found mentions
> a difference).
The simpler patch would be then (but of course in that case the question is
why iwmmxt.md doesn't use those q constraints for the output_move_double
alternatives).
2019-02-08 Jakub Jelinek
PR bootstrap/88714
* config/arm/ldrdstrd.md (*arm_ldrd, *arm_strd): Use q constraint
instead of r.
--- gcc/config/arm/ldrdstrd.md.jj 2019-02-08 11:25:42.368916124 +0100
+++ gcc/config/arm/ldrdstrd.md 2019-02-08 12:38:33.647585108 +0100
@@ -157,9 +157,9 @@ (define_peephole2 ; swap the destination
;; We use gen_operands_ldrd_strd() with a modify argument as false so that the
;; operands are not changed.
(define_insn "*arm_ldrd"
- [(parallel [(set (match_operand:SI 0 "s_register_operand" "=r")
+ [(parallel [(set (match_operand:SI 0 "s_register_operand" "=q")
(match_operand:SI 2 "memory_operand" "m"))
- (set (match_operand:SI 1 "s_register_operand" "=r")
+ (set (match_operand:SI 1 "s_register_operand" "=q")
(match_operand:SI 3 "memory_operand" "m"))])]
"TARGET_LDRD && TARGET_ARM && reload_completed
&& valid_operands_ldrd_strd (operands, true)"
@@ -178,9 +178,9 @@ (define_insn "*arm_ldrd"
(define_insn "*arm_strd"
[(parallel [(set (match_operand:SI 2 "memory_operand" "=m")
- (match_operand:SI 0 "s_register_operand" "r"))
+ (match_operand:SI 0 "s_register_operand" "q"))
(set (match_operand:SI 3 "memory_operand" "=m")
- (match_operand:SI 1 "s_register_operand" "r"))])]
+ (match_operand:SI 1 "s_register_operand" "q"))])]
"TARGET_LDRD && TARGET_ARM && reload_completed
&& valid_operands_ldrd_strd (operands, false)"
{
Jakub
[Ada] Fix out-of-bounds read with wild unchecked conversion
This is not really a regression (or maybe a very old one) but the behavior of
the compiler is a bit annoying so IMO worth fixing: when you put a size clause
to silence the warning about an unchecked conversion from a small type to a
(very) large one, the compiler effectively disregards the clause if the source
is an aggregate, thus generating an out-of-bounds read.
Tested on x86_64-suse-linux, applied on mainline.
2019-02-08 Eric Botcazou
* gcc-interface/trans.c (gnat_to_gnu) : Minor tweak.
* gcc-interface/utils.c (convert): Do not pad when doing an unchecked
conversion here. Use TREE_CONSTANT throughout the function.
(unchecked_convert): Also pad if the source is a CONSTRUCTOR and the
destination is a more aligned array type or a larger aggregate type,
but not between original and packable versions of a type.
--
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c (revision 268674)
+++ gcc-interface/trans.c (working copy)
@@ -7230,13 +7230,10 @@ gnat_to_gnu (Node_Id gnat_node)
{
tree gnu_aggr_type;
- /* ??? It is wrong to evaluate the type now, but there doesn't
- seem to be any other practical way of doing it. */
-
+ /* Check that this aggregate has not slipped through the cracks. */
gcc_assert (!Expansion_Delayed (gnat_node));
- gnu_aggr_type = gnu_result_type
- = get_unpadded_type (Etype (gnat_node));
+ gnu_result_type = get_unpadded_type (Etype (gnat_node));
if (TREE_CODE (gnu_result_type) == RECORD_TYPE
&& TYPE_CONTAINS_TEMPLATE_P (gnu_result_type))
@@ -7244,6 +7241,8 @@ gnat_to_gnu (Node_Id gnat_node)
= TREE_TYPE (DECL_CHAIN (TYPE_FIELDS (gnu_result_type)));
else if (TREE_CODE (gnu_result_type) == VECTOR_TYPE)
gnu_aggr_type = TYPE_REPRESENTATIVE_ARRAY (gnu_result_type);
+ else
+ gnu_aggr_type = gnu_result_type;
if (Null_Record_Present (gnat_node))
gnu_result = gnat_build_constructor (gnu_aggr_type, NULL);
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c (revision 268675)
+++ gcc-interface/utils.c (working copy)
@@ -4356,19 +4356,12 @@ convert (tree type, tree expr)
/* If the inner type is of self-referential size and the expression type
is a record, do this as an unchecked conversion unless both types are
- essentially the same. But first pad the expression if possible to
- have the same size on both sides. */
+ essentially the same. */
if (ecode == RECORD_TYPE
&& CONTAINS_PLACEHOLDER_P (DECL_SIZE (TYPE_FIELDS (type)))
&& TYPE_MAIN_VARIANT (etype)
!= TYPE_MAIN_VARIANT (TREE_TYPE (TYPE_FIELDS (type
- {
- if (TREE_CODE (TYPE_SIZE (etype)) == INTEGER_CST)
- expr = convert (maybe_pad_type (etype, TYPE_SIZE (type), 0, Empty,
- false, false, false, true),
- expr);
- return unchecked_convert (type, expr, false);
- }
+ return unchecked_convert (type, expr, false);
/* If we are converting between array types with variable size, do the
final conversion as an unchecked conversion, again to avoid the need
@@ -4483,9 +4476,9 @@ convert (tree type, tree expr)
case STRING_CST:
/* If we are converting a STRING_CST to another constrained array type,
just make a new one in the proper type. */
- if (code == ecode && AGGREGATE_TYPE_P (etype)
- && !(TREE_CODE (TYPE_SIZE (etype)) == INTEGER_CST
- && TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST))
+ if (code == ecode
+ && !(TREE_CONSTANT (TYPE_SIZE (etype))
+ && !TREE_CONSTANT (TYPE_SIZE (type
{
expr = copy_node (expr);
TREE_TYPE (expr) = type;
@@ -5332,7 +5325,7 @@ unchecked_convert (tree type, tree expr,
so we skip all expressions that are references. */
else if (!REFERENCE_CLASS_P (expr)
&& !AGGREGATE_TYPE_P (etype)
- && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
+ && TREE_CONSTANT (TYPE_SIZE (type))
&& (c = tree_int_cst_compare (TYPE_SIZE (etype), TYPE_SIZE (type
{
if (c < 0)
@@ -5380,16 +5373,36 @@ unchecked_convert (tree type, tree expr,
return unchecked_convert (type, expr, notrunc_p);
}
- /* If we are converting a CONSTRUCTOR to a more aligned RECORD_TYPE, bump
- the alignment of the CONSTRUCTOR to speed up the copy operation. */
+ /* If we are converting a CONSTRUCTOR to a more aligned aggregate type, bump
+ the alignment of the CONSTRUCTOR to speed up the copy operation. But do
+ not do it for a conversion between original and packable version to avoid
+ an infinite recursion. */
else if (TREE_CODE (expr) == CONSTRUCTOR
- && code == RECORD_TYPE
+ && AGGREGATE_TYPE_P (type)
+ && TYPE_NAME (type) != TYPE_NAME (etype)
&& TYPE_ALIGN (etype) < TYPE_ALIGN (type))
{
expr = convert (maybe_pad_type (etype, NULL_TREE, TYPE_ALIGN (type),
Empty, false,
Re: [PATCH] i386: Use OI/TImode in *mov[ot]i_internal_avx with AVX512VL
On Fri, Feb 8, 2019 at 1:51 AM Uros Bizjak wrote: > > On Thu, Feb 7, 2019 at 10:11 PM H.J. Lu wrote: > > > > OImode and TImode moves must be done in XImode to access upper 16 > > vector registers without AVX512VL. With AVX512VL, we can access > > upper 16 vector registers in OImode and TImode. > > > > PR target/89229 > > * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for > > upper 16 vector registers without TARGET_AVX512VL. > > (*movti_internal): Likewise. > > Please use (not (match_test "...")) instead of (match_test "!...") and > put the new test as the first argument of the AND rtx. > > LGTM with the above change. This is the patch I am checking in. Thanks. H.J. --- OImode and TImode moves must be done in XImode to access upper 16 vector registers without AVX512VL. With AVX512VL, we can access upper 16 vector registers in OImode and TImode. PR target/89229 * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for upper 16 vector registers without TARGET_AVX512VL. (*movti_internal): Likewise. --- gcc/config/i386/i386.md | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index c1492363bca..3d9141ae450 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -1933,8 +1933,9 @@ (set_attr "type" "sselog1,sselog1,ssemov,ssemov") (set_attr "prefix" "vex") (set (attr "mode") - (cond [(ior (match_operand 0 "ext_sse_reg_operand") - (match_operand 1 "ext_sse_reg_operand")) + (cond [(and (not (match_test "TARGET_AVX512VL")) + (ior (match_operand 0 "ext_sse_reg_operand") + (match_operand 1 "ext_sse_reg_operand"))) (const_string "XI") (and (eq_attr "alternative" "1") (match_test "TARGET_AVX512VL")) @@ -2012,8 +2013,9 @@ (set (attr "mode") (cond [(eq_attr "alternative" "0,1") (const_string "DI") -(ior (match_operand 0 "ext_sse_reg_operand") - (match_operand 1 "ext_sse_reg_operand")) +(and (not (match_test "TARGET_AVX512VL")) + (ior (match_operand 0 "ext_sse_reg_operand") + (match_operand 1 "ext_sse_reg_operand"))) (const_string "XI") (and (eq_attr "alternative" "3") (match_test "TARGET_AVX512VL")) --
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On 08/02/19 10:23, Jakub Jelinek wrote: > On Fri, Feb 08, 2019 at 11:06:02AM +0100, Christophe Lyon wrote: >> On Fri, 8 Feb 2019 at 10:51, Jakub Jelinek wrote: >>> >>> On Fri, Feb 08, 2019 at 10:18:03AM +0100, Christophe Lyon wrote: I'm afaid this patch causes several regressions. Maybe they have already been fixed post-commit (I have several validations for later commits still running)? >>> >>> The following patch fixes the single ICE I've tried to reproduce. >>> While iwmmxt.md movdi pattern uses =m, r and =r, m constraints, both >>> arm.md and vfp.md movdi patterns use =m, q and =q, m constraints and thus >>> allow also ip register. The following patch is an attempt to do the same >>> thing, just in the same patterns through arch_enabled attribute. >>> >>> Completely untested. >>> >>> 2019-02-08 Jakub Jelinek >>> >>> PR bootstrap/88714 >>> * config/arm/ldrdstrd.md (*arm_ldrd, *arm_strd): Add alternative >>> with >>> q constraint instead of r, enable it only if not >>> TARGET_REALLY_IWMMXT. >> >> I've started validations with this patch, I expect the results later today. > > Thanks. Note, I don't understand why iwmmxt.md doesn't use q constraint, if > it is only some omission, or some hw requirement. So, the patch just > follows what iwmmxt.md does. I guess because iwmmxt.md movsi also uses r > constraint, ip really shouldn't appear on that target. But if just changing > all r constraints to q without any arch_enabled works, that would be even > simpler. > > Jakub > Apologies for the break to everyone, and thanks Jakub for the fix. I believe my mistake was that I didn't match the behaviour of the function `operands_ok_ldrd_strd` (that the peepholes use) and the constraints: so the peepholes allow using ip while the insn patterns don't. I'm pretty sure there's no difference between the iwmmxt target and others so believe your simpler fix of just using 'q' is a good idea. (there's no difference in gas and no documentation I have found mentions a difference). I had decided to use 'r' instead of 'q' based on the phrase "ARM strongly recommends that you do not use R12 for Rt." under the "doubleword register restrictions" heading in the link below http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0802a/CIHGJHED.html But don't think the statement is something that *must* be followed, since it's a recommendation in the same list as hard requirements. There's no mention of this in any ARM ARM that I can find. Matthew
[Ada] Fix crash on very dynamic record types
This is a regression present on the mainline and 8 branch: in some specific circumstances, you can get a crash when max_size is called from the gimplifier on very dynamic record types. This makes the function a bit more robust. Tested on x86_64-suse-linux, applied on mainline and 8 branch. 2019-02-08 Eric Botcazou * gcc-interface/utils.c (max_size) : Be prepared for an operand with VOID_TYPE. -- Eric BotcazouIndex: gcc-interface/utils.c === --- gcc-interface/utils.c (revision 268508) +++ gcc-interface/utils.c (working copy) @@ -3642,7 +3642,10 @@ fntype_same_flags_p (const_tree t, tree /* EXP is an expression for the size of an object. If this size contains discriminant references, replace them with the maximum (if MAX_P) or - minimum (if !MAX_P) possible value of the discriminant. */ + minimum (if !MAX_P) possible value of the discriminant. + + Note that the expression may have already been gimplified,in which case + COND_EXPRs have VOID_TYPE and no operands, and this must be handled. */ tree max_size (tree exp, bool max_p) @@ -3714,11 +3717,15 @@ max_size (tree exp, bool max_p) return build_int_cst (type, max_p ? 1 : 0); case tcc_unary: + op0 = TREE_OPERAND (exp, 0); + if (code == NON_LVALUE_EXPR) - return max_size (TREE_OPERAND (exp, 0), max_p); + return max_size (op0, max_p); + + if (VOID_TYPE_P (TREE_TYPE (op0))) + return max_p ? TYPE_MAX_VALUE (type) : TYPE_MIN_VALUE (type); - op0 = max_size (TREE_OPERAND (exp, 0), - code == NEGATE_EXPR ? !max_p : max_p); + op0 = max_size (op0, code == NEGATE_EXPR ? !max_p : max_p); if (op0 == TREE_OPERAND (exp, 0)) return exp;
[Ada] Fix profile mismatch with -fprofile-{generate,use} at -O2
This is a regression present on all active branches: in some cases, you get
bogus profile mismatches with -fprofile-{generate,use} at -O2 because some -O3
optimizations are automatically enabled by -fprofile-use.
Tested on x86_64-suse-linux, applied on all active branches.
2019-02-08 Eric Botcazou
* gcc-interface/trans.c (Regular_Loop_to_gnu): Replace tests on
individual flag_unswitch_loops and flag_tree_loop_vectorize switches
with test on global optimize switch.
(Raise_Error_to_gnu): Likewise.
--
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c (revision 268508)
+++ gcc-interface/trans.c (working copy)
@@ -3787,7 +3787,7 @@ Regular_Loop_to_gnu (Node_Id gnat_node,
unswitching is enabled, do not require the loop bounds to be also
invariant, as their evaluation will still be ahead of the loop. */
if (vec_safe_length (gnu_loop_info->checks) > 0
- && (make_invariant (_low, _high) || flag_unswitch_loops))
+ && (make_invariant (_low, _high) || optimize >= 3))
{
struct range_check_info_d *rci;
unsigned int i, n_remaining_checks = 0;
@@ -3840,22 +3840,21 @@ Regular_Loop_to_gnu (Node_Id gnat_node,
/* Note that loop unswitching can only be applied a small number of
times to a given loop (PARAM_MAX_UNSWITCH_LEVEL default to 3). */
if (IN_RANGE (n_remaining_checks, 1, 3)
- && optimize > 1
+ && optimize >= 2
&& !optimize_size)
FOR_EACH_VEC_ELT (*gnu_loop_info->checks, i, rci)
if (rci->invariant_cond != boolean_false_node)
{
TREE_OPERAND (rci->inserted_cond, 0) = rci->invariant_cond;
- if (flag_unswitch_loops)
+ if (optimize >= 3)
add_stmt_with_node_force (rci->inserted_cond, gnat_node);
}
}
/* Second, if loop vectorization is enabled and the iterations of the
loop can easily be proved as independent, mark the loop. */
- if (optimize
- && flag_tree_loop_vectorize
+ if (optimize >= 3
&& independent_iterations_p (LOOP_STMT_BODY (gnu_loop_stmt)))
LOOP_STMT_IVDEP (gnu_loop_stmt) = 1;
@@ -6478,7 +6477,7 @@ Raise_Error_to_gnu (Node_Id gnat_node, t
= build1 (SAVE_EXPR, boolean_type_node, boolean_true_node);
vec_safe_push (loop->checks, rci);
gnu_cond = build_noreturn_cond (gnat_to_gnu (gnat_cond));
- if (flag_unswitch_loops)
+ if (optimize >= 3)
gnu_cond = build_binary_op (TRUTH_ANDIF_EXPR,
boolean_type_node,
rci->inserted_cond,
Re: [rs6000] 64-bit integer loads/stores and FP instructions
> Backporting this is okay. (It was not done because it does not affect > correctness). What is the "almost", btw? The predicate of operand #0 of movdi_internal32 is rs6000_nonimmediate_operand on the 7 branch and nonimmediate_operand on the 8 branch and later. > (In https://gcc.gnu.org/ml/gcc-patches/2016-11/txt4j9hLRLCzf.txt and you > presumably mean the *movdi_internal32 hunk, and the first line of it: > > - "=Y,r, r, ?m,?*d,?*d, > + "=Y,r, r, ^m,^d, ^d, > > where the last three are mem<-reg, reg<-mem, reg<-reg, for fp regs). > > This patch was a year before we switched to LRA, and for non-LRA ports * > does not do any register preferencing. Are you sure? See record_reg_classes in ira-costs.c: case '*': /* Ignore the next letter for this pass. */ c = *++p; break; > For 6xx/7xx people *wanted* to use FP loads and stores whenever possible, > because you get twice the bandwidth with them. > > If you do not want the FP registers used (or FP insns used), use > -msoft-float, that's what it's for. No disagreement, but you can't force people to use it when they use FP code in other parts of their software. Believe me, we (and probably others) tried... > Patches are welcome, but complicating this already very complex code by > introducing an arbitrary distinction between "embedded and other" (or > actually, "server and other") isn't the way to go. Sorry. Not clear what you mean by complicating here... the approach is rather simple and precisely aimed at not disturbing the common path, i.e. server processors, while preserving the historical path for other processors. AFAIK nobody has evaluated the effects of the original change beyond POWER 7/8/9. > Could you open a PR please? Sure, but about what now? ;-) -- Eric Botcazou
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On Fri, Feb 08, 2019 at 11:06:02AM +0100, Christophe Lyon wrote: > On Fri, 8 Feb 2019 at 10:51, Jakub Jelinek wrote: > > > > On Fri, Feb 08, 2019 at 10:18:03AM +0100, Christophe Lyon wrote: > > > I'm afaid this patch causes several regressions. Maybe they have > > > already been fixed post-commit (I have several validations for later > > > commits still running)? > > > > The following patch fixes the single ICE I've tried to reproduce. > > While iwmmxt.md movdi pattern uses =m, r and =r, m constraints, both > > arm.md and vfp.md movdi patterns use =m, q and =q, m constraints and thus > > allow also ip register. The following patch is an attempt to do the same > > thing, just in the same patterns through arch_enabled attribute. > > > > Completely untested. > > > > 2019-02-08 Jakub Jelinek > > > > PR bootstrap/88714 > > * config/arm/ldrdstrd.md (*arm_ldrd, *arm_strd): Add alternative > > with > > q constraint instead of r, enable it only if not > > TARGET_REALLY_IWMMXT. > > I've started validations with this patch, I expect the results later today. Thanks. Note, I don't understand why iwmmxt.md doesn't use q constraint, if it is only some omission, or some hw requirement. So, the patch just follows what iwmmxt.md does. I guess because iwmmxt.md movsi also uses r constraint, ip really shouldn't appear on that target. But if just changing all r constraints to q without any arch_enabled works, that would be even simpler. Jakub
Re: [PATCH] Narrow ARRAY*_REF indexes to sizetype for gimple (PR tree-optimization/89223)
On Fri, Feb 08, 2019 at 10:48:30AM +0100, Richard Biener wrote:
> > --- gcc/gimplify.c.jj 2019-01-28 23:30:53.199762928 +0100
> > +++ gcc/gimplify.c 2019-02-06 17:15:35.368976785 +0100
> > @@ -2977,6 +2977,12 @@ gimplify_compound_lval (tree *expr_p, gi
> >
> >if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF)
> > {
> > + /* Expansion will cast the index to sizetype, so any excess bits
> > +aren't useful for optimizations. */
> > + if (!error_operand_p (TREE_OPERAND (t, 1))
> > + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (t, 1)))
> > + > TYPE_PRECISION (sizetype)))
> > + TREE_OPERAND (t, 1) = fold_convert (sizetype, TREE_OPERAND (t, 1));
>
> Shouldn't we use ssizetype in case the index was signed (or always)?
As get_inner_reference uses sizetype, I think it is best to do what will it
do in the end. We use sizetype for POINTER_PLUS_EXPR as well and SCEV needs
to handle that too. If we changed POINTER_PLUS_EXPR to use ssizetype, using
ssizetype for the indexes would make sense too and we could even do the
casts if it is sizetype originally. That said, adding conversions to either
sizetype or ssizetype if the index is narrowed is IMHO harmful.
> I wonder what the effects are on SCEV-analyzability when the IV is, say
> [unsigned] __int128 and the uses we analyze are then ([s]sizetype) IV
> vs. plain IV. That is, I'm not sure exposing the casting on GIMPLE
> is always a good idea. Did you look at how SCEV behaves with the change?
Don't see it would change any SCEV decisions, there are minor differences
like
(instantiate_scev
(instantiate_below = 2 -> 3)
(evolution_loop = 1)
- (chrec = {0x40004, +, 0xfffe}_1)
- (res = {0x40004, +, 0xfffe}_1))
+ (chrec = {4, +, 18446744073709551615}_1)
+ (res = {4, +, 18446744073709551615}_1))
in -fdump-tree-all-scev, but the optimizations are pretty much same,
initially the IL is tiny bit larger (because of the extra converts), soon it
is smaller:
- _15 = a[0x80008];
- a[0x40004] = _15;
- _23 = a[0x60006];
- a[0x30003] = _23;
- _31 = a[0x40004];
- a[0x20002] = _31;
- a[0x10001] = _31;
+ _5 = a[8];
+ a[4] = _5;
+ _26 = a[6];
+ a[3] = _26;
+ a[2] = _5;
+ a[1] = _5;
in *.optimized. By exposing the casts, guess e.g. GIMPLE opts could find
out that a[2] and a[0x20002] is the same thing etc. (rather than
(sometimes?) discovering that only during RTL optimizations when it is
expanded that way).
> Otherwise I agree. Not sure if we want to do this during stage4 though.
If you want to defer for stage1, I can certainly wait with that.
Jakub
Re: [testsuite, ada] Don't XPASS gnat.dg/lto19.adb
> Between 20181106 (r265849) and 20181107 (r265879), gnat.dg/lto19.adb > started to XPASS everywhere: > > XPASS: gnat.dg/lto19.adb (test for excess errors) The idea was to wait until Jan fixes the lto8 failure introduced roughly at the same time... -- Eric Botcazou
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On Fri, 8 Feb 2019 at 10:51, Jakub Jelinek wrote:
>
> On Fri, Feb 08, 2019 at 10:18:03AM +0100, Christophe Lyon wrote:
> > I'm afaid this patch causes several regressions. Maybe they have
> > already been fixed post-commit (I have several validations for later
> > commits still running)?
>
> The following patch fixes the single ICE I've tried to reproduce.
> While iwmmxt.md movdi pattern uses =m, r and =r, m constraints, both
> arm.md and vfp.md movdi patterns use =m, q and =q, m constraints and thus
> allow also ip register. The following patch is an attempt to do the same
> thing, just in the same patterns through arch_enabled attribute.
>
> Completely untested.
>
> 2019-02-08 Jakub Jelinek
>
> PR bootstrap/88714
> * config/arm/ldrdstrd.md (*arm_ldrd, *arm_strd): Add alternative with
> q constraint instead of r, enable it only if not TARGET_REALLY_IWMMXT.
I've started validations with this patch, I expect the results later today.
Thanks
>
> --- gcc/config/arm/ldrdstrd.md.jj 2019-02-07 17:33:38.841669141 +0100
> +++ gcc/config/arm/ldrdstrd.md 2019-02-08 10:42:56.515325579 +0100
> @@ -157,10 +157,10 @@ (define_peephole2 ; swap the destination
> ;; We use gen_operands_ldrd_strd() with a modify argument as false so that
> the
> ;; operands are not changed.
> (define_insn "*arm_ldrd"
> - [(parallel [(set (match_operand:SI 0 "s_register_operand" "=r")
> - (match_operand:SI 2 "memory_operand" "m"))
> - (set (match_operand:SI 1 "s_register_operand" "=r")
> - (match_operand:SI 3 "memory_operand" "m"))])]
> + [(parallel [(set (match_operand:SI 0 "s_register_operand" "=q,r")
> + (match_operand:SI 2 "memory_operand" "m,m"))
> + (set (match_operand:SI 1 "s_register_operand" "=q,r")
> + (match_operand:SI 3 "memory_operand" "m,m"))])]
>"TARGET_LDRD && TARGET_ARM && reload_completed
>&& valid_operands_ldrd_strd (operands, true)"
>{
> @@ -173,14 +173,17 @@ (define_insn "*arm_ldrd"
> (symbol_ref "arm_count_ldrdstrd_insns (operands, true) * 4"))
> (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "type" "load_8")
> - (set_attr "predicable" "yes")]
> -)
> + (set_attr "predicable" "yes")
> + (set (attr "arch_enabled")
> + (if_then_else (and (match_test "TARGET_REALLY_IWMMXT")
> + (eq_attr "alternative" "0"))
> + (const_string "no") (const_string "yes")))])
>
> (define_insn "*arm_strd"
> - [(parallel [(set (match_operand:SI 2 "memory_operand" "=m")
> - (match_operand:SI 0 "s_register_operand" "r"))
> - (set (match_operand:SI 3 "memory_operand" "=m")
> - (match_operand:SI 1 "s_register_operand" "r"))])]
> + [(parallel [(set (match_operand:SI 2 "memory_operand" "=m,m")
> + (match_operand:SI 0 "s_register_operand" "q,r"))
> + (set (match_operand:SI 3 "memory_operand" "=m,m")
> + (match_operand:SI 1 "s_register_operand" "q,r"))])]
>"TARGET_LDRD && TARGET_ARM && reload_completed
>&& valid_operands_ldrd_strd (operands, false)"
>{
> @@ -193,5 +196,8 @@ (define_insn "*arm_strd"
> (symbol_ref "arm_count_ldrdstrd_insns (operands, false) * 4"))
> (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "type" "store_8")
> - (set_attr "predicable" "yes")]
> -)
> + (set_attr "predicable" "yes")
> + (set (attr "arch_enabled")
> + (if_then_else (and (match_test "TARGET_REALLY_IWMMXT")
> + (eq_attr "alternative" "0"))
> + (const_string "no") (const_string "yes")))])
>
> Jakub
Re: [PATCH] Fix ICE due to copy_reg_eh_region_note_forward (PR rtl-optimization/89234)
> The following testcase ICEs on ppc64le. The problem is that > copy_reg_eh_region_note_* functions accept either some instruction, or > REG_EH_REGION note directly. To differentiate between those it uses INSN_P > test (and returns early if the insn doesn't contain any REG_EH_REGION > notes). If the function is called on a rtx_insn * that isn't INSN_P, like > on the testcase on a NOTE, it assumes it must be REG_EH_REGION note, uses > XEXP (note, 0) on it, which is actually PREV_INSN in this case and stores > an instruction (JUMP_INSN in this case) into REG_EH_REGION notes it creates. It took me a few minutes to clear the confusion between NOTE & REG_NOTE here. > I believe we should treat rtx_insn * that aren't INSN_P like instructions > that don't have any REG_EH_REGION notes. > > 2019-02-07 Jakub Jelinek > > PR rtl-optimization/89234 > * except.c (copy_reg_eh_region_note_forward): Return if note_or_insn > is a NOTE, CODE_LABEL etc. - rtx_insn * other than INSN_P. > (copy_reg_eh_region_note_backward): Likewise. > > * g++.dg/ubsan/pr89234.C: New test. OK, thanks. -- Eric Botcazou
Re: [PATCH] i386: Use OI/TImode in *mov[ot]i_internal_avx with AVX512VL
On Thu, Feb 7, 2019 at 10:11 PM H.J. Lu wrote: > > OImode and TImode moves must be done in XImode to access upper 16 > vector registers without AVX512VL. With AVX512VL, we can access > upper 16 vector registers in OImode and TImode. > > PR target/89229 > * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for > upper 16 vector registers without TARGET_AVX512VL. > (*movti_internal): Likewise. Please use (not (match_test "...")) instead of (match_test "!...") and put the new test as the first argument of the AND rtx. LGTM with the above change. Uros. > --- > gcc/config/i386/i386.md | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index c1492363bca..e7f4b9a9c8d 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -1933,8 +1933,9 @@ > (set_attr "type" "sselog1,sselog1,ssemov,ssemov") > (set_attr "prefix" "vex") > (set (attr "mode") > - (cond [(ior (match_operand 0 "ext_sse_reg_operand") > - (match_operand 1 "ext_sse_reg_operand")) > + (cond [(and (ior (match_operand 0 "ext_sse_reg_operand") > +(match_operand 1 "ext_sse_reg_operand")) > + (match_test "!TARGET_AVX512VL")) > (const_string "XI") >(and (eq_attr "alternative" "1") > (match_test "TARGET_AVX512VL")) > @@ -2012,8 +2013,9 @@ > (set (attr "mode") > (cond [(eq_attr "alternative" "0,1") > (const_string "DI") > - (ior (match_operand 0 "ext_sse_reg_operand") > - (match_operand 1 "ext_sse_reg_operand")) > + (and (ior (match_operand 0 "ext_sse_reg_operand") > +(match_operand 1 "ext_sse_reg_operand")) > + (match_test "!TARGET_AVX512VL")) > (const_string "XI") >(and (eq_attr "alternative" "3") > (match_test "TARGET_AVX512VL")) > -- > 2.20.1 >
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On Fri, Feb 08, 2019 at 10:18:03AM +0100, Christophe Lyon wrote:
> I'm afaid this patch causes several regressions. Maybe they have
> already been fixed post-commit (I have several validations for later
> commits still running)?
The following patch fixes the single ICE I've tried to reproduce.
While iwmmxt.md movdi pattern uses =m, r and =r, m constraints, both
arm.md and vfp.md movdi patterns use =m, q and =q, m constraints and thus
allow also ip register. The following patch is an attempt to do the same
thing, just in the same patterns through arch_enabled attribute.
Completely untested.
2019-02-08 Jakub Jelinek
PR bootstrap/88714
* config/arm/ldrdstrd.md (*arm_ldrd, *arm_strd): Add alternative with
q constraint instead of r, enable it only if not TARGET_REALLY_IWMMXT.
--- gcc/config/arm/ldrdstrd.md.jj 2019-02-07 17:33:38.841669141 +0100
+++ gcc/config/arm/ldrdstrd.md 2019-02-08 10:42:56.515325579 +0100
@@ -157,10 +157,10 @@ (define_peephole2 ; swap the destination
;; We use gen_operands_ldrd_strd() with a modify argument as false so that the
;; operands are not changed.
(define_insn "*arm_ldrd"
- [(parallel [(set (match_operand:SI 0 "s_register_operand" "=r")
- (match_operand:SI 2 "memory_operand" "m"))
- (set (match_operand:SI 1 "s_register_operand" "=r")
- (match_operand:SI 3 "memory_operand" "m"))])]
+ [(parallel [(set (match_operand:SI 0 "s_register_operand" "=q,r")
+ (match_operand:SI 2 "memory_operand" "m,m"))
+ (set (match_operand:SI 1 "s_register_operand" "=q,r")
+ (match_operand:SI 3 "memory_operand" "m,m"))])]
"TARGET_LDRD && TARGET_ARM && reload_completed
&& valid_operands_ldrd_strd (operands, true)"
{
@@ -173,14 +173,17 @@ (define_insn "*arm_ldrd"
(symbol_ref "arm_count_ldrdstrd_insns (operands, true) * 4"))
(set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "type" "load_8")
- (set_attr "predicable" "yes")]
-)
+ (set_attr "predicable" "yes")
+ (set (attr "arch_enabled")
+ (if_then_else (and (match_test "TARGET_REALLY_IWMMXT")
+ (eq_attr "alternative" "0"))
+ (const_string "no") (const_string "yes")))])
(define_insn "*arm_strd"
- [(parallel [(set (match_operand:SI 2 "memory_operand" "=m")
- (match_operand:SI 0 "s_register_operand" "r"))
- (set (match_operand:SI 3 "memory_operand" "=m")
- (match_operand:SI 1 "s_register_operand" "r"))])]
+ [(parallel [(set (match_operand:SI 2 "memory_operand" "=m,m")
+ (match_operand:SI 0 "s_register_operand" "q,r"))
+ (set (match_operand:SI 3 "memory_operand" "=m,m")
+ (match_operand:SI 1 "s_register_operand" "q,r"))])]
"TARGET_LDRD && TARGET_ARM && reload_completed
&& valid_operands_ldrd_strd (operands, false)"
{
@@ -193,5 +196,8 @@ (define_insn "*arm_strd"
(symbol_ref "arm_count_ldrdstrd_insns (operands, false) * 4"))
(set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "type" "store_8")
- (set_attr "predicable" "yes")]
-)
+ (set_attr "predicable" "yes")
+ (set (attr "arch_enabled")
+ (if_then_else (and (match_test "TARGET_REALLY_IWMMXT")
+ (eq_attr "alternative" "0"))
+ (const_string "no") (const_string "yes")))])
Jakub
Re: [PATCH] Narrow ARRAY*_REF indexes to sizetype for gimple (PR tree-optimization/89223)
On Thu, 7 Feb 2019, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, given that during expansion we expand ARRAY_REFs
> using get_inner_reference that casts ARRAY_*REF indexes to sizetype, all the
> bits above sizetype are ignored (whether one uses long long indexes on
> 32-bit targets or __int128 indexes on 64-bit ones), so I think it is
> desirable to make that visible already for GIMPLE optimizations, where we
> can narrow other arithmetic stmts feeding those indexes etc., it could help
> vectorization (e.g. gather/scatter), etc. Of course, fixing tree-data-ref.c
> etc. to cope with wider constants or chrecs is desirable as well.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-02-06 Jakub Jelinek
>
> PR tree-optimization/89223
> * gimplify.c (gimplify_compound_lval): Narrow ARRAY*_REF indexes wider
> than sizetype to sizetype.
>
> * gcc.c-torture/execute/pr89223.c: New test.
>
> --- gcc/gimplify.c.jj 2019-01-28 23:30:53.199762928 +0100
> +++ gcc/gimplify.c2019-02-06 17:15:35.368976785 +0100
> @@ -2977,6 +2977,12 @@ gimplify_compound_lval (tree *expr_p, gi
>
>if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF)
> {
> + /* Expansion will cast the index to sizetype, so any excess bits
> + aren't useful for optimizations. */
> + if (!error_operand_p (TREE_OPERAND (t, 1))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (t, 1)))
> + > TYPE_PRECISION (sizetype)))
> + TREE_OPERAND (t, 1) = fold_convert (sizetype, TREE_OPERAND (t, 1));
Shouldn't we use ssizetype in case the index was signed (or always)?
I wonder what the effects are on SCEV-analyzability when the IV is, say
[unsigned] __int128 and the uses we analyze are then ([s]sizetype) IV
vs. plain IV. That is, I'm not sure exposing the casting on GIMPLE
is always a good idea. Did you look at how SCEV behaves with the change?
Otherwise I agree. Not sure if we want to do this during stage4 though.
Thanks,
Richard.
> /* Gimplify the dimension. */
> if (!is_gimple_min_invariant (TREE_OPERAND (t, 1)))
> {
> --- gcc/testsuite/gcc.c-torture/execute/pr89223.c.jj 2019-02-07
> 18:09:39.869088769 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr89223.c 2019-02-07
> 18:09:12.359543231 +0100
> @@ -0,0 +1,27 @@
> +/* PR tree-optimization/89223 */
> +/* { dg-do run { target int128 } } */
> +
> +int a[9] = { 1, 2, 3, 4, 5, 6, 7, 8, 9 };
> +unsigned __int128 b;
> +
> +__attribute__((noipa)) void
> +foo (void)
> +{
> + for (b = (((unsigned __int128) 4) << 64) + 4; b != 0; b -= unsigned
> __int128) 1) << 64) + 1))
> +a[b] = a[b + b];
> +}
> +
> +int
> +main ()
> +{
> + int i;
> + if (sizeof (__UINTPTR_TYPE__) * __CHAR_BIT__ != 64
> + || sizeof (__SIZE_TYPE__) * __CHAR_BIT__ != 64
> + || sizeof (__int128) * __CHAR_BIT__ != 128)
> +return 0;
> + foo ();
> + for (i = 0; i < 9; ++i)
> +if (a[i] != (((1 << i) & 0x16) != 0 ? 9 : i == 3 ? 7 : i + 1))
> + __builtin_abort ();
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
[testsuite, ada] Don't XPASS gnat.dg/lto19.adb
Between 20181106 (r265849) and 20181107 (r265879), gnat.dg/lto19.adb
started to XPASS everywhere:
XPASS: gnat.dg/lto19.adb (test for excess errors)
Fixed as follows, tested on i386-pc-solaris2.11 and
sparc-sun-solaris2.11.
Ok for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2019-02-08 Rainer Orth
* gnat.dg/lto19.adb: Remove dg-excess-errors.
# HG changeset patch
# Parent 8fbd3c9e3f106341e6b4070ccd57077b71274fb8
Don't XPASS gnat.dg/lto19.adb
diff --git a/gcc/testsuite/gnat.dg/lto19.adb b/gcc/testsuite/gnat.dg/lto19.adb
--- a/gcc/testsuite/gnat.dg/lto19.adb
+++ b/gcc/testsuite/gnat.dg/lto19.adb
@@ -1,6 +1,5 @@
-- { dg-do run }
-- { dg-options "-flto" { target lto } }
--- { dg-excess-errors "does not match original declaration" }
with Lto19_Pkg1;
Re: [PATCH][libbacktrace] Declare external backtrace fns noinline
On Fri, Feb 8, 2019 at 10:41 AM Tom de Vries wrote:
>
> Hi,
>
> The backtrace functions backtrace_full, backtrace_print and backtrace_simple
> walk the call stack, but make sure to skip the first entry, in order to skip
> over the functions themselves, and start the backtrace at the caller of the
> functions.
>
> When compiling with -flto, the functions may be inlined, causing them to skip
> over the caller instead.
>
> Fix this by declaring the functions with __attribute__((noinline)).
>
> OK for trunk?
OK.
Richard.
> Thanks,
> - Tom
>
> [libbacktrace] Declare external backtrace fns noinline
>
> 2019-02-08 Tom de Vries
>
> * backtrace.c (backtrace_full): Declare with
> __attribute__((noinline)).
> * print.c (backtrace_print): Same.
> * simple.c (backtrace_simple): Same.
>
> ---
> libbacktrace/backtrace.c | 2 +-
> libbacktrace/print.c | 2 +-
> libbacktrace/simple.c| 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libbacktrace/backtrace.c b/libbacktrace/backtrace.c
> index 29204c63313..c579e803825 100644
> --- a/libbacktrace/backtrace.c
> +++ b/libbacktrace/backtrace.c
> @@ -98,7 +98,7 @@ unwind (struct _Unwind_Context *context, void *vdata)
>
> /* Get a stack backtrace. */
>
> -int
> +int __attribute__((noinline))
> backtrace_full (struct backtrace_state *state, int skip,
> backtrace_full_callback callback,
> backtrace_error_callback error_callback, void *data)
> diff --git a/libbacktrace/print.c b/libbacktrace/print.c
> index b2f45446443..0767facecae 100644
> --- a/libbacktrace/print.c
> +++ b/libbacktrace/print.c
> @@ -80,7 +80,7 @@ error_callback (void *data, const char *msg, int errnum)
>
> /* Print a backtrace. */
>
> -void
> +void __attribute__((noinline))
> backtrace_print (struct backtrace_state *state, int skip, FILE *f)
> {
>struct print_data data;
> diff --git a/libbacktrace/simple.c b/libbacktrace/simple.c
> index d439fcee8e2..118936397da 100644
> --- a/libbacktrace/simple.c
> +++ b/libbacktrace/simple.c
> @@ -90,7 +90,7 @@ simple_unwind (struct _Unwind_Context *context, void *vdata)
>
> /* Get a simple stack backtrace. */
>
> -int
> +int __attribute__((noinline))
> backtrace_simple (struct backtrace_state *state, int skip,
> backtrace_simple_callback callback,
> backtrace_error_callback error_callback, void *data)
Vectorizer Data Structure re-org
I am working on an internal data structure overhaul in the vectorizer to support future changes. The first task is to remove the non-SLP representation which will simplify code and improve maintainability. This first task is actually the most difficult and time-consuming one. I have pushed the current state of my work as a series of commits onto the rguenth/forceslp git branch. The first commits in the series are improvements/fixes for trunk I plan to commit once stage1 opens again. The real changes done sofar are - allow group_size == 1 SLP instances - force vectorization fail when non-SLP stmts are left as debugging tool to see what remains to be done - make the SLP graph cyclic to handle cycles in inner loops with outer loop vectorization - start to rewrite reduction handling - I've split out nested cycles handling sofar which now is much simpler and handles almost arbitrary inner loops vectorizer testsuite summary on x86_64 is === gcc Summary === # of expected passes3103 # of unexpected failures85 # of unexpected successes 10 # of expected failures 52 # of unsupported tests 30 === g++ Summary === # of expected passes255 # of expected failures 6 # of unsupported tests 3 === gfortran Summary === # of expected passes131 # of unexpected failures8 # of unsupported tests 1 where a PASS of course doesn't mean the generated code has the same quality than before. The main remaining issues are reduction handling (esp. condition reduction and other cases that were never implemented for SLP) plus interleaving cases that cannot be handled within the current SLP permutation framework (require more than two source vectors). For the interleaving issue I have to tackle part two of the data structure overhaul and rework permutation handling to be represented more explicit in the SLP graph and make sure we do not ever need to throw away / dissolve DR groups. There are various vectorizations that lack SLP support on current trunk and you are free to contribute missing pieces there. I hope to not get too many extra road-blocks from changes done on trunk, so please do not add new features w/o considering SLP support ;) Thanks, Richard.
Re: [PATCH, fortran ieee]: PR 88678, Many gfortran.dg/ieee/ieee_X.f90 test cases fail starting with r267465
On Fri, Feb 8, 2019 at 12:53 AM Steve Ellcey wrote:
>
> On Thu, 2019-01-31 at 08:46 +0100, Uros Bizjak wrote:
> > On Wed, Jan 30, 2019 at 9:51 PM Janne Blomqvist
> >
> > > This seems to change the only user of support_fpu_trap() that is
> > > different from support_fpu_flag(), so with this change one could
> > > remove support_fpu_trap() entirely and modify all callers (since
> > > it's an internal function it's not used outside libgfortran) to
> > > call support_fpu_flag() directly. Otherwise Ok.
> >
> > Some targets only support IEEE flags (so, flags in some FP status
> > register), but not exceptions (traps) based on these flags that halt
> > the program. Currently, fpu-glibc.h assumes that all flags can
> > generate exceptions (that is true for the current set of gfortran
> > targets), but some future target wants to return 0 from
> > support_fpu_trap.
> >
> > Uros.
>
> Is this actually true for all existing gfortran targets? Specifically
> I am wondering about Arm and Aarch64. PR 78314 says that ARM trapping
> FPU exceptions are optional.
This is assumed in the tests and in the library, please see bellow.
> I am currently seeing gfortran.dg/ieee/ieee_6.f90 fail on my Aarch64
> ThunderX box. I wonder if we should have an Arm/Aarch64 specific
> version of the fpu header file (fpu-arm.h) that would use the previous
> version of support_fpu_trap.
The fix for PR78314 was wrong on two accounts:
a) Probing for supported traps by trying to enable and disable trap
will fire exception when the trap is disabled, but the flag in the
status register is set. You can try this on x86 by forcing it to use
unpatched previous fpu-glibc instead of fpu-387 in
libgfortran/configure.host. Please also note PR78314, comment #16,
where it is reported that the test passes if target (QEMU arm/aarch64)
supports exceptions. So, if you use the old approach in arm specific
version of the file, I suspect you will regress other tests on
arm/aarch64 targets when/if optional exceptions are supported.
b) As reported in PR78314, comment #12, libgfortran currently assumes that:
gcc/fortran/simplify.c:
gfc_expr *
simplify_ieee_support (gfc_expr *expr)
{
/* We consider that if the IEEE modules are loaded, we have full support
for flags, halting and rounding, which are the three functions
(IEEE_SUPPORT_{FLAG,HALTING,ROUNDING}) allowed in constant
expressions. One day, we will need libgfortran to detect support and
communicate it back to us, allowing for partial support. */
return gfc_get_logical_expr (gfc_default_logical_kind, >where,
true);
}
so, the reverted patch neglected this assumption. Ignoring this, we can use
--cut here--
Index: libgfortran/config/fpu-glibc.h
===
--- libgfortran/config/fpu-glibc.h (revision 268424)
+++ libgfortran/config/fpu-glibc.h (working copy)
@@ -129,6 +129,10 @@
int
support_fpu_trap (int flag)
{
+#if defined(__arm__) || defined(__aarch64__)
+ return 0;
+#endif
+
return support_fpu_flag (flag);
}
--cut here--
which would in effect revert to the previous status on arm/aarch64
targets, while using correct setting for other targets, where
exception support is assumed.
Uros.
[PATCH][libbacktrace] Add btest_lto
Hi, Add libbacktrace test-case using -flto. OK for trunk? Thanks, - Tom [libbacktrace] Add btest_lto 2019-02-08 Tom de Vries * Makefile.am (BUILDTESTS): Add btest_lto. * Makefile.in: Regenerate. * btest.c (test1, f2, f3, test3, f22, f23): Declare with __attribute__((noclone)). --- libbacktrace/Makefile.am | 6 ++ libbacktrace/Makefile.in | 47 --- libbacktrace/btest.c | 12 ++-- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index 71a2ed478cc..46d7de48fd1 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -222,6 +222,12 @@ btest_LDADD = libbacktrace.la BUILDTESTS += btest +btest_lto_SOURCES = btest.c testlib.c +btest_lto_CFLAGS = $(AM_CFLAGS) -g -O -flto +btest_lto_LDADD = libbacktrace.la + +BUILDTESTS += btest_lto + btest_alloc_SOURCES = $(btest_SOURCES) btest_alloc_CFLAGS = $(btest_CFLAGS) btest_alloc_LDADD = libbacktrace_alloc.la diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in index a2b595e9bb0..c65c40d95d8 100644 --- a/libbacktrace/Makefile.in +++ b/libbacktrace/Makefile.in @@ -126,8 +126,8 @@ TESTS = $(am__append_4) $(am__append_6) $(am__append_7) \ @HAVE_ELF_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_1 = libbacktrace_elf_for_test.la @NATIVE_TRUE@am__append_2 = test_elf test_xcoff_32 test_xcoff_64 \ @NATIVE_TRUE@ test_pecoff test_unknown unittest unittest_alloc \ -@NATIVE_TRUE@ btest btest_alloc stest stest_alloc ztest \ -@NATIVE_TRUE@ ztest_alloc edtest edtest_alloc +@NATIVE_TRUE@ btest btest_lto btest_alloc stest stest_alloc \ +@NATIVE_TRUE@ ztest ztest_alloc edtest edtest_alloc @NATIVE_TRUE@am__append_3 = allocfail @NATIVE_TRUE@am__append_4 = allocfail.sh @HAVE_ELF_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_5 = b2test \ @@ -205,10 +205,10 @@ libbacktrace_noformat_la_OBJECTS = \ @NATIVE_TRUE@ test_xcoff_64$(EXEEXT) test_pecoff$(EXEEXT) \ @NATIVE_TRUE@ test_unknown$(EXEEXT) unittest$(EXEEXT) \ @NATIVE_TRUE@ unittest_alloc$(EXEEXT) btest$(EXEEXT) \ -@NATIVE_TRUE@ btest_alloc$(EXEEXT) stest$(EXEEXT) \ -@NATIVE_TRUE@ stest_alloc$(EXEEXT) ztest$(EXEEXT) \ -@NATIVE_TRUE@ ztest_alloc$(EXEEXT) edtest$(EXEEXT) \ -@NATIVE_TRUE@ edtest_alloc$(EXEEXT) +@NATIVE_TRUE@ btest_lto$(EXEEXT) btest_alloc$(EXEEXT) \ +@NATIVE_TRUE@ stest$(EXEEXT) stest_alloc$(EXEEXT) \ +@NATIVE_TRUE@ ztest$(EXEEXT) ztest_alloc$(EXEEXT) \ +@NATIVE_TRUE@ edtest$(EXEEXT) edtest_alloc$(EXEEXT) @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__EXEEXT_4 = ttest$(EXEEXT) \ @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@ ttest_alloc$(EXEEXT) @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__EXEEXT_5 = \ @@ -253,6 +253,13 @@ btest_alloc_OBJECTS = $(am_btest_alloc_OBJECTS) btest_alloc_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ $(LIBTOOLFLAGS) --mode=link $(CCLD) $(btest_alloc_CFLAGS) \ $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@ +@NATIVE_TRUE@am_btest_lto_OBJECTS = btest_lto-btest.$(OBJEXT) \ +@NATIVE_TRUE@ btest_lto-testlib.$(OBJEXT) +btest_lto_OBJECTS = $(am_btest_lto_OBJECTS) +@NATIVE_TRUE@btest_lto_DEPENDENCIES = libbacktrace.la +btest_lto_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ + $(LIBTOOLFLAGS) --mode=link $(CCLD) $(btest_lto_CFLAGS) \ + $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am_ctesta_OBJECTS = ctesta-btest.$(OBJEXT) \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctesta-testlib.$(OBJEXT) ctesta_OBJECTS = $(am_ctesta_OBJECTS) @@ -410,7 +417,7 @@ SOURCES = $(libbacktrace_la_SOURCES) $(EXTRA_libbacktrace_la_SOURCES) \ $(libbacktrace_instrumented_alloc_la_SOURCES) \ $(libbacktrace_noformat_la_SOURCES) $(allocfail_SOURCES) \ $(b2test_SOURCES) $(b3test_SOURCES) $(btest_SOURCES) \ - $(btest_alloc_SOURCES) $(ctesta_SOURCES) \ + $(btest_alloc_SOURCES) $(btest_lto_SOURCES) $(ctesta_SOURCES) \ $(ctesta_alloc_SOURCES) $(ctestg_SOURCES) \ $(ctestg_alloc_SOURCES) $(edtest_SOURCES) \ $(edtest_alloc_SOURCES) $(stest_SOURCES) \ @@ -880,6 +887,9 @@ BUILDTESTS = $(am__append_2) $(am__append_11) $(am__append_13) @NATIVE_TRUE@btest_SOURCES = btest.c testlib.c @NATIVE_TRUE@btest_CFLAGS = $(AM_CFLAGS) -g -O @NATIVE_TRUE@btest_LDADD = libbacktrace.la +@NATIVE_TRUE@btest_lto_SOURCES = btest.c testlib.c +@NATIVE_TRUE@btest_lto_CFLAGS = $(AM_CFLAGS) -g -O -flto +@NATIVE_TRUE@btest_lto_LDADD = libbacktrace.la @NATIVE_TRUE@btest_alloc_SOURCES = $(btest_SOURCES) @NATIVE_TRUE@btest_alloc_CFLAGS = $(btest_CFLAGS) @NATIVE_TRUE@btest_alloc_LDADD = libbacktrace_alloc.la @@ -1065,6 +1075,10 @@ btest_alloc$(EXEEXT): $(btest_alloc_OBJECTS) $(btest_alloc_DEPENDENCIES) $(EXTRA @rm -f btest_alloc$(EXEEXT) $(AM_V_CCLD)$(btest_alloc_LINK) $(btest_alloc_OBJECTS) $(btest_alloc_LDADD) $(LIBS) +btest_lto$(EXEEXT): $(btest_lto_OBJECTS)
[PATCH][libbacktrace] Declare external backtrace fns noinline
Hi,
The backtrace functions backtrace_full, backtrace_print and backtrace_simple
walk the call stack, but make sure to skip the first entry, in order to skip
over the functions themselves, and start the backtrace at the caller of the
functions.
When compiling with -flto, the functions may be inlined, causing them to skip
over the caller instead.
Fix this by declaring the functions with __attribute__((noinline)).
OK for trunk?
Thanks,
- Tom
[libbacktrace] Declare external backtrace fns noinline
2019-02-08 Tom de Vries
* backtrace.c (backtrace_full): Declare with __attribute__((noinline)).
* print.c (backtrace_print): Same.
* simple.c (backtrace_simple): Same.
---
libbacktrace/backtrace.c | 2 +-
libbacktrace/print.c | 2 +-
libbacktrace/simple.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libbacktrace/backtrace.c b/libbacktrace/backtrace.c
index 29204c63313..c579e803825 100644
--- a/libbacktrace/backtrace.c
+++ b/libbacktrace/backtrace.c
@@ -98,7 +98,7 @@ unwind (struct _Unwind_Context *context, void *vdata)
/* Get a stack backtrace. */
-int
+int __attribute__((noinline))
backtrace_full (struct backtrace_state *state, int skip,
backtrace_full_callback callback,
backtrace_error_callback error_callback, void *data)
diff --git a/libbacktrace/print.c b/libbacktrace/print.c
index b2f45446443..0767facecae 100644
--- a/libbacktrace/print.c
+++ b/libbacktrace/print.c
@@ -80,7 +80,7 @@ error_callback (void *data, const char *msg, int errnum)
/* Print a backtrace. */
-void
+void __attribute__((noinline))
backtrace_print (struct backtrace_state *state, int skip, FILE *f)
{
struct print_data data;
diff --git a/libbacktrace/simple.c b/libbacktrace/simple.c
index d439fcee8e2..118936397da 100644
--- a/libbacktrace/simple.c
+++ b/libbacktrace/simple.c
@@ -90,7 +90,7 @@ simple_unwind (struct _Unwind_Context *context, void *vdata)
/* Get a simple stack backtrace. */
-int
+int __attribute__((noinline))
backtrace_simple (struct backtrace_state *state, int skip,
backtrace_simple_callback callback,
backtrace_error_callback error_callback, void *data)
[committed][libbacktrace] Handle DW_FORM_ref_addr
Hi,
Add handling of the DW_FORM_ref_addr encoding to libbacktrace.
Committed to trunk.
Thanks,
- Tom
[libbacktrace] Handle DW_FORM_ref_addr
2019-02-08 Tom de Vries
PR libbacktrace/78063
* dwarf.c (build_address_map): Keep all parsed units.
(read_referenced_name_from_attr): Handle DW_FORM_ref_addr.
---
libbacktrace/dwarf.c | 32
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index e78d36b0b43..d7dacf3ef32 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -1513,14 +1513,12 @@ build_address_map (struct backtrace_state *state,
uintptr_t base_address,
size_t units_count;
size_t i;
struct unit **pu;
- size_t prev_addrs_count;
size_t unit_offset = 0;
memset (>vec, 0, sizeof addrs->vec);
memset (_vec->vec, 0, sizeof unit_vec->vec);
addrs->count = 0;
unit_vec->count = 0;
- prev_addrs_count = 0;
/* Read through the .debug_info section. FIXME: Should we use the
.debug_aranges section? gdb and addr2line don't use it, but I'm
@@ -1620,18 +1618,6 @@ build_address_map (struct backtrace_state *state,
uintptr_t base_address,
if (unit_buf.reported_underflow)
goto fail;
-
- if (addrs->count > prev_addrs_count || unit_tag == DW_TAG_partial_unit)
- prev_addrs_count = addrs->count;
- else
- {
- /* Unit was not used; remove it from the vector. */
- --units_count;
- units.size -= sizeof (u);
- units.alc += sizeof (u);
- free_abbrevs (state, >abbrevs, error_callback, data);
- backtrace_free (state, u, sizeof *u, error_callback, data);
- }
}
if (info.reported_underflow)
goto fail;
@@ -2189,13 +2175,19 @@ read_referenced_name_from_attr (struct dwarf_data
*ddata, struct unit *u,
return NULL;
}
- if (attr->form == DW_FORM_ref_addr
- || attr->form == DW_FORM_ref_sig8)
+ if (attr->form == DW_FORM_ref_sig8)
+return NULL;
+
+ if (val->encoding == ATTR_VAL_REF_INFO)
{
- /* This refers to an abstract origin defined in
-some other compilation unit. We can handle
-this case if we must, but it's harder. */
- return NULL;
+ struct unit *unit
+ = find_unit (ddata->units, ddata->units_count,
+val->u.uint);
+ if (unit == NULL)
+ return NULL;
+
+ uint64_t offset = val->u.uint - unit->low_offset;
+ return read_referenced_name (ddata, unit, offset, error_callback, data);
}
if (val->encoding == ATTR_VAL_UINT
Re: [Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes
On Tue, 5 Feb 2019 at 15:44, Matthew Malcomson wrote: > > These peepholes match a pair of SImode loads or stores that can be > implemented with a single LDRD or STRD instruction. > When compiling for TARGET_ARM, these peepholes originally created a set > pattern in DI mode to be caught by movdi patterns. > > This approach failed to take into account the possibility that the two > matched insns operated on memory with different aliasing information. > The peepholes lost the aliasing information on one of the insns, which > could then cause the scheduler to make an invalid transformation. > > This patch changes the peepholes so they generate a PARALLEL expression > of the two relevant loads or stores, which means the aliasing > information of both is kept. Such a PARALLEL pattern is what the > peepholes currently produce for TARGET_THUMB2. > > In order to match these new insn patterns, we add two new define_insn's. > These > define_insn's use the same checks as the peepholes to find valid insns. > > Note that the patterns now created by the peepholes for LDRD and STRD > are very similar to those created by the peepholes for LDM and STM. > Many patterns could be matched by the LDM and STM define_insns, which > means we rely on the order the define_insn patterns are defined in the > machine description, with those for LDRD/STRD defined before those for > LDM/STM. > > The difference between the peepholes for LDRD/STRD and those for LDM/STM > are mainly that those for LDRD/STRD have some logic to ensure that the > two registers are consecutive and the first one is even. > > Bootstrapped and regtested on arm-none-linux-gnu. > Demonstrated fix of bug 88714 by bootstrapping on armv7l. > > > gcc/ChangeLog: > > 2019-02-05 Matthew Malcomson > > PR bootstrap/88714 > * config/arm/arm-protos.h (valid_operands_ldrd_strd, > arm_count_ldrdstrd_insns): New declarations. > * config/arm/arm.c (mem_ok_for_ldrd_strd): Remove broken handling of > MINUS. > (valid_operands_ldrd_strd): New function. > (arm_count_ldrdstrd_insns): New function. > * config/arm/ldrdstrd.md: Change peepholes to generate PARALLEL SImode > sets instead of single DImode set and define new insns to match this. > > gcc/testsuite/ChangeLog: > > 2019-02-05 Matthew Malcomson > > * gcc.c-torture/execute/pr88714.c: New test. > * gcc.dg/rtl/arm/ldrd-peepholes.c: New test. > Hi! I'm afaid this patch causes several regressions. Maybe they have already been fixed post-commit (I have several validations for later commits still running)? For the whole picture, see: http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/268644/report-build-info.html Namely there are some ICEs: --target arm-none-linux-gnueabi --with-mode arm --with-cpu cortex-a9 --with-fpu default gcc.c-torture/execute/builtins/memcpy-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/memmove-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/mempcpy-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/memset-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/sprintf-chk.c compilation, -O2 (internal compiler error) gcc.c-torture/execute/builtins/sprintf-chk.c compilation, -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error) gcc.c-torture/execute/builtins/sprintf-chk.c compilation, -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/execute/builtins/sprintf-chk.c compilation, -O3 -g (internal compiler error) gcc.c-torture/execute/builtins/stpcpy-chk.c compilation, -O2 (internal compiler error) gcc.c-torture/execute/builtins/stpcpy-chk.c compilation, -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error) gcc.c-torture/execute/builtins/stpcpy-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/stpcpy-chk.c compilation, -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gcc.c-torture/execute/builtins/stpcpy-chk.c compilation, -O3 -g (internal compiler error) gcc.c-torture/execute/builtins/strcat-chk.c compilation, -O2 (internal compiler error) gcc.c-torture/execute/builtins/strcat-chk.c compilation, -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error) gcc.c-torture/execute/builtins/strcat-chk.c compilation, -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) gcc.c-torture/execute/builtins/strcat-chk.c
Re: [PATCH] improve out-of-bounds pointer warning (PR 88771)
On Thu, Feb 7, 2019 at 2:14 AM Martin Sebor wrote: > > The attached patch doesn't avoid the false positive but only improves > the warning to make it more readable (as suggested in the PR by Richard > for GCC 9). With the patch, for a call like: > >memcpy (d, s, -1); > > where d and s are pointers with unknown provenances the patch has GCC > in LP32 issue > >warning: ‘memcpy’ specified bound 4294967295 exceeds maximum object > size 2147483647 [-Wstringop-overflow=] > > instead of the somewhat mystifying > >‘memcpy’ pointer overflow between offset 0 and size [4294967295, > 2147483647] [-Warray-bounds] > > There are a few places the warning is issued from and it would make > sense to consolidate them into the same utility function. Since that > will require some not entirely trivial changes to the warning code > I defer it until GCC 10 as well, along with a fix for the false > positive. > > Tested on x86_64-linux. OK. > Martin
Re: [C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE
On Feb 7, 2019, Jason Merrill wrote: > In protected_accessible_p and shared_member_p, if we're left with a > USING_DECL after strip_using_decl, we can't give a meaningful answer, > and should probably abort; we shouldn't get here with a dependent > expression. In g++.dg/lookup/using39.C, shared_member_p is called by cp_parser_primary_expression -> finish_id_expression -> finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using A::f' in B. That was the only regression in the patch I posted earlier today. I'm going back to the previous version of shared_member_p. -- Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain EngineerFree Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
[PATCH] Fix PR89223
The followng fixes the data-ref code not properly checking whether it
can handle some constants.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
Richard.
2019-02-08 Richard Biener
PR middle-end/89223
* tree-data-ref.c (initialize_matrix_A): Fail if constant
doesn't fit in HWI.
(analyze_subscript_affine_affine): Handle failure from
initialize_matrix_A.
* gcc.dg/torture/pr89223.c: New testcase.
Index: gcc/tree-data-ref.c
===
--- gcc/tree-data-ref.c (revision 268609)
+++ gcc/tree-data-ref.c (working copy)
@@ -3191,6 +3191,8 @@ initialize_matrix_A (lambda_matrix A, tr
switch (TREE_CODE (chrec))
{
case POLYNOMIAL_CHREC:
+ if (!cst_and_fits_in_hwi (CHREC_RIGHT (chrec)))
+ return chrec_dont_know;
A[index][0] = mult * int_cst_value (CHREC_RIGHT (chrec));
return initialize_matrix_A (A, CHREC_LEFT (chrec), index + 1, mult);
@@ -3574,7 +3576,7 @@ analyze_subscript_affine_affine (tree ch
tree *last_conflicts)
{
unsigned nb_vars_a, nb_vars_b, dim;
- HOST_WIDE_INT init_a, init_b, gamma, gcd_alpha_beta;
+ HOST_WIDE_INT gamma, gcd_alpha_beta;
lambda_matrix A, U, S;
struct obstack scratch_obstack;
@@ -3611,9 +3613,20 @@ analyze_subscript_affine_affine (tree ch
A = lambda_matrix_new (dim, 1, _obstack);
S = lambda_matrix_new (dim, 1, _obstack);
- init_a = int_cst_value (initialize_matrix_A (A, chrec_a, 0, 1));
- init_b = int_cst_value (initialize_matrix_A (A, chrec_b, nb_vars_a, -1));
- gamma = init_b - init_a;
+ tree init_a = initialize_matrix_A (A, chrec_a, 0, 1);
+ tree init_b = initialize_matrix_A (A, chrec_b, nb_vars_a, -1);
+ if (init_a == chrec_dont_know
+ || init_b == chrec_dont_know)
+{
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, "affine-affine test failed: "
+"representation issue.\n");
+ *overlaps_a = conflict_fn_not_known ();
+ *overlaps_b = conflict_fn_not_known ();
+ *last_conflicts = chrec_dont_know;
+ goto end_analyze_subs_aa;
+}
+ gamma = int_cst_value (init_b) - int_cst_value (init_a);
/* Don't do all the hard work of solving the Diophantine equation
when we already know the solution: for example,
Index: gcc/testsuite/gcc.dg/torture/pr89223.c
===
--- gcc/testsuite/gcc.dg/torture/pr89223.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr89223.c (working copy)
@@ -0,0 +1,10 @@
+/* { dg-do compile { target int128 } } */
+
+int a[5];
+unsigned __int128 b;
+void c()
+{
+ b = 4;
+ for (;; b--)
+a[b] = ({ a[b + b]; });
+}
