[doc, committed] document that --help and --help= options cannot be combined
This patch is for PR 31357. I've applied the patch in the issue with a
minor copy-edit.
-Sandra
2018-11-17 Nick Clifton
Sandra Loosemore
PR driver/31357
gcc/
* doc/invoke.texi (Overall Options): Document that --help and
--help= options cannot be combined.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 266239)
+++ doc/invoke.texi (working copy)
@@ -1676,7 +1676,9 @@ optimization options, use:
The @option{--help=} option can be repeated on the command line. Each
successive use displays its requested class of options, skipping
-those that have already been displayed.
+those that have already been displayed. If @option{--help} is also
+specified anywhere on the command line then this takes precedence
+over any @option{--help=} option.
If the @option{-Q} option appears on the command line before the
@option{--help=} option, then the descriptive text displayed by
OpenACC ICV acc-default-async-var (was: [gomp4] Async related additions to OpenACC runtime library)
Hi Chung-Lin!
On Mon, 13 Feb 2017 18:13:42 +0800, Chung-Lin Tang
wrote:
> This patch adds:
>
> // New functions to set/get the current default async queue
> void acc_set_default_async (int);
> int acc_get_default_async (void);
>
> and _async versions of a few existing API functions:
(Please, separate patches for separate features/changes.)
Reviewing the OpenACC ICV acc-default-async-var changes here.
> --- include/gomp-constants.h (revision 245382)
> +++ include/gomp-constants.h (working copy)
> /* Asynchronous behavior. Keep in sync with
> libgomp/{openacc.h,openacc.f90,openacc_lib.h}:acc_async_t. */
>
> +#define GOMP_ASYNC_DEFAULT 0
> #define GOMP_ASYNC_NOVAL -1
> #define GOMP_ASYNC_SYNC -2
This means that "acc_set_default_async(acc_async_default)" will set
acc-default-async-var to "0", that is, the same as
"acc_set_default_async(0)". It thus follows that
"async"/"async(acc_async_noval)" is the same as "async(0)". Is that
intentional?
It is in line with the OpenACC 2.5 specification: "initial value [...] is
implementation defined", but I wonder why map it to "async(0)", and not
to its own, unspecified, but separate queue. In the latter case,
"acc_async_default" etc. would then map to a negative value to denote
this unspecified, but separate queue (and your changes would need to be
adapted for that).
I have not verified whether we're currently already having (on trunk
and/or openacc-gcc-8-branch) the semantics of the queue of
"async(acc_async_noval)" mapping to the same queue as "async(0)"?
I'm fine to accept your changes as proposed (basically, everthing from
your patch posted that has a "default_async" in it), for that's an
incremental improvement anyway. But -- unless you tell me I've
misunderstood something -- I'll get the issue I raised clarified with the
OpenACC technical committee, and we will then later improve this further.
No matter what the outcome, the implementation-defined behavior should be
documented. (Can do that once we get the intentions clarified.)
> --- libgomp/oacc-async.c (revision 245382)
> +++ libgomp/oacc-async.c (working copy)
> +int
> +acc_get_default_async (void)
> +{
> + struct goacc_thread *thr = goacc_thread ();
> +
> + if (!thr || !thr->dev)
> +gomp_fatal ("no device active");
I suppose that instead, this might also either just "return
acc_async_sync", or in fact "goacc_lazy_initialize", and then return the
correct value? As far as I remember now, I have an issue open with the
OpenACC technical committee to clarify which constructs/API calls are
expected to implicitly initialize. I'll fold this question in.
So, OK to leave 'gomp_fatal ("no device active")', as that's what all
other async routines also seem to be doing at the moment.
> +
> + return thr->default_async;
> +}
> +
> +void
> +acc_set_default_async (int async)
> +{
> + if (async < acc_async_sync)
> +gomp_fatal ("invalid async argument: %d", async);
(This will nowadays use "async_valid_stream_id_p" or some such.)
> +
> + struct goacc_thread *thr = goacc_thread ();
> +
> + if (!thr || !thr->dev)
> +gomp_fatal ("no device active");
As above.
> + thr->default_async = async;
> +}
> --- libgomp/oacc-plugin.c (revision 245382)
> +++ libgomp/oacc-plugin.c (working copy)
> +/* Return the default async number from the TLS data for the current thread.
> */
> +
> +int
> +GOMP_PLUGIN_acc_thread_default_async (void)
> +{
> + struct goacc_thread *thr = goacc_thread ();
> + return thr ? thr->default_async : acc_async_default;
> +}
As I understand, the need for this function will disappear with your
later "async re-work" changes, so OK as posted, but I wondered in which
cases we would not have a valid "goacc_thread" when coming here? (Might
again related to the "goacc_lazy_initialize" issue mentioned above.)
> --- libgomp/plugin/plugin-nvptx.c (revision 245382)
> +++ libgomp/plugin/plugin-nvptx.c (working copy)
> @@ -414,13 +414,10 @@ select_stream_for_async (int async, pthread_t thre
>struct ptx_stream *stream = NULL;
>int orig_async = async;
>
> - /* The special value acc_async_noval (-1) maps (for now) to an
> - implicitly-created stream, which is then handled the same as any other
> - numbered async stream. Other options are available, e.g. using the null
> - stream for anonymous async operations, or choosing an idle stream from
> an
> - active set. But, stick with this for now. */
> - if (async > acc_async_sync)
> -async++;
Is that actually a separate change from the acc-default-async-var
changes?
Is this one relevant in the question raised above, whether
"async(acc_async_noval)" maps to the same queue as "async(0)"?
> + /* The special value acc_async_noval (-1) maps to the thread-specific
> + default async stream. */
> + if (async == acc_async_noval)
> +async = GOMP_PLUGIN_acc_thread_default_async ();
> ---
Re: [PATCH] Implement std::pmr::synchronized_pool_resource
On 08/11/18 18:02 +, Jonathan Wakely wrote: Define the thread-safe pool resource, using a shared_mutex to allow multiple threads to concurrently allocate from thread-specific pools. Define new weak symbols for the pthread_rwlock_t functions, to avoid making libstdc++.so depend on libpthread.so * config/abi/pre/gnu.ver: Add new symbols. * include/std/memory_resource [_GLIBCXX_HAS_GTHREADS] (synchronized_pool_resource): New class. * include/std/shared_mutex (__glibcxx_rwlock_rdlock) (__glibcxx_rwlock_tryrdlock, __glibcxx_rwlock_wrlock) (__glibcxx_rwlock_trywrlock, __glibcxx_rwlock_unlock) (__glibcxx_rwlock_destroy, __glibcxx_rwlock_init) (__glibcxx_rwlock_timedrdlock, __glibcxx_rwlock_timedwrlock): Define weak symbols for POSIX rwlock functions. (__shared_mutex_pthread): Use weak symbols. * src/c++17/memory_resource.cc [_GLIBCXX_HAS_GTHREADS] (synchronized_pool_resource::_TPools): New class. (destroy_TPools): New function for pthread_key_create destructor. (synchronized_pool_resource::synchronized_pool_resource) (synchronized_pool_resource::~synchronized_pool_resource) (synchronized_pool_resource::release) (synchronized_pool_resource::do_allocate) (synchronized_pool_resource::do_deallocate): Define public members. (synchronized_pool_resource::_M_thread_specific_pools) (synchronized_pool_resource::_M_alloc_tpools) (synchronized_pool_resource::_M_alloc_shared_tpools): Define private members. The performance of this implementation is ... not great. But it's better than simply adding a mutex to unsynchronized_pool_resource and locking that around every allocation and deallocation. I am not committing this yet, because I still need to test on non-GNU targets, where the new weak wrappers around the pthread_rwlock_t functions might not work properly. Here's the updated patch I'm committing, which includes more conformance tests, and also performance tests. Tested x86_64-linux, powerpc64le-linux, x86_64-freebsd11.0, x86_64-w64-mingw32 and powerpc-aix7.2.0.0, committed to trunk. commit 273ae309f8ee48d495054d0981da5d604bd36517 Author: Jonathan Wakely Date: Sat Nov 3 23:13:26 2018 + Implement std::pmr::synchronized_pool_resource Define the thread-safe pool resource, using a shared_mutex to allow multiple threads to concurrently allocate from thread-specific pools. Define new weak symbols for the pthread_rwlock_t functions, to avoid making libstdc++.so depend on libpthread.so When the necessary Gthread support is absent only define the feature-test macro to 1, rather than 201603. This is intended to imply incomplete support, because everything except synchronized_pool_resource works. Implement std::pmr::synchronized_pool_resource * config/abi/pre/gnu.ver: Add new symbols. * include/std/memory_resource [_GLIBCXX_HAS_GTHREADS] (__cpp_lib_memory_resource): Define to expected value, 201603. (synchronized_pool_resource): New class. [!_GLIBCXX_HAS_GTHREADS] (__cpp_lib_memory_resource): Define to 1. * include/std/shared_mutex (__glibcxx_rwlock_rdlock) (__glibcxx_rwlock_tryrdlock, __glibcxx_rwlock_wrlock) (__glibcxx_rwlock_trywrlock, __glibcxx_rwlock_unlock) (__glibcxx_rwlock_destroy, __glibcxx_rwlock_init) (__glibcxx_rwlock_timedrdlock, __glibcxx_rwlock_timedwrlock): Define weak symbols for POSIX rwlock functions. (__shared_mutex_pthread): Use weak symbols. * include/std/version (__cpp_lib_memory_resource): Define. * src/c++17/memory_resource.cc [_GLIBCXX_HAS_GTHREADS] (synchronized_pool_resource::_TPools): New class. (destroy_TPools): New function for pthread_key_create destructor. (synchronized_pool_resource::synchronized_pool_resource) (synchronized_pool_resource::~synchronized_pool_resource) (synchronized_pool_resource::release) (synchronized_pool_resource::do_allocate) (synchronized_pool_resource::do_deallocate): Define public members. (synchronized_pool_resource::_M_thread_specific_pools) (synchronized_pool_resource::_M_alloc_tpools) (synchronized_pool_resource::_M_alloc_shared_tpools): Define private members. * testsuite/20_util/synchronized_pool_resource/allocate.cc: New test. * testsuite/20_util/synchronized_pool_resource/cons.cc: New test. * testsuite/20_util/synchronized_pool_resource/is_equal.cc: New test. * testsuite/20_util/synchronized_pool_resource/multithreaded.cc: New test. * testsuite/20_util/synchronized_pool_resource/release.cc: New test. *
[PATCH] RISC-V: Fix epilogue unwind info with fp and single sp adjust.
Without this patch, compiling a non-leaf function with -g -O0, we get
addis0,sp,16
.cfi_def_cfa 8, 0
...
lw s0,8(sp)
.cfi_restore 8
addisp,sp,16
.cfi_def_cfa_register 2
which clobbers the frame pointer before resetting the cfa to use the stack
pointer, which means there is at least one instruction where we can't unwind.
This was noticed while looking at gdb testsuite failures. This patch is
patterned after the MIPS port solution, and fixes the problem by emitting
.cfi_def_cfa 2, 16
when we clobber the frame pointer.
This was tested with a riscv64-linux gcc bootstrap and make check. There
were no regressions. This was also tested with a gdb make check, where it
fixed 25 failures in gdb.base/recurse.exp, gdb.base/watchpoint.exp, and
gdb.mi/mi-watch.exp.
gcc/
* config/riscv/riscv.c (epilogue_cfa_sp_offset): New.
(riscv_restore_reg): If restoring HARD_FRAME_POINTER_REGNUM, and
epilogue_cfa_sp_offset set, then add REG_CFA_DEF_CFA regnote.
(riscv_expand_epilogue): Initialize epilogue_cfa_sp_offset. Set it
to step2 if frame_pointer_needed and step1 is 0.
---
gcc/config/riscv/riscv.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 381a203da9b..47d0b6e849e 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -237,6 +237,11 @@ bool riscv_slow_unaligned_access_p;
/* Stack alignment to assume/maintain. */
unsigned riscv_stack_boundary;
+/* If non-zero, this is an offset to be added to SP to redefine the CFA
+ when restoring the FP register from the stack. Only valid when generating
+ the epilogue. */
+static int epilogue_cfa_sp_offset;
+
/* Which tuning parameters to use. */
static const struct riscv_tune_info *tune_info;
@@ -3627,8 +3632,15 @@ riscv_restore_reg (rtx reg, rtx mem)
rtx insn = riscv_emit_move (reg, mem);
rtx dwarf = NULL_RTX;
dwarf = alloc_reg_note (REG_CFA_RESTORE, reg, dwarf);
- REG_NOTES (insn) = dwarf;
+ if (epilogue_cfa_sp_offset && REGNO (reg) == HARD_FRAME_POINTER_REGNUM)
+{
+ rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
+GEN_INT (epilogue_cfa_sp_offset));
+ dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
+}
+
+ REG_NOTES (insn) = dwarf;
RTX_FRAME_RELATED_P (insn) = 1;
}
@@ -3877,6 +3889,9 @@ riscv_expand_epilogue (int style)
return;
}
+ /* Reset the epilogue cfa info before starting to emit the epilogue. */
+ epilogue_cfa_sp_offset = 0;
+
/* Move past any dynamic stack allocations. */
if (cfun->calls_alloca)
{
@@ -3941,6 +3956,12 @@ riscv_expand_epilogue (int style)
REG_NOTES (insn) = dwarf;
}
+ else if (frame_pointer_needed)
+{
+ /* Tell riscv_restore_reg to emit dwarf to redefine CFA when restoring
+old value of FP. */
+ epilogue_cfa_sp_offset = step2;
+}
if (use_restore_libcall)
frame->mask = 0; /* Temporarily fib that we need not save GPRs. */
--
2.17.1
[PATCH] avoid error_mark_node in -Wsizeof-pointer-memaccess (PR 88065)
-Wsizeof-pointer-memaccess fails with an ICE when one of
the arguments is ill-formed (error_mark_node). To avoid
the error the attached patch has the function bail in this
case.
Martin
PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
gcc/c-family/ChangeLog:
PR c/88065
* c-warn.c (sizeof_pointer_memaccess_warning): Bail if source
or destination is an error.
gcc/testsuite/ChangeLog:
PR c/88065
* gcc.dg/Wsizeof-pointer-memaccess2.c: New test.
Index: gcc/c-family/c-warn.c
===
--- gcc/c-family/c-warn.c (revision 266240)
+++ gcc/c-family/c-warn.c (working copy)
@@ -784,7 +784,10 @@ sizeof_pointer_memaccess_warning (location_t *size
if (idx >= 3)
return;
- if (sizeof_arg[idx] == NULL || sizeof_arg[idx] == error_mark_node)
+ if (src == error_mark_node
+ || dest == error_mark_node
+ || sizeof_arg[idx] == NULL
+ || sizeof_arg[idx] == error_mark_node)
return;
type = TYPE_P (sizeof_arg[idx])
Index: gcc/testsuite/gcc.dg/Wsizeof-pointer-memaccess2.c
===
--- gcc/testsuite/gcc.dg/Wsizeof-pointer-memaccess2.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wsizeof-pointer-memaccess2.c (working copy)
@@ -0,0 +1,24 @@
+/* PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+char* strncpy (char*, const char*, size_t);
+
+struct S { char a[4], b[6]; };
+
+void test_invalid_dst (struct S *p)
+{
+ strncpy (q->a, p->b, sizeof p->b);/* { dg-error ".q. undeclared" } */
+}
+
+void test_invalid_src (struct S *p)
+{
+ strncpy (p->a, q->b, sizeof p->b);/* { dg-error ".q. undeclared" } */
+}
+
+void test_invalid_bound (struct S *p)
+{
+ strncpy (p->a, p->b, sizeof q->b);/* { dg-error ".q. undeclared" } */
+}
Re: [PATCH] Support simd function declarations via a pre-include. (was: [PATCH][RFC]Overloading intrinsics)
On 15 November 2018 21:54:23 CET, Jakub Jelinek wrote: >> Can we use plain -include like in C? > >Wouldn't that be confusing whether it is included in preprocessor only >or if >it is included as a magic fortran include line at the beginning? Yes, of course. Forgot that its a cpp argument. So -fpre-include is fine. Thanks,
Re: Fix hashtable node deallocation
Here is the same patch but this time with a test change which is
supposed to show the problem.
However it doesn't because of:
_Pointer_adapter(element_type* __arg = 0)
{ _Storage_policy::set(__arg); }
which is not explicit.
So is this patch really necessary ? If it isn't, is usage of
pointer_traits<>::pointer_to really necessary ?
Note that I also found a bug in the
__gnu_test::CustomPointerAlloc::allocate, the signature with hint is wrong.
* include/ext/throw_allocator.h
(annotate_base::insert(void*, size_t)): Use insert result to check for
double insert attempt.
(annotate_base::insert_construct(void*)): Likewise.
(annotate_base::check_allocated(void*, size_t)): Return found iterator.
(annotate_base::erase(void*, size_t)): Use latter method returned
iterator.
(annotate_base::check_constructed(void*, size_t)): Return found
iterator.
(annotate_base::erase_construct(void*)): Use latter method returned
iterator.
* libstdc++-v3/testsuite/util/testsuite_allocator.h
(CustomPointerAlloc<>::allocate(size_t, pointer)): Replace by...
(CustomPointerAlloc<>::allocate(size_t, const_void_pointer)): ...this.
* testsuite/23_containers/unordered_set/allocator/ext_ptr.cc: Add
check.
François
On 11/10/18 10:40 PM, François Dumont wrote:
While working on a hashtable enhancement I noticed that we are not
using the correct method to deallocate node if the constructor throws
in _ReuseOrAllocNode operator(). I had to introduce a new
_M_deallocate_node_ptr for that as node value shall not be destroy again.
I also check other places and noticed that a __node_type destructor
call was missing.
* include/bits/hashtable_policy.h
(_Hashtable_alloc<>::_M_deallocate_node_ptr(__node_type*)): New.
(_Hashtable_alloc<>::_M_deallocate_node(__node_type*)): Use latter.
(_ReuseOrAllocNode<>::operator<_Arg>()(_Arg&&)): Likewise.
(_Hashtable_alloc<>::_M_allocate_node): Add ~__node_type call.
Tested under Linux x86_64.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index d8e8c13858d..c87c65fd9f7 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -148,8 +148,7 @@ namespace __detail
}
__catch(...)
{
- __node->~__node_type();
- __node_alloc_traits::deallocate(__a, __node, 1);
+ _M_h._M_deallocate_node_ptr(__node);
__throw_exception_again;
}
return __node;
@@ -2116,6 +2115,9 @@ namespace __detail
void
_M_deallocate_node(__node_type* __n);
+ void
+ _M_deallocate_node_ptr(__node_type* __n);
+
// Deallocate the linked list of nodes pointed to by __n
void
_M_deallocate_nodes(__node_type* __n);
@@ -2146,6 +2148,7 @@ namespace __detail
}
__catch(...)
{
+ __n->~__node_type();
__node_alloc_traits::deallocate(_M_node_allocator(), __nptr, 1);
__throw_exception_again;
}
@@ -2154,10 +2157,17 @@ namespace __detail
template
void
_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node(__node_type* __n)
+{
+ __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
+ _M_deallocate_node_ptr(__n);
+}
+
+ template
+void
+_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node_ptr(__node_type* __n)
{
typedef typename __node_alloc_traits::pointer _Ptr;
auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__n);
- __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
__n->~__node_type();
__node_alloc_traits::deallocate(_M_node_allocator(), __ptr, 1);
}
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
index 7e4a6e02900..3b2a9a1ab35 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
@@ -41,6 +41,9 @@ void test01()
test_type v;
v.insert(T());
VERIFY( ++v.begin() == v.end() );
+
+ v = { { 1 }, { 2 }, { 3 }};
+ VERIFY( v.size() == 3 );
}
int main()
diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index b0fecfb59a3..c18223475c9 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -582,7 +582,7 @@ namespace __gnu_test
typedef Ptr void_pointer;
typedef Ptr const_void_pointer;
- pointer allocate(std::size_t n, pointer = {})
+ pointer allocate(std::size_t n, const_void_pointer = {})
{ return pointer(std::allocator::allocate(n)); }
void deallocate(pointer p, std::size_t n)
Re: [PATCH] Fix debuginfo in -fopenmp code (PR debug/87039)
On Sat, Nov 17, 2018 at 05:51:05PM +0100, Richard Biener wrote:
> On November 17, 2018 4:14:58 PM GMT+01:00, Jakub Jelinek
> wrote:
> >On Sat, Nov 17, 2018 at 08:31:32AM +0100, Richard Biener wrote:
> >> On November 16, 2018 10:10:00 PM GMT+01:00, Jakub Jelinek
> >> wrote: Can you add a comment why doing it
> >differently
> >> for this case is necessary or do it the same way in all cases?
> >
> >Do you mean in omp-expand.c or dwarf2out.c? I believe I'm doing it the
> >same
>
> I meant in dwarf2out.c. IIRC we have multiple calls into dwarf2out_decl and
> you adjust only one?
We don't need to change the case where decl isn't a FUNCTION_DECL,
and must not change the case where we call dwarf2out_decl (decl), that would
lead to infinite recursion.
The only case might be replace
current_function_decl = origin;
dwarf2out_decl (origin);
with the dwarf2out_early_global_decl (origin); call, not really sure if that
is needed though. Do we ever have functions where
decl_function_context (decl) != decl_function_context (DECL_ABSTRACT_ORIGIN
(decl))
?
The other possibility is to move this decl_function_context handling code
from dwarf2out_early_global_decl to dwarf2out_decl, guarded with
if (early_dwarf
&& decl_function_context (decl)))
...
Something like (completely untested):
--- gcc/dwarf2out.c.jj 2018-11-16 17:33:42.899215778 +0100
+++ gcc/dwarf2out.c 2018-11-17 20:11:26.847301806 +0100
@@ -26390,22 +26390,6 @@ dwarf2out_early_global_decl (tree decl)
{
tree save_fndecl = current_function_decl;
- /* For nested functions, make sure we have DIEs for the parents first
-so that all nested DIEs are generated at the proper scope in the
-first shot. */
- tree context = decl_function_context (decl);
- if (context != NULL)
- {
- dw_die_ref context_die = lookup_decl_die (context);
- current_function_decl = context;
-
- /* Avoid emitting DIEs multiple times, but still process CONTEXT
-enough so that it lands in its own context. This avoids type
-pruning issues later on. */
- if (context_die == NULL || is_declaration_die (context_die))
- dwarf2out_decl (context);
- }
-
/* Emit an abstract origin of a function first. This happens
with C++ constructor clones for example and makes
dwarf2out_abstract_function happy which requires the early
@@ -26716,11 +26700,31 @@ dwarf2out_decl (tree decl)
we're a method, it will be ignored, since we already have a DIE.
Avoid doing this late though since clones of class methods may
otherwise end up in limbo and create type DIEs late. */
- if (early_dwarf
- && decl_function_context (decl)
- /* But if we're in terse mode, we don't care about scope. */
- && debug_info_level > DINFO_LEVEL_TERSE)
- context_die = NULL;
+ if (early_dwarf)
+ {
+ /* For nested functions, make sure we have DIEs for the parents first
+so that all nested DIEs are generated at the proper scope in the
+first shot. */
+ tree context = decl_function_context (decl);
+ if (context != NULL)
+ {
+ dw_die_ref ctx_die = lookup_decl_die (context);
+ tree save_fndecl = current_function_decl;
+ current_function_decl = context;
+
+ /* Avoid emitting DIEs multiple times, but still process CONTEXT
+enough so that it lands in its own context. This avoids type
+pruning issues later on. */
+ if (ctx_die == NULL || is_declaration_die (ctx_die))
+ dwarf2out_decl (context);
+
+ current_function_decl = save_fndecl;
+
+ /* But if we're in terse mode, we don't care about scope. */
+ if (debug_info_level > DINFO_LEVEL_TERSE)
+ context_die = NULL;
+ }
+ }
break;
case VAR_DECL:
Jakub
Re: [PATCH] Support simd function declarations via a pre-include.
Hi
>I'm sending version, I changed the container to hash_map that should
>provide
>faster look up.
>
>I've been testing the patch right now.
In find_fortran_preinclude_file() you allocate the filename.
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 55d6dafdb5d..4e500f88174 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2428,6 +2428,10 @@ gfc_new_file (void)
{
bool result;
+ if (flag_pre_include != NULL
+ && !load_file (flag_pre_include, NULL, false))
+exit (FATAL_EXIT_CODE);
+
if (gfc_cpp_enabled ())
{
result = gfc_cpp_preprocess (gfc_source_file);
Don't you leak the filename here?
I.e.
else free()
thanks,
Re: [C++ Patch] Remove obsolete _vptr check (and fix location)
Hi again...
On 17/11/18 12:56, Paolo Carlini wrote:
Never mind, got confused for various reasons... too bad anyway that we
unconditionally reject such member name even when there is no virtual
pointer around. Anyway, I'll send a new, straightforward patch only
fixing the two locations.
... earlier today - I was running some errands - it occurred to me that,
for example, gdb would have issues with a _vptr member, but that's not
the case. Thus, all in all, I'm still not convinced that we want that
old check. Anyway, in case we want to go for something super-safe at
this stage, I'm attaching the promised patchlet only taking care of the
locations.
Paolo.
/
Index: cp/decl2.c
===
--- cp/decl2.c (revision 266233)
+++ cp/decl2.c (working copy)
@@ -836,14 +836,16 @@ grokfield (const cp_declarator *declarator,
{
if (TREE_CODE (name) == TEMPLATE_ID_EXPR)
{
- error ("explicit template argument list not allowed");
+ error_at (declarator->id_loc,
+ "explicit template argument list not allowed");
return error_mark_node;
}
if (IDENTIFIER_POINTER (name)[0] == '_'
&& id_equal (name, "_vptr"))
- error ("member %qD conflicts with virtual function table field name",
- value);
+ error_at (declarator->id_loc,
+ "member %qD conflicts with virtual function "
+ "table field name", value);
}
/* Stash away type declarations. */
Index: testsuite/g++.dg/other/vptr.C
===
--- testsuite/g++.dg/other/vptr.C (nonexistent)
+++ testsuite/g++.dg/other/vptr.C (working copy)
@@ -0,0 +1,4 @@
+struct S
+{
+ virtual void _vptr() = 0; // { dg-error "16:member .virtual void
S::_vptr\\(\\). conflicts with virtual function table field name" }
+};
Index: testsuite/g++.dg/template/crash91.C
===
--- testsuite/g++.dg/template/crash91.C (revision 266231)
+++ testsuite/g++.dg/template/crash91.C (working copy)
@@ -4,5 +4,5 @@ template void foo();
struct A
{
- typedef void foo<0>(); // { dg-error "explicit template argument list not
allowed" }
+ typedef void foo<0>(); // { dg-error "16:explicit template argument list not
allowed" }
};
Re: [PATCH] Support simd function declarations via a pre-include.
Hi All,
Forget my remark about mp_prop_design.f90. ifort -parallel means just
that and has nothing to do with vectorization.
Sorry for the noise.
Paul
On Sat, 17 Nov 2018 at 13:29, Paul Richard Thomas
wrote:
>
> Hi All,
>
> I took a few moments away from what I really must be doing to try out
> an earlier version of the patch. There are quite a few CRs in the
> patch and the third chunk in gcc.c was rejected, although I cannot see
> why. I made a change to scanner.c to prevent the segfault that results
> from not having the pre-include file in the right directory - see
> attached.
>
> Everything works fine with the testcases and a slightly evolved
> version shows reasonable speed-ups:
> program test_overloaded_intrinsic
> real(4) :: x4(3200), y4(3200, 1)
> real(8) :: x8(3200), y8(3200)
>
>do i = 1,1
> y4(:,i) = cos(x4)
> y4(:,i) = x4*y4(:,i)
> y4(:,i) = sqrt(y4(:,i))
> end do
> print *, y4(1,1)
> end
>
> Disappointingly, though, one of the worst cases in
> https://www.fortran.uk/fortran-compiler-comparisons/ of a big
> difference between gfortran and ifort with vectorization,
> mp_prop_design.f90, doesn't seem to pick up any of the vectorization
> opportunities, even though it is peppered with trig functions. Should
> I be expecting this? Yes, I did add the other trig functions to the
> pre-include file :-)
>
> Best regards and thanks for taking care of this
>
> Paul
> On Fri, 16 Nov 2018 at 15:13, Martin Liška wrote:
> >
> > On 11/16/18 2:49 PM, Jakub Jelinek wrote:
> > > On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote:
> > >> + if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
> > >> +return MATCH_ERROR;
> > >> +
> > >> + int builtin_kind = 0;
> > >> + if (gfc_match (" (notinbranch)") == MATCH_YES)
> > >
> > > I think you need " ( notinbranch )" here.
> > >
> > >> +builtin_kind = -1;
> > >> + else if (gfc_match (" (inbranch)") == MATCH_YES)
> > >> +builtin_kind = 1;
> > >
> > > And similarly here (+ testsuite coverage for whether you can in free form
> > > insert spaces in all the spots that should be allowed).
> > > !gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
> > > e.g. should be valid in free form (and fixed form too).
> > >
> > >> --- a/gcc/fortran/gfortran.h
> > >> +++ b/gcc/fortran/gfortran.h
> > >> @@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
> > >> match gfc_match_char_spec (gfc_typespec *);
> > >> extern int directive_unroll;
> > >>
> > >> +/* Tuple for parsing of vectorized built-ins. */
> > >> +struct vect_builtin_tuple
> > >> +{
> > >> + vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)
> > >
> > > gfc_vect_builtin_tuple ?
> > > + document what the simd_type is (or make it enum or whatever).
> > > One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
> > > the case where the argument isn't specified, but I think generally
> > > gfortran.h doesn't depend on tree* stuff and wants to have its own
> > > enums etc.
> > >
> > >> +extern vec vectorized_builtins;
> > >
> > > gfc_vectorized_builtins ?
> > >
> > >> --- a/gcc/fortran/trans-intrinsic.c
> > >> +++ b/gcc/fortran/trans-intrinsic.c
> > >> @@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type,
> > >> bool is_const)
> > >>return fndecl;
> > >> }
> > >>
> > >> +/* Add SIMD attribute for FNDECL built-in if the built-in
> > >> + name is in VECTORIZED_BUILTINS. */
> > >> +#include "print-tree.h"
> > >
> > > If you need to include a header, include it at the start of the file.
> > >
> > >> +static void
> > >> +add_simd_flag_for_built_in (tree fndecl)
> > >> +{
> > >> + if (fndecl == NULL_TREE)
> > >> +return;
> > >> +
> > >> + const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
> > >> + for (unsigned i = 0; i < vectorized_builtins.length (); i++)
> > >> +if (strcmp (vectorized_builtins[i].name, name) == 0)
> > >
> > > How many add_simd_flag_for_built_in calls are we expecting and how many
> > > vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
> > > the vector by name and do a binary search. At least if it turns out to be
> > > non-trivial compile time.
> > >> +
> > >> + vectorized_builtins.truncate (0);
> > >
> > > That is a memory leak, right? The names are malloced.
> > > And why truncate rather than release?
> > >> + const char *path = find_a_file (_prefixes, argv[1], R_OK,
> > >> true);
> > >> + if (path != NULL)
> > >> + return concat (argv[0], path, NULL);
> > >
> > > Formatting.
> > >> --- /dev/null
> > >> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
> > >> @@ -0,0 +1,4 @@
> > >> +!GCC$ builtin (sinf) attributes simd
> > >> +!GCC$ builtin (sinf) attributes simd (inbranch)
> > >> +!GCC$ builtin (sinf) attributes simd (notinbranch)
> > >> +!GCC$ builtin (cosf) attributes simd (notinbranch)
> > >
> > > Are you sure it is a good idea to have the 3 first lines for the same
> > > builtin, rather than different?
> > >
> > >
[doc, committed] document implicit extern "C" block in system headers
This patch is for PR4225, another ancient C++ documentation issue, in
which a user expressed surprise at the implicit extern "C" block the
preprocessor added to system headers specified via CPLUS_INCLUDE_PATH.
Presently RS/6000 AIX is the only remaining target that gives this
special treatment to system headers; see
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00116.html
-Sandra
2018-11-17 Sandra Loosemore
PR c++/4225
gcc/
* doc/cpp.texi (System Headers): Add note about implicit
extern "C" block on targets that define SYSTEM_IMPLICIT_EXTERN_C.
Index: gcc/doc/cpp.texi
===
--- gcc/doc/cpp.texi (revision 266238)
+++ gcc/doc/cpp.texi (working copy)
@@ -1131,6 +1131,9 @@ header, no matter where it was found. C
system_header}} has no effect in the primary source file.
@end itemize
+On some targets, such as RS/6000 AIX, GCC implicitly surrounds all
+system headers with an @samp{extern "C"} block when compiling as C++.
+
@node Macros
@chapter Macros
Re: [PATCH] Fix debuginfo in -fopenmp code (PR debug/87039)
On November 17, 2018 4:14:58 PM GMT+01:00, Jakub Jelinek wrote: >On Sat, Nov 17, 2018 at 08:31:32AM +0100, Richard Biener wrote: >> On November 16, 2018 10:10:00 PM GMT+01:00, Jakub Jelinek >> wrote: Can you add a comment why doing it >differently >> for this case is necessary or do it the same way in all cases? > >Do you mean in omp-expand.c or dwarf2out.c? I believe I'm doing it the >same I meant in dwarf2out.c. IIRC we have multiple calls into dwarf2out_decl and you adjust only one? >way in all cases (except for the grid stuff for now where I'm deferring >to >Martin). In dwarf2out.c the change just makes sure that for each >function >its decl_function_context is handled before that function, while before >it >was guaranteed just for one level. > >> I guess >> there might be latent issues for nested functions inside nested >functions? > >I think the change was for Ada, not really sure if one can have nested >functions inside of nested functions. E.g. I think in Fortran one >can't, in >GNU C one can. > > Jakub
Re: *Ping* Fix PR 70260, ICE on invalid
Hi Thomas, OK for trunk. Thanks for working on it. Paul On Sat, 17 Nov 2018 at 15:10, Thomas Koenig wrote: > > Hi, > > > the attached patch fixes both ICEs in the PR by adding some tests. > > It was necessary to shuffle around a bit of code, plus to make sure that > > double error reporting did not become too bad. > > > > Regression-tested. OK for trunk? > > Ping? > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[doc, committed] clarify -fno-implicit-templates usage
This patch is for PR 4025, one of the oldest documentation issues in
bugzilla. I think that since 2001 (GCC 3.0) expectations for C++
template handling have evolved so that users expect it to "just work"
without messing with options like this, but it's a trivial fix to the docs.
-Sandra
2018-11-17 Sandra Loosemore
PR c++/4025
gcc/
* doc/invoke.texi (C++ Dialect Options): Clarify usage of
-fno-implicit-templates.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 266233)
+++ gcc/doc/invoke.texi (working copy)
@@ -2574,6 +2574,9 @@ This option is implied by the strict ISO
@opindex fimplicit-templates
Never emit code for non-inline templates that are instantiated
implicitly (i.e.@: by use); only emit code for explicit instantiations.
+If you use this option, you must take care to structure your code to
+include all the necessary explicit instantiations to avoid getting
+undefined symbols at link time.
@xref{Template Instantiation}, for more information.
@item -fno-implicit-inline-templates
Re: [patch, fortran] Fix PR 70260, ICE on invalid
On 11/11/18 7:59 AM, Thomas Koenig wrote: Hello world, the attached patch fixes both ICEs in the PR by adding some tests. It was necessary to shuffle around a bit of code, plus to make sure that double error reporting did not become too bad. Regression-tested. OK for trunk? Regards Thomas On vacation in Florida for a few days with Grandkids. Looks Good To Me. OK Jerry
[PATCH, libphobos] Committed IEEE quadruple support to std.conv
This patch is backported from phobos 2.079, in continuation of
finishing off AArch64 library support.
Bootstrapped and ran D2 testsuite on x86_64-linux-gnu.
Committed to trunk as r266238.
--
Iain
---
diff --git a/libphobos/src/std/conv.d b/libphobos/src/std/conv.d
index 127849d0131..76ac5321382 100644
--- a/libphobos/src/std/conv.d
+++ b/libphobos/src/std/conv.d
@@ -2644,7 +2644,6 @@ Target parse(Target, Source)(ref Source source)
if (isInputRange!Source && isSomeChar!(ElementType!Source) && !is(Source == enum) &&
isFloatingPoint!Target && !is(Target == enum))
{
-import core.stdc.math : HUGE_VAL;
import std.ascii : isDigit, isAlpha, toLower, toUpper, isHexDigit;
import std.exception : enforce;
@@ -2669,7 +2668,7 @@ if (isInputRange!Source && isSomeChar!(ElementType!Source) && !is(Source == enum
{
if (msg == null)
msg = "Floating point conversion error";
-return new ConvException(text(msg, " for input \"", p, "\"."), fn, ln);
+return new ConvException(text(msg, " for input \"", source, "\"."), fn, ln);
}
@@ -2684,29 +2683,24 @@ if (isInputRange!Source && isSomeChar!(ElementType!Source) && !is(Source == enum
enforce(!p.empty, bailOut());
if (toLower(p.front) == 'i')
goto case 'i';
-enforce(!p.empty, bailOut());
break;
case '+':
p.popFront();
enforce(!p.empty, bailOut());
break;
case 'i': case 'I':
+// inf
p.popFront();
-enforce(!p.empty, bailOut());
-if (toLower(p.front) == 'n')
-{
-p.popFront();
-enforce(!p.empty, bailOut());
-if (toLower(p.front) == 'f')
-{
-// 'inf'
-p.popFront();
-static if (isNarrowString!Source)
-source = cast(Source) p;
-return sign ? -Target.infinity : Target.infinity;
-}
-}
-goto default;
+enforce(!p.empty && toUpper(p.front) == 'N',
+ bailOut("error converting input to floating point"));
+p.popFront();
+enforce(!p.empty && toUpper(p.front) == 'F',
+ bailOut("error converting input to floating point"));
+// skip past the last 'f'
+p.popFront();
+static if (isNarrowString!Source)
+source = cast(Source) p;
+return sign ? -Target.infinity : Target.infinity;
default: {}
}
@@ -2723,247 +2717,97 @@ if (isInputRange!Source && isSomeChar!(ElementType!Source) && !is(Source == enum
}
isHex = p.front == 'x' || p.front == 'X';
+if (isHex) p.popFront();
}
+else if (toLower(p.front) == 'n')
+{
+// nan
+p.popFront();
+enforce(!p.empty && toUpper(p.front) == 'A',
+ bailOut("error converting input to floating point"));
+p.popFront();
+enforce(!p.empty && toUpper(p.front) == 'N',
+ bailOut("error converting input to floating point"));
+// skip past the last 'n'
+p.popFront();
+static if (isNarrowString!Source)
+source = cast(Source) p;
+return typeof(return).nan;
+}
+
+/*
+ * The following algorithm consists of 2 steps:
+ * 1) parseDigits processes the textual input into msdec and possibly
+ *lsdec/msscale variables, followed by the exponent parser which sets
+ *exp below.
+ *Hex: input is 0xa...p+000... where is the mantissa in hex
+ *and 000 is the exponent in decimal format with base 2.
+ *Decimal: input is 0.00333...p+000... where 0.0033 is the mantissa
+ *in decimal and 000 is the exponent in decimal format with base 10.
+ * 2) Convert msdec/lsdec and exp into native real format
+ */
real ldval = 0.0;
char dot = 0;/* if decimal point has been seen */
int exp = 0;
-long msdec = 0, lsdec = 0;
+ulong msdec = 0, lsdec = 0;
ulong msscale = 1;
+bool sawDigits;
-if (isHex)
-{
-int guard = 0;
-int anydigits = 0;
-uint ndigits = 0;
-
-p.popFront();
-while (!p.empty)
-{
-int i = p.front;
-while (isHexDigit(i))
-{
-anydigits = 1;
-i = isAlpha(i) ? ((i & ~0x20) - ('A' - 10)) : i - '0';
-if (ndigits < 16)
-{
-msdec = msdec * 16 + i;
-if (msdec)
-ndigits++;
-}
-else if (ndigits == 16)
-{
-while (msdec >= 0)
-{
-exp--;
-msdec <<= 1;
-i <<= 1;
-if (i & 0x10)
-msdec |= 1;
-}
-guard = i << 4;
-
Re: [PATCH] Fix debuginfo in -fopenmp code (PR debug/87039)
On Sat, Nov 17, 2018 at 08:31:32AM +0100, Richard Biener wrote: > On November 16, 2018 10:10:00 PM GMT+01:00, Jakub Jelinek > wrote: Can you add a comment why doing it differently > for this case is necessary or do it the same way in all cases? Do you mean in omp-expand.c or dwarf2out.c? I believe I'm doing it the same way in all cases (except for the grid stuff for now where I'm deferring to Martin). In dwarf2out.c the change just makes sure that for each function its decl_function_context is handled before that function, while before it was guaranteed just for one level. > I guess > there might be latent issues for nested functions inside nested functions? I think the change was for Ada, not really sure if one can have nested functions inside of nested functions. E.g. I think in Fortran one can't, in GNU C one can. Jakub
*Ping* Fix PR 70260, ICE on invalid
Hi, the attached patch fixes both ICEs in the PR by adding some tests. It was necessary to shuffle around a bit of code, plus to make sure that double error reporting did not become too bad. Regression-tested. OK for trunk? Ping? Regards Thomas
[PATCH, libphobos] Fix libphobos.shared testsuite for multilib tests
Hi,
the loadDR test in the libphobos.shared testsuite tries to dynamically load the
phobos library. The path for the library currently points to the main multilib
variant phobos library, causing other multilib variants to fail the test. The
attached patch uses $blddir instead of $objdir to fix this issue.
---
libphobos/ChangeLog:
2018-11-17 Johannes Pfau
PR d/87824
* testsuite/libphobos.shared/shared.exp: Set proper path to phobos library
for multilib builds.
diff --git a/libphobos/testsuite/libphobos.shared/shared.exp
b/libphobos/testsuite/libphobos.shared/shared.exp
index b3bdd..623e06259 100644
--- a/libphobos/testsuite/libphobos.shared/shared.exp
+++ b/libphobos/testsuite/libphobos.shared/shared.exp
@@ -94,7 +94,7 @@ if { [is-effective-target dlopen] && [is-effective-target
pthread] } {
dg-test "$srcdir/$subdir/host.c" "-ldl -pthread" "$DEFAULT_CFLAGS"
# Test requires a command line argument to be passed to the program.
-set libphobos_run_args "$objdir/../src/.libs/libgphobos.so"
+set libphobos_run_args "${blddir}/src/.libs/libgphobos.${shlib_ext}"
dg-test "$srcdir/$subdir/loadDR.c" "-ldl -pthread -g" "$DEFAULT_CFLAGS"
set libphobos_run_args ""
}
Re: [PATCH 0/2] asm qualifiers (PR55681) and asm input
Ping x2.
On Sat, Nov 10, 2018 at 06:33:37PM -0600, Segher Boessenkool wrote:
> Ping.
>
> On Tue, Oct 30, 2018 at 05:30:32PM +, Segher Boessenkool wrote:
> > Hi!
> >
> > This is the same "asm input" patch as before, but now preceded by a patch
> > that makes all orderings of volatile/goto/inline valid, also the other type
> > qualifiers for C, and also repetitions for C.
> >
> > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk?
> >
> >
> > Segher
> >
> >
> > gcc/c/c-parser.c| 79 -
> > gcc/c/c-tree.h | 3 +-
> > gcc/c/c-typeck.c| 3 +-
> > gcc/cp/cp-tree.h| 2 +-
> > gcc/cp/parser.c | 92
> > +
> > gcc/cp/pt.c | 2 +-
> > gcc/cp/semantics.c | 3 +-
> > gcc/doc/extend.texi | 10 ++-
> > gcc/gimple-pretty-print.c | 2 +
> > gcc/gimple.h| 24 ++-
> > gcc/gimplify.c | 1 +
> > gcc/ipa-icf-gimple.c| 3 +
> > gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 ++
> > gcc/tree-core.h | 3 +
> > gcc/tree-inline.c | 3 +
> > gcc/tree.h | 3 +
> > 16 files changed, 221 insertions(+), 65 deletions(-)
> > create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c
> >
> > --
> > 1.8.3.1
Re: [PATCH 21/25] GCN Back-end (part 2/2).
On Fri, Nov 16, 2018 at 10:09:59AM -0600, Segher Boessenkool wrote:
> On Mon, Nov 12, 2018 at 11:54:58AM -0700, Jeff Law wrote:
> > On 11/12/18 10:52 AM, Andrew Stubbs wrote:
> > > If there are two instructions that both have an UNSPEC_VOLATILE, will
> > > combine coalesce them into one in the combined pattern?
> > I think you can put a different constant on each.
>
> combine (like everything else) will not remove an unspec_volatile. Two
> identical ones can be merged, in theory anyway, but the resulting program
> will always still execute the same number of them, in the same order.
> unspec_volatile's have unknown side effects, and those have to happen.
>
> If you really need to prevent merging them, then sure, using different
> unspec numbers will work, sure.
Here is a simple example, PowerPC code:
long f(long x)
{
if (x)
return __builtin_ppc_mftb();
else
return __builtin_ppc_mftb();
}
This compiles to just
mfspr 3,268
blr
[ That builtin is an unspec_volatile:
(define_insn "rs6000_mftb_"
[(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
(unspec_volatile:GPR [(const_int 0)] UNSPECV_MFTB))]
...)
]
It is optimised by the pre pass. If you say -fno-tree-pre -fno-code-hoisting
it is still two mftb's in RTL, but the jump2 pass (crossjumping) optimises
that away, too. So, both gimple and RTL passes know how to do this, and
both have the same semantics for unspec_voilatile.
Changing the testcase to
long f(long x)
{
if (x) {
__builtin_ppc_mftb();
return __builtin_ppc_mftb();
} else {
__builtin_ppc_mftb();
return __builtin_ppc_mftb();
}
}
results in
mfspr 9,268
mfspr 3,268
blr
showing that two identical unspec_volatile's are not elided if both are
executed.
Segher
Re: [PATCH] Support simd function declarations via a pre-include.
Hi All,
I took a few moments away from what I really must be doing to try out
an earlier version of the patch. There are quite a few CRs in the
patch and the third chunk in gcc.c was rejected, although I cannot see
why. I made a change to scanner.c to prevent the segfault that results
from not having the pre-include file in the right directory - see
attached.
Everything works fine with the testcases and a slightly evolved
version shows reasonable speed-ups:
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200, 1)
real(8) :: x8(3200), y8(3200)
do i = 1,1
y4(:,i) = cos(x4)
y4(:,i) = x4*y4(:,i)
y4(:,i) = sqrt(y4(:,i))
end do
print *, y4(1,1)
end
Disappointingly, though, one of the worst cases in
https://www.fortran.uk/fortran-compiler-comparisons/ of a big
difference between gfortran and ifort with vectorization,
mp_prop_design.f90, doesn't seem to pick up any of the vectorization
opportunities, even though it is peppered with trig functions. Should
I be expecting this? Yes, I did add the other trig functions to the
pre-include file :-)
Best regards and thanks for taking care of this
Paul
On Fri, 16 Nov 2018 at 15:13, Martin Liška wrote:
>
> On 11/16/18 2:49 PM, Jakub Jelinek wrote:
> > On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote:
> >> + if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
> >> +return MATCH_ERROR;
> >> +
> >> + int builtin_kind = 0;
> >> + if (gfc_match (" (notinbranch)") == MATCH_YES)
> >
> > I think you need " ( notinbranch )" here.
> >
> >> +builtin_kind = -1;
> >> + else if (gfc_match (" (inbranch)") == MATCH_YES)
> >> +builtin_kind = 1;
> >
> > And similarly here (+ testsuite coverage for whether you can in free form
> > insert spaces in all the spots that should be allowed).
> > !gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
> > e.g. should be valid in free form (and fixed form too).
> >
> >> --- a/gcc/fortran/gfortran.h
> >> +++ b/gcc/fortran/gfortran.h
> >> @@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
> >> match gfc_match_char_spec (gfc_typespec *);
> >> extern int directive_unroll;
> >>
> >> +/* Tuple for parsing of vectorized built-ins. */
> >> +struct vect_builtin_tuple
> >> +{
> >> + vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)
> >
> > gfc_vect_builtin_tuple ?
> > + document what the simd_type is (or make it enum or whatever).
> > One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
> > the case where the argument isn't specified, but I think generally
> > gfortran.h doesn't depend on tree* stuff and wants to have its own
> > enums etc.
> >
> >> +extern vec vectorized_builtins;
> >
> > gfc_vectorized_builtins ?
> >
> >> --- a/gcc/fortran/trans-intrinsic.c
> >> +++ b/gcc/fortran/trans-intrinsic.c
> >> @@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type,
> >> bool is_const)
> >>return fndecl;
> >> }
> >>
> >> +/* Add SIMD attribute for FNDECL built-in if the built-in
> >> + name is in VECTORIZED_BUILTINS. */
> >> +#include "print-tree.h"
> >
> > If you need to include a header, include it at the start of the file.
> >
> >> +static void
> >> +add_simd_flag_for_built_in (tree fndecl)
> >> +{
> >> + if (fndecl == NULL_TREE)
> >> +return;
> >> +
> >> + const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
> >> + for (unsigned i = 0; i < vectorized_builtins.length (); i++)
> >> +if (strcmp (vectorized_builtins[i].name, name) == 0)
> >
> > How many add_simd_flag_for_built_in calls are we expecting and how many
> > vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
> > the vector by name and do a binary search. At least if it turns out to be
> > non-trivial compile time.
> >> +
> >> + vectorized_builtins.truncate (0);
> >
> > That is a memory leak, right? The names are malloced.
> > And why truncate rather than release?
> >> + const char *path = find_a_file (_prefixes, argv[1], R_OK, true);
> >> + if (path != NULL)
> >> + return concat (argv[0], path, NULL);
> >
> > Formatting.
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
> >> @@ -0,0 +1,4 @@
> >> +!GCC$ builtin (sinf) attributes simd
> >> +!GCC$ builtin (sinf) attributes simd (inbranch)
> >> +!GCC$ builtin (sinf) attributes simd (notinbranch)
> >> +!GCC$ builtin (cosf) attributes simd (notinbranch)
> >
> > Are you sure it is a good idea to have the 3 first lines for the same
> > builtin, rather than different?
> >
> > It should be testsuite covered what we do in that case, but with the above
> > you don't cover what happens e.g. with notinbranch alone, or no argument.
> >
> > Plus, as I said, I think you should have one *.f and one *.f90 test where
> > you just use many of those !gcc$ builtin lines with spaces in various spots
> > to verify it is parsed properly.
> >
> > Jakub
> >
>
> Hi.
>
> I'm sending version, I changed the container to hash_map that should provide
>
Re: [C++ Patch] Remove obsolete _vptr check (and fix location)
Hi, Il 17 Novembre 2018 11:00:45 CET, Paolo Carlini ha scritto: >Hi, > >while I was working on some location issues I noticed this check which >seems obsolete to me and means that we unnecessarily reject kosher >code: >indeed we don't test for it anywhere and neither ICC nor clang enforce >it. Never mind, got confused for various reasons... too bad anyway that we unconditionally reject such member name even when there is no virtual pointer around. Anyway, I'll send a new, straightforward patch only fixing the two locations. Thanks, Paolo
Fix ICE while outputting ODR warnings
Hi,
this patch fixes ICE with the testcase in PR 87957 which is caused by
type variants now having only IDENTIFIER_NODE as TYPE_NAME.
I did not add the testcase yet becasue bit more fixing is needed. The
code outputs incorrectly warning about subtype (it misses there is ODR
violation in it and sort of accidently only warns about alignment).
This is because processing types depends on the order they are added
into the odr table and we no longer do that in streaming order that goes
by SCCs. I need to add recursion to look into subtypes first before
trying to output info about the type itself.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
PR ipa/87957
* ipa-devirt.c (warn_odr): Look for main variant to get TYPE_DECL.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 266100)
+++ ipa-devirt.c(working copy)
@@ -1000,11 +1000,11 @@ static void
warn_odr (tree t1, tree t2, tree st1, tree st2,
bool warn, bool *warned, const char *reason)
{
- tree decl2 = TYPE_NAME (t2);
+ tree decl2 = TYPE_NAME (TYPE_MAIN_VARIANT (t2));
if (warned)
*warned = false;
- if (!warn || !TYPE_NAME(t1))
+ if (!warn || !TYPE_NAME(TYPE_MAIN_VARIANT (t1)))
return;
/* ODR warnings are output druing LTO streaming; we must apply location
@@ -1013,10 +1013,22 @@ warn_odr (tree t1, tree t2, tree st1, tr
lto_location_cache::current_cache->apply_location_cache ();
auto_diagnostic_group d;
- if (!warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (t1)), OPT_Wodr,
- "type %qT violates the C++ One Definition Rule",
- t1))
-return;
+ if (t1 != TYPE_MAIN_VARIANT (t1)
+ && TYPE_NAME (t1) != DECL_NAME (TYPE_MAIN_VARIANT (t1)))
+{
+ if (!warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (TYPE_MAIN_VARIANT
(t1))),
+ OPT_Wodr, "type %qT (typedef of %qT) violates the "
+ "C++ One Definition Rule",
+ t1, TYPE_MAIN_VARIANT (t1)))
+ return;
+}
+ else
+{
+ if (!warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (TYPE_MAIN_VARIANT
(t1))),
+ OPT_Wodr, "type %qT violates the C++ One Definition
Rule",
+ t1))
+ return;
+}
if (!st1 && !st2)
;
/* For FIELD_DECL support also case where one of fields is
[PATCH, PR d/87824] Committed return min_align_of_type from alignsize hook.
Hi,
This patch addresses some of the failing tests on x86_64-linux/-m32.
The failing tests in question checked that data with 'long' was
sufficiently aligned.
assert(( % long.alignof) == 0);
Where long.alignof was replaced with the result of TYPE_ALIGN_UNIT.
The D language expects the minimum alignment from .alignof, so on -m32
alignof should return 4 instead of 8.
Bootstrapped and ran D2 testsuite on x86_64-linux-gnu with
{-m64,-m32/-mno-sse/-mno-mmx,-mx32} runtest flags.
Committed to trunk as r266234.
---
gcc/d/ChangeLog:
2018-11-17 Iain Buclaw
PR d/87824
* d-target.cc (Target::alignsize): Return min_align_of_type.
diff --git a/gcc/d/d-target.cc b/gcc/d/d-target.cc
index 3ae791b5f70..86b042938af 100644
--- a/gcc/d/d-target.cc
+++ b/gcc/d/d-target.cc
@@ -189,7 +189,7 @@ unsigned
Target::alignsize (Type *type)
{
gcc_assert (type->isTypeBasic ());
- return TYPE_ALIGN_UNIT (build_ctype (type));
+ return min_align_of_type (build_ctype (type));
}
/* Return GCC field alignment size for type TYPE. */
[C++ Patch] Remove obsolete _vptr check (and fix location)
Hi,
while I was working on some location issues I noticed this check which
seems obsolete to me and means that we unnecessarily reject kosher code:
indeed we don't test for it anywhere and neither ICC nor clang enforce
it. I also went through the SVN history and the check is *extremely*
old, I think rms is the last person who touched it. The location fix is
rather obvious (in principle we could do the same for the check I'm
proposing to remove).
Tested 86_64-linux.
Thanks, Paolo.
/
/cp
2018-11-17 Paolo Carlini
* decl2.c (grokfield): Remove obsolete _vptr check; fix
explicit template argument list error location.
/testsuite
2018-11-17 Paolo Carlini
* g++.dg/template/crash91.C: Check location too.Index: cp/decl2.c
===
--- cp/decl2.c (revision 266233)
+++ cp/decl2.c (working copy)
@@ -804,7 +804,6 @@ grokfield (const cp_declarator *declarator,
tree value;
const char *asmspec = 0;
int flags;
- tree name;
if (init
&& TREE_CODE (init) == TREE_LIST
@@ -829,21 +828,12 @@ grokfield (const cp_declarator *declarator,
if (value == void_type_node)
return value;
-
- name = DECL_NAME (value);
-
- if (name != NULL_TREE)
+ if (DECL_NAME (value)
+ && TREE_CODE (DECL_NAME (value)) == TEMPLATE_ID_EXPR)
{
- if (TREE_CODE (name) == TEMPLATE_ID_EXPR)
- {
- error ("explicit template argument list not allowed");
- return error_mark_node;
- }
-
- if (IDENTIFIER_POINTER (name)[0] == '_'
- && id_equal (name, "_vptr"))
- error ("member %qD conflicts with virtual function table field name",
- value);
+ error_at (declarator->id_loc,
+ "explicit template argument list not allowed");
+ return error_mark_node;
}
/* Stash away type declarations. */
Index: testsuite/g++.dg/template/crash91.C
===
--- testsuite/g++.dg/template/crash91.C (revision 266231)
+++ testsuite/g++.dg/template/crash91.C (working copy)
@@ -4,5 +4,5 @@ template void foo();
struct A
{
- typedef void foo<0>(); // { dg-error "explicit template argument list not
allowed" }
+ typedef void foo<0>(); // { dg-error "16:explicit template argument list not
allowed" }
};
