[PATCH] Increase move cost between mask and gpr.
kmovd only uses port5 which is often the bottleneck of
performance. Also from latency perspective, spill and reload mostly
could be STLF or even MRN which only take 1 cycle.
So the patch increase move cost between gpr and mask to be the same as
gpr <-> sse register.
Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ready to push to trunk.
gcc/ChangeLog:
* config/i386/x86-tune-costs.h (skylake_cost): Increase gpr
<-> mask cost from 5 to 6.
(icelake_cost): Ditto.
gcc/testsuite/ChangeLog:
* gcc.target/i386/spill_to_mask-1.c: New test.
---
gcc/config/i386/x86-tune-costs.h| 4 ++--
gcc/testsuite/gcc.target/i386/spill_to_mask-1.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/config/i386/x86-tune-costs.h b/gcc/config/i386/x86-tune-costs.h
index 017ffa69958..05cbd49ec87 100644
--- a/gcc/config/i386/x86-tune-costs.h
+++ b/gcc/config/i386/x86-tune-costs.h
@@ -1866,7 +1866,7 @@ struct processor_costs skylake_cost = {
{8, 8, 8, 12, 24}, /* cost of storing SSE registers
in 32,64,128,256 and 512-bit */
6, 6,/* SSE->integer and integer->SSE moves
*/
- 5, 5,/* mask->integer and integer->mask
moves */
+ 6, 6,/* mask->integer and integer->mask
moves */
{8, 8, 8}, /* cost of loading mask register
in QImode, HImode, SImode. */
{6, 6, 6}, /* cost if storing mask register
@@ -1992,7 +1992,7 @@ struct processor_costs icelake_cost = {
{8, 8, 8, 12, 24}, /* cost of storing SSE registers
in 32,64,128,256 and 512-bit */
6, 6,/* SSE->integer and integer->SSE moves
*/
- 5, 5,/* mask->integer and integer->mask
moves */
+ 6, 6,/* mask->integer and integer->mask
moves */
{8, 8, 8}, /* cost of loading mask register
in QImode, HImode, SImode. */
{6, 6, 6}, /* cost if storing mask register
diff --git a/gcc/testsuite/gcc.target/i386/spill_to_mask-1.c
b/gcc/testsuite/gcc.target/i386/spill_to_mask-1.c
index 94d6764fc56..be19239a685 100644
--- a/gcc/testsuite/gcc.target/i386/spill_to_mask-1.c
+++ b/gcc/testsuite/gcc.target/i386/spill_to_mask-1.c
@@ -120,7 +120,7 @@ void foo (DTYPE in[16], DTYPE out[8], const DTYPE C[16])
out[7] += h;
}
-/* { dg-final { scan-assembler "kmovd" } } */
+/* { dg-final { scan-assembler "kmovd" { xfail *-*-* } } } */
/* { dg-final { scan-assembler-not "knot" } } */
/* { dg-final { scan-assembler-not "kxor" } } */
/* { dg-final { scan-assembler-not "kor" } } */
--
2.18.1
Re: [PATCH] gdc 9, 10 and 11 bug fix
On Tue, 17 May 2022, Marc Aurèle La France wrote:
On Tue, 17 May 2022, Iain Buclaw wrote:
Excerpts from Marc Aurèle La France's message of Mai 17, 2022 5:31 pm:
On Tue, 17 May 2022, Iain Buclaw wrote:
Excerpts from Marc Aurèle La France's message of Mai 16, 2022 11:34 pm:
On Sun, 15 May 2022, Iain Buclaw wrote:
Excerpts from Marc Aurèle La France's message of Mai 12, 2022 10:29 pm:
No compiler has any business rejecting files for the sole crime of
being symlinked to. The following applies, modulo patch fuzz, to the
9, 10 and 11 series of compilers.
Given my use of shadow trees, this bug attempted to prevent me from
building 12.1.0. The D-based gdc in 12.1.0 and up does not exhibit
this quirky behaviour.
Thanks, I've checked upstream and see the following change:
https://github.com/dlang/dmd/pull/11836/commits/ebda81e44fd0ca4b247a1860d9bef411c41c16cb
It should be fine to just backport that.
Thanks for the pointer.
I ended up with the three slightly different diffs below, one each for
the 9, 10 and 11 branches. Each was rebuilt using 8.5.0, then used to
rebuild 12.1.0. All of this ran smoothly without complaint, although I
wouldn't want to do this on a 486...
Signed-off-by: Marc Aurèle La France
For GCC 9 -- 8< --
diff -aNpRruz -X /etc/diff.excludes gcc-9.4.0/gcc/d/dmd/root/filename.c
devel-9.4.0/gcc/d/dmd/root/filename.c
--- gcc-9.4.0/gcc/d/dmd/root/filename.c 2021-06-01 01:53:04.716474774
-0600
+++ devel-9.4.0/gcc/d/dmd/root/filename.c 2022-05-15 15:02:49.995441507
-0600
@@ -475,53 +475,7 @@ const char *FileName::safeSearchPath(Strings *path,
const char *name)
return FileName::searchPath(path, name, false);
#elif POSIX
-/* Even with realpath(), we must check for // and disallow it
- */
-for (const char *p = name; *p; p++)
-{
-char c = *p;
-if (c == '/' && p[1] == '/')
-{
-return NULL;
-}
-}
I'd keep this check in, otherwise removing/replacing only the `if
(path)` branch looks OK to me.
The corresponding D code doesn't care about double slashes and neither
should this. Also, the comment is misleading as realpath() would no
longer be used here.
True, but the D front-end does check the path in other places now:
https://github.com/dlang/dmd/blob/e9ba29d71b557fe079e95ee6554f116b24159bab/src/dmd/root/filename.d#L787-L803
https://github.com/dlang/dmd/blob/e9ba29d71b557fe079e95ee6554f116b24159bab/src/dmd/expressionsem.d#L5984-L6007
If we remove all checks, then there'd be nothing to prevent either
import("/file") or import("../file") from being used.
There is still no check for double slashes. All I want here is to fix a C++
-> D migration bug without leaving any detritus behind.
Anything more on this?
Marc.
[PATCH] libstdc++-v3: Set 26_numerics/random/n.b.d./op./values.cc timeout-factor to 3
Also, how about shortening those test-suite file-paths?
They're path sort-of overlong for any git (and ChangeLog)
commit-line limit. 1/2 :-)
Ok to commit (without renaming)?
--- 8< ---
I happened to run my cris-elf regression-tester and some
large job in parallel when this test was running. According
to "time" it takes 4:39.07 for the GNU cris-elf simulator on
this yesteryear machine. The default timeout is 360
seconds. Let's "up" the time by a factor 3.
libstdc++-v3:
*
testsuite/26_numerics/random/negative_binomial_distribution/operators/values.cc:
Set dg-timeout-factor to 3.
---
.../random/negative_binomial_distribution/operators/values.cc| 1 +
1 file changed, 1 insertion(+)
diff --git
a/libstdc++-v3/testsuite/26_numerics/random/negative_binomial_distribution/operators/values.cc
b/libstdc++-v3/testsuite/26_numerics/random/negative_binomial_distribution/operators/values.cc
index dda6f43b2540..89a2b3dc4b76 100644
---
a/libstdc++-v3/testsuite/26_numerics/random/negative_binomial_distribution/operators/values.cc
+++
b/libstdc++-v3/testsuite/26_numerics/random/negative_binomial_distribution/operators/values.cc
@@ -1,4 +1,5 @@
// { dg-do run { target c++11 } }
+// { dg-timeout-factor 3 }
// { dg-require-cstdint "" }
// { dg-require-cmath "" }
//
--
2.30.2
Re: [PATCH] Add divide by zero side effect.
On 5/19/22 18:23, Segher Boessenkool wrote: On Thu, May 19, 2022 at 09:22:32AM -0400, Andrew MacLeod wrote: On 5/18/22 16:40, Segher Boessenkool wrote: On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote: On 5/18/22 14:13, Segher Boessenkool wrote: "Side effect" already has a meaning, very commonly used in language theory, and even in the C standard itself: a function has a side effect if it does something more than just return a value: if it changes state. This can be some I/O, or it can just be writing to some non-local data. Side effects are crucial to what a compiler does, and they are used all over the place (the gcc/ dir has some thousand mentions of it for example). Please don't make life hard for everyone by overloading this term. I'm open to suggestions for a better term! Glad to hear that, and this isn't set in stione yet! Is there a commonly used alternate term to describe an observable effect on the value of an input operand? I'd use something with "known" in the name. But: As far as I understand what you are doing this is not an effect on the operand at all! It cannot be one even, the operand is an input only after all. Instead, it changes what is known about the value of that input: it cannot be 0 in this case, it is known to not be 0. This is similar to other known value things we have in GCC already. Can you not just use one of those, even? What are the benefit to this new abstraction? Well, This is a component of ranger tracking value ranges.. it is recording the "side-effect" of the stmt on the known range of an object. The file is called "gimple-range-side-effect.h" So the file name is confusingly wrong as well. Its a generalization of how ranger tracks non-null pointer values, enabling it to track arbitrary observable ranges values. (The old mechanism also utilized immediate-use chains, which prevented it from being utilized in gimple-folding) WIth this change, we can also track things like a = b / c causing the effect that c is known non-zero after the statement if there were no traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which after 15 years, we can now simply indicate that for a = b >> c , its only defined behaviour if c is in the range [0, PRECISION(b)] So its basically just a generalization of how we track known values within the range system. Sure. Just the name is harmful :-( Still waiting for a suggestion, since "side effect" is the description that made sense to me :-) Andrew
Re: [PATCH v2] x86: Document -mcet-switch
On Thu, May 19, 2022 at 1:49 PM Florian Weimer wrote: > > * H. J. Lu: > > > How about this? > > > > @item -mcet-switch > > @opindex mcet-switch > > By default, CET instrumentation is turned off on switch statements that > > use a jump table and indirect branch track is disabled. > > Maybe add here: “Since jump tables are stored in read-only memory, this > does not result in a direct loss of hardening. But if the jump table > index is attacker-controlled, the indirect jump may not be constrained > by CET.” > > > This option turns on CET instrumentation to enable indirect branch > > track for switch statements with jump tables. > > “This results in a loss of hardening because the jump targets are mow > reachable via all indirect jumps.” Like this? @item -mcet-switch @opindex mcet-switch By default, CET instrumentation is turned off on switch statements that use a jump table and indirect branch track is disabled. Since jump tables are stored in read-only memory, this does not result in a direct loss of hardening. But if the jump table index is attacker-controlled, the indirect jump may not be constrained by CET. This option turns on CET instrumentation to enable indirect branch track for switch statements with jump tables which leads to the jump targets reachable via any indirect jumps. > Maybe GCC should just emit a forced (unoptimized) bounds check for jump > tables in CET mode … > > Thanks, > Florian > -- H.J.
Re: [PATCH] Remove GIMPLE restriction of ! using match.pd patterns
> From: Richard Biener via Gcc-patches > Date: Mon, 9 May 2022 13:42:27 +0200 > This removes #if GIMPLE guards around patterns using ! which is > now also provided in the GENERIC implementation. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. > > 2022-05-09 Richard Biener > > * match.pd: Remove #if GIMPLE guards around ! using patterns. This cause PR105629, "[13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x". Code quality seems to have regressed for cris-elf by a quick diff of the pr94589-2.s:es. brgds, H-P
[committed] libstdc++: Avoid including for std::char_traits
Tested powerpc64le-linux, pushed to trunk.
-- >8 --
We should prefer the __UINT_LEAST16_TYPE__ and __UINT_LEAST32_TYPE__
macros, if available, so that we don't need all of in every
header that uses std::char_traits.
libstdc++-v3/ChangeLog:
* include/bits/char_traits.h: Only include when
necessary.
* include/std/stacktrace: Use __UINTPTR_TYPE__ instead of
uintptr_t.
* src/c++11/cow-stdexcept.cc: Include .
* src/c++17/floating_to_chars.cc: Likewise.
* testsuite/20_util/assume_aligned/1.cc: Include .
* testsuite/20_util/assume_aligned/3.cc: Likewise.
* testsuite/20_util/shared_ptr/creation/array.cc: Likewise.
---
libstdc++-v3/include/bits/char_traits.h | 17 +
libstdc++-v3/include/std/stacktrace | 13 +++--
libstdc++-v3/src/c++11/cow-stdexcept.cc | 4
libstdc++-v3/src/c++17/floating_to_chars.cc | 2 ++
.../testsuite/20_util/assume_aligned/1.cc | 1 +
.../testsuite/20_util/assume_aligned/3.cc | 1 +
.../20_util/shared_ptr/creation/array.cc| 1 +
7 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/libstdc++-v3/include/bits/char_traits.h
b/libstdc++-v3/include/bits/char_traits.h
index cac1326d4b5..b856b1da320 100644
--- a/libstdc++-v3/include/bits/char_traits.h
+++ b/libstdc++-v3/include/bits/char_traits.h
@@ -40,6 +40,9 @@
#include// For WEOF, wmemmove, wmemset, etc.
#if __cplusplus >= 201103L
# include
+#if !defined __UINT_LEAST16_TYPE__ || !defined __UINT_LEAST32_TYPE__
+# include
+#endif
#endif
#if __cplusplus >= 202002L
# include
@@ -728,8 +731,6 @@ _GLIBCXX_END_NAMESPACE_VERSION
#if __cplusplus >= 201103L
-#include
-
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -738,10 +739,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct char_traits
{
typedef char16_t char_type;
-#ifdef _GLIBCXX_USE_C99_STDINT_TR1
- typedef uint_least16_tint_type;
-#elif defined __UINT_LEAST16_TYPE__
+#ifdef __UINT_LEAST16_TYPE__
typedef __UINT_LEAST16_TYPE__int_type;
+#elif defined _GLIBCXX_USE_C99_STDINT_TR1
+ typedef uint_least16_tint_type;
#else
typedef make_unsigned::type int_type;
#endif
@@ -859,10 +860,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct char_traits
{
typedef char32_t char_type;
-#ifdef _GLIBCXX_USE_C99_STDINT_TR1
- typedef uint_least32_tint_type;
-#elif defined __UINT_LEAST32_TYPE__
+#ifdef __UINT_LEAST32_TYPE__
typedef __UINT_LEAST32_TYPE__int_type;
+#elif defined _GLIBCXX_USE_C99_STDINT_TR1
+ typedef uint_least32_tint_type;
#else
typedef make_unsigned::type int_type;
#endif
diff --git a/libstdc++-v3/include/std/stacktrace
b/libstdc++-v3/include/std/stacktrace
index 1efaf253abb..39472d7236a 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -52,20 +52,20 @@ __glibcxx_backtrace_create_state(const char*, int,
int
__glibcxx_backtrace_simple(__glibcxx_backtrace_state*, int,
- int (*) (void*, uintptr_t),
+ int (*) (void*, __UINTPTR_TYPE__),
void(*)(void*, const char*, int),
void*);
int
-__glibcxx_backtrace_pcinfo(__glibcxx_backtrace_state*, uintptr_t,
- int (*)(void*, uintptr_t,
+__glibcxx_backtrace_pcinfo(__glibcxx_backtrace_state*, __UINTPTR_TYPE__,
+ int (*)(void*, __UINTPTR_TYPE__,
const char*, int, const char*),
void(*)(void*, const char*, int),
void*);
int
-__glibcxx_backtrace_syminfo(__glibcxx_backtrace_state*, uintptr_t addr,
- void (*) (void*, uintptr_t, const char*,
- uintptr_t, uintptr_t),
+__glibcxx_backtrace_syminfo(__glibcxx_backtrace_state*, __UINTPTR_TYPE__ addr,
+ void (*) (void*, __UINTPTR_TYPE__, const char*,
+ __UINTPTR_TYPE__, __UINTPTR_TYPE__),
void(*)(void*, const char*, int),
void*);
}
@@ -225,6 +225,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
class basic_stacktrace
{
using _AllocTraits = allocator_traits<_Allocator>;
+ using uintptr_t = __UINTPTR_TYPE__;
public:
using value_type = stacktrace_entry;
diff --git a/libstdc++-v3/src/c++11/cow-stdexcept.cc
b/libstdc++-v3/src/c++11/cow-stdexcept.cc
index 5ccf2ca2d74..9e8787485eb 100644
--- a/libstdc++-v3/src/c++11/cow-stdexcept.cc
+++ b/libstdc++-v3/src/c++11/cow-stdexcept.cc
@@ -200,6 +200,10 @@ _GLIBCXX_END_NAMESPACE_VERSION
#if _GLIBCXX_USE_WEAK_REF
#ifdef _GLIBCXX_USE_C99_STDINT_TR1
+#include
+
+using std::size_t;
+
extern "C" {
#ifndef _GLIBCXX_MANGLE_SIZE_T
diff --git
[committed] libstdc++: Only include for COW string
Tested powerpc64le-linux, pushed to trunk. -- >8 -- Since the COW std::string was moved to its own header, we don't need the atomic dispatch helpers in the definition of std::__cxx11::string. Move the inclusion of the header to where it's needed. libstdc++-v3/ChangeLog: * include/bits/basic_string.h: Do not include here. * include/bits/cow_string.h: Include it here. --- libstdc++-v3/include/bits/basic_string.h | 1 - libstdc++-v3/include/bits/cow_string.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index c3fbc53953c..6041d05815b 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -36,7 +36,6 @@ #pragma GCC system_header -#include #include #include diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h index a49a5b04f2f..7bceb06a79b 100644 --- a/libstdc++-v3/include/bits/cow_string.h +++ b/libstdc++-v3/include/bits/cow_string.h @@ -34,6 +34,8 @@ #if ! _GLIBCXX_USE_CXX11_ABI +#include // _Atomic_word, __is_single_threaded + #ifdef __cpp_lib_is_constant_evaluated // Support P1032R1 in C++20 (but not P0980R1 for COW strings). # define __cpp_lib_constexpr_string 201811L -- 2.34.3
[committed] libstdc++: Ensure pmr aliases work without
Tested powerpc64le-linux, pushed to trunk. This adds a lot more code to every container header, which is unfortunate. I plan to replace the inclusion of in the header with something smaller that just defines std:tuple, and not all the helpers like std::make_tuple. -- >8 -- Currently the alias templates for std::pmr::vector, std::pmr::string etc. are defined using a forward declaration for polymorphic_allocator. This means you can't actually use the alias templates unless you also include . The rationale for that is that it's a fairly large header, and most users don't need it. This isn't uncontroversial though, and LWG 3681 questions whether it's even conforming. This change adds a new header with the minimum needed to use polymorphic_allocator and the std::pmr container aliases. Including is still necessary to use the program-wide resource objects, or the pool resources or monotonic buffer resource. libstdc++-v3/ChangeLog: * include/Makefile.am: Add new header. * include/Makefile.in: Regenerate. * include/bits/memory_resource.h: New file. * include/std/deque: Include . * include/std/forward_list: Likewise. * include/std/list: Likewise. * include/std/map: Likewise. * include/std/memory_resource (pmr::memory_resource): Move to new header. (pmr::polymorphic_allocator): Likewise. * include/std/regex: Likewise. * include/std/set: Likewise. * include/std/stacktrace: Likewise. * include/std/string: Likewise. * include/std/unordered_map: Likewise. * include/std/unordered_set: Likewise. * include/std/vector: Likewise. * testsuite/21_strings/basic_string/types/pmr_typedefs.cc: Remove header and check construction. * testsuite/23_containers/deque/types/pmr_typedefs.cc: Likewise. * testsuite/23_containers/forward_list/pmr_typedefs.cc: Likewise. * testsuite/23_containers/list/pmr_typedefs.cc: Likewise. * testsuite/23_containers/map/pmr_typedefs.cc: Likewise. * testsuite/23_containers/multimap/pmr_typedefs.cc: Likewise. * testsuite/23_containers/multiset/pmr_typedefs.cc: Likewise. * testsuite/23_containers/set/pmr_typedefs.cc: Likewise. * testsuite/23_containers/unordered_map/pmr_typedefs.cc: Likewise. * testsuite/23_containers/unordered_multimap/pmr_typedefs.cc: Likewise. * testsuite/23_containers/unordered_multiset/pmr_typedefs.cc: Likewise. * testsuite/23_containers/unordered_set/pmr_typedefs.cc: Likewise. * testsuite/23_containers/vector/pmr_typedefs.cc: Likewise. * testsuite/28_regex/match_results/pmr_typedefs.cc: Likewise. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/variadic-tuple.C: Qualify function to avoid ADL finding std::make_tuple. --- gcc/testsuite/g++.dg/cpp0x/variadic-tuple.C | 2 +- libstdc++-v3/include/Makefile.am | 1 + libstdc++-v3/include/Makefile.in | 1 + libstdc++-v3/include/bits/memory_resource.h | 507 ++ libstdc++-v3/include/std/deque| 2 +- libstdc++-v3/include/std/forward_list | 2 +- libstdc++-v3/include/std/list | 2 +- libstdc++-v3/include/std/map | 2 +- libstdc++-v3/include/std/memory_resource | 464 +--- libstdc++-v3/include/std/regex| 2 +- libstdc++-v3/include/std/set | 2 +- libstdc++-v3/include/std/stacktrace | 2 +- libstdc++-v3/include/std/string | 2 +- libstdc++-v3/include/std/unordered_map| 2 +- libstdc++-v3/include/std/unordered_set| 2 +- libstdc++-v3/include/std/vector | 2 +- .../basic_string/types/pmr_typedefs.cc| 3 +- .../23_containers/deque/types/pmr_typedefs.cc | 3 +- .../forward_list/pmr_typedefs.cc | 3 +- .../23_containers/list/pmr_typedefs.cc| 3 +- .../23_containers/map/pmr_typedefs.cc | 3 +- .../23_containers/multimap/pmr_typedefs.cc| 3 +- .../23_containers/multiset/pmr_typedefs.cc| 3 +- .../23_containers/set/pmr_typedefs.cc | 3 +- .../unordered_map/pmr_typedefs.cc | 3 +- .../unordered_multimap/pmr_typedefs.cc| 3 +- .../unordered_multiset/pmr_typedefs.cc| 3 +- .../unordered_set/pmr_typedefs.cc | 3 +- .../23_containers/vector/pmr_typedefs.cc | 3 +- .../28_regex/match_results/pmr_typedefs.cc| 3 +- 30 files changed, 551 insertions(+), 488 deletions(-) create mode 100644 libstdc++-v3/include/bits/memory_resource.h diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-tuple.C b/gcc/testsuite/g++.dg/cpp0x/variadic-tuple.C index b1c6b3d7814..e2699d94ebf 100644 --- a/gcc/testsuite/g++.dg/cpp0x/variadic-tuple.C +++ b/gcc/testsuite/g++.dg/cpp0x/variadic-tuple.C @@ -272,7 +272,7 @@ int
Re: [PATCH] Add divide by zero side effect.
On Thu, May 19, 2022 at 09:22:32AM -0400, Andrew MacLeod wrote: > On 5/18/22 16:40, Segher Boessenkool wrote: > >On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote: > >>On 5/18/22 14:13, Segher Boessenkool wrote: > >>>"Side effect" already has a meaning, very commonly used in language > >>>theory, and even in the C standard itself: a function has a side effect > >>>if it does something more than just return a value: if it changes state. > >>>This can be some I/O, or it can just be writing to some non-local data. > >>> > >>>Side effects are crucial to what a compiler does, and they are used all > >>>over the place (the gcc/ dir has some thousand mentions of it for > >>>example). > >>> > >>>Please don't make life hard for everyone by overloading this term. > >>> > >>I'm open to suggestions for a better term! > >Glad to hear that, and this isn't set in stione yet! > > > >>Is there a commonly used alternate term to describe an observable effect > >>on the value of an input operand? > >I'd use something with "known" in the name. But: > > > >As far as I understand what you are doing this is not an effect on the > >operand at all! It cannot be one even, the operand is an input only > >after all. Instead, it changes what is known about the value of that > >input: it cannot be 0 in this case, it is known to not be 0. > > > >This is similar to other known value things we have in GCC already. Can > >you not just use one of those, even? What are the benefit to this new > >abstraction? > > Well, This is a component of ranger tracking value ranges.. it is > recording the "side-effect" of the stmt on the known range of an object. > The file is called "gimple-range-side-effect.h" So the file name is confusingly wrong as well. > Its a generalization of how ranger tracks non-null pointer values, > enabling it to track arbitrary observable ranges values. (The old > mechanism also utilized immediate-use chains, which prevented it from > being utilized in gimple-folding) > > WIth this change, we can also track things like a = b / c causing the > effect that c is known non-zero after the statement if there were no > traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which > after 15 years, we can now simply indicate that for a = b >> c , its > only defined behaviour if c is in the range [0, PRECISION(b)] > > So its basically just a generalization of how we track known values > within the range system. Sure. Just the name is harmful :-( Segher
Re: [PATCH v2] x86: Document -mcet-switch
* H. J. Lu: > How about this? > > @item -mcet-switch > @opindex mcet-switch > By default, CET instrumentation is turned off on switch statements that > use a jump table and indirect branch track is disabled. Maybe add here: “Since jump tables are stored in read-only memory, this does not result in a direct loss of hardening. But if the jump table index is attacker-controlled, the indirect jump may not be constrained by CET.” > This option turns on CET instrumentation to enable indirect branch > track for switch statements with jump tables. “This results in a loss of hardening because the jump targets are mow reachable via all indirect jumps.” Maybe GCC should just emit a forced (unoptimized) bounds check for jump tables in CET mode … Thanks, Florian
Re: [PATCH v2] x86: Document -mcet-switch
On Wed, May 18, 2022 at 11:02 PM Richard Biener
wrote:
>
> On Wed, May 18, 2022 at 7:32 PM H.J. Lu via Gcc-patches
> wrote:
> >
> > On Wed, May 11, 2022 at 1:52 PM H.J. Lu wrote:
> > >
> > > When -fcf-protection=branch is used, the compiler will generate jump
> > > tables for switch statements where the indirect jump is prefixed with
> > > the NOTRACK prefix, so it can jump to non-ENDBR targets. Since the
> > > indirect jump targets are generated by the compiler, they are always
> > > valid.
> > >
> > > Document -mcet-switch to generate jump tables for switch statements with
> > > ENDBR and skip the NOTRACK prefix for indirect jump. This option should
> > > be used when the NOTRACK prefix is disabled.
> > >
> > > PR target/104816
> > > * config/i386/i386.opt: Remove Undocumented.
> > > * doc/invoke.texi: Document -mcet-switch.
> > > ---
> > > gcc/config/i386/i386.opt | 2 +-
> > > gcc/doc/invoke.texi | 8 +++-
> > > 2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
> > > index a6b0e28f238..0dbaacb57ed 100644
> > > --- a/gcc/config/i386/i386.opt
> > > +++ b/gcc/config/i386/i386.opt
> > > @@ -1047,7 +1047,7 @@ Enable shadow stack built-in functions from
> > > Control-flow Enforcement
> > > Technology (CET).
> > >
> > > mcet-switch
> > > -Target Undocumented Var(flag_cet_switch) Init(0)
> > > +Target Var(flag_cet_switch) Init(0)
> > > Turn on CET instrumentation for switch statements that use a jump table
> > > and
> > > an indirect jump.
> > >
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 7a35d9613a4..968853690f9 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -1420,7 +1420,8 @@ See RS/6000 and PowerPC Options.
> > > -msse4a -m3dnow -m3dnowa -mpopcnt -mabm -mbmi -mtbm -mfma4 -mxop
> > > @gol
> > > -madx -mlzcnt -mbmi2 -mfxsr -mxsave -mxsaveopt -mrtm -mhle -mlwp
> > > @gol
> > > -mmwaitx -mclzero -mpku -mthreads -mgfni -mvaes -mwaitpkg @gol
> > > --mshstk -mmanual-endbr -mforce-indirect-call -mavx512vbmi2 -mavx512bf16
> > > -menqcmd @gol
> > > +-mshstk -mmanual-endbr -mcet-switch -mforce-indirect-call @gol
> > > +-mavx512vbmi2 -mavx512bf16 -menqcmd @gol
> > > -mvpclmulqdq -mavx512bitalg -mmovdiri -mmovdir64b -mavx512vpopcntdq
> > > @gol
> > > -mavx5124fmaps -mavx512vnni -mavx5124vnniw -mprfchw -mrdpid @gol
> > > -mrdseed -msgx -mavx512vp2intersect -mserialize -mtsxldtrk@gol
> > > @@ -32641,6 +32642,11 @@ function attribute. This is useful when used
> > > with the option
> > > @option{-fcf-protection=branch} to control ENDBR insertion at the
> > > function entry.
> > >
> > > +@item -mcet-switch
> > > +@opindex mcet-switch
> > > +Turn on CET instrumentation for switch statements that use a jump table
> > > +and an indirect jump.
> > > +
>
> Can you explain the difference between -mcet-switch and -mno-cet-switch here
> please and indicate the default? The above doesn't sound useful to me.
How about this?
@item -mcet-switch
@opindex mcet-switch
By default, CET instrumentation is turned off on switch statements that
use a jump table and indirect branch track is disabled. This option
turns on CET instrumentation to enable indirect branch track for switch
statements with jump tables.
--
H.J.
Re: [PATCH v3 09/10] libgcc: Add support for HF mode (aka _Float16) in libbid
On Thu, 19 May 2022, Christophe Lyon via Gcc-patches wrote: > Hi Joseph, > > May I ping you on this version? (I've moved the tests to gcc.dg/torture/ and > checked they work on aarch64 and x86_64. This version is OK, given a bug report filed in Bugzilla for the double-rounding issues with conversions from DFP modes to HFmode. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, OpenMP, v2] Implement uses_allocators clause for target regions
On Thu, May 19, 2022 at 06:02:43PM +0100, Andrew Stubbs wrote: > On 19/05/2022 17:00, Jakub Jelinek wrote: > > Without requires dynamic_allocators, there are various extra restrictions > > imposed: > > 1) omp_init_allocator/omp_destroy_allocator may not be called (except for > > implicit calls to it from uses_allocators) in a target region > > I interpreted that more like "omp_init_allocator/... is not required to > work", as in the set-up steps provided by dynamic_allocators/uses_allocators > won't be available. Since we don't have any such on/off mode I don't believe > we need to worry about this (and adding extra logic for this is make-work > which will not improve the user-experience). Unfortunately OpenMP as the standard doesn't bother too much with distinctions that e.g. the C/C++ standards make, whether something makes the TU invalid or whether something is only invalid at runtime when reaching it. In any case, I think it would be nice if we diagnosed such uses, doesn't need to be an error, warning would be fine, but help users write portable code, either that they requires dynamic_allocators, or don't and limit themselves to what the standard says should be used in that case. I guess a warning might be better, because we really don't know if it will be actually called at runtime or not from the target region. > > 2) omp_alloc etc. can't be called with omp_null_allocator and the argument > > has to be a constant expression for a predefined memory allocator > > (that is also mentioned on uses_allocators, though that doesn't have to > > be visible in the source because it could be lexically included in > > a target construct's body) > > Again, does a conforming implementation reject this, or is it merely not > required to accept it? I think it is another fuzzy area, but again it would be nice to at least get warnings. Jakub
Re: [PATCH, OpenMP, v2] Implement uses_allocators clause for target regions
On Tue, May 10, 2022 at 07:29:23PM +0800, Chung-Lin Tang wrote:
> @@ -15624,6 +15626,233 @@ c_parser_omp_clause_allocate (c_parser *parser,
> tree list)
>return nl;
> }
>
> +/* OpenMP 5.2:
> + uses_allocators ( allocator-list )
As uses_allocators is a 5.0 feature already, the above should say
/* OpenMP 5.0:
> +
> + allocator-list:
> + allocator
> + allocator , allocator-list
> + allocator ( traits-array )
> + allocator ( traits-array ) , allocator-list
> +
And here it should add
OpenMP 5.2:
> + uses_allocators ( modifier : allocator )
> + uses_allocators ( modifier , modifier : allocator )
> +
> + modifier:
> + traits ( traits-array )
> + memspace ( mem-space-handle ) */
> +
> +static tree
> +c_parser_omp_clause_uses_allocators (c_parser *parser, tree list)
> +{
> + location_t clause_loc = c_parser_peek_token (parser)->location;
> + tree t = NULL_TREE, nl = list;
> + matching_parens parens;
> + if (!parens.require_open (parser))
> +return list;
> +
> + bool has_modifiers = false;
> + tree memspace_expr = NULL_TREE;
> + tree traits_var = NULL_TREE;
> +
> + if (c_parser_next_token_is (parser, CPP_NAME))
> +{
> + c_token *tok = c_parser_peek_token (parser);
> + const char *p = IDENTIFIER_POINTER (tok->value);
> +
> + if (strcmp ("traits", p) == 0 || strcmp ("memspace", p) == 0)
> + {
> + has_modifiers = true;
> + c_parser_consume_token (parser);
> + matching_parens parens2;;
Double ;;, should be just ;
But more importantly, it is more complex.
When you see
uses_allocators(traits or
uses_allocators(memspace
it is not given that it has modifiers. While the 5.0/5.1 syntax had
a restriction that when allocator is not a predefined allocator (and
traits or memspace aren't predefined allocators) it must use ()s with
traits, so
uses_allocators(traits)
uses_allocators(memspace)
uses_allocators(traits,memspace)
are all invalid,
omp_allocator_handle_t traits;
uses_allocators(traits(mytraits))
or
omp_allocator_handle_t memspace;
uses_allocators(memspace(mytraits),omp_default_mem_alloc)
are valid in the old syntax.
So, I'm afraid to find out if the traits or memspace identifier
seen after uses_allocator ( are modifiers or not we need to
peek (for C with c_parser_peek_nth_token_raw) through all the
modifiers whether we see a : and only in that case say they
are modifiers rather than the old style syntax.
> + parens2.require_open (parser);
> +
> + if (c_parser_next_token_is (parser, CPP_NAME)
> + && (c_parser_peek_token (parser)->id_kind == C_ID_ID
> + || c_parser_peek_token (parser)->id_kind == C_ID_TYPENAME))
> + {
> + tok = c_parser_peek_token (parser);
> + t = lookup_name (tok->value);
> +
> + if (t == NULL_TREE)
> + {
> + undeclared_variable (tok->location, tok->value);
> + t = error_mark_node;
> + }
> + else
> + {
> + if (strcmp ("memspace", p) == 0)
I think it would be better to have bool variable whether
it was memspace or traits modifier, so strcmp just once
with each string, not multiple times.
In the 5.2 syntax, for memspace it is clear that it has to be
an identifier that is the predefined namespace, but for
traits it just says it is a variable of alloctrait array type,
it is unclear if it must be an identifier or could be say structure
element or array element etc. Guess something to discuss on omp-lang
and for now pretend it must be an identifier.
> + if (c_parser_next_token_is (parser, CPP_COMMA))
> + {
> + c_parser_consume_token (parser);
> + tok = c_parser_peek_token (parser);
> + const char *q = "";
> + if (c_parser_next_token_is (parser, CPP_NAME))
> + q = IDENTIFIER_POINTER (tok->value);
> + if (strcmp (q, "memspace") != 0 && strcmp (q, "traits") != 0)
> + {
> + c_parser_error (parser, "expected % or
> %");
> + parens.skip_until_found_close (parser);
> + return list;
> + }
I don't really like the modifiers handling not done in a loop.
As I said above, there needs to be some check whether there are modifiers or
not, but once we figure out there are modifiers, it should be done in a loop
with say some mask var on which traits have been already handled to diagnose
duplicates, we don't want to do the parsing code twice.
> + if (has_modifiers)
> +{
> + if (!c_parser_require (parser, CPP_COLON, "expected %<:%>"))
> + {
> + parens.skip_until_found_close (parser);
> + return list;
> + }
> +
> + if (c_parser_next_token_is (parser, CPP_NAME)
> + && c_parser_peek_token (parser)->id_kind == C_ID_ID)
> + {
> + tree t = lookup_name (c_parser_peek_token (parser)->value);
> +
> + if (t == NULL_TREE)
> + {
> + undeclared_variable (c_parser_peek_token
Re: [PATCH, OpenMP, v2] Implement uses_allocators clause for target regions
On 19/05/2022 17:00, Jakub Jelinek wrote:
Without requires dynamic_allocators, there are various extra restrictions
imposed:
1) omp_init_allocator/omp_destroy_allocator may not be called (except for
implicit calls to it from uses_allocators) in a target region
I interpreted that more like "omp_init_allocator/... is not required to
work", as in the set-up steps provided by
dynamic_allocators/uses_allocators won't be available. Since we don't
have any such on/off mode I don't believe we need to worry about this
(and adding extra logic for this is make-work which will not improve the
user-experience).
2) omp_alloc etc. can't be called with omp_null_allocator and the argument
has to be a constant expression for a predefined memory allocator
(that is also mentioned on uses_allocators, though that doesn't have to
be visible in the source because it could be lexically included in
a target construct's body)
Again, does a conforming implementation reject this, or is it merely not
required to accept it?
3) for allocate directive on static vars the above applies plus it has
to be mentioned in uses_allocators
4) for allocate clause e.g. when privatizing stuff or allocate directive
for automatic vars no such restrictions exist
Now, that means that e.g. the user provided uses_allocators without
requires dynamic_allocators are only useful for the case 4), it is unclear
if that was really intended.
With uses_allocators in particular, it is unclear if
uses_allocators(omp_null_allocator) is allowed or not, IMHO it shouldn't,
but I really don't see a restriction disallowing it.
Then there is the issue that 5.0/5.1 said for C/C++ that traits-array
should be
"an identifier of const omp_alloctrait_t * type."
which is wrong for multiple reasons, because identifiers don't have type,
expressions or variables do, but more importantly because from the pointer
to const omp_alloctrait_t it is impossible to find out how many elements
the traits have. 5.2 fixed that to say that it must be an array
(so we thankfully know the size), so we certainly should consider that
change like a defect report against 5.0/5.1 too and require even in the
old syntax an array. Note, I'm afraid we need to support even VLAs,
not just constant size arrays.
We are only implementing 5.0 at this time. If 5.2 is less work and the
only way to achieve correctness then maybe that's the way to go, but in
general, one step at a time, please.
There is also in the spec that when allocator in uses_allocators is
a variable, it is treated as a private variable that can't be explicitly
privatized, but nothing said about the traits array, so is say:
void foo () {
omp_allocator_handle_t h;
omp_alloctrait_t t[3] = { ... };
#pragma omp target uses_allocators(h(t)) firstprivate(t)
{
}
ok or not? We need to firstprivatize t so that we can call
h = omp_init_allocator (omp_default_mem_space, 3, t); in the target region
and it is kind of difficult to privatize the same var multiple times.
And yet another issue, in omp_alloctrait_t one can point to other allocators
(with { omp_atk_fallback, some_alloc }). If some_alloc is a predefined
allocator, fine, I don't see big deal with that, especially if that
predefined allocator is also mentioned in uses_allocators clause (before or
after). But if it is a user allocator, there is no restriction on that, and
no way how to map that, say that there should be some specific ordering
of uses_allocators induced omp_init_allocator calls and that we should
somehow replace the host value with privatized target replacement.
More on the actual patch later.
Thank you.
Re: [COMMITTED] Add ranger side effect infrastructure.
On Thu, 19 May 2022 10:52:35 -0400
David Malcolm wrote:
> > In file included from ../../../src/gcc-13.mine/gcc/tree-vrp.h:23,
> > from ../../../src/gcc-13.mine/gcc/ssa.h:28,
> > from ../../../src/gcc-13.mine/gcc/gimple-warn-
> > types.cc:30:
> > ../../../src/gcc-13.mine/gcc/value-range.h: In member function
> > ‘unsigned int irange::num_pairs() const’:
> > ../../../src/gcc-13.mine/gcc/value-range.h:217:22: warning: Function
> > ‘num_pairs’ could return ‘unsigned char’ [--all-warnings]
> > 217 | irange::num_pairs () const
> > | ^
> > | unsigned char
> > ../../../src/gcc-13.mine/gcc/value-range.h:217:22: note: with a range
> > of [0,255]
>
> The location for that warning does look wrong, but this doesn't quite
> look like one of our warnings - was this from GCC? Which version?
> Otherwise, which compiler emitted it?
It's a pass i'm writing to help me deal with some fortran PR.
And yes, as said, the location is wrong, see
https://gcc.gnu.org/pipermail/gcc-help/2022-April/141434.html
where i asked about how to deal with this. Moving the building to
grokfndecl, like Jason suggested, does not seem completely trivial, so
i'm leaving this alone. We don't have them in the fortran FE, so
i don't really care much right now TBH. Maybe i'll get back to that
later.
Of course I won't complain if somebody more familiar with the C++ FE
fixes it.
> > The C++ member function decl result locations are all wrong, i know,
> > but i think the warning is correct. We've got:
> > class GTY((user)) irange
> > {
> > unsigned char m_num_ranges;
> > }
> > but
> > inline unsigned
> > irange::num_pairs () const
> > {
> > if (m_kind == VR_ANTI_RANGE)
> > return constant_p () ? 2 : 1;
> > else
> > return m_num_ranges;
> > }
>
> I don't see what's wrong with the code. It's OK to promote an unsigned
> char to an unsigned int, isn't it? The compiler is required to
> preserve the value.
It's sure ok to promote it, at least later on, if
targetm.calls.promote_prototypes or by other means if deemed profitable.
For PR78798 i want to demote the types though.
> Sorry if I'm missing something here
np :)
Re: [PATCH, OpenMP, v2] Implement uses_allocators clause for target regions
On Tue, May 10, 2022 at 07:29:23PM +0800, Chung-Lin Tang wrote:
> I've attached v2 of the patch. Currently in testing.
Just a general rant, the non-requires dynamic_allocators support
seems to be a total mess in the standard. Probably something
that should be discussed on omp-lang.
Allocators can appear in various places, with requires dynamic_allocators
it is all clear, the only allocators required to be constant expressions
(aka predefined allocators) are on allocate for non-automatic variables
(my understanding is that omp_null_allocator isn't valid for those but I
could be wrong), but there are no other requirements imposed (except of
course referencing a destroyed or not yet initialized allocator is UB),
in particular omp_alloc etc. can be passed omp_null_allocator, or a
variable, and similarly for allocate clause etc.
Without requires dynamic_allocators, there are various extra restrictions
imposed:
1) omp_init_allocator/omp_destroy_allocator may not be called (except for
implicit calls to it from uses_allocators) in a target region
2) omp_alloc etc. can't be called with omp_null_allocator and the argument
has to be a constant expression for a predefined memory allocator
(that is also mentioned on uses_allocators, though that doesn't have to
be visible in the source because it could be lexically included in
a target construct's body)
3) for allocate directive on static vars the above applies plus it has
to be mentioned in uses_allocators
4) for allocate clause e.g. when privatizing stuff or allocate directive
for automatic vars no such restrictions exist
Now, that means that e.g. the user provided uses_allocators without
requires dynamic_allocators are only useful for the case 4), it is unclear
if that was really intended.
With uses_allocators in particular, it is unclear if
uses_allocators(omp_null_allocator) is allowed or not, IMHO it shouldn't,
but I really don't see a restriction disallowing it.
Then there is the issue that 5.0/5.1 said for C/C++ that traits-array
should be
"an identifier of const omp_alloctrait_t * type."
which is wrong for multiple reasons, because identifiers don't have type,
expressions or variables do, but more importantly because from the pointer
to const omp_alloctrait_t it is impossible to find out how many elements
the traits have. 5.2 fixed that to say that it must be an array
(so we thankfully know the size), so we certainly should consider that
change like a defect report against 5.0/5.1 too and require even in the
old syntax an array. Note, I'm afraid we need to support even VLAs,
not just constant size arrays.
There is also in the spec that when allocator in uses_allocators is
a variable, it is treated as a private variable that can't be explicitly
privatized, but nothing said about the traits array, so is say:
void foo () {
omp_allocator_handle_t h;
omp_alloctrait_t t[3] = { ... };
#pragma omp target uses_allocators(h(t)) firstprivate(t)
{
}
ok or not? We need to firstprivatize t so that we can call
h = omp_init_allocator (omp_default_mem_space, 3, t); in the target region
and it is kind of difficult to privatize the same var multiple times.
And yet another issue, in omp_alloctrait_t one can point to other allocators
(with { omp_atk_fallback, some_alloc }). If some_alloc is a predefined
allocator, fine, I don't see big deal with that, especially if that
predefined allocator is also mentioned in uses_allocators clause (before or
after). But if it is a user allocator, there is no restriction on that, and
no way how to map that, say that there should be some specific ordering
of uses_allocators induced omp_init_allocator calls and that we should
somehow replace the host value with privatized target replacement.
More on the actual patch later.
Jakub
Re: [PATCH] ipa-icf: skip variables with body_removed
> Similarly to cgraph_nodes, it may happen that body_removed is set
> during merging of symbols.
>
> PR ipa/105600
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> * ipa-icf.cc (sem_item_optimizer::filter_removed_items):
> Skip variables with body_removed.
> ---
> gcc/ipa-icf.cc | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
> index 765ae746745..6528a7a10b2 100644
> --- a/gcc/ipa-icf.cc
> +++ b/gcc/ipa-icf.cc
> @@ -2411,10 +2411,11 @@ sem_item_optimizer::filter_removed_items (void)
> {
> /* Filter out non-readonly variables. */
> tree decl = item->decl;
> - if (TREE_READONLY (decl))
> - filtered.safe_push (item);
> - else
> + varpool_node *vnode = static_cast *>(item)->get_node ();
> + if (!TREE_READONLY (decl) || vnode->body_removed)
> remove_item (item);
> + else
> + filtered.safe_push (item);
So the situation here is that we merge symbols but keep syntactic alias
because the declarations are not compatible (have different attributes
perhaps because of fortify source)?
Will ICF still see through the aliases and do merging? I think you can
craft a testcase by triggering the attribute mismatch and see what
happens. At the time we implemented ICF these aliases did not exists,
so maybe some TLC is needed here.
Honza
Re: [COMMITTED] Add ranger side effect infrastructure.
On Thu, 2022-05-19 at 15:16 +0200, Bernhard Reutner-Fischer via Gcc-
patches wrote:
> Hi Andrew,
>
> On Tue, 17 May 2022 14:38:39 -0400
> Andrew MacLeod via Gcc-patches wrote:
>
> > commit b7501739f3b14ac7749aace93f636d021fd607f7
> > Author: Andrew MacLeod
> > Date: Mon May 9 15:35:14 2022 -0400
>
> > diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> > index d3cf8be9bd8..56f4577cfbb 100644
> > --- a/gcc/gimple-range-cache.cc
> > +++ b/gcc/gimple-range-cache.cc
> > @@ -1545,81 +1426,42 @@ ranger_cache::range_from_dom (irange ,
> > tree name, basic_block start_bb,
>
> > void
> > -ranger_cache::update_to_nonnull (basic_block bb, tree name)
> > +ranger_cache::apply_side_effects (gimple *s)
> > {
>
> > - if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) ==
> > TARGET_MEM_REF)
> > + // Do not update the on-netry cache for block ending stmts.
>
> s/on-netry/on-entry/
>
> > diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-
> > side-effect.cc
> > new file mode 100644
> > index 000..2c8c77dc569
> > --- /dev/null
> > +++ b/gcc/gimple-range-side-effect.cc
>
> > +// ---
> > --
> > +
> > +// This class is an element in list of side effect ranges.
>
> s/in list/in the list/
>
> > +// Construct a side effects manager. DO_SEARCH indicates whether
> > an immediate
> > +// use scan should be made the first time a name is processed.
> > This is for
> > +// on-demand clients who may not visit every statement and may
> > miss uses.
>
> may otherwise miss uses, i suppose. But ok.
>
> > diff --git a/gcc/gimple-range-side-effect.h b/gcc/gimple-range-
> > side-effect.h
> > new file mode 100644
> > index 000..848d94ba6d7
> > --- /dev/null
> > +++ b/gcc/gimple-range-side-effect.h
> > @@ -0,0 +1,82 @@
>
> > +// New side effects should added in the constructor of this class.
>
> s/should added/should be added/
>
> > +
> > +class stmt_side_effects
> > +{
>
> > +};
> > +
> > +// This class manages a list of side effect ranges for each basic
> > block.
> > +// As side effects are seen, they can be registered to a block and
> > later
> > +// queried. WHen constructed with a TRUE flag, immediate uses
> > chains are
> > +// followed the first time a name is referenced and block
> > populated if
> > +// thre are any side effects.
>
> s/WHen/When/
> s/thre are/there are/
>
> > +
> > +class side_effect_manager
> > +{
>
> > diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > index 1fdee026a4b..f5e9e77bc71 100644
> > --- a/gcc/gimple-range.cc
> > +++ b/gcc/gimple-range.cc
>
> > @@ -118,9 +118,11 @@ gimple_ranger::range_of_expr (irange , tree
> > expr, gimple *stmt)
> > // If name is defined in this block, try to get an range
> > from S.
> > if (def_stmt && gimple_bb (def_stmt) == bb)
> > {
> > - // Check for a definition override from a block walk.
> > - if (!POINTER_TYPE_P (TREE_TYPE (expr))
> > - || !m_cache.block_range (r, bb, expr, false))
> > + // Declared in ths block, if it has a global set, check
> > for an
> > + // override from a block walk, otherwise calculate it.
>
> s/ths/this/
>
> And, btw, i see
> In file included from ../../../src/gcc-13.mine/gcc/tree-vrp.h:23,
> from ../../../src/gcc-13.mine/gcc/ssa.h:28,
> from ../../../src/gcc-13.mine/gcc/gimple-warn-
> types.cc:30:
> ../../../src/gcc-13.mine/gcc/value-range.h: In member function
> ‘unsigned int irange::num_pairs() const’:
> ../../../src/gcc-13.mine/gcc/value-range.h:217:22: warning: Function
> ‘num_pairs’ could return ‘unsigned char’ [--all-warnings]
> 217 | irange::num_pairs () const
> | ^
> | unsigned char
> ../../../src/gcc-13.mine/gcc/value-range.h:217:22: note: with a range
> of [0,255]
The location for that warning does look wrong, but this doesn't quite
look like one of our warnings - was this from GCC? Which version?
Otherwise, which compiler emitted it?
>
> The C++ member function decl result locations are all wrong, i know,
> but i think the warning is correct. We've got:
> class GTY((user)) irange
> {
> unsigned char m_num_ranges;
> }
> but
> inline unsigned
> irange::num_pairs () const
> {
> if (m_kind == VR_ANTI_RANGE)
> return constant_p () ? 2 : 1;
> else
> return m_num_ranges;
> }
I don't see what's wrong with the code. It's OK to promote an unsigned
char to an unsigned int, isn't it? The compiler is required to
preserve the value.
Sorry if I'm missing something here
Dave
[committed] libstdc++: Implement LWG 3683 for pmr::polymorphic_allocator
Tested x86_64-linux, pushed to trunk.
-- >8 --
This issue has recently been moved to Tentatively Ready, and seems
uncontroversial. This allows equality comparison with types that are
convertible to pmr::polymorphic_allocator, which fail deduction for the
existing equality operator.
libstdc++-v3/ChangeLog:
* include/std/memory_resource (polymorphic_allocator): Add
non-template equality operator, as proposed for LWG 3683.
* testsuite/20_util/polymorphic_allocator/lwg3683.cc: New test.
---
libstdc++-v3/include/std/memory_resource | 16
.../20_util/polymorphic_allocator/lwg3683.cc | 13 +
2 files changed, 29 insertions(+)
create mode 100644
libstdc++-v3/testsuite/20_util/polymorphic_allocator/lwg3683.cc
diff --git a/libstdc++-v3/include/std/memory_resource
b/libstdc++-v3/include/std/memory_resource
index 88e8abd60fa..745422ad5eb 100644
--- a/libstdc++-v3/include/std/memory_resource
+++ b/libstdc++-v3/include/std/memory_resource
@@ -356,6 +356,22 @@ namespace pmr
__attribute__((__returns_nonnull__))
{ return _M_resource; }
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 3683. operator== for polymorphic_allocator cannot deduce template arg
+ [[nodiscard]]
+ friend bool
+ operator==(const polymorphic_allocator& __a,
+const polymorphic_allocator& __b) noexcept
+ { return *__a.resource() == *__b.resource(); }
+
+#if __cpp_impl_three_way_comparison < 201907L
+ [[nodiscard]]
+ friend bool
+ operator!=(const polymorphic_allocator& __a,
+const polymorphic_allocator& __b) noexcept
+ { return !(__a == __b); }
+#endif
+
private:
#if ! __cpp_lib_make_obj_using_allocator
using __uses_alloc1_ = __uses_alloc1;
diff --git a/libstdc++-v3/testsuite/20_util/polymorphic_allocator/lwg3683.cc
b/libstdc++-v3/testsuite/20_util/polymorphic_allocator/lwg3683.cc
new file mode 100644
index 000..acc91f540b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/polymorphic_allocator/lwg3683.cc
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++17 } }
+
+#include
+
+bool
+test_lwg3683(const std::pmr::polymorphic_allocator& a)
+{
+ if (a == std::pmr::get_default_resource())
+return true;
+ if (std::pmr::get_default_resource() != a)
+return false;
+ throw a;
+}
--
2.34.3
Re: [PATCH] Fix OMP CAS expansion with separate condition
On Thu, 19 May 2022, Jakub Jelinek wrote: > On Thu, May 19, 2022 at 02:13:33PM +, Richard Biener wrote: > > > Though, depending on what exactly you allow or disallow, maybe even > > > the im != 0 might not be acceptable. > > > Oh, and if COND_EXPRs can only use some limited set of comparisons, we > > > might > > > need to adjust e.g. arith_overflow_check_p and various other spots in > > > tree-ssa-math-opts.cc and other passes. > > > > With the changes I have in the queue the !cond_stmt path should be > > effectively dead code (though I don't remove such immediately). > > > > Not sure what you think the issue with arith_overflow_check_p would be? > > Note this is only about COND_EXPRs in gimple assignments. > > Just that there will be more dead code in the compiler. > At least tree-ssa-math-opts.cc in various spots (but I think phiopt too and > other spots I've touched too) are looking for all 3 kinds of comparisons, > GIMPLE_CONDs, comparisons in COND_EXPR first operand and assignments > with tcc_comparison rhs codes and deal with all 3. If we restrict what > can appear in COND_EXPR first operand in gimple assignments, perhaps we'll > need to only deal with the first and last case and not the COND_EXPR ones. Ah, yeah - there will be followup cleanup opportunities like shaving off 5% of gimple-match.cc when I'm happy with a required phiopt adjustment. Richard.
Re: [PATCH] Fix OMP CAS expansion with separate condition
On Thu, May 19, 2022 at 02:13:33PM +, Richard Biener wrote: > > Though, depending on what exactly you allow or disallow, maybe even > > the im != 0 might not be acceptable. > > Oh, and if COND_EXPRs can only use some limited set of comparisons, we might > > need to adjust e.g. arith_overflow_check_p and various other spots in > > tree-ssa-math-opts.cc and other passes. > > With the changes I have in the queue the !cond_stmt path should be > effectively dead code (though I don't remove such immediately). > > Not sure what you think the issue with arith_overflow_check_p would be? > Note this is only about COND_EXPRs in gimple assignments. Just that there will be more dead code in the compiler. At least tree-ssa-math-opts.cc in various spots (but I think phiopt too and other spots I've touched too) are looking for all 3 kinds of comparisons, GIMPLE_CONDs, comparisons in COND_EXPR first operand and assignments with tcc_comparison rhs codes and deal with all 3. If we restrict what can appear in COND_EXPR first operand in gimple assignments, perhaps we'll need to only deal with the first and last case and not the COND_EXPR ones. Jakub
Re: [PATCH] Mitigate -Wmaybe-uninitialized in expmed.cc.
On 5/16/22 10:56, Richard Sandiford wrote:
> Martin Liška writes:
>> It's the warning I see every time I build GCC:
>>
>> In file included from /home/marxin/Programming/gcc/gcc/coretypes.h:478,
>> from /home/marxin/Programming/gcc/gcc/expmed.cc:26:
>> In function ‘poly_uint16 mode_to_bytes(machine_mode)’,
>> inlined from ‘typename if_nonpoly::type
>> GET_MODE_SIZE(const T&) [with T = scalar_int_mode]’ at
>> /home/marxin/Programming/gcc/gcc/machmode.h:647:24,
>> inlined from ‘rtx_def* emit_store_flag_1(rtx, rtx_code, rtx, rtx,
>> machine_mode, int, int, machine_mode)’ at
>> /home/marxin/Programming/gcc/gcc/expmed.cc:5728:56:
>> /home/marxin/Programming/gcc/gcc/machmode.h:550:49: warning: ‘*(unsigned
>> int*)((char*)_mode + offsetof(scalar_int_mode,
>> scalar_int_mode::m_mode))’ may be used uninitialized [-Wmaybe-uninitialized]
>> 550 | ? mode_size_inline (mode) : mode_size[mode]);
>> | ^~~~
>> /home/marxin/Programming/gcc/gcc/expmed.cc: In function ‘rtx_def*
>> emit_store_flag_1(rtx, rtx_code, rtx, rtx, machine_mode, int, int,
>> machine_mode)’:
>> /home/marxin/Programming/gcc/gcc/expmed.cc:5657:19: note: ‘*(unsigned
>> int*)((char*)_mode + offsetof(scalar_int_mode,
>> scalar_int_mode::m_mode))’ was declared here
>> 5657 | scalar_int_mode int_mode;
>> | ^~~~
>>
>> Can we please mitigate it?
>>
>> gcc/ChangeLog:
>>
>> * expmed.cc (emit_store_flag_1): Mitigate -Wmaybe-uninitialized
>> warning.
>
> Not a strong objection, but TBH I'd rather we didn't work around false
> positives like this.
Sure, but as you know, -Wmaybe-uninitialized has pretty high rate
of false positivity :/
Martin
>
> Richard
>
>> ---
>> gcc/expmed.cc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
>> index 41738c1efe9..f23d63471ea 100644
>> --- a/gcc/expmed.cc
>> +++ b/gcc/expmed.cc
>> @@ -5654,7 +5654,7 @@ emit_store_flag_1 (rtx target, enum rtx_code code, rtx
>> op0, rtx op1,
>>
>>/* If we are comparing a double-word integer with zero or -1, we can
>> convert the comparison into one involving a single word. */
>> - scalar_int_mode int_mode;
>> + scalar_int_mode int_mode = {};
>>if (is_int_mode (mode, _mode)
>>&& GET_MODE_BITSIZE (int_mode) == BITS_PER_WORD * 2
>>&& (!MEM_P (op0) || ! MEM_VOLATILE_P (op0)))
[Ada] Do not give warnings for compiler-generated entities by default
The rationale is that these entities are almost always the result of expansion activities in the front-end, over which the user has very limited control. These warnings can be restored by means of -gnatD. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * gcc-interface/utils.cc (gnat_pushdecl): Also set TREE_NO_WARNING on the decl if Comes_From_Source is false for the associated node.diff --git a/gcc/ada/gcc-interface/utils.cc b/gcc/ada/gcc-interface/utils.cc --- a/gcc/ada/gcc-interface/utils.cc +++ b/gcc/ada/gcc-interface/utils.cc @@ -852,8 +852,11 @@ gnat_pushdecl (tree decl, Node_Id gnat_node) if (!deferred_decl_context) DECL_CONTEXT (decl) = context; - suppress_warning (decl, all_warnings, - No (gnat_node) || Warnings_Off (gnat_node)); + /* Disable warnings for compiler-generated entities or explicit request. */ + if (No (gnat_node) + || !Comes_From_Source (gnat_node) + || Warnings_Off (gnat_node)) +suppress_warning (decl); /* Set the location of DECL and emit a declaration for it. */ if (Present (gnat_node) && !renaming_from_instantiation_p (gnat_node))
[Ada] Small housekeeping work in gnat_gimplify_expr
This alphabetizes the large switch statement, removes a useless nested
switch statement, an artificial fall through and adds a default return.
No functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/trans.cc (gnat_gimplify_expr): Tidy up.diff --git a/gcc/ada/gcc-interface/trans.cc b/gcc/ada/gcc-interface/trans.cc
--- a/gcc/ada/gcc-interface/trans.cc
+++ b/gcc/ada/gcc-interface/trans.cc
@@ -8638,30 +8638,6 @@ gnat_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
switch (TREE_CODE (expr))
{
-case NULL_EXPR:
- /* If this is an aggregate type, build a null pointer of the appropriate
- type and dereference it. */
- if (AGGREGATE_TYPE_P (type)
- || TREE_CODE (type) == UNCONSTRAINED_ARRAY_TYPE)
- *expr_p = build_unary_op (INDIRECT_REF, NULL_TREE,
- convert (build_pointer_type (type),
- integer_zero_node));
- /* Otherwise, just make a VAR_DECL. */
- else
- {
- *expr_p = create_tmp_var (type, NULL);
- suppress_warning (*expr_p);
- }
-
- gimplify_and_add (TREE_OPERAND (expr, 0), pre_p);
- return GS_OK;
-
-case UNCONSTRAINED_ARRAY_REF:
- /* We should only do this if we are just elaborating for side-effects,
- but we can't know that yet. */
- *expr_p = TREE_OPERAND (*expr_p, 0);
- return GS_OK;
-
case ADDR_EXPR:
op = TREE_OPERAND (expr, 0);
@@ -8695,8 +8671,7 @@ gnat_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
else
op = inner;
}
-
- return GS_UNHANDLED;
+ break;
case CALL_EXPR:
/* If we are passing a constant fat pointer CONSTRUCTOR, make sure it is
@@ -8720,8 +8695,62 @@ gnat_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
*(CALL_EXPR_ARGP (expr) + i) = tree_output_constant_def (arg);
}
}
+ break;
+
+case DECL_EXPR:
+ op = DECL_EXPR_DECL (expr);
+
+ /* The expressions for the RM bounds must be gimplified to ensure that
+ they are properly elaborated. See gimplify_decl_expr. */
+ if ((TREE_CODE (op) == TYPE_DECL || TREE_CODE (op) == VAR_DECL)
+ && !TYPE_SIZES_GIMPLIFIED (TREE_TYPE (op))
+ && (INTEGRAL_TYPE_P (TREE_TYPE (op))
+ || SCALAR_FLOAT_TYPE_P (TREE_TYPE (op
+ {
+ tree type = TYPE_MAIN_VARIANT (TREE_TYPE (op)), t, val;
+
+ val = TYPE_RM_MIN_VALUE (type);
+ if (val)
+ {
+ gimplify_one_sizepos (, pre_p);
+ for (t = type; t; t = TYPE_NEXT_VARIANT (t))
+ SET_TYPE_RM_MIN_VALUE (t, val);
+ }
+
+ val = TYPE_RM_MAX_VALUE (type);
+ if (val)
+ {
+ gimplify_one_sizepos (, pre_p);
+ for (t = type; t; t = TYPE_NEXT_VARIANT (t))
+ SET_TYPE_RM_MAX_VALUE (t, val);
+ }
+ }
+ break;
+
+case NULL_EXPR:
+ /* If this is an aggregate type, build a null pointer of the appropriate
+ type and dereference it. */
+ if (AGGREGATE_TYPE_P (type)
+ || TREE_CODE (type) == UNCONSTRAINED_ARRAY_TYPE)
+ *expr_p = build_unary_op (INDIRECT_REF, NULL_TREE,
+ convert (build_pointer_type (type),
+ integer_zero_node));
+
+ /* Otherwise, just make a VAR_DECL. */
+ else
+ {
+ *expr_p = create_tmp_var (type, NULL);
+ suppress_warning (*expr_p);
+ }
- return GS_UNHANDLED;
+ gimplify_and_add (TREE_OPERAND (expr, 0), pre_p);
+ return GS_OK;
+
+case UNCONSTRAINED_ARRAY_REF:
+ /* We should only do this if we are just elaborating for side effects,
+ but we can't know that yet. */
+ *expr_p = TREE_OPERAND (*expr_p, 0);
+ return GS_OK;
case VIEW_CONVERT_EXPR:
op = TREE_OPERAND (expr, 0);
@@ -8742,53 +8771,13 @@ gnat_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
TREE_OPERAND (expr, 0) = new_var;
return GS_OK;
}
-
- return GS_UNHANDLED;
-
-case DECL_EXPR:
- op = DECL_EXPR_DECL (expr);
-
- /* The expressions for the RM bounds must be gimplified to ensure that
- they are properly elaborated. See gimplify_decl_expr. */
- if ((TREE_CODE (op) == TYPE_DECL || TREE_CODE (op) == VAR_DECL)
- && !TYPE_SIZES_GIMPLIFIED (TREE_TYPE (op)))
- switch (TREE_CODE (TREE_TYPE (op)))
- {
- case INTEGER_TYPE:
- case ENUMERAL_TYPE:
- case BOOLEAN_TYPE:
- case REAL_TYPE:
- {
- tree type = TYPE_MAIN_VARIANT (TREE_TYPE (op)), t, val;
-
- val = TYPE_RM_MIN_VALUE (type);
- if (val)
- {
- gimplify_one_sizepos (, pre_p);
- for (t = type; t; t = TYPE_NEXT_VARIANT (t))
- SET_TYPE_RM_MIN_VALUE (t, val);
- }
-
- val = TYPE_RM_MAX_VALUE (type);
- if (val)
- {
- gimplify_one_sizepos (, pre_p);
- for (t = type; t; t = TYPE_NEXT_VARIANT (t))
- SET_TYPE_RM_MAX_VALUE (t, val);
- }
-
- }
- break;
-
- default:
- break;
- }
-
- /* ... fall through ... */
+ break;
default:
- return GS_UNHANDLED;
+ break;
}
+
+ return GS_UNHANDLED;
}
/* Generate GIMPLE in place for the statement at *STMT_P. */
[Ada] Fix internal error on unchecked union with component clauses (2)
The issue arises when the unchecked union contains both a fixed part and
a variant part, and is subject to a full representation clause covering
all the components in all the variants, when the component clauses do not
align the variant boundaries with byte boundaries consistently.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/decl.cc (components_to_record): Use NULL recursively
as P_GNU_REP_LIST for the innermost variant level in the unchecked
union case with a fixed part.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -7981,6 +7981,7 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
tree gnu_union_type;
tree this_first_free_pos, gnu_variant_list = NULL_TREE;
bool union_field_needs_strict_alignment = false;
+ bool innermost_variant_level = true;
auto_vec variant_types;
vinfo_t *gnu_variant;
unsigned int variants_align = 0;
@@ -8026,6 +8027,19 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
}
}
+ /* For an unchecked union with a fixed part, we need to compute whether
+ we are at the innermost level of the variant part. */
+ if (unchecked_union && gnu_field_list)
+ for (variant = First_Non_Pragma (Variants (gnat_variant_part));
+ Present (variant);
+ variant = Next_Non_Pragma (variant))
+ if (Present (Component_List (variant))
+ && Present (Variant_Part (Component_List (variant
+ {
+ innermost_variant_level = false;
+ break;
+ }
+
/* We build the variants in two passes. The bulk of the work is done in
the first pass, that is to say translating the GNAT nodes, building
the container types and computing the associated properties. However
@@ -8066,11 +8080,12 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
/* Add the fields into the record type for the variant but note that
we aren't sure to really use it at this point, see below. In the
- case of an unchecked union, we force the fields with a rep clause
- present in a nested variant to be moved to the outermost variant,
- so as to flatten the rep-ed layout as much as possible, the reason
- being that we cannot do any flattening when a subtype statically
- selects a variant later on, for example for an aggregate. */
+ case of an unchecked union with a fixed part, we force the fields
+ with a rep clause present in the innermost variant to be moved to
+ the outer variant, so as to flatten the rep-ed layout as much as
+ possible, the reason being that we cannot do any flattening when
+ a subtype statically selects a variant later on, for example for
+ an aggregate. */
has_rep
= components_to_record (Component_List (variant), gnat_record_type,
NULL_TREE, gnu_variant_type, packed,
@@ -8078,7 +8093,9 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
unchecked_union, true, needs_xv_encodings,
true, this_first_free_pos,
(all_rep || this_first_free_pos)
-&& !(in_variant && unchecked_union)
+&& !(unchecked_union
+ && gnu_field_list
+ && innermost_variant_level)
? NULL : _rep_list);
/* Translate the qualifier and annotate the GNAT node. */
[Ada] Do not set Current_Error_Node to a node without location
The message "No source file position information available" is displayed in the bugbox when Current_Error_Node has no location, which is useless. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * gcc-interface/trans.cc (gnat_to_gnu): Do not set Current_Error_Node to a node without location.diff --git a/gcc/ada/gcc-interface/trans.cc b/gcc/ada/gcc-interface/trans.cc --- a/gcc/ada/gcc-interface/trans.cc +++ b/gcc/ada/gcc-interface/trans.cc @@ -6139,7 +6139,8 @@ gnat_to_gnu (Node_Id gnat_node) bool aa_sync; /* Save node number for error message and set location information. */ - Current_Error_Node = gnat_node; + if (Sloc (gnat_node) > No_Location) +Current_Error_Node = gnat_node; Sloc_to_locus (Sloc (gnat_node), _location); /* If we are only annotating types and this node is a statement, return
[Ada] Fix internal error on semi-circular record types
The front-end properly computes a linear elaboration order for them, but
there was a loophole in the handling of the delayed case.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/decl.cc (gnat_to_gnu_entity) : And
skip the elaboration of the designated subtype when that of its base
type has been delayed.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -3846,17 +3846,18 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
maybe_present = true;
/* The designated subtype must be elaborated as well, if it does
- not have its own freeze node. But designated subtypes created
- for constrained components of records with discriminants are
- not frozen by the front-end and not elaborated here, because
- their use may appear before the base type is frozen and it is
- not clear that they are needed in gigi. With the current model,
- there is no correct place where they could be elaborated. */
+ not have its own freeze node. */
if (Is_Itype (Directly_Designated_Type (gnat_entity))
&& !present_gnu_tree (Directly_Designated_Type (gnat_entity))
&& Is_Frozen (Directly_Designated_Type (gnat_entity))
&& No (Freeze_Node (Directly_Designated_Type (gnat_entity
{
+ tree gnu_base_type = TREE_TYPE (gnu_decl);
+ tree gnu_desig_base_type
+ = TYPE_IS_FAT_POINTER_P (gnu_base_type)
+ ? TREE_TYPE (TREE_TYPE (TYPE_FIELDS (gnu_base_type)))
+ : TREE_TYPE (gnu_base_type);
+
/* If we are to defer elaborating incomplete types, make a dummy
type node and elaborate it later. */
if (defer_incomplete_level != 0)
@@ -3869,8 +3870,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
p->next = defer_incomplete_list;
defer_incomplete_list = p;
}
- else if (!Is_Incomplete_Or_Private_Type
- (Base_Type (Directly_Designated_Type (gnat_entity
+
+ /* Otherwise elaborate the designated subtype only if its base type
+ has already been elaborated. */
+ else if (!TYPE_IS_DUMMY_P (gnu_desig_base_type))
gnat_to_gnu_entity (Directly_Designated_Type (gnat_entity),
NULL_TREE, false);
}
[Ada] Fix for internal error on semi-circular record aggregate
This creates a couple of record subtypes pointing to each other through
access subtypes, and we break the circularity at the latter subtypes.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/decl.cc (gnat_to_gnu_entity) : If
it is a special subtype designated by an access subtype, then defer
the completion of incomplete types.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -2134,6 +2134,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
suppress expanding incomplete types. */
gnu_type = make_node (UNCONSTRAINED_ARRAY_TYPE);
+ /* The component may refer to this type, so defer completion of any
+ incomplete types. */
if (!definition)
{
defer_incomplete_level++;
@@ -3066,7 +3068,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
process_attributes (_type, _list, true, gnat_entity);
- /* If we are not defining it, suppress expanding incomplete types. */
+ /* Some component may refer to this type, so defer completion of any
+ incomplete types. */
if (!definition)
{
defer_incomplete_level++;
@@ -3439,7 +3442,14 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
{
Entity_Id gnat_base_type = Implementation_Base_Type (gnat_entity);
- if (!definition)
+ /* Some component may refer to this type, so defer completion of any
+ incomplete types. We also need to do it for the special subtypes
+ designated by access subtypes in case they are recursive, see the
+ E_Access_Subtype case below. */
+ if (!definition
+ || (Is_Itype (gnat_entity)
+ && Is_Frozen (gnat_entity)
+ && No (Freeze_Node (gnat_entity
{
defer_incomplete_level++;
this_deferred = true;
[Ada] Preserve unchecked conversion of string constant
This makes it possible to pass the result to a C function directly.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/utils.cc (unchecked_convert): Do not fold a string
constant if the target type is pointer to character.diff --git a/gcc/ada/gcc-interface/utils.cc b/gcc/ada/gcc-interface/utils.cc
--- a/gcc/ada/gcc-interface/utils.cc
+++ b/gcc/ada/gcc-interface/utils.cc
@@ -5637,6 +5637,13 @@ unchecked_convert (tree type, tree expr, bool notrunc_p)
return unchecked_convert (type, expr, notrunc_p);
}
+ /* If we are converting a string constant to a pointer to character, make
+ sure that the string is not folded into an integer constant. */
+ else if (TREE_CODE (expr) == STRING_CST
+ && POINTER_TYPE_P (type)
+ && TYPE_STRING_FLAG (TREE_TYPE (type)))
+expr = build1 (VIEW_CONVERT_EXPR, type, expr);
+
/* Otherwise, just build a VIEW_CONVERT_EXPR of the expression. */
else
{
[Ada] Remove redundant marking of illegal pragma with error posted
We flag illegal pragma Elaborate with a call to Error_Msg on the pragma
argument, which in turn calls Set_Error_Posted on the enclosing
statement, i.e. on the pragma itself. The explicit call to
Set_Error_Posted on the pragma itself was redundant.
Cleanup related to handling of illegal code when detecting uninitialized
scalar objects.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_prag.adb (Analyze_Pragma): Remove redundant call to
Set_Error_Posted.diff --git a/gcc/ada/sem_prag.adb b/gcc/ada/sem_prag.adb
--- a/gcc/ada/sem_prag.adb
+++ b/gcc/ada/sem_prag.adb
@@ -16008,7 +16008,6 @@ package body Sem_Prag is
end loop Innr;
if Citem = N then
- Set_Error_Posted (N);
Error_Pragma_Arg
("argument of pragma% is not withed unit", Arg);
end if;
[Ada] Fix continuation message without a prior error
When resolution of an expanded name fails, we call routine
Error_Missing_With_Of_Known_Unit which emits an error continuation
message (i.e. an error string starting with \\). However, for error
continuations to work properly there must be some prior error, because
continuation itself doesn't set flags like Serious_Errors_Detected.
Without these flags the problematic statement is not marked with
Error_Posted, which in turn is needed to prevent cascaded errors.
In particular, when unresolved procedure call uses a direct name or an
extended name with an unknown prefix, e.g.:
Unknown (1, 2, 3);
Unknown.Call (1, 2, 3);
then the N_Procedure_Call statements are marked with Error_Posted. But
when a call uses an extended name with a known prefix we failed to flag
the N_Procedure_Call with Error_Posted.
Found while improving the robustness of a feature that detects
uninitialized scalar objects.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_ch8.adb (Find_Expanded_Name): Emit a main error message
before adding a continuation with the call to
Error_Missing_With_Of_Known_Unit.diff --git a/gcc/ada/sem_ch8.adb b/gcc/ada/sem_ch8.adb
--- a/gcc/ada/sem_ch8.adb
+++ b/gcc/ada/sem_ch8.adb
@@ -6995,6 +6995,8 @@ package body Sem_Ch8 is
Standard_Standard)
then
if not Error_Posted (N) then
+ Error_Msg_NE
+ ("& is not a visible entity of&", Prefix (N), Selector);
Error_Missing_With_Of_Known_Unit (Prefix (N));
end if;
[Ada] Mark Requires_Transient_Scope as Inline
The predicate is now a simple disjunction of two other predicates. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_util.ads (Requires_Transient_Scope): Add pragma Inline.diff --git a/gcc/ada/sem_util.ads b/gcc/ada/sem_util.ads --- a/gcc/ada/sem_util.ads +++ b/gcc/ada/sem_util.ads @@ -3068,6 +3068,7 @@ package Sem_Util is -- previous errors (particularly in -gnatq mode). function Requires_Transient_Scope (Typ : Entity_Id) return Boolean; + pragma Inline (Requires_Transient_Scope); -- Return true if temporaries of Typ need to be wrapped in a transient -- scope, either because they are allocated on the secondary stack or -- finalization actions must be generated before the next instruction.
[Ada] Fix spurious violations of No_Secondary_Stack restriction
Now that finalization and return on the secondary stack are decoupled, the transient scopes created because of the former need not necessarily manage the secondary stack and trigger a violation of the associated restriction. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch7.adb (Wrap_Transient_Declaration): Propagate Uses_Sec_Stack to enclosing function if it does not return on the secondary stack. * exp_ch6.adb (Expand_Call_Helper): Call Establish_Transient_Scope with Manage_Sec_Stack set to True only when necessary. * sem_res.adb (Resolve_Call): Likewise. (Resolve_Entry_Call): Likewise.diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb --- a/gcc/ada/exp_ch6.adb +++ b/gcc/ada/exp_ch6.adb @@ -4927,10 +4927,11 @@ package body Exp_Ch6 is and then (Ekind (Current_Scope) /= E_Loop or else Nkind (Parent (Call_Node)) /= N_Function_Call - or else not Is_Build_In_Place_Function_Call - (Parent (Call_Node))) + or else not + Is_Build_In_Place_Function_Call (Parent (Call_Node))) then -Establish_Transient_Scope (Call_Node, Manage_Sec_Stack => True); +Establish_Transient_Scope + (Call_Node, Returns_On_Secondary_Stack (Etype (Subp))); end if; end if; end Expand_Call_Helper; diff --git a/gcc/ada/exp_ch7.adb b/gcc/ada/exp_ch7.adb --- a/gcc/ada/exp_ch7.adb +++ b/gcc/ada/exp_ch7.adb @@ -10336,7 +10336,7 @@ package body Exp_Ch7 is -- reclamation is done by the caller. if Ekind (Curr_S) = E_Function - and then Requires_Transient_Scope (Etype (Curr_S)) + and then Returns_On_Secondary_Stack (Etype (Curr_S)) then null; diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -6955,7 +6955,8 @@ package body Sem_Res is and then Requires_Transient_Scope (Etype (Nam)) and then not Is_Ignored_Ghost_Entity (Nam) then - Establish_Transient_Scope (N, Manage_Sec_Stack => True); + Establish_Transient_Scope + (N, Returns_On_Secondary_Stack (Etype (Nam))); -- If the call appears within the bounds of a loop, it will be -- rewritten and reanalyzed, nothing left to do here. @@ -8535,7 +8536,8 @@ package body Sem_Res is elsif Expander_Active and then Requires_Transient_Scope (Etype (Nam)) then - Establish_Transient_Scope (N, Manage_Sec_Stack => True); + Establish_Transient_Scope + (N, Returns_On_Secondary_Stack (Etype (Nam))); end if; -- Now we know that this is not a call to a function that returns an
[Ada] Fix compilation of raise-gcc.c with -DSTANDALONE under windows
This is needed in particular by GNAT LLVM builds. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * raise-gcc.c: Fix compilation with -DSTANDALONE under windows.diff --git a/gcc/ada/raise-gcc.c b/gcc/ada/raise-gcc.c --- a/gcc/ada/raise-gcc.c +++ b/gcc/ada/raise-gcc.c @@ -78,7 +78,7 @@ (SJLJ or DWARF). We need a consistently named interface to import from a-except, so wrappers are defined here. */ -#ifdef __CYGWIN__ +#if defined (__CYGWIN__) || (defined(__SEH__) && defined(STANDALONE)) /* Prevent compile error due to unwind-generic.h including , see comment above #include in mingw32.h. */ #include "mingw32.h"
[Ada] Preserve and reuse original type in Narrow_Large_Operation
Instead of using that of Original_Node (N) after rewriting, which does not work if N had previously been rewritten. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch4.adb (Narrow_Large_Operation): Preserve and reuse Etype.diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb --- a/gcc/ada/exp_ch4.adb +++ b/gcc/ada/exp_ch4.adb @@ -14108,6 +14108,7 @@ package body Exp_Ch4 is procedure Narrow_Large_Operation (N : Node_Id) is Kind : constant Node_Kind := Nkind (N); + Otyp : constant Entity_Id := Etype (N); In_Rng : constant Boolean := Kind = N_In; Binary : constant Boolean := Kind in N_Binary_Op or else In_Rng; Compar : constant Boolean := Kind in N_Op_Compare or else In_Rng; @@ -14262,8 +14263,7 @@ package body Exp_Ch4 is -- Analyze it with the comparison type and checks suppressed since -- the conversions of the operands cannot overflow. - Analyze_And_Resolve - (N, Etype (Original_Node (N)), Suppress => Overflow_Check); + Analyze_And_Resolve (N, Otyp, Suppress => Overflow_Check); else -- Analyze it with the narrower type and checks suppressed, but only
[Ada] Wrong interface dynamic dispatch via access parameter
When the prefix of an Access attribute is an explicit dereference of an access parameter (or a renaming of such a dereference, or a subcomponent of such a dereference), the context is a general access type to a class-wide interface type, and an accessibility check must be generated, the frontend silently skips generating an implicit type conversion to force the displacement of the pointer to reference the secondary dispatch table. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_attr.adb (Add_Implicit_Interface_Type_Conversion): New subprogram which factorizes code. (Expand_N_Attribute_Reference): Call the new subprogram to add the missing implicit interface type conversion.diff --git a/gcc/ada/exp_attr.adb b/gcc/ada/exp_attr.adb --- a/gcc/ada/exp_attr.adb +++ b/gcc/ada/exp_attr.adb @@ -2108,12 +2108,86 @@ package body Exp_Attr is Ref_Object : constant Node_Id := Get_Referenced_Object (Pref); Btyp_DDT : Entity_Id; +procedure Add_Implicit_Interface_Type_Conversion; +-- Ada 2005 (AI-251): The designated type is an interface type; +-- add an implicit type conversion to force the displacement of +-- the pointer to reference the secondary dispatch table. + function Enclosing_Object (N : Node_Id) return Node_Id; -- If N denotes a compound name (selected component, indexed -- component, or slice), returns the name of the outermost such -- enclosing object. Otherwise returns N. If the object is a -- renaming, then the renamed object is returned. + +-- Add_Implicit_Interface_Type_Conversion -- + + +procedure Add_Implicit_Interface_Type_Conversion is +begin + pragma Assert (Is_Interface (Btyp_DDT)); + + -- Handle cases were no action is required. + + if not Comes_From_Source (N) + and then not Comes_From_Source (Ref_Object) + and then (Nkind (Ref_Object) not in N_Has_Chars + or else Chars (Ref_Object) /= Name_uInit) + then + return; + end if; + + -- Common case + + if Nkind (Ref_Object) /= N_Explicit_Dereference then + + -- No implicit conversion required if types match, or if + -- the prefix is the class_wide_type of the interface. In + -- either case passing an object of the interface type has + -- already set the pointer correctly. + + if Btyp_DDT = Etype (Ref_Object) +or else + (Is_Class_Wide_Type (Etype (Ref_Object)) + and then + Class_Wide_Type (Btyp_DDT) = Etype (Ref_Object)) + then + null; + + else + Rewrite (Prefix (N), + Convert_To (Btyp_DDT, + New_Copy_Tree (Prefix (N; + + Analyze_And_Resolve (Prefix (N), Btyp_DDT); + end if; + + -- When the object is an explicit dereference, convert the + -- dereference's prefix. + + else + declare + Obj_DDT : constant Entity_Id := + Base_Type + (Directly_Designated_Type + (Etype (Prefix (Ref_Object; + begin + -- No implicit conversion required if designated types + -- match. + + if Obj_DDT /= Btyp_DDT + and then not (Is_Class_Wide_Type (Obj_DDT) + and then Etype (Obj_DDT) = Btyp_DDT) + then +Rewrite (N, + Convert_To (Typ, +New_Copy_Tree (Prefix (Ref_Object; +Analyze_And_Resolve (N, Typ); + end if; + end; + end if; +end Add_Implicit_Interface_Type_Conversion; + -- -- Enclosing_Object -- -- @@ -2398,62 +2472,20 @@ package body Exp_Attr is then Apply_Accessibility_Check (Prefix (Enc_Object), Typ, N); + -- Ada 2005 (AI-251): If the designated type is an interface we + -- add an implicit conversion to force the displacement of the + -- pointer to reference the secondary dispatch table. + + if Is_Interface (Btyp_DDT) then +
[Ada] Fix bug in handling of Predicate_Failure aspect
The run-time behavior of the Ada 2022 Predicate_Failure aspect was incorrectly implemented. This could cause incorrect exception messages at execution time in the case of a predicate check failure, as demonstrated by ACATS test C324006. In addition, a new attribute (Predicate_Expression) is defined in order to improve the FE/SPARK interface. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * einfo-utils.ads, einfo-utils.adb: Delete Predicate_Function_M function and Set_Predicate_Function_M procedure. * einfo.ads: Delete comments for Is_Predicate_Function_M and Predicate_Function_M functions. Add comment for new Predicate_Expression function. Update comment describing predicate functions. * exp_util.ads, exp_util.adb (Make_Predicate_Call): Replace Mem formal parameter with Static_Mem and Dynamic_Mem formals. (Make_Predicate_Check): Delete Add_Failure_Expression and call to it. * exp_ch4.adb (Expand_N_In.Predicate_Check): Update Make_Predicate_Call call to match profile change. * gen_il-fields.ads: Delete Is_Predicate_Function_M field, add Predicate_Expression field. * gen_il-gen-gen_entities.adb: Delete Is_Predicate_Function_M use, add Predicate_Expression use. * sem_ch13.adb (Build_Predicate_Functions): Rename as singular, not plural; we no longer build a Predicate_M function. Delete Predicate_M references. Add new Boolean parameter for predicate functions when needed. Restructure body of generated predicate functions to implement required Predicate_Failure behavior and to set new Predicate_Expression attribute. Remove special treatment of raise expressions within predicate expressions. * sem_util.ads (Predicate_Failure_Expression, Predicate_Function_Needs_Membership_Parameter): New functions. * sem_util.adb (Is_Current_Instance): Fix bugs which caused wrong result. (Is_Current_Instance_Reference_In_Type_Aspect): Delete Is_Predicate_Function_M reference. (Predicate_Failure_Expression): New function. (Propagate_Predicate_Attributes): Delete Is_Predicate_Function_M references. patch.diff.gz Description: application/gzip
[Ada] Ignore Predicate_Failure in GNATprove mode
In GNATprove mode we are don't want predicate failure to pollute the predicate expression extracted from the predicate function. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch13.adb (Build_Predicate_Function): Ignore predicate failure in GNATprove mode.diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb --- a/gcc/ada/sem_ch13.adb +++ b/gcc/ada/sem_ch13.adb @@ -10487,7 +10487,13 @@ package body Sem_Ch13 is Second_Formal : constant Entity_Id := Next_Entity (Object_Entity); begin - if Present (PF_Expr) then + -- In GNATprove mode we are only interested in the predicate + -- expression itself and don't want a raise expression that + -- comes from the Predicate_Failure. + + if Present (PF_Expr) + and then not GNATprove_Mode + then pragma Assert (Present (Second_Formal)); -- This is an ugly hack to cope with an ugly situation.
[Ada] Avoid copy operation for returns involving function calls
The underlying issue is that the front-end does not create transient scopes for return statements, so objects copied for these statements can never be finalized properly. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch6.adb (Expand_Call_Helper): Adjust comment. (Expand_Simple_Function_Return): For the case of a type which needs finalization and is returned on the primary stack, do not create a copy if the expression originates from a function call. * exp_ch7.adb (Transient Scope Management): Adjust comment. * exp_util.ads (Is_Related_To_Func_Return): Add WARNING line. * fe.h (Is_Related_To_Func_Return): Declare.diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb --- a/gcc/ada/exp_ch6.adb +++ b/gcc/ada/exp_ch6.adb @@ -4899,8 +4899,8 @@ package body Exp_Ch6 is -- the return type is limited, then the context is initialization and -- different processing applies. If the call is to a protected function, -- the expansion above will call Expand_Call recursively. Otherwise the - -- function call is transformed into a temporary which obtains the - -- result from the secondary stack. + -- function call is transformed into a reference to the result that has + -- been built either on the return or the secondary stack. if Needs_Finalization (Etype (Subp)) then if not Is_Build_In_Place_Function_Call (Call_Node) @@ -7369,7 +7369,7 @@ package body Exp_Ch6 is if Present (Utyp) and then Needs_Finalization (Utyp) - and then not (Nkind (Exp) = N_Function_Call + and then not (Exp_Is_Function_Call and then Needs_Finalization (Exp_Typ)) then declare diff --git a/gcc/ada/exp_ch7.adb b/gcc/ada/exp_ch7.adb --- a/gcc/ada/exp_ch7.adb +++ b/gcc/ada/exp_ch7.adb @@ -109,18 +109,13 @@ package body Exp_Ch7 is -- pass the address of a constrained object as the target object for the -- function result. - -- By allocating tagged results in the secondary stack a number of + -- By always allocating tagged results in the secondary stack, a couple of -- implementation difficulties are avoided: - --- If it is a dispatching function call, the computation of the size of - -- the result is possible but complex from the outside. + --- If this is a dispatching function call, the computation of the size + -- of the result is possible but complex from the outside. - --- If the returned type is controlled, the assignment of the returned - -- value to the anonymous object involves an Adjust, and we have no - -- easy way to access the anonymous object created by the back end. - - --- If the returned type is class-wide, this is an unconstrained type - -- anyway. + --- If the result type is class-wide, it is unconstrained anyway. -- Furthermore, the small loss in efficiency which is the result of this -- decision is not such a big deal because functions returning tagged types diff --git a/gcc/ada/exp_util.ads b/gcc/ada/exp_util.ads --- a/gcc/ada/exp_util.ads +++ b/gcc/ada/exp_util.ads @@ -820,6 +820,8 @@ package Exp_Util is -- Determine whether object Id is related to an expanded return statement. -- The case concerned is "return Id.all;". + -- WARNING: There is a matching C declaration of this subprogram in fe.h + function Is_Renamed_Object (N : Node_Id) return Boolean; -- Returns True if the node N is a renamed object. An expression is -- considered to be a renamed object if either it is the Name of an object diff --git a/gcc/ada/fe.h b/gcc/ada/fe.h --- a/gcc/ada/fe.h +++ b/gcc/ada/fe.h @@ -183,9 +183,11 @@ extern Boolean Is_Init_Proc (Entity_Id); /* exp_util: */ #define Is_Fully_Repped_Tagged_Type exp_util__is_fully_repped_tagged_type +#define Is_Related_To_Func_Return exp_util__is_related_to_func_return #define Find_Interface_Tag exp_util__find_interface_tag -extern Boolean Is_Fully_Repped_Tagged_Type (Entity_Id); +extern Boolean Is_Fully_Repped_Tagged_Type (Entity_Id); +extern Boolean Is_Related_To_Func_Return (Entity_Id); extern Entity_Id Find_Interface_Tag (Entity_Id, Entity_Id); /* lib: */
[Ada] Fix invalid expanded code for entry families
Expansion of entry families created a slightly illegal AST with Elsif_Parts being an empty list. Cleanup uncovered by the work on detection of uninitialized scalars. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch9.adb (Build_Find_Body_Index): Remove empty Elsif_Parts from the constructed IF statement.diff --git a/gcc/ada/exp_ch9.adb b/gcc/ada/exp_ch9.adb --- a/gcc/ada/exp_ch9.adb +++ b/gcc/ada/exp_ch9.adb @@ -2800,6 +2800,15 @@ package body Exp_Ch9 is begin Remove (Nod); Set_Else_Statements (Ret, Then_Statements (Nod)); + + -- If Elsif_Parts becomes empty then remove it entirely, as + -- otherwise we would violate the invariant of If_Statement + -- node described in Sinfo. + + if Is_Empty_List (Elsif_Parts (Ret)) then + pragma Assert (Elsif_Parts (Ret) /= No_List); + Set_Elsif_Parts (Ret, No_List); + end if; end; end if; end if;
[Ada] Cleanup expansion of protected entry families
Expansion of entry families contained a condition that was always true. Cleanup related to detection of uninitialized scalar objects (which uncovered that expansion of entry families creates a slightly illegal AST with Elsif_Parts being an empty list). Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch9.adb (Build_Find_Body_Index): Remove IF statement whose condition was true-by-construction; remove excessive assertion (since the call to Elsif_Parts will check that Nod is present and it is an if-statement).diff --git a/gcc/ada/exp_ch9.adb b/gcc/ada/exp_ch9.adb --- a/gcc/ada/exp_ch9.adb +++ b/gcc/ada/exp_ch9.adb @@ -2793,20 +2793,14 @@ package body Exp_Ch9 is Expression => Make_Integer_Literal (Loc, 1)); else -pragma Assert (Present (Ret)); +-- Ranges are in increasing order, so last one doesn't need guard -if Nkind (Ret) = N_If_Statement then - - -- Ranges are in increasing order, so last one doesn't need - -- guard. - - declare - Nod : constant Node_Id := Last (Elsif_Parts (Ret)); - begin - Remove (Nod); - Set_Else_Statements (Ret, Then_Statements (Nod)); - end; -end if; +declare + Nod : constant Node_Id := Last (Elsif_Parts (Ret)); +begin + Remove (Nod); + Set_Else_Statements (Ret, Then_Statements (Nod)); +end; end if; end if;
[Ada] Fix proof of runtime unit a-strfix and a-strsup
Update to provers caused some proof regressions. Fix the proof by adding an assertion. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/a-strfix.adb: Add assertions. * libgnat/a-strsup.adb: Idem.diff --git a/gcc/ada/libgnat/a-strfix.adb b/gcc/ada/libgnat/a-strfix.adb --- a/gcc/ada/libgnat/a-strfix.adb +++ b/gcc/ada/libgnat/a-strfix.adb @@ -628,6 +628,11 @@ package body Ada.Strings.Fixed with SPARK_Mode is (Result (1 .. Integer'Max (0, Low - Source'First)) = Source (Source'First .. Low - 1)); Result (Front_Len + 1 .. Front_Len + By'Length) := By; + pragma Assert + (Result +(Integer'Max (0, Low - Source'First) + 1 + .. Integer'Max (0, Low - Source'First) + By'Length) + = By); if High < Source'Last then Result (Front_Len + By'Length + 1 .. Result'Last) := diff --git a/gcc/ada/libgnat/a-strsup.adb b/gcc/ada/libgnat/a-strsup.adb --- a/gcc/ada/libgnat/a-strsup.adb +++ b/gcc/ada/libgnat/a-strsup.adb @@ -1150,6 +1150,14 @@ package body Ada.Strings.Superbounded with SPARK_Mode is Result.Data (Position .. Position - 1 + New_Item'Length) := Super_String_Data (New_Item); Result.Current_Length := Source.Current_Length; + pragma Assert + (String'(Super_Slice (Result, 1, Position - 1)) = + Super_Slice (Source, 1, Position - 1)); + pragma Assert + (Super_Slice (Result, +Position, Position - 1 + New_Item'Length) = + New_Item); + return Result; elsif Position - 1 <= Max_Length - New_Item'Length then @@ -1157,6 +1165,14 @@ package body Ada.Strings.Superbounded with SPARK_Mode is Result.Data (Position .. Position - 1 + New_Item'Length) := Super_String_Data (New_Item); Result.Current_Length := Position - 1 + New_Item'Length; + pragma Assert + (String'(Super_Slice (Result, 1, Position - 1)) = + Super_Slice (Source, 1, Position - 1)); + pragma Assert + (Super_Slice (Result, +Position, Position - 1 + New_Item'Length) = + New_Item); + return Result; else @@ -1189,6 +1205,7 @@ package body Ada.Strings.Superbounded with SPARK_Mode is end case; Result.Current_Length := Max_Length; + pragma Assert (Super_Length (Result) = Source.Max_Length); return Result; end if; end Super_Overwrite;
[Ada] Improve optimization of "=" on bit-packed arrays
This patch fixes a performance regression, introduced by a previous bug fix. That fix had the unintended side effect of removing the optimization in cases where the two operands are of two different compiler-generated modular types. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_pakd.adb (Expand_Packed_Eq): Replace the check for *same* modular type, with a check for any modular type, and assert that the two types have the same modulus and so on. * exp_pakd.ads: Minor comment improvements.diff --git a/gcc/ada/exp_pakd.adb b/gcc/ada/exp_pakd.adb --- a/gcc/ada/exp_pakd.adb +++ b/gcc/ada/exp_pakd.adb @@ -1930,9 +1930,17 @@ package body Exp_Pakd is -- modular case we guarantee that the unused bits are always zeroes. -- We do have to compare the lengths because we could be comparing -- two different subtypes of the same base type. We can only do this - -- if the PATs on both sides are the same. + -- if the PATs on both sides are modular (in which case they are + -- necessarily structurally the same -- same Modulus and so on); + -- otherwise, we have a case where the right operand is not of + -- compile time known size. + + if Is_Modular_Integer_Type (PAT) +and then Is_Modular_Integer_Type (Etype (R)) + then + pragma Assert (RM_Size (Etype (R)) = RM_Size (PAT)); + pragma Assert (Modulus (Etype (R)) = Modulus (PAT)); - if Is_Modular_Integer_Type (PAT) and then PAT = Etype (R) then Rewrite (N, Make_And_Then (Loc, Left_Opnd => diff --git a/gcc/ada/exp_pakd.ads b/gcc/ada/exp_pakd.ads --- a/gcc/ada/exp_pakd.ads +++ b/gcc/ada/exp_pakd.ads @@ -41,7 +41,8 @@ package Exp_Pakd is -- This packed array type has the name xxxPn, where xxx is the name -- of the packed type, and n is the component size. The expanded - -- declaration declares a type that is one of the following: + -- declaration declares a type that is one of the following (sizes + -- below are in bytes): --For an unconstrained array with component size 1,2,4 or any other --odd component size. These are the cases in which we do not need @@ -49,10 +50,10 @@ package Exp_Pakd is -- type xxxPn is new Packed_Bytes1; - --For an unconstrained array with component size that is divisible - --by 2, but not divisible by 4 (other than 2 itself). These are the - --cases in which we can generate better code if the underlying array - --is 2-byte aligned (see System.Pack_14 in file s-pack14 for example). + --For an unconstrained array with component size greater than 2, that is + --divisible by 2, but not divisible by 4. These are the cases in which + --we can generate better code if the underlying array is 2-byte aligned + --(see System.Pack_14 in file s-pack14 for example). -- type xxxPn is new Packed_Bytes2; @@ -942,7 +943,7 @@ package Exp_Pakd is -- N is an N_Op_Eq node where the operands are packed arrays whose -- representation is an array-of-bytes type (the case where a modular -- type is used for the representation does not require any special - -- handling, because in the modular case, unused bits are zeroes. + -- handling, because in the modular case, unused bits are zeroes). procedure Expand_Packed_Not (N : Node_Id); -- N is an N_Op_Not node where the operand is packed array of Boolean
[Ada] Support Ada 2022 null array aggregates
Add support for Ada 2022's "[]" null array aggregates (thanks to Ed Schonberg for producing most of this patch). Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * erroutc.ads: Fix a single-character typo in a comment. * exp_aggr.adb: Fix a single-character typo in a comment. Replace several pairs of calls to Low_Bound and High_Bound (which do not handle an identifier that denotes a scalar subtype) with corresponding calls to Get_Index_Bounds (which does handle that case). * par-ch4.adb (P_Aggregate_Or_Paren_Expr): Set the Component_Associations attribute of a null array aggregate to New_List. * sem_aggr.ads: New visible function Is_Null_Array_Aggregate_High_Bound. * sem_aggr.adb (Is_Null_Array_Aggregate_High_Bound, Is_Null_Aggregate, Resolve_Null_Array_Aggregate): New functions. (Resolve_Aggregate): Recognize null array aggregates (using Is_Null_Aggregate) and, when one is recognized, resolve it (using Resolve_Null_Array_Aggregate). Avoid calling Array_Aggr_Subtype for a null array aggregate; the needed subtype is built in Resolve_Null_Array_Aggregate. Do not incorrectly flag a null aggregate (after it is transformed by expansion) as being both positional and named. * sem_attr.adb (Eval_Attribute): Special treatment for null array aggregate high bounds to avoid incorrectly flagging something like Integer'Pred (Integer'First) as an illegal static expression. * sem_eval.adb (Out_Of_Range): Special treatment for null array aggregate high bounds to avoid incorrectly flagging something like Integer'Pred (Integer'First) as an illegal static expression.diff --git a/gcc/ada/erroutc.ads b/gcc/ada/erroutc.ads --- a/gcc/ada/erroutc.ads +++ b/gcc/ada/erroutc.ads @@ -465,7 +465,7 @@ package Erroutc is -- Tests if message buffer ends with given string preceded by a space procedure Buffer_Remove (C : Character); - -- Remove given character fron end of buffer if it is present + -- Remove given character from end of buffer if it is present procedure Buffer_Remove (S : String); -- Removes given string from end of buffer if it is present at end of diff --git a/gcc/ada/exp_aggr.adb b/gcc/ada/exp_aggr.adb --- a/gcc/ada/exp_aggr.adb +++ b/gcc/ada/exp_aggr.adb @@ -2280,8 +2280,10 @@ package body Exp_Aggr is New_Code : constant List_Id := New_List; - Aggr_L : constant Node_Id := Low_Bound (Aggregate_Bounds (N)); - Aggr_H : constant Node_Id := High_Bound (Aggregate_Bounds (N)); + Aggr_Bounds : constant Range_Nodes := +Get_Index_Bounds (Aggregate_Bounds (N)); + Aggr_L : Node_Id renames Aggr_Bounds.First; + Aggr_H : Node_Id renames Aggr_Bounds.Last; -- The aggregate bounds of this specific subaggregate. Note that if the -- code generated by Build_Array_Aggr_Code is executed then these bounds -- are OK. Otherwise a Constraint_Error would have been raised. @@ -2577,7 +2579,7 @@ package body Exp_Aggr is -- If Typ is derived, and constrains discriminants of the parent type, -- these discriminants are not components of the aggregate, and must be -- initialized. The assignments are appended to List. The same is done - -- if Typ derives fron an already constrained subtype of a discriminated + -- if Typ derives from an already constrained subtype of a discriminated -- parent type. procedure Init_Stored_Discriminants; @@ -5226,6 +5228,11 @@ package body Exp_Aggr is Others_Present := False; if Present (Component_Associations (N)) then +if Is_Empty_List (Component_Associations (N)) then + -- an expanded null array aggregate + return False; +end if; + declare Assoc : Node_Id; Choice : Node_Id; @@ -5914,8 +5921,10 @@ package body Exp_Aggr is procedure Check_Same_Aggr_Bounds (Sub_Aggr : Node_Id; Dim : Pos) is - Sub_Lo : constant Node_Id := Low_Bound (Aggregate_Bounds (Sub_Aggr)); - Sub_Hi : constant Node_Id := High_Bound (Aggregate_Bounds (Sub_Aggr)); + Sub_Bounds : constant Range_Nodes + := Get_Index_Bounds (Aggregate_Bounds (Sub_Aggr)); + Sub_Lo : Node_Id renames Sub_Bounds.First; + Sub_Hi : Node_Id renames Sub_Bounds.Last; -- The bounds of this specific subaggregate Aggr_Lo : constant Node_Id := Aggr_Low (Dim); @@ -6019,7 +6028,9 @@ package body Exp_Aggr is if Present (Component_Associations (Sub_Aggr)) then Assoc := Last (Component_Associations (Sub_Aggr)); -if Nkind (First (Choice_List (Assoc))) = N_Others_Choice then +if Present (Assoc) + and then Nkind (First
[Ada] Further adapt proof of double arithmetic runtime unit
After changes in Why3 and generation of VCs, ghost code needs to be adapted for proofs to remain automatic. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/s-aridou.adb (Lemma_Abs_Range, Lemma_Double_Shift_Left, Lemma_Shift_Left): New lemmas. (Double_Divide): Add ghost code. (Lemma_Concat_Definition, Lemma_Double_Shift_Left, Lemma_Shift_Left, Lemma_Shift_Right): Define or complete lemmas. (Scaled_Divide): Add ghost code.diff --git a/gcc/ada/libgnat/s-aridou.adb b/gcc/ada/libgnat/s-aridou.adb --- a/gcc/ada/libgnat/s-aridou.adb +++ b/gcc/ada/libgnat/s-aridou.adb @@ -208,6 +208,13 @@ is Ghost, Post => abs (X * Y) = abs X * abs Y; + procedure Lemma_Abs_Range (X : Big_Integer) + with + Ghost, + Pre => In_Double_Int_Range (X), + Post => abs (X) <= Big_2xxDouble_Minus_1 + and then In_Double_Int_Range (-abs (X)); + procedure Lemma_Abs_Rem_Commutation (X, Y : Big_Integer) with Ghost, @@ -306,6 +313,20 @@ is Pre => S <= Double_Size - S1, Post => Shift_Left (Shift_Left (X, S), S1) = Shift_Left (X, S + S1); + procedure Lemma_Double_Shift_Left (X : Double_Uns; S, S1 : Double_Uns) + with + Ghost, + Pre => S <= Double_Uns (Double_Size) + and then S1 <= Double_Uns (Double_Size), + Post => Shift_Left (Shift_Left (X, Natural (S)), Natural (S1)) = + Shift_Left (X, Natural (S + S1)); + + procedure Lemma_Double_Shift_Left (X : Double_Uns; S, S1 : Natural) + with + Ghost, + Pre => S <= Double_Size - S1, + Post => Shift_Left (Shift_Left (X, S), S1) = Shift_Left (X, S + S1); + procedure Lemma_Double_Shift_Right (X : Double_Uns; S, S1 : Double_Uns) with Ghost, @@ -505,6 +526,13 @@ is Pre => A = B * Q + R and then R < B, Post => Q = A / B and then R = A rem B; + procedure Lemma_Shift_Left (X : Double_Uns; Shift : Natural) + with + Ghost, + Pre => Shift < Double_Size + and then Big (X) * Big_2xx (Shift) < Big_2xxDouble, + Post => Big (Shift_Left (X, Shift)) = Big (X) * Big_2xx (Shift); + procedure Lemma_Shift_Right (X : Double_Uns; Shift : Natural) with Ghost, @@ -560,10 +588,10 @@ is procedure Inline_Le3 (X1, X2, X3, Y1, Y2, Y3 : Single_Uns) is null; procedure Lemma_Abs_Commutation (X : Double_Int) is null; procedure Lemma_Abs_Mult_Commutation (X, Y : Big_Integer) is null; + procedure Lemma_Abs_Range (X : Big_Integer) is null; procedure Lemma_Add_Commutation (X : Double_Uns; Y : Single_Uns) is null; procedure Lemma_Add_One (X : Double_Uns) is null; procedure Lemma_Bounded_Powers_Of_2_Increasing (M, N : Natural) is null; - procedure Lemma_Concat_Definition (X, Y : Single_Uns) is null; procedure Lemma_Deep_Mult_Commutation (Factor : Big_Integer; X, Y : Single_Uns) @@ -581,6 +609,8 @@ is procedure Lemma_Double_Big_2xxSingle is null; procedure Lemma_Double_Shift (X : Double_Uns; S, S1 : Double_Uns) is null; procedure Lemma_Double_Shift (X : Single_Uns; S, S1 : Natural) is null; + procedure Lemma_Double_Shift_Left (X : Double_Uns; S, S1 : Double_Uns) + is null; procedure Lemma_Double_Shift_Right (X : Double_Uns; S, S1 : Double_Uns) is null; procedure Lemma_Ge_Commutation (A, B : Double_Uns) is null; @@ -949,6 +979,7 @@ is pragma Assert (if X = Double_Int'First and then Round then Mult > Big_2xxDouble); elsif Ylo > 0 then + pragma Assert (Double_Uns'(Ylo * Zhi) > 0); pragma Assert (Big (Double_Uns'(Ylo * Zhi)) > 0); pragma Assert (if X = Double_Int'First and then Round then Mult > Big_2xxDouble); @@ -1024,15 +1055,24 @@ is pragma Assert (Big (Double_Uns (Hi (T2))) >= 1); pragma Assert (Big (Double_Uns (Lo (T2))) >= 0); pragma Assert (Big (Double_Uns (Lo (T1))) >= 0); +pragma Assert (Mult >= Big_2xxDouble * Big (Double_Uns (Hi (T2; pragma Assert (Mult >= Big_2xxDouble); if Hi (T2) > 1 then pragma Assert (Big (Double_Uns (Hi (T2))) > 1); + pragma Assert (if X = Double_Int'First and then Round then + Mult > Big_2xxDouble); elsif Lo (T2) > 0 then pragma Assert (Big (Double_Uns (Lo (T2))) > 0); + pragma Assert (if X = Double_Int'First and then Round then + Mult > Big_2xxDouble); elsif Lo (T1) > 0 then pragma Assert (Double_Uns (Lo (T1)) > 0); Lemma_Gt_Commutation (Double_Uns (Lo (T1)), 0); pragma Assert (Big (Double_Uns (Lo (T1))) > 0); + pragma Assert (if X = Double_Int'First and then Round then + Mult > Big_2xxDouble); +
[PATCH] Remove get_or_alloc_expression_id
This function is no longer needed.
Bootstrapped and tested on x86_64-unknown-linux-gnu with a change
to assert it is computed when queried.
Pushed.
2022-05-19 Richard Biener
* tree-ssa-pre.cc (get_or_alloc_expression_id): Remove.
(add_to_value): Use get_expression_id.
(bitmap_insert_into_set): Likewise.
(bitmap_value_insert_into_set): Likewise.
---
gcc/tree-ssa-pre.cc | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc
index 34d77f1e2cf..09a5e2055ca 100644
--- a/gcc/tree-ssa-pre.cc
+++ b/gcc/tree-ssa-pre.cc
@@ -384,18 +384,6 @@ lookup_expression_id (const pre_expr expr)
}
}
-/* Return the existing expression id for EXPR, or create one if one
- does not exist yet. */
-
-static inline unsigned int
-get_or_alloc_expression_id (pre_expr expr)
-{
- unsigned int id = lookup_expression_id (expr);
- if (id == 0)
-return alloc_expression_id (expr);
- return expr->id = id;
-}
-
/* Return the expression that has expression id ID */
static inline pre_expr
@@ -729,7 +717,7 @@ add_to_value (unsigned int v, pre_expr e)
set = BITMAP_ALLOC (_bitmap_obstack);
value_expressions[v] = set;
}
- bitmap_set_bit (set, get_or_alloc_expression_id (e));
+ bitmap_set_bit (set, get_expression_id (e));
}
}
@@ -792,7 +780,7 @@ bitmap_insert_into_set (bitmap_set_t set, pre_expr expr)
for the same value to appear in a set. This is needed for
TMP_GEN, PHI_GEN and NEW_SETs. */
bitmap_set_bit (>values, val);
- bitmap_set_bit (>expressions, get_or_alloc_expression_id (expr));
+ bitmap_set_bit (>expressions, get_expression_id (expr));
}
}
@@ -1030,7 +1018,7 @@ bitmap_value_insert_into_set (bitmap_set_t set, pre_expr
expr)
{
unsigned int val = get_expr_value_id (expr);
- gcc_checking_assert (expr->id == get_or_alloc_expression_id (expr));
+ gcc_checking_assert (expr->id == get_expression_id (expr));
/* Constant values are always considered to be part of the set. */
if (value_id_constant_p (val))
--
2.35.3
[Ada] Casing style on record components
This patch fixes a bug where the -gnatyr switch fails to detect incorrect casing of record components. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * style.adb (Check_Identifier): Deal with the case where a record component definition has been transformed; we want to warn if the original came from source. * libgnat/s-objrea.ads, libgnat/s-objrea.adb: Fix casing of MF to be consistent. * uname.adb: Fix casing of Chars to be consistent. * sem_util.ads: Minor comment fix.diff --git a/gcc/ada/libgnat/s-objrea.adb b/gcc/ada/libgnat/s-objrea.adb --- a/gcc/ada/libgnat/s-objrea.adb +++ b/gcc/ada/libgnat/s-objrea.adb @@ -979,7 +979,7 @@ package body System.Object_Reader is -- Map section table - Opt_Stream := Create_Stream (Res.Mf, Signature_Loc_Offset, 4); + Opt_Stream := Create_Stream (Res.MF, Signature_Loc_Offset, 4); Hdr_Offset := Offset (uint32'(Read (Opt_Stream))); Close (Opt_Stream); Res.Sectab_Stream := Create_Stream @@ -999,7 +999,7 @@ package body System.Object_Reader is Opt_32 : Optional_Header_PE32; begin Opt_Stream := Create_Stream - (Res.Mf, Opt_Offset, Opt_32'Size / SSU); + (Res.MF, Opt_Offset, Opt_32'Size / SSU); Read_Raw (Opt_Stream, Opt_32'Address, uint32 (Opt_32'Size / SSU)); Res.ImageBase := uint64 (Opt_32.ImageBase); @@ -1011,7 +1011,7 @@ package body System.Object_Reader is Opt_64 : Optional_Header_PE64; begin Opt_Stream := Create_Stream - (Res.Mf, Opt_Offset, Opt_64'Size / SSU); + (Res.MF, Opt_Offset, Opt_64'Size / SSU); Read_Raw (Opt_Stream, Opt_64'Address, uint32 (Opt_64'Size / SSU)); Res.ImageBase := Opt_64.ImageBase; @@ -1367,7 +1367,7 @@ package body System.Object_Reader is Strtab_Sz : uint32; begin - Res.Mf := F; + Res.MF := F; Res.In_Exception := In_Exception; Res.Arch := PPC; @@ -1515,14 +1515,14 @@ package body System.Object_Reader is end Arch; function Create_Stream - (Mf : Mapped_File; + (MF : Mapped_File; File_Offset : File_Size; File_Length : File_Size) return Mapped_Stream is Region : Mapped_Region; begin - Read (Mf, Region, File_Offset, File_Length, False); + Read (MF, Region, File_Offset, File_Length, False); return (Region, 0, Offset (File_Length)); end Create_Stream; @@ -1531,7 +1531,7 @@ package body System.Object_Reader is Sec : Object_Section) return Mapped_Stream is begin - return Create_Stream (Obj.Mf, File_Size (Sec.Off), File_Size (Sec.Size)); + return Create_Stream (Obj.MF, File_Size (Sec.Off), File_Size (Sec.Size)); end Create_Stream; procedure Tell (Obj : in out Mapped_Stream; Off : out Offset) is @@ -1573,7 +1573,7 @@ package body System.Object_Reader is null; end case; - Close (Obj.Mf); + Close (Obj.MF); end Close; diff --git a/gcc/ada/libgnat/s-objrea.ads b/gcc/ada/libgnat/s-objrea.ads --- a/gcc/ada/libgnat/s-objrea.ads +++ b/gcc/ada/libgnat/s-objrea.ads @@ -187,7 +187,7 @@ package System.Object_Reader is type Mapped_Stream is private; -- Provide an abstraction of a stream on a memory mapped file - function Create_Stream (Mf : System.Mmap.Mapped_File; + function Create_Stream (MF : System.Mmap.Mapped_File; File_Offset : System.Mmap.File_Size; File_Length : System.Mmap.File_Size) return Mapped_Stream; @@ -381,7 +381,7 @@ private subtype Any_PECOFF is Object_Format range PECOFF .. PECOFF_PLUS; type Object_File (Format : Object_Format) is record - Mf : System.Mmap.Mapped_File := System.Mmap.Invalid_Mapped_File; + MF : System.Mmap.Mapped_File := System.Mmap.Invalid_Mapped_File; Arch : Object_Arch := Unknown; Num_Sections : uint32 := 0; diff --git a/gcc/ada/sem_util.ads b/gcc/ada/sem_util.ads --- a/gcc/ada/sem_util.ads +++ b/gcc/ada/sem_util.ads @@ -3169,9 +3169,8 @@ package Sem_Util is -- This procedure has the same calling sequence as Set_Entity, but it -- performs additional checks as follows: -- - --If Style_Check is set, then it calls a style checking routine which - --can check identifier spelling style. This procedure also takes care - --of checking the restriction No_Implementation_Identifiers. + --If Style_Check is set, then it calls a style checking routine that + --can check identifier spelling style. -- --If restriction No_Abort_Statements is set, then it checks that the --entity is not Ada.Task_Identification.Abort_Task. diff --git
[Ada] Get rid of secondary stack for controlled components
This eliminates the use of the secondary stack to return composite types with controlled components from functions, by exposing the return slot of these functions through the support interface of memory pools, much like for the secondary stack itself. This is piggybacked on the support of a specific intrinsic function by the code generator, and can be disabled if this support is not available, as well with the -gnatd_r debug switch. The change also streamlines a bit the implementation by consistently using the Needs_Finalization predicate, or its derivatives, in various places. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * Makefile.rtl (GNATRTL_NONTASKING_OBJS): Add s-retsta. * debug.adb (d_r): Document usage. * exp_ch4.adb (Expand_N_Allocato): Deal with the return stack pool. * exp_ch6.adb (Expand_Simple_Function_Return): Replace calls to Requires_Transient_Scope with Returns_On_Secondary_Stack. Deal with types that need finalization returned on the primary stack, use CW_Or_Needs_Finalization for those returned on the secondary. * exp_util.adb (Build_Allocate_Deallocate_Proc): Return early for the return stack pool. (Remove_Side_Effects): Call CW_Or_Needs_Finalization. * fe.h (Requires_Transient_Scope): Delete. (Returns_On_Secondary_Stack): Declare. * gnat1drv.adb (Adjust_Global_Switches): Set Back_End_Return_Slot to False when generating C code or if -gnatd_r is specified. * opt.ads (Back_End_Return_Slot): New boolean variable. * rtsfind.ads (RTU_Id): Add System_Return_Stack. (RE_Id): Add RE_RS_Allocate and RE_RS_Pool. (RE_Unit_Table): Add entries for RE_RS_Allocate and RE_RS_Pool. * sem_util.ads (CW_Or_Has_Controlled_Part): Delete. (CW_Or_Needs_Finalization): Declare. (Requires_Transient_Scope): Adjust description. (Returns_On_Secondary_Stack): Declare. * sem_util.adb (Compute_Returns_By_Ref): Set Returns_By_Ref on types which need finalization if they are returned on the secondary stack. (CW_Or_Has_Controlled_Part): Rename to... (CW_Or_Needs_Finalization): ...this. (Requires_Transient_Scope): Move bulk of implementation to... (Returns_On_Secondary_Stack): ...here. Return true for types which need finalization only if the back-end return slot is not supported. * libgnat/s-retsta.ads: New file. * gcc-interface/ada-builtin-types.def (BT_FN_PTR_SSIZE): Define. * gcc-interface/ada-builtins.def (return_slot): Likewise. * gcc-interface/ada-tree.h (BUILT_IN_RETURN_SLOT): Likewise. * gcc-interface/decl.cc (gnat_to_gnu_subprog_type): Replace call to Requires_Transient_Scope with Returns_On_Secondary_Stack. * gcc-interface/trans.cc (gnat_to_gnu) : In the return by invisible reference, skip the copy if the source is the same as the destination. * gcc-interface/utils2.cc (build_call_alloc_dealloc_proc): Deal with the return stack pool. patch.diff.gz Description: application/gzip
[Ada] Small housekeeping work continued
No functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/trans.cc: Fix formatting issues in comments.
(Subprogram_Body_to_gnu): Tidy up.
(Exception_Handler_to_gnu_gcc): Rename into...
(Exception_Handler_to_gnu): ...this.
(gnat_to_gnu) : Adjust to above renaming.diff --git a/gcc/ada/gcc-interface/trans.cc b/gcc/ada/gcc-interface/trans.cc
--- a/gcc/ada/gcc-interface/trans.cc
+++ b/gcc/ada/gcc-interface/trans.cc
@@ -1149,9 +1149,9 @@ Gigi_Types_Compatible (Entity_Id type, Entity_Id def_type)
return false;
}
-/* Subroutine of gnat_to_gnu to translate gnat_node, an N_Identifier,
- to a GCC tree, which is returned. GNU_RESULT_TYPE_P is a pointer
- to where we should place the result type. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Identifier, to a GCC
+ tree, which is returned. GNU_RESULT_TYPE_P is a pointer to where we should
+ place the result type. */
static tree
Identifier_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p)
@@ -1353,8 +1353,8 @@ Identifier_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p)
return gnu_result;
}
-/* Subroutine of gnat_to_gnu to process gnat_node, an N_Pragma. Return
- any statements we generate. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Pragma, to a GCC
+ tree, which is returned. */
static tree
Pragma_to_gnu (Node_Id gnat_node)
@@ -1700,9 +1700,9 @@ get_type_length (tree type, tree result_type)
return length;
}
-/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Attribute node,
- to a GCC tree, which is returned. GNU_RESULT_TYPE_P is a pointer to
- where we should place the result type. ATTRIBUTE is the attribute ID. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Attribute node, to a
+ GCC tree, which is returned. GNU_RESULT_TYPE_P is a pointer to where we
+ should place the result type. ATTRIBUTE is the attribute ID. */
static tree
Attribute_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p, int attribute)
@@ -2613,8 +2613,8 @@ Attribute_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p, int attribute)
return gnu_result;
}
-/* Subroutine of gnat_to_gnu to translate gnat_node, an N_Case_Statement,
- to a GCC tree, which is returned. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Case_Statement, to a
+ GCC tree, which is returned. */
static tree
Case_Statement_to_gnu (Node_Id gnat_node)
@@ -2951,8 +2951,8 @@ independent_iterations_p (tree stmt_list)
return true;
}
-/* Subroutine of gnat_to_gnu to translate gnat_node, an N_Loop_Statement,
- to a GCC tree, which is returned. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Loop_Statement, to a
+ GCC tree, which is returned. */
static tree
Loop_Statement_to_gnu (Node_Id gnat_node)
@@ -3836,22 +3836,19 @@ build_return_expr (tree ret_obj, tree ret_val)
return build1 (RETURN_EXPR, void_type_node, result_expr);
}
-/* Subroutine of gnat_to_gnu to process gnat_node, an N_Subprogram_Body. We
- don't return anything. */
+/* Subroutine of gnat_to_gnu to translate GNAT_NODE, an N_Subprogram_Body. */
static void
Subprogram_Body_to_gnu (Node_Id gnat_node)
{
- /* Defining identifier of a parameter to the subprogram. */
- Entity_Id gnat_param;
/* The defining identifier for the subprogram body. Note that if a
specification has appeared before for this body, then the identifier
- occurring in that specification will also be a defining identifier and all
- the calls to this subprogram will point to that specification. */
+ occurring in that specification will also be a defining identifier
+ and calls to this subprogram will point to that specification. */
Entity_Id gnat_subprog
= (Present (Corresponding_Spec (gnat_node))
? Corresponding_Spec (gnat_node) : Defining_Entity (gnat_node));
- /* The FUNCTION_DECL node corresponding to the subprogram spec. */
+ /* The FUNCTION_DECL node corresponding to the defining identifier. */
tree gnu_subprog;
/* Its RESULT_DECL node. */
tree gnu_result_decl;
@@ -3860,11 +3857,9 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
/* The TYPE_CI_CO_LIST of its FUNCTION_TYPE node, if any. */
tree gnu_cico_list;
/* The entry in the CI_CO_LIST that represents a function return, if any. */
- tree gnu_return_var_elmt = NULL_TREE;
- tree gnu_result;
+ tree gnu_return_var_elmt;
+ /* Its source location. */
location_t locus;
- struct language_function *gnu_subprog_language;
- vec *cache;
/* If this is a generic subprogram or it has been eliminated, ignore it. */
if (Is_Generic_Subprogram (gnat_subprog) || Is_Eliminated (gnat_subprog))
@@ -3886,9 +3881,11 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
gnu_cico_list = TYPE_CI_CO_LIST (gnu_subprog_type);
if (gnu_cico_list && TREE_VALUE (gnu_cico_list) == void_type_node)
gnu_return_var_elmt =
[Ada] Small housekeeping work
No functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* gcc-interface/trans.cc (Subprogram_Body_to_gnu): Rename a couple
of local variables and use Is_Generic_Subprogram predicate.
(process_decls): Likewise.diff --git a/gcc/ada/gcc-interface/trans.cc b/gcc/ada/gcc-interface/trans.cc
--- a/gcc/ada/gcc-interface/trans.cc
+++ b/gcc/ada/gcc-interface/trans.cc
@@ -3848,11 +3848,11 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
specification has appeared before for this body, then the identifier
occurring in that specification will also be a defining identifier and all
the calls to this subprogram will point to that specification. */
- Entity_Id gnat_subprog_id
+ Entity_Id gnat_subprog
= (Present (Corresponding_Spec (gnat_node))
? Corresponding_Spec (gnat_node) : Defining_Entity (gnat_node));
/* The FUNCTION_DECL node corresponding to the subprogram spec. */
- tree gnu_subprog_decl;
+ tree gnu_subprog;
/* Its RESULT_DECL node. */
tree gnu_result_decl;
/* Its FUNCTION_TYPE node. */
@@ -3866,11 +3866,8 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
struct language_function *gnu_subprog_language;
vec *cache;
- /* If this is a generic object or if it has been eliminated,
- ignore it. */
- if (Ekind (gnat_subprog_id) == E_Generic_Procedure
- || Ekind (gnat_subprog_id) == E_Generic_Function
- || Is_Eliminated (gnat_subprog_id))
+ /* If this is a generic subprogram or it has been eliminated, ignore it. */
+ if (Is_Generic_Subprogram (gnat_subprog) || Is_Eliminated (gnat_subprog))
return;
/* If this subprogram acts as its own spec, define it. Otherwise, just get
@@ -3879,13 +3876,13 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
treat it as not being defined in that case. Such a subprogram cannot
have an address clause or a freeze node, so this test is safe, though it
does disable some otherwise-useful error checking. */
- gnu_subprog_decl
-= gnat_to_gnu_entity (gnat_subprog_id, NULL_TREE,
+ gnu_subprog
+= gnat_to_gnu_entity (gnat_subprog, NULL_TREE,
Acts_As_Spec (gnat_node)
- && !present_gnu_tree (gnat_subprog_id));
- DECL_FUNCTION_IS_DEF (gnu_subprog_decl) = true;
- gnu_result_decl = DECL_RESULT (gnu_subprog_decl);
- gnu_subprog_type = TREE_TYPE (gnu_subprog_decl);
+ && !present_gnu_tree (gnat_subprog));
+ DECL_FUNCTION_IS_DEF (gnu_subprog) = true;
+ gnu_result_decl = DECL_RESULT (gnu_subprog);
+ gnu_subprog_type = TREE_TYPE (gnu_subprog);
gnu_cico_list = TYPE_CI_CO_LIST (gnu_subprog_type);
if (gnu_cico_list && TREE_VALUE (gnu_cico_list) == void_type_node)
gnu_return_var_elmt = gnu_cico_list;
@@ -3900,25 +3897,25 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
}
/* Set the line number in the decl to correspond to that of the body. */
- if (DECL_IGNORED_P (gnu_subprog_decl))
+ if (DECL_IGNORED_P (gnu_subprog))
locus = UNKNOWN_LOCATION;
- else if (!Sloc_to_locus (Sloc (gnat_node), , false, gnu_subprog_decl))
+ else if (!Sloc_to_locus (Sloc (gnat_node), , false, gnu_subprog))
locus = input_location;
- DECL_SOURCE_LOCATION (gnu_subprog_decl) = locus;
+ DECL_SOURCE_LOCATION (gnu_subprog) = locus;
/* Try to create a bona-fide thunk and hand it over to the middle-end. */
- if (Is_Thunk (gnat_subprog_id)
- && maybe_make_gnu_thunk (gnat_subprog_id, gnu_subprog_decl))
+ if (Is_Thunk (gnat_subprog)
+ && maybe_make_gnu_thunk (gnat_subprog, gnu_subprog))
return;
/* Initialize the information structure for the function. */
- allocate_struct_function (gnu_subprog_decl, false);
+ allocate_struct_function (gnu_subprog, false);
gnu_subprog_language = ggc_cleared_alloc ();
- DECL_STRUCT_FUNCTION (gnu_subprog_decl)->language = gnu_subprog_language;
- DECL_STRUCT_FUNCTION (gnu_subprog_decl)->function_start_locus = locus;
+ DECL_STRUCT_FUNCTION (gnu_subprog)->language = gnu_subprog_language;
+ DECL_STRUCT_FUNCTION (gnu_subprog)->function_start_locus = locus;
set_cfun (NULL);
- begin_subprog_body (gnu_subprog_decl);
+ begin_subprog_body (gnu_subprog);
/* If there are copy-in/copy-out parameters, we need to ensure that they are
properly copied out by the return statement. We do this by making a new
@@ -3946,7 +3943,7 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
= create_var_decl (get_identifier ("RETVAL"), NULL_TREE,
gnu_return_type, NULL_TREE,
false, false, false, false, false,
- true, false, NULL, gnat_subprog_id);
+ true, false, NULL, gnat_subprog);
TREE_VALUE (gnu_return_var_elmt) = gnu_return_var;
}
@@ -3957,7 +3954,7 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
put it into TYPE_CI_CO_LIST, which must contain an empty entry too.
We can match up the entries because TYPE_CI_CO_LIST is in the order
of the parameters. */
- for (gnat_param = First_Formal_With_Extras (gnat_subprog_id);
+
[Ada] Remove dead code for scope entity having E_Subprogram_Body kind
In GNAT AST the Scope field always points to the semantic scope (e.g. subprogram) and never to syntactic scope (e.g. subprogram body). Cleanup related to handling of circular access-to-record types. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * gcc-interface/decl.cc (gnat_to_gnu_entity): Remove dead code which expected Scope to return E_Subprogram_Body entity.diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc --- a/gcc/ada/gcc-interface/decl.cc +++ b/gcc/ada/gcc-interface/decl.cc @@ -363,10 +363,6 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) if (Is_Type (gnat_temp)) gnat_temp = Underlying_Type (gnat_temp); - if (Ekind (gnat_temp) == E_Subprogram_Body) - gnat_temp - = Corresponding_Spec (Parent (Declaration_Node (gnat_temp))); - if (Is_Subprogram (gnat_temp) && Present (Protected_Body_Subprogram (gnat_temp))) gnat_temp = Protected_Body_Subprogram (gnat_temp);
Re: [PATCH] Fix OMP CAS expansion with separate condition
On Thu, 19 May 2022, Jakub Jelinek wrote:
> On Fri, May 13, 2022 at 12:23:01PM +0200, Richard Biener wrote:
> > 2022-05-13 Richard Biener
> >
> > * omp-expand.cc (expand_omp_atomic_cas): Do not short-cut
> > computation of the new value.
>
> Ok, thanks.
>
> Though, depending on what exactly you allow or disallow, maybe even
> the im != 0 might not be acceptable.
> Oh, and if COND_EXPRs can only use some limited set of comparisons, we might
> need to adjust e.g. arith_overflow_check_p and various other spots in
> tree-ssa-math-opts.cc and other passes.
With the changes I have in the queue the !cond_stmt path should be
effectively dead code (though I don't remove such immediately).
Not sure what you think the issue with arith_overflow_check_p would be?
Note this is only about COND_EXPRs in gimple assignments.
I'm going to push this OMP patch later but will delay pushing the
COND_EXPR patches until next week so I'm around more reliably to
deal with fallout.
Richard.
> > diff --git a/gcc/omp-expand.cc b/gcc/omp-expand.cc
> > index ee708314793..9fcc67a3448 100644
> > --- a/gcc/omp-expand.cc
> > +++ b/gcc/omp-expand.cc
> > @@ -9092,16 +9092,17 @@ expand_omp_atomic_cas (basic_block load_bb, tree
> > addr,
> >
> >if (cond_stmt)
> > {
> > - g = gimple_build_assign (gimple_assign_lhs (cond_stmt),
> > - NOP_EXPR, im);
> > + g = gimple_build_assign (cond, NOP_EXPR, im);
> > gimple_set_location (g, loc);
> > gsi_insert_before (, g, GSI_SAME_STMT);
> > }
> > - else if (need_new)
> > +
> > + if (need_new)
> > {
> > g = gimple_build_assign (create_tmp_reg (itype), COND_EXPR,
> > - build2 (NE_EXPR, boolean_type_node,
> > - im, build_zero_cst (itype)),
> > + cond_stmt
> > + ? cond : build2 (NE_EXPR, boolean_type_node,
> > + im, build_zero_cst (itype)),
> >d, re);
> > gimple_set_location (g, loc);
> > gsi_insert_before (, g, GSI_SAME_STMT);
> > --
> > 2.35.3
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
Re: [Patch] OpenMP: Handle descriptors in target's firstprivate [PR104949]
On Wed, May 11, 2022 at 07:33:00PM +0200, Tobias Burnus wrote: > gcc/fortran/ChangeLog: > > PR fortran/104949 > * f95-lang.cc (LANG_HOOKS_OMP_ARRAY_SIZE): Redefine. > * trans-openmp.cc (gfc_omp_array_size): New. > (gfc_trans_omp_variable_list): Never turn has_device_addr > to firstprivate. > * trans.h (gfc_omp_array_size): New. > > gcc/ChangeLog: > > PR fortran/104949 > * langhooks-def.h (lhd_omp_array_size): New. > (LANG_HOOKS_OMP_ARRAY_SIZE): Define Missing full stop above. > (LANG_HOOKS_DECLS): Add it. > * langhooks.cc (lhd_omp_array_size): New. > * langhooks.h (struct lang_hooks_for_decls): Add hook. > * omp-low.cc (scan_sharing_clauses, lower_omp_target): > Handle GOMP_MAP_FIRSTPRIVATE for array descriptors. > > libgomp/ChangeLog: > > PR fortran/104949 > * target.c (gomp_map_vars_internal, copy_firstprivate_data): > Support attach for GOMP_MAP_FIRSTPRIVATE. > * testsuite/libgomp.fortran/target-firstprivate-1.f90: New test. > * testsuite/libgomp.fortran/target-firstprivate-2.f90: New test. > * testsuite/libgomp.fortran/target-firstprivate-3.f90: New test. I guess ok like this for now, but handling the further deep copy cases (allocatable members of derived types) wouldn't be very nice, I think generally we need a target hook to handle the stuff that is target specific and express it say in further clauses or their modified copies (perhaps some flags on them, or new clause types) which will allow the pointer attachments to be done. Generally, for firstprivate on constructs other than target we invoke a copy constructor or its language equivalent (memcpy for C, perhaps some deep copying for Fortran), which takes care of stuff like in C++ embedded reference type members etc. But for target we don't have such a luxury, we don't have copy ctors between different devices and so we need to do something different, for now it has been mainly just bitwise copying the aggregate between devices. But eventually it would be nice to have a target hook that emulates the cross-device copy construction. And we probably need also something to emulate destruction... Jakub
Re: [PATCH] Fix OMP CAS expansion with separate condition
On Fri, May 13, 2022 at 12:23:01PM +0200, Richard Biener wrote:
> 2022-05-13 Richard Biener
>
> * omp-expand.cc (expand_omp_atomic_cas): Do not short-cut
> computation of the new value.
Ok, thanks.
Though, depending on what exactly you allow or disallow, maybe even
the im != 0 might not be acceptable.
Oh, and if COND_EXPRs can only use some limited set of comparisons, we might
need to adjust e.g. arith_overflow_check_p and various other spots in
tree-ssa-math-opts.cc and other passes.
> diff --git a/gcc/omp-expand.cc b/gcc/omp-expand.cc
> index ee708314793..9fcc67a3448 100644
> --- a/gcc/omp-expand.cc
> +++ b/gcc/omp-expand.cc
> @@ -9092,16 +9092,17 @@ expand_omp_atomic_cas (basic_block load_bb, tree addr,
>
>if (cond_stmt)
> {
> - g = gimple_build_assign (gimple_assign_lhs (cond_stmt),
> -NOP_EXPR, im);
> + g = gimple_build_assign (cond, NOP_EXPR, im);
> gimple_set_location (g, loc);
> gsi_insert_before (, g, GSI_SAME_STMT);
> }
> - else if (need_new)
> +
> + if (need_new)
> {
> g = gimple_build_assign (create_tmp_reg (itype), COND_EXPR,
> -build2 (NE_EXPR, boolean_type_node,
> -im, build_zero_cst (itype)),
> +cond_stmt
> +? cond : build2 (NE_EXPR, boolean_type_node,
> + im, build_zero_cst (itype)),
> d, re);
> gimple_set_location (g, loc);
> gsi_insert_before (, g, GSI_SAME_STMT);
> --
> 2.35.3
Jakub
Re: [PATCH] Add divide by zero side effect.
On 5/18/22 16:40, Segher Boessenkool wrote: On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote: On 5/18/22 14:13, Segher Boessenkool wrote: "Side effect" already has a meaning, very commonly used in language theory, and even in the C standard itself: a function has a side effect if it does something more than just return a value: if it changes state. This can be some I/O, or it can just be writing to some non-local data. Side effects are crucial to what a compiler does, and they are used all over the place (the gcc/ dir has some thousand mentions of it for example). Please don't make life hard for everyone by overloading this term. I'm open to suggestions for a better term! Glad to hear that, and this isn't set in stione yet! Is there a commonly used alternate term to describe an observable effect on the value of an input operand? I'd use something with "known" in the name. But: As far as I understand what you are doing this is not an effect on the operand at all! It cannot be one even, the operand is an input only after all. Instead, it changes what is known about the value of that input: it cannot be 0 in this case, it is known to not be 0. This is similar to other known value things we have in GCC already. Can you not just use one of those, even? What are the benefit to this new abstraction? Well, This is a component of ranger tracking value ranges.. it is recording the "side-effect" of the stmt on the known range of an object. The file is called "gimple-range-side-effect.h" Its a generalization of how ranger tracks non-null pointer values, enabling it to track arbitrary observable ranges values. (The old mechanism also utilized immediate-use chains, which prevented it from being utilized in gimple-folding) WIth this change, we can also track things like a = b / c causing the effect that c is known non-zero after the statement if there were no traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which after 15 years, we can now simply indicate that for a = b >> c , its only defined behaviour if c is in the range [0, PRECISION(b)] So its basically just a generalization of how we track known values within the range system. Andrew
Re: [COMMITTED] Add ranger side effect infrastructure.
Hi Andrew,
On Tue, 17 May 2022 14:38:39 -0400
Andrew MacLeod via Gcc-patches wrote:
> commit b7501739f3b14ac7749aace93f636d021fd607f7
> Author: Andrew MacLeod
> Date: Mon May 9 15:35:14 2022 -0400
> diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> index d3cf8be9bd8..56f4577cfbb 100644
> --- a/gcc/gimple-range-cache.cc
> +++ b/gcc/gimple-range-cache.cc
> @@ -1545,81 +1426,42 @@ ranger_cache::range_from_dom (irange , tree name,
> basic_block start_bb,
> void
> -ranger_cache::update_to_nonnull (basic_block bb, tree name)
> +ranger_cache::apply_side_effects (gimple *s)
> {
> - if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
> + // Do not update the on-netry cache for block ending stmts.
s/on-netry/on-entry/
> diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
> new file mode 100644
> index 000..2c8c77dc569
> --- /dev/null
> +++ b/gcc/gimple-range-side-effect.cc
> +// -
> +
> +// This class is an element in list of side effect ranges.
s/in list/in the list/
> +// Construct a side effects manager. DO_SEARCH indicates whether an
> immediate
> +// use scan should be made the first time a name is processed. This is for
> +// on-demand clients who may not visit every statement and may miss uses.
may otherwise miss uses, i suppose. But ok.
> diff --git a/gcc/gimple-range-side-effect.h b/gcc/gimple-range-side-effect.h
> new file mode 100644
> index 000..848d94ba6d7
> --- /dev/null
> +++ b/gcc/gimple-range-side-effect.h
> @@ -0,0 +1,82 @@
> +// New side effects should added in the constructor of this class.
s/should added/should be added/
> +
> +class stmt_side_effects
> +{
> +};
> +
> +// This class manages a list of side effect ranges for each basic block.
> +// As side effects are seen, they can be registered to a block and later
> +// queried. WHen constructed with a TRUE flag, immediate uses chains are
> +// followed the first time a name is referenced and block populated if
> +// thre are any side effects.
s/WHen/When/
s/thre are/there are/
> +
> +class side_effect_manager
> +{
> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> index 1fdee026a4b..f5e9e77bc71 100644
> --- a/gcc/gimple-range.cc
> +++ b/gcc/gimple-range.cc
> @@ -118,9 +118,11 @@ gimple_ranger::range_of_expr (irange , tree expr,
> gimple *stmt)
>// If name is defined in this block, try to get an range from S.
>if (def_stmt && gimple_bb (def_stmt) == bb)
> {
> - // Check for a definition override from a block walk.
> - if (!POINTER_TYPE_P (TREE_TYPE (expr))
> - || !m_cache.block_range (r, bb, expr, false))
> + // Declared in ths block, if it has a global set, check for an
> + // override from a block walk, otherwise calculate it.
s/ths/this/
And, btw, i see
In file included from ../../../src/gcc-13.mine/gcc/tree-vrp.h:23,
from ../../../src/gcc-13.mine/gcc/ssa.h:28,
from ../../../src/gcc-13.mine/gcc/gimple-warn-types.cc:30:
../../../src/gcc-13.mine/gcc/value-range.h: In member function ‘unsigned int
irange::num_pairs() const’:
../../../src/gcc-13.mine/gcc/value-range.h:217:22: warning: Function
‘num_pairs’ could return ‘unsigned char’ [--all-warnings]
217 | irange::num_pairs () const
| ^
| unsigned char
../../../src/gcc-13.mine/gcc/value-range.h:217:22: note: with a range of [0,255]
The C++ member function decl result locations are all wrong, i know,
but i think the warning is correct. We've got:
class GTY((user)) irange
{
unsigned char m_num_ranges;
}
but
inline unsigned
irange::num_pairs () const
{
if (m_kind == VR_ANTI_RANGE)
return constant_p () ? 2 : 1;
else
return m_num_ranges;
}
thanks,
Re: [PATCH v3 09/10] libgcc: Add support for HF mode (aka _Float16) in libbid
Hi Joseph,
May I ping you on this version? (I've moved the tests to gcc.dg/torture/
and checked they work on aarch64 and x86_64.
Thanks,
Christophe
On 5/16/22 15:47, Christophe Lyon wrote:
This patch adds support for trunc and extend operations between HF
mode (_Float16) and Decimal Floating Point formats (_Decimal32,
_Decimal64 and _Decimal128).
For simplicity we rely on the implicit conversions inserted by the
compiler between HF and SD/DF/TF modes. The existing bid*_to_binary*
and binary*_to_bid* functions are non-trivial and at this stage it is
not clear if there is a performance-critical use case involving _Float16
and _Decimal* formats.
The patch also adds two executable tests, to make sure the right
functions are called, available (link phase) and functional.
Tested on aarch64 and x86_86. The number of symbol matches in the
testcases includes the .global XXX to avoid having to match different
call instructions for different targets.
2022-05-04 Christophe Lyon
libgcc/ChangeLog:
* Makefile.in (D32PBIT_FUNCS): Add _hf_to_sd and _sd_to_hf.
(D64PBIT_FUNCS): Add _hf_to_dd and _dd_to_hf.
(D128PBIT_FUNCS): Add _hf_to_td _td_to_hf.
libgcc/config/libbid/ChangeLog:
* bid_gcc_intrinsics.h (LIBGCC2_HAS_HF_MODE): Define according to
__LIBGCC_HAS_HF_MODE__.
(BID_HAS_HF_MODE): Define.
(HFtype): Define.
(__bid_extendhfsd): New prototype.
(__bid_extendhfdd): Likewise.
(__bid_extendhftd): Likewise.
(__bid_truncsdhf): Likewise.
(__bid_truncddhf): Likewise.
(__bid_trunctdhf): Likewise.
* _dd_to_hf.c: New file.
* _hf_to_dd.c: New file.
* _hf_to_sd.c: New file.
* _hf_to_td.c: New file.
* _sd_to_hf.c: New file.
* _td_to_hf.c: New file.
gcc/testsuite/ChangeLog:
* gcc.dg/torture/convert-dfp-2.c: New test.
* gcc.dg/torture/convert-dfp.c: New test.
---
gcc/testsuite/gcc.dg/torture/convert-dfp-2.c | 45 ++
gcc/testsuite/gcc.dg/torture/convert-dfp.c | 63
libgcc/Makefile.in | 9 ++-
libgcc/config/libbid/_dd_to_hf.c | 38
libgcc/config/libbid/_hf_to_dd.c | 36 +++
libgcc/config/libbid/_hf_to_sd.c | 36 +++
libgcc/config/libbid/_hf_to_td.c | 36 +++
libgcc/config/libbid/_sd_to_hf.c | 38
libgcc/config/libbid/_td_to_hf.c | 38
libgcc/config/libbid/bid_gcc_intrinsics.h| 30 +-
10 files changed, 364 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/convert-dfp-2.c
create mode 100644 gcc/testsuite/gcc.dg/torture/convert-dfp.c
create mode 100644 libgcc/config/libbid/_dd_to_hf.c
create mode 100644 libgcc/config/libbid/_hf_to_dd.c
create mode 100644 libgcc/config/libbid/_hf_to_sd.c
create mode 100644 libgcc/config/libbid/_hf_to_td.c
create mode 100644 libgcc/config/libbid/_sd_to_hf.c
create mode 100644 libgcc/config/libbid/_td_to_hf.c
diff --git a/gcc/testsuite/gcc.dg/torture/convert-dfp-2.c
b/gcc/testsuite/gcc.dg/torture/convert-dfp-2.c
new file mode 100644
index 000..3e4ecb57ba6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/convert-dfp-2.c
@@ -0,0 +1,45 @@
+/* { dg-do run } */
+/* { dg-require-effective-target float16_runtime } */
+/* { dg-require-effective-target dfprt } */
+/* { dg-options "-save-temps" } */
+/* { dg-add-options float16 } */
+
+/* Test conversions from DFP to smaller types. */
+
+_Decimal32 var32;
+_Decimal64 var64;
+_Decimal128 var128;
+_Float16 var16;
+
+void __attribute__ ((__noinline__)) foo32 (_Decimal32 param32)
+{
+ var16 = param32;
+}
+
+void __attribute__ ((__noinline__)) foo64 (_Decimal64 param64)
+{
+ var16 = param64;
+ var32 = param64;
+}
+
+void __attribute__ ((__noinline__)) foo128 (_Decimal128 param128)
+{
+ var16 = param128;
+ var32 = param128;
+ var64 = param128;
+}
+
+int main ()
+{
+ foo32 (var32);
+ foo64 (var64);
+ foo128 (var128);
+ return 0;
+}
+
+/* { dg-final { scan-assembler-times {\t__bid_truncsdhf} 2 { target { dfp_bid
} } } } */
+/* { dg-final { scan-assembler-times {\t__bid_truncddhf} 2 { target { dfp_bid
} } } } */
+/* { dg-final { scan-assembler-times {\t__bid_truncddsd2} 2 { target { dfp_bid
} } } } */
+/* { dg-final { scan-assembler-times {\t__bid_trunctdhf} 2 { target { dfp_bid
} } } } */
+/* { dg-final { scan-assembler-times {\t__bid_trunctdsd2} 2 { target { dfp_bid
} } } } */
+/* { dg-final { scan-assembler-times {\t__bid_trunctddd2} 2 { target { dfp_bid
} } } } */
diff --git a/gcc/testsuite/gcc.dg/torture/convert-dfp.c
b/gcc/testsuite/gcc.dg/torture/convert-dfp.c
new file mode 100644
index 000..ec136896ca7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/convert-dfp.c
@@ -0,0 +1,63 @@
+/* { dg-do run } */
+/* { dg-require-effective-target float16_runtime } */
+/* { dg-require-effective-target
Re: [PATCH] PR105647 Update pr105169* so it does not fail on powerpc64le
On Thu, 2022-05-19 at 06:34 +, Richard Biener wrote:
> On Wed, 18 May 2022, Giuliano Belinassi wrote:
>
> > On powerpc64le, the tests related to pr105169 failed because the
> > .localentry was not on a power of two address due to the extra nop
> > instruction taking one byte and thus moving its position one byte
> > further. Generating two nops instead moves .localentry to a valid
> > position.
>
> OK
Pushed to trunk.
>
> > gcc/testsuite/ChangeLog
> > 2022-05-18 Giuliano Belinassi
> >
> > PR target/105647
> > * g++.dg/modules/pr105169_a.C: Change -fpatchable-function-
> > entry to 2.
> > * g++.dg/modules/pr105169_b.C: Likewise.
> > ---
> > gcc/testsuite/g++.dg/modules/pr105169_a.C | 4 ++--
> > gcc/testsuite/g++.dg/modules/pr105169_b.C | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > index 66dc4b7901f..02660b3a0e4 100644
> > --- a/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > +++ b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > @@ -1,6 +1,6 @@
> > /* { dg-module-do link } */
> > -/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> > -/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=1 -O2" } */
> > +/* { dg-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
> > +/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=2 -O2" } */
> >
> > /* This test is in the "modules" package because it supports
> > multiple files
> > linkage. */
> > diff --git a/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > index 5f8b00dfe51..7a9c5863a6a 100644
> > --- a/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > +++ b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > @@ -1,6 +1,6 @@
> > /* { dg-module-do link } */
> > -/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> > -/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=1 -O2" } */
> > +/* { dg-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
> > +/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=2 -O2" } */
> >
> > /* This test is in the "modules" package because it supports
> > multiple files
> > linkage. */
> >
[committed] analyzer: update docs about -fanalyzer-checker=taint
Tested as part of a bootstrap (on x86_64-pc-linux-gnu); verified
generated manpage.
Committed to trunk as r13-632-g19258b113872bd (as a followup to
r13-514-g2402dc6b982c4d).
gcc/ChangeLog:
* doc/invoke.texi (-fanalyzer-checker=): Add
-Wanalyzer-va-list-leak and -Wanalyzer-va-list-use-after-va-end to
the list of analyzer warnings disabled by
-fanalyzer-checker=taint.
---
gcc/doc/invoke.texi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e8e6d4e039b..37e6556943e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10108,6 +10108,8 @@ following warnings from @option{-fanalyzer}:
-Wanalyzer-possible-null-dereference @gol
-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
+-Wanalyzer-va-list-leak @gol
+-Wanalyzer-va-list-use-after-va-end @gol
}
@item -fno-analyzer-feasibility
--
2.26.3
Re: [PATCH 8/8] aarch64: update for move of FINAL and OVERRIDE
David Malcolm via Gcc-patches writes:
> gcc/ChangeLog:
> * config/aarch64/aarch64-sve-builtins-base.cc: Replace uses of
> "FINAL" and "OVERRIDE" with "final" and "override".
> * config/aarch64/aarch64-sve-builtins-functions.h: Likewise.
> * config/aarch64/aarch64-sve-builtins-shapes.cc: Likewise.
> * config/aarch64/aarch64-sve-builtins-sve2.cc: Likewise.
OK, thanks.
It was a pity that this work was ahead of the move to C++11 by
just 1 release :-)
Richard
> Signed-off-by: David Malcolm
> ---
> .../aarch64/aarch64-sve-builtins-base.cc | 260 +--
> .../aarch64/aarch64-sve-builtins-functions.h | 48 +-
> .../aarch64/aarch64-sve-builtins-shapes.cc| 416 +-
> .../aarch64/aarch64-sve-builtins-sve2.cc | 44 +-
> 4 files changed, 384 insertions(+), 384 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index c24c0548724..bee410929bd 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -148,7 +148,7 @@ class svabd_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* The integer operations are represented as the subtraction of the
> minimum from the maximum, with the signedness of the instruction
> @@ -179,7 +179,7 @@ public:
>CONSTEXPR svac_impl (int unspec) : m_unspec (unspec) {}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> e.add_ptrue_hint (0, e.gp_mode (0));
> insn_code icode = code_for_aarch64_pred_fac (m_unspec, e.vector_mode
> (0));
> @@ -194,7 +194,7 @@ class svadda_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Put the predicate last, as required by mask_fold_left_plus_optab. */
> e.rotate_inputs_left (0, 3);
> @@ -211,7 +211,7 @@ public:
>CONSTEXPR svadr_bhwd_impl (unsigned int shift) : m_shift (shift) {}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> machine_mode mode = GET_MODE (e.args[0]);
> if (m_shift == 0)
> @@ -231,7 +231,7 @@ class svbic_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Convert svbic of a constant into svand of its inverse. */
> if (CONST_INT_P (e.args[2]))
> @@ -261,7 +261,7 @@ public:
>CONSTEXPR svbrk_binary_impl (int unspec) : m_unspec (unspec) {}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> return e.use_exact_insn (code_for_aarch64_brk (m_unspec));
>}
> @@ -277,7 +277,7 @@ public:
>CONSTEXPR svbrk_unary_impl (int unspec) : m_unspec (unspec) {}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> return e.use_cond_insn (code_for_aarch64_brk (m_unspec));
>}
> @@ -290,7 +290,7 @@ class svcadd_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Convert the rotation amount into a specific unspec. */
> int rot = INTVAL (e.args.pop ());
> @@ -311,7 +311,7 @@ public:
>CONSTEXPR svclast_impl (int unspec) : m_unspec (unspec) {}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Match the fold_extract_optab order. */
> std::swap (e.args[0], e.args[1]);
> @@ -332,7 +332,7 @@ class svcmla_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Convert the rotation amount into a specific unspec. */
> int rot = INTVAL (e.args.pop ());
> @@ -355,7 +355,7 @@ class svcmla_lane_impl : public function_base
> {
> public:
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> /* Convert the rotation amount into a specific unspec. */
> int rot = INTVAL (e.args.pop ());
> @@ -384,7 +384,7 @@ public:
> : m_code (code), m_unspec_for_fp (unspec_for_fp) {}
>
>gimple *
> - fold (gimple_folder ) const OVERRIDE
> + fold (gimple_folder ) const override
>{
> tree pg = gimple_call_arg (f.call, 0);
> tree rhs1 = gimple_call_arg (f.call, 1);
> @@ -406,7 +406,7 @@ public:
>}
>
>rtx
> - expand (function_expander ) const OVERRIDE
> + expand (function_expander ) const override
>{
> machine_mode mode = e.vector_mode (0);
>
> @@ -442,7 +442,7 @@ public:
>
Re: [PATCH 1/8] Move FINAL and OVERRIDE from ansidecl.h to gcc-plugin.h
On Thu, May 19, 2022 at 2:43 PM David Malcolm via Gcc-patches wrote: > > On Thu, 2022-05-19 at 08:32 -0400, David Malcolm wrote: > > As of GCC 11 onwards we have required a C++11 compiler, such as GCC > > 4.8 > > or later. On the assumption that any such compiler correctly > > implements > > "final" and "override", this patch removes the FINAL and OVERRIDE > > macros > > from ansidecl.h. It adds them back to gcc-plugins.h in case any > > plugins > > make use of them. > > > > Followup patches within this kit replace all uses of the macros in > > the > > tree with the lower-case versions; I've split them up for ease of > > review, but would apply them as one combined commit if approved. > > I forgot to say: > I've successfully bootstrapped & regrtested the combined patches on > x86_64-pc-linux-gnu, and (very) lightly tested them on aarch64-linux- > gnu. And I forgot to say - the patches doing the replacement are fine en-bloc Thanks, Richard. > Dave > > > > > gcc/ChangeLog: > > * gcc-plugin.h: Add macros OVERRIDE and FINAL. > > > > include/ChangeLog: > > * ansidecl.h: Drop macros OVERRIDE and FINAL. > > > > Signed-off-by: David Malcolm > > --- > > gcc/gcc-plugin.h | 7 +++ > > include/ansidecl.h | 41 - > > 2 files changed, 7 insertions(+), 41 deletions(-) > > > > diff --git a/gcc/gcc-plugin.h b/gcc/gcc-plugin.h > > index ee9aa865114..f0f3049d0ce 100644 > > --- a/gcc/gcc-plugin.h > > +++ b/gcc/gcc-plugin.h > > @@ -44,4 +44,11 @@ along with GCC; see the file COPYING3. If not see > > #include "tree-check.h" > > #include "plugin.h" > > > > +/* Versions of GCC prior to GCC 13 defined these macros in > > ansidecl.h > > + for C++98 compatibility. Given that we require C++11 from GCC 11 > > onwards, > > + keep definitions here in case plugins made use of them. */ > > + > > +#define OVERRIDE override > > +#define FINALfinal > > + > > #endif /* GCC_PLUGIN_H */ > > diff --git a/include/ansidecl.h b/include/ansidecl.h > > index efee5b6904b..119748deb68 100644 > > --- a/include/ansidecl.h > > +++ b/include/ansidecl.h > > @@ -345,47 +345,6 @@ So instead we use the macro below and test it > > against specific values. */ > > #define CONSTEXPR > > #endif > > > > -/* C++11 adds the ability to add "override" after an implementation > > of a > > - virtual function in a subclass, to: > > - (A) document that this is an override of a virtual function > > - (B) allow the compiler to issue a warning if it isn't (e.g. a > > mismatch > > - of the type signature). > > - > > - Similarly, it allows us to add a "final" to indicate that no > > subclass > > - may subsequently override the vfunc. > > - > > - Provide OVERRIDE and FINAL as macros, allowing us to get these > > benefits > > - when compiling with C++11 support, but without requiring C++11. > > - > > - For gcc, use "-std=c++11" to enable C++11 support; gcc 6 onwards > > enables > > - this by default (actually GNU++14). */ > > - > > -#if defined __cplusplus > > -# if __cplusplus >= 201103 > > - /* C++11 claims to be available: use it. Final/override were > > only > > - implemented in 4.7, though. */ > > -# if GCC_VERSION < 4007 > > -# define OVERRIDE > > -# define FINAL > > -# else > > -# define OVERRIDE override > > -# define FINAL final > > -# endif > > -# elif GCC_VERSION >= 4007 > > - /* G++ 4.7 supports __final in C++98. */ > > -# define OVERRIDE > > -# define FINAL __final > > -# else > > - /* No C++11 support; leave the macros empty. */ > > -# define OVERRIDE > > -# define FINAL > > -# endif > > -#else > > - /* No C++11 support; leave the macros empty. */ > > -# define OVERRIDE > > -# define FINAL > > -#endif > > - > > /* A macro to disable the copy constructor and assignment operator. > > When building with C++11 and above, the methods are explicitly > > deleted, causing a compile-time error if something tries to copy. > >
Re: [PATCH 1/8] Move FINAL and OVERRIDE from ansidecl.h to gcc-plugin.h
On Thu, May 19, 2022 at 2:33 PM David Malcolm via Gcc-patches wrote: > > As of GCC 11 onwards we have required a C++11 compiler, such as GCC 4.8 > or later. On the assumption that any such compiler correctly implements > "final" and "override", this patch removes the FINAL and OVERRIDE macros > from ansidecl.h. It adds them back to gcc-plugins.h in case any plugins > make use of them. I don't think we need to care for that case so please do not add them to gcc-plugins.h. But ansidecl.h is shared between gcc and binutils/gdb (and libiberty in general), can you get an ack from their side as well please? Thanks, Richard. > Followup patches within this kit replace all uses of the macros in the > tree with the lower-case versions; I've split them up for ease of > review, but would apply them as one combined commit if approved. > > gcc/ChangeLog: > * gcc-plugin.h: Add macros OVERRIDE and FINAL. > > include/ChangeLog: > * ansidecl.h: Drop macros OVERRIDE and FINAL. > > Signed-off-by: David Malcolm > --- > gcc/gcc-plugin.h | 7 +++ > include/ansidecl.h | 41 - > 2 files changed, 7 insertions(+), 41 deletions(-) > > diff --git a/gcc/gcc-plugin.h b/gcc/gcc-plugin.h > index ee9aa865114..f0f3049d0ce 100644 > --- a/gcc/gcc-plugin.h > +++ b/gcc/gcc-plugin.h > @@ -44,4 +44,11 @@ along with GCC; see the file COPYING3. If not see > #include "tree-check.h" > #include "plugin.h" > > +/* Versions of GCC prior to GCC 13 defined these macros in ansidecl.h > + for C++98 compatibility. Given that we require C++11 from GCC 11 onwards, > + keep definitions here in case plugins made use of them. */ > + > +#define OVERRIDE override > +#define FINALfinal > + > #endif /* GCC_PLUGIN_H */ > diff --git a/include/ansidecl.h b/include/ansidecl.h > index efee5b6904b..119748deb68 100644 > --- a/include/ansidecl.h > +++ b/include/ansidecl.h > @@ -345,47 +345,6 @@ So instead we use the macro below and test it against > specific values. */ > #define CONSTEXPR > #endif > > -/* C++11 adds the ability to add "override" after an implementation of a > - virtual function in a subclass, to: > - (A) document that this is an override of a virtual function > - (B) allow the compiler to issue a warning if it isn't (e.g. a mismatch > - of the type signature). > - > - Similarly, it allows us to add a "final" to indicate that no subclass > - may subsequently override the vfunc. > - > - Provide OVERRIDE and FINAL as macros, allowing us to get these benefits > - when compiling with C++11 support, but without requiring C++11. > - > - For gcc, use "-std=c++11" to enable C++11 support; gcc 6 onwards enables > - this by default (actually GNU++14). */ > - > -#if defined __cplusplus > -# if __cplusplus >= 201103 > - /* C++11 claims to be available: use it. Final/override were only > - implemented in 4.7, though. */ > -# if GCC_VERSION < 4007 > -# define OVERRIDE > -# define FINAL > -# else > -# define OVERRIDE override > -# define FINAL final > -# endif > -# elif GCC_VERSION >= 4007 > - /* G++ 4.7 supports __final in C++98. */ > -# define OVERRIDE > -# define FINAL __final > -# else > - /* No C++11 support; leave the macros empty. */ > -# define OVERRIDE > -# define FINAL > -# endif > -#else > - /* No C++11 support; leave the macros empty. */ > -# define OVERRIDE > -# define FINAL > -#endif > - > /* A macro to disable the copy constructor and assignment operator. > When building with C++11 and above, the methods are explicitly > deleted, causing a compile-time error if something tries to copy. > -- > 2.26.3 >
Re: [PATCH 1/8] Move FINAL and OVERRIDE from ansidecl.h to gcc-plugin.h
On Thu, 2022-05-19 at 08:32 -0400, David Malcolm wrote: > As of GCC 11 onwards we have required a C++11 compiler, such as GCC > 4.8 > or later. On the assumption that any such compiler correctly > implements > "final" and "override", this patch removes the FINAL and OVERRIDE > macros > from ansidecl.h. It adds them back to gcc-plugins.h in case any > plugins > make use of them. > > Followup patches within this kit replace all uses of the macros in > the > tree with the lower-case versions; I've split them up for ease of > review, but would apply them as one combined commit if approved. I forgot to say: I've successfully bootstrapped & regrtested the combined patches on x86_64-pc-linux-gnu, and (very) lightly tested them on aarch64-linux- gnu. Dave > > gcc/ChangeLog: > * gcc-plugin.h: Add macros OVERRIDE and FINAL. > > include/ChangeLog: > * ansidecl.h: Drop macros OVERRIDE and FINAL. > > Signed-off-by: David Malcolm > --- > gcc/gcc-plugin.h | 7 +++ > include/ansidecl.h | 41 - > 2 files changed, 7 insertions(+), 41 deletions(-) > > diff --git a/gcc/gcc-plugin.h b/gcc/gcc-plugin.h > index ee9aa865114..f0f3049d0ce 100644 > --- a/gcc/gcc-plugin.h > +++ b/gcc/gcc-plugin.h > @@ -44,4 +44,11 @@ along with GCC; see the file COPYING3. If not see > #include "tree-check.h" > #include "plugin.h" > > +/* Versions of GCC prior to GCC 13 defined these macros in > ansidecl.h > + for C++98 compatibility. Given that we require C++11 from GCC 11 > onwards, > + keep definitions here in case plugins made use of them. */ > + > +#define OVERRIDE override > +#define FINAL final > + > #endif /* GCC_PLUGIN_H */ > diff --git a/include/ansidecl.h b/include/ansidecl.h > index efee5b6904b..119748deb68 100644 > --- a/include/ansidecl.h > +++ b/include/ansidecl.h > @@ -345,47 +345,6 @@ So instead we use the macro below and test it > against specific values. */ > #define CONSTEXPR > #endif > > -/* C++11 adds the ability to add "override" after an implementation > of a > - virtual function in a subclass, to: > - (A) document that this is an override of a virtual function > - (B) allow the compiler to issue a warning if it isn't (e.g. a > mismatch > - of the type signature). > - > - Similarly, it allows us to add a "final" to indicate that no > subclass > - may subsequently override the vfunc. > - > - Provide OVERRIDE and FINAL as macros, allowing us to get these > benefits > - when compiling with C++11 support, but without requiring C++11. > - > - For gcc, use "-std=c++11" to enable C++11 support; gcc 6 onwards > enables > - this by default (actually GNU++14). */ > - > -#if defined __cplusplus > -# if __cplusplus >= 201103 > - /* C++11 claims to be available: use it. Final/override were > only > - implemented in 4.7, though. */ > -# if GCC_VERSION < 4007 > -# define OVERRIDE > -# define FINAL > -# else > -# define OVERRIDE override > -# define FINAL final > -# endif > -# elif GCC_VERSION >= 4007 > - /* G++ 4.7 supports __final in C++98. */ > -# define OVERRIDE > -# define FINAL __final > -# else > - /* No C++11 support; leave the macros empty. */ > -# define OVERRIDE > -# define FINAL > -# endif > -#else > - /* No C++11 support; leave the macros empty. */ > -# define OVERRIDE > -# define FINAL > -#endif > - > /* A macro to disable the copy constructor and assignment operator. > When building with C++11 and above, the methods are explicitly > deleted, causing a compile-time error if something tries to copy.
Re: [PATCH take #2] PR middle-end/98865: Expand X*Y as X&-Y when Y is [0.1].
On Wed, May 18, 2022 at 6:55 PM Roger Sayle wrote:
>
>
> The patch is a revised solution for PR middle-end/98865 incorporating
> the feedback/suggestions from Richard Biener's review here:
> https://gcc.gnu.org/pipermail/gcc-patches/2022-May/593928.html
> Most significantly, this patch now performs the transformation/optimization
> during RTL expansion, where the target's rtx_costs can be used to determine
> whether the original multiplication (that may be possibly be implemented by
> a shift or lea) is cheaper than a negation and a bit-wise and.
>
> Previously the expression (x>>63)*y would be compiled with -O2 as
> shrq$63, %rdi
> movq%rdi, %rax
> imulq %rsi, %rax
>
> but with this patch now produces:
> sarq$63, %rdi
> movq%rdi, %rax
> andq%rsi, %rax
>
> Likewise the expression (x>>63)*135 [that appears in a hot-spot of the
> Botan AES-128 benchmark] was previously:
>
> shrq$63, %rdi
> leaq(%rdi,%rdi,8), %rdx
> movq%rdx, %rax
> salq$4, %rax
> subq%rdx, %rax
>
> now becomes:
> movq%rdi, %rax
> sarq$63, %rax
> andl$135, %eax
>
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> make -k check, both with and without --target_board=unix{-m32}, with no
> new failures. Many thanks to Uros for the speedy review and approval of
> my x86_rtx_costs patch that enables this transformation on -m32 using the
> correct cost of DImode multiplication. Ok for mainline?
Looks good to me
Thanks,
Richard.
> 2022-05-18 Roger Sayle
>
> gcc/ChangeLog
> PR middle-end/98865
> * expr.cc (expand_expr_real_2) [MULT_EXPR]: Expand X*Y as X
> when both X and Y are [0, 1], X*Y as X&-Y when Y is [0,1] and
> likewise X*Y as -X when X is [0,1] using tree_nonzero_bits.
>
> gcc/testsuite/ChangeLog
> PR middle-end/98865
> * gcc.target/i386/pr98865.c: New test case.
>
>
> Thanks in advance,
> Roger
> --
>
[PATCH 8/8] aarch64: update for move of FINAL and OVERRIDE
gcc/ChangeLog:
* config/aarch64/aarch64-sve-builtins-base.cc: Replace uses of
"FINAL" and "OVERRIDE" with "final" and "override".
* config/aarch64/aarch64-sve-builtins-functions.h: Likewise.
* config/aarch64/aarch64-sve-builtins-shapes.cc: Likewise.
* config/aarch64/aarch64-sve-builtins-sve2.cc: Likewise.
Signed-off-by: David Malcolm
---
.../aarch64/aarch64-sve-builtins-base.cc | 260 +--
.../aarch64/aarch64-sve-builtins-functions.h | 48 +-
.../aarch64/aarch64-sve-builtins-shapes.cc| 416 +-
.../aarch64/aarch64-sve-builtins-sve2.cc | 44 +-
4 files changed, 384 insertions(+), 384 deletions(-)
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
index c24c0548724..bee410929bd 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
@@ -148,7 +148,7 @@ class svabd_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* The integer operations are represented as the subtraction of the
minimum from the maximum, with the signedness of the instruction
@@ -179,7 +179,7 @@ public:
CONSTEXPR svac_impl (int unspec) : m_unspec (unspec) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
e.add_ptrue_hint (0, e.gp_mode (0));
insn_code icode = code_for_aarch64_pred_fac (m_unspec, e.vector_mode (0));
@@ -194,7 +194,7 @@ class svadda_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Put the predicate last, as required by mask_fold_left_plus_optab. */
e.rotate_inputs_left (0, 3);
@@ -211,7 +211,7 @@ public:
CONSTEXPR svadr_bhwd_impl (unsigned int shift) : m_shift (shift) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
machine_mode mode = GET_MODE (e.args[0]);
if (m_shift == 0)
@@ -231,7 +231,7 @@ class svbic_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Convert svbic of a constant into svand of its inverse. */
if (CONST_INT_P (e.args[2]))
@@ -261,7 +261,7 @@ public:
CONSTEXPR svbrk_binary_impl (int unspec) : m_unspec (unspec) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
return e.use_exact_insn (code_for_aarch64_brk (m_unspec));
}
@@ -277,7 +277,7 @@ public:
CONSTEXPR svbrk_unary_impl (int unspec) : m_unspec (unspec) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
return e.use_cond_insn (code_for_aarch64_brk (m_unspec));
}
@@ -290,7 +290,7 @@ class svcadd_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Convert the rotation amount into a specific unspec. */
int rot = INTVAL (e.args.pop ());
@@ -311,7 +311,7 @@ public:
CONSTEXPR svclast_impl (int unspec) : m_unspec (unspec) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Match the fold_extract_optab order. */
std::swap (e.args[0], e.args[1]);
@@ -332,7 +332,7 @@ class svcmla_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Convert the rotation amount into a specific unspec. */
int rot = INTVAL (e.args.pop ());
@@ -355,7 +355,7 @@ class svcmla_lane_impl : public function_base
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
/* Convert the rotation amount into a specific unspec. */
int rot = INTVAL (e.args.pop ());
@@ -384,7 +384,7 @@ public:
: m_code (code), m_unspec_for_fp (unspec_for_fp) {}
gimple *
- fold (gimple_folder ) const OVERRIDE
+ fold (gimple_folder ) const override
{
tree pg = gimple_call_arg (f.call, 0);
tree rhs1 = gimple_call_arg (f.call, 1);
@@ -406,7 +406,7 @@ public:
}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
machine_mode mode = e.vector_mode (0);
@@ -442,7 +442,7 @@ public:
m_unspec_for_uint (unspec_for_uint) {}
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
machine_mode mode = e.vector_mode (0);
bool unsigned_p = e.type_suffix (0).unsigned_p;
@@ -480,7 +480,7 @@ class svcmpuo_impl : public quiet
{
public:
rtx
- expand (function_expander ) const OVERRIDE
+ expand (function_expander ) const override
{
[PATCH 6/8] jit: update for move of FINAL and OVERRIDE
I can self-approve this one, assuming that patch 1 is approved.
gcc/jit/ChangeLog:
* jit-playback.h: Replace uses of "FINAL" and "OVERRIDE" with
"final" and "override".
* jit-recording.cc: Likewise.
* jit-recording.h: Likewise.
Signed-off-by: David Malcolm
---
gcc/jit/jit-playback.h | 12 +-
gcc/jit/jit-recording.cc | 2 +-
gcc/jit/jit-recording.h | 586 +++
3 files changed, 300 insertions(+), 300 deletions(-)
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index b18aa4d38da..3ba02a0451a 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -378,7 +378,7 @@ class compile_to_memory : public context
{
public:
compile_to_memory (recording::context *ctxt);
- void postprocess (const char *ctxt_progname) FINAL OVERRIDE;
+ void postprocess (const char *ctxt_progname) final override;
result *get_result_obj () const { return m_result; }
@@ -392,7 +392,7 @@ class compile_to_file : public context
compile_to_file (recording::context *ctxt,
enum gcc_jit_output_kind output_kind,
const char *output_path);
- void postprocess (const char *ctxt_progname) FINAL OVERRIDE;
+ void postprocess (const char *ctxt_progname) final override;
private:
void
@@ -483,7 +483,7 @@ public:
function(context *ctxt, tree fndecl, enum gcc_jit_function_kind kind);
void gt_ggc_mx ();
- void finalizer () FINAL OVERRIDE;
+ void finalizer () final override;
tree get_return_type_as_tree () const;
@@ -562,7 +562,7 @@ public:
block (function *func,
const char *name);
- void finalizer () FINAL OVERRIDE;
+ void finalizer () final override;
tree as_label_decl () const { return m_label_decl; }
@@ -750,7 +750,7 @@ class source_file : public wrapper
{
public:
source_file (tree filename);
- void finalizer () FINAL OVERRIDE;
+ void finalizer () final override;
source_line *
get_source_line (int line_num);
@@ -771,7 +771,7 @@ class source_line : public wrapper
{
public:
source_line (source_file *file, int line_num);
- void finalizer () FINAL OVERRIDE;
+ void finalizer () final override;
location *
get_location (recording::location *rloc, int column_num);
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index a31720f043e..2eaba825bcc 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -3743,7 +3743,7 @@ class rvalue_usage_validator : public
recording::rvalue_visitor
recording::statement *stmt);
void
- visit (recording::rvalue *rvalue) FINAL OVERRIDE;
+ visit (recording::rvalue *rvalue) final override;
private:
const char *m_api_funcname;
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 78ee36488c7..2dd8a957ca7 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -441,11 +441,11 @@ public:
static string * from_printf (context *ctxt, const char *fmt, ...)
GNU_PRINTF(2, 3);
- void replay_into (replayer *) FINAL OVERRIDE {}
+ void replay_into (replayer *) final override {}
private:
- string * make_debug_string () FINAL OVERRIDE;
- void write_reproducer (reproducer ) FINAL OVERRIDE;
+ string * make_debug_string () final override;
+ void write_reproducer (reproducer ) final override;
private:
size_t m_len;
@@ -469,7 +469,7 @@ public:
m_created_by_user (created_by_user)
{}
- void replay_into (replayer *r) FINAL OVERRIDE;
+ void replay_into (replayer *r) final override;
playback::location *
playback_location (replayer *r)
@@ -498,12 +498,12 @@ public:
return static_cast (m_playback_obj);
}
- location *dyn_cast_location () FINAL OVERRIDE { return this; }
+ location *dyn_cast_location () final override { return this; }
bool created_by_user () const { return m_created_by_user; }
private:
- string * make_debug_string () FINAL OVERRIDE;
- void write_reproducer (reproducer ) FINAL OVERRIDE;
+ string * make_debug_string () final override;
+ void write_reproducer (reproducer ) final override;
private:
string *m_filename;
@@ -603,11 +603,11 @@ public:
: type (ctxt),
m_kind (kind) {}
- type *dereference () FINAL OVERRIDE;
+ type *dereference () final override;
- size_t get_size () FINAL OVERRIDE;
+ size_t get_size () final override;
- bool accepts_writes_from (type *rtype) FINAL OVERRIDE
+ bool accepts_writes_from (type *rtype) final override
{
if (m_kind == GCC_JIT_TYPE_VOID_PTR)
{
@@ -629,20 +629,20 @@ public:
return type::accepts_writes_from (rtype);
}
- bool is_int () const FINAL OVERRIDE;
- bool is_float () const FINAL OVERRIDE;
- bool is_bool () const FINAL OVERRIDE;
- type *is_pointer () FINAL OVERRIDE { return dereference (); }
- type *is_array () FINAL OVERRIDE { return NULL; }
- bool is_void () const FINAL OVERRIDE { return m_kind == GCC_JIT_TYPE_VOID; }
- bool is_signed () const FINAL OVERRIDE;
+
[PATCH 3/8] analyzer: update for move of FINAL and OVERRIDE
I can self-approve this one, assuming that patch 1 is approved.
gcc/analyzer/ChangeLog:
* analyzer-pass.cc: Replace uses of "FINAL" and "OVERRIDE" with
"final" and "override".
* call-info.h: Likewise.
* checker-path.h: Likewise.
* constraint-manager.cc: Likewise.
* diagnostic-manager.cc: Likewise.
* engine.cc: Likewise.
* exploded-graph.h: Likewise.
* feasible-graph.h: Likewise.
* pending-diagnostic.h: Likewise.
* region-model-impl-calls.cc: Likewise.
* region-model.cc: Likewise.
* region-model.h: Likewise.
* region.h: Likewise.
* sm-file.cc: Likewise.
* sm-malloc.cc: Likewise.
* sm-pattern-test.cc: Likewise.
* sm-sensitive.cc: Likewise.
* sm-signal.cc: Likewise.
* sm-taint.cc: Likewise.
* state-purge.h: Likewise.
* store.cc: Likewise.
* store.h: Likewise.
* supergraph.h: Likewise.
* svalue.h: Likewise.
* trimmed-graph.h: Likewise.
* varargs.cc: Likewise.
Signed-off-by: David Malcolm
---
gcc/analyzer/analyzer-pass.cc | 4 +-
gcc/analyzer/call-info.h| 8 +-
gcc/analyzer/checker-path.h | 52 +++
gcc/analyzer/constraint-manager.cc | 4 +-
gcc/analyzer/diagnostic-manager.cc | 36 ++---
gcc/analyzer/engine.cc | 90 ++--
gcc/analyzer/exploded-graph.h | 52 +++
gcc/analyzer/feasible-graph.h | 6 +-
gcc/analyzer/pending-diagnostic.h | 4 +-
gcc/analyzer/region-model-impl-calls.cc | 14 +-
gcc/analyzer/region-model.cc| 54 +++
gcc/analyzer/region-model.h | 84 +--
gcc/analyzer/region.h | 142 +--
gcc/analyzer/sm-file.cc | 36 ++---
gcc/analyzer/sm-malloc.cc | 130 -
gcc/analyzer/sm-pattern-test.cc | 14 +-
gcc/analyzer/sm-sensitive.cc| 20 +--
gcc/analyzer/sm-signal.cc | 24 ++--
gcc/analyzer/sm-taint.cc| 60
gcc/analyzer/state-purge.h | 4 +-
gcc/analyzer/store.cc | 2 +-
gcc/analyzer/store.h| 12 +-
gcc/analyzer/supergraph.h | 26 ++--
gcc/analyzer/svalue.h | 178
gcc/analyzer/trimmed-graph.h| 4 +-
gcc/analyzer/varargs.cc | 54 +++
26 files changed, 557 insertions(+), 557 deletions(-)
diff --git a/gcc/analyzer/analyzer-pass.cc b/gcc/analyzer/analyzer-pass.cc
index 0bf131bb40a..f6cef58bc7c 100644
--- a/gcc/analyzer/analyzer-pass.cc
+++ b/gcc/analyzer/analyzer-pass.cc
@@ -66,8 +66,8 @@ public:
{}
/* opt_pass methods: */
- bool gate (function *) FINAL OVERRIDE;
- unsigned int execute (function *) FINAL OVERRIDE;
+ bool gate (function *) final override;
+ unsigned int execute (function *) final override;
}; // class pass_analyzer
/* Only run the analyzer if -fanalyzer. */
diff --git a/gcc/analyzer/call-info.h b/gcc/analyzer/call-info.h
index bdcfff79571..4bb7dd7e198 100644
--- a/gcc/analyzer/call-info.h
+++ b/gcc/analyzer/call-info.h
@@ -30,9 +30,9 @@ namespace ana {
class call_info : public custom_edge_info
{
public:
- void print (pretty_printer *pp) const FINAL OVERRIDE;
+ void print (pretty_printer *pp) const final override;
void add_events_to_path (checker_path *emission_path,
- const exploded_edge ) const FINAL OVERRIDE;
+ const exploded_edge ) const final override;
const gcall *get_call_stmt () const { return m_call_stmt; }
tree get_fndecl () const { return m_fndecl; }
@@ -58,7 +58,7 @@ private:
class success_call_info : public call_info
{
public:
- label_text get_desc (bool can_colorize) const FINAL OVERRIDE;
+ label_text get_desc (bool can_colorize) const final override;
protected:
success_call_info (const call_details ) : call_info (cd) {}
@@ -72,7 +72,7 @@ protected:
class failed_call_info : public call_info
{
public:
- label_text get_desc (bool can_colorize) const FINAL OVERRIDE;
+ label_text get_desc (bool can_colorize) const final override;
protected:
failed_call_info (const call_details ) : call_info (cd) {}
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index 545d7db06a2..fd274e52ac3 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -91,9 +91,9 @@ public:
/* Implementation of diagnostic_event. */
- location_t get_location () const FINAL OVERRIDE { return m_loc; }
- tree get_fndecl () const FINAL OVERRIDE { return m_fndecl; }
- int get_stack_depth () const FINAL OVERRIDE { return m_depth; }
+ location_t get_location () const final override { return m_loc; }
+ tree get_fndecl () const final override { return m_fndecl; }
+ int get_stack_depth () const final override { return
[PATCH 7/8] gcc: update for move of FINAL and OVERRIDE
gcc/ChangeLog:
* diagnostic-path.h: Replace uses of "FINAL" and "OVERRIDE" with
"final" and "override".
* digraph.cc: Likewise.
* gcc-rich-location.h: Likewise.
* gimple-array-bounds.cc: Likewise.
* gimple-loop-versioning.cc: Likewise.
* gimple-range-cache.cc: Likewise.
* gimple-range-cache.h: Likewise.
* gimple-range-fold.cc: Likewise.
* gimple-range-fold.h: Likewise.
* gimple-range-tests.cc: Likewise.
* gimple-range.h: Likewise.
* gimple-ssa-evrp.cc: Likewise.
* input.cc: Likewise.
* json.h: Likewise.
* read-rtl-function.cc: Likewise.
* tree-complex.cc: Likewise.
* tree-diagnostic-path.cc: Likewise.
* tree-ssa-ccp.cc: Likewise.
* tree-ssa-copy.cc: Likewise.
* tree-vrp.cc: Likewise.
* value-query.h: Likewise.
* vr-values.h: Likewise.
Signed-off-by: David Malcolm
---
gcc/diagnostic-path.h | 12 ++--
gcc/digraph.cc| 4 ++--
gcc/gcc-rich-location.h | 6 +++---
gcc/gimple-array-bounds.cc| 2 +-
gcc/gimple-loop-versioning.cc | 4 ++--
gcc/gimple-range-cache.cc | 12 ++--
gcc/gimple-range-cache.h | 2 +-
gcc/gimple-range-fold.cc | 8
gcc/gimple-range-fold.h | 10 +-
gcc/gimple-range-tests.cc | 2 +-
gcc/gimple-range.h| 8
gcc/gimple-ssa-evrp.cc| 26 +-
gcc/input.cc | 4 ++--
gcc/json.h| 24
gcc/read-rtl-function.cc | 10 +-
gcc/tree-complex.cc | 4 ++--
gcc/tree-diagnostic-path.cc | 2 +-
gcc/tree-ssa-ccp.cc | 8
gcc/tree-ssa-copy.cc | 6 +++---
gcc/tree-vrp.cc | 22 +++---
gcc/value-query.h | 8
gcc/vr-values.h | 12 ++--
22 files changed, 98 insertions(+), 98 deletions(-)
diff --git a/gcc/diagnostic-path.h b/gcc/diagnostic-path.h
index 3bd89d65a73..6c1190d6f2d 100644
--- a/gcc/diagnostic-path.h
+++ b/gcc/diagnostic-path.h
@@ -106,10 +106,10 @@ class simple_diagnostic_event : public diagnostic_event
const char *desc);
~simple_diagnostic_event ();
- location_t get_location () const FINAL OVERRIDE { return m_loc; }
- tree get_fndecl () const FINAL OVERRIDE { return m_fndecl; }
- int get_stack_depth () const FINAL OVERRIDE { return m_depth; }
- label_text get_desc (bool) const FINAL OVERRIDE
+ location_t get_location () const final override { return m_loc; }
+ tree get_fndecl () const final override { return m_fndecl; }
+ int get_stack_depth () const final override { return m_depth; }
+ label_text get_desc (bool) const final override
{
return label_text::borrow (m_desc);
}
@@ -130,8 +130,8 @@ class simple_diagnostic_path : public diagnostic_path
simple_diagnostic_path (pretty_printer *event_pp)
: m_event_pp (event_pp) {}
- unsigned num_events () const FINAL OVERRIDE;
- const diagnostic_event & get_event (int idx) const FINAL OVERRIDE;
+ unsigned num_events () const final override;
+ const diagnostic_event & get_event (int idx) const final override;
diagnostic_event_id_t add_event (location_t loc, tree fndecl, int depth,
const char *fmt, ...)
diff --git a/gcc/digraph.cc b/gcc/digraph.cc
index 06fd70b89cc..6a1cb1bfe44 100644
--- a/gcc/digraph.cc
+++ b/gcc/digraph.cc
@@ -51,7 +51,7 @@ struct test_graph_traits
struct test_node : public dnode
{
test_node (const char *name, int index) : m_name (name), m_index (index) {}
- void dump_dot (graphviz_out *, const dump_args_t &) const OVERRIDE
+ void dump_dot (graphviz_out *, const dump_args_t &) const override
{
}
@@ -65,7 +65,7 @@ struct test_edge : public dedge
: dedge (src, dest)
{}
- void dump_dot (graphviz_out *gv, const dump_args_t &) const OVERRIDE
+ void dump_dot (graphviz_out *gv, const dump_args_t &) const override
{
gv->println ("%s %s %s%c", m_src->m_name, "->", m_dest->m_name, ';');
}
diff --git a/gcc/gcc-rich-location.h b/gcc/gcc-rich-location.h
index a43fe3899b5..789ce0eed3a 100644
--- a/gcc/gcc-rich-location.h
+++ b/gcc/gcc-rich-location.h
@@ -113,7 +113,7 @@ class text_range_label : public range_label
public:
text_range_label (const char *text) : m_text (text) {}
- label_text get_text (unsigned /*range_idx*/) const FINAL OVERRIDE
+ label_text get_text (unsigned /*range_idx*/) const final override
{
return label_text::borrow (m_text);
}
@@ -159,7 +159,7 @@ class range_label_for_type_mismatch : public range_label
{
}
- label_text get_text (unsigned range_idx) const OVERRIDE;
+ label_text get_text (unsigned range_idx) const override;
protected:
tree m_labelled_type;
@@ -178,7 +178,7 @@ class maybe_range_label_for_tree_type_mismatch : public
range_label
{
}
[PATCH 5/8] c++: update for move of FINAL and OVERRIDE
gcc/cp/ChangeLog:
* cxx-pretty-print.h: Replace uses of "FINAL" and "OVERRIDE" with
"final" and "override".
* error.cc: Likewise.
Signed-off-by: David Malcolm
---
gcc/cp/cxx-pretty-print.h | 2 +-
gcc/cp/error.cc | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 920af649ceb..5080f70a8e4 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -34,7 +34,7 @@ class cxx_pretty_printer : public c_pretty_printer
public:
cxx_pretty_printer ();
- pretty_printer *clone () const OVERRIDE;
+ pretty_printer *clone () const override;
void constant (tree);
void id_expression (tree);
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 250e012c008..94181e76d0e 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -144,12 +144,12 @@ class cxx_format_postprocessor : public
format_postprocessor
: m_type_a (), m_type_b ()
{}
- format_postprocessor *clone() const FINAL OVERRIDE
+ format_postprocessor *clone() const final override
{
return new cxx_format_postprocessor ();
}
- void handle (pretty_printer *pp) FINAL OVERRIDE;
+ void handle (pretty_printer *pp) final override;
deferred_printed_type m_type_a;
deferred_printed_type m_type_b;
--
2.26.3
[PATCH 4/8] c-family: update for move of FINAL and OVERRIDE
gcc/c-family/ChangeLog:
* c-format.cc: Replace uses of "FINAL" and "OVERRIDE" with "final"
and "override".
* c-pretty-print.h: Likewise.
Signed-off-by: David Malcolm
---
gcc/c-family/c-format.cc | 2 +-
gcc/c-family/c-pretty-print.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/c-family/c-format.cc b/gcc/c-family/c-format.cc
index ea57fde801c..0a899375d4f 100644
--- a/gcc/c-family/c-format.cc
+++ b/gcc/c-family/c-format.cc
@@ -4613,7 +4613,7 @@ class range_label_for_format_type_mismatch
{
}
- label_text get_text (unsigned range_idx) const FINAL OVERRIDE
+ label_text get_text (unsigned range_idx) const final override
{
label_text text = range_label_for_type_mismatch::get_text (range_idx);
if (text.m_buffer == NULL)
diff --git a/gcc/c-family/c-pretty-print.h b/gcc/c-family/c-pretty-print.h
index ba7624dab1f..be86bed4fee 100644
--- a/gcc/c-family/c-pretty-print.h
+++ b/gcc/c-family/c-pretty-print.h
@@ -51,7 +51,7 @@ class c_pretty_printer : public pretty_printer
{
public:
c_pretty_printer ();
- pretty_printer *clone () const OVERRIDE;
+ pretty_printer *clone () const override;
// Format string, possibly translated.
void translate_string (const char *);
--
2.26.3
[PATCH 1/8] Move FINAL and OVERRIDE from ansidecl.h to gcc-plugin.h
As of GCC 11 onwards we have required a C++11 compiler, such as GCC 4.8 or later. On the assumption that any such compiler correctly implements "final" and "override", this patch removes the FINAL and OVERRIDE macros from ansidecl.h. It adds them back to gcc-plugins.h in case any plugins make use of them. Followup patches within this kit replace all uses of the macros in the tree with the lower-case versions; I've split them up for ease of review, but would apply them as one combined commit if approved. gcc/ChangeLog: * gcc-plugin.h: Add macros OVERRIDE and FINAL. include/ChangeLog: * ansidecl.h: Drop macros OVERRIDE and FINAL. Signed-off-by: David Malcolm --- gcc/gcc-plugin.h | 7 +++ include/ansidecl.h | 41 - 2 files changed, 7 insertions(+), 41 deletions(-) diff --git a/gcc/gcc-plugin.h b/gcc/gcc-plugin.h index ee9aa865114..f0f3049d0ce 100644 --- a/gcc/gcc-plugin.h +++ b/gcc/gcc-plugin.h @@ -44,4 +44,11 @@ along with GCC; see the file COPYING3. If not see #include "tree-check.h" #include "plugin.h" +/* Versions of GCC prior to GCC 13 defined these macros in ansidecl.h + for C++98 compatibility. Given that we require C++11 from GCC 11 onwards, + keep definitions here in case plugins made use of them. */ + +#define OVERRIDE override +#define FINALfinal + #endif /* GCC_PLUGIN_H */ diff --git a/include/ansidecl.h b/include/ansidecl.h index efee5b6904b..119748deb68 100644 --- a/include/ansidecl.h +++ b/include/ansidecl.h @@ -345,47 +345,6 @@ So instead we use the macro below and test it against specific values. */ #define CONSTEXPR #endif -/* C++11 adds the ability to add "override" after an implementation of a - virtual function in a subclass, to: - (A) document that this is an override of a virtual function - (B) allow the compiler to issue a warning if it isn't (e.g. a mismatch - of the type signature). - - Similarly, it allows us to add a "final" to indicate that no subclass - may subsequently override the vfunc. - - Provide OVERRIDE and FINAL as macros, allowing us to get these benefits - when compiling with C++11 support, but without requiring C++11. - - For gcc, use "-std=c++11" to enable C++11 support; gcc 6 onwards enables - this by default (actually GNU++14). */ - -#if defined __cplusplus -# if __cplusplus >= 201103 - /* C++11 claims to be available: use it. Final/override were only - implemented in 4.7, though. */ -# if GCC_VERSION < 4007 -# define OVERRIDE -# define FINAL -# else -# define OVERRIDE override -# define FINAL final -# endif -# elif GCC_VERSION >= 4007 - /* G++ 4.7 supports __final in C++98. */ -# define OVERRIDE -# define FINAL __final -# else - /* No C++11 support; leave the macros empty. */ -# define OVERRIDE -# define FINAL -# endif -#else - /* No C++11 support; leave the macros empty. */ -# define OVERRIDE -# define FINAL -#endif - /* A macro to disable the copy constructor and assignment operator. When building with C++11 and above, the methods are explicitly deleted, causing a compile-time error if something tries to copy. -- 2.26.3
[PATCH 2/8] libcpp: update for move of FINAL And OVERRIDE
I can self-approve this one, assuming that patch 1 is approved.
libcpp/ChangeLog:
* lex.cc: Replace uses of "FINAL" and "OVERRIDE" with "final" and
"override".
Signed-off-by: David Malcolm
---
libcpp/lex.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcpp/lex.cc b/libcpp/lex.cc
index fb1dfabb7af..f891d3e17df 100644
--- a/libcpp/lex.cc
+++ b/libcpp/lex.cc
@@ -1523,7 +1523,7 @@ class unpaired_bidi_rich_location : public rich_location
class custom_range_label : public range_label
{
public:
- label_text get_text (unsigned range_idx) const FINAL OVERRIDE
+ label_text get_text (unsigned range_idx) const final override
{
/* range 0 is the primary location; each subsequent range i + 1
is for bidi::vec[i]. */
--
2.26.3
Re: [wwwdocs] OpenMP - minor update/fixes for gomp + gcc-13
On Thu, May 19, 2022 at 02:13:03PM +0200, Tobias Burnus wrote: > Hi Jakub, hi all, > > the main reason for this patch is to change Documentation from to > in projects/gomp/ as I realized that those cross references weren't as > prominently visible as I had hoped for. > > Additionally: > * gcc-13/changes.html: Add a feature I had missed (non-rect loop nest in F90) > + new features committed after the last change. > and in projects/gomp/ > * Fix a mis-placed > * Mark two additional items as GCC-13 implemented. > > OK? Comments? Ok. Jakub
[wwwdocs] OpenMP - minor update/fixes for gomp + gcc-13
Hi Jakub, hi all, the main reason for this patch is to change Documentation from to in projects/gomp/ as I realized that those cross references weren't as prominently visible as I had hoped for. Additionally: * gcc-13/changes.html: Add a feature I had missed (non-rect loop nest in F90) + new features committed after the last change. and in projects/gomp/ * Fix a mis-placed * Mark two additional items as GCC-13 implemented. OK? Comments? Tobias - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 OpenMP - minor update/fixes for gomp + gcc-13 * gcc-13/changes.html (OpenMP): Update. * projects/gomp/: Make Documentation a ''; fix location; mark two features as GCC-13 supported. diff --git a/htdocs/gcc-13/changes.html b/htdocs/gcc-13/changes.html index 969fb00b..2d974ae5 100644 --- a/htdocs/gcc-13/changes.html +++ b/htdocs/gcc-13/changes.html @@ -41,9 +41,12 @@ a work-in-progress. https://gcc.gnu.org/projects/gomp/;>OpenMP The following OpenMP 5.1 features have been added: the -omp_all_memory reserved locator and the -omp_target_is_accessible and omp_get_mapped_ptr -API routines. +omp_all_memory reserved locator, the inoutset +modifier to the depend clause and the +omp_target_is_accessible, omp_target_memcpy_async, +omp_target_memcpy_rect_async and +omp_get_mapped_ptr API routines. Fortran now supports +non-rectangular loop nests, which were added for C/C++ in GCC 11. diff --git a/htdocs/projects/gomp/index.html b/htdocs/projects/gomp/index.html index c282b4f5..68531a57 100644 --- a/htdocs/projects/gomp/index.html +++ b/htdocs/projects/gomp/index.html @@ -20,6 +20,7 @@ OpenMP and OpenACC are supported with GCC's C, C++ and Fortran compilers. Content Usage + Documentation History and Project Goal Contributing Reporting Bugs @@ -66,7 +67,7 @@ OpenMP and OpenACC are supported with GCC's C, C++ and Fortran compilers. details. -Documentation +Documentation https://gcc.gnu.org/onlinedocs/libgomp/;>GNU libgomp manual for API routines, environment variables and implementation details. @@ -550,7 +551,7 @@ than listed, depending on resolved corner cases and optimizations. -seq_cst clause on a flush construct +seq_cst clause on a flush construct GCC12 @@ -604,11 +605,21 @@ than listed, depending on resolved corner cases and optimizations. GCC12 + +inoutset argument to the depend clause +GCC13 + + omp_target_is_accessible runtime routine GCC13 + +omp_target_memcpy_async and omp_target_memcpy_rect_async runtime routines +GCC13 + + omp_get_mapped_ptr runtime routine GCC13 @@ -684,21 +695,11 @@ than listed, depending on resolved corner cases and optimizations. No - -inoutset argument to the depend clause -No - - present argument to defaultmap clause No - -omp_target_memcpy_async and omp_target_memcpy_rect_async runtime routines -No - - ompt_scope_endpoint_t enum: ompt_scope_beginend No
Re: [PATCH] middle-end/105604 - snprintf dianostics and non-constant sizes/offsets
On Wed, 18 May 2022, Martin Sebor wrote:
> On 5/18/22 00:26, Richard Biener wrote:
> > On Tue, 17 May 2022, Martin Sebor wrote:
> >
> >> On 5/16/22 03:16, Richard Biener wrote:
> >>> The following tries to correct get_origin_and_offset_r not handling
> >>> non-constant sizes of array elements in ARRAY_REFs and non-constant
> >>> offsets of COMPONENT_REFs. It isn't exactly clear how such failures
> >>> should be treated in this API and existing handling isn't consistent
> >>> here either. The following applies two different variants, treating
> >>> non-constant array sizes like non-constant array indices and
> >>> treating non-constant offsets of COMPONENT_REFs by terminating
> >>> the recursion (not sure what that means to the callers).
> >>>
> >>> Basically the code failed to use component_ref_field_offset and
> >>> array_ref_element_size and instead relies on inappropriate
> >>> helpers (that shouldn't exist in the first place ...). The code
> >>> is also not safe-guarded against overflows in the final offset/size
> >>> computations but I'm not trying to rectify that.
> >>>
> >>> Martin - can you comment on how the API should handle such
> >>> situations?
> >>
> >> It looks like the -Wrestrict warning here ignores offsets equal to
> >> HOST_WIDE_INT_MIN so presumably setting dst_offset (via *fldoff) to
> >> that should avoid it. Or maybe to HWI_MAX as it does for variable
> >> offsets.
> >
> > Can you suggest wording for the function comment as to how it handles
> > the case when offset or size cannot be determined exactly? The
> > comment currently only suggests that the caller possibly cannot
> > trust fldsize or off when the function returns NULL but the actual
> > implementation differs from that.
>
>
>
> >
> >> It also looks like the function only handles constant offsets and
> >> sizes, and I have a vague recollection of enhancing it to work with
> >> ranges. That should avoid the overflow problem too.
> >
> > So the correct thing is to return NULL?
>
> No, I don't think so. The recursive get_origin_and_offset_r() assumes
> its own invocations never return null (the one place it does that should
> probably be moved to the nonrecursive caller).
>
> >
> > Is the patch OK as-is?
>
> It's an improvement but it's not complete as the following also ICEs
> (albeit somewhere else):
>
> void* f (void);
>
> void g (int n)
> {
> struct {
> char a[n], b[];
> } *p = f ();
>
> __builtin_sprintf (p->b, "%s", p->a);
> }
>
> With the ICE fixed the warning triggers. That's not ideal but it's
> unavoidable given the IR (I believe I mentioned this caveat some time
> back). This is the same as for:
>
> struct {
> char a[8], b[8];
> } *p = f ();
>
> __builtin_sprintf (>b[n], "%s", p->a);
>
> because the IR looks more or less the same for >a[n] as it is for
> >b[n].
>
> > As said, I'm not sure how the caller interprets
> > the result and how it can distinguish the exact vs. non-exact cases
> > or what a "conservative" inexact answer would be.
>
> The warning triggers in both the certain cases and the inexact
> ones like the one above when an overlap cannot be ruled out. To
> differentiate the two it's phrased as "may overlap". The handling
> is in maybe_warn_overlap().
>
> >
> > Please help properly documenting this API.
>
> I can spend some time in the next few days to page it all in, see
> if I can clean it up a bit in addition to fixing the ICEs and
> improve the comment. Let me know if you have a different
> preference.
That works for me - thanks for taking it from here.
Richard.
Patch structure and timing for re-adding gcj
Over the past few months I have been working on re-adding gcj to gcc ( https://github.com/Zopolis4/gcj/tree/mster). It is now at the point where all of the code in gcc/java and the related additions to other gcc files compiles successfully, although libjava still breaks. Given that I do not know how long it will be before it compiles successfully, nor the remaining issues preventing it from compiling it successfully, I was wondering if it would make sense to submit the gcc/java (and the related additions) code now, as to have it receive treewide changes rather than mysteriously breaking when I rebase and further slowing down development. This would also allow me to test files in libjava with the system gcc, among other things, as they make use of several types declared in gcc/java. Regardless of this, when all the code works, how should I submit it? My current plan is to have an initial commit re-adding the gcc/java and libjava directories in the exact state that they were removed, as so to have a clean history, and also adding the java-related code in other gcc files in a modern state. Then, a second commit modernising the code in gcc/java and libjava.
Re: [0/9] [middle-end] Add param to vec_perm_const hook to specify mode of input operand
On Wed, May 18, 2022 at 9:08 AM Prathamesh Kulkarni via Gcc-patches wrote: > > Hi, > The attached patch adds another parameter machine_mode op_mode to > vec_perm_const > hook to specify mode of input operands. The motivation for doing this > is PR96463, > where we create vec_perm_expr of the form: > lhs = vec_perm_expr > where lhs and rhs have different vector types but same element type > (lhs is SVE and rhs is corresponding advsimd vector). > > It seems the following targets were affected: aarch64, i386, arm, ia64, > mips, rs6000, s390, sparc, gcn. > > Bootstrapped+tested on x86_64-linux-gnu, aarch64-linux-gnu. > For other targets, I did make all-gcc stage1, which seems to build OK. So it would also allow to express extract even/odd and interleave operations with a VEC_PERM. The interleave currently has the issue that we have to artificially widen the inputs with "dont-care" elements. The lo/high variants can already do with a bitfield-ref or vector-of-vector CTOR. Richard. > > Thanks, > Prathamesh
Re: [PATCH] cfgexpand: Yet another spot with debug insns references to global vars without varpool nodes [PR105630]
On Thu, 19 May 2022, Jakub Jelinek wrote:
> Hi!
>
> This is similar to the earlier patch to avoid having MEM_EXPRs
> referencing global vars without varpool nodes, but this time
> the difference is that during gimplification some hashing
> actually created DECL_RTLs for the n VAR_DECL and the previous
> change was in the if above this when DECL_RTL is NULL and we are
> considering creating it.
>
> The following patch drops on the floor references to vars where
> we've optimized away the varpool node even when it has DECL_RTL.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, plus
> bootstrapped on those without the cfgexpand.cc change, reapplied
> it and rebuilt stage3 cc1/cc1plus, the resulting cc1/cc1plus
> binaries on both targets were identical except for the 16-byte
> executable_checksum (I've done the second bootstraps in the same
> directory as the first one after moving the previous one elsewhere,
> so pathnames were the same, just checksum hasn't been regenerated).
> So, at least on those binaries this patch doesn't affect debug info
> at all.
>
> Ok for trunk?
OK.
Thanks,
Richard.
> 2022-05-19 Jakub Jelinek
>
> PR debug/105630
> * cfgexpand.cc (expand_debug_expr): For VAR_DECL, punt for
> global vars without symtab node even when they have DECL_RTL
> set.
>
> * gcc.dg/pr105630.c: New test.
>
> --- gcc/cfgexpand.cc.jj 2022-05-09 09:09:20.005477502 +0200
> +++ gcc/cfgexpand.cc 2022-05-18 13:53:49.622983222 +0200
> @@ -4575,6 +4575,10 @@ expand_debug_expr (tree exp)
> || SYMBOL_REF_DECL (XEXP (op0, 0)) != exp)
> return NULL;
> }
> + else if (VAR_P (exp)
> +&& is_global_var (exp)
> +&& symtab_node::get (exp) == NULL)
> + return NULL;
>else
> op0 = copy_rtx (op0);
>
> --- gcc/testsuite/gcc.dg/pr105630.c.jj2022-05-18 14:02:18.426050242
> +0200
> +++ gcc/testsuite/gcc.dg/pr105630.c 2022-05-18 14:02:07.103204525 +0200
> @@ -0,0 +1,22 @@
> +/* PR debug/105630 */
> +/* { dg-do compile { target pthread } } */
> +/* { dg-options "-O1 -ftree-parallelize-loops=2 -fcompare-debug" } */
> +
> +int m;
> +static int n;
> +
> +void
> +foo (void)
> +{
> + int *arr[] = { , , };
> + int unused = n;
> +
> + m = 0;
> +}
> +
> +void
> +bar (int *arr, int i)
> +{
> + while (i < 1)
> +arr[i++] = 1;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
Re: [PATCH] pointer-query: Fix ICE with non-pointer param [PR105635]
On Thu, 19 May 2022, Jakub Jelinek wrote:
> Hi!
>
> The gimple_parm_array_size function comment talks about pointe parameters
> but doesn't actually verify it, it checks whether an attribute is present
> on the function and then just uses TREE_TYPE (TREE_TYPE (var)) which
> assumes a pointer type (or in theory could work for ARRAY_TYPE but
> c-family languages which only have that attribute will never have ARRAY_TYPE
> parameters; and for VECTOR_TYPE/COMPLEX_TYPE it would mean something quite
> different).
>
> So, this patch punts early if var doesn't have pointer/reference type.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2022-05-19 Jakub Jelinek
>
> PR c/105635
> * poiinter-query.cc (gimple_parm_array_size): Return NULL if var
> doesn't have pointer or reference type.
>
> * gcc.dg/pr105635.c: New test.
>
> --- gcc/pointer-query.cc.jj 2022-05-13 09:34:23.996147311 +0200
> +++ gcc/pointer-query.cc 2022-05-18 12:12:21.398849571 +0200
> @@ -555,7 +555,7 @@ gimple_parm_array_size (tree ptr, wide_i
> from the current function declaratation (e.g., attribute access or
> related). */
>tree var = SSA_NAME_VAR (ptr);
> - if (TREE_CODE (var) != PARM_DECL)
> + if (TREE_CODE (var) != PARM_DECL || !POINTER_TYPE_P (TREE_TYPE (var)))
> return NULL_TREE;
>
>const unsigned prec = TYPE_PRECISION (sizetype);
> --- gcc/testsuite/gcc.dg/pr105635.c.jj2022-05-18 12:23:56.868371600
> +0200
> +++ gcc/testsuite/gcc.dg/pr105635.c 2022-05-18 12:23:41.728577929 +0200
> @@ -0,0 +1,11 @@
> +/* PR c/105635 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall" } */
> +
> +void foo (int, int[*]); /* { dg-message "previous declaration of 'foo'
> with type" } */
> +
> +foo (int x, int y) /* { dg-warning "return type defaults to 'int'" } */
> +{/* { dg-warning "conflicting types for 'foo'" "" {
> target *-*-* } .-1 } */
> + /* { dg-message "declared here" "" { target *-*-* } .-2
> } */
> + return (x >= 0) != (y < 0);/* { dg-warning "'return' with a value,
> in function returning void" } */
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
Re: [PATCH] Modula-2: merge proposal/review: 7/9 07.patch-set-05
Joseph Myers writes:
> On Wed, 18 May 2022, Gaius Mulley via Gcc-patches wrote:
>
>> /* This is the contribution to the `documented_lang_options' array in
>>toplev.c for gm2. */
>
> I'm not sure what this is (an unused file?), but documented_lang_options
> was removed in 2003. And in general, comments referring to .c files,
> throughout the front end, need to be updated to refer to the current names
> of those files (C++ sources have been renamed from .c to .cc in GCC).
ah yes - many thanks for spotting this - I will git rm lang-options.h.
Also thanks for the suggestion to check for other comments mentioning .c
files. I've found about 4 in gcc/m2/gm2-gcc/*.{h,cc} - which I'll
change immediately,
regards,
Gaius
Re: [PATCH] OpenMP, libgomp: Add new runtime routines omp_target_memcpy_async and omp_target_memcpy_rect_async
On Thu, May 19, 2022 at 10:39:05AM +0200, Marcel Vollweiler wrote:
> > add here
> >else
> > {
> >depend[0] = 0;
> > ...
> > }
>
> Added the "depend" definition to the "if" branch (instead the "else" branch).
Thanks for correcting my thinko.
> gcc/ChangeLog:
>
> * omp-low.cc (omp_runtime_api_call): Added target_memcpy_async and
> target_memcpy_rect_async to omp_runtime_apis array.
>
> libgomp/ChangeLog:
>
> * libgomp.map: Added omp_target_memcpy_async and
> omp_target_memcpy_rect_async.
> * libgomp.texi: Both functions are now supported.
> * omp.h.in: Added omp_target_memcpy_async and
> omp_target_memcpy_rect_async.
> * omp_lib.f90.in: Added interfaces for both new functions.
> * omp_lib.h.in: Likewise.
> * target.c (ialias_redirect): Added for GOMP_task.
> (omp_target_memcpy): Restructured into check and copy part.
> (omp_target_memcpy_check): New helper function for omp_target_memcpy and
> omp_target_memcpy_async that checks requirements.
> (omp_target_memcpy_copy): New helper function for omp_target_memcpy and
> omp_target_memcpy_async that performs the memcpy.
> (omp_target_memcpy_async_helper): New helper function that is used in
> omp_target_memcpy_async for the asynchronous task.
> (omp_target_memcpy_async): Added.
> (omp_target_memcpy_rect): Restructured into check and copy part.
> (omp_target_memcpy_rect_check): New helper function for
> omp_target_memcpy_rect and omp_target_memcpy_rect_async that checks
> requirements.
> (omp_target_memcpy_rect_copy): New helper function for
> omp_target_memcpy_rect and omp_target_memcpy_rect_async that performs
> the memcpy.
> (omp_target_memcpy_rect_async_helper): New helper function that is used
> in omp_target_memcpy_rect_async for the asynchronous task.
> (omp_target_memcpy_rect_async): Added.
> * task.c (ialias): Added for GOMP_task.
> * testsuite/libgomp.c-c++-common/target-memcpy-async-1.c: New test.
> * testsuite/libgomp.c-c++-common/target-memcpy-async-2.c: New test.
> * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-1.c: New test.
> * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-2.c: New test.
> * testsuite/libgomp.fortran/target-memcpy-async-1.f90: New test.
> * testsuite/libgomp.fortran/target-memcpy-async-2.f90: New test.
> * testsuite/libgomp.fortran/target-memcpy-rect-async-1.f90: New test.
> * testsuite/libgomp.fortran/target-memcpy-rect-async-2.f90: New test.
Ok, thanks.
Jakub
Re: [PATCH] OpenMP, libgomp: Add new runtime routines omp_target_memcpy_async and omp_target_memcpy_rect_async
Hi Jakub,
Am 17.05.2022 um 20:08 schrieb Jakub Jelinek:
On Tue, May 17, 2022 at 11:57:02AM +0200, Marcel Vollweiler wrote:
More importantly, I have no idea how this can work when you pass arg_size 0
and arg_align 0. The s variable is in the current function frame, with
arg_size 0 nothing is really copied to the generated task.
arg_size should be sizeof (memcpy_t) and arg_align __alignof__ (memcpy_t)
(well, struct omp_target_memcpy_data).
The copy function of GOMP_task ("cpyfn") is not used here (set to NULL) and thus
also arg_size and arg_align are set to 0 since they are related to cpyfn if I
understand it correctly.
No, arg_size and arg_align are for all (explicit) tasks the size and
alignment of the arguments. For an included task (one executed by the
encountering thread) we indeed use data directly instead of allocating
arg_size arg_align aligned bytes and copying data to it. But when we create
a deferred task (that is the only thing that actually can be asynchronous), we
allocate struct gomp_task together with memory for the data (arg_size bytes
aligned to arg_align). If cpyfn, we invoke that copy function (from source
data to the destination buffer), otherwise memcpy. cpyfn is a callback that
will do memcpy for parts that need bitwise copy and copy construction /
whatever else is needed for other data.
Looking at your patch, you call GOMP_task always with if_clause = false,
that means it is always included task (like with #pragma omp task if(0)),
but that also means calling GOMP_task doesn't bring any advantages and it is
not asynchronous.
If you called it with if_clause = true, like what #pragma omp task would do,
then the arg_size = 0 and arg_align = 0 would make it not work at all,
so after fixing if_clause, you need to supply sizeof (s) and __alignof__ (s).
Good explanation, thanks. Changed accordingly.
Also, it would be nice to avoid GOMP_task for the depobj_count == 0 case
at least sometimes (but perhaps that can be done incrementally) and instead
use some CUDA etc. asynchronous copy APIs. We don't really need to wait
for anything in that case, and from OpenMP POV all we need to make sure is
that barrier/taskwait/taskgroup end will know about these "tasks" and
wait for them. So, it can be implemented more like #pragma omp target nowait
instead of #pragma omp task that calls the synchronous omp_target_memcpy.
Though, maybe that is how it should be implemented always, something like
gomp_create_target_task and its caller. We already use that single routine
for multiple purposes (target nowait as well as target enter/exit data
nowait), so just telling it somehow that it shouldn't do mapping/unmapping
and perhaps target execution and instead copying would be nice.
I dont't see/understand the advantage using gomp_create_target_task over
GOMP_task. Whether the task waits for dependencies
("gomp_task_maybe_wait_for_dependencies") depends on GOMP_TASK_FLAG_DEPEND which
is only set if depobj_count > 0 and depobj_list != NULL. Thus, there shouldn't
be any waiting in case of depobj_count == 0? Additionally, in both functions a
new thread is created - independently of dependencies.
GOMP_task never creates a new thread.
gomp_create_target_task can create (but just once) an unshackeled thread
that runs on the side, doesn't do normal OpenMP user work and just polls the
offloading device and performs unmapping or whatever is needed to finish a
nowait offloaded task.
The disadvantage of GOMP_task is:
1) if you call say omp_target_memcpy_async from outside of parallel, it will
not be actually asynchronous even if you call GOMP_task with if_clause =
true
2) if you call it from inside of parallel, it might be scheduled only when
some host thread is ready for work (e.g. when reaching #pragma omp barrier,
implicit barrier, #pragma omp taskwait etc.), so even when the offloading
device is unused but host has lots of work to do, it might take quite a
while before starting the work, and then one of the OpenMP host threads
will be blocked waiting for the copying to be done
gomp_create_target_task doesn't have these disadvantages, it can fire off the
copying right away and then just needs to be able to figure out when it
finished (either the unshackeled thread polls the device, or some other way
how to find out that it finished; but OpenMP certainly needs to know that,
because user code can say #pragma omp taskwait for it, or it should be
complete at the end of a taskgroup, or at the end of #pragma omp barrier
or implicit barrier etc.).
Anyway, I guess it is ok to use GOMP_task in the initial patch and change it
later, but if_clause = false and 0, 0 for arg_{size,align} are definitely
wrong.
Agreed. Thanks for the details.
+int
+omp_target_memcpy (void *dst, const void *src, size_t length, size_t
dst_offset,
+ size_t src_offset, int dst_device_num, int src_device_num)
+{
+ struct gomp_device_descr *dst_devicep = NULL, *src_devicep = NULL;
+ int
[PATCH] cfgexpand: Yet another spot with debug insns references to global vars without varpool nodes [PR105630]
Hi!
This is similar to the earlier patch to avoid having MEM_EXPRs
referencing global vars without varpool nodes, but this time
the difference is that during gimplification some hashing
actually created DECL_RTLs for the n VAR_DECL and the previous
change was in the if above this when DECL_RTL is NULL and we are
considering creating it.
The following patch drops on the floor references to vars where
we've optimized away the varpool node even when it has DECL_RTL.
Bootstrapped/regtested on x86_64-linux and i686-linux, plus
bootstrapped on those without the cfgexpand.cc change, reapplied
it and rebuilt stage3 cc1/cc1plus, the resulting cc1/cc1plus
binaries on both targets were identical except for the 16-byte
executable_checksum (I've done the second bootstraps in the same
directory as the first one after moving the previous one elsewhere,
so pathnames were the same, just checksum hasn't been regenerated).
So, at least on those binaries this patch doesn't affect debug info
at all.
Ok for trunk?
2022-05-19 Jakub Jelinek
PR debug/105630
* cfgexpand.cc (expand_debug_expr): For VAR_DECL, punt for
global vars without symtab node even when they have DECL_RTL
set.
* gcc.dg/pr105630.c: New test.
--- gcc/cfgexpand.cc.jj 2022-05-09 09:09:20.005477502 +0200
+++ gcc/cfgexpand.cc2022-05-18 13:53:49.622983222 +0200
@@ -4575,6 +4575,10 @@ expand_debug_expr (tree exp)
|| SYMBOL_REF_DECL (XEXP (op0, 0)) != exp)
return NULL;
}
+ else if (VAR_P (exp)
+ && is_global_var (exp)
+ && symtab_node::get (exp) == NULL)
+ return NULL;
else
op0 = copy_rtx (op0);
--- gcc/testsuite/gcc.dg/pr105630.c.jj 2022-05-18 14:02:18.426050242 +0200
+++ gcc/testsuite/gcc.dg/pr105630.c 2022-05-18 14:02:07.103204525 +0200
@@ -0,0 +1,22 @@
+/* PR debug/105630 */
+/* { dg-do compile { target pthread } } */
+/* { dg-options "-O1 -ftree-parallelize-loops=2 -fcompare-debug" } */
+
+int m;
+static int n;
+
+void
+foo (void)
+{
+ int *arr[] = { , , };
+ int unused = n;
+
+ m = 0;
+}
+
+void
+bar (int *arr, int i)
+{
+ while (i < 1)
+arr[i++] = 1;
+}
Jakub
[PATCH v2] libgccjit: allow common objects in $(EXTRA_GCC_OBJS) and $(EXTRA_OBJS)
Hello, This patch fixes libgccjit build failure on loongarch* targets, and could probably be useful for future ports. For now, libgccjit is linked with objects from $(EXTRA_GCC_OBJS) and libbackend.a, which contains object files from $(EXTRA_OBJS). This effectively forbids any overlap between those two lists, i.e. all target-specific shared code between the gcc driver and compiler executables must go into gcc/common/config//-common.cc, which feels a bit inconvenient when there are a lot of "common" stuff that we want to put into separate source files. By linking libgccjit with $(EXTRA_GCC_OBJS_EXCLUSIVE), which contains all elements from $(EXTRA_GCC_OBJS) but not $(EXTRA_OBJS), this problem can be alleviated. This patch does not affect any other target architecture than loongarch, and has been bootstrapped and regression-tested on loongarch64-linux-gnuf64 an x86_64-pc-linux-gnu. Any recommendations? Please review. Thanks a lot. Yujie * gcc/jit/ChangeLog: * Make-lang.in: only link objects from $(EXTRA_GCC_OBJS) that's not in $(EXTRA_OBJS) into libgccjit. --- gcc/jit/Make-lang.in | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in index 6e10abfd0ac..248ec45b729 100644 --- a/gcc/jit/Make-lang.in +++ b/gcc/jit/Make-lang.in @@ -157,18 +157,23 @@ LIBGCCJIT_EXTRA_OPTS = $(LIBGCCJIT_VERSION_SCRIPT_OPTION) \ endif endif +# Only link objects from $(EXTRA_GCC_OBJS) that's not already +# included in libbackend.a ($(EXTRA_OBJS)). +EXTRA_GCC_OBJS_EXCLUSIVE = $(foreach _obj1, $(EXTRA_GCC_OBJS), \ + $(if $(filter $(_obj1), $(EXTRA_OBJS)),, $(_obj1))) + # We avoid using $(BACKEND) from Makefile.in in order to avoid pulling # in main.o $(LIBGCCJIT_FILENAME): $(jit_OBJS) \ libbackend.a libcommon-target.a libcommon.a \ $(CPPLIB) $(LIBDECNUMBER) \ $(LIBDEPS) $(srcdir)/jit/libgccjit.map \ - $(EXTRA_GCC_OBJS) $(jit.prev) + $(EXTRA_GCC_OBJS_EXCLUSIVE) $(jit.prev) @$(call LINK_PROGRESS,$(INDEX.jit),start) +$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ -shared \ $(jit_OBJS) libbackend.a libcommon-target.a libcommon.a \ $(CPPLIB) $(LIBDECNUMBER) $(EXTRA_GCC_LIBS) $(LIBS) $(BACKENDLIBS) \ -$(EXTRA_GCC_OBJS) \ +$(EXTRA_GCC_OBJS_EXCLUSIVE) \ $(LIBGCCJIT_EXTRA_OPTS) @$(call LINK_PROGRESS,$(INDEX.jit),end) -- 2.31.1
[PATCH] pointer-query: Fix ICE with non-pointer param [PR105635]
Hi!
The gimple_parm_array_size function comment talks about pointe parameters
but doesn't actually verify it, it checks whether an attribute is present
on the function and then just uses TREE_TYPE (TREE_TYPE (var)) which
assumes a pointer type (or in theory could work for ARRAY_TYPE but
c-family languages which only have that attribute will never have ARRAY_TYPE
parameters; and for VECTOR_TYPE/COMPLEX_TYPE it would mean something quite
different).
So, this patch punts early if var doesn't have pointer/reference type.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-05-19 Jakub Jelinek
PR c/105635
* poiinter-query.cc (gimple_parm_array_size): Return NULL if var
doesn't have pointer or reference type.
* gcc.dg/pr105635.c: New test.
--- gcc/pointer-query.cc.jj 2022-05-13 09:34:23.996147311 +0200
+++ gcc/pointer-query.cc2022-05-18 12:12:21.398849571 +0200
@@ -555,7 +555,7 @@ gimple_parm_array_size (tree ptr, wide_i
from the current function declaratation (e.g., attribute access or
related). */
tree var = SSA_NAME_VAR (ptr);
- if (TREE_CODE (var) != PARM_DECL)
+ if (TREE_CODE (var) != PARM_DECL || !POINTER_TYPE_P (TREE_TYPE (var)))
return NULL_TREE;
const unsigned prec = TYPE_PRECISION (sizetype);
--- gcc/testsuite/gcc.dg/pr105635.c.jj 2022-05-18 12:23:56.868371600 +0200
+++ gcc/testsuite/gcc.dg/pr105635.c 2022-05-18 12:23:41.728577929 +0200
@@ -0,0 +1,11 @@
+/* PR c/105635 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+void foo (int, int[*]);/* { dg-message "previous declaration of 'foo'
with type" } */
+
+foo (int x, int y) /* { dg-warning "return type defaults to 'int'" } */
+{ /* { dg-warning "conflicting types for 'foo'" "" {
target *-*-* } .-1 } */
+ /* { dg-message "declared here" "" { target *-*-* } .-2
} */
+ return (x >= 0) != (y < 0); /* { dg-warning "'return' with a value, in
function returning void" } */
+}
Jakub
[PATCH] ipa-icf: skip variables with body_removed
Similarly to cgraph_nodes, it may happen that body_removed is set
during merging of symbols.
PR ipa/105600
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
* ipa-icf.cc (sem_item_optimizer::filter_removed_items):
Skip variables with body_removed.
---
gcc/ipa-icf.cc | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
index 765ae746745..6528a7a10b2 100644
--- a/gcc/ipa-icf.cc
+++ b/gcc/ipa-icf.cc
@@ -2411,10 +2411,11 @@ sem_item_optimizer::filter_removed_items (void)
{
/* Filter out non-readonly variables. */
tree decl = item->decl;
- if (TREE_READONLY (decl))
- filtered.safe_push (item);
- else
+ varpool_node *vnode = static_cast (item)->get_node ();
+ if (!TREE_READONLY (decl) || vnode->body_removed)
remove_item (item);
+ else
+ filtered.safe_push (item);
}
}
}
--
2.36.1
Re: [PATCH] PR105647 Update pr105169* so it does not fail on powerpc64le
On Wed, 18 May 2022, Giuliano Belinassi wrote:
> On powerpc64le, the tests related to pr105169 failed because the
> .localentry was not on a power of two address due to the extra nop
> instruction taking one byte and thus moving its position one byte
> further. Generating two nops instead moves .localentry to a valid
> position.
OK
> gcc/testsuite/ChangeLog
> 2022-05-18 Giuliano Belinassi
>
> PR target/105647
> * g++.dg/modules/pr105169_a.C: Change -fpatchable-function-entry to 2.
> * g++.dg/modules/pr105169_b.C: Likewise.
> ---
> gcc/testsuite/g++.dg/modules/pr105169_a.C | 4 ++--
> gcc/testsuite/g++.dg/modules/pr105169_b.C | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/testsuite/g++.dg/modules/pr105169_a.C
> b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> index 66dc4b7901f..02660b3a0e4 100644
> --- a/gcc/testsuite/g++.dg/modules/pr105169_a.C
> +++ b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> @@ -1,6 +1,6 @@
> /* { dg-module-do link } */
> -/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> -/* { dg-additional-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> +/* { dg-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
> +/* { dg-additional-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
>
> /* This test is in the "modules" package because it supports multiple files
> linkage. */
> diff --git a/gcc/testsuite/g++.dg/modules/pr105169_b.C
> b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> index 5f8b00dfe51..7a9c5863a6a 100644
> --- a/gcc/testsuite/g++.dg/modules/pr105169_b.C
> +++ b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> @@ -1,6 +1,6 @@
> /* { dg-module-do link } */
> -/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> -/* { dg-additional-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> +/* { dg-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
> +/* { dg-additional-options "-std=c++11 -fpatchable-function-entry=2 -O2" } */
>
> /* This test is in the "modules" package because it supports multiple files
> linkage. */
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
Re: [x86 PATCH] Some additional ix86_rtx_costs clean-ups: NEG, AND and pandn.
On Wed, May 18, 2022 at 7:30 PM Roger Sayle wrote:
>
>
> Hi Uros,
> Very many thanks for the speedy review and approval of my ix86_rtx_costs
> patch to correct the cost of multi-word multiplications. In the same
> spirit, this patch tidies up a few additional nits I noticed while there:
> Multi-word NOT requires two operations, but multi-word NEG requires
> three operations. Using SSE, vector NOT requires a pxor with -1, but
> AND of NOT is cheap thanks to the existence of pandn. There's also some
> legacy (aka incorrect) logic explicitly testing for DImode [independently
> of TARGET_64BIT] in determining the cost of logic operations that's not
> required.
>
> There should be no behavioural changes from this patch (as confirmed by
> testing) but hopefully vectorization and other middle-end passes can now
> rely on more sensible "cost" estimates.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}, with
> no new failures. Ok for mainline?
>
>
> 2022-05-18 Roger Sayle
>
> gcc/ChangeLog
> * config/i386/i386.cc (ix86_rtx_costs): Split AND from XOR/IOR.
> Multi-word binary logic operations require two instructions.
> For vector integer modes, AND with a NOT operand requires only
> a single instruction (pandn).
> [NOT]: Vector integer NOT requires more than 1 instruction (pxor).
> [NEG]: Multi-word negation requires 3 instructions.
-case AND:
case IOR:
case XOR:
if (GET_MODE_CLASS (mode) == MODE_INT
&& GET_MODE_SIZE (mode) > UNITS_PER_WORD)
{
- *total = (cost->add * 2
-+ (rtx_cost (XEXP (x, 0), mode, outer_code, opno, speed)
- << (GET_MODE (XEXP (x, 0)) != DImode))
-+ (rtx_cost (XEXP (x, 1), mode, outer_code, opno, speed)
- << (GET_MODE (XEXP (x, 1)) != DImode)));
+ *total = cost->add * 2
+ + rtx_cost (XEXP (x, 0), mode, outer_code, opno, speed)
+ + rtx_cost (XEXP (x, 1), mode, outer_code, opno, speed);
+ return true;
+}
+ else if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
+*total = ix86_vec_cost (mode, cost->sse_op);
+ else if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
+*total = cost->add * 2;
+ else
+*total = cost->add;
+ return false;
Shouldn't this be just:
if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
*total = ix86_vec_cost (mode, cost->sse_op);
else if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
*total = cost->add * 2;
else
*total = cost->add;
return false;
When returning false, subexpressions will be scanned automatically.
+ else if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
+{
+ /* pandn is a single instruction. */
+ if (GET_CODE (XEXP (x, 0)) == NOT)
pandn is also a single instruction with BMI. IMO, this should be also
reflected in RTX costs.
Uros.
Re: [PATCH v2] x86: Document -mcet-switch
On Wed, May 18, 2022 at 7:32 PM H.J. Lu via Gcc-patches
wrote:
>
> On Wed, May 11, 2022 at 1:52 PM H.J. Lu wrote:
> >
> > When -fcf-protection=branch is used, the compiler will generate jump
> > tables for switch statements where the indirect jump is prefixed with
> > the NOTRACK prefix, so it can jump to non-ENDBR targets. Since the
> > indirect jump targets are generated by the compiler, they are always
> > valid.
> >
> > Document -mcet-switch to generate jump tables for switch statements with
> > ENDBR and skip the NOTRACK prefix for indirect jump. This option should
> > be used when the NOTRACK prefix is disabled.
> >
> > PR target/104816
> > * config/i386/i386.opt: Remove Undocumented.
> > * doc/invoke.texi: Document -mcet-switch.
> > ---
> > gcc/config/i386/i386.opt | 2 +-
> > gcc/doc/invoke.texi | 8 +++-
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
> > index a6b0e28f238..0dbaacb57ed 100644
> > --- a/gcc/config/i386/i386.opt
> > +++ b/gcc/config/i386/i386.opt
> > @@ -1047,7 +1047,7 @@ Enable shadow stack built-in functions from
> > Control-flow Enforcement
> > Technology (CET).
> >
> > mcet-switch
> > -Target Undocumented Var(flag_cet_switch) Init(0)
> > +Target Var(flag_cet_switch) Init(0)
> > Turn on CET instrumentation for switch statements that use a jump table and
> > an indirect jump.
> >
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 7a35d9613a4..968853690f9 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -1420,7 +1420,8 @@ See RS/6000 and PowerPC Options.
> > -msse4a -m3dnow -m3dnowa -mpopcnt -mabm -mbmi -mtbm -mfma4 -mxop
> > @gol
> > -madx -mlzcnt -mbmi2 -mfxsr -mxsave -mxsaveopt -mrtm -mhle -mlwp
> > @gol
> > -mmwaitx -mclzero -mpku -mthreads -mgfni -mvaes -mwaitpkg @gol
> > --mshstk -mmanual-endbr -mforce-indirect-call -mavx512vbmi2 -mavx512bf16
> > -menqcmd @gol
> > +-mshstk -mmanual-endbr -mcet-switch -mforce-indirect-call @gol
> > +-mavx512vbmi2 -mavx512bf16 -menqcmd @gol
> > -mvpclmulqdq -mavx512bitalg -mmovdiri -mmovdir64b -mavx512vpopcntdq
> > @gol
> > -mavx5124fmaps -mavx512vnni -mavx5124vnniw -mprfchw -mrdpid @gol
> > -mrdseed -msgx -mavx512vp2intersect -mserialize -mtsxldtrk@gol
> > @@ -32641,6 +32642,11 @@ function attribute. This is useful when used with
> > the option
> > @option{-fcf-protection=branch} to control ENDBR insertion at the
> > function entry.
> >
> > +@item -mcet-switch
> > +@opindex mcet-switch
> > +Turn on CET instrumentation for switch statements that use a jump table
> > +and an indirect jump.
> > +
Can you explain the difference between -mcet-switch and -mno-cet-switch here
please and indicate the default? The above doesn't sound useful to me.
> > @item -mcall-ms2sysv-xlogues
> > @opindex mcall-ms2sysv-xlogues
> > @opindex mno-call-ms2sysv-xlogues
> > --
> > 2.35.1
> >
>
> Any comments?
>
> Thanks.
>
> --
> H.J.
