[PATCH] Fix UBSAN ICE with C++ attribute types (PR c++/88215)
Hi!
On the following testcase we ICE, because op1 doesn't have normal
int type, but a distinct attribute type, while their main variants
are different, they are still treated as compatible types and the C++ FE
doesn't actually try to ensure both arguments have the same type.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-11-28 Jakub Jelinek
PR c++/88215
* c-ubsan.c: Include langhooks.h.
(ubsan_instrument_division): Change gcc_assert that main variants
of op0 and op1 types are equal to gcc_checking_assert that the
main variants are compatible types.
* c-c++-common/ubsan/pr88215.c: New test.
--- gcc/c-family/c-ubsan.c.jj 2018-11-14 17:44:02.159912988 +0100
+++ gcc/c-family/c-ubsan.c 2018-11-27 12:30:15.211687948 +0100
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
#include "stringpool.h"
#include "attribs.h"
#include "asan.h"
+#include "langhooks.h"
/* Instrument division by zero and INT_MIN / -1. If not instrumenting,
return NULL_TREE. */
@@ -44,8 +45,9 @@ ubsan_instrument_division (location_t lo
/* At this point both operands should have the same type,
because they are already converted to RESULT_TYPE.
Use TYPE_MAIN_VARIANT since typedefs can confuse us. */
- gcc_assert (TYPE_MAIN_VARIANT (TREE_TYPE (op0))
- == TYPE_MAIN_VARIANT (TREE_TYPE (op1)));
+ tree top0 = TYPE_MAIN_VARIANT (type);
+ tree top1 = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
+ gcc_checking_assert (lang_hooks.types_compatible_p (top0, top1));
op0 = unshare_expr (op0);
op1 = unshare_expr (op1);
--- gcc/testsuite/c-c++-common/ubsan/pr88215.c.jj 2018-11-27
12:33:59.102998653 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr88215.c 2018-11-27 12:33:35.694384374
+0100
@@ -0,0 +1,11 @@
+/* PR c++/88215 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=integer-divide-by-zero" } */
+
+int
+foo (void)
+{
+ int a = 2, __attribute__ ((__unused__)) b = 1;
+ int f = a / b;
+ return f;
+}
Jakub
[PATCH] Use {,v}blendvp{s,d} for [SD]Fmode sse_movcc (PR target/88189)
Hi!
This patch implments Marc's idea of using {,v}blenvp{s,d} for scalar
[SD]Fmode ix86_expand_sse_movcc for -msse4.1 and above.
Without this patch we emit sequences like
andpd %xmm2, %xmm0
andnpd %xmm1, %xmm2
orpd%xmm2, %xmm0
or
andps %xmm2, %xmm0
andnps %xmm1, %xmm2
orps%xmm2, %xmm0
and this replaces it with
blendvpd%xmm0, %xmm2, %xmm1
movapd %xmm1, %xmm0
or
blendvps%xmm0, %xmm2, %xmm1
movaps %xmm1, %xmm0
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-11-28 Jakub Jelinek
PR target/88189
* config/i386/i386.c (ix86_expand_sse_movcc): Handle DFmode and
SFmode using sse4_1_blendvs[sd] with TARGET_SSE4_1. Formatting fixes.
* config/i386/sse.md (sse4_1_blendv): New pattern.
* gcc.target/i386/sse4_1-pr88189-1.c: New test.
* gcc.target/i386/sse4_1-pr88189-2.c: New test.
* gcc.target/i386/avx-pr88189-1.c: New test.
* gcc.target/i386/avx-pr88189-2.c: New test.
--- gcc/config/i386/i386.c.jj 2018-11-26 22:25:50.716253308 +0100
+++ gcc/config/i386/i386.c 2018-11-27 11:18:23.135715272 +0100
@@ -23585,15 +23585,13 @@ ix86_expand_sse_movcc (rtx dest, rtx cmp
{
emit_insn (gen_rtx_SET (dest, cmp));
}
- else if (op_false == CONST0_RTX (mode)
- && !maskcmp)
+ else if (op_false == CONST0_RTX (mode) && !maskcmp)
{
op_true = force_reg (mode, op_true);
x = gen_rtx_AND (mode, cmp, op_true);
emit_insn (gen_rtx_SET (dest, x));
}
- else if (op_true == CONST0_RTX (mode)
- && !maskcmp)
+ else if (op_true == CONST0_RTX (mode) && !maskcmp)
{
op_false = force_reg (mode, op_false);
x = gen_rtx_NOT (mode, cmp);
@@ -23601,14 +23599,13 @@ ix86_expand_sse_movcc (rtx dest, rtx cmp
emit_insn (gen_rtx_SET (dest, x));
}
else if (INTEGRAL_MODE_P (mode) && op_true == CONSTM1_RTX (mode)
- && !maskcmp)
+ && !maskcmp)
{
op_false = force_reg (mode, op_false);
x = gen_rtx_IOR (mode, cmp, op_false);
emit_insn (gen_rtx_SET (dest, x));
}
- else if (TARGET_XOP
- && !maskcmp)
+ else if (TARGET_XOP && !maskcmp)
{
op_true = force_reg (mode, op_true);
@@ -23639,6 +23636,20 @@ ix86_expand_sse_movcc (rtx dest, rtx cmp
if (TARGET_SSE4_1)
gen = gen_sse4_1_blendvpd;
break;
+ case E_SFmode:
+ if (TARGET_SSE4_1)
+ {
+ gen = gen_sse4_1_blendvss;
+ op_true = force_reg (mode, op_true);
+ }
+ break;
+ case E_DFmode:
+ if (TARGET_SSE4_1)
+ {
+ gen = gen_sse4_1_blendvsd;
+ op_true = force_reg (mode, op_true);
+ }
+ break;
case E_V16QImode:
case E_V8HImode:
case E_V4SImode:
--- gcc/config/i386/sse.md.jj 2018-11-21 17:39:51.0 +0100
+++ gcc/config/i386/sse.md 2018-11-27 10:48:38.500120925 +0100
@@ -15641,6 +15641,46 @@ (define_insn "_blendv")])
+;; Also define scalar versions. These are used for conditional move.
+;; Using subregs into vector modes causes register allocation lossage.
+;; These patterns do not allow memory operands because the native
+;; instructions read the full 128-bits.
+
+(define_insn "sse4_1_blendv"
+ [(set (match_operand:MODEF 0 "register_operand" "=Yr,*x,x")
+ (unspec:MODEF
+ [(match_operand:MODEF 1 "register_operand" "0,0,x")
+ (match_operand:MODEF 2 "register_operand" "Yr,*x,x")
+ (match_operand:MODEF 3 "register_operand" "Yz,Yz,x")]
+ UNSPEC_BLENDV))]
+ "TARGET_SSE4_1"
+{
+ if (get_attr_mode (insn) == MODE_V4SF)
+return (which_alternative == 2
+ ? "vblendvps\t{%3, %2, %1, %0|%0, %1, %2, %3}"
+ : "blendvps\t{%3, %2, %0|%0, %2, %3}");
+ else
+return (which_alternative == 2
+ ? "vblendv\t{%3, %2, %1, %0|%0, %1, %2, %3}"
+ : "blendv\t{%3, %2, %0|%0, %2, %3}");
+}
+ [(set_attr "isa" "noavx,noavx,avx")
+ (set_attr "type" "ssemov")
+ (set_attr "length_immediate" "1")
+ (set_attr "prefix_data16" "1,1,*")
+ (set_attr "prefix_extra" "1")
+ (set_attr "prefix" "orig,orig,vex")
+ (set_attr "btver2_decode" "vector,vector,vector")
+ (set (attr "mode")
+ (cond [(match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
+(const_string "V4SF")
+ (match_test "TARGET_AVX")
+(const_string "")
+ (match_test "optimize_function_for_size_p (cfun)")
+(const_string "V4SF")
+ ]
+ (const_string "")))])
+
(define_insn "_dp"
[(set (match_operand:VF_128_256 0 "register_operand" "=Yr,*x,x")
(unspec:VF_128_256
--- gcc/testsuite/gcc.target/i386/sse4_1-pr88189-1.c.jj 2018-11-27
11:00:34.746322991 +0100
+++ gcc/testsuite/gcc.target/i386/sse4_1-pr88189-1.c2018-11-27
11:11:36.116423601 +010
Re: Fix hashtable memory leak
On 11/27/18 11:00 PM, Jonathan Wakely wrote:
On 27/11/18 22:31 +0100, François Dumont wrote:
I eventually called the new method _M_assign_elements.
Perfect.
And yes, tracker_allocator was enough.
Great.
Committed on trunk for the moment.
Great, thanks.
Please note that GCC 7.4 RC1 is scheduled for this week:
https://gcc.gnu.org/ml/gcc/2018-11/msg00118.html
Will you be able to backport the simpler patch (without the
refactoring to remove code duplication) to the branch before then?
If not I can take care of it.
I just backport to gcc-7/8-branch the same patch.
2018-11-28 François Dumont
PR libstdc++/88199
* include/bits/hashtable.h
(_Hashtable<>::_M_move_assign(_Hashtable&&, false_type)): Deallocate
former buckets after assignment.
* testsuite/23_containers/unordered_set/allocator/move_assign.cc
(test03): New.
Bests, François
Index: libstdc++-v3/include/bits/hashtable.h
===
--- libstdc++-v3/include/bits/hashtable.h (révision 266538)
+++ libstdc++-v3/include/bits/hashtable.h (copie de travail)
@@ -1222,6 +1222,9 @@
_M_assign(__ht,
[&__roan](__node_type* __n)
{ return __roan(std::move_if_noexcept(__n->_M_v())); });
+
+ if (__former_buckets)
+ _M_deallocate_buckets(__former_buckets, __former_bucket_count);
__ht.clear();
}
__catch(...)
Index: libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
===
--- libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc (révision 266538)
+++ libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc (copie de travail)
@@ -18,6 +18,7 @@
// { dg-do run { target c++11 } }
#include
+
#include
#include
#include
@@ -24,10 +25,15 @@
using __gnu_test::propagating_allocator;
using __gnu_test::counter_type;
+using __gnu_test::tracker_allocator;
+using __gnu_test::tracker_allocator_counter;
void test01()
{
- typedef propagating_allocator alloc_type;
+ tracker_allocator_counter::reset();
+ {
+typedef propagating_allocator> alloc_type;
typedef __gnu_test::counter_type_hasher hash;
typedef std::unordered_set,
@@ -50,9 +56,19 @@
VERIFY( counter_type::destructor_count == 2 );
}
+ // Check there's nothing left allocated or constructed.
+ VERIFY( tracker_allocator_counter::get_construct_count()
+ == tracker_allocator_counter::get_destruct_count() );
+ VERIFY( tracker_allocator_counter::get_allocation_count()
+ == tracker_allocator_counter::get_deallocation_count() );
+}
+
void test02()
{
- typedef propagating_allocator alloc_type;
+ tracker_allocator_counter::reset();
+ {
+typedef propagating_allocator> alloc_type;
typedef __gnu_test::counter_type_hasher hash;
typedef std::unordered_set,
@@ -80,9 +96,55 @@
VERIFY( it == v2.begin() );
}
+ // Check there's nothing left allocated or constructed.
+ VERIFY( tracker_allocator_counter::get_construct_count()
+ == tracker_allocator_counter::get_destruct_count() );
+ VERIFY( tracker_allocator_counter::get_allocation_count()
+ == tracker_allocator_counter::get_deallocation_count() );
+}
+
+void test03()
+{
+ tracker_allocator_counter::reset();
+ {
+typedef propagating_allocator> alloc_type;
+typedef __gnu_test::counter_type_hasher hash;
+typedef std::unordered_set,
+ alloc_type> test_type;
+
+test_type v1(alloc_type(1));
+v1.emplace(0);
+
+test_type v2(alloc_type(2));
+int i = 0;
+v2.emplace(i++);
+for (; v2.bucket_count() == v1.bucket_count(); ++i)
+ v2.emplace(i);
+
+counter_type::reset();
+
+v2 = std::move(v1);
+
+VERIFY( 1 == v1.get_allocator().get_personality() );
+VERIFY( 2 == v2.get_allocator().get_personality() );
+
+VERIFY( counter_type::move_count == 1 );
+VERIFY( counter_type::destructor_count == i + 1 );
+ }
+
+ // Check there's nothing left allocated or constructed.
+ VERIFY( tracker_allocator_counter::get_construct_count()
+ == tracker_allocator_counter::get_destruct_count() );
+ VERIFY( tracker_allocator_counter::get_allocation_count()
+ == tracker_allocator_counter::get_deallocation_count() );
+}
+
int main()
{
test01();
test02();
+ test03();
return 0;
}
Re: [PATCH/coding style] clarify pointers and operators
On 11/26/18 10:59 AM, Martin Sebor wrote: Martin suggested we update the Coding Conventions to describe the expected style for function declarations with a pointer return types, and for overloaded operators. Below is the patch. As an aside, regarding the space convention in casts: a crude grep search yields about 10,000 instances of the "(type)x" kinds of casts in GCC sources and 40,000 of the preferred "(type) x" style with the space. That's a consistency of only 80%. Is it worth documenting a preference for a convention that's so inconsistently followed? Martin Index: htdocs/codingconventions.html === RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v retrieving revision 1.85 diff -u -r1.85 codingconventions.html --- htdocs/codingconventions.html 30 Sep 2018 14:38:46 - 1.85 +++ htdocs/codingconventions.html 26 Nov 2018 17:59:21 - @@ -803,12 +803,17 @@ - x cast - (foo) x - (foo)x + (type) x + (type)x - pointer dereference - *x - * x + pointer cast + (type *) x + (type*)x + + + pointer return type + type *f (void) + type* f (void) @@ -992,6 +997,21 @@ Rationale and Discussion + +Note: in declarations of operator functions or in invocations of +such functions that involve the keyword operator This sentence would be more readable with a comma inserted here (after operator), I think. +the full name of the operator should be considered as including +the keyword with no spaces in between the keyowrd and the operator s/keyowrd/keyword/ +token. Thus, the expected format of a declaration of an operator +is + T &operator== (const T & const T &); + +and not for example s/ for example// + T &operator == (const T & const T &); + +(with the space between operator and ==). + + Default Arguments -Sandra
Re: [PATCH] [PR86397] set p_t_decl while canonicalizing eh specs for mangling
On Nov 27, 2018, Jason Merrill wrote:
> On 11/22/18 6:40 PM, Alexandre Oliva wrote:
>> Mangling visits the base template function type, prior to template
>> resolution, and on such types, exception specifications may contain
>> unresolved noexcept expressions. nothrow_spec_p is called on them
>> even when exception specifications are not part of function types, and
>> it rejects unresolved noexcept expressions if processing_template_decl
>> is not set.
> The problem here is that the noexcept expression is unresolved even
> though it isn't dependent
Yeah, but that seems to be on purpose, according to these comments, that
precede the hunk below.
/* This isn't part of the signature, so don't bother trying to evaluate
it until instantiation. */
Taking out the 'flag_noexcept_type && ' subexpr fixes the problem, but
defeats the intended deferral of unnecessary computation:
diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index 3449b59b3cc0..dbd233c94c3a 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1193,7 +1193,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
it until instantiation. */
if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
&& (!processing_template_decl
- || (flag_noexcept_type && !value_dependent_expression_p (expr
+ || !value_dependent_expression_p (expr)))
{
expr = perform_implicit_conversion_flags (boolean_type_node, expr,
complain,
In order to retain that deferral, we could change the mangling logic to
also refrain from canonicalizing the EH spec when it's not part of the
type:
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 64415894bc57..4c8086c9f9bd 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -418,9 +418,12 @@ canonicalize_for_substitution (tree node)
|| TREE_CODE (node) == METHOD_TYPE)
{
node = build_ref_qualified_type (node, type_memfn_rqual (orig));
- tree r = canonical_eh_spec (TYPE_RAISES_EXCEPTIONS (orig));
+ tree r = TYPE_RAISES_EXCEPTIONS (orig);
if (flag_noexcept_type)
- node = build_exception_variant (node, r);
+ {
+ r = canonical_eh_spec (r);
+ node = build_exception_variant (node, r);
+ }
else
/* Set the warning flag if appropriate. */
write_exception_spec (r);
This would bypass the nothrow_spec_p call in canonical_eh_spec at
C++1[14], but it might produce unintended -Wnoexcept-type warnings when
the noexcept expression would resolve to false. The canonical_eh_spec
call wouldn't have avoided it anyway.
Which one?
--
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] be more permissive about function alignments (PR 88208)
The tests for the new __builtin_has_attribute function have been
failing on a number of targets because of a couple of assumptions
that only hold on some.
First, they expect that it's safe to apply attribute aligned with
a smaller alignment than the target provides when GCC rejects such
arguments. The tests pass on i86 and elsewhere but fail on
strictly aligned targets like aarch64 or sparc. After some testing
and thinking I don't think this is helpful -- I believe it's better
to instead silently accept attributes that ask for a less restrictive
alignment than the function ultimately ends up with (see * below).
This is what testing shows Clang does on those targets. The attached
patch implements this change.
Second, the tests assume that the priority forms of the constructor
and destructor attributes are universally supported. That's also
not the case, even though the manual doesn't mention that. To
avoid these failures the attached patch moves the priority forms
of the attribute constructor and destructor tests into its own
file that's compiled only for init_priority targets.
Finally, I noticed that attribute aligned accepts zero as
an argument even though it's not a power of two as the manual
documents as a precondition (zero is treated the same as
the attribute without an argument). A zero argument is likely
to be a mistake, especially when the zero comes from macro
expansion, that users might want to know about. Clang rejects
a zero with an error but I think a warning is more in line with
established GCC practice, so the patch also implements that.
Besides x86_64-linux, I tested this change with cross-compilers
for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11.
I added tests for the changed aligned attribute for those targets
To make the gcc.dg/builtin-has-attribute.c test pass with
the cross-compilers I changed from a runtime test into a compile
only one.
Martin
PS I'm not happy about duplicating the same test across all those
targets. It would be much nicer to have a single test somewhere
in dg.exp #include a target-specific header with macros describing
the target-specific parameters.
[*] See the following discussion for some background:
https://gcc.gnu.org/ml/gcc/2018-11/msg00127.html
PR testsuite/88208 - new test case c-c++-common/builtin-has-attribute-3.c in r266335 has multiple excess errors
gcc/ChangeLog:
PR testsuite/88208
* doc/extend.texi (attribute constructor): Clarify.
gcc/c/ChangeLog:
PR testsuite/88208
* c-decl.c (declspec_add_alignas): Adjust call to check_user_alignment.
gcc/c-family/ChangeLog:
PR testsuite/88208
* c-attribs.c (common_handle_aligned_attribute): Silently avoid setting
alignments to values less than the target requires.
(has_attribute): For attribute aligned consider both the attribute
and the alignment bits.
* c-common.c (c_init_attributes): Optionally issue a warning for
zero alignment.
gcc/testsuite/ChangeLog:
PR testsuite/88208
* gcc.dg/attr-aligned-2.c: New test.
* gcc.dg/builtin-has-attribute.c: Adjust.
* c-c++-common/builtin-has-attribute-2.c: Same.
* c-c++-common/builtin-has-attribute-3.c: Same.
* c-c++-common/builtin-has-attribute-4.c: Same.
* c-c++-common/builtin-has-attribute-5.c: New test.
* gcc.target/aarch64/attr-aligned.c: Same.
* gcc.target/i386/attr-aligned.c: Same.
* gcc.target/powerpc/attr-aligned.c: Same.
* gcc.target/sparc/attr-aligned.c: Same.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 266521)
+++ gcc/doc/extend.texi (working copy)
@@ -2552,8 +2552,9 @@ called. Functions with these attributes are usefu
initializing data that is used implicitly during the execution of
the program.
-You may provide an optional integer priority to control the order in
-which constructor and destructor functions are run. A constructor
+On some targets the attributes also accept an integer argument to
+specify a priority to control the order in which constructor and
+destructor functions are run. A constructor
with a smaller priority number runs before a constructor with a larger
priority number; the opposite relationship holds for destructors. So,
if you have a constructor that allocates a resource and a destructor
@@ -2566,6 +2567,10 @@ decorated with attribute @code{constructor} are in
In mixed declarations, attribute @code{init_priority} can be used to
impose a specific ordering.
+Using the argument forms of the @code{constructor} and @code{destructor}
+attributes on targets where the feature is not supported is rejected with
+an error.
+
@item copy
@itemx copy (@var{function})
@cindex @code{copy} function attribute
Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c (revision 266521)
+++ gcc/c/c-decl.c (working copy)
@@ -11061,12 +11061,15 @@ struct c_declspecs *
declspecs_add_alignas (location_t loc,
struct c_declspecs *specs, tree al
Re: [PATCH 6/6] [RS6000] inline plt call sequences
On Tue, Nov 27, 2018 at 11:17:48AM -0600, Segher Boessenkool wrote: > That all sounds great :-) Has this been tested with older binutils, too? Yes, lots of times. Sometimes by accident when I forgot to install a new binutils. :-) > > +(define_insn "*sibcall_indirect_nonlocal_sysv" > > > + (set (attr "length") > > + (cond [(and (and (match_test "!rs6000_speculate_indirect_jumps") > > +(match_test "which_alternative != 1")) > > + (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | > > CALL_V4_CLEAR_FP_ARGS))")) > > + (const_string "12") > > + (ior (and (match_test "!rs6000_speculate_indirect_jumps") > > +(match_test "which_alternative != 1")) > > + (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | > > CALL_V4_CLEAR_FP_ARGS))")) > > + (const_string "8")] > > + (const_string "4")))]) > > Please use set_attr_alternative for these. Are you sure about that? The expression blows out to something like (completely untested): (set_attr_alternative "length" [(cond [(and (match_test "!rs6000_speculate_indirect_jumps") (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))")) (const_string "12") (ior (match_test "!rs6000_speculate_indirect_jumps") (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))")) (const_string "8")] (const_string "4")) (cond [(match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))") (const_string "8")] (const_string "4")) (cond [(and (match_test "!rs6000_speculate_indirect_jumps") (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))")) (const_string "12") (ior (match_test "!rs6000_speculate_indirect_jumps") (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))")) (const_string "8")] (const_string "4"))]) -- Alan Modra Australia Development Lab, IBM
Re: [PATCH 5/6] [RS6000] Use standard call patterns for __tls_get_addr calls
On Tue, Nov 27, 2018 at 10:29:29AM -0600, Segher Boessenkool wrote:
> Hi!
Thanks for the review!
> > +(define_insn "*tls_gdld_aix"
> > + [(match_parallel 3 ""
>
> A match_parallel without predicate... Does this work?!
Yes. In fact, rs6000/predicates.md any_parallel_operand is useless
except as documentation. The only thing it checks is that its operand
is a parallel, but that has already been checked.
> Does this not
> accidentally pick up the wrong things?
No. The purpose of the predicate is to match anything beyond the
vector of expressions. So tls_gdld_aix* matches insns that look like:
(set (match_operand:P 0 "gpc_reg_operand" "=b")
(call (mem:SI (match_operand:P 1))
(match_operand:P 2 "unspec_tls")))
(match_dup 2)
...
This is sufficiently different from other calls, by virtue of the
"(match_dup 2)".
Incidentally, I think tls_gdld_aix and tls_gdld_sysv could be merged
at the expense of complicating the length attribute expression.
> Do you think we should to deprecate -mtls-markers in GCC 9?
Support for the TLS marker relocs was added to binutils in 2009 (git
commit 727fc41e077), so yes, the option is not likely to be useful
nowadays.
> Please test with -mtls-markers, too, if you can, and test on AIX.
Presumably you mean -mno-tls-markers. -mtls-markers is the default.
> Looks fine. Thank you for the cleanup! Okay for trunk, but please do the
> extra testing.
Huh, local testing of -mno-tls-markers showed a lack of a
TARGET_TLS_MARKERS check in rs6000_call_template_1. Likely this would
blow up on AIX. I'll test with the following delta.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 56ca117a0a0..5f4fcee3b33 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -8622,7 +8622,7 @@ edit_tls_call_insn (rtx arg)
}
/* Passes the tls arg value for global dynamic and local dynamic
- emit_library_call_value in rs6000_legitimize_Tls_address to
+ emit_library_call_value in rs6000_legitimize_tls_address to
rs6000_call_aix and rs6000_call_sysv. This is used to emit the
marker relocs put on __tls_get_addr calls. */
static rtx global_tlsarg;
@@ -21429,7 +21429,7 @@ rs6000_call_template_1 (rtx *operands, unsigned int
funop, bool sibcall)
char arg[12];
arg[0] = 0;
- if (GET_CODE (operands[funop + 1]) == UNSPEC)
+ if (TARGET_TLS_MARKERS && GET_CODE (operands[funop + 1]) == UNSPEC)
{
if (XINT (operands[funop + 1], 1) == UNSPEC_TLSGD)
sprintf (arg, "(%%%u@tlsgd)", funop + 1);
--
Alan Modra
Australia Development Lab, IBM
Go patch committed: Tweaks for importing inline function bodies
This patch to the Go frontend adds some tweaks for importing inline
function bodies.
We track whether we've seen an error when importing a function; we
will use error tracking to avoid knock-on errors.
We stop importing identifiers at a ')'.
We provide a way to adjust the indentation level while importing.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266534)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-267d91b41571329e71a88f56df46444b305482da
+b013405f2c66596c47cb9be493c798db1087c0f0
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/import.cc
===
--- gcc/go/gofrontend/import.cc (revision 266530)
+++ gcc/go/gofrontend/import.cc (working copy)
@@ -1225,7 +1225,7 @@ Import::read_identifier()
while (true)
{
c = stream->peek_char();
- if (c == -1 || c == ' ' || c == '\n' || c == ';')
+ if (c == -1 || c == ' ' || c == '\n' || c == ';' || c == ')')
break;
ret += c;
stream->advance(1);
@@ -1450,7 +1450,7 @@ Import_function_body::read_identifier()
for (size_t i = start; i < this->body_.length(); i++)
{
int c = static_cast(this->body_[i]);
- if (c == ' ' || c == '\n' || c == ';')
+ if (c == ' ' || c == '\n' || c == ';' || c == ')')
{
this->off_ = i;
return this->body_.substr(start, i - start);
Index: gcc/go/gofrontend/import.h
===
--- gcc/go/gofrontend/import.h (revision 266530)
+++ gcc/go/gofrontend/import.h (working copy)
@@ -554,7 +554,7 @@ class Import_function_body : public Impo
const std::string& body, size_t off, Block* block,
int indent)
: gogo_(gogo), imp_(imp), named_object_(named_object), body_(body),
- off_(off), block_(block), indent_(indent)
+ off_(off), block_(block), indent_(indent), saw_error_(false)
{ }
// The IR.
@@ -597,6 +597,16 @@ class Import_function_body : public Impo
indent() const
{ return this->indent_; }
+ // Increment the indentation level.
+ void
+ increment_indent()
+ { ++this->indent_; }
+
+ // Decrement the indentation level.
+ void
+ decrement_indent()
+ { --this->indent_; }
+
// The name of the function we are parsing.
const std::string&
name() const;
@@ -652,6 +662,16 @@ class Import_function_body : public Impo
ifb()
{ return this; }
+ // Return whether we have seen an error.
+ bool
+ saw_error() const
+ { return this->saw_error_; }
+
+ // Record that we have seen an error.
+ void
+ set_saw_error()
+ { this->saw_error_ = true; }
+
private:
// The IR.
Gogo* gogo_;
Re: [PATCH] [PR85569] skip constexpr target_expr constructor dummy type conversion
On 11/22/18 6:39 PM, Alexandre Oliva wrote:
The testcase is the work-around testcase for the PR; even that had
started failing. The problem was that, when unqualifying the type of
a TARGET_EXPR, we'd create a variant of the type, then request the
conversion of the TARGET_EXPR_INITIAL to that variant type. Though
the types are different pointer-wise, they're the same_type_p, so the
resulting modified expr compares cp_tree_equal to the original, which
maybe_constant_value flags as an error. There's no reason to
construct an alternate TARGET_EXPR or CONSTRUCTOR just because of an
equivalent type, except for another spot that expected pointer
equality that would no longer be satisfied. Without relaxing the
assert in constexpr_call_hasher::equal, g++.robertl/eb73.C would
trigger an assertion failure.
Regstrapped on i686- and x86_64-linux-gnu. Ok to install?
for gcc/cp/ChangeLog
PR c++/85569
* constexpr.c (adjust_temp_type): Test for type equality with
same_type_p.
for gcc/testsuite
PR c++/85569
* g++.dg/cpp1z/pr85569.C: New.
---
gcc/cp/constexpr.c |4 +
gcc/testsuite/g++.dg/cpp1z/pr85569.C | 93 ++
2 files changed, 95 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr85569.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 92fd2b2d9d59..bb5d1301b332 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1060,7 +1060,7 @@ constexpr_call_hasher::equal (constexpr_call *lhs,
constexpr_call *rhs)
{
tree lhs_arg = TREE_VALUE (lhs_bindings);
tree rhs_arg = TREE_VALUE (rhs_bindings);
- gcc_assert (TREE_TYPE (lhs_arg) == TREE_TYPE (rhs_arg));
+ gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
if (!cp_tree_equal (lhs_arg, rhs_arg))
return false;
lhs_bindings = TREE_CHAIN (lhs_bindings);
@@ -1276,7 +1276,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx,
tree t, tree fun,
static tree
adjust_temp_type (tree type, tree temp)
{
- if (TREE_TYPE (temp) == type)
+ if (TREE_TYPE (temp) == type || same_type_p (TREE_TYPE (temp), type))
return temp;
/* Avoid wrapping an aggregate value in a NOP_EXPR. */
Hmm, I'm a bit uneasy about this change, but it does make sense to
follow cp_tree_equal.
Let's replace the == comparison rather than supplement it. OK with that
change.
Jason
[PATCH] Clean up temporary files created by std::filesystem testsuite
* testsuite/27_io/filesystem/operations/canonical.cc: Remove directory created by test. * testsuite/27_io/filesystem/operations/symlink_status.cc: Remove symlink created by test. Tested x86_64-linux, committed to trunk. commit c981ab59c04fe12ffee89bbfe466a40d66e40268 Author: Jonathan Wakely Date: Tue Nov 27 23:27:00 2018 + Clean up temporary files created by std::filesystem testsuite * testsuite/27_io/filesystem/operations/canonical.cc: Remove directory created by test. * testsuite/27_io/filesystem/operations/symlink_status.cc: Remove symlink created by test. diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc index f7b6649adfe..6fed419dcc9 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc @@ -36,6 +36,7 @@ test01() VERIFY( ec ); create_directory(p); + __gnu_test::scoped_file l(p, __gnu_test::scoped_file::adopt_file); auto p2 = canonical( p, ec ); compare_paths( p2, fs::current_path()/p ); VERIFY( !ec ); diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/symlink_status.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/symlink_status.cc index 919f826b957..318a0866e56 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/symlink_status.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/symlink_status.cc @@ -42,6 +42,7 @@ test01() fs::path link = __gnu_test::nonexistent_path(); create_directory_symlink(dot, link); + __gnu_test::scoped_file l(link, __gnu_test::scoped_file::adopt_file); st1 = fs::symlink_status(link); VERIFY( st1.type() == fs::file_type::symlink );
Go patch committed: Record final type for numeric expressions
This patch changes the Go frontend to record the final type for
numeric expressions in export data. Inlinable function bodies are
generated after the determine_types pass, so we know the type for all
constants. Rather than try to determine it again when inlining,
record the type in the export data, using a $convert expression.
Reduce the number of explicit $convert expressions by recording a type
context with the expected type in cases where that type is known.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266532)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-5d0c788cd6099c2bb28bb0ff6a04d94006fbfca8
+267d91b41571329e71a88f56df46444b305482da
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/export.h
===
--- gcc/go/gofrontend/export.h (revision 266523)
+++ gcc/go/gofrontend/export.h (working copy)
@@ -303,7 +303,7 @@ class Export_function_body : public Stri
{
public:
Export_function_body(Export* exp, int indent)
-: exp_(exp), indent_(indent)
+: exp_(exp), type_context_(NULL), indent_(indent)
{ }
// Write a character to the body.
@@ -326,6 +326,16 @@ class Export_function_body : public Stri
write_type(const Type* type)
{ this->exp_->write_type_to(type, this); }
+ // Return the current type context.
+ Type*
+ type_context() const
+ { return this->type_context_; }
+
+ // Set the current type context.
+ void
+ set_type_context(Type* type)
+ { this->type_context_ = type; }
+
// Append as many spaces as the current indentation level.
void
indent()
@@ -354,6 +364,8 @@ class Export_function_body : public Stri
Export* exp_;
// The body we are building.
std::string body_;
+ // Current type context. Used to avoid duplicate type conversions.
+ Type* type_context_;
// Current indentation level: the number of spaces before each statement.
int indent_;
};
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266529)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -2142,11 +2142,25 @@ Integer_expression::export_integer(Strin
void
Integer_expression::do_export(Export_function_body* efb) const
{
+ bool added_type = false;
+ if (this->type_ != NULL
+ && !this->type_->is_abstract()
+ && this->type_ != efb->type_context())
+{
+ efb->write_c_string("$convert(");
+ efb->write_type(this->type_);
+ efb->write_c_string(", ");
+ added_type = true;
+}
+
Integer_expression::export_integer(efb, this->val_);
if (this->is_character_constant_)
efb->write_c_string("'");
// A trailing space lets us reliably identify the end of the number.
efb->write_c_string(" ");
+
+ if (added_type)
+efb->write_c_string(")");
}
// Import an integer, floating point, or complex value. This handles
@@ -2509,9 +2523,23 @@ Float_expression::export_float(String_du
void
Float_expression::do_export(Export_function_body* efb) const
{
+ bool added_type = false;
+ if (this->type_ != NULL
+ && !this->type_->is_abstract()
+ && this->type_ != efb->type_context())
+{
+ efb->write_c_string("$convert(");
+ efb->write_type(this->type_);
+ efb->write_c_string(", ");
+ added_type = true;
+}
+
Float_expression::export_float(efb, this->val_);
// A trailing space lets us reliably identify the end of the number.
efb->write_c_string(" ");
+
+ if (added_type)
+efb->write_c_string(")");
}
// Dump a floating point number to the dump file.
@@ -2699,9 +2727,23 @@ Complex_expression::export_complex(Strin
void
Complex_expression::do_export(Export_function_body* efb) const
{
+ bool added_type = false;
+ if (this->type_ != NULL
+ && !this->type_->is_abstract()
+ && this->type_ != efb->type_context())
+{
+ efb->write_c_string("$convert(");
+ efb->write_type(this->type_);
+ efb->write_c_string(", ");
+ added_type = true;
+}
+
Complex_expression::export_complex(efb, this->val_);
// A trailing space lets us reliably identify the end of the number.
efb->write_c_string(" ");
+
+ if (added_type)
+efb->write_c_string(")");
}
// Dump a complex expression to the dump file.
@@ -3620,7 +3662,14 @@ Type_conversion_expression::do_export(Ex
efb->write_c_string("$convert(");
efb->write_type(this->type_);
efb->write_c_string(", ");
+
+ Type* old_context = efb->type_context();
+ efb->set_type_context(this->type_);
+
this->expr_->export_expression(efb);
+
+ efb->set_type_context(old_context);
+
efb->write_c_string(")");
}
Index: gcc/go/gofrontend/gogo.cc
=
[PATCH] PR libstdc++/67843 set shared_ptr lock policy at build-time
This resolves a longstanding issue where the lock policy for shared_ptr reference counting depends on compilation options when the header is included, so that different -march options can cause ABI changes. For example, objects compiled with -march=armv7 will use atomics to synchronize reference counts, and objects compiled with -march=armv5t will use a mutex. That means the shared_ptr control block will have a different layout in different objects, causing ODR violations and undefined behaviour. This was the root cause of PR libstdc++/42734 as well as PR libstdc++/67843. The solution is to decide on the lock policy at build time, when libstdc++ is configured. The configure script checks for the availability of the necessary atomic built-ins for the target and fixes that choice permanently. Different -march flags used to compile user code will not cause changes to the lock policy. This results in an ABI change for certain compilations, but only where there was already an ABI incompatibility between the libstdc++.so library and objects built with an incompatible -march option. In general, this means a more stable ABI that isn't silently altered when -march flags make addition atomic ops available. To force a target to use "atomic" or "mutex" the new configure option --with-libstdcxx-lock-policy can be used. In order to turn ODR violations into linker errors, the uses of shared_ptr in filesystem directory iterators have been replaced with __shared_ptr, and explicit instantiations are declared. This ensures that object files using those types cannot link to libstdc++ libs unless they use the same lock policy. PR libstdc++/67843 * acinclude.m4 (GLIBCXX_ENABLE_LOCK_POLICY): Add new macro that defines _GLIBCXX_HAVE_ATOMIC_LOCK_POLICY. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Use GLIBCXX_ENABLE_LOCK_POLICY. * doc/xml/manual/configure.xml: Document new configure option. * include/bits/fs_dir.h (directory_iterator): Use __shared_ptr instead of shared_ptr. (recursive_directory_iterator): Likewise. (__shared_ptr<_Dir>): Add explicit instantiation declaration. (__shared_ptr): Likewise. * include/bits/shared_ptr_base.h (__allocate_shared, __make_shared): Add default template argument for _Lock_policy template parameter. * include/ext/concurrence.h (__default_lock_policy): Check macro _GLIBCXX_HAVE_ATOMIC_LOCK_POLICY instead of checking if the current target supports the builtins for compare-and-swap. * src/filesystem/std-dir.cc (__shared_ptr<_Dir>): Add explicit instantiation definition. (__shared_ptr): Likewise. (directory_iterator, recursive_directory_iterator): Use __make_shared instead of make_shared. Tested x86_64-linux and aarch64-linux, committed to trunk. I would appreciate testing on ARM, especially Christophe's -march=armv5t set up. commit 5ae8a02becb13b5d9c0cdd72bce5312efd6bc569 Author: Jonathan Wakely Date: Tue Nov 27 12:13:56 2018 + PR libstdc++/67843 set shared_ptr lock policy at build-time This resolves a longstanding issue where the lock policy for shared_ptr reference counting depends on compilation options when the header is included, so that different -march options can cause ABI changes. For example, objects compiled with -march=armv7 will use atomics to synchronize reference counts, and objects compiled with -march=armv5t will use a mutex. That means the shared_ptr control block will have a different layout in different objects, causing ODR violations and undefined behaviour. This was the root cause of PR libstdc++/42734 as well as PR libstdc++/67843. The solution is to decide on the lock policy at build time, when libstdc++ is configured. The configure script checks for the availability of the necessary atomic built-ins for the target and fixes that choice permanently. Different -march flags used to compile user code will not cause changes to the lock policy. This results in an ABI change for certain compilations, but only where there was already an ABI incompatibility between the libstdc++.so library and objects built with an incompatible -march option. In general, this means a more stable ABI that isn't silently altered when -march flags make addition atomic ops available. To force a target to use "atomic" or "mutex" the new configure option --with-libstdcxx-lock-policy can be used. In order to turn ODR violations into linker errors, the uses of shared_ptr in filesystem directory iterators have been replaced with __shared_ptr, and explicit instantiations are declared. This ensures that object files using those types cannot link to libstdc++ libs unless they use the same lock policy. PR libstdc++/67843 * acinclude.m4 (GLIBCXX_E
Re: [PATCH/coding style] clarify pointers and operators
On 11/27/18 11:46 AM, Segher Boessenkool wrote: > On Mon, Nov 26, 2018 at 01:41:07PM -0700, Jeff Law wrote: >> On 11/26/18 10:59 AM, Martin Sebor wrote: >>> As an aside, regarding the space convention in casts: a crude >>> grep search yields about 10,000 instances of the "(type)x" kinds >>> of casts in GCC sources and 40,000 of the preferred "(type) x" >>> style with the space. That's a consistency of only 80%. Is >>> it worth documenting a preference for a convention that's so >>> inconsistently followed? >> Please do. It's a fairly recent change -- I suspect some old code was >> never fixed and some folks (perhaps myself) have that extraneous >> whitespace in their muscle memory and still need to eliminate it. > > Huh? Spaces after casts are required, and make things much more > readable. This isn't recent. > > A lot of old code writes spaces after single-character unary operators, > too, which is ugly and less readable. It's not recent that this is > explicitly documented as wrong, either. Sorry. Got confused with something else. jeff
Re: [PATCH] Fix 87380.
On 11/27/18 2:16 PM, Iain Sandoe wrote: > Hi > > This is [intentionally] broken C++ ABI, that was catering for a tool problem > that existed in a very old Darwin toolchain. > > I checked that the issue is not present after Darwin7 (using default Xcode > tools). Of course, more modern tools are probably required to build trunk > GCC for Darwin7, but somehow I doubt anyone has time to try that… > > anyway, this is long-standing breakage on all open branches. > > NOTE: re comment #18 in the PR, rs6000/darwin7.h is included after the > generic header darwin.h, and thus it is sufficient to cover the case there. > > OK for trunk? > > open branches? > > Iain > > gcc/ > > * gcc/config/darwin.h (TARGET_WEAK_NOT_IN_ARCHIVE_TOC): > Set to 0. Update comment. > * gcc/config/rs6000/darwin7.h (TARGET_WEAK_NOT_IN_ARCHIVE_TOC): New. You know the Darwin situation better than just about anyone. So if you think it's the right thing to do, then go for it. jeff
Re: [PATCH] [PR86397] set p_t_decl while canonicalizing eh specs for mangling
On 11/22/18 6:40 PM, Alexandre Oliva wrote: Mangling visits the base template function type, prior to template resolution, and on such types, exception specifications may contain unresolved noexcept expressions. nothrow_spec_p is called on them even when exception specifications are not part of function types, and it rejects unresolved noexcept expressions if processing_template_decl is not set. The problem here is that the noexcept expression is unresolved even though it isn't dependent; it should have been resolved to 'true'. Ideally with a warning, since using the name of a function as a boolean value is pretty strange. nothrow_spec_p is right to reject this. Jason
Re: [C++ PATCH] Fix wrong-code with generic lambda (PR c++/86943)
On Tue, Nov 27, 2018 at 05:20:56PM -0500, Jason Merrill wrote:
> On 11/23/18 4:15 PM, Jakub Jelinek wrote:
> > Hi!
> >
> > On the following testcase, the call to operator () is marked as
> > CALL_FROM_THUNK_P and therefore genericization ignores all subtrees thereof.
> > Unfortunately, one of the arguments is a move ctor call which is not itself
> > CALL_FROM_THUNK_P and thus we need to genericize its arguments, otherwise
> > we pass address of a temporary which holds a reference value instead of the
> > reference itself.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > Or should CALL_FROM_THUNK_P not be set in this call (it is set in
> > maybe_add_lambda_conv_op and then copied over during tsubst*).
>
> The call with CALL_FROM_THUNK_P set should be inside the thunk whose address
> we get when converting the lambda to a function pointer (on return from
> foo). Its arguments should just be the parms of that thunk, I don't know
> where this temporary is coming from.
In the *.original dump this is:
<::operator() (0B, &TARGET_EXPR >>
(struct S &) &D.2402 ) >;
return;
where CALL_FROM_THUNK_P is set on the call to
foo()operator() and D.2402 is the
PARM_DECL which shouldn't have the &.
In *.gimple it is:
foo()_FUN (struct S & restrict D.2402) // the arg is
DECL_BY_REFERENCE
{
struct S D.2406;
struct S & D.2413;
D.2413 = D.2402;
S::S (&D.2406, &D.2413); // this ctor wants S & arg, so it should be just
D.2402
try
{
try
{
foo()operator() (0B, &D.2406);
}
finally
{
S::~S (&D.2406);
}
}
finally
{
D.2406 = {CLOBBER};
}
return;
}
CALL_FROM_THUNK_P is set on the operator() call in maybe_add_lambda_conv_op
and at that the call is:
readonly
arg:0
constant
arg:0
constant
arg:0 >>>
arg:1 >>
arg:0 chain >
side-effects
arg:0
side-effects arg:0 >>>
The rest (TARGET_EXPR, ctor etc.) appears during instantiation.
Jakub
Re: [PATCH] Fix vect/costmodel/ppc/costmodel-vect-33.c testcase (PR middle-end/87157)
On 11/27/18 8:47 AM, Jakub Jelinek wrote: > Hi! > > This testcase started FAILing, because function splitting recently started > splitting main1 into inline containing the cheap loop and main1.part.0 which > contains the verification in abort. I believe the test is meant to test > the vectorizer behavior, not how many times that loop has been inlined > somewhere. This patch arranges for main1 not to be inlined or split. > > Tested on powerpc64le-linux, ok for trunk? > > 2018-11-27 Jakub Jelinek > > PR middle-end/87157 > * gcc.dg/vect/costmodel/ppc/costmodel-vect-33.c (main1): Add noipa > attribute. OK. jeff
[build] Fix libgphobos linking on Solaris 11
As mentioned in passing in PR d/87864, libgphobos.so currently fails to link before Solaris 11.4. Until then, you needed to link with -lsocket -lnsl for the networking functions, in S11.4 they were merged into libc. To fix this, I've adapted the check from libgo/configure.ac, for the moment just moving it into an autoconf macro, reindenting it, renaming the variables for the new location, and removing the check for sendfile which isn't used in libphobos. With that patch (and the one from PR d/87864 to provide __start_minfo/__stop_minfo when ld does not), I could bootstrap with --enable-libphobos on i386-pc-solaris2.11 with gas and sparc-sun-solaris2.11 with as on both S11.3 and S11.4. On the former, libsocket and libnsl were properly detected and linked into libgdruntime.so and libgphobos.so, leaving no undefined symbols, while on the latter nothing more than libc is needed. Ok for mainline? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2018-11-25 Rainer Orth * m4/druntime/libraries.m4 (DRUNTIME_LIBRARIES_NET): New macro. * configure.ac: Invoke it. * configure: Regenerate. # HG changeset patch # Parent 8068f906d5f73ddb769b1241419ed3a330243e2b Fix libphobos linking on Solaris 11 diff --git a/libphobos/configure.ac b/libphobos/configure.ac --- a/libphobos/configure.ac +++ b/libphobos/configure.ac @@ -140,6 +140,7 @@ WITH_LOCAL_DRUNTIME([ DRUNTIME_LIBRARIES_ATOMIC DRUNTIME_LIBRARIES_BACKTRACE DRUNTIME_LIBRARIES_DLOPEN +DRUNTIME_LIBRARIES_NET DRUNTIME_LIBRARIES_ZLIB DRUNTIME_INSTALL_DIRECTORIES diff --git a/libphobos/m4/druntime/libraries.m4 b/libphobos/m4/druntime/libraries.m4 --- a/libphobos/m4/druntime/libraries.m4 +++ b/libphobos/m4/druntime/libraries.m4 @@ -42,6 +42,40 @@ AC_DEFUN([DRUNTIME_LIBRARIES_DLOPEN], ]) +# DRUNTIME_LIBRARIES_NET +# --- +# Autodetect and add networking library to LIBS if necessary. +AC_DEFUN([DRUNTIME_LIBRARIES_NET], +[ + dnl Test for -lsocket and -lnsl. Copied from libjava/configure.ac. + AC_CACHE_CHECK([for socket libraries], druntime_cv_lib_sockets, +[druntime_cv_lib_sockets= + druntime_check_both=no + AC_CHECK_FUNC(connect, druntime_check_socket=no, druntime_check_socket=yes) + if test "$druntime_check_socket" = "yes"; then + unset ac_cv_func_connect + AC_CHECK_LIB(socket, main, druntime_cv_lib_sockets="-lsocket", + druntime_check_both=yes) + fi + if test "$druntime_check_both" = "yes"; then + druntime_old_libs=$LIBS + LIBS="$LIBS -lsocket -lnsl" + unset ac_cv_func_accept + AC_CHECK_FUNC(accept, + [druntime_check_nsl=no + druntime_cv_lib_sockets="-lsocket -lnsl"]) + unset ac_cv_func_accept + LIBS=$druntime_old_libs + fi + unset ac_cv_func_gethostbyname + druntime_old_libs="$LIBS" + AC_CHECK_FUNC(gethostbyname, , + [AC_CHECK_LIB(nsl, main, + [druntime_cv_lib_sockets="$druntime_cv_lib_sockets -lnsl"])]) + ]) + LIBS="$LIBS $druntime_cv_lib_sockets" +]) + # DRUNTIME_LIBRARIES_ZLIB # --- # Allow specifying whether to use the system zlib or
Re: [C++ PATCH] Don't incorrectly reject {un,}signed char constexpr array initialization in templates (PR c++/87476)
On 11/16/18 4:00 AM, Jakub Jelinek wrote:
Hi!
The following two testcases, one is a regression from GCC 8 (introduced by
the constructor to STRING_CST optimization), the other seems to fail since
C++11 support has been introduced (but is accepted by clang++) fail,
because during parsing with processing_template_decl we end up with creating
a STRING_CST with type of {un,}signed char array and later during
instantiation digest_init_r rejects it, because it already has such a type
rather than what it expects (char array) and bogusly complains that it is a
wide string.
The following patch fixes that.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-11-16 Jakub Jelinek
PR c++/87476
* typeck2.c (digest_init_r): Re-add handing of signed/unsigned char
strings and add it to the initialization of wide array from non-wide
string diagnostics too.
OK.
Jason
Re: [C++ PATCH] Fix wrong-code with generic lambda (PR c++/86943)
On 11/23/18 4:15 PM, Jakub Jelinek wrote: Hi! On the following testcase, the call to operator () is marked as CALL_FROM_THUNK_P and therefore genericization ignores all subtrees thereof. Unfortunately, one of the arguments is a move ctor call which is not itself CALL_FROM_THUNK_P and thus we need to genericize its arguments, otherwise we pass address of a temporary which holds a reference value instead of the reference itself. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Or should CALL_FROM_THUNK_P not be set in this call (it is set in maybe_add_lambda_conv_op and then copied over during tsubst*). The call with CALL_FROM_THUNK_P set should be inside the thunk whose address we get when converting the lambda to a function pointer (on return from foo). Its arguments should just be the parms of that thunk, I don't know where this temporary is coming from. Jason
Re: Fix hashtable memory leak
On 27/11/18 22:31 +0100, François Dumont wrote: I eventually called the new method _M_assign_elements. Perfect. And yes, tracker_allocator was enough. Great. Committed on trunk for the moment. Great, thanks. Please note that GCC 7.4 RC1 is scheduled for this week: https://gcc.gnu.org/ml/gcc/2018-11/msg00118.html Will you be able to backport the simpler patch (without the refactoring to remove code duplication) to the branch before then? If not I can take care of it.
Re: [PATCH] target/58397: add host_hooks for NetBSD to make precompiled headers work
Hej! On Sun, 25 Nov 2018, Krister Walfridsson wrote: > On Sun, 25 Nov 2018, Maya Rashish wrote: > > > gcc/config.host | 4 ++ > > gcc/config/host-netbsd.c | 85 > > gcc/config/x-netbsd | 4 ++ > > 3 files changed, 93 insertions(+) > > create mode 100644 gcc/config/host-netbsd.c > > create mode 100644 gcc/config/x-netbsd > > I started a new job a wile back, and my paper work is currently not in order. > So I do not think I am allowed to approve patches now... :( Ackchyually... You're right to be cautious about these matters of course, but to avoid this leading to any kind of precedent, I'll just say that in my book approving != submitting or committing. IOW, I wouldn't hesitate to (only) *review* and *approve* patches that are otherwise in good standing regarding paperwork (though I don't know if that's the case here), if I had changed jobs thereby voiding my filed FSF paperwork. brgds, H-P
Go patch committed: Add result parameter names for inlinable functions
This patch to the Go frontend adds result parameter names for
inlinable functions. An inlinable function body may need to refer to
result parameters, so each result parameter needs a name. We already
give them all names in start_function (via create_result_variables).
Change the export data so that for an inlinable function we use those
names for the function declaration's result parameters. Bootstrapped
and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266531)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-21cf8069ceb078de54cc43ac25c9c89bd15cba56
+5d0c788cd6099c2bb28bb0ff6a04d94006fbfca8
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 266530)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -5487,7 +5487,7 @@ Function::export_func(Export* exp, const
Block* block = NULL;
if (this->export_for_inlining())
block = this->block_;
- Function::export_func_with_type(exp, name, this->type_,
+ Function::export_func_with_type(exp, name, this->type_, this->results_,
this->is_method() && this->nointerface(),
block, this->location_);
}
@@ -5496,8 +5496,9 @@ Function::export_func(Export* exp, const
void
Function::export_func_with_type(Export* exp, const std::string& name,
- const Function_type* fntype, bool nointerface,
- Block* block, Location loc)
+ const Function_type* fntype,
+ Function::Results* result_vars,
+ bool nointerface, Block* block, Location loc)
{
exp->write_c_string("func ");
@@ -5549,31 +5550,45 @@ Function::export_func_with_type(Export*
}
exp->write_c_string(")");
- const Typed_identifier_list* results = fntype->results();
- if (results != NULL)
+ const Typed_identifier_list* result_decls = fntype->results();
+ if (result_decls != NULL)
{
- if (results->size() == 1 && results->begin()->name().empty())
+ if (result_decls->size() == 1
+ && result_decls->begin()->name().empty()
+ && block == NULL)
{
exp->write_c_string(" ");
- exp->write_type(results->begin()->type());
+ exp->write_type(result_decls->begin()->type());
}
else
{
exp->write_c_string(" (");
bool first = true;
- for (Typed_identifier_list::const_iterator p = results->begin();
- p != results->end();
- ++p)
+ Results::const_iterator pr;
+ if (result_vars != NULL)
+ pr = result_vars->begin();
+ for (Typed_identifier_list::const_iterator pd = result_decls->begin();
+ pd != result_decls->end();
+ ++pd)
{
if (first)
first = false;
else
exp->write_c_string(", ");
- exp->write_name(p->name());
- exp->write_escape(p->note());
+ // We only use pr->name, which may be artificial, if
+ // need it for inlining.
+ if (block == NULL || result_vars == NULL)
+ exp->write_name(pd->name());
+ else
+ exp->write_name((*pr)->name());
+ exp->write_escape(pd->note());
exp->write_c_string(" ");
- exp->write_type(p->type());
+ exp->write_type(pd->type());
+ if (result_vars != NULL)
+ ++pr;
}
+ if (result_vars != NULL)
+ go_assert(pr == result_vars->end());
exp->write_c_string(")");
}
}
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 266530)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -1513,8 +1513,8 @@ class Function
// Export a function with a type.
static void
export_func_with_type(Export*, const std::string& name,
- const Function_type*, bool nointerface, Block* block,
- Location);
+ const Function_type*, Results*, bool nointerface,
+ Block* block, Location);
// Import a function.
static void
@@ -1740,7 +1740,7 @@ class Function_declaration
void
export_func(Export* exp, const std::string& name) const
{
-Function::export_func_with_type(exp, name, this->fntype_,
+Function::export_func_with_type(exp, name, this->fntype_, NULL,
this->is_method() && this->nointerface(),
Go patch committed: Add types used by inline functions to export data
This Go frontend patch add types used by inline functions to the
export data. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266530)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-c11d9528a0846293e4d615c86fc773c97252fdce
+21cf8069ceb078de54cc43ac25c9c89bd15cba56
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/export.cc
===
--- gcc/go/gofrontend/export.cc (revision 266523)
+++ gcc/go/gofrontend/export.cc (working copy)
@@ -290,6 +290,11 @@ Find_types_to_prepare::type(Type* type)
if (type->is_void_type())
return TRAVERSE_SKIP_COMPONENTS;
+ // Skip abstract types. We should never see these in real code,
+ // only in things like const declarations.
+ if (type->is_abstract())
+return TRAVERSE_SKIP_COMPONENTS;
+
if (!this->exp_->set_type_index(type))
{
// We've already seen this type.
@@ -367,7 +372,12 @@ Find_types_to_prepare::traverse_named_ty
methods->begin_definitions();
pm != methods->end_definitions();
++pm)
- this->traverse_function((*pm)->func_value()->type());
+ {
+ Function* fn = (*pm)->func_value();
+ this->traverse_function(fn->type());
+ if (fn->export_for_inlining())
+ fn->block()->traverse(this);
+ }
for (Bindings::const_declarations_iterator pm =
methods->begin_declarations();
@@ -434,7 +444,12 @@ Export::prepare_types(const std::vector<
break;
case Named_object::NAMED_OBJECT_FUNC:
- find.traverse_function(no->func_value()->type());
+ {
+ Function* fn = no->func_value();
+ find.traverse_function(fn->type());
+ if (fn->export_for_inlining())
+ fn->block()->traverse(&find);
+ }
break;
case Named_object::NAMED_OBJECT_FUNC_DECLARATION:
Go patch committed: Finalize types parsed for inline functions
When the Go frontend inlines functions from other packages, we may
parse types that we have not seen before inlining. Inlining runs
after the finalize_methods pass, so those types will not be finalized,
meaning that we don't have an accurate list of which methods they
support. Explicitly finalize them when we parse them. Bootstrapped
and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266529)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-ce81aad0e3d53215e2c0f1f060c7fd6219e6fb23
+c11d9528a0846293e4d615c86fc773c97252fdce
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 266526)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -3243,6 +3243,17 @@ Gogo::finalize_methods()
this->traverse(&finalize);
}
+// Finalize the method list for a type. This is called when a type is
+// parsed for an inlined function body, which happens after the
+// finalize_methods pass.
+
+void
+Gogo::finalize_methods_for_type(Type* type)
+{
+ Finalize_methods finalize(this);
+ Type::traverse(type, &finalize);
+}
+
// Set types for unspecified variables and constants.
void
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 266526)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -637,6 +637,10 @@ class Gogo
void
finalize_methods();
+ // Finalize the method list for one type.
+ void
+ finalize_methods_for_type(Type*);
+
// Work out the types to use for unspecified variables and
// constants.
void
Index: gcc/go/gofrontend/import.cc
===
--- gcc/go/gofrontend/import.cc (revision 266526)
+++ gcc/go/gofrontend/import.cc (working copy)
@@ -886,7 +886,9 @@ Import::read_type()
if (c == '>')
{
// A reference to a type defined earlier.
- return this->type_for_index(index, "import data", stream->pos());
+ bool parsed;
+ return this->type_for_index(index, "import data", stream->pos(),
+ &parsed);
}
if (this->version_ >= EXPORT_FORMAT_V3)
@@ -1092,12 +1094,13 @@ Import::read_named_type(int index)
return type;
}
-// Return the type given an index.
+// Return the type given an index. Set *PARSED if we parsed it here.
Type*
Import::type_for_index(int index, const std::string& input_name,
- size_t input_offset)
+ size_t input_offset, bool* parsed)
{
+ *parsed = false;
if (index >= 0 && !this->type_data_.empty())
{
if (static_cast(index) >= this->type_offsets_.size())
@@ -1114,6 +1117,7 @@ Import::type_for_index(int index, const
{
if (!this->parse_type(index))
return Type::make_error_type();
+ *parsed = true;
}
}
@@ -1497,6 +1501,15 @@ Import_function_body::read_type()
return Type::make_error_type();
}
- return this->imp_->type_for_index(static_cast(val), this->name(),
- static_cast(start));
+ bool parsed;
+ Type* type = this->imp_->type_for_index(static_cast(val), this->name(),
+ static_cast(start),
+ &parsed);
+
+ // If we just read this type's information, its methods will not
+ // have been finalized. Do that now.
+ if (parsed)
+this->gogo_->finalize_methods_for_type(type);
+
+ return type;
}
Index: gcc/go/gofrontend/import.h
===
--- gcc/go/gofrontend/import.h (revision 266529)
+++ gcc/go/gofrontend/import.h (working copy)
@@ -284,10 +284,11 @@ class Import : public Import_expression
read_type();
// Return the type for a type index. INPUT_NAME and INPUT_OFFSET
- // are only for error reporting.
+ // are only for error reporting. PARSED is set to whether we parsed
+ // the type information for a new type.
Type*
type_for_index(int index, const std::string& input_name,
-size_t input_offset);
+size_t input_offset, bool* parsed);
// Read an escape note.
std::string
Re: Fix hashtable memory leak
I eventually called the new method _M_assign_elements.
And yes, tracker_allocator was enough.
Committed on trunk for the moment.
François
On 11/26/18 1:12 PM, Jonathan Wakely wrote:
On 24/11/18 22:58 +0100, François Dumont wrote:
---
a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
+++
b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
@@ -18,6 +18,8 @@
// { dg-do run { target c++11 } }
#include
+#include
+
#include
#include
#include
@@ -27,7 +29,9 @@ using __gnu_test::counter_type;
void test01()
{
- typedef propagating_allocator alloc_type;
+ {
+ typedef propagating_allocator> alloc_type;
You don't seem to need the throw_allocator here, can't you use
__gnu_test::tracker_allocator from instead?
.
Re: [C++ PATCH] Fix ICE in grokdeclarator (PR c++/88187)
On 11/26/18 3:49 PM, Jakub Jelinek wrote: Hi! Marek has changed grokdeclarator in r263836, so that in this part of code it is either a funcdecl_p (previously the only allowed one), which implies inner_declarator is non-NULL and therefore unqualified_id too, or newly inner_declarator == NULL. In that case, we IMHO shouldn't be testing for the deduction guides errors and let it be rejected as before later. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-11-26 Jakub Jelinek PR c++/88187 * decl.c (grokdeclarator): Don't diagnose deduction guide errors if inner_declarator is NULL. * g++.dg/other/pr88187.C: New test. --- gcc/cp/decl.c.jj2018-11-17 00:16:41.0 +0100 +++ gcc/cp/decl.c 2018-11-26 11:18:30.518620651 +0100 @@ -11276,7 +11276,7 @@ grokdeclarator (const cp_declarator *dec if (!tmpl) if (tree late_auto = type_uses_auto (late_return_type)) tmpl = CLASS_PLACEHOLDER_TEMPLATE (late_auto); - if (tmpl) + if (tmpl && inner_declarator) Let's check funcdecl_p rather than inner_declarator. OK with that change. Jason
Go patch committed: Add '$' to names in expression export data
This patch to the Go frontend adds '$' to names in expression export
data. For inlined function bodies we're going to need to refer to
variables, so change the existing export data to add a '$' to names
that look like identifiers: true, false, nil, convert. While we're
here drop an unnecessary space character after operators.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266526)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-6e0974fc6c9aa6ef19f72fbb5698e4b3734a4220
+ce81aad0e3d53215e2c0f1f060c7fd6219e6fb23
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266526)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1610,7 +1610,7 @@ class Boolean_expression : public Expres
void
do_export(Export_function_body* efb) const
- { efb->write_c_string(this->val_ ? "true" : "false"); }
+ { efb->write_c_string(this->val_ ? "$true" : "$false"); }
void
do_dump_expression(Ast_dump_context* ast_dump_context) const
@@ -1651,6 +1651,8 @@ Boolean_expression::do_determine_type(co
Expression*
Boolean_expression::do_import(Import_expression* imp, Location loc)
{
+ if (imp->version() >= EXPORT_FORMAT_V3)
+imp->require_c_string("$");
if (imp->peek_char() == 't')
{
imp->require_c_string("true");
@@ -3162,7 +3164,7 @@ class Nil_expression : public Expression
void
do_export(Export_function_body* efb) const
- { efb->write_c_string("nil"); }
+ { efb->write_c_string("$nil"); }
void
do_dump_expression(Ast_dump_context* ast_dump_context) const
@@ -3174,6 +3176,8 @@ class Nil_expression : public Expression
Expression*
Nil_expression::do_import(Import_expression* imp, Location loc)
{
+ if (imp->version() >= EXPORT_FORMAT_V3)
+imp->require_c_string("$");
imp->require_c_string("nil");
return Expression::make_nil(loc);
}
@@ -3613,7 +3617,7 @@ Type_conversion_expression::do_get_backe
void
Type_conversion_expression::do_export(Export_function_body* efb) const
{
- efb->write_c_string("convert(");
+ efb->write_c_string("$convert(");
efb->write_type(this->type_);
efb->write_c_string(", ");
this->expr_->export_expression(efb);
@@ -3625,7 +3629,7 @@ Type_conversion_expression::do_export(Ex
Expression*
Type_conversion_expression::do_import(Import_expression* imp, Location loc)
{
- imp->require_c_string("convert(");
+ imp->require_c_string("$convert(");
Type* type = imp->read_type();
imp->require_c_string(", ");
Expression* val = Expression::import_expression(imp, loc);
@@ -4612,16 +4616,16 @@ Unary_expression::do_export(Export_funct
switch (this->op_)
{
case OPERATOR_PLUS:
- efb->write_c_string("+ ");
+ efb->write_c_string("+");
break;
case OPERATOR_MINUS:
- efb->write_c_string("- ");
+ efb->write_c_string("-");
break;
case OPERATOR_NOT:
- efb->write_c_string("! ");
+ efb->write_c_string("!");
break;
case OPERATOR_XOR:
- efb->write_c_string("^ ");
+ efb->write_c_string("^");
break;
case OPERATOR_AND:
case OPERATOR_MULT:
@@ -4654,7 +4658,8 @@ Unary_expression::do_import(Import_expre
default:
go_unreachable();
}
- imp->require_c_string(" ");
+ if (imp->version() < EXPORT_FORMAT_V3)
+imp->require_c_string(" ");
Expression* expr = Expression::import_expression(imp, loc);
return Expression::make_unary(op, expr, loc);
}
@@ -12959,7 +12964,7 @@ Struct_construction_expression::do_get_b
void
Struct_construction_expression::do_export(Export_function_body* efb) const
{
- efb->write_c_string("convert(");
+ efb->write_c_string("$convert(");
efb->write_type(this->type_);
for (Expression_list::const_iterator pv = this->vals()->begin();
pv != this->vals()->end();
@@ -13192,7 +13197,7 @@ Array_construction_expression::get_const
void
Array_construction_expression::do_export(Export_function_body* efb) const
{
- efb->write_c_string("convert(");
+ efb->write_c_string("$convert(");
efb->write_type(this->type_);
if (this->vals() != NULL)
{
@@ -13709,7 +13714,7 @@ Map_construction_expression::do_get_back
void
Map_construction_expression::do_export(Export_function_body* efb) const
{
- efb->write_c_string("convert(");
+ efb->write_c_string("$convert(");
efb->write_type(this->type_);
for (Expression_list::const_iterator pv = this->vals_->begin();
pv != this->vals_->end();
@@ -16141,14 +16146,15 @@ Expression*
Expression::import_expression(Import_expression* imp, Location loc)
{
int c = imp->peek_char();
- if (imp->match_c_string("- ")
- || imp->match_c_s
[PATCH] Fix 87380.
Hi This is [intentionally] broken C++ ABI, that was catering for a tool problem that existed in a very old Darwin toolchain. I checked that the issue is not present after Darwin7 (using default Xcode tools). Of course, more modern tools are probably required to build trunk GCC for Darwin7, but somehow I doubt anyone has time to try that… anyway, this is long-standing breakage on all open branches. NOTE: re comment #18 in the PR, rs6000/darwin7.h is included after the generic header darwin.h, and thus it is sufficient to cover the case there. OK for trunk? open branches? Iain gcc/ * gcc/config/darwin.h (TARGET_WEAK_NOT_IN_ARCHIVE_TOC): Set to 0. Update comment. * gcc/config/rs6000/darwin7.h (TARGET_WEAK_NOT_IN_ARCHIVE_TOC): New. diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h index 87f610259c..974eb9fbf6 100644 --- a/gcc/config/darwin.h +++ b/gcc/config/darwin.h @@ -511,10 +511,9 @@ extern GTY(()) int darwin_ms_struct; links to, so there's no need for weak-ness for that. */ #define GTHREAD_USE_WEAK 0 -/* The Darwin linker doesn't want coalesced symbols to appear in - a static archive's table of contents. */ +/* Modern Darwin toolchains export weak symbols from archive TOCs. */ #undef TARGET_WEAK_NOT_IN_ARCHIVE_TOC -#define TARGET_WEAK_NOT_IN_ARCHIVE_TOC 1 +#define TARGET_WEAK_NOT_IN_ARCHIVE_TOC 0 /* On Darwin, we don't (at the time of writing) have linkonce sections with names, so it's safe to make the class data not comdat. */ diff --git a/gcc/config/rs6000/darwin7.h b/gcc/config/rs6000/darwin7.h index d35b65d699..85ea18e53e 100644 --- a/gcc/config/rs6000/darwin7.h +++ b/gcc/config/rs6000/darwin7.h @@ -28,5 +28,10 @@ along with GCC; see the file COPYING3. If not see %:version-compare(!< 10.3 mmacosx-version-min= -lmx)\ -lSystem}" +/* This generation of tools (specifically the archive tool) did not + export weak symbols from the TOC. */ +#undef TARGET_WEAK_NOT_IN_ARCHIVE_TOC +#define TARGET_WEAK_NOT_IN_ARCHIVE_TOC 1 + #undef DEF_MIN_OSX_VERSION #define DEF_MIN_OSX_VERSION "10.3.9" -- 2.17.1
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
Am 27.11.18 um 17:22 schrieb Steve Ellcey: Why wouldn't clang (flang) want to use the same mechanism as GCC/gfortran? I know there is some interest/work going on here for flang and we would like a consistent way to use pre-includes to define SIMD vector functions in both gfortran and flang. I think this should be documented so flang and other compilers can use it. Even if no other compilers did use it I think it should be documented because it crosses project/package boundries, i.e. it is created by glibc and used by gfortran. Absolutely. As soon as this is committed, we should document all the specifics in the gfortran manual. Regards Thomas
Re: C++ PATCH to implement P1094R2, Nested inline namespaces
On 11/23/18 4:12 PM, Marek Polacek wrote: I wasn't aware you had worked on this. Perhaps we should track the progress of C++20 features in Bugzilla (to keep track of who's working on what). Yes, I think so. I created a couple of PRs, for contracts and cmpxchg with padding bits, but more are needed. Jason
Re: [C++ PATCH] Propagate TYPE_PACKED to template variants (PR c++/88181)
On 11/26/18 3:54 PM, Jakub Jelinek wrote: Hi! On the following patch -fpack-struct forces TYPE_PACKED on all the classes and their variants, but we then create a variant of a class instantiation (const) which doesn't have the TYPE_PACKED set and later finalize the template main variant, but don't propagate that to the already created variants. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-11-26 Jakub Jelinek PR c++/88181 * class.c (fixup_attribute_variants): Also propagate TYPE_PACKED to variants. OK. Jason
Re: C++ PATCH to implement P1094R2, Nested inline namespaces
On 11/21/18 8:46 PM, Marek Polacek wrote:
On Tue, Nov 20, 2018 at 04:59:46PM -0500, Jason Merrill wrote:
On 11/19/18 5:12 PM, Marek Polacek wrote:
On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
2018-11-19 Marek Polacek
Implement P1094R2, Nested inline namespaces.
* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.
* g++.dg/cpp2a/nested-inline-ns3.C: New test.
Just a small testsuite comment.
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
Especially because 2a testing isn't included by default, but also
to make sure it works right even with -std=c++17, wouldn't it be better to
drop the nested-inline-ns3.C test, make this test c++17 or
even better always enabled, add dg-options "-Wpedantic" and
just add dg-warning with c++17_down and c++14_down what should be
warned on the 3 lines (with .-1 for c++14_down)?
Or if you want add some further testcases that will test how
c++17 etc. will dg-error on those with -pedantic-errors etc.
Sure, I've made it { target c++11 } and dropped the third test:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-11-19 Marek Polacek
Implement P1094R2, Nested inline namespaces.
* parser.c (cp_parser_namespace_definition): Parse the optional inline
keyword in a nested-namespace-definition. Adjust push_namespace call.
Formatting fix.
* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 292cce15676..f39e9d753d2 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
cp_ensure_no_oacc_routine (parser);
bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer,
RID_INLINE);
+ const bool topmost_inline_p = is_inline;
if (is_inline)
{
@@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
{
identifier = NULL_TREE;
+ bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+RID_INLINE);
+ if (nested_inline_p && nested_definition_count != 0)
+ {
+ if (cxx_dialect < cxx2a)
+ pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+OPT_Wpedantic, "nested inline namespace definitions only "
+"available with -std=c++2a or -std=gnu++2a");
+ cp_lexer_consume_token (parser->lexer);
+ }
This looks like we won't get any diagnostic in lower conformance modes if
there are multiple namespace scopes before the inline keyword.
If you mean something like
namespace A::B:C::inline D { }
then we do get a diagnostic. nested-inline-ns1.C tests that. Or do you
mean something else?
No, this is fine then.
if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
{
identifier = cp_parser_identifier (parser);
@@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
}
if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
- break;
+ {
+ /* Don't forget that the innermost namespace might have been
+marked as inline. */
+ is_inline |= nested_inline_p;
This looks wrong: an inline namespace does not make its nested namespaces
inline as well.
A nested namespace definition cannot be inline. This is supposed to handle
cases such as
namespace A::B::inline C { ... }
because after 'C' we don't see :: so it breaks and we call push_namespace
outside the for loop.
Ah, yes, I see. I was confused by the use of '|='; what is that for?
Why not use '='? For that matter, why not modify is_inline in the loop
instead of a new nested_inline_p variable?
Jason
Re: [PATCH v3] [aarch64] Correct the maximum shift amount for shifted operands.
> On 27.11.2018, at 14:04, Sam Tebbs wrote:
>
>
> On 11/26/18 7:50 PM, Christoph Muellner wrote:
>> The aarch64 ISA specification allows a left shift amount to be applied
>> after extension in the range of 0 to 4 (encoded in the imm3 field).
>>
>> This is true for at least the following instructions:
>>
>> * ADD (extend register)
>> * ADDS (extended register)
>> * SUB (extended register)
>>
>> The result of this patch can be seen, when compiling the following code:
>>
>> uint64_t myadd(uint64_t a, uint64_t b)
>> {
>> return a+(((uint8_t)b)<<4);
>> }
>>
>> Without the patch the following sequence will be generated:
>>
>> :
>>0:d37c1c21 ubfizx1, x1, #4, #8
>>4:8b20 addx0, x1, x0
>>8:d65f03c0 ret
>>
>> With the patch the ubfiz will be merged into the add instruction:
>>
>> :
>>0:8b211000 addx0, x0, w1, uxtb #4
>>4:d65f03c0 ret
>
> Hi Christoph,
>
> Thanks for this, I'm not a maintainer but this looks good to me. A good
> point to mention would be if it affects shifts smaller than 4 bits,
> since I don't see any testing for that in the files you have changed.
>
Hi Sam,
shifts smaller than 4 bits are already tested in
gcc/testsuite/gcc.target/aarch64/extend.c.
E.g. subdi_sxtw() does so for shift-by-3.
All existing test cases where executed and did not show any regressions.
In other words: shifts smaller than 4 bits are not affected.
>> Tested with "make check" and no regressions found.
> Could you perhaps elaborate on your testing? So what triplets you
> tested, if you bootstrapped successfully etc.
For the "make check" regression test, I compiled native on an AArch64 machine
running CentOS 7.5. Configure flags were "--enable-bootstrap".
I did so with and without the patch and compared the results for differences.
I think that's the essential requirement to get an OK for this patch.
Besides that we've had this change in our aarch64-linux-gnu toolchain since
2014.
This toolchain has been used to compile a wide range of OSS projects, benchmarks
as well as proprietary code over the years.
For example we ran the SPEC CPU2000, CPU2006, CPU2017 benchmarks,
built Linux kernels (at least from 4.4 to 4.19), glibc (from 2.17 to 2.28),
binutils
(ancient times till 2.30), TCmalloc (2.6 and 2.7), jemalloc (4 and 5),
buildroot (2017, 2018),
U-Boot (2015-2018), Tianocore, HHVM, OpenSSL, MySQL,...
Our work involved interwork with existing libraries (e.g. HHVM and its
dependencies
to 100 external shared libraries) on several operating systems (Ubuntu, Fedora,
CentOS, OpenSuse, Oracle Linux, Debian, ...).
Besides LP64 we also used (and still use) the ILP32 ABI.
The binaries created by the compilers using that change were running
at least on APM Xgene processors, Ampere Computer's eMAG processors,
as well as on ARM's Cortex-A53, Cortex-A72 cores.
If you want me to test something specific, then just let me know.
Thanks,
Christoph
Go patch committed: Change expressions importing to use Import_expression
This patch to the Go frontend changes expression importing to use a
new abstract interface class Import_expression, so that we can more
easily import expressions from inlinable function bodies. This is a
refactoring with no affect on compiler behavior. Bootstrapped and ran
Go tests on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266525)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-75d48ff977a2865d12b03857362ea48016a4b885
+6e0974fc6c9aa6ef19f72fbb5698e4b3734a4220
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266525)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1583,7 +1583,7 @@ class Boolean_expression : public Expres
{ }
static Expression*
- do_import(Import*, Location);
+ do_import(Import_expression*, Location);
protected:
bool
@@ -1649,7 +1649,7 @@ Boolean_expression::do_determine_type(co
// Import a boolean constant.
Expression*
-Boolean_expression::do_import(Import* imp, Location loc)
+Boolean_expression::do_import(Import_expression* imp, Location loc)
{
if (imp->peek_char() == 't')
{
@@ -1768,7 +1768,7 @@ String_expression::do_export(Export_func
// Import a string expression.
Expression*
-String_expression::do_import(Import* imp, Location loc)
+String_expression::do_import(Import_expression* imp, Location loc)
{
imp->require_c_string("\"");
std::string val;
@@ -1944,7 +1944,7 @@ class Integer_expression : public Expres
{ mpz_init_set(this->val_, *val); }
static Expression*
- do_import(Import*, Location);
+ do_import(Import_expression*, Location);
// Write VAL to string dump.
static void
@@ -2151,7 +2151,7 @@ Integer_expression::do_export(Export_fun
// all these types because they all start with digits.
Expression*
-Integer_expression::do_import(Import* imp, Location loc)
+Integer_expression::do_import(Import_expression* imp, Location loc)
{
std::string num = imp->read_identifier();
imp->require_c_string(" ");
@@ -3133,7 +3133,7 @@ class Nil_expression : public Expression
{ }
static Expression*
- do_import(Import*, Location);
+ do_import(Import_expression*, Location);
protected:
bool
@@ -3172,7 +3172,7 @@ class Nil_expression : public Expression
// Import a nil expression.
Expression*
-Nil_expression::do_import(Import* imp, Location loc)
+Nil_expression::do_import(Import_expression* imp, Location loc)
{
imp->require_c_string("nil");
return Expression::make_nil(loc);
@@ -3623,7 +3623,7 @@ Type_conversion_expression::do_export(Ex
// Import a type conversion or a struct construction.
Expression*
-Type_conversion_expression::do_import(Import* imp, Location loc)
+Type_conversion_expression::do_import(Import_expression* imp, Location loc)
{
imp->require_c_string("convert(");
Type* type = imp->read_type();
@@ -4634,7 +4634,7 @@ Unary_expression::do_export(Export_funct
// Import a unary expression.
Expression*
-Unary_expression::do_import(Import* imp, Location loc)
+Unary_expression::do_import(Import_expression* imp, Location loc)
{
Operator op;
switch (imp->get_char())
@@ -6403,7 +6403,7 @@ Binary_expression::do_export(Export_func
// Import a binary expression.
Expression*
-Binary_expression::do_import(Import* imp, Location loc)
+Binary_expression::do_import(Import_expression* imp, Location loc)
{
imp->require_c_string("(");
@@ -16138,7 +16138,7 @@ Expression::make_backend(Bexpression* be
// various class definitions.
Expression*
-Expression::import_expression(Import* imp, Location loc)
+Expression::import_expression(Import_expression* imp, Location loc)
{
int c = imp->peek_char();
if (imp->match_c_string("- ")
Index: gcc/go/gofrontend/expressions.h
===
--- gcc/go/gofrontend/expressions.h (revision 266525)
+++ gcc/go/gofrontend/expressions.h (working copy)
@@ -66,7 +66,7 @@ class Compound_expression;
class Numeric_constant;
class Named_object;
class Export_function_body;
-class Import;
+class Import_expression;
class Temporary_statement;
class Label;
class Ast_dump_context;
@@ -1018,7 +1018,7 @@ class Expression
// returned expression. Errors should be reported using the
// Import's location method.
static Expression*
- import_expression(Import*, Location);
+ import_expression(Import_expression*, Location);
// Return an expression which checks that VAL, of arbitrary integer type,
// is non-negative and is not more than the maximum integer value.
@@ -1567,7 +1567,7 @@ class String_expression : public Express
{ return this->val_; }
static Expression*
- do_import(Imp
Go patch committed: Pass a Location to import_expression
This patch changes the Go frontend to pass a Location to
import_expression. This separates the Location that import_expression
uses when creating a new Expression from the Location used to report
an error. This is a step toward importing expressions for inlined
functions. This is a pure refactoring that does not affect compiler
behavior. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266523)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-db5240278b3b62a919dd88f857e718a66be50346
+75d48ff977a2865d12b03857362ea48016a4b885
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266523)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1583,7 +1583,7 @@ class Boolean_expression : public Expres
{ }
static Expression*
- do_import(Import*);
+ do_import(Import*, Location);
protected:
bool
@@ -1649,17 +1649,17 @@ Boolean_expression::do_determine_type(co
// Import a boolean constant.
Expression*
-Boolean_expression::do_import(Import* imp)
+Boolean_expression::do_import(Import* imp, Location loc)
{
if (imp->peek_char() == 't')
{
imp->require_c_string("true");
- return Expression::make_boolean(true, imp->location());
+ return Expression::make_boolean(true, loc);
}
else
{
imp->require_c_string("false");
- return Expression::make_boolean(false, imp->location());
+ return Expression::make_boolean(false, loc);
}
}
@@ -1768,7 +1768,7 @@ String_expression::do_export(Export_func
// Import a string expression.
Expression*
-String_expression::do_import(Import* imp)
+String_expression::do_import(Import* imp, Location loc)
{
imp->require_c_string("\"");
std::string val;
@@ -1800,11 +1800,11 @@ String_expression::do_import(Import* imp
else
{
go_error_at(imp->location(), "bad string constant");
- return Expression::make_error(imp->location());
+ return Expression::make_error(loc);
}
}
}
- return Expression::make_string(val, imp->location());
+ return Expression::make_string(val, loc);
}
// Ast dump for string expression.
@@ -1944,7 +1944,7 @@ class Integer_expression : public Expres
{ mpz_init_set(this->val_, *val); }
static Expression*
- do_import(Import*);
+ do_import(Import*, Location);
// Write VAL to string dump.
static void
@@ -2151,7 +2151,7 @@ Integer_expression::do_export(Export_fun
// all these types because they all start with digits.
Expression*
-Integer_expression::do_import(Import* imp)
+Integer_expression::do_import(Import* imp, Location loc)
{
std::string num = imp->read_identifier();
imp->require_c_string(" ");
@@ -2169,7 +2169,7 @@ Integer_expression::do_import(Import* im
{
go_error_at(imp->location(), "bad number in import data: %qs",
num.c_str());
- return Expression::make_error(imp->location());
+ return Expression::make_error(loc);
}
if (pos == std::string::npos)
mpfr_set_ui(real, 0, GMP_RNDN);
@@ -2180,7 +2180,7 @@ Integer_expression::do_import(Import* im
{
go_error_at(imp->location(), "bad number in import data: %qs",
real_str.c_str());
- return Expression::make_error(imp->location());
+ return Expression::make_error(loc);
}
}
@@ -2195,14 +2195,14 @@ Integer_expression::do_import(Import* im
{
go_error_at(imp->location(), "bad number in import data: %qs",
imag_str.c_str());
- return Expression::make_error(imp->location());
+ return Expression::make_error(loc);
}
mpc_t cval;
mpc_init2(cval, mpc_precision);
mpc_set_fr_fr(cval, real, imag, MPC_RNDNN);
mpfr_clear(real);
mpfr_clear(imag);
- Expression* ret = Expression::make_complex(&cval, NULL, imp->location());
+ Expression* ret = Expression::make_complex(&cval, NULL, loc);
mpc_clear(cval);
return ret;
}
@@ -2218,13 +2218,13 @@ Integer_expression::do_import(Import* im
{
go_error_at(imp->location(), "bad number in import data: %qs",
num.c_str());
- return Expression::make_error(imp->location());
+ return Expression::make_error(loc);
}
Expression* ret;
if (is_character_constant)
- ret = Expression::make_character(&val, NULL, imp->location());
+ ret = Expression::make_character(&val, NULL, loc);
else
- ret = Expression::make_integer
Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.
Hi, On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote: > > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those > > targets > > that have a non-executable default stack based on when they call > > file_end_indicate_exec_stack. > > As Paul says, that name isn't so good. > > TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or similar? Would the slightly shorter TARGET_NEEDS_EXEC_STACK_MARKER_FOR_TRAMPOLINES be descriptive enough? > > diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h > > index 0c67634..9330acf 100644 > > --- a/gcc/config/rs6000/sysv4.h > > +++ b/gcc/config/rs6000/sysv4.h > > @@ -972,6 +972,11 @@ ncrtn.o%s" > > /* Generate entries in .fixup for relocatable addresses. */ > > #define RELOCATABLE_NEEDS_FIXUP 1 > > > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD) > > + #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \ > > + || DEFAULT_ABI == ABI_ELFv2) > > +#endif > > I don't think this belongs in sysv4.h . I might have gotten lost in the tree of defines/macros. There are two sysv4.h files gcc/config/rs6000/sysv4.h and gcc/config/powerpcspe/sysv4.h Both define the TARGET_ASM_FILE_END hook to rs6000_elf_file_end. Which are defined in config/rs6000/rs6000.c and config/powerpcspe/powerpcspe.c. Both have: #if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD) if (TARGET_32BIT || DEFAULT_ABI == ABI_ELFv2) file_end_indicate_exec_stack (); #endif And file_end_indicate_exec_stack () is what you call to flip the stack to be executable (if an executable stack marker is needed for generating the trampoline). That is why both sysv4.h files have the new target macro defined the same way. But maybe only one of them really needs it? Thanks, Mark
Re: [PATCH] Added information about inline assembler in stack calculations (.su files)
Hi!
Thanks for the feedback.
Attached is an updated patch where I switched to the NOP define instead.
I'm not sure if stack-usage-naked.c should be moved to gcc.dg-tree, or
if it should skip using the nop.h file (it feels wrong to do #include
"../../gcc.dg/nop.h" from within gcc.taget-tree).
Torbjörn
On 27/11/2018 19:40, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Nov 26, 2018 at 02:02:49PM +, Torbjorn SVENSSON wrote:
>> Attached is a small patch that, in case of inline assembler code,
>> indicates that the function stack usage is uncertain due to inline
>> assembler.
>>
>> The test suite are using "nop" as an assembler instruction on all
>> targets, is this acceptable or is there a better way to test this?
> Maybe see testsuite/gcc.dg/nop.h ?
>
>
> Segher
From e8af10ab24c7e095604b16206c0bd544e8699b93 Mon Sep 17 00:00:00 2001
From: Niklas DAHLQUIST
Date: Fri, 9 Nov 2018 18:48:34 +0100
Subject: [PATCH] Added information about inline assembler in stack
calculations
The stack usage calculation in GCC ignores any possible stack usage that
any inline assembly might contribute, and this is not reflected in the
.su file currently.
Since inline assembly will add to the uncertainty of the actual stack
usage for a function, this should be shown in the .su file as well.
This changeset will add "ignoring_inline_assembly" to the .su-file "type"
information of functions containing inline assembly.
The resulting stack usage type for functions containing inline assembly
will be according to the following table:
Static | Dynamic | Inline asm() | Resulting stack usage type
* | 0 | False| static
* | 0 | True | static,ignoring_inline_assembler
* | 0 < x | False| dynamic
* | 0 < x | True | dynamic,ignoring_inline_assembler
* | 0 < x < MAX | False| dynamic,bounded
* | 0 < x < MAX | True | dynamic,bounded,ignoring_inline_assembler
Added test case for ignore inline assembler in stack analysis files (.su)
Signed-off-by: Niklas DAHLQUIST
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 38e27a50a1e..e61ddc3260b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6456,7 +6456,7 @@ Warn if the stack usage of a function might exceed @var{byte-size}.
The computation done to determine the stack usage is conservative.
Any space allocated via @code{alloca}, variable-length arrays, or related
constructs is included by the compiler when determining whether or not to
-issue a warning.
+issue a warning. @code{asm} statements are ignored when computing stack usage.
The message is in keeping with the output of @option{-fstack-usage}.
diff --git a/gcc/function.c b/gcc/function.c
index 69523c1d723..197f80c0df3 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1822,6 +1822,10 @@ instantiate_virtual_regs_in_insn (rtx_insn *insn)
if (asm_noperands (PATTERN (insn)) >= 0)
{
+ if (flag_stack_usage_info)
+ {
+ current_function_has_inline_assembler = 1;
+ }
if (!check_asm_operands (PATTERN (insn)))
{
error_for_asm (insn, "impossible constraint in %");
diff --git a/gcc/function.h b/gcc/function.h
index 7e59050e8a6..8c3ef49e866 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -208,11 +208,16 @@ struct GTY(()) stack_usage
/* Nonzero if the amount of stack space allocated dynamically cannot
be bounded at compile-time. */
unsigned int has_unbounded_dynamic_stack_size : 1;
+
+ /* NonZero if body contains asm statement (ignored in stack calculations) */
+ unsigned int has_inline_assembler: 1;
};
#define current_function_static_stack_size (cfun->su->static_stack_size)
#define current_function_dynamic_stack_size (cfun->su->dynamic_stack_size)
#define current_function_pushed_stack_size (cfun->su->pushed_stack_size)
+#define current_function_has_inline_assembler \
+ (cfun->su->has_inline_assembler)
#define current_function_has_unbounded_dynamic_stack_size \
(cfun->su->has_unbounded_dynamic_stack_size)
#define current_function_allocates_dynamic_stack_space\
diff --git a/gcc/testsuite/gcc.dg/stack-usage-3.c b/gcc/testsuite/gcc.dg/stack-usage-3.c
new file mode 100644
index 000..ccb935f358e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/stack-usage-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-fstack-usage" } */
+/* nvptx doesn't have a reg allocator, and hence no stack usage data. */
+/* { dg-skip-if "" { nvptx-*-* } { "*" } { "" } } */
+
+#include "nop.h"
+
+void foo()
+{
+int i;
+i = 1;
+}
+
+void bar()
+{
+int i;
+i = 1;
+asm(NOP);
+}
+
+/* { dg-final { scan-stack-usage "foo\t\[1-9\]\[0-9\]*\tstatic" } } */
+/* { dg-final { scan-stack-usage "bar\t\[1-9\]\[0-9\]*\tstatic,ignoring_inline_asm" } } */
+/* { dg-final { cleanup-stack-usage } } */
+
diff --git a/gcc/testsuite/gcc.dg/stack-usage-4.c b/gcc/testsuite/gcc.dg/stack-usage-4.
[PATCH v4] Add sinh(atanh(x)) and cosh(atanh(x)) optimizations
Only do this optimization if funsafe-math and -fno-math-errno are
enabled, as pointed in the previous iteration.
Also added one more test case to ensure that fno-math-errno is
required for the optimization.
Special thanks for Wilco Dijsktra for all his help :-)
gcc/ChangeLog
2018-11-27 Giuliano Belinassi
* match.pd (sinh (atanh (x))): New simplification rules.
(cosh (atanh (x))): Likewise.
gcc/testsuite/ChangeLog
2018-11-27 Giuliano Belinassi
* gcc.dg/sinhatanh-1.c: New test.
* gcc.dg/sinhatanh-2.c: New test.
* gcc.dg/sinhatanh-3.c: New test.
Index: gcc/match.pd
===
--- gcc/match.pd (revision 266469)
+++ gcc/match.pd (working copy)
@@ -4342,6 +4342,25 @@
(rdiv { t_one; } (sqrts (plus (mult @0 @0) { t_one; })))
(copysigns { t_zero; } @0))
+ (if (!flag_errno_math)
+ /* Simplify sinh(atanh(x)) -> x / sqrt((1 - x)*(1 + x)). */
+ (for sinhs (SINH)
+ atanhs (ATANH)
+ sqrts (SQRT)
+ (simplify
+(sinhs (atanhs:s @0))
+(with { tree t_one = build_one_cst (type); }
+(rdiv @0 (sqrts (mult (minus { t_one; } @0) (plus { t_one; } @0)))
+
+ /* Simplify cosh(atanh(x)) -> 1 / sqrt((1 - x)*(1 + x)) */
+ (for coshs (COSH)
+ atanhs (ATANH)
+ sqrts (SQRT)
+ (simplify
+(coshs (atanhs:s @0))
+(with { tree t_one = build_one_cst (type); }
+(rdiv { t_one; } (sqrts (mult (minus { t_one; } @0) (plus { t_one; } @0
+
/* cabs(x+0i) or cabs(0+xi) -> abs(x). */
(simplify
(CABS (complex:C @0 real_zerop@1))
Index: gcc/testsuite/gcc.dg/sinhatanh-1.c
===
--- gcc/testsuite/gcc.dg/sinhatanh-1.c (nonexistent)
+++ gcc/testsuite/gcc.dg/sinhatanh-1.c (working copy)
@@ -0,0 +1,62 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-optimized" } */
+
+extern float sinhf (float);
+extern float coshf (float);
+extern float atanhf (float);
+extern float sqrtf (float);
+extern double sinh (double);
+extern double cosh (double);
+extern double atanh (double);
+extern double sqrt (double);
+extern long double sinhl (long double);
+extern long double coshl (long double);
+extern long double atanhl (long double);
+extern long double sqrtl (long double);
+
+double __attribute__ ((noinline))
+sinhatanh_ (double x)
+{
+return sinh (atanh (x));
+}
+
+double __attribute__ ((noinline))
+coshatanh_ (double x)
+{
+return cosh (atanh (x));
+}
+
+float __attribute__ ((noinline))
+sinhatanhf_(float x)
+{
+return sinhf (atanhf (x));
+}
+
+float __attribute__ ((noinline))
+coshatanhf_(float x)
+{
+return coshf (atanhf (x));
+}
+
+long double __attribute__ ((noinline))
+sinhatanhl_ (long double x)
+{
+return sinhl (atanhl (x));
+}
+
+long double __attribute__ ((noinline))
+coshatanhl_ (long double x)
+{
+return coshl (atanhl (x));
+}
+
+/* There must be no calls to sinh, cosh, or atanh */
+/* {dg-final { scan-tree-dump-not "sinh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "cosh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "atanh " "optimized" }} */
+/* {dg-final { scan-tree-dump-not "sinfh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "cosfh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "atanfh " "optimized" }} */
+/* {dg-final { scan-tree-dump-not "sinlh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "coslh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "atanlh " "optimized" }} */
Index: gcc/testsuite/gcc.dg/sinhatanh-2.c
===
--- gcc/testsuite/gcc.dg/sinhatanh-2.c (nonexistent)
+++ gcc/testsuite/gcc.dg/sinhatanh-2.c (working copy)
@@ -0,0 +1,68 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-optimized" } */
+
+extern float sinhf (float);
+extern float coshf (float);
+extern float atanhf (float);
+extern float sqrtf (float);
+extern double sinh (double);
+extern double cosh (double);
+extern double atanh (double);
+extern double sqrt (double);
+extern long double sinhl (long double);
+extern long double coshl (long double);
+extern long double atanhl (long double);
+extern long double sqrtl (long double);
+
+float __attribute__ ((noinline))
+coshatanhf_(float x)
+{
+float atg = atanhf(x);
+return coshf(atg) + atg;
+}
+
+double __attribute__ ((noinline))
+cosatan_(double x)
+{
+double atg = atanh(x);
+return cosh(atg) + atg;
+}
+
+long double __attribute__ ((noinline))
+cosatanl_(long double x)
+{
+long double atg = atanhl(x);
+return coshl(atg) + atg;
+}
+
+float __attribute__ ((noinline))
+sinatanf_(float x)
+{
+float atg = atanhf(x);
+return sinhf(atg) + atg;
+}
+
+double __attribute__ ((noinline))
+sinatan_(double x)
+{
+double atg = atanh(x);
+return sinh(atg) + atg;
+}
+
+long double __attribute__ ((noinline))
+sinatanl_(long double x)
+{
+long double atg = atanhl(x);
+return sinhl(atg) + atg;
+}
+
[gcc-10 PATCH, i386]: Use accessible_reg_set to disable unsupported register sets
Hello!
This patch is based on the discussion in PR88178. Apparently, MIPS
uses accessible_reg_set array to disable register sets, unsupported by
currently active ISAs. The patch implements the same approach for x86,
which also results in IMO better error messages. The middle-end misses
to error out in asm having inaccessible clobber registers, so the
patch implements the missing handling (the error message could
probably be improved...)
A couple of test cases now emit better diagnostics, e.g.:
* gcc.target/i386/asm-1.c:5:23: error: the register specified for
'EAX' cannot be accessed by the current target
* gcc.target/i386/pr62120.c:6:16: error: the register specified for
'zmm_var' cannot be accessed by the current target
* gcc.target/i386/pr62120.c:7:16: error: the register specified for
'zmm_var2' cannot be accessed by the current target
instead of "invalid register name" messages.
2018-11-27 Uros Bizjak
* cfgexpand.c (expand_asm_stmt): Reject clobbers outside of
accessible_reg_set.
* config/i386/i386.c (ix86_conditional_register_usage):
Disable register sets by clearing corresponding bits in
accessible_reg_set. Do not set corresponding bits in fixed_regs,
call_used_regs and don't clear corresponding reg_names array members.
Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
This is gcc-10 material, so the patch is not committed to the repository.
Uros.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 21bdcdaeaa35..691e0c7c1b0b 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2981,6 +2981,14 @@ expand_asm_stmt (gasm *stmt)
regname);
return;
}
+ else if (!in_hard_reg_set_p
+(accessible_reg_set, reg_raw_mode[reg], reg))
+ {
+ /* ??? Diagnose during gimplification? */
+ error ("the register %qs cannot be clobbered in %"
+ " for the current target", regname);
+ return;
+ }
SET_HARD_REG_BIT (clobbered_regs, reg);
rtx x = gen_rtx_REG (reg_raw_mode[reg], reg);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 95abde95f89d..9abd441930f4 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4743,15 +4743,15 @@ ix86_conditional_register_usage (void)
if (!fixed_regs[i] && !ix86_function_value_regno_p (i))
call_used_regs[i] = 0;
- /* For 32-bit targets, squash the REX registers. */
+ /* For 32-bit targets, disable the REX registers. */
if (! TARGET_64BIT)
{
for (i = FIRST_REX_INT_REG; i <= LAST_REX_INT_REG; i++)
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+ CLEAR_HARD_REG_BIT (accessible_reg_set, i);
for (i = FIRST_REX_SSE_REG; i <= LAST_REX_SSE_REG; i++)
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+ CLEAR_HARD_REG_BIT (accessible_reg_set, i);
for (i = FIRST_EXT_REX_SSE_REG; i <= LAST_EXT_REX_SSE_REG; i++)
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+ CLEAR_HARD_REG_BIT (accessible_reg_set, i);
}
/* See the definition of CALL_USED_REGISTERS in i386.h. */
@@ -4773,32 +4773,29 @@ ix86_conditional_register_usage (void)
SET_HARD_REG_BIT (reg_class_contents[(int)CLOBBERED_REGS], i);
}
- /* If MMX is disabled, squash the registers. */
+ /* If MMX is disabled, disable the registers. */
if (! TARGET_MMX)
-for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (TEST_HARD_REG_BIT (reg_class_contents[(int)MMX_REGS], i))
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+AND_COMPL_HARD_REG_SET (accessible_reg_set,
+ reg_class_contents[(int) MMX_REGS]);
- /* If SSE is disabled, squash the registers. */
+ /* If SSE is disabled, disable the registers. */
if (! TARGET_SSE)
-for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (TEST_HARD_REG_BIT (reg_class_contents[(int)SSE_REGS], i))
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+AND_COMPL_HARD_REG_SET (accessible_reg_set,
+ reg_class_contents[(int) ALL_SSE_REGS]);
- /* If the FPU is disabled, squash the registers. */
+ /* If the FPU is disabled, disable the registers. */
if (! (TARGET_80387 || TARGET_FLOAT_RETURNS_IN_80387))
-for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (TEST_HARD_REG_BIT (reg_class_contents[(int)FLOAT_REGS], i))
- fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = "";
+AND_COMPL_HARD_REG_SET (accessible_reg_set,
+ reg_class_contents[(int) FLOAT_REGS]);
- /* If AVX512F is disabled, squash the registers. */
+ /* If AVX512F is disabled, disable the registers. */
if (! TARGET_AVX512F)
{
for (i = FIRST_EXT_REX_SSE_REG; i <= LAST_EXT_REX_SSE_REG; i++)
- fixed_reg
Re: [testsuite] Require ucn support in gdc.test/compilable/ddoc12.d (PR d/88039)
Hi Mike, > On Nov 27, 2018, at 2:18 AM, Rainer Orth > wrote: >> >> Some assemblers, including the Solaris one, don't support UTF-8 >> identifiers, which breaks the gdc.test/compilable/ddoc12.d testcase as >> reported in the PR. > > So, another style of fix, would be to change the binding from the language > front-end to encode unsupported symbols using a fixed, documented, abi > defined technique. there's been some discussion on this in the PR. Joseph's suggestion was to follow the system compilers if this were done, and indeed they do encode UTF-8 identifiers in some way which could either be reverse-engineered or a spec obtained. However, given Iain's stance towards UTF-8 identifiers in D, I very much doubt this is worth the effort. Ultimately, it's his call, of course. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Fix vect/costmodel/ppc/costmodel-vect-33.c testcase (PR middle-end/87157)
On Tue, Nov 27, 2018 at 04:47:29PM +0100, Jakub Jelinek wrote: > This testcase started FAILing, because function splitting recently started > splitting main1 into inline containing the cheap loop and main1.part.0 which > contains the verification in abort. I believe the test is meant to test > the vectorizer behavior, not how many times that loop has been inlined > somewhere. This patch arranges for main1 not to be inlined or split. > > Tested on powerpc64le-linux, ok for trunk? This is fine, thanks! Segher > 2018-11-27 Jakub Jelinek > > PR middle-end/87157 > * gcc.dg/vect/costmodel/ppc/costmodel-vect-33.c (main1): Add noipa > attribute.
Re: [testsuite] Require ucn support in gdc.test/compilable/ddoc12.d (PR d/88039)
On Nov 27, 2018, at 2:18 AM, Rainer Orth wrote: > > Some assemblers, including the Solaris one, don't support UTF-8 > identifiers, which breaks the gdc.test/compilable/ddoc12.d testcase as > reported in the PR. So, another style of fix, would be to change the binding from the language front-end to encode unsupported symbols using a fixed, documented, abi defined technique.
Re: [PATCH] Added information about inline assembler in stack calculations (.su files)
Hi! On Mon, Nov 26, 2018 at 02:02:49PM +, Torbjorn SVENSSON wrote: > Attached is a small patch that, in case of inline assembler code, > indicates that the function stack usage is uncertain due to inline > assembler. > > The test suite are using "nop" as an assembler instruction on all > targets, is this acceptable or is there a better way to test this? Maybe see testsuite/gcc.dg/nop.h ? Segher
Re: FIO, powerpc-darwin mangling patch [7.x and earlier].
On Nov 27, 2018, at 7:05 AM, Iain Sandoe wrote: > > So it turns out that the Darwin PPC port was broken essentially “forever” > (before the tidy-up in 8.2) with respect to mangling the symbols for __ibm128 > type (the default long double for the port). > > In 7.x and earlier (at least back to Apple’s 4.2.1) the use passes right > through rs6000_mangle_type, which causes it to mangle to ‘e’ - which is the > representation for long double as a 64b value (-mlong-double-64). > > This is fixable, even quite easily, but I think it’s better to have the break > in the upstream sources on a major boundary (so we leave it alone and have > the correction in 8+) So, I'll pre-approve the back port to all active gcc release branches for anyway that cares. This is the perfect type of fix to back port in my book.
Go patch committed: Change Expression export to use Export_function_body
This patch to the Go frontend changes Expression export to use
Export_function_body instead of Export. This is in preparation for
writing expressions to inline function bodies. This is a refactoring
that doesn't affect compiler output. Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266517)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-f551ab95f46c3d7bb7c032711e10b03bfa995ee2
+db5240278b3b62a919dd88f857e718a66be50346
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/export.cc
===
--- gcc/go/gofrontend/export.cc (revision 266510)
+++ gcc/go/gofrontend/export.cc (working copy)
@@ -936,21 +936,41 @@ Export::write_unsigned(unsigned value)
this->write_c_string(buf);
}
-// Export a type.
+// Return the index of a type.
-void
-Export::write_type(const Type* type)
+int
+Export::type_index(const Type* type)
{
type = type->forwarded();
Type_refs::const_iterator p = type_refs.find(type);
go_assert(p != type_refs.end());
int index = p->second;
go_assert(index != 0);
+ return index;
+}
+
+// Export a type.
+
+void
+Export::write_type(const Type* type)
+{
+ int index = this->type_index(type);
char buf[30];
snprintf(buf, sizeof buf, "", index);
this->write_c_string(buf);
}
+// Export a type to a function body.
+
+void
+Export::write_type_to(const Type* type, Export_function_body* efb)
+{
+ int index = this->type_index(type);
+ char buf[30];
+ snprintf(buf, sizeof buf, "", index);
+ efb->write_c_string(buf);
+}
+
// Export escape note.
void
Index: gcc/go/gofrontend/export.h
===
--- gcc/go/gofrontend/export.h (revision 266510)
+++ gcc/go/gofrontend/export.h (working copy)
@@ -12,6 +12,7 @@
class Go_sha1_helper;
class Gogo;
class Named_object;
+class Export_function_body;
class Import_init;
class Named_object;
class Bindings;
@@ -183,6 +184,10 @@ class Export : public String_dump
void
write_type(const Type*);
+ // Write a type to an exported function body.
+ void
+ write_type_to(const Type*, Export_function_body*);
+
// Write the escape note to the export stream. If NOTE is NULL, write
// nothing.
void
@@ -241,6 +246,10 @@ class Export : public String_dump
void
register_builtin_type(Gogo*, const char* name, Builtin_code);
+ // Return the index of a type in the export data.
+ int
+ type_index(const Type*);
+
// The stream to which we are writing data.
Stream* stream_;
// Index number of next type.
@@ -290,11 +299,11 @@ class Stream_to_string : public Export::
// to Statements and Expressions. It builds up the export data for
// the function.
-class Export_function_body
+class Export_function_body : public String_dump
{
public:
- Export_function_body(int indent)
-: indent_(indent)
+ Export_function_body(Export* exp, int indent)
+: exp_(exp), indent_(indent)
{ }
// Write a character to the body.
@@ -312,6 +321,11 @@ class Export_function_body
write_string(const std::string& str)
{ this->body_.append(str); }
+ // Write a type reference to the body.
+ void
+ write_type(const Type* type)
+ { this->exp_->write_type_to(type, this); }
+
// Append as many spaces as the current indentation level.
void
indent()
@@ -336,6 +350,8 @@ class Export_function_body
{ return this->body_; }
private:
+ // The overall export data.
+ Export* exp_;
// The body we are building.
std::string body_;
// Current indentation level: the number of spaces before each statement.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266517)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -82,7 +82,7 @@ Expression::do_discarding_value()
// only be used by expressions which may be constant.
void
-Expression::do_export(Export*) const
+Expression::do_export(Export_function_body*) const
{
go_unreachable();
}
@@ -1609,8 +1609,8 @@ class Boolean_expression : public Expres
{ return context->backend()->boolean_constant_expression(this->val_); }
void
- do_export(Export* exp) const
- { exp->write_c_string(this->val_ ? "true" : "false"); }
+ do_export(Export_function_body* efb) const
+ { efb->write_c_string(this->val_ ? "true" : "false"); }
void
do_dump_expression(Ast_dump_context* ast_dump_context) const
@@ -1760,9 +1760,9 @@ String_expression::export_string(String_
// Export a string expression.
void
-String_expression::do_export(Export* exp) const
+String_expression::do_export(Export_function_body* efb) const
{
- String_expression::export_string(exp, thi
Re: [PATCH 5/7][MSP430][TESTSUITE] Prune messages about ISO C not supporting __int20 from output of tests
On Nov 26, 2018, at 3:07 PM, Jozef Lawrynowicz wrote: > > On Mon, 26 Nov 2018 12:51:26 -0800 > Mike Stump wrote: > >> On Nov 14, 2018, at 7:56 AM, Jozef Lawrynowicz >> wrote: >>> >>> Patch 5 deals with ISO C errors emitted by tests when the large memory >>> model is >>> used. size_t and ptrdiff_t are __int20 with -mlarge, and if the test is >>> compiled with -pedantic-errors and -std=* or -ansi, then use of these types >>> causes an error of the form: >>> ISO C does not support __int20 types >>> I fixed this by adding dg-prune-output directives to tests which cause this >>> error. >> >> So, it is important that standard code not produce errors. Kinda >> fundamental. >> >> I think this should be fixed in some other way. If a type is to be used as >> a standard type, producing an error for that type's use is wrong. Instead, >> find a way (cough, __extension__), to mark or not warn (error) for it >> instead. > > Thanks for the feedback. > >> Can you fix __SIZE_TYPE__ itself to have __extension__ in it? If not, then >> I'd find another way to remove that warning for the type. > > It appears you can actually add __extension__ everywhere (that I tried), > except > for the argument types in the declaration of a function i.e. > foo (__extension__ __SIZE_TYPE__ a) > doesn't work. Ouch. Seems like a bug in the parsers. Could you file a bug report for this against C and C++. Once that bug is fixed, then you can just add __extension__ to SIZE_TYPE in your port file. > So I added __extension__ where possible, and in a couple of other cases I > instead typedef'd with __extension__. > Ok for trunk? No. I mean, __extension__ should be added in SIZE_TYPE in your port.h file. That, or, you need a type that won't complain when used. Recall, you're not doing the port for the testsuite, you're doing the port for users. Users don't want a ton of warnings or errors when compiling trivial standard code.
Re: [PATCH/coding style] clarify pointers and operators
On Mon, Nov 26, 2018 at 01:41:07PM -0700, Jeff Law wrote: > On 11/26/18 10:59 AM, Martin Sebor wrote: > > As an aside, regarding the space convention in casts: a crude > > grep search yields about 10,000 instances of the "(type)x" kinds > > of casts in GCC sources and 40,000 of the preferred "(type) x" > > style with the space. That's a consistency of only 80%. Is > > it worth documenting a preference for a convention that's so > > inconsistently followed? > Please do. It's a fairly recent change -- I suspect some old code was > never fixed and some folks (perhaps myself) have that extraneous > whitespace in their muscle memory and still need to eliminate it. Huh? Spaces after casts are required, and make things much more readable. This isn't recent. A lot of old code writes spaces after single-character unary operators, too, which is ugly and less readable. It's not recent that this is explicitly documented as wrong, either. Segher
Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.
Hi Mark, On Mon, Nov 26, 2018 at 10:13:29AM +0100, Mark Wielaard wrote: > With -Wtrampolines a warning is produced whenever gcc generates executable > code on the stack at runtime to support taking a nested function address > that is used to call the nested function indirectly when it needs to access > any variables in its lexical scope. > > As a result the stack has to be marked as executable even for targets which > have a non-executable stack as default. > > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets > that have a non-executable default stack based on when they call > file_end_indicate_exec_stack. As Paul says, that name isn't so good. TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or similar? > diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h > index 0c67634..9330acf 100644 > --- a/gcc/config/rs6000/sysv4.h > +++ b/gcc/config/rs6000/sysv4.h > @@ -972,6 +972,11 @@ ncrtn.o%s" > /* Generate entries in .fixup for relocatable addresses. */ > #define RELOCATABLE_NEEDS_FIXUP 1 > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD) > + #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \ > +|| DEFAULT_ABI == ABI_ELFv2) > +#endif I don't think this belongs in sysv4.h . Segher
Re: [PING][PATCH] correct handling of EXCESS_PRECISION_EXPR in function calls (PR 88091)
On Mon, 26 Nov 2018, Martin Sebor wrote: > Ping: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01674.html This patch needs an update to the comment on convert_argument to explain the semantics of the new valtype parameter and how it differs from the previously used TREE_TYPE (val). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 6/6] [RS6000] inline plt call sequences
Hi Alan,
On Tue, Nov 13, 2018 at 11:23:43PM +1030, Alan Modra wrote:
> Finally, the point of the previous patches in this series, support for
> inline PLT calls, keyed off -fno-plt. This emits code using new
> relocations that tie all insns in the sequence together, so that the
> linker can edit the sequence back to a direct call should the call
> target turn out to be local. An example of ELFv2 code to call puts is
> as follows:
>
> .reloc .,R_PPC64_PLTSEQ,puts
> std 2,24(1)
> .reloc .,R_PPC64_PLT16_HA,puts
> addis 12,2,0
> .reloc .,R_PPC64_PLT16_LO_DS,puts
> ld 12,0(12)
> .reloc .,R_PPC64_PLTSEQ,puts
> mtctr 12
> .reloc .,R_PPC64_PLTCALL,puts
> bctrl
> ld 2,24(1)
>
> "addis 12,2,puts@plt@ha" and "ld 12,puts@plt@l(12)" are also supported
> by the assembler. gcc instead uses the explicit R_PPC64_PLT16_HA and
> R_PPC64_PLT16_LO_DS relocs because when the call is to __tls_get_addr
> an extra reloc is emitted at every place where one is shown above, to
> specify the __tls_get_addr arg. The linker expects the extra reloc to
> come first. .reloc enforces that ordering.
>
> The patch also changes code emitted for longcalls if the assembler
> supports the new marker relocs, so that these too can be edited. One
> side effect of longcalls using PLT16 relocs is that they can now be
> resolved lazily by ld.so.
>
> I don't support lazy inline PLT calls for ELFv1, because ELFv1 would
> need barriers to reliably load both the function address and toc
> pointer from the PLT. ELFv1 -fno-plt uses the longcall sequence
> instead, which isn't edited by GNU ld.
That all sounds great :-) Has this been tested with older binutils, too?
> * config.in (HAVE_AS_PLTSEQ): Add.
> * config/rs6000/predicates.md (indirect_call_operand): New.
> * config/rs6000/rs6000-protos.h (rs6000_pltseq_template),
> (rs6000_sibcall_sysv): Declare.
> * config/rs6000/rs6000.c (init_cumulative_args): Set cookie
> CALL_LONG for -fno-plt.
> (print_operand ): Handle UNSPEC_PLTSEQ.
> (rs6000_indirect_call_template_1): Emit .reloc directives for
> UNSPEC_PLTSEQ calls.
> (rs6000_pltseq_template): New function.
> (rs6000_longcall_ref): Add arg parameter. Use PLT16 insns if
> relocs supported by assembler. Move SYMBOL_REF test to callers.
> (rs6000_call_aix): Adjust rs6000_longcall_ref call. Package
> insns in UNSPEC_PLTSEQ, preserving original func_desc.
> (rs6000_call_sysv): Likewise.
> (rs6000_sibcall_sysv): New function.
> * config/rs6000/rs6000.h (HAVE_AS_PLTSEQ): Provide default.
> * config/rs6000/rs6000.md (UNSPEC_PLTSEQ, UNSPEC_PLT16_HA,
> UNSPEC_PLT16_LO): New.
> (pltseq_tocsave, pltseq_plt16_ha, pltseq_plt16_lo, pltseq_mtctr): New.
> (call_indirect_nonlocal_sysv): Don't differentiate zero from non-zero
> cookie in constraints. Test explicitly for flags in length attr.
> Handle unspec operand 1.
> (call_value_indirect_nonlocal_sysv): Likewise.
> (call_indirect_aix, call_value_indirect_aix): Handle unspec operand 1.
> (call_indirect_elfv2, call_value_indirect_elfv2): Likewise.
> (sibcall, sibcall_value): Use rs6000_sibcall_sysv.
> (sibcall_indirect_nonlocal_sysv): New pattern.
> (sibcall_value_indirect_nonlocal_sysv): Likewise.
> (sibcall_nonlocal_sysv, sibcall_value_nonlocal_sysv): Remove indirect
> call alternatives.
> * configure.ac: Check for gas plt sequence marker support.
> * configure: Regenerate.
> @@ -10727,10 +10727,20 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree
> fntype,
> cum->nargs_prototype = n_named_args;
>
>/* Check for a longcall attribute. */
> - if ((!fntype && rs6000_default_long_calls)
> - || (fntype
> - && lookup_attribute ("longcall", TYPE_ATTRIBUTES (fntype))
> - && !lookup_attribute ("shortcall", TYPE_ATTRIBUTES (fntype
> + if (((!fntype && rs6000_default_long_calls)
> + || (fntype
> +&& lookup_attribute ("longcall", TYPE_ATTRIBUTES (fntype))
> +&& !lookup_attribute ("shortcall", TYPE_ATTRIBUTES (fntype
> + || (DEFAULT_ABI != ABI_DARWIN
> + && !(fndecl
> +&& !DECL_EXTERNAL (fndecl)
> +&& !DECL_WEAK (fndecl)
> +&& (*targetm.binds_local_p) (fndecl))
> + && (flag_plt
> + ? (fntype
> + && lookup_attribute ("noplt", TYPE_ATTRIBUTES (fntype)))
> + : !(fntype
> + && lookup_attribute ("plt", TYPE_ATTRIBUTES (fntype))
> cum->call_cookie |= CALL_LONG;
Could you split this into a few cases? Maybe with some extra locals. It
is hard to understand the code as it is.
I mean like
if (...)
cum->call_cookie |= CALL_LONG;
if (...)
cum->call_cookie |= CALL_LONG;
if (...)
cum->call_cookie |= CALL_LONG;
> +(define_insn "*sibcall_indirect_nonlocal_sysv"
> + (se
Re: [PATCH] detect missing nuls in address of const char (PR 87756)
On 11/26/18 10:52 AM, Rainer Orth wrote: Hi Martin, I have now committed this patch as r266418. this patch has created a bunch of XPASSes everywhere: +XPASS: gcc.dg/tree-ssa/builtin-fprintf-warn-1.c pr87756 (test for warnings, line 119) +XPASS: gcc.dg/tree-ssa/builtin-fprintf-warn-1.c pr87756 (test for warnings, line 120) +XPASS: gcc.dg/tree-ssa/builtin-printf-warn-1.c pr87756 (test for warnings, line 116) +XPASS: gcc.dg/tree-ssa/builtin-printf-warn-1.c pr87756 (test for warnings, line 117) +XPASS: gcc.dg/tree-ssa/builtin-printf-warn-1.c pr87756 (test for warnings, line 84) +XPASS: gcc.dg/tree-ssa/user-printf-warn-1.c pr87756 (test for warnings, line 110) +XPASS: gcc.dg/tree-ssa/user-printf-warn-1.c pr87756 (test for warnings, line 142) +XPASS: gcc.dg/tree-ssa/user-printf-warn-1.c pr87756 (test for warnings, line 143) Thanks for the heads up. I like those much better than FAILs :) I've removed the passing xfails from the tests via r266522 and replaced the remaining with references to the new bugs I created for the outstanding gaps: 88226 and 88211. Martin
Re: [PATCH] Fix PR78444.
On Tue, Nov 27, 2018 at 5:17 PM Iain Sandoe wrote:
>
> Hi,
>
> As described in comment #4/5 in the PR, there are [corner] cases where the
> the compiler incorrectly concludes that only 64b alignment is needed at a
> call site. This is caught by Darwin’s dynamic linker which enforces the ABI
> requirement for 128b alignment, causing the exe be aborted.
>
> With this fix, we no longer need the work-around that was applied when
> profiling was on, so we can remove the special-casing there.
>
> The attached patch has been tested on x86_64-darwin and x86_64-linux-gnu.
>
> The problem exists on all open branches,
>
> OK for trunk?
>
> Backports?
>
> thanks
> Iain
>
> gcc/
> * config/i386/darwin.h (STACK_BOUNDARY): Remove macro.
> * config/i386/i386.c (ix86_compute_frame_layout): Ensure at least 128b
> stack alignment in non-leaf functions.
OK for mainline and branches after a week or so without problems in mainline.
Thanks,
Uros.
> From e199bce63f952ae1c3f6bffcdc20360aaf4deae8 Mon Sep 17 00:00:00 2001
> From: Iain Sandoe
> Date: Fri, 16 Nov 2018 16:25:47 +
> Subject: [PATCH] [patch] Fix PR78444, update stack alignment for non-leaf
> functions.
>
> ---
> gcc/config/i386/darwin.h | 3 ---
> gcc/config/i386/i386.c | 10 --
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
> index 735abeddb2..64133aa8dc 100644
> --- a/gcc/config/i386/darwin.h
> +++ b/gcc/config/i386/darwin.h
> @@ -85,9 +85,6 @@ extern int darwin_emit_branch_islands;
> /* On Darwin, the stack is 128-bit aligned at the point of every call.
> Failure to ensure this will lead to a crash in the system libraries
> or dynamic loader. */
> -#undef STACK_BOUNDARY
> -#define STACK_BOUNDARY \
> - ((profile_flag || TARGET_64BIT_MS_ABI) ? 128 : BITS_PER_WORD)
>
> #undef MAIN_STACK_BOUNDARY
> #define MAIN_STACK_BOUNDARY 128
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index ef58533344..c88617e3c1 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -11161,10 +11161,16 @@ ix86_compute_frame_layout (void)
>/* 64-bit MS ABI seem to require stack alignment to be always 16,
> except for function prologues, leaf functions and when the defult
> incoming stack boundary is overriden at command line or via
> - force_align_arg_pointer attribute. */
> - if ((TARGET_64BIT_MS_ABI && crtl->preferred_stack_boundary < 128)
> + force_align_arg_pointer attribute.
> +
> + Darwin's ABI specifies 128b alignment for both 32 and 64 bit variants
> + at call sites, including profile function calls.
> + */
> + if (((TARGET_64BIT_MS_ABI || TARGET_MACHO)
> +&& crtl->preferred_stack_boundary < 128)
>&& (!crtl->is_leaf || cfun->calls_alloca != 0
> || ix86_current_function_calls_tls_descriptor
> + || (TARGET_MACHO && crtl->profile)
> || ix86_incoming_stack_boundary < 128))
> {
>crtl->preferred_stack_boundary = 128;
> --
> 2.17.1
>
>
Re: [committed] sh and riscv fixes for move_by_pieces change
On 11/27/18 4:34 PM, Jeff Law wrote: > > sh and riscv this time. Sigh. Hopefully that's all the ports... These > will also test via bootstraps during the day. > > > Committed. > > Jeff > Thank you Jeff for the help. I grepped for a usage in a target code, but apparently wrongly. Sorry. Martin
Re: [PATCH 5/6] [RS6000] Use standard call patterns for __tls_get_addr calls
Hi!
On Tue, Nov 13, 2018 at 11:22:43PM +1030, Alan Modra wrote:
> Version 2.
>
> The current code handling __tls_get_addr calls for powerpc*-linux
> generates a call then overwrites the call insn with a special
> tls_{gd,ld}_{aix,sysv} pattern. It's done that way to support
> !TARGET_TLS_MARKERS, where the arg setup insns need to be emitted
> immediately before the branch and link. When TARGET_TLS_MARKERS, the
> arg setup insns are split from the actual call, but we then have a
> non-standard call pattern that needs to be carried through to output.
>
> This patch changes that scheme, to instead use the standard call
> patterns for __tls_get_addr calls, except for the now rare
> !TARGET_TLS_MARKERS case. Doing it this way should be better for
> maintenance as the !TARGET_TLS_MARKERS code can eventually disappear.
> It also makes it possible to support longcalls (and in following
> patches, inline plt calls) for __tls_get_addr without introducing yet
> more special call patterns.
>
> __tls_get_addr calls do however need to be different to standard
> calls, because when TARGET_TLS_MARKERS the calls are decorated with an
> argument specifier, eg. "bl __tls_get_addr(thread_var@tlsgd)" that
> causes a reloc to be emitted by the assembler tying the call to its
> arg setup insns. I chose to smuggle the arg in the currently unused
> stack size rtl.
>
> I've also introduced rs6000_call_sysv to generate rtl for sysv calls,
> as rs6000_call_aix does for aix and elfv2 calls. This allows
> rs6000_longcall_ref to be local to rs6000.c since the calls in the
> expanders never did anything for darwin.
>
> * config/rs6000/predicates.md (unspec_tls): New.
> * config/rs6000/rs6000-protos.h (rs6000_call_template),
> (rs6000_sibcall_template): Update prototype.
> (rs6000_longcall_ref): Delete.
> (rs6000_call_sysv): Declare.
> * config/rs6000/rs6000.c (edit_tls_call_insn): New function.
> (global_tlsarg): New variable.
> (rs6000_legitimize_tls_address): Rewrite __tls_get_addr call
> handling.
> (print_operand): Extract UNSPEC_TLSGD address operand.
> (rs6000_call_template, rs6000_sibcall_template): Remove arg
> parameter, extract from second call operand instead.
> (rs6000_longcall_ref): Make static, localize vars.
> (rs6000_call_aix): Rename parameter to reflect new usage. Take
> tlsarg from global_tlsarg. Don't create unused rtl or nop insns.
> (rs6000_sibcall_aix): Rename parameter to reflect new usage. Take
> tlsarg from global_tlsarg.
> (rs6000_call_sysv): New function.
> * config/rs6000/rs6000.md: Adjust rs6000_call_template and
> rs6000_sibcall_template throughout.
> (tls_gd_aix, tls_gd_sysv, tls_gd_call_aix, tls_gd_call_sysv): Delete.
> (tls_ld_aix, tls_ld_sysv, tls_ld_call_aix, tls_ld_call_sysv): Delete.
> (tls_gdld_aix, tls_gdld_sysv): New insns, replacing above.
> (tls_gd): Swap operand order. Simplify mode selection.
> (tls_gd_high, tls_gd_low): Swap operand order.
> (tls_ld): Remove const_int 0 vector element from UNSPEC_TLSLD.
> Simplify mode selection.
> (tls_ld_high, tls_ld_low): Similarly adjust UNSPEC_TLSLD.
> (call, call_value): Don't assert for second call operand.
> Use rs6000_call_sysv.
> +/* Passes the tls arg value for global dynamic and local dynamic
> + emit_library_call_value in rs6000_legitimize_Tls_address to
> + rs6000_call_aix and rs6000_call_sysv. This is used to emit the
> + marker relocs put on __tls_get_addr calls. */
> +static rtx global_tlsarg;
Typo (s/_Tls/_tls/).
> +(define_insn "*tls_gdld_aix"
> + [(match_parallel 3 ""
A match_parallel without predicate... Does this work?! Does this not
accidentally pick up the wrong things?
Do you think we should to deprecate -mtls-markers in GCC 9?
Please test with -mtls-markers, too, if you can, and test on AIX.
Looks fine. Thank you for the cleanup! Okay for trunk, but please do the
extra testing.
Segher
[PATCH] gcov: do not ICE on NULL string in JSON export.
Hi.
One obvious fix that can be triggered:
gcov asdnkjasndjkn
asdnkjasndjkn.gcno:cannot open notes file
asdnkjasndjkn.gcda:cannot open data file, assuming not executed
gcov: internal compiler error: in string, at json.cc:159
0x4038e7 json::string::string(char const*)
/home/marxin/Programming/gcc/gcc/json.cc:159
0x4038e7 json::string::string(char const*)
/home/marxin/Programming/gcc/gcc/json.cc:157
0x40b2b3 generate_results
/home/marxin/Programming/gcc/gcc/gcov.c:1402
I'm going to install the patch.
Martin
gcc/ChangeLog:
2018-11-27 Martin Liska
* gcov.c (generate_results): Append current_working_directory
only when exists.
---
gcc/gcov.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 5fb83c08179..23d75f89265 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -1399,7 +1399,9 @@ generate_results (const char *file_name)
json::object *root = new json::object ();
root->set ("format_version", new json::string ("1"));
root->set ("gcc_version", new json::string (version_string));
- root->set ("current_working_directory", new json::string (bbg_cwd));
+
+ if (bbg_cwd != NULL)
+root->set ("current_working_directory", new json::string (bbg_cwd));
json::array *json_files = new json::array ();
root->set ("files", json_files);
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
On Mon, 2018-11-26 at 17:35 +0100, Martin Liška wrote: > On 11/26/18 5:19 PM, Matthias Klose wrote: > > On 26.11.18 13:20, Martin Liška wrote: > > > On 11/23/18 7:08 PM, Joseph Myers wrote: > > > > In the multiarch case, do you want > > > > /include/finclude/ or > > > > /include//finclude? (This is where I'd > > > > hope Debian > > > > / Ubuntu GCC people would comment.) > > > > > > Mathias can you please reply to this? > > > > this should not matter, as long as you use the multilib name, and the > > correct > > directory is used with the -m32/-m64/-mabi options. Are other compilers > > like a > > clang based compiler supposed to access this directory as well? > > I don't think so. > > > In that case > > the include directories should be documented. Why wouldn't clang (flang) want to use the same mechanism as GCC/gfortran? I know there is some interest/work going on here for flang and we would like a consistent way to use pre-includes to define SIMD vector functions in both gfortran and flang. I think this should be documented so flang and other compilers can use it. Even if no other compilers did use it I think it should be documented because it crosses project/package boundries, i.e. it is created by glibc and used by gfortran. Steve Ellcey sell...@cavium.com
Re: [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location
On 11/27/18 11:07 AM, David Malcolm wrote: > On Tue, 2018-11-27 at 09:53 -0500, Mike Gulick wrote: >> 2018-11-27 Mike Gulick >> >> PR preprocessor/83173 >> * libcpp/files.c (_cpp_stack_include): Check if >> line_table->highest_location is past current line before >> decrementing. > > I've committed these patches to trunk as r266516, r266518, and r266520 > respectively. > > Thanks for all your work on this (and your patience; sorry about the > delays in reviewing it) > > Dave > Hi Dave, Thanks so much for your time and help in reviewing these patches. I'm glad to see this bug finally fixed (coincidentally exactly a year after I reported it)! Thanks, Mike
[PATCH] Fix PR78444.
Hi,
As described in comment #4/5 in the PR, there are [corner] cases where the the
compiler incorrectly concludes that only 64b alignment is needed at a call
site. This is caught by Darwin’s dynamic linker which enforces the ABI
requirement for 128b alignment, causing the exe be aborted.
With this fix, we no longer need the work-around that was applied when
profiling was on, so we can remove the special-casing there.
The attached patch has been tested on x86_64-darwin and x86_64-linux-gnu.
The problem exists on all open branches,
OK for trunk?
Backports?
thanks
Iain
gcc/
* config/i386/darwin.h (STACK_BOUNDARY): Remove macro.
* config/i386/i386.c (ix86_compute_frame_layout): Ensure at least 128b
stack alignment in non-leaf functions.
From e199bce63f952ae1c3f6bffcdc20360aaf4deae8 Mon Sep 17 00:00:00 2001
From: Iain Sandoe
Date: Fri, 16 Nov 2018 16:25:47 +
Subject: [PATCH] [patch] Fix PR78444, update stack alignment for non-leaf
functions.
---
gcc/config/i386/darwin.h | 3 ---
gcc/config/i386/i386.c | 10 --
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index 735abeddb2..64133aa8dc 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -85,9 +85,6 @@ extern int darwin_emit_branch_islands;
/* On Darwin, the stack is 128-bit aligned at the point of every call.
Failure to ensure this will lead to a crash in the system libraries
or dynamic loader. */
-#undef STACK_BOUNDARY
-#define STACK_BOUNDARY \
- ((profile_flag || TARGET_64BIT_MS_ABI) ? 128 : BITS_PER_WORD)
#undef MAIN_STACK_BOUNDARY
#define MAIN_STACK_BOUNDARY 128
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ef58533344..c88617e3c1 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11161,10 +11161,16 @@ ix86_compute_frame_layout (void)
/* 64-bit MS ABI seem to require stack alignment to be always 16,
except for function prologues, leaf functions and when the defult
incoming stack boundary is overriden at command line or via
- force_align_arg_pointer attribute. */
- if ((TARGET_64BIT_MS_ABI && crtl->preferred_stack_boundary < 128)
+ force_align_arg_pointer attribute.
+
+ Darwin's ABI specifies 128b alignment for both 32 and 64 bit variants
+ at call sites, including profile function calls.
+ */
+ if (((TARGET_64BIT_MS_ABI || TARGET_MACHO)
+&& crtl->preferred_stack_boundary < 128)
&& (!crtl->is_leaf || cfun->calls_alloca != 0
|| ix86_current_function_calls_tls_descriptor
+ || (TARGET_MACHO && crtl->profile)
|| ix86_incoming_stack_boundary < 128))
{
crtl->preferred_stack_boundary = 128;
--
2.17.1
Re: [PATCH/coding style] clarify pointers and operators
On 11/27/18 5:57 AM, Jakub Jelinek wrote: On Mon, Nov 26, 2018 at 10:59:47AM -0700, Martin Sebor wrote: --- htdocs/codingconventions.html 30 Sep 2018 14:38:46 - 1.85 +++ htdocs/codingconventions.html 26 Nov 2018 17:59:21 - @@ -803,12 +803,17 @@ - x cast - (foo) x - (foo)x + (type) x + (type)x - pointer dereference - *x - * x Why have you removed the pointer dereference case, rather than just added the new cases and replaced foo with type for cast? We want people to write *x rather than * x. Whoops, that was unintentional. I just restored it. Martin
Re: [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location
On Tue, 2018-11-27 at 09:53 -0500, Mike Gulick wrote: > 2018-11-27 Mike Gulick > > PR preprocessor/83173 > * libcpp/files.c (_cpp_stack_include): Check if > line_table->highest_location is past current line before > decrementing. I've committed these patches to trunk as r266516, r266518, and r266520 respectively. Thanks for all your work on this (and your patience; sorry about the delays in reviewing it) Dave
Re: [PATCH v4] Repeat jump threading after combine
> Am 26.11.2018 um 16:26 schrieb Ilya Leoshkevich :
>
>> Am 26.11.2018 um 16:07 schrieb Segher Boessenkool
>> :
>>
>>> # ppc64le-redhat-linux:
>>> 511.povray_r -1.29%
>>> 482.sphinx3-0.65%
>>> 456.hmmer -0.53%
>>> 519.lbm_r -0.51%
>>> # skip |dt| < 0.5%
>>> 549.fotonik3d_r+1.13%
>>> 403.gcc+1.76%
>>> 500.perlbench_r+2.35%
>>
>> 2% degradation on gcc and perlbench isn't really acceptable. It is
>> certainly possible this is an uarch effect of indirect jumps and we are
>> just very unlucky now (and were lucky before), but this doesn't sound
>> good to me :-/
>>
>> What did you run this on? p8?
>
> That was p9 (gcc135).
I've had a look at gcc regression with perf. I made a 5x re-run, and
confirmed that the run time grew from 225.76s to 228.82s (+1.4%).
perf stat shows that the slow version consumes additional ~11E+9 cycles:
856,588,095,385 cycles:u #3.740 GHz
(33.33%)
36,451,588,171 stalled-cycles-frontend:u #4.26% frontend cycles
idle (50.01%)
438,654,175,652 stalled-cycles-backend:u # 51.21% backend cycles
idle (16.68%)
937,926,993,826 instructions:u#1.09 insn per cycle
#0.47 stalled cycles per
insn (33.36%)
205,289,212,856 branches:u# 896.253 M/sec
(50.02%)
9,019,757,337 branch-misses:u #4.39% of all branches
(16.65%)
vs
867,688,505,674 cycles:u #3.731 GHz
(33.34%) Δ=11100410289 (+1.29%)
36,672,094,462 stalled-cycles-frontend:u #4.23% frontend cycles
idle (50.02%) Δ= 220506291 (+0.60%)
438,837,922,096 stalled-cycles-backend:u # 50.58% backend cycles
idle (16.68%) Δ= 183746444 (+0.04%)
937,918,212,318 instructions:u#1.08 insn per cycle
#0.47 stalled cycles per
insn (33.37%)
205,201,306,341 branches:u# 882.403 M/sec
(50.02%)
9,072,857,028 branch-misses:u #4.42% of all branches
(16.65%) Δ= 53099691 (+0.58%)
It also shows that the slowdown cannot be explained by pipeline stalls,
additional instructions or branch misses (the latter could still be the
case if a single branch miss somehow translated to 200 cycles on p9).
perf diff -c wdiff:1,1 shows, that there is just one function
(htab_traverse) that is significantly slower now:
2.98% 11768891764 exe[.] htab_traverse
1.91% 563949986 exe[.] compute_dominance_frontiers_1
The additional cycles consumed by this function matches the overall
number of additionaly consumed cycles, and the contribution of the
runner up (compute_dominance_frontiers_1) is 20 times smaller, so I
think it's really just this one function.
However, the generated assembly is completely identical in both cases!
I saw similar situations in the past, so I tried adding a nop to
htab_traverse:
--- hashtab.c
+++ hashtab.c
@@ -529,6 +529,8 @@ htab_traverse (htab, callback, info)
htab_trav callback;
PTR info;
{
+ __asm__ volatile("nop\n");
+
PTR *slot = htab->entries;
PTR *limit = slot + htab->size;
and made a 5x re-run. The new measurements are 227.01s and 227.44s
(+0.19%). With two nops I get 227.25s and 227.29s (+0.02%), which also
looks like noise.
Can this be explained by some microarchitectural quirk after all?
[PATCH] Fix slowness in gcov (PR gcov-profile/88225).
Hi.
The patch is about addition of 2 maps that significantly speed up
gcov for tramp3d from 2.0s to 0.2s.
Survives gcov.exp tests, I'm planning to install the patch in couple
of days.
Martin
gcc/ChangeLog:
2018-11-27 Martin Liska
PR gcov-profile/88225
* gcov.c(source_info::get_functions_at_location):
Use newly added line_to_function_map.
(source_info::add_function): New.
(output_json_intermediate_file): Use a pointer return
type for get_functions_at_location.
(process_all_functions): Use add_function instead
of direct push to a s->functions container.
(release_structures): Release ident_to_fn.
(read_graph_file): Register function into ident_to_fn.
(read_count_file): Use the map.
(output_lines): Handle pointer return type of
get_functions_at_location.
---
gcc/gcov.c | 112 ++---
1 file changed, 64 insertions(+), 48 deletions(-)
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 361b696ea78..5fb83c08179 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -358,7 +358,10 @@ struct source_info
/* Default constructor. */
source_info ();
- vector get_functions_at_location (unsigned line_num) const;
+ vector *get_functions_at_location (unsigned line_num) const;
+
+ /* Register a new function. */
+ void add_function (function_info *fn);
/* Index of the source_info in sources vector. */
unsigned index;
@@ -377,7 +380,10 @@ struct source_info
/* Functions in this source file. These are in ascending line
number order. */
- vector functions;
+ vector functions;
+
+ /* Line number to functions map. */
+ vector *> line_to_function_map;
};
source_info::source_info (): index (0), name (NULL), file_time (),
@@ -385,21 +391,33 @@ source_info::source_info (): index (0), name (NULL), file_time (),
{
}
-vector
-source_info::get_functions_at_location (unsigned line_num) const
+/* Register a new function. */
+void
+source_info::add_function (function_info *fn)
{
- vector r;
+ functions.push_back (fn);
- for (vector::const_iterator it = functions.begin ();
- it != functions.end (); it++)
-{
- if ((*it)->start_line == line_num && (*it)->src == index)
- r.push_back (*it);
-}
+ if (fn->start_line >= line_to_function_map.size ())
+line_to_function_map.resize (fn->start_line + 1);
+
+ vector **slot = &line_to_function_map[fn->start_line];
+ if (*slot == NULL)
+*slot = new vector ();
- std::sort (r.begin (), r.end (), function_line_start_cmp ());
+ (*slot)->push_back (fn);
+}
+
+vector *
+source_info::get_functions_at_location (unsigned line_num) const
+{
+ if (line_num >= line_to_function_map.size ())
+return NULL;
- return r;
+ vector *slot = line_to_function_map[line_num];
+ if (slot != NULL)
+std::sort (slot->begin (), slot->end (), function_line_start_cmp ());
+
+ return slot;
}
class name_map
@@ -438,6 +456,9 @@ public:
/* Vector of all functions. */
static vector functions;
+/* Function ident to function_info * map. */
+static map ident_to_fn;
+
/* Vector of source files. */
static vector sources;
@@ -1121,19 +1142,20 @@ output_json_intermediate_file (json::array *json_files, source_info *src)
for (unsigned line_num = 1; line_num <= src->lines.size (); line_num++)
{
- vector fns = src->get_functions_at_location (line_num);
+ vector *fns = src->get_functions_at_location (line_num);
- /* Print first group functions that begin on the line. */
- for (vector::iterator it2 = fns.begin ();
- it2 != fns.end (); it2++)
- {
- vector &lines = (*it2)->lines;
- for (unsigned i = 0; i < lines.size (); i++)
- {
- line_info *line = &lines[i];
- output_intermediate_json_line (lineso, line, line_num + i);
- }
- }
+ if (fns != NULL)
+ /* Print first group functions that begin on the line. */
+ for (vector::iterator it2 = fns->begin ();
+ it2 != fns->end (); it2++)
+ {
+ vector &lines = (*it2)->lines;
+ for (unsigned i = 0; i < lines.size (); i++)
+ {
+ line_info *line = &lines[i];
+ output_intermediate_json_line (lineso, line, line_num + i);
+ }
+ }
/* Follow with lines associated with the source file. */
if (line_num < src->lines.size ())
@@ -1256,7 +1278,7 @@ process_all_functions (void)
if (!fn->counts.empty () || no_data_file)
{
source_info *s = &sources[src];
- s->functions.push_back (fn);
+ s->add_function (fn);
/* Mark last line in files touched by function. */
for (unsigned block_no = 0; block_no != fn->blocks.size ();
@@ -1473,6 +1495,7 @@ release_structures (void)
sources.resize (0);
names.resize (0);
functions.resize (0);
+ ident_to_fn.clear ();
}
/* Generate the names of the graph and data files. If OBJECT_DIRECTORY
@@ -1691,6 +1714,8 @@ read_graph_file (void)
fn = new function_info ();
functions.push_back (f
Re: [PATCH v2] MIPS: Add `-mfix-r5900' option for the R5900 short loop erratum
On Fri, 9 Nov 2018, Fredrik Noring wrote: > gcc/ > * config/mips/mips.c (mips_reorg_process_insns) > (mips_option_override): Handle `-mfix-r5900'. > * config/mips/mips.h (ASM_SPEC): Add `mfix-r5900' and > `mno-fix-r5900'. > * config/mips/mips.opt (mfix-r5900): New option. > * doc/invoke.texi: Document the `r5900' processor name, and > `-mfix-r5900' and `-mno-fix-r5900' options. > --- FSF has just confirmed your copyright assignment, so I have applied your change now. Thank you for your contribution to the GNU project! Maciej
Go patch committed: Import inlinable functions from package export data
This patch to the Go frontend imports inlinable functions from other
packages, when the body is recorded in the package export data. At
this point we will inline direct calls to empty functions and methods
defined in different packages. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline.
Ian
2018-11-27 Ian Lance Taylor
* go-gcc.cc (Gcc_backend::function): Handle function_only_inline
flag.
Index: gcc/go/go-gcc.cc
===
--- gcc/go/go-gcc.cc(revision 266510)
+++ gcc/go/go-gcc.cc(working copy)
@@ -3086,6 +3086,11 @@ Gcc_backend::function(Btype* fntype, con
TREE_THIS_VOLATILE(decl) = 1;
if ((flags & function_in_unique_section) != 0)
resolve_unique_section(decl, 0, 1);
+ if ((flags & function_only_inline) != 0)
+{
+ DECL_EXTERNAL(decl) = 1;
+ DECL_DECLARED_INLINE_P(decl) = 1;
+}
go_preserve_from_gc(decl);
return new Bfunction(decl);
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266510)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-3ecc845c337c15d9a19ed8d277e5ee9eaf49c3ad
+f551ab95f46c3d7bb7c032711e10b03bfa995ee2
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/backend.h
===
--- gcc/go/gofrontend/backend.h (revision 266510)
+++ gcc/go/gofrontend/backend.h (working copy)
@@ -726,6 +726,13 @@ class Backend
// possible. This is used for field tracking.
static const unsigned int function_in_unique_section = 1 << 5;
+ // Set if the function should be available for inlining in the
+ // backend, but should not be emitted as a standalone function. Any
+ // call to the function that is not inlined should be treated as a
+ // call to a function defined in a different compilation unit. This
+ // is like a C99 function marked inline but not extern.
+ static const unsigned int function_only_inline = 1 << 6;
+
// Declare or define a function of FNTYPE.
// NAME is the Go name of the function. ASM_NAME, if not the empty
// string, is the name that should be used in the symbol table; this
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 266510)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -9785,6 +9785,15 @@ Call_expression::do_lower(Gogo* gogo, Na
}
}
+ // If this is a call to an imported function for which we have an
+ // inlinable function body, add it to the list of functions to give
+ // to the backend as inlining opportunities.
+ Func_expression* fe = this->fn_->func_expression();
+ if (fe != NULL
+ && fe->named_object()->is_function_declaration()
+ && fe->named_object()->func_declaration_value()->has_imported_body())
+gogo->add_imported_inlinable_function(fe->named_object());
+
return this;
}
Index: gcc/go/gofrontend/go.cc
===
--- gcc/go/gofrontend/go.cc (revision 266510)
+++ gcc/go/gofrontend/go.cc (working copy)
@@ -97,8 +97,6 @@ go_parse_input_files(const char** filena
}
}
- ::gogo->linemap()->stop();
-
::gogo->clear_file_scope();
// If the global predeclared names are referenced but not defined,
@@ -122,6 +120,10 @@ go_parse_input_files(const char** filena
// form which is easier to use.
::gogo->lower_parse_tree();
+ // At this point we have handled all inline functions, so we no
+ // longer need the linemap.
+ ::gogo->linemap()->stop();
+
// Create function descriptors as needed.
::gogo->create_function_descriptors();
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 266510)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -62,7 +62,9 @@ Gogo::Gogo(Backend* backend, Linemap* li
specific_type_functions_are_written_(false),
named_types_are_converted_(false),
analysis_sets_(),
-gc_roots_()
+gc_roots_(),
+imported_inlinable_functions_(),
+imported_inline_functions_()
{
const Location loc = Linemap::predeclared_location();
@@ -1557,6 +1559,13 @@ Gogo::write_globals()
}
}
+ // Output inline functions, which are in different packages.
+ for (std::vector::const_iterator p =
+this->imported_inline_functions_.begin();
+ p != this->imported_inline_functions_.end();
+ ++p)
+(*p)->get_backend(this, const_decls, type_decls, func_decls);
+
// Register global variables with the garbage collector.
this->register_gc_vars(var_gc, init_stmts, init_bfn);
@@ -2234,6 +2243,20 @@ Gogo::declare_package_function(const std
[PATCH] Fix vect/costmodel/ppc/costmodel-vect-33.c testcase (PR middle-end/87157)
Hi!
This testcase started FAILing, because function splitting recently started
splitting main1 into inline containing the cheap loop and main1.part.0 which
contains the verification in abort. I believe the test is meant to test
the vectorizer behavior, not how many times that loop has been inlined
somewhere. This patch arranges for main1 not to be inlined or split.
Tested on powerpc64le-linux, ok for trunk?
2018-11-27 Jakub Jelinek
PR middle-end/87157
* gcc.dg/vect/costmodel/ppc/costmodel-vect-33.c (main1): Add noipa
attribute.
--- gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-33.c.jj
2017-03-21 11:10:40.276012822 +
+++ gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-33.c 2018-11-27
15:29:00.685427859 +
@@ -11,7 +11,7 @@ struct test {
extern struct test s;
-int main1 ()
+__attribute__((noipa)) int main1 ()
{
int i;
Jakub
[committed] sh and riscv fixes for move_by_pieces change
sh and riscv this time. Sigh. Hopefully that's all the ports... These will also test via bootstraps during the day. Committed. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index fe8f6dc6ad7..96215ee5cd7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,11 @@ 2018-11-27 Jeff Law + * config/riscv/riscv (riscv_block_mvoe_straight): Use RETURN_BEGIN + in call to move_by_pieces. + + * config/sh/sh-mem.c (expand_block_move): Use RETURN_BEGIN in call + to move_by_pieces. + * config/lm32/lm32.c (lm32_block_move_inline): Use RETURN_BEGIN in call to move_by_pieces. diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c index 47d0b6e849e..7c1319e36de 100644 --- a/gcc/config/riscv/riscv.c +++ b/gcc/config/riscv/riscv.c @@ -2882,7 +2882,7 @@ riscv_block_move_straight (rtx dest, rtx src, HOST_WIDE_INT length) src = adjust_address (src, BLKmode, offset); dest = adjust_address (dest, BLKmode, offset); move_by_pieces (dest, src, length - offset, - MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), 0); + MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), RETURN_BEGIN); } } diff --git a/gcc/config/sh/sh-mem.cc b/gcc/config/sh/sh-mem.cc index efa958e7939..113cb8e04cd 100644 --- a/gcc/config/sh/sh-mem.cc +++ b/gcc/config/sh/sh-mem.cc @@ -91,7 +91,7 @@ expand_block_move (rtx *operands) move_by_pieces (adjust_address (dest, BLKmode, copied), adjust_automodify_address (src, BLKmode, src_addr, copied), - bytes - copied, align, 0); + bytes - copied, align, RETURN_BEGIN); return true; }
[committed] lm32 port fix for move_by_pieces
And the lm32 port... It'll go through cross testing at some point today. Committed. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a6e8729f370..fe8f6dc6ad7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,8 @@ 2018-11-27 Jeff Law + * config/lm32/lm32.c (lm32_block_move_inline): Use RETURN_BEGIN in + call to move_by_pieces. + * config/mips/mips.c (mips_block_move_straight): Use RETURN_BEGIN in call to move_by_pieces. diff --git a/gcc/config/lm32/lm32.c b/gcc/config/lm32/lm32.c index 74d87813f95..0f7633afa8d 100644 --- a/gcc/config/lm32/lm32.c +++ b/gcc/config/lm32/lm32.c @@ -868,7 +868,7 @@ lm32_block_move_inline (rtx dest, rtx src, HOST_WIDE_INT length, src = adjust_address (src, BLKmode, offset); dest = adjust_address (dest, BLKmode, offset); move_by_pieces (dest, src, length - offset, - MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), 0); + MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), RETURN_BEGIN); } }
[committed] Fix mips calls to move_by_pieces
And the same for the mips port. I'll go through all kinds of build tests during the day, including a bootstrap on mipsisa32r2. Committed. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4be85e3c366..a6e8729f370 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,8 @@ 2018-11-27 Jeff Law + * config/mips/mips.c (mips_block_move_straight): Use RETURN_BEGIN + in call to move_by_pieces. + * config/microblaze/microblaze.c (microblaze_block_move_straight): Use RETURN_BEGIN in call to move_by_pieces. (microblaze_expand_block_move): Likewise. diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 17a2a66956e..09b2ae72199 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -8064,7 +8064,7 @@ mips_block_move_straight (rtx dest, rtx src, HOST_WIDE_INT length) src = adjust_address (src, BLKmode, offset); dest = adjust_address (dest, BLKmode, offset); move_by_pieces (dest, src, length - offset, - MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), 0); + MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), RETURN_BEGIN); } }
Re: [PATCH 4/6] [RS6000] Remove constraints on call rounded_stack_size_rtx arg
On Tue, Nov 13, 2018 at 11:21:30PM +1030, Alan Modra wrote: > Version 2. (Same as before, here for completeness.) > > This call arg is unused on rs6000. > > * config/rs6000/darwin.md (call_indirect_nonlocal_darwin64), > (call_nonlocal_darwin64, call_value_indirect_nonlocal_darwin64), > (call_value_nonlocal_darwin64): Remove constraints from second call > arg, the rounded_stack_size_rtx arg. > * config/rs6000/rs6000.md (tls_gd_aix, tls_gd_sysv, tls_gd_call_aix), > (tls_gd_call_sysv, tls_ld_aix, tls_ld_sysv, tls_ld_call_aix), > (tls_ld_call_sysv, call_local32, call_local64, call_value_local32), > (call_value_local64, call_indirect_nonlocal_sysv), > (call_nonlocal_sysv, call_nonlocal_sysv_secure), > (call_value_indirect_nonlocal_sysv, call_value_nonlocal_sysv), > (call_value_nonlocal_sysv_secure, call_local_aix), > (call_value_local_aix, call_nonlocal_aix, call_value_nonlocal_aix), > (call_indirect_aix, call_value_indirect_aix, call_indirect_elfv2), > (call_value_indirect_elfv2, sibcall_local32, sibcall_local64), > (sibcall_value_local32, sibcall_value_local64, sibcall_aix), > (sibcall_value_aix): Likewise. This is still OK for trunk. Thanks! Segher
[committed] Trivial fix to microblaze port
The recent changes to the move_by_pieces API were slightly incomplete.
They failed to update the mips and microblaze ports (there may be
others, that's what has complained so far).
This patch fixes the microblaze port. I've verified we can build it
through libgcc. The tester will run a more complete build later today.
Committed to the trunk. I'll be fixing the MIPS port momentarily.
Jeff
* config/microblaze/microblaze.c (microblaze_block_move_straight): Use
RETURN_BEGIN in call to move_by_pieces.
(microblaze_expand_block_move): Likewise.
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index 06aa50e2556..6c4a62c3113 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -1180,7 +1180,7 @@ microblaze_block_move_straight (rtx dest, rtx src,
HOST_WIDE_INT length)
src = adjust_address (src, BLKmode, offset);
dest = adjust_address (dest, BLKmode, offset);
move_by_pieces (dest, src, length - offset,
- MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), 0);
+ MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), RETURN_BEGIN);
}
}
@@ -1269,7 +1269,7 @@ microblaze_expand_block_move (rtx dest, rtx src, rtx
length, rtx align_rtx)
{
if (INTVAL (length) <= MAX_MOVE_BYTES)
{
- move_by_pieces (dest, src, bytes, align, 0);
+ move_by_pieces (dest, src, bytes, align, RETURN_BEGIN);
return true;
}
else
FIO, powerpc-darwin mangling patch [7.x and earlier].
Hi, So it turns out that the Darwin PPC port was broken essentially “forever” (before the tidy-up in 8.2) with respect to mangling the symbols for __ibm128 type (the default long double for the port). In 7.x and earlier (at least back to Apple’s 4.2.1) the use passes right through rs6000_mangle_type, which causes it to mangle to ‘e’ - which is the representation for long double as a 64b value (-mlong-double-64). This is fixable, even quite easily, but I think it’s better to have the break in the upstream sources on a major boundary (so we leave it alone and have the correction in 8+) In due course, I will apply the following fix to my “vendor” branches (7, 6 and 5) for anyone who actually cares. cheers Iain diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index d377e24..ccb771b 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -35375,6 +35375,10 @@ rs6000_mangle_type (const_tree type) && !TARGET_IEEEQUAD) return "g"; + if (TARGET_MACHO && type == long_double_type_node + && TREE_INT_CST_LOW (TYPE_SIZE (type)) == 128) +return "g"; + /* For all other types, use normal C++ mangling. */ return NULL; }
[PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output
2018-11-27 Mike Gulick
PR preprocessor/83173
* gcc/input.c (dump_location_info): Dump reason and
included_from fields from line_map_ordinary struct. Fix
indentation when location > 5 digits.
* libcpp/location-example.txt: Update example
-fdump-internal-locations output.
* gcc/diagnostic-show-locus.c (num_digits, test_num_digits):
Move to gcc/diagnostic.c to allow it to be utilized by
gcc/input.c.
* gcc/diagnostic.c (num_digits, test_num_digits): Moved here.
(diagnostic_c_tests): Run test_num_digits.
* gcc/diagnostic.h (num_digits): Add extern definition.
---
gcc/diagnostic-show-locus.c | 51 --
gcc/diagnostic.c| 46 +
gcc/diagnostic.h| 3 +
gcc/input.c | 41 -
libcpp/location-example.txt | 325
5 files changed, 274 insertions(+), 192 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 278e17274e9..65fb102a817 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -819,56 +819,6 @@ fixit_cmp (const void *p_a, const void *p_b)
return hint_a->get_start_loc () - hint_b->get_start_loc ();
}
-/* Get the number of digits in the decimal representation
- of VALUE. */
-
-static int
-num_digits (int value)
-{
- /* Perhaps simpler to use log10 for this, but doing it this way avoids
- using floating point. */
- gcc_assert (value >= 0);
-
- if (value == 0)
-return 1;
-
- int digits = 0;
- while (value > 0)
-{
- digits++;
- value /= 10;
-}
- return digits;
-}
-
-
-#if CHECKING_P
-
-/* Selftest for num_digits. */
-
-static void
-test_num_digits ()
-{
- ASSERT_EQ (1, num_digits (0));
- ASSERT_EQ (1, num_digits (9));
- ASSERT_EQ (2, num_digits (10));
- ASSERT_EQ (2, num_digits (99));
- ASSERT_EQ (3, num_digits (100));
- ASSERT_EQ (3, num_digits (999));
- ASSERT_EQ (4, num_digits (1000));
- ASSERT_EQ (4, num_digits ());
- ASSERT_EQ (5, num_digits (1));
- ASSERT_EQ (5, num_digits (9));
- ASSERT_EQ (6, num_digits (10));
- ASSERT_EQ (6, num_digits (99));
- ASSERT_EQ (7, num_digits (100));
- ASSERT_EQ (7, num_digits (999));
- ASSERT_EQ (8, num_digits (1000));
- ASSERT_EQ (8, num_digits ());
-}
-
-#endif /* #if CHECKING_P */
-
/* Implementation of class layout. */
/* Constructor for class layout.
@@ -3761,7 +3711,6 @@ void
diagnostic_show_locus_c_tests ()
{
test_line_span ();
- test_num_digits ();
test_layout_range_for_single_point ();
test_layout_range_for_single_line ();
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 27e98fa9434..1b572aec6de 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -1035,6 +1035,27 @@ diagnostic_report_diagnostic (diagnostic_context
*context,
return true;
}
+/* Get the number of digits in the decimal representation of VALUE. */
+
+int
+num_digits (int value)
+{
+ /* Perhaps simpler to use log10 for this, but doing it this way avoids
+ using floating point. */
+ gcc_assert (value >= 0);
+
+ if (value == 0)
+return 1;
+
+ int digits = 0;
+ while (value > 0)
+{
+ digits++;
+ value /= 10;
+}
+ return digits;
+}
+
/* Given a partial pathname as input, return another pathname that
shares no directory elements with the pathname of __FILE__. This
is used by fancy_abort() to print `Internal compiler error in expr.c'
@@ -1785,6 +1806,29 @@ test_diagnostic_get_location_text ()
progname = old_progname;
}
+/* Selftest for num_digits. */
+
+static void
+test_num_digits ()
+{
+ ASSERT_EQ (1, num_digits (0));
+ ASSERT_EQ (1, num_digits (9));
+ ASSERT_EQ (2, num_digits (10));
+ ASSERT_EQ (2, num_digits (99));
+ ASSERT_EQ (3, num_digits (100));
+ ASSERT_EQ (3, num_digits (999));
+ ASSERT_EQ (4, num_digits (1000));
+ ASSERT_EQ (4, num_digits ());
+ ASSERT_EQ (5, num_digits (1));
+ ASSERT_EQ (5, num_digits (9));
+ ASSERT_EQ (6, num_digits (10));
+ ASSERT_EQ (6, num_digits (99));
+ ASSERT_EQ (7, num_digits (100));
+ ASSERT_EQ (7, num_digits (999));
+ ASSERT_EQ (8, num_digits (1000));
+ ASSERT_EQ (8, num_digits ());
+}
+
/* Run all of the selftests within this file. */
void
@@ -1796,6 +1840,8 @@ diagnostic_c_tests ()
test_print_parseable_fixits_remove ();
test_print_parseable_fixits_replace ();
test_diagnostic_get_location_text ();
+ test_num_digits ();
+
}
} // namespace selftest
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index a926f9bdd0b..596717e331c 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -421,4 +421,7 @@ extern char *build_message_string (const char *, ...)
ATTRIBUTE_PRINTF_1;
extern void diagnostic_output_format_init (diagnostic_context *,
enum diagnostics_output_format);
+/* Compute the number of digits in the decimal representation of
[PATCH v5 2/3] PR preprocessor/83173: New test
2018-11-27 Mike Gulick
PR preprocessor/83173
* gcc.dg/plugin/location-overflow-test-pr83173.c: New test.
* gcc.dg/plugin/location-overflow-test-pr83173.h: Header for
pr83173.c.
* gcc.dg/plugin/location-overflow-test-pr83173-1.h: Header for
pr83173.c.
* gcc.dg/plugin/location-overflow-test-pr83173-2.h: Header for
pr83173.c.
* gcc.dg/plugin/location_overflow_plugin.c: Use PLUGIN_PRAGMAS
instead of PLUGIN_START_UNIT.
* gcc.dg/plugin/plugin.exp: Enable new test.
---
.../plugin/location-overflow-test-pr83173-1.h | 2 ++
.../plugin/location-overflow-test-pr83173-2.h | 2 ++
.../plugin/location-overflow-test-pr83173.c | 21 +++
.../plugin/location-overflow-test-pr83173.h | 2 ++
.../gcc.dg/plugin/location_overflow_plugin.c | 13 +++-
gcc/testsuite/gcc.dg/plugin/plugin.exp| 3 ++-
6 files changed, 37 insertions(+), 6 deletions(-)
create mode 100644
gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h
create mode 100644
gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h
create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c
create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h
diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h
b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h
new file mode 100644
index 000..f47dc3643c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h
@@ -0,0 +1,2 @@
+#pragma once
+#define LOCATION_OVERFLOW_TEST_PR83173_1_H
diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h
b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h
new file mode 100644
index 000..bc23ed2a188
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h
@@ -0,0 +1,2 @@
+#pragma once
+#define LOCATION_OVERFLOW_TEST_PR83173_2_H
diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c
b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c
new file mode 100644
index 000..2d581283474
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c
@@ -0,0 +1,21 @@
+/*
+ { dg-options "-fplugin-arg-location_overflow_plugin-value=0x6001" }
+ { dg-do preprocess }
+*/
+
+#include "location-overflow-test-pr83173.h"
+
+int
+main ()
+{
+ return 0;
+}
+
+/*
+ The preprocessor output should contain
+ '# 1 "path/to/location-overflow-test-pr83173-1.h" 1', but should not
+ contain any other references to location-overflow-test-pr83183-1.h.
+
+ { dg-final { scan-file location-overflow-test-pr83173.i "# 1
\[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1" } }
+ { dg-final { scan-file-not location-overflow-test-pr83173.i "# (?!1
\[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1)\[0-9\]+
\[^\r\n\]+location-overflow-test-pr83173-1\.h\"" } }
+*/
diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h
b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h
new file mode 100644
index 000..49076f7ea3c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h
@@ -0,0 +1,2 @@
+#include "location-overflow-test-pr83173-1.h"
+#include "location-overflow-test-pr83173-2.h"
diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c
b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c
index 5c9d5bae77e..09bac50d2af 100644
--- a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c
@@ -12,11 +12,14 @@ int plugin_is_GPL_compatible;
static location_t base_location;
-/* Callback handler for the PLUGIN_START_UNIT event; pretend
- we parsed a very large include file. */
+/* Callback handler for the PLUGIN_PRAGMAS event; pretend we parsed a
+ very large include file. This is used to set the initial line table
+ offset for the preprocessor, to make it appear as if we had parsed a
+ very large file. PRAGMA_START_UNIT is not suitable here as is not
+ invoked during the preprocessor stage. */
static void
-on_start_unit (void */*gcc_data*/, void */*user_data*/)
+on_pragma_registration (void */*gcc_data*/, void */*user_data*/)
{
/* Act as if we've already parsed a large body of code;
so that we can simulate various fallbacks in libcpp:
@@ -81,8 +84,8 @@ plugin_init (struct plugin_name_args *plugin_info,
error_at (UNKNOWN_LOCATION, "missing plugin argument");
register_callback (plugin_info->base_name,
-PLUGIN_START_UNIT,
-on_start_unit,
+PLUGIN_PRAGMAS,
+on_pragma_registration,
NULL); /* void *user_data */
/* Hack in additional testing, based on the exact value supplied. */
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp
b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index d9
[PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location
2018-11-27 Mike Gulick
PR preprocessor/83173
* libcpp/files.c (_cpp_stack_include): Check if
line_table->highest_location is past current line before
decrementing.
---
libcpp/files.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/libcpp/files.c b/libcpp/files.c
index 3771fad75ec..fe80e84454b 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname,
int angle_brackets,
struct cpp_dir *dir;
_cpp_file *file;
bool stacked;
+ bool decremented = false;
/* For -include command-line flags we have type == IT_CMDLINE.
When the first -include file is processed we have the case, where
@@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const char
*fname, int angle_brackets,
return false;
/* Compensate for the increment in linemap_add that occurs if
- _cpp_stack_file actually stacks the file. In the case of a
- normal #include, we're currently at the start of the line
- *following* the #include. A separate location_t for this
- location makes no sense (until we do the LC_LEAVE), and
- complicates LAST_SOURCE_LINE_LOCATION. This does not apply if we
- found a PCH file (in which case linemap_add is not called) or we
- were included from the command-line. */
+ _cpp_stack_file actually stacks the file. In the case of a normal
+ #include, we're currently at the start of the line *following* the
+ #include. A separate location_t for this location makes no
+ sense (until we do the LC_LEAVE), and complicates
+ LAST_SOURCE_LINE_LOCATION. This does not apply if we found a PCH
+ file (in which case linemap_add is not called) or we were included
+ from the command-line. In the case that the #include is the last
+ line in the file, highest_location still points to the current
+ line, not the start of the next line, so we do not decrement in
+ this case. See plugin/location-overflow-test-pr83173.h for an
+ example. */
if (file->pchname == NULL && file->err_no == 0
&& type != IT_CMDLINE && type != IT_DEFAULT)
-pfile->line_table->highest_location--;
+{
+ int highest_line = linemap_get_expansion_line (pfile->line_table,
+
pfile->line_table->highest_location);
+ int source_line = linemap_get_expansion_line (pfile->line_table, loc);
+ if (highest_line > source_line)
+ {
+ pfile->line_table->highest_location--;
+ decremented = true;
+ }
+}
stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc);
- if (!stacked)
+ if (decremented && !stacked)
/* _cpp_stack_file didn't stack the file, so let's rollback the
compensation dance we performed above. */
pfile->line_table->highest_location++;
--
2.19.1
Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output
On Tue, 2018-11-27 at 14:37 +, Mike Gulick wrote: > On 11/27/18 9:27 AM, David Malcolm wrote: > > [...] > > > > > I can commit them for you if you like. Please can you repost the > > latest version of the patches as one kit, for clarity, so I can > > commit > > them. > > > > Do you want me to repost them as a patch series, or as a single > patch? Whatever's most convenient for you. Dave
Re: [PATCH, OpenACC] Properly handle wait clause with no arguments
On 2018/11/8 3:13 AM, Thomas Schwinge wrote:
NACK. Instead let's do the following, similar to C, C++, and also
similar to Fortran's OpenACC async clause handling without explicit
async-argument:
--- gcc/fortran/openmp.c
+++ gcc/fortran/openmp.c
@@ -1885,7 +1885,19 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
omp_mask mask,
break;
}
else if (m == MATCH_NO)
- needs_space = true;
+ {
+ gfc_expr *expr
+ = gfc_get_constant_expr (BT_INTEGER,
+gfc_default_integer_kind,
+&gfc_current_locus);
+ mpz_set_si (expr->value.integer, GOMP_ASYNC_NOVAL);
+ gfc_expr_list **expr_list = &c->wait_list;
+ while (*expr_list)
+ expr_list = &(*expr_list)->next;
+ *expr_list = gfc_get_expr_list ();
+ (*expr_list)->expr = expr;
+ needs_space = true;
+ }
continue;
}
if ((mask & OMP_CLAUSE_WORKER)
Okay, I see what you mean.
Now, why do we need the following changes, in this rather "convoluted"
form:
+ tree wait_expr = OMP_CLAUSE_WAIT_EXPR (c);
+
+ if (TREE_CODE (wait_expr) == INTEGER_CST
+ && tree_int_cst_compare (wait_expr, noval) == 0)
+ {
+ noval_seen = true;
+ continue;
+ }
+
args.safe_push (fold_convert_loc (OMP_CLAUSE_LOCATION (c),
- integer_type_node,
- OMP_CLAUSE_WAIT_EXPR (c)));
+ integer_type_node, wait_expr));
num_waits++;
}
- if (!tagging || num_waits)
+ if (noval_seen && num_waits == 0)
+ args[t_wait_idx] =
+ (tagging
+? oacc_launch_pack (GOMP_LAUNCH_WAIT, NULL_TREE, GOMP_ASYNC_NOVAL)
+: noval);
+ else if (!tagging || num_waits)
{
tree len;
case GOMP_LAUNCH_WAIT:
{
- unsigned num_waits = GOMP_LAUNCH_OP (tag);
+ /* Be careful to cast the op field as a signed 16-bit, and
+ sign-extend to full integer. */
+ int num_waits = ((signed short) GOMP_LAUNCH_OP (tag));
- if (num_waits)
+ if (num_waits > 0)
goacc_wait (async, num_waits, &ap);
+ else if (num_waits == acc_async_noval)
+ acc_wait_all_async (async);
Why can't we just pass "GOMP_ASYNC_NOVAL" through like any other
async-argument (that is, map a single "wait" clause to "num_waits == 1,
*ap == GOMP_ASYNC_NOVAL"), and then handle that case in "goacc_wait",
avoiding all these interface changes and special casing in different
functions?
Or am I not understanding correctly what the purpose of this is?
I think the original intention was that wait(acc_async_noval) should
correspond to "wait all" semantics, hence we should be able to ignore
and discard other wait() clauses if they exist.
Having that said, I think there is some incorrect code in my patch wrt
this intended behavior, which I'll revise.
(The assumption of an argument-less wait clause to mean "wait all" is
derived from the closely documented OpenACC wait *directive* specification.
Frankly speaking, the prior section on the wait *clause* is not explicitly
clear on this, though 'wait all' is a reasonable assumption. It would still
be helpful if we asked the OpenACC SC to clarify)
As for the idea on stuffing more code into goacc_wait(), I think that's
a pretty good suggestion, since all uses of it in oacc-parallel.c are
actually quite similar; re-factoring this part should make things more elegant.
My understanding is that before, GCC never generates "negative
async-arguments" (now used for "GOMP_ASYNC_NOVAL"), but only non-negative
ones (real "async-arguments"), which we continue to handle, as before.
Isn't that sufficient for the ABI compatibility that we promise, which is
(unless I'm confused now?) that old (existing) executables continue to
run correctly when dynamically linking against a new libgomp. Or do we
also have to care about the case that an executable built with a new
version of GCC has to work when dynamically linked against an old
libgomp?
I think either way, encoding GOMP_ASYNC_NOVAL in num_waits or as an argument
should be okay for backward compatibility, i.e. old binaries should still
work with new libgomp with this modification.
As for new binaries vs old libgomp, I believe with the original libgomp
oacc-parallel.c code, it's not quite possible to achieve the intended wait all
behavior by playing with num_waits or arguments.
I'll revise the patch and re-submit later.
Thanks,
Chung-Lin
Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output
On 11/27/18 9:27 AM, David Malcolm wrote: [...] > > I can commit them for you if you like. Please can you repost the > latest version of the patches as one kit, for clarity, so I can commit > them. > Do you want me to repost them as a patch series, or as a single patch? > Thanks > Dave >
Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output
On Tue, 2018-11-27 at 14:10 +, Mike Gulick wrote: > On 11/26/18 8:29 PM, David Malcolm wrote: > > On Mon, 2018-11-26 at 22:17 +, Mike Gulick wrote: > > > On 11/13/18 3:12 PM, David Malcolm wrote: > > > > On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: > > > > > 2018-11-13 Mike Gulick > > > > > > > > [...] > > > > > > > > > * gcc/diagnostic-core.h (num_digits): Add extern > > > > > definition. > > > > > > > > FWIW you moved the decl to diagnostic.h, but didn't update the > > > > above > > > > ChangeLog entry. > > > > > > > > [...] > > > > > > > > > diff --git a/libcpp/location-example.txt b/libcpp/location- > > > > > example.txt > > > > > index 14b5c2e284a..dc448b0493e 100644 > > > > > --- a/libcpp/location-example.txt > > > > > +++ b/libcpp/location-example.txt > > > > > > > > You're going to need to regenerate this file again; I touched > > > > many > > > > of > > > > the same lines as your patch does, in r266085 (sorry). > > > > > > > > > > Thanks. I updated this file and fixed the changelog. I will > > > send an > > > updated patch after this email. The contents of this file were a > > > little > > > stale, so many of the locations in the file have changed in > > > addition > > > to > > > the fields I added. I verified that the changed locations aren't > > > due > > > to > > > any of these patches. > > > > Excellent; thanks. > > > > Did the latest patch go through a bootstrap and regression testing? > > > > I built gcc with and without the patches and tested using > > ../gcc/configure --enable-languages=c,c++ --disable-multilib > make > make -k check > > I compared the test results using contrib/compare_tests, and the only > difference between the two was the new test that was added. Thanks. > > > > Other than the nits above, this looks good to me (once you have > > > > your > > > > contribution paperwork in place). > > > > > > The contribution paperwork is now in place. There are no other > > > changes > > > to the previous patches (other than updating the changelog date). > > > Please let me know if there is anything else I need to do. > > > > Do you have an account with commit rights to the repository? > > I do not. If you or someone else wants to commit these for me, that > is > obviously easier for me, but if you prefer me to do it just let me > know > where to start. I've been using the git mirror to create these > patches, > and haven't used subversion for a quite a few years. I can commit them for you if you like. Please can you repost the latest version of the patches as one kit, for clarity, so I can commit them. Thanks Dave
Re: [PATCH][Arm] Fix FPU configurations for Cortex-R7 and Cortex-R8
On 27/11/2018 14:10, Andre Vieira (lists) wrote: > > Hi, > > This patch fixes the FPU configurations of Cortex-R7 and Cortex-R8, > enabling the use of FP16 conversion instructions for both and adding the > option to disable double precision instruction support using '+nofp.dp'. > > Passes the self-check during building for an arm target. > > Is this OK for trunk? > > And could I also backport this to GCC-8? > > gcc/ChangeLog: > 2018-11-27 Andre Vieira > > * config/arm/arm-cpus.in (armv7-r): Add FP16conv configurations. > (cortex-r8, cortex-r7): Update default and add new configuration. > > > 0001-Arm-Fix-fpu-configurations-for-Cortex-R7-and-Cortex-.patch > > From 9878f38a3de6001e8876d21f03adbb7ebf551a79 Mon Sep 17 00:00:00 2001 > From: Andre Vieira > Date: Mon, 26 Nov 2018 16:48:46 + > Subject: [PATCH] [Arm] Fix fpu configurations for Cortex-R7 and Cortex-R8 > > Both Cortex-R7 and Cortex-R8 support FP16 conversion instructions and both > have > SP only and SP + DP configurations. You're missing the updates to the documentation. R. > --- > gcc/config/arm/arm-cpus.in | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in > index > c71409e5dd9bcad525ed98cefd839bed0fd6f8ee..2fed508a46bc138caaac5d9905d8955e29eb7097 > 100644 > --- a/gcc/config/arm/arm-cpus.in > +++ b/gcc/config/arm/arm-cpus.in > @@ -476,6 +476,8 @@ begin arch armv7-r > optalias vfpv3xd fp.sp > option fp add VFPv3 FP_DBL > optalias vfpv3-d16 fp > + option vfpv3xd-fp16 add VFPv3 fp16conv > + option vfpv3-d16-fp16 add VFPv3 FP_DBL fp16conv > option idiv add adiv > option nofp remove ALL_FP > option noidiv remove adiv > @@ -1086,7 +1088,8 @@ end cpu cortex-r5 > begin cpu cortex-r7 > cname cortexr7 > tune flags LDSCHED > - architecture armv7-r+idiv+fp > + architecture armv7-r+idiv+vfpv3-d16-fp16 > + option nofp.dp remove FP_DBL > option nofp remove ALL_FP > costs cortex > vendor 41 > @@ -1097,7 +1100,8 @@ begin cpu cortex-r8 > cname cortexr8 > tune for cortex-r7 > tune flags LDSCHED > - architecture armv7-r+idiv+fp > + architecture armv7-r+idiv+vfpv3-d16-fp16 > + option nofp.dp remove FP_DBL > option nofp remove ALL_FP > costs cortex > vendor 41 >
Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output
On 11/26/18 8:29 PM, David Malcolm wrote: > On Mon, 2018-11-26 at 22:17 +, Mike Gulick wrote: >> On 11/13/18 3:12 PM, David Malcolm wrote: >>> On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: 2018-11-13 Mike Gulick >>> >>> [...] >>> * gcc/diagnostic-core.h (num_digits): Add extern definition. >>> >>> FWIW you moved the decl to diagnostic.h, but didn't update the >>> above >>> ChangeLog entry. >>> >>> [...] >>> diff --git a/libcpp/location-example.txt b/libcpp/location- example.txt index 14b5c2e284a..dc448b0493e 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt >>> >>> You're going to need to regenerate this file again; I touched many >>> of >>> the same lines as your patch does, in r266085 (sorry). >>> >> >> Thanks. I updated this file and fixed the changelog. I will send an >> updated patch after this email. The contents of this file were a >> little >> stale, so many of the locations in the file have changed in addition >> to >> the fields I added. I verified that the changed locations aren't due >> to >> any of these patches. > > Excellent; thanks. > > Did the latest patch go through a bootstrap and regression testing? > I built gcc with and without the patches and tested using ../gcc/configure --enable-languages=c,c++ --disable-multilib make make -k check I compared the test results using contrib/compare_tests, and the only difference between the two was the new test that was added. >>> Other than the nits above, this looks good to me (once you have >>> your >>> contribution paperwork in place). >> >> The contribution paperwork is now in place. There are no other >> changes >> to the previous patches (other than updating the changelog date). >> Please let me know if there is anything else I need to do. > > Do you have an account with commit rights to the repository? I do not. If you or someone else wants to commit these for me, that is obviously easier for me, but if you prefer me to do it just let me know where to start. I've been using the git mirror to create these patches, and haven't used subversion for a quite a few years. > > Dave >
[PATCH][Arm] Fix FPU configurations for Cortex-R7 and Cortex-R8
Hi, This patch fixes the FPU configurations of Cortex-R7 and Cortex-R8, enabling the use of FP16 conversion instructions for both and adding the option to disable double precision instruction support using '+nofp.dp'. Passes the self-check during building for an arm target. Is this OK for trunk? And could I also backport this to GCC-8? gcc/ChangeLog: 2018-11-27 Andre Vieira * config/arm/arm-cpus.in (armv7-r): Add FP16conv configurations. (cortex-r8, cortex-r7): Update default and add new configuration. From 9878f38a3de6001e8876d21f03adbb7ebf551a79 Mon Sep 17 00:00:00 2001 From: Andre Vieira Date: Mon, 26 Nov 2018 16:48:46 + Subject: [PATCH] [Arm] Fix fpu configurations for Cortex-R7 and Cortex-R8 Both Cortex-R7 and Cortex-R8 support FP16 conversion instructions and both have SP only and SP + DP configurations. --- gcc/config/arm/arm-cpus.in | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in index c71409e5dd9bcad525ed98cefd839bed0fd6f8ee..2fed508a46bc138caaac5d9905d8955e29eb7097 100644 --- a/gcc/config/arm/arm-cpus.in +++ b/gcc/config/arm/arm-cpus.in @@ -476,6 +476,8 @@ begin arch armv7-r optalias vfpv3xd fp.sp option fp add VFPv3 FP_DBL optalias vfpv3-d16 fp + option vfpv3xd-fp16 add VFPv3 fp16conv + option vfpv3-d16-fp16 add VFPv3 FP_DBL fp16conv option idiv add adiv option nofp remove ALL_FP option noidiv remove adiv @@ -1086,7 +1088,8 @@ end cpu cortex-r5 begin cpu cortex-r7 cname cortexr7 tune flags LDSCHED - architecture armv7-r+idiv+fp + architecture armv7-r+idiv+vfpv3-d16-fp16 + option nofp.dp remove FP_DBL option nofp remove ALL_FP costs cortex vendor 41 @@ -1097,7 +1100,8 @@ begin cpu cortex-r8 cname cortexr8 tune for cortex-r7 tune flags LDSCHED - architecture armv7-r+idiv+fp + architecture armv7-r+idiv+vfpv3-d16-fp16 + option nofp.dp remove FP_DBL option nofp remove ALL_FP costs cortex vendor 41 -- 1.9.1
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
On 11/26/18 7:44 PM, Joseph Myers wrote: > On Mon, 26 Nov 2018, Martin Liška wrote: > >>> I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is >>> properly sysrooted. Note there's add_sysrooted_prefix separate from >>> add_prefix (but that's *not* the correct thing to use here because it uses >>> target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix). >> >> I address that in updated version of the patch. > > However, this version seems to make TOOL_INCLUDE_DIR sysrooted as well. > I don't think that's correct; TOOL_INCLUDE_DIR ($prefix/$target/include, > roughly) is a non-sysroot location for headers. Note that it's not > sysrooted in cppdefault.c, which is a good guide to which directories > should or should not be sysrooted, and what order they should come in > (though as discussed, various of the directories there are not relevant > for the present issue). > > The patch appears to be against some tree other than current trunk. At > least, it shows a function find_fortran_preinclude_file in gcc.c as > already existing in the diff context, but I see no such function in the > current sources. > I've just installed a prerequisite patch and now you should be able to apply the patch on top of current trunk. Thanks, Martin
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
On 11/26/18 7:33 PM, Joseph Myers wrote: > On Mon, 26 Nov 2018, Martin Liška wrote: > >> The header file will be install by glibc (glibc-devel package). > > To confirm: you intend to submit a patch to glibc upstream to install this > file (rather than it only being something in distribution packaging)? > Yes, I've already ask glibc people about it: https://sourceware.org/ml/libc-help/2018-11/msg00015.html I would appreciate a help with that. Martin
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
On 11/27/18 8:57 AM, Thomas Koenig wrote: > Hi Martin, > >> he header file will be install by glibc (glibc-devel package). > > Why actually glibc-devel? Needing glibc-devel for fast compilation > of Fortran seems to be counter-intuitive. Maybe glibc would be better. Works for me. Martin > > Regards > > Thomas
Re: [PATCH v3] [aarch64] Correct the maximum shift amount for shifted operands.
Sam,
> On 27.11.2018, at 14:06, Sam Tebbs wrote:
>
>
> On 11/26/18 7:50 PM, Christoph Muellner wrote:
>> The aarch64 ISA specification allows a left shift amount to be applied
>> after extension in the range of 0 to 4 (encoded in the imm3 field).
>>
>> This is true for at least the following instructions:
>>
>> * ADD (extend register)
>> * ADDS (extended register)
>> * SUB (extended register)
>>
>> The result of this patch can be seen, when compiling the following code:
>>
>> uint64_t myadd(uint64_t a, uint64_t b)
>> {
>> return a+(((uint8_t)b)<<4);
>> }
>>
>> Without the patch the following sequence will be generated:
>>
>> :
>> 0: d37c1c21 ubfiz x1, x1, #4, #8
>> 4: 8b20 add x0, x1, x0
>> 8: d65f03c0 ret
>>
>> With the patch the ubfiz will be merged into the add instruction:
>>
>> :
>> 0: 8b211000 add x0, x0, w1, uxtb #4
>> 4: d65f03c0 ret
>
> Hi Christoph,
>
> Thanks for this, I'm not a maintainer but this looks good to me. A good
> point to mention would be how it affects shifts smaller than 4 bits,
> since I don't see any testing for that in the files you have changed.
>
>> Tested with "make check" and no regressions found.
> Could you perhaps elaborate on your testing? So what triplets you
> tested, if you bootstrapped successfully etc.
Just one bit of background info...
We’ve had this change in production since 2014 both in AppliedMicro’s
(now Ampere’s) compiler branch as well as in Ubuntu PPA for various
workloads.
Thanks,
Philipp.
Re: [PATCH v3] [aarch64] Correct the maximum shift amount for shifted operands.
On 11/26/18 7:50 PM, Christoph Muellner wrote:
> The aarch64 ISA specification allows a left shift amount to be applied
> after extension in the range of 0 to 4 (encoded in the imm3 field).
>
> This is true for at least the following instructions:
>
> * ADD (extend register)
> * ADDS (extended register)
> * SUB (extended register)
>
> The result of this patch can be seen, when compiling the following code:
>
> uint64_t myadd(uint64_t a, uint64_t b)
> {
> return a+(((uint8_t)b)<<4);
> }
>
> Without the patch the following sequence will be generated:
>
> :
> 0: d37c1c21 ubfiz x1, x1, #4, #8
> 4: 8b20 add x0, x1, x0
> 8: d65f03c0 ret
>
> With the patch the ubfiz will be merged into the add instruction:
>
> :
> 0: 8b211000 add x0, x0, w1, uxtb #4
> 4: d65f03c0 ret
Hi Christoph,
Thanks for this, I'm not a maintainer but this looks good to me. A good
point to mention would be how it affects shifts smaller than 4 bits,
since I don't see any testing for that in the files you have changed.
> Tested with "make check" and no regressions found.
Could you perhaps elaborate on your testing? So what triplets you
tested, if you bootstrapped successfully etc.
Re: [PATCH/coding style] clarify pointers and operators
On Mon, Nov 26, 2018 at 10:59:47AM -0700, Martin Sebor wrote: > --- htdocs/codingconventions.html 30 Sep 2018 14:38:46 - 1.85 > +++ htdocs/codingconventions.html 26 Nov 2018 17:59:21 - > @@ -803,12 +803,17 @@ >- x > >cast > - (foo) x > - (foo)x > + (type) x > + (type)x > > - pointer dereference > - *x > - * x Why have you removed the pointer dereference case, rather than just added the new cases and replaced foo with type for cast? We want people to write *x rather than * x. Jakub
Re: [PATCH/coding style] clarify pointers and operators
On 11/26/18 6:59 PM, Martin Sebor wrote: > Martin suggested we update the Coding Conventions to describe > the expected style for function declarations with a pointer > return types, and for overloaded operators. Below is the patch. > > As an aside, regarding the space convention in casts: a crude > grep search yields about 10,000 instances of the "(type)x" kinds > of casts in GCC sources and 40,000 of the preferred "(type) x" > style with the space. That's a consistency of only 80%. Is > it worth documenting a preference for a convention that's so > inconsistently followed? > > Martin > > Index: htdocs/codingconventions.html > === > RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v > retrieving revision 1.85 > diff -u -r1.85 codingconventions.html > --- htdocs/codingconventions.html 30 Sep 2018 14:38:46 - 1.85 > +++ htdocs/codingconventions.html 26 Nov 2018 17:59:21 - > @@ -803,12 +803,17 @@ > - x > > cast > - (foo) x > - (foo)x > + (type) x > + (type)x > > - pointer dereference > - *x > - * x > + pointer cast > + (type *) x > + (type*)x > + > + > + pointer return type > + type *f (void) > + type* f (void) > > > > @@ -992,6 +997,21 @@ > Rationale and Discussion > > > + > +Note: in declarations of operator functions or in invocations of > +such functions that involve the keyword operator > +the full name of the operator should be considered as including > +the keyword with no spaces in between the keyowrd and the operator > +token. Thus, the expected format of a declaration of an operator > +is > + T &operator== (const T & const T &); > + > +and not for example > + T &operator == (const T & const T &); > + > +(with the space between operator and ==). > + > + > > Default Arguments > Thank you Martin for the patch, I like it. Martin
[PATCH][wwwdocs] Mention Ampere eMAG support in GCC9 changes.html
This patch adds "Ampere eMAG" to the list of new supported AArch64 cores in changes.html for GCC 9. Ok to commit to CVS? Thanks, Christoph --- htdocs/gcc-9/changes.html.orig 2018-11-26 20:40:38.069556986 +0100 +++ htdocs/gcc-9/changes.html 2018-11-26 20:40:44.620096365 +0100 @@ -144,6 +144,7 @@ Arm Cortex-A76 (cortex-a76). Arm Cortex-A55/Cortex-A76 DynamIQ big.LITTLE (cortex-a76.cortex-a55). + Ampere Computing eMAG (emag). The GCC identifiers can be used as arguments to the -mcpu or -mtune options,
Re: [PATCH] x86: Add -march=cascadelake
Thanks for the helpful information! But I'm still checking with hardware team about the family/model/stepping numbers for Cascadelake which are not officially disclosed by Intel (to my best knowledge). Wei Martin Liška 于2018年11月26日周一 下午10:00写道： > > On 11/26/18 12:18 PM, Jakub Jelinek wrote: > > On Mon, Nov 26, 2018 at 12:03:53PM +0100, Martin Liška wrote: > >>> For Cascade Lake the model number is the same as Skylake Server, > >>> it can only be distinguished based on the stepping (5 vs 4) > >> > >> Very interesting, probably the first time a distinguish is based on > >> stepping number? > > > > Wouldn't it be better to distinguish it based on availability of VNNI, like > > we do for unknown family/model? > > > >>> Like gcc -mcpu=native needs to learn about this. > >> > >> I'm attaching patch that does that. Note that it's completely untested as > >> I don't have > >> access to any of the new machines (Skylake server). > > Would be possible, the only ugly place would be in > libgcc/config/i386/cpuinfo.c where we > call: > > get_intel_cpu (family, model, stepping, brand_id); > /* Find available features. */ > get_available_features (ecx, edx, max_level, &avx512_vnni); > > one would need a feature to distinguish CPU model. Do we really want that? > > Martin > > > > > Jakub > > >
[c-family] Fix -fdump-ada-spec regressions in C++
This fixes a few regressions introduced by the switch to Ada 2012 for the
output of -fdump-ada-spec that are present when the input is in C++.
Tested on x86_64-suse-linux, applied on the mainline.
2018-11-27 Eric Botcazou
* c-ada-spec.c: Include stringpool.h.
(has_static_fields): Return false for incomplete types.
(is_tagged_type): Likewise.
(has_nontrivial_methods): Likewise.
(dump_ada_node) : Deal specifically with __int128.
(struct overloaded_name_hash): New structure.
(struct overloaded_name_hasher): Likewise.
(overloaded_names): New global variable.
(init_overloaded_names): New static function.
(overloaded_name_p): New predicate.
(dump_ada_declaration) : Tidy up and set TREE_VISITED
on the TYPE_STUB_DECL of the original type of a typedef, if any.
: Bail out for an unsupported overloaded name.
Remove always-true condition and dump forward types.
(dump_ada_specs): Delete overloaded_names.
--
Eric BotcazouIndex: c-family/c-ada-spec.c
===
--- c-family/c-ada-spec.c (revision 266268)
+++ c-family/c-ada-spec.c (working copy)
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.
#include "system.h"
#include "coretypes.h"
#include "tm.h"
+#include "stringpool.h"
#include "tree.h"
#include "c-ada-spec.h"
#include "fold-const.h"
@@ -1041,7 +1042,7 @@ get_underlying_decl (tree type)
static bool
has_static_fields (const_tree type)
{
- if (!type || !RECORD_OR_UNION_TYPE_P (type))
+ if (!type || !RECORD_OR_UNION_TYPE_P (type) || !COMPLETE_TYPE_P (type))
return false;
for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
@@ -1057,7 +1058,7 @@ has_static_fields (const_tree type)
static bool
is_tagged_type (const_tree type)
{
- if (!type || !RECORD_OR_UNION_TYPE_P (type))
+ if (!type || !RECORD_OR_UNION_TYPE_P (type) || !COMPLETE_TYPE_P (type))
return false;
for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
@@ -1075,7 +1076,7 @@ is_tagged_type (const_tree type)
static bool
has_nontrivial_methods (tree type)
{
- if (!type || !RECORD_OR_UNION_TYPE_P (type))
+ if (!type || !RECORD_OR_UNION_TYPE_P (type) || !COMPLETE_TYPE_P (type))
return false;
/* Only C++ types can have methods. */
@@ -2092,7 +2093,10 @@ dump_ada_node (pretty_printer *buffer, t
case INTEGER_TYPE:
case FIXED_POINT_TYPE:
case BOOLEAN_TYPE:
- if (TYPE_NAME (node))
+ if (TYPE_NAME (node)
+ && !(TREE_CODE (TYPE_NAME (node)) == TYPE_DECL
+ && !strcmp (IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (node))),
+ "__int128")))
{
if (TREE_CODE (TYPE_NAME (node)) == IDENTIFIER_NODE)
pp_ada_tree_identifier (buffer, TYPE_NAME (node), node,
@@ -2568,6 +2572,73 @@ dump_nested_type (pretty_printer *buffer
}
}
+/* Hash table of overloaded names that we cannot support. It is needed even
+ in Ada 2012 because we merge different types, e.g. void * and const void *
+ in System.Address, so we cannot have overloading for them in Ada. */
+
+struct overloaded_name_hash {
+ hashval_t hash;
+ tree name;
+ unsigned int n;
+};
+
+struct overloaded_name_hasher : delete_ptr_hash
+{
+ static inline hashval_t hash (overloaded_name_hash *t)
+{ return t->hash; }
+ static inline bool equal (overloaded_name_hash *a, overloaded_name_hash *b)
+{ return a->name == b->name; }
+};
+
+static hash_table *overloaded_names;
+
+/* Initialize the table with the problematic overloaded names. */
+
+static hash_table *
+init_overloaded_names (void)
+{
+ static const char *names[] =
+ /* The overloaded names from the /usr/include/string.h file. */
+ { "memchr", "rawmemchr", "memrchr", "strchr", "strrchr", "strchrnul",
+"strpbrk", "strstr", "strcasestr", "index", "rindex", "basename" };
+
+ hash_table *table
+= new hash_table (64);
+
+ for (unsigned int i = 0; i < ARRAY_SIZE (names); i++)
+{
+ struct overloaded_name_hash in, *h, **slot;
+ tree id = get_identifier (names[i]);
+ hashval_t hash = htab_hash_pointer (id);
+ in.hash = hash;
+ in.name = id;
+ slot = table->find_slot_with_hash (&in, hash, INSERT);
+ h = new overloaded_name_hash;
+ h->hash = hash;
+ h->name = id;
+ h->n = 0;
+ *slot = h;
+}
+
+ return table;
+}
+
+/* Return whether NAME cannot be supported as overloaded name. */
+
+static bool
+overloaded_name_p (tree name)
+{
+ if (!overloaded_names)
+overloaded_names = init_overloaded_names ();
+
+ struct overloaded_name_hash in, *h;
+ hashval_t hash = htab_hash_pointer (name);
+ in.hash = hash;
+ in.name = name;
+ h = overloaded_names->find_with_hash (&in, hash);
+ return h && ++h->n > 1;
+}
+
/* Dump in BUFFER constructor spec corresponding to T for TYPE. */
static void
@@ -2603,7 +2674,7 @@ type_name (tree t)
return IDENTIFIER_POINTER (DECL_NAME (n));
}
[testsuite] Require ucn support in gdc.test/compilable/ddoc12.d (PR d/88039)
Some assemblers, including the Solaris one, don't support UTF-8
identifiers, which breaks the gdc.test/compilable/ddoc12.d testcase as
reported in the PR.
Since we don't want to modify the upstream testcase directly, I've
instead modified the gdc.test driver to require ucn support via a
dg-require-effective-target directive, based on the file name.
Tested on i386-pc-solaris2.11 with gas (which does support them and the
test still PASSes) and sparc-sun-solaris2.11 with as (which doesn't, so
the test turns out as UNSUPPORTED).
Ok for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-11-25 Rainer Orth
PR d/88039
* gdc.test/gdc-test.exp (dmd2dg): Require ucn support for tests
using UCNs in identifiers.
# HG changeset patch
# Parent c94c294504322187c207bc70ad6734280adc7043
Require ucn support in gdc.test/compilable/ddoc12.d (PR d/88039)
diff --git a/gcc/testsuite/gdc.test/gdc-test.exp b/gcc/testsuite/gdc.test/gdc-test.exp
--- a/gcc/testsuite/gdc.test/gdc-test.exp
+++ b/gcc/testsuite/gdc.test/gdc-test.exp
@@ -276,6 +276,14 @@ proc dmd2dg { base test } {
# Since GCC 6-20160131 blank lines are not allowed in the output by default.
dg-allow-blank-lines-in-output { 1 }
+# Require UCN support for tests with UCN identifiers.
+switch $test {
+ compilable/ddoc12.d {
+ set out_line "// { dg-require-effective-target ucn }"
+ puts $fdout $out_line
+ }
+}
+
# Compilable files are successful if an output is generated.
# Fail compilable are successful if an output is not generated.
# Runnable must compile, link, and return 0 to be successful by default.
Re: [PATCH 07/10] Add dg-require-effective-target exceptions
On 26/11/2018 22:08, Mike Stump wrote: On Nov 16, 2018, at 8:28 AM, Andrew Stubbs wrote: [This patch was previously approved by Richard Sandiford (with added documentation I've still not done), but objected to by Mike Stump. I need to figure out who's right.] Since the planned port is done and someone isn't actively finishing this part of it and you'd like to not have to triage these again or otherwise see the failures, Ok. Thanks, I'll do the documentation when the main part of the port is reposted. Andrew
Re: [PATCH 10/10] Port testsuite to GCN
On 26/11/2018 21:13, Mike Stump wrote:
On Nov 26, 2018, at 12:04 PM, Mike Stump wrote:
I'll Ok the signal one, if you prefer it over a dummy signal routine. Though,
would be nice for you to add signal if possible/reasonable.
Oh, and my long term thinking on signal is that logically, it's fine to have:
#if __has_include("signal.h")
signal(...);
#endif
and once we have a way to do that, then the processor specific test goes away.
Over time, it does seem that we add more introspection capabilities to the
compiler, and introspection on what headers are there, is pretty basic, so I
can see that one day, I expect we'll get support for it.
Indeed, I was looking at the laundry list of changes a port did recently
(effective target changes), and was wondering if all these would be better
served by the compiler setting up those values for introspection, and the
testsuite using those values from introspection.
Hum, [ testing ] we already have it, welcome to the future I guess. Could you
please use the above form instead? I can't think of any down side to doing it
this way.
Thanks, I've been meaning to test why we have that, but I'm working on
addressing Jeff's review of my other patch right now.
I'll do it as you suggest, if I don't find a better way.
Andrew
Re: [PATCH] Fix PR88182
On Mon, 26 Nov 2018, Jakub Jelinek wrote: > On Mon, Nov 26, 2018 at 04:36:26PM +0100, Richard Biener wrote: > > > > With the relatex outer loop reduction support we need to avoid picking > > up a different nested cycles reduction def. That's easy given we > > record the PHI we are looking at - almost, at least. > > Thanks for fixing it. Just a nit, I guess the testcase could very well be > in g++.dg/vect/ or similar and just use -fopenmp-simd option instead of > -fopenmp, then the runtime library isn't needed nor being linked in. Ah, forgot about that. I ended up moving it to libgomp because g++.dg/gomp isn't set up to find libgomp.spec or the library... I'll try moving it back using -fopenmp-simd. Richard. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > > > Richard. > > > > 2018-11-26 Richard Biener > > > > PR tree-optimization/88182 > > * tree-vect-loop.c (vectorizable_reduction): Pick up single > > correct reduc_def_info. > > * tree-vect-slp.c (vect_analyze_slp_instance): Set > > STMT_VINFO_REDUC_DEF of the first stmt. > > > > libgomp/ > > * testsuite/libgomp.c++/pr88182.C: New testcase. > > Jakub
