Re: [PATCH 1/2] RISC-V: Describe correct USEs for gpr_save pattern [PR95252]
Committed with adding comments for those two functions. On Thu, Jun 11, 2020 at 5:10 AM Jim Wilson wrote: > > On Wed, Jun 10, 2020 at 1:08 AM Kito Cheng wrote: > > * config/riscv/riscv.c (gpr_save_reg_order): New. > > (riscv_expand_prologue): Use riscv_gen_gpr_save_insn to gen > > gpr_save. > > (riscv_gen_gpr_save_insn): New. > > ... > > Looks good to me. Though these two new functions should have > explanatory comments before them indicating what they do. > > Jim
Re: [PATCH 2/2] RISC-V: Unify the output asm pattern between gpr_save and gpr_restore pattern.
Committed. On Thu, Jun 11, 2020 at 5:13 AM Jim Wilson wrote: > > On Wed, Jun 10, 2020 at 1:08 AM Kito Cheng wrote: > > * config/riscv/riscv-protos.h (riscv_output_gpr_save): Remove. > > * config/riscv/riscv-sr.c (riscv_sr_match_prologue): Update > > value. > > * config/riscv/riscv.c (riscv_output_gpr_save): Remove. > > * config/riscv/riscv.md (gpr_save): Update output asm pattern. > > Looks good to me also. > > Jim
Re: collect2.exe errors not pruned
On May 26, 2020, Alexandre Oliva wrote:
> On May 19, 2020, Joseph Myers wrote:
>> Allowing a missing executable name is reasonable enough, but I was
>> actually thinking that the messages should print "gcc" or whatever command
>> the user ran in place of "collect2".
> Should we make the regexps '\[^\n\]*', as in so many other pruned
> messages?
Like this. Regstrapped on x86_64-linux-gnu. Ok to install?
match any program name when pruning collect messages
From: Alexandre Oliva
When collect* programs have an executable suffix, they may include it
in their outputs. Match them when pruning gcc output, making room for
other program names to print them.
for gcc/testsuite/ChangeLog
* lib/prune.exp (prune_gcc_output): Match any executable name
in collect messages.
---
gcc/testsuite/lib/prune.exp |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index eea4bf3..1c77624 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -38,8 +38,8 @@ proc prune_gcc_output { text } {
regsub -all "(^|\n)\[^\n\]*: in .constexpr. expansion \[^\n\]*" $text ""
text
regsub -all "(^|\n)\[^\n\]*: in requirements \[^\n\]*" $text "" text
regsub -all "(^|\n)inlined from \[^\n\]*" $text "" text
-regsub -all "(^|\n)collect2: error: ld returned \[^\n\]*" $text "" text
-regsub -all "(^|\n)collect: re(compiling|linking)\[^\n\]*" $text "" text
+regsub -all "(^|\n)\[^\n\]*: error: ld returned \[^\n\]*" $text "" text
+regsub -all "(^|\n)\[^\n\]*: re(compiling|linking)\[^\n\]*" $text "" text
regsub -all "(^|\n)Please submit.*instructions\[^\n\]*" $text "" text
regsub -all "(^|\n)\[0-9\]\[0-9\]* errors\." $text "" text
regsub -all "(^|\n)(In file included|\[ \]+from)\[^\n\]*" $text "" text
--
Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/
Free Software Evangelist Stallman was right, but he's left :(
GNU Toolchain Engineer Live long and free, and prosper ethically
Re: avoid line breaks in -fverbose-asm tree exprs
On Jun 9, 2020, Richard Biener wrote:
> How about simply unconditionally doing dump_flags | TDF_SLIM here
> to have the whole mem_expr on one line.
SGTM
> OK with that change.
Thanks, here's what I'm installing.
slim up mem exprs to avoid line breaks in -fverbose-asm
From: Alexandre Oliva
An asm operand with a "VIEW_CONVERT_EXPR" will output the definition of the struct as asm code. Oops.
Enable TDF_SLIM in print_mem_expr to avoid such line breaks.
for gcc/ChangeLog
* print-rtl.c (print_mem_expr): Enable TDF_SLIM in dump_flags.
---
print-rtl.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git gcc/print-rtl.c gcc/print-rtl.c
index 611ea07..25265ef 100644
--- gcc/print-rtl.c
+++ gcc/print-rtl.c
@@ -183,7 +183,8 @@ void
print_mem_expr (FILE *outfile, const_tree expr)
{
fputc (' ', outfile);
- print_generic_expr (outfile, CONST_CAST_TREE (expr), dump_flags);
+ print_generic_expr (outfile, CONST_CAST_TREE (expr),
+ dump_flags | TDF_SLIM);
}
#endif
--
Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/
Free Software Evangelist Stallman was right, but he's left :(
GNU Toolchain Engineer Live long and free, and prosper ethically
RE: [PATCH PR95523] aarch64:ICE in register_tuple_type，at config/aarch64/aarch64-sve-builtins.cc:3434
Thanks for viewing and pushing this.
Thanks,
Haijian Zhang
> -Original Message-
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Thursday, June 11, 2020 12:01 AM
> To: Zhanghaijian (A)
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR95523] aarch64:ICE in register_tuple_type，at
> config/aarch64/aarch64-sve-builtins.cc:3434
>
> Hi,
>
> "Zhanghaijian (A)" writes:
> > This is a simple fix for pr95523.
> > When registering the tuple type in register_tuple_type, the TYPE_ALIGN
> (tuple_type) will be changed by -fpack-struct=n.
> > We need to maintain natural alignment in handle_arm_sve_h.
> > Bootstrap and tested on aarch64 Linux platform. No new regression
> witnessed.
>
> Sorry for dropping the ball on the bugzilla PR and not replying to your
> comment
> there. For the record...
>
> IMO it's a misfeature that -fpack-struct=N and “#pragma pack (1)”
> override explicit alignments. E.g. it means that for:
>
> struct __attribute__((packed)) s1 { __attribute__((aligned(2))) int x; };
> #pragma pack (1)
> struct s2 { __attribute__((aligned(2))) int x; };
>
> s1 has alignment 2 but s2 has alignment 1. There's a comment about this in
> stor-layout.c:
>
> /* Should this be controlled by DECL_USER_ALIGN, too? */
> if (mfa != 0)
> SET_DECL_ALIGN (decl, MIN (DECL_ALIGN (decl), mfa));
>
> I think the condition probably should have checked DECL_USER_ALIGN.
>
> However, there's no telling whether anything now relies on the current
> behaviour, and since this bug is about internally-defined structures, it's
> probably not a good motivating example for changing the code above.
>
> Also, I agree the patch is a clean way of avoiding the problem, and is
> probably
> more robust than the DECL_USER_ALIGN thing would have been.
>
> Pushed to master, thanks.
>
> Richard
Re: [PATCH V2] Practical Improvement to Double Precision Complex Divide
On Thu, 11 Jun 2020, Patrick McGehearty wrote: > I will study real.c carefully along with the C99 std > to determine if I can find useful values for RMIN2 and RMINSCAL > for each format which are within range for all instances > of that format. A quick skim of real.c shows we have ieee half precision > and two arm half precision formats, for example. The difficulty will be cases such as TFmode which can be IEEE binary128 or IBM long double, which have very different exponent ranges. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH V2] Practical Improvement to Double Precision Complex Divide
I will study real.c carefully along with the C99 std to determine if I can find useful values for RMIN2 and RMINSCAL for each format which are within range for all instances of that format. A quick skim of real.c shows we have ieee half precision and two arm half precision formats, for example. I appreciate the pointer to real.c as I was sure that information about various platform formats was somewhere in the src tree, but had not found it on my own. - patrick On 6/10/2020 5:39 PM, Joseph Myers wrote: On Wed, 10 Jun 2020, Patrick McGehearty wrote: #ifdef L_divhc3 #define RBIG (correct value for half precision) #define RMIN (correct value for half precision) #define RMIN2 ... (correct value for half precision) #define RMINSCAL ... (correct value for half precision) #endif #ifdef L_divsc3 #define RBIG FLT_MAX #define RMIN FLT_MIN #define RMIN2 ... #define RMINSCAL ... #endif ... would get the job done once I determine (and test) the correct values for all formats. But some of those machine modes can represent several different floating-point formats, depending on the target. There are many more floating-point formats supported in real.c than there are names for floating-point machine modes. Not all the differences are relevant here (those differences relating to default NaNs, for example), but some are.
Re: BRIG FE testsuite: Fix all dump-scans
On Jun 9, 2020, Martin Jambor wrote: >> Before your changes, GCC produced the expected: >> >> $ ls -1 build-gcc/gcc/testsuite/brig/variables.*???t.* >> build-gcc/gcc/testsuite/brig/variables.hsail.brig.004t.original >> build-gcc/gcc/testsuite/brig/variables.hsail.brig.005t.gimple >> >> Are you able to easily create a patch for that? How/where to adjust: >> producer-side (GCC driver, or BRIG (HSAIL) front end?), or consumer-side >> (testsuite: tree scanning machinery, or have to put some '-dumpbase' into >> all test case files?)? > I looked into the issue yesterday and decided the simplest fix is > probably the following. I concur, FWIW. Thanks for looking into it. Since dumpbase is now based on dest rather than src, at least for compilation without linking, a change that aligns source and dest basenames makes for a least-surprise scenario. I suppose we might want to eventually adjust the expectations of dump names in the testsuite to compute names based on outputs rather than inputs, or based on both when compiling and linking, but that will require quite significant internal API changes in the testsuite infrastructure. I'm sort of hoping we can make do with what we got, at least until the new naming conventions are solidly established. > * lib/brig.exp (brig_target_compile): Strip hsail extension when > gnerating the name of the binary brig file. LGTM, thanks. -- Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/ Free Software Evangelist Stallman was right, but he's left :( GNU Toolchain Engineer Live long and free, and prosper ethically
Re: [PATCH] Use GCC_PICFLAG to collect host-specific PICFLAG from ../config/picflag.m4
On Wed, Jun 10, 2020 at 04:27:27PM -0600, Jeff Law wrote:
> On Mon, 2019-07-22 at 12:39 -0400, Arvind Sankar wrote:
> > The gcc configure script does not use the config/picflag.m4 macro to
> > customize PICFLAG according to the host when using --enable-host-shared.
> >
> > Fix configure.ac to do so.
> >
> > Tested bootstrap on x86_64-linux-gnu.
> >
> > 2019-07-22 Arvind Sankar
> >
> > * gcc/configure.ac: Use GCC_PICFLAG.
> I know this is old
>
> Can you be more specific here about what you're trying to fix? ie, what
> host/target combination are you working on. What behavior are you seeing
> (presumably usage of -fPIC) what behavior did you expect (some other flag
> presumably).
>
> From looking at picflag.m4 the thing I worry the most about is the various
> ix86/x86_64 clauses which specify -fpic. It looks like your change would
> cause
> us to start using -fpic rather than -fPIC as we've been doing for eons and I
> worry that might have unintended consequences.
>
> Thanks,
> Jeff
> >
>
I don't remember exactly, but I don't think there was any actual
problem. At the time, I was playing around with trying to build the bulk
of cc1 etc as a shared library to reduce the size of the compiler
installation. IIRC I just came across this, noticed that there's a
config/picflag.m4 which wasn't getting used and posted this as a
cleanup.
This was originally added in r177967 ("Centralize PICFLAG
configuration") which used it for PICFLAG_FOR_TARGET (which still goes
via config/picflag.m4) but the host code which was added later just
hardcodes -fPIC.
Re: [PATCH V2] Practical Improvement to Double Precision Complex Divide
On Wed, 10 Jun 2020, Patrick McGehearty wrote: > #ifdef L_divhc3 > #define RBIG (correct value for half precision) > #define RMIN (correct value for half precision) > #define RMIN2 ... (correct value for half precision) > #define RMINSCAL ... (correct value for half precision) > #endif > #ifdef L_divsc3 > #define RBIG FLT_MAX > #define RMIN FLT_MIN > #define RMIN2 ... > #define RMINSCAL ... > #endif > ... > would get the job done once I determine (and test) the correct > values for all formats. But some of those machine modes can represent several different floating-point formats, depending on the target. There are many more floating-point formats supported in real.c than there are names for floating-point machine modes. Not all the differences are relevant here (those differences relating to default NaNs, for example), but some are. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH V2] Practical Improvement to Double Precision Complex Divide
Joseph, Thank you again for your prompt and insightful response. It's now clear that I was engaged in wishful thinking that I only needed to handle double precision in my proposed change. I understand now that the text above the routine: - - - - - #if defined(L_divhc3) || defined(L_divsc3) || defined(L_divdc3) \ || defined(L_divxc3) || defined(L_divtc3) - - - - - implies the same code is to be built for L_divhc3 half precision L_divsc3 single precision L_divdc3 double precision L_divxc3 extended precision L_divtc3 quad precision I will need expand my test suites for these other floating point formats and do my best to understand how the libgcc build mechanisms work before I'm ready to resubmit. At first glance, assuming only one of the L_div?c3 names are defined at a time, it seems like something like: #ifdef L_divhc3 #define RBIG (correct value for half precision) #define RMIN (correct value for half precision) #define RMIN2 ... (correct value for half precision) #define RMINSCAL ... (correct value for half precision) #endif #ifdef L_divsc3 #define RBIG FLT_MAX #define RMIN FLT_MIN #define RMIN2 ... #define RMINSCAL ... #endif ... would get the job done once I determine (and test) the correct values for all formats. - Patrick McGehearty On 6/9/2020 6:11 PM, Joseph Myers wrote: On Wed, 10 Jun 2020, Patrick McGehearty wrote: I see your point about other floating point formats. According to the C standard, DOUBLE precision must have a DBL_MAX of at least 1E+37. That is (slightly) greater than 0x1.0p+63. Would #define RMIN2 (0x1.0p-53) #define RMINSCAL (0x1.0p+51) be acceptable? Those will be in range for any architecture that supports the C standard. But this code is used for different machine modes, not just double. In particular, on Arm / AArch64 it may be used to build __divhc3. And those constants are certainly outside the range of HFmode (IEEE binary16). I think you need to work out appropriate logic for what values are correct for a given machine mode, in terms of the parameters of that mode (maximum and minimum exponents etc.). Then, if you can't represent the logic for determining those values (with the correct type, not hardcoding the absence of a suffix which would mean double) directly in C, see how c-cppbuiltin.c predefines extra macros if -fbuilding-libgcc (macros relating to built-in function suffixes and the number of bits in the mantissa of a floating-point mode are included there - it would certainly make sense to add more such macros if necessary).
Re: [PATCH] Use GCC_PICFLAG to collect host-specific PICFLAG from ../config/picflag.m4
On Mon, 2019-07-22 at 12:39 -0400, Arvind Sankar wrote: > The gcc configure script does not use the config/picflag.m4 macro to > customize PICFLAG according to the host when using --enable-host-shared. > > Fix configure.ac to do so. > > Tested bootstrap on x86_64-linux-gnu. > > 2019-07-22 Arvind Sankar > > * gcc/configure.ac: Use GCC_PICFLAG. I know this is old Can you be more specific here about what you're trying to fix? ie, what host/target combination are you working on. What behavior are you seeing (presumably usage of -fPIC) what behavior did you expect (some other flag presumably). >From looking at picflag.m4 the thing I worry the most about is the various ix86/x86_64 clauses which specify -fpic. It looks like your change would cause us to start using -fpic rather than -fPIC as we've been doing for eons and I worry that might have unintended consequences. Thanks, Jeff >
Re: drop -aux{dir,base}, revamp -dump{dir,base}
On Jun 9, 2020, Thomas Schwinge wrote: > Are you able to easily create/suggest patches for these? (You're > probably not set up for offloading compilation...) I can try, but I can certainly use help, if not in coding, at least with testing. > Can you suggest > how/where to adjust: producer-side (GCC driver, 'mkoffload's?), or > consumer-side (testsuite: offload tree scanning machinery etc., or have > to put some '-dumpbase' or similar into all such test case files?)? Given that files we used to output have now moved to /tmp, I suspect we're failing to pass -dumpbase at some point, so the offloading compiler ends up naming dump files after the temporary outputs rather than as per the requested/expected names inferred from the main compilation. I guess mkoffload might need some tweaking; I see pieces of lto-wrapper to call it, and the offload compiler, that I suspected might require tweaking for the dumpbase revamp as well. Let's see how far I can get by just looking at the code ;-) If I were to get something like a -v compile and link session, from someone all set for offloading compilation, with the command line passed to lto-wrapper and the full commands it runs, I might be able to figure out the dynamics more readily (hint, hint ;-) Thanks in advance, -- Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/ Free Software Evangelist Stallman was right, but he's left :( GNU Toolchain Engineer Live long and free, and prosper ethically
Re: [PATCH V2] Practical Improvement to Double Precision Complex Divide
A follow up note relating to use of fused multiply add in complex divide:
While reviewing bugs relating to complex divide in libgcc, I
rediscovered bug 59714 - complex division is surprising on targets
with FMA.
The specific concern was when you divide (1.0 + 3.0i) by itself and
fused multiply add was active, you did not get (1.0 + 0.0i) as an
answer. Instead you got 1.0 + epsilon off of zero.
The question raised in that bug report was whether it was appropriate
to use fused multiply add. I.e. on the whole, did it contribute to
greater or less accuracy over a range of cases. No follow up data has
been posted.
My data sets (10M randomly generated values over the full exponent and
mantissa range) showed the following with/without FMA:
Moderate exponent data set (between -512 and 511 for exponent)
error current current new new
exceeds no fma fma no fma fma
1 ulp 0.34492% 0.24715% 0.34492% 0.24715%
2 ulp 0.02360% 0.01764% 0.02360% 0.01764%
Full exponent data set (all exponents generated)
error current current new new
exceeds no fma fma no fma fma
1 ulp 1.90432% 1.87195% 0.23167% 0.19746%
2 ulp 1.70836% 1.70646% 0.04067% 0.03856%
While the differences are not dramatic between using fma and no fma,
they seem fairly clear that fma provides slightly more accurate
results more often than not.
I concur with the current practice of using fma if the architecture
provides it.
As a side note, my new method for complex divide gets the exact result
for dividing (1.0 + 3.0i) by itself whether or not fused multiply add
is used. That's because the calculation of the imaginary part
includes subtracting two nearly equal values which turns out to be
less than DBL_MIN. That means the new algorithm changes the order of
operations which does not involve the use of fused mul-add.
- Patrick McGehearty
On 6/4/2020 6:38 PM, Patrick McGehearty via Gcc-patches wrote:
The following patch to libgcc/libgcc2.c __divdc3 provides an
opportunity to gain important improvements to the quality of answers
for the default double precision complex divide routine when dealing
with very large or very small exponents.
The current code correctly implements Smith's method (1962) [1]
further modified by c99's requirements for dealing with NaN (not a
number) results. When working with input values where the exponents
are greater than 512 (i.e. 2.0^512) or less than -512 (i.e. 2.0^-512),
results are substantially different from the answers provided by quad
precision more than 1% of the time. Since the allowed exponent range
for double precision numbers is -1076 to +1023, the error rate may be
unacceptable for many applications. The proposed method reduces the
frequency of "substantially different" answers by more than 99% at a
modest cost of performance.
Differences between current gcc methods and the new method will be
described. Then accuracy and performance differences will be discussed.
(Added for Version 2)
In my initial research, I missed Elen Kalda's proposed patch
https://gcc.gnu.org/legacy-ml/gcc-patches/2019-08/msg01629.html [3]
Thanks to Joseph Myers for providing me with the pointer.
This version includes performance and accuracy comparisions
between Elen's proposed patch and my latest patch version which
has been modified to eliminate two branches.
NOTATION
For all of the following, the notation is:
Input complex values:
a+bi (a= 64bit real part, b= 64bit imaginary part)
c+di
Output complex value:
e+fi = (a+bi)/(c+di)
For the result tables:
current = current method (SMITH)
b1div = method proposed by Elen Kalda
b2div = alternate method considered by Elen Kalda
new1 = new method using 1 divide and 2 multiplies
new = new method proposed by this patch
DESCRIPTIONS of different complex divide methods:
NAIVE COMPUTATION (-fcx-limited-range):
e = (a*c + b*d)/(c*c + d*d)
f = (b*c - a*d)/(c*c + d*d)
Note that c*c and d*d will overflow or underflow if either
c or d is outside the range 2^-538 to 2^512.
This method is available in gcc when the switch -fcx-limited-range is
used. That switch is also enabled by -ffast-math. Only one who has a
clear understanding of the maximum range of intermediate values
generated by a computation should consider using this switch.
SMITH's METHOD (current libgcc):
if(fabs(c) RBIG) || (FABS (a) > RBIG) || (FABS (b) > RBIG) ) {
a = a * 0.5;
b = b * 0.5;
c = c * 0.5;
d = d * 0.5;
}
/* minimize overflow/underflow issues when c and d are small */
else if (FABS (d) < RMIN2) {
a = a * RMINSCAL;
b = b * RMINSCAL;
c = c * RMINSCAL;
d = d * RMINSCAL;
}
r = c/d; denom = (c*r) + d;
if( r > RMIN ) {
e = (a*r + b) / denom ;
f = (b*r - a) / denom
} else {
e = (c * (a/d) + b) / denom;
f = (c * (b/d) - a) / denom;
}
[ only presenting the fabs(c) < fabs(d) case here, full code in patch. ]
Re: [PATCH] libstdc++: Fix some ranges algos optimizations [PR95578]
On Wed, 10 Jun 2020, Jonathan Wakely wrote:
> On 10/06/20 15:32 -0400, Patrick Palka via Libstdc++ wrote:
> > ranges::copy and a number of other ranges algorithms have unwrapping
> > optimizations for iterators of type __normal_iterator, move_iterator and
> > reverse_iterator. But in the checks that guard these optimizations we
> > currently only test that the iterator of the iterator/sentinel pair has
> > the appropriate type before proceeding with the corresponding
> > optimization, and we fail to also test the sentinel type.
> >
> > This breaks the testcase in this PR because this testcase constructs
> > via range adaptors a range whose begin() is a __normal_iterator and
> > whose end() is a custom sentinel type, and then does ranges::copy on it.
> > From there we bogusly perform the __normal_iterator unwrapping
> > optimization on this iterator/sentinel pair, which immediately leads to
> > a constraint failure since the custom sentinel type does not model
> > sentinel_for.
> >
> > This patch fixes this issue by augmenting each of the problematic checks
> > to also test that the iterator and sentinel types are the same before
> > applying the corresponding unwrapping optimization.
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK to commit to master to
> > and to the 10.2 branch?
> >
> > libstdc++-v3/ChangeLog:
> >
> > PR libstdc++/95578
> > * include/bits/ranges_algo.h (__lexicographical_compare_fn):
> > Also check that the iterator and sentinel have the same type before
> > applying the unwrapping optimization for __normal_iterator.
> > Split the check into two, one for the first iterator/sentinel
> > pair and another for second iterator/sentinel pair. Stop using
> > __niter_base and stop doing std::move on a __normal_iterator.
> > * include/bits/ranges_algobase.c (__equal_fn): Likewise.
> > (__copy_or_move): Likewise. Perform similar adjustments for
> > the reverse_iterator and move_iterator optimizations. Inline
> > the checks into the if-constexprs, and use using-declarations to
> > make them less visually noisy. Don't use __niter_wrap.
> > (__copy_or_move_backward): Likewise.
> > * testsuite/25_algorithms/copy/95578.cc: New test.
> > * testsuite/25_algorithms/copy_backward/95578.cc: New test.
> > * testsuite/25_algorithms/equal/95578.cc: New test.
> > * testsuite/25_algorithms/lexicographical_compare/95578.cc: New test.
> > * testsuite/25_algorithms/move/95578.cc: New test.
> > * testsuite/25_algorithms/move_backward/95578.cc: New test.
> > ---
> > libstdc++-v3/include/bits/ranges_algo.h | 14 +--
> > libstdc++-v3/include/bits/ranges_algobase.h | 88 ++-
> > .../testsuite/25_algorithms/copy/95578.cc | 74
> > .../25_algorithms/copy_backward/95578.cc | 62 +
> > .../testsuite/25_algorithms/equal/95578.cc| 74
> > .../lexicographical_compare/95578.cc | 74
> > .../testsuite/25_algorithms/move/95578.cc | 62 +
> > .../25_algorithms/move_backward/95578.cc | 62 +
> > 8 files changed, 465 insertions(+), 45 deletions(-)
> > create mode 100644 libstdc++-v3/testsuite/25_algorithms/copy/95578.cc
> > create mode 100644
> > libstdc++-v3/testsuite/25_algorithms/copy_backward/95578.cc
> > create mode 100644 libstdc++-v3/testsuite/25_algorithms/equal/95578.cc
> > create mode 100644
> > libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/95578.cc
> > create mode 100644 libstdc++-v3/testsuite/25_algorithms/move/95578.cc
> > create mode 100644
> > libstdc++-v3/testsuite/25_algorithms/move_backward/95578.cc
> >
> > diff --git a/libstdc++-v3/include/bits/ranges_algo.h
> > b/libstdc++-v3/include/bits/ranges_algo.h
> > index c038a505afa..94ca7b6488d 100644
> > --- a/libstdc++-v3/include/bits/ranges_algo.h
> > +++ b/libstdc++-v3/include/bits/ranges_algo.h
> > @@ -3452,11 +3452,15 @@ namespace ranges
> > _Proj1 __proj1 = {}, _Proj2 __proj2 = {}) const
> > {
> > if constexpr (__detail::__is_normal_iterator<_Iter1>
> > - || __detail::__is_normal_iterator<_Iter2>)
> > - return (*this)(std::__niter_base(std::move(__first1)),
> > -std::__niter_base(std::move(__last1)),
> > -std::__niter_base(std::move(__first2)),
> > -std::__niter_base(std::move(__last2)),
> > + && same_as<_Iter1, _Sent1>)
> > + return (*this)(__first1.base(), __last1.base(),
> > +std::move(__first2), std::move(__last2),
> > +std::move(__comp),
> > +std::move(__proj1), std::move(__proj2));
> > + else if constexpr (__detail::__is_normal_iterator<_Iter2>
> > + && same_as<_Iter2, _Sent2>)
> > + return (*this)(std::move(__first1), std::move(__last1),
> > +__first2.base(), __last2.base(),
> >
[PATCH] c++: ICE with IMPLICIT_CONV_EXPR in array subscript [PR95508]
Since r10-7096 convert_like, when called in a template, creates an
IMPLICIT_CONV_EXPR when we're converting to/from array type.
In this test, we have e[f], and we're converting f (of type class A) to
int, so convert_like in build_new_op_1 created the IMPLICIT_CONV_EXPR
that got into cp_build_array_ref which calls maybe_constant_value. My
patch above failed to adjust this spot to call fold_non_dependent_expr
instead, which can handle codes like I_C_E in a template.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
gcc/cp/ChangeLog:
PR c++/95508
* typeck.c (cp_build_array_ref): Call fold_non_dependent_expr instead
of maybe_constant_value.
gcc/testsuite/ChangeLog:
PR c++/95508
* g++.dg/template/conv16.C: New test.
---
gcc/cp/typeck.c| 2 +-
gcc/testsuite/g++.dg/template/conv16.C | 17 +
2 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/conv16.C
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index f01ae656254..07144d4c1fc 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3565,7 +3565,7 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
pointer arithmetic.) */
idx = cp_perform_integral_promotions (idx, complain);
- idx = maybe_constant_value (idx);
+ idx = fold_non_dependent_expr (idx, complain);
/* An array that is indexed by a non-constant
cannot be stored in a register; we must be able to do
diff --git a/gcc/testsuite/g++.dg/template/conv16.C
b/gcc/testsuite/g++.dg/template/conv16.C
new file mode 100644
index 000..c0843efdf9d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/conv16.C
@@ -0,0 +1,17 @@
+// PR c++/95508
+// { dg-do compile }
+
+template
+struct A;
+template
+struct B {
+ operator int () { return 0; }
+};
+template <>
+struct A : B {};
+struct D {
+ template
+ int foo () { return e[f]; }
+ int e[6];
+ A f;
+};
base-commit: a2c2cee92e5defff9bf23d3b1184ee96e57e5fdd
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
[PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
Another indication that perhaps this warning is emitted too early. We
crash because same_type_p gets a null type: we have an enumerator
without a fixed underlying type and finish_enum_value_list hasn't yet
run. So check if the type is null before calling same_type_p.
(This is a regression and this fix is suitable for backporting. Delaying
the warning would not be suitable to backport.)
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?
gcc/cp/ChangeLog:
PR c++/95560
* name-lookup.c (check_local_shadow): Check if types are
non-null before calling same_type_p.
gcc/testsuite/ChangeLog:
PR c++/95560
* g++.dg/warn/Wshadow-local-3.C: New test.
---
gcc/cp/name-lookup.c| 4 +++-
gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2ff85f1cf5e..159c98a67cd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
enum opt_code warning_code;
if (warn_shadow)
warning_code = OPT_Wshadow;
- else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+ else if ((TREE_TYPE (old)
+ && TREE_TYPE (decl)
+ && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
|| TREE_CODE (decl) == TYPE_DECL
|| TREE_CODE (old) == TYPE_DECL
|| (!dependent_type_p (TREE_TYPE (decl))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
new file mode 100644
index 000..fd743eca347
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
@@ -0,0 +1,7 @@
+// PR c++/95560
+// { dg-do compile { target c++11 } }
+
+template void fn1() {
+ bool ready;
+ enum class State { ready };
+}
base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Re: [PATCH 1/2] RISC-V: Describe correct USEs for gpr_save pattern [PR95252]
On Wed, Jun 10, 2020 at 1:08 AM Kito Cheng wrote: > * config/riscv/riscv.c (gpr_save_reg_order): New. > (riscv_expand_prologue): Use riscv_gen_gpr_save_insn to gen gpr_save. > (riscv_gen_gpr_save_insn): New. > ... Looks good to me. Though these two new functions should have explanatory comments before them indicating what they do. Jim
Re: [PATCH 2/2] RISC-V: Unify the output asm pattern between gpr_save and gpr_restore pattern.
On Wed, Jun 10, 2020 at 1:08 AM Kito Cheng wrote: > * config/riscv/riscv-protos.h (riscv_output_gpr_save): Remove. > * config/riscv/riscv-sr.c (riscv_sr_match_prologue): Update > value. > * config/riscv/riscv.c (riscv_output_gpr_save): Remove. > * config/riscv/riscv.md (gpr_save): Update output asm pattern. Looks good to me also. Jim
Re: [PATCH] libstdc++: Fix some ranges algos optimizations [PR95578]
On 10/06/20 15:32 -0400, Patrick Palka via Libstdc++ wrote:
ranges::copy and a number of other ranges algorithms have unwrapping
optimizations for iterators of type __normal_iterator, move_iterator and
reverse_iterator. But in the checks that guard these optimizations we
currently only test that the iterator of the iterator/sentinel pair has
the appropriate type before proceeding with the corresponding
optimization, and we fail to also test the sentinel type.
This breaks the testcase in this PR because this testcase constructs
via range adaptors a range whose begin() is a __normal_iterator and
whose end() is a custom sentinel type, and then does ranges::copy on it.
From there we bogusly perform the __normal_iterator unwrapping
optimization on this iterator/sentinel pair, which immediately leads to
a constraint failure since the custom sentinel type does not model
sentinel_for.
This patch fixes this issue by augmenting each of the problematic checks
to also test that the iterator and sentinel types are the same before
applying the corresponding unwrapping optimization.
Tested on x86_64-pc-linux-gnu, does this look OK to commit to master to
and to the 10.2 branch?
libstdc++-v3/ChangeLog:
PR libstdc++/95578
* include/bits/ranges_algo.h (__lexicographical_compare_fn):
Also check that the iterator and sentinel have the same type before
applying the unwrapping optimization for __normal_iterator.
Split the check into two, one for the first iterator/sentinel
pair and another for second iterator/sentinel pair. Stop using
__niter_base and stop doing std::move on a __normal_iterator.
* include/bits/ranges_algobase.c (__equal_fn): Likewise.
(__copy_or_move): Likewise. Perform similar adjustments for
the reverse_iterator and move_iterator optimizations. Inline
the checks into the if-constexprs, and use using-declarations to
make them less visually noisy. Don't use __niter_wrap.
(__copy_or_move_backward): Likewise.
* testsuite/25_algorithms/copy/95578.cc: New test.
* testsuite/25_algorithms/copy_backward/95578.cc: New test.
* testsuite/25_algorithms/equal/95578.cc: New test.
* testsuite/25_algorithms/lexicographical_compare/95578.cc: New test.
* testsuite/25_algorithms/move/95578.cc: New test.
* testsuite/25_algorithms/move_backward/95578.cc: New test.
---
libstdc++-v3/include/bits/ranges_algo.h | 14 +--
libstdc++-v3/include/bits/ranges_algobase.h | 88 ++-
.../testsuite/25_algorithms/copy/95578.cc | 74
.../25_algorithms/copy_backward/95578.cc | 62 +
.../testsuite/25_algorithms/equal/95578.cc| 74
.../lexicographical_compare/95578.cc | 74
.../testsuite/25_algorithms/move/95578.cc | 62 +
.../25_algorithms/move_backward/95578.cc | 62 +
8 files changed, 465 insertions(+), 45 deletions(-)
create mode 100644 libstdc++-v3/testsuite/25_algorithms/copy/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/copy_backward/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/equal/95578.cc
create mode 100644
libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/move/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/move_backward/95578.cc
diff --git a/libstdc++-v3/include/bits/ranges_algo.h
b/libstdc++-v3/include/bits/ranges_algo.h
index c038a505afa..94ca7b6488d 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -3452,11 +3452,15 @@ namespace ranges
_Proj1 __proj1 = {}, _Proj2 __proj2 = {}) const
{
if constexpr (__detail::__is_normal_iterator<_Iter1>
- || __detail::__is_normal_iterator<_Iter2>)
- return (*this)(std::__niter_base(std::move(__first1)),
-std::__niter_base(std::move(__last1)),
-std::__niter_base(std::move(__first2)),
-std::__niter_base(std::move(__last2)),
+ && same_as<_Iter1, _Sent1>)
+ return (*this)(__first1.base(), __last1.base(),
+std::move(__first2), std::move(__last2),
+std::move(__comp),
+std::move(__proj1), std::move(__proj2));
+ else if constexpr (__detail::__is_normal_iterator<_Iter2>
+ && same_as<_Iter2, _Sent2>)
+ return (*this)(std::move(__first1), std::move(__last1),
+__first2.base(), __last2.base(),
std::move(__comp),
std::move(__proj1), std::move(__proj2));
else
So if all four iterators are normal_iterator then we first unwrap the
first pair, and recurse, and then unwrap the second pair, and
Re: [PATCH/RFC] How to fix PR95440
Hi Jason,
Jason Merrill wrote:
> On Tue, Jun 9, 2020 at 5:04 AM Iain Sandoe wrote:
>
> /* Don't bother reversing an operator with two identical parameters.
> */
> - else if (args->length () == 2 && (flags & LOOKUP_REVERSED))
> + else if (args && args->length () == 2 && (flags & LOOKUP_REVERSED))
>
> The usual pattern is to use vec_safe_length here. Similarly, in
> build_new_method_call_1 I think
>
> !user_args->is_empty()
>
> should be
>
> vec_safe_is_empty (user_args)
>
> Those changes are OK.
Thanks,
What I applied (after regtest on x86_64-linux/darwin and powerpc64-linux) is
below.
Given that this fixes an IDE-on-valid filed against 10.1, I’d like to backport
it for 10.2,
is that OK?
thanks
Iain
coroutines: Make call argument handling more robust [PR95440]
build_new_method_call is supposed to be able to handle a null
arguments list pointer (when the method has no parms). There
were a couple of places where uses of the argument list pointer
were not defended against NULL.
gcc/cp/ChangeLog:
PR c++/95440
* call.c (add_candidates): Use vec_safe_length() for
testing the arguments list.
(build_new_method_call_1): Use vec_safe_is_empty() when
checking for an empty args list.
gcc/testsuite/ChangeLog:
PR c++/95440
* g++.dg/coroutines/pr95440.C: New test.
---
gcc/cp/call.c | 4 +--
gcc/testsuite/g++.dg/coroutines/pr95440.C | 39 +++
2 files changed, 41 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95440.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 3c97b9846e2..b99959f76f9 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5862,7 +5862,7 @@ add_candidates (tree fns, tree first_arg, const vec *args,
}
/* Don't bother reversing an operator with two identical parameters. */
- else if (args->length () == 2 && (flags & LOOKUP_REVERSED))
+ else if (vec_safe_length (args) == 2 && (flags & LOOKUP_REVERSED))
{
tree parmlist = TYPE_ARG_TYPES (TREE_TYPE (fn));
if (same_type_p (TREE_VALUE (parmlist),
@@ -10263,7 +10263,7 @@ build_new_method_call_1 (tree instance, tree fns,
vec **args,
&& !(flags & LOOKUP_ONLYCONVERTING)
&& cxx_dialect >= cxx20
&& CP_AGGREGATE_TYPE_P (basetype)
- && !user_args->is_empty ())
+ && !vec_safe_is_empty (user_args))
{
/* Create a CONSTRUCTOR from ARGS, e.g. {1, 2} from <1, 2>. */
tree list = build_tree_list_vec (user_args);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95440.C
b/gcc/testsuite/g++.dg/coroutines/pr95440.C
new file mode 100644
index 000..8542880d1ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95440.C
@@ -0,0 +1,39 @@
+#if __has_include()
+#include
+#else
+#include
+namespace std { using namespace experimental; }
+#endif
+#if 0
+struct suspend_n {
+ const int x;
+ constexpr suspend_n (int x) : x (x) {}
+ constexpr static bool await_ready() { return false; }
+ constexpr static void await_suspend(std::coroutine_handle<>) {}
+ constexpr static void await_resume() {}
+};
+#endif
+struct task
+{
+ struct promise_type
+ {
+auto get_return_object() const { return task{}; }
+#if 0
+//static constexpr suspend_n initial_suspend() { return {2}; }
+#endif
+static constexpr std::suspend_always initial_suspend() { return {}; }
+static constexpr std::suspend_never final_suspend() { return {}; }
+static constexpr void return_void() {}
+static constexpr void unhandled_exception() {}
+ };
+};
+
+task
+test_task ()
+{
+ co_await std::suspend_always{};
+}
+
+auto t = test_task();
+
+int main() {}
--
2.24.1
Re: [PR95416] outputs.exp: skip lto tests when not using linker plugin
Hi Alexandre, Alexandre Oliva wrote: On Jun 9, 2020, Iain Sandoe wrote: I have an ugly patch that makes this work for Darwin (essentially, by having two versions of the LTO tests We could deal with that in a similar way to how .dwo files are handled, namely, with explicit handling in the main test procedure. perhaps; we’d need to figure out the conditions that hold (presumably, as I’ve done in the patch in PR95577, make something conditional on c/l opts). Or, even better, we could introduce an alternate syntax, using a nested list/pair with condition and file, that would require the file only if the condition holds. This could then be used for dwo, for lto, and possibly for other situations. that seems nicer - albeit more work. All that said, I don't object to putting this test machinery to other uses (hey!, it's Free Software! :-), so if you feel that would be useful, let's go for it. Well, I guess I’ve thought for some time that the driver is only tested in a rather ad hoc manner (e.g. there are tests sprinkled here and there for Darwin-specific c/l options and relevant output). In fact, much of the driver’s operation could be verified with the output of -### .. (but not, unfortunately, when there’s a linker-like sub-process). —— Thinking things are a good idea is sadly not the same as having time to address them. short “shopping list” for this .exp: what’s noted above ... .. plus separation of the enumeration and running of the tests so that: * target-specific exclusions / inclusions can be done with dg-* * there’s a way to implement RUNTESTFLAGS=outputs.exp=some-test unless that already works somehow? In the short-term, I think to post/apply the first part of the patch in PR95577 once you’ve put in the change to exclude LTO (otherwise we get 85 fails on Darwin). thanks for adding what’s there - it does cover stuff that had no testing at all! cheers Iain -- Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/ Free Software Evangelist Stallman was right, but he's left :( GNU Toolchain Engineer Live long and free, and prosper ethically
Re: [PATCH, PR fortran/95503] [9/10/11 Regression] ICE in gfc_is_simply_contiguous, at fortran/expr.c:5844
Early ping. > Gesendet: Donnerstag, 04. Juni 2020 um 20:47 Uhr > Von: "Harald Anlauf" > An: "fortran" , "gcc-patches" > Betreff: [PATCH, PR fortran/95503] [9/10/11 Regression] ICE in > gfc_is_simply_contiguous, at fortran/expr.c:5844 > > The following patch fixes an almost obvious ICE in invalid. > > Regtested on x86_64-pc-linux-gnu. > > OK for master, and backports to 9/10? > > Thanks, > Harald > > > PR fortran/95503 - ICE in gfc_is_simply_contiguous, at fortran/expr.c:5844 > > The check for assigning a pointer that cannot be determined to be simply > contiguous at compile time to a contiguous pointer does not need to be > invoked if the lhs of the assignment is known to have conflicting attributes. > > 2020-06-04 Harald Anlauf > > gcc/fortran/ > PR fortran/95503 > * expr.c (gfc_check_pointer_assign): Skip contiguity check of rhs > of pointer assignment if lhs cannot be simply contiguous. > > gcc/testsuite/ > PR fortran/95503 > * gfortran.dg/pr95503.f90: New test. > >
[PATCH] libstdc++: Fix some ranges algos optimizations [PR95578]
ranges::copy and a number of other ranges algorithms have unwrapping
optimizations for iterators of type __normal_iterator, move_iterator and
reverse_iterator. But in the checks that guard these optimizations we
currently only test that the iterator of the iterator/sentinel pair has
the appropriate type before proceeding with the corresponding
optimization, and we fail to also test the sentinel type.
This breaks the testcase in this PR because this testcase constructs
via range adaptors a range whose begin() is a __normal_iterator and
whose end() is a custom sentinel type, and then does ranges::copy on it.
>From there we bogusly perform the __normal_iterator unwrapping
optimization on this iterator/sentinel pair, which immediately leads to
a constraint failure since the custom sentinel type does not model
sentinel_for.
This patch fixes this issue by augmenting each of the problematic checks
to also test that the iterator and sentinel types are the same before
applying the corresponding unwrapping optimization.
Tested on x86_64-pc-linux-gnu, does this look OK to commit to master to
and to the 10.2 branch?
libstdc++-v3/ChangeLog:
PR libstdc++/95578
* include/bits/ranges_algo.h (__lexicographical_compare_fn):
Also check that the iterator and sentinel have the same type before
applying the unwrapping optimization for __normal_iterator.
Split the check into two, one for the first iterator/sentinel
pair and another for second iterator/sentinel pair. Stop using
__niter_base and stop doing std::move on a __normal_iterator.
* include/bits/ranges_algobase.c (__equal_fn): Likewise.
(__copy_or_move): Likewise. Perform similar adjustments for
the reverse_iterator and move_iterator optimizations. Inline
the checks into the if-constexprs, and use using-declarations to
make them less visually noisy. Don't use __niter_wrap.
(__copy_or_move_backward): Likewise.
* testsuite/25_algorithms/copy/95578.cc: New test.
* testsuite/25_algorithms/copy_backward/95578.cc: New test.
* testsuite/25_algorithms/equal/95578.cc: New test.
* testsuite/25_algorithms/lexicographical_compare/95578.cc: New test.
* testsuite/25_algorithms/move/95578.cc: New test.
* testsuite/25_algorithms/move_backward/95578.cc: New test.
---
libstdc++-v3/include/bits/ranges_algo.h | 14 +--
libstdc++-v3/include/bits/ranges_algobase.h | 88 ++-
.../testsuite/25_algorithms/copy/95578.cc | 74
.../25_algorithms/copy_backward/95578.cc | 62 +
.../testsuite/25_algorithms/equal/95578.cc| 74
.../lexicographical_compare/95578.cc | 74
.../testsuite/25_algorithms/move/95578.cc | 62 +
.../25_algorithms/move_backward/95578.cc | 62 +
8 files changed, 465 insertions(+), 45 deletions(-)
create mode 100644 libstdc++-v3/testsuite/25_algorithms/copy/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/copy_backward/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/equal/95578.cc
create mode 100644
libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/move/95578.cc
create mode 100644 libstdc++-v3/testsuite/25_algorithms/move_backward/95578.cc
diff --git a/libstdc++-v3/include/bits/ranges_algo.h
b/libstdc++-v3/include/bits/ranges_algo.h
index c038a505afa..94ca7b6488d 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -3452,11 +3452,15 @@ namespace ranges
_Proj1 __proj1 = {}, _Proj2 __proj2 = {}) const
{
if constexpr (__detail::__is_normal_iterator<_Iter1>
- || __detail::__is_normal_iterator<_Iter2>)
- return (*this)(std::__niter_base(std::move(__first1)),
-std::__niter_base(std::move(__last1)),
-std::__niter_base(std::move(__first2)),
-std::__niter_base(std::move(__last2)),
+ && same_as<_Iter1, _Sent1>)
+ return (*this)(__first1.base(), __last1.base(),
+std::move(__first2), std::move(__last2),
+std::move(__comp),
+std::move(__proj1), std::move(__proj2));
+ else if constexpr (__detail::__is_normal_iterator<_Iter2>
+ && same_as<_Iter2, _Sent2>)
+ return (*this)(std::move(__first1), std::move(__last1),
+__first2.base(), __last2.base(),
std::move(__comp),
std::move(__proj1), std::move(__proj2));
else
diff --git a/libstdc++-v3/include/bits/ranges_algobase.h
b/libstdc++-v3/include/bits/ranges_algobase.h
index 49ca5ed4155..3bdc7cc471b 100644
---
Re: [PATCH 5/6] rs6000, Add vector splat builtin support
Hi! On Wed, Jun 10, 2020 at 09:14:07AM -0700, Carl Love wrote: > On Wed, 2020-06-10 at 10:46 -0500, will schmidt wrote: > > Compare that to the other predicates (config/rs6000/predicates.md) > > > > Those have explicit checks against both ends of the valid range of > > values. i.e. > > > > ;; Return 1 if op is a signed 5-bit constant integer. > > (define_predicate "s5bit_cint_operand" > > (and (match_code "const_int") > >(match_test "INTVAL (op) >= -16 && INTVAL (op) <= 15"))) > > Well, that is what I did originally. But if you see your comment > above, "There probably is a nicer way to write this than with big > decimal numbers." so I was trying to figure out how to do it without > using big numbers. Big *decimal* numbers aren't great. But you could use hex :-) 2147483647 looks like it could be 0x7fff, but so does 2147438647, and that one is 0x7fff5037. > I seemed like shifting the value right 32 bits and > checking if the result was zero would tell us that op fits in 32-bits > but it doesn't seem to work. So, now I have conflicting feedback. :-) For signed you could do ((0x8000 + UINTVAL (op)) >> 32) == 0 (or INTVAL even, makes no difference here), but that is much less readable :-) Maybe for signed it is neatest if you use trunc_int_for_mode for it? INTVAL (op) == INTVAL (trunc_int_for_mode (op, SImode)) (which neatly uses the _target_ SImode). trunc_int_for_mode always sign-extends; we don't have one that zero- extends afaik, but that one is much easier to write anyway: IN_RANGE (UINTVAL (op), 0, 0x) Segher
Re: [PATCH 5/6] rs6000, Add vector splat builtin support
Hi! On Tue, Jun 09, 2020 at 05:01:45PM -0700, Carl Love wrote: > On Fri, 2020-06-05 at 16:28 -0500, Segher Boessenkool wrote: > > > +;; Return 1 if op is a 32-bit constant signed integer > > > +(define_predicate "s32bit_cint_operand" > > > + (and (match_code "const_int") > > > + (match_test "INTVAL (op) >= -2147483648 > > > + && INTVAL (op) <= 2147483647"))) > > > > There probably is a nicer way to write this than with big decimal > > numbers. (I'll not suggest one here because I'll just make a fool of > > myself with overflow or signed/unsigned etc. :-) ) > > > > > +;; Return 1 if op is a constant 32-bit signed or unsigned integer > > > +(define_predicate "c32bit_cint_operand" > > > + (and (match_code "const_int") > > > + (match_test "((INTVAL (op) >> 32) == 0)"))) > > The more I look at the above two they really are the same. Basically, > it boils down to ... can the value signed or unsigned fit in 32-bits or > not? s32bit_cint_operand is testing if it fits in signed 32 bit. c32bit_cint_operand is testing if it fits in unsigned 32 bit (but that is not what its description says). > It seems like both of the above just need to test if the INTVAL > (op) has any bits above bits 0:31 set. So seems like (INTVAL (op) >> > 32) == 0) should be sufficient for both predicates, i.e. replace the > two with a single generic predicate "cint_32bit_operand". That isn't the range for signed 32 bit though. As a 64-bit number, the top half is the sign extension of the bottom half, so the top half is -1 (i.e. all bits set) for negative numbers. A 64-bit number with 0 as the high half but the top bit of the low half set is out of range for a signed 32-bit number. > > > +;; Return 1 if op is a constant 32-bit floating point value > > > +(define_predicate "f32bit_const_operand" > > > + (match_code "const_double")) > > > > Either the predicate name is misleading (if you do allow all > > const_double values), or there should be some test for the alloed > > values > > here. > > The predicate is used to check for a 32-bit float constant. Looking > thru the code not sure if const_double is a 64-bit float? It is. > I don't think > that is what I want. I want a 32-bit floating point value, > const_float, const_real? As a constant number that will still be done as a const_double. You should test its actual value to see if it is a valid single-presicision floating point number. There probably already is a helper for that? > Don't see a a const_float or anything that > looks like that? Not having much luck to see where const_double gets > defined to see what other definitions there are. It is defined in rtl.def: /* numeric floating point or integer constant. If the mode is VOIDmode it is an int otherwise it has a floating point mode and a floating point value. Operands hold the value. They are all 'w' and there may be from 2 to 6; see real.h. */ DEF_RTL_EXPR(CONST_DOUBLE, "const_double", CONST_DOUBLE_FORMAT, RTX_CONST_OBJ) (this is probably more confusing than helpful, but you asked ;-) ) > I am assuming at this point in the compilation process, the constant > that is passed has type info (signed int, unsigned int, float) > associated with it from the front end parsing of the source code. RTL uses modes, not types. What is the mode of the const_double here? If it is SFmode you are fine (but do test for that mode then). If it is DFmode, you need to check its actual value. We want to support *both*, and checking the value works in all cases. (What you need to check is if that floating point value can be exactly expressed as a 32-bit IEEE float, no rounding or truncation etc.) Segher
Re: [PATCH resend] rs6000, pr 94833: fix vec_first_match_index for nulls
Hi Carl, On Wed, Jun 10, 2020 at 09:05:36AM -0700, Carl Love wrote: > I committed this patch to mainline and backported to GCC 9. > > I have looked at GCC 8. The functional issue is there, i.e. the > vcmpnez is used instead of vcmpne. However the test case > builtins-8-p9-runnable.c does not exist in GCC 8. The patch consists > of the functional fix: > > --- a/gcc/config/rs6000/vsx.md > +++ b/gcc/config/rs6000/vsx.md > @@ -4803,8 +4803,8 @@ >rtx cmp_result = gen_reg_rtx (mode); >rtx not_result = gen_reg_rtx (mode); > > - emit_insn (gen_vcmpnez (cmp_result, operands[1], > -operands[2])); > + emit_insn (gen_vcmpne (cmp_result, operands[1], > + operands[2])); >emit_insn (gen_one_cmpl2 (not_result, cmp_result)); > >sh = GET_MODE_SIZE (GET_MODE_INNER (mode)) / 2; > > So, I am a bit unsure how to proceed. I think we need the functional > change. But without applying the test cases fixes I don't feel that I > am really backporting the patch as approved for backporting. > > I think we could reference the mainline commit, as we always do when > backporting, but then note the test case fix was not included since the > testcase does not exist. Would that be OK? Yes, that is fine, this fix is obvious enough :-) You could also include the whole testcase from trunk (or 9), but that can be quite a bit of (testing) work, and is it worth it at all. > Please let me know how you would like me to handle this issue. Either way is okay (I'd go for just the 2-line patch). Thanks, Segher
Re: [PR95416] outputs.exp: skip lto tests when not using linker plugin
On Jun 9, 2020, Iain Sandoe wrote: > That means that the intermediate objects proceed all the way to .s > output - and thus the ‘final’ pass is run (producing the extra files seen). > You can mimic this with x86 Linux by appending -ffat-lto-objects to an > LTO command line. I see, thanks. > I have an ugly patch that makes this work for Darwin (essentially, by > having two versions of the LTO tests We could deal with that in a similar way to how .dwo files are handled, namely, with explicit handling in the main test procedure. Or, even better, we could introduce an alternate syntax, using a nested list/pair with condition and file, that would require the file only if the condition holds. This could then be used for dwo, for lto, and possibly for other situations. > and I was wondering (as an aside) if the -ffat-lto-objects case should > be tested on targets which default to thin LTO anyway? The purpose of this testset was to check the new aux and dump file naming was implemented correctly. I don't see that -ffat-lto-objects adds to that: it just runs the compilation phase of an lto build all the way to the end and thus creating dump files for passes that would otherwise be skipped. Alas, some of the naming logic is only exercised when certain features are present/enabled, so expanding the testset to cover those when required conditions are met is in line with the original purpose. Thus dwo conditionals, lto with or without plugin, (missing) offloading target testing, ... [insert other unforeseen issues here] :-) All that said, I don't object to putting this test machinery to other uses (hey!, it's Free Software! :-), so if you feel that would be useful, let's go for it. -- Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/ Free Software Evangelist Stallman was right, but he's left :( GNU Toolchain Engineer Live long and free, and prosper ethically
Re: [PR95416] outputs.exp: skip lto tests when not using linker plugin
On Jun 9, 2020, Rainer Orth wrote:
> this is wrong unfortunately: braces are the Tcl equivalent of single
> quotes so you're setting skip_lto to the string inside.
Aah, thanks. So when $skip_lto is expanded in the ifs, the whole thing
is evaluated, and that's why it works anyway?
> While this worked for me on i386-pc-solaris2.11 (both with ld, thus
> without the lto plugin, and with gld and the plugin), I wonder if
> there's not a better way than just skipping the lto tests,
My initial focus was on relieving the pain by removing the symptoms, so
I can then work on the improvements with lower urgency.
Here's what I'd like to check in for now:
[PR95416] outputs.exp: skip lto tests when not using linker plugin
From: Alexandre Oliva
When the linker plugin is not available, dump outputs in lto runs are
different from those outputs.exp tests expect, so skip them for now.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/testsuite/ChangeLog
* outputs.exp (skip_lto): Set when missing the linker plugin.
---
testsuite/gcc.misc-tests/outputs.exp |4
1 file changed, 4 insertions(+)
diff --git gcc/testsuite/gcc.misc-tests/outputs.exp
gcc/testsuite/gcc.misc-tests/outputs.exp
index 06a32db..8e05c92 100644
--- gcc/testsuite/gcc.misc-tests/outputs.exp
+++ gcc/testsuite/gcc.misc-tests/outputs.exp
@@ -43,6 +43,10 @@ if ![check_no_compiler_messages gsplitdwarf object {
# Check for -flto support. We explicitly test the result to skip
# tests that use -flto.
set skip_lto ![check_effective_target_lto]
+if !$skip_lto {
+ # LTO tests' expectations assume a linker plugin ATM.
+ set skip_lto ![check_linker_plugin_available]
+}
# Prepare additional options to be used for linking.
# We do not compile to an executable, because that requires naming an output.
--
Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/
Free Software Evangelist Stallman was right, but he's left :(
GNU Toolchain Engineer Live long and free, and prosper ethically
Re: libstdc++: Extend memcmp optimization in std::lexicographical_compare
On 10/06/20 18:40 +0200, François Dumont wrote: On 10/06/20 4:49 pm, Jonathan Wakely wrote: On 10/06/20 08:18 +0200, François Dumont via Libstdc++ wrote: On 09/06/20 10:53 pm, Jonathan Wakely wrote: This reminds me that I was going to extend the condition for using memcmp to also apply to unsigned integers with sizeof(T) > 1 on big endian targets. This illustrates what I tried to avoid in my original patch, the code duplication. I was calling __lexicographical_compare_aux1 because I didn't want to duplicate the code to compute __simple. Of course it can be expose as a template using. Not for C++98. I'm not very concerned about duplicating the boolean condition. I definitely prefer that to codegen changes that affect every user of lexicographical_compare just to benefit the handful of people using it with std::deque. If __lexicographical_compare_aux1 could be reused without changes, great, but it needed changes with consequences for more code than just deque iterators. That's fine with me, the patch looks good. Do you want me to get rid of the enable_if usage before the commit ? Otherwise I let you commit it ? I've pushed it to master now.
Re: [PATCH] Testsuite: Mark check_effective_target_exceptions_enabled test as C++ test input.
Tamar Christina writes: > Hi All, > > The test in check_effective_target_exceptions_enabled uses a C++ keyword > `throw` > and the test fails with a syntax error on any non-g++ test. I now tell the > testsuite driver that this is a C++ input file so it runs it as such in all > the > drivers. Nice! Seen that message a few times but never quite drummed up the enthusiasm to fix it. > Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? OK, thanks. Richard > > Thanks, > Tamar > > gcc/testsuite/ChangeLog: > > * lib/target-supports.exp (check_effective_target_exceptions_enabled): > Mark as C++ test input.
Re: [PATCH] AArch64: Don't check for amdgcn-amdhsa at all on arm targets.
On Wed, Jun 10, 2020 at 9:57 AM Tamar Christina wrote: > > Hi All, > > The amdgcn-amdhsa test seems to be running for all targets unconditionally > while > only really makes sense for certain targets. This patch adds an opt-out list > and opts out arm targets. > > Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? I don't think this is right. It does make sense to have aarch64 to support it as an offload; though someone needs to test out the GPU card on an ARM64 server. Maybe checking for *-*-elf *-*-eabi and returning false for those targets should be enough for most embedded targets. Thanks, Andrew Pinski > > Thanks, > Tamar > > gcc/testsuite/ChangeLog: > > * lib/target-supports.exp (check_effective_target_offload_gcn): Skip > for > arm targets. > > --
[PATCH] Testsuite: Mark check_effective_target_exceptions_enabled test as C++ test input.
Hi All,
The test in check_effective_target_exceptions_enabled uses a C++ keyword `throw`
and the test fails with a syntax error on any non-g++ test. I now tell the
testsuite driver that this is a C++ input file so it runs it as such in all the
drivers.
Regtested on aarch64-none-linux-gnu and no issues.
Ok for master?
Thanks,
Tamar
gcc/testsuite/ChangeLog:
* lib/target-supports.exp (check_effective_target_exceptions_enabled):
Mark as C++ test input.
--
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index b4da4ddf86063288abd353569c4dc7c75ce326f0..a9a57ce5518cfe29e3888c28011cb6f84f18e75f 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9518,6 +9518,7 @@ proc check_effective_target_exceptions_enabled {} {
return [check_cached_effective_target exceptions_enabled {
if { [check_effective_target_exceptions] } {
return [check_no_compiler_messages exceptions_enabled assembly {
+ // C++
void foo (void)
{
throw 1;
Re: [PATCH 4/4] vect: Factor out and rename some functions/macros
"Kewen.Lin" writes:
> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
> index ca68d04a919..1fac5898525 100644
> --- a/gcc/tree-vect-loop-manip.c
> +++ b/gcc/tree-vect-loop-manip.c
> @@ -420,8 +420,8 @@ vect_set_loop_controls_directly (class loop *loop,
> loop_vec_info loop_vinfo,
>rgroup_controls *rgc, tree niters,
>tree niters_skip, bool might_wrap_p)
> {
> - tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo);
> - tree iv_type = LOOP_VINFO_MASK_IV_TYPE (loop_vinfo);
> + tree compare_type = LOOP_VINFO_COMPARE_TYPE (loop_vinfo);
> + tree iv_type = LOOP_VINFO_IV_TYPE (loop_vinfo);
How about s/MASK/RGROUP/ instead? COMPARE_TYPE and IV_TYPE sound a bit
too generic, and might give the impression that they're meaningful for
classic full-vector vectorisation too.
> @@ -748,13 +748,12 @@ vect_set_loop_condition_masked (class loop *loop,
> loop_vec_info loop_vinfo,
> }
>
> /* Like vect_set_loop_condition, but handle the case in which there
> - are no loop masks. */
> + are no partial vectorization loops. */
Maybe:
… in which the vector loop handles exactly VF scalars per iteration.
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 7ea75d6d095..b6e96f77f69 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -155,6 +155,7 @@ along with GCC; see the file COPYING3. If not see
> static void vect_estimate_min_profitable_iters (loop_vec_info, int *, int *);
> static stmt_vec_info vect_is_simple_reduction (loop_vec_info, stmt_vec_info,
> bool *, bool *);
> +static bool known_niters_smaller_than_vf (loop_vec_info);
Please instead define the function before its first caller.
Adding “vect_” to the beginning of the name would probably be
more consistent.
> [...]
> @@ -959,14 +960,41 @@ vect_get_max_nscalars_per_iter (loop_vec_info
> loop_vinfo)
>return res;
> }
>
> +/* Calculate the minimal bits necessary to represent the maximal iteration
> + count of loop with loop_vec_info LOOP_VINFO which is scaling with a given
> + factor FACTOR. */
How about:
/* Calculate the minimum precision necessary to represent:
MAX_NITERS * FACTOR
as an unsigned integer, where MAX_NITERS is the maximum number of
loop header iterations for the original scalar form of LOOP_VINFO. */
> +
> +static unsigned
> +min_prec_for_max_niters (loop_vec_info loop_vinfo, unsigned int factor)
Here too I think a “vect_” prefix would be more consistent.
> +{
> + class loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> +
> + /* Get the maximum number of iterations that is representable
> + in the counter type. */
> + tree ni_type = TREE_TYPE (LOOP_VINFO_NITERSM1 (loop_vinfo));
> + widest_int max_ni = wi::to_widest (TYPE_MAX_VALUE (ni_type)) + 1;
> +
> + /* Get a more refined estimate for the number of iterations. */
> + widest_int max_back_edges;
> + if (max_loop_iterations (loop, _back_edges))
> +max_ni = wi::smin (max_ni, max_back_edges + 1);
> +
> + /* Account for factor, in which each bit is replicated N times. */
The “, in which each bit …” part no longer makes sense in this generic
context. Probably best just to drop the comment altogether and…
> + max_ni *= factor;
> +
> + /* Work out how many bits we need to represent the limit. */
> + unsigned int min_ni_width = wi::min_precision (max_ni, UNSIGNED);
> +
> + return min_ni_width;
…change this to:
/* Work out how many bits we need to represent the limit. */
return wi::min_precision (max_ni * factor, UNSIGNED);
> [...]
> @@ -6813,8 +6820,8 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
> {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> - "can't use a fully-masked loop because no"
> - " conditional operation is available.\n");
> + "can't use a partial vectorization loop because"
> + " no conditional operation is available.\n");
Maybe “can't operate on partial vectors because…”. Same for the
later changes.
> @@ -9194,12 +9202,13 @@ optimize_mask_stores (class loop *loop)
> }
>
> /* Decide whether it is possible to use a zero-based induction variable
> - when vectorizing LOOP_VINFO with a fully-masked loop. If it is,
> - return the value that the induction variable must be able to hold
> - in order to ensure that the loop ends with an all-false mask.
> + when vectorizing LOOP_VINFO with a partial vectorization loop. If
Maybe ”…with partial vectors”
> + it is, return the value that the induction variable must be able to
> + hold in order to ensure that the loop ends with an all-false rgroup
> + control like mask.
> Return -1 otherwise. */
This was originally meant to be a single paragraph, so I think it reads
better if the ”Return -1 otherwise.” is on the
[PATCH] AArch64: Don't check for amdgcn-amdhsa at all on arm targets.
Hi All,
The amdgcn-amdhsa test seems to be running for all targets unconditionally while
only really makes sense for certain targets. This patch adds an opt-out list
and opts out arm targets.
Regtested on aarch64-none-linux-gnu and no issues.
Ok for master?
Thanks,
Tamar
gcc/testsuite/ChangeLog:
* lib/target-supports.exp (check_effective_target_offload_gcn): Skip for
arm targets.
--
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index b335108358d0c878193ce07771b69933f3ec4d26..b4da4ddf86063288abd353569c4dc7c75ce326f0 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9787,6 +9787,10 @@ proc check_effective_target_offload_hsa { } {
# Return 1 if the compiler has been configured with hsa offloading.
proc check_effective_target_offload_gcn { } {
+if { [istarget aarch64*-*-*] || [istarget arm*-*-*] } {
+ return 0
+}
+
return [check_no_compiler_messages offload_gcn assembly {
int main () {return 0;}
} "-foffload=amdgcn-amdhsa" ]
Re: libstdc++: Extend memcmp optimization in std::lexicographical_compare
On 10/06/20 4:49 pm, Jonathan Wakely wrote: On 10/06/20 08:18 +0200, François Dumont via Libstdc++ wrote: On 09/06/20 10:53 pm, Jonathan Wakely wrote: This reminds me that I was going to extend the condition for using memcmp to also apply to unsigned integers with sizeof(T) > 1 on big endian targets. This illustrates what I tried to avoid in my original patch, the code duplication. I was calling __lexicographical_compare_aux1 because I didn't want to duplicate the code to compute __simple. Of course it can be expose as a template using. Not for C++98. I'm not very concerned about duplicating the boolean condition. I definitely prefer that to codegen changes that affect every user of lexicographical_compare just to benefit the handful of people using it with std::deque. If __lexicographical_compare_aux1 could be reused without changes, great, but it needed changes with consequences for more code than just deque iterators. That's fine with me, the patch looks good. Do you want me to get rid of the enable_if usage before the commit ? Otherwise I let you commit it ?
Re: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning
"Yangfei (Felix)" writes: > Hi, > > PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95570 > > Here, we are doing loop versioning for alignment. The only dr here is a > gather-statter operation: x[start][i]. > Scalar evolution analysis for this dr failed, so DR_STEP is NULL_TREE, which > leads to the segfault. > But scatter-gather operation should be filtered out in > vect_enhance_data_refs_alignment. > There are similar issues in vect_verify_datarefs_alignment, > vect_get_peeling_costs_all_drs and vect_peeling_supportable. > Proposed patch adds back the necessary tests. Bootstrapped and tested on > aarch64-linux-gnu & x86_64-linux-gnu. > > Test coverage: > Existing tests [1] and newly added test ensures coverage for all the changes > except for the changes in vect_peeling_supportable. > Currently I don't have a test to cover the changes in > vect_peeling_supportable. Should we keep them? Rather than add several instances of the new test, I think it would make sense to split the (hopefully) correct conditions in vect_enhance_data_refs_alignment out into a subroutine and use it in the other sites. Doing that for vect_peeling_supportable would then be justifiable as a clean-up. How about something like vect_relevant_for_alignment_p as the name of the subroutine? Thanks, Richard
Re: [PATCH] wwwdocs: Document devel/omp/gcc-10 branch
The inactive development branches are sorted alphabetically except for the openacc-gcc-*-branch entries, which are grouped with the gomp*-branch entries. Would it be better to place devel/omp/gcc-9 alphabetically (in which case it will go between debuglocus and dwarf4), or by topic (in which case it should probably go after openacc-gcc-9-branch)? Kwok On 10/06/2020 4:15 pm, Tobias Burnus wrote: On 6/10/20 3:34 PM, Kwok Cheung Yeung wrote: This patch updates the previous entry on the website for devel/omp/gcc-9 to refer to the new branch instead. This removes references to the old branch, as new development should occur on the newer branch. Can you move the old entry to "Inactive Development Branches" instead?
Re: 1-800-GIT-HELP
On 6/10/20 12:00 PM, Andreas Schwab wrote: On Jun 10 2020, Nathan Sidwell wrote: needed 'origin' -- eg 'git merge origin master' That's an octopus merge. I don't think you want that. ah. Somehow I convinced myself that was how to merge from master, but I guess my formulation resulted in one of those octopus arms either being a nop (origin/branch/HEAD?) or the same as the other (origin/master/HEAD?). Given how my attempt at picking a point on master went, I speculate the latter? nathan -- Nathan Sidwell
RE: [PATCH 5/6] rs6000, Add vector splat builtin support
On Wed, 2020-06-10 at 10:46 -0500, will schmidt wrote: > > On Fri, 2020-06-05 at 16:28 -0500, Segher Boessenkool wrote: > > > > +;; Return 1 if op is a 32-bit constant signed integer > > > > +(define_predicate "s32bit_cint_operand" > > > > + (and (match_code "const_int") > > > > + (match_test "INTVAL (op) >= -2147483648 > > > > + && INTVAL (op) <= 2147483647"))) > > > > > > There probably is a nicer way to write this than with big decimal > > > numbers. (I'll not suggest one here because I'll just make a > > > fool > > > of > > > myself with overflow or signed/unsigned etc. :-) ) > > > > > > > +;; Return 1 if op is a constant 32-bit signed or unsigned > > > > integer > > > > +(define_predicate "c32bit_cint_operand" > > > > + (and (match_code "const_int") > > > > + (match_test "((INTVAL (op) >> 32) == 0)"))) > > > > The more I look at the above two they really are the > > same. Basically, > > it boils down to ... can the value signed or unsigned fit in 32- > > bits > > or > > not? It seems like both of the above just need to test if the > > INTVAL > > (op) has any bits above bits 0:31 set. So seems like (INTVAL (op) > > >> > > 32) == 0) should be sufficient for both predicates, i.e. replace > > the > > two with a single generic predicate "cint_32bit_operand". > > > > For starters, I tried changing the definition for > > s32bit_cint_operand > > to: > > > > ; Return 1 if op is a 32-bit constant signed > > integer > > (define_predicate > > "s32bit_cint_operand" > > (and (match_code > > "const_int") > >(match_test "((INTVAL (op) >> 32) == 0)"))) > > > Compare that to the other predicates (config/rs6000/predicates.md) > > Those have explicit checks against both ends of the valid range of > values. i.e. > > ;; Return 1 if op is a signed 5-bit constant integer. > (define_predicate "s5bit_cint_operand" > (and (match_code "const_int") >(match_test "INTVAL (op) >= -16 && INTVAL (op) <= 15"))) Well, that is what I did originally. But if you see your comment above, "There probably is a nicer way to write this than with big decimal numbers." so I was trying to figure out how to do it without using big numbers. I seemed like shifting the value right 32 bits and checking if the result was zero would tell us that op fits in 32-bits but it doesn't seem to work. So, now I have conflicting feedback. :-) Carl
Re: [PATCH] diagnostics: Add options to control the column units [PR49973] [PR86904]
On Fri, 2020-05-08 at 15:35 -0400, Lewis Hyatt wrote: > On Fri, Jan 31, 2020 at 03:31:59PM -0500, David Malcolm wrote: > > On Fri, 2020-01-31 at 14:31 -0500, Lewis Hyatt wrote: > > > Hello- > > > > > > Here is the second patch that I mentioned when I submitted the > > > other > > > related > > > patch (which is awaiting review): > > > https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01626.html. > > > > Sorry about that; I'm v. busy with analyzer bugs right now. > > > > > This second patch > > > is based on top of the first one and it closes out PR49973 and > > > PR86904 by > > > adding the new option -fdiagnostics-column-unit=[display|byte]. > > > This > > > allows > > > to specify whether columns are output as simple byte counts (the > > > current > > > behavior), or as display columns including handling multibyte > > > characters and > > > tabs. The patch makes display columns the new default. > > > Additionally, > > > a > > > second new option -fdiagnostics-column-origin is added, which > > > allows > > > to make > > > the column 0-based (or N-based for any N) instead of 1-based. The > > > default > > > remains at 1-based as it is now. > > > > > > A number of testcases were explicitly testing for the old > > > behavior, > > > so I > > > have updated them to test for the new behavior instead, since the > > > column > > > number adjusted for tabs is more natural to test for, and matches > > > what > > > editors typically show (give or take 1 for the origin > > > convention). > > > > > > One other testcase (go.dg/arrayclear.go) was a bit of an oddity. > > > It > > > failed > > > after this patch, although it doesn't test for any column > > > numbers. > > > The > > > answer turned out to be, this test checks for identical error > > > text on > > > two > > > different lines. When the column units are changed to display > > > columns, then > > > the column of the second error happens to match the line of the > > > first > > > one. dejagnu then misinterprets the second error as if it matched > > > the > > > location of the first one (it doesn't distinguish whether it > > > checks > > > for the > > > line number or the column number in the output). I added a > > > comment to > > > the > > > test explaining the situation; since adding the comment has the > > > side > > > effect > > > of making the first line number no longer match the second column > > > number, it > > > also makes the test pass again. > > > > > > It wasn't quite clear to me whether this change was appropriate > > > for > > > GCC 10 > > > or not at this point. We discussed it a couple months ago here: > > > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02171.html. Either > > > way, > > > I hope > > > it isn't a problem that I submitted the patch for review now, > > > whether > > > it > > > will end up in 10 or 11. Please let me know what's normally > > > expected? > > > Thanks! > > > > Thanks Lewis. > > > > This patch looks very promising, but should wait until gcc 11; > > we're > > trying to stabilize gcc 10 right now (I'm knee-deep in analyzer > > bug- > > fixing, so I don't want to add any more diagnostics changes). > > > > Hi Dave- > > Well GCC 10 was released for a whole day so I thought I would bug you > with this > patch again now :). To summarize, I previously sent this in two > separate parts. > > Part 1: > https://gcc.gnu.org/legacy-ml/gcc-patches/2020-01/msg01626.html > Part 2: > https://gcc.gnu.org/legacy-ml/gcc-patches/2020-01/msg02108.html > > Part 1 added the support for converting tabs to spaces when > outputting > diagnostics. Part 2 added the new options -fdiagnostics-column-unit > and > -fdiagnostics-column-origin to control whether the column number is > printed > in display or byte units. Together they resolve both PR49973 and > PR86904. > > You provided me with feedback on part 2, which is quoted below with > some > notes interspersed. The new version of the patch incorporates all of > your > suggestions. Part 1 has not changed other than some trivial rebasing > conflicts. The two patches touch nearly disjoint sets of files and > are > logically linked together, so I thought it would be simpler if I just > sent > one combined patch now. If you prefer them to be separated as before, > please > let me know and I can send them that way as well. > > Bootstrap and reg tests were done on x86-64 Linux for all > languages. Tests > look good: > > type, before, after > FAIL 96 96 > PASS 474637 475097 > UNSUPPORTED 11607 11607 > UNTESTED 195 195 > XFAIL 1816 1816 > XPASS 36 362 Thanks for the patch; sorry about the delay in reviewing it. Some high-level review points - I like the patch overall - This will deserve an item in the release notes - I don't like adding "global_tabstop" (I don't like global variables). Is there nowhere else we can handle this? I believe there's a cluster of functions in the callgraph that make use of it; can we simply pass around the tabstop value instead? "tabstop" seems to have
[PATCH] avoid stmt-info allocation for debug stmts
The following avoids allocating stmt info structs for debug stmts.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
2020-06-10 Richard Biener
* tree-vect-loop.c (vect_determine_vectorization_factor):
Skip debug stmts.
(_loop_vec_info::_loop_vec_info): Likewise.
(vect_update_vf_for_slp): Likewise.
(vect_analyze_loop_operations): Likewise.
(update_epilogue_loop_vinfo): Likewise.
* tree-vect-patterns.c (vect_determine_precisions): Likewise.
(vect_pattern_recog): Likewise.
* tree-vect-slp.c (vect_detect_hybrid_slp): Likewise.
(_bb_vec_info::_bb_vec_info): Likewise.
* tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized):
Likewise.
---
gcc/tree-vect-loop.c | 13 -
gcc/tree-vect-patterns.c | 9 ++---
gcc/tree-vect-slp.c | 4
gcc/tree-vect-stmts.c| 2 ++
4 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index f0b33258ac5..53def19a4fb 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -342,6 +342,8 @@ vect_determine_vectorization_factor (loop_vec_info
loop_vinfo)
for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si);
gsi_next ())
{
+ if (is_gimple_debug (gsi_stmt (si)))
+ continue;
stmt_info = loop_vinfo->lookup_stmt (gsi_stmt (si));
opt_result res
= vect_determine_vf_for_stmt (loop_vinfo,
@@ -847,6 +849,8 @@ _loop_vec_info::_loop_vec_info (class loop *loop_in,
vec_info_shared *shared)
{
gimple *stmt = gsi_stmt (si);
gimple_set_uid (stmt, 0);
+ if (is_gimple_debug (stmt))
+ continue;
add_stmt (stmt);
/* If .GOMP_SIMD_LANE call for the current loop has 3 arguments, the
third argument is the #pragma omp simd if (x) condition, when 0,
@@ -1393,6 +1397,8 @@ vect_update_vf_for_slp (loop_vec_info loop_vinfo)
for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si);
gsi_next ())
{
+ if (is_gimple_debug (gsi_stmt (si)))
+ continue;
stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (gsi_stmt (si));
stmt_info = vect_stmt_to_vectorize (stmt_info);
if ((STMT_VINFO_RELEVANT_P (stmt_info)
@@ -1584,7 +1590,8 @@ vect_analyze_loop_operations (loop_vec_info loop_vinfo)
gsi_next ())
{
gimple *stmt = gsi_stmt (si);
- if (!gimple_clobber_p (stmt))
+ if (!gimple_clobber_p (stmt)
+ && !is_gimple_debug (stmt))
{
opt_result res
= vect_analyze_stmt (loop_vinfo,
@@ -2345,6 +2352,8 @@ again:
for (gimple_stmt_iterator si = gsi_start_bb (bb);
!gsi_end_p (si); gsi_next ())
{
+ if (is_gimple_debug (gsi_stmt (si)))
+ continue;
stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (gsi_stmt (si));
STMT_SLP_TYPE (stmt_info) = loop_vect;
if (STMT_VINFO_IN_PATTERN_P (stmt_info))
@@ -8373,6 +8382,8 @@ update_epilogue_loop_vinfo (class loop *epilogue, tree
advance)
!gsi_end_p (epilogue_gsi); gsi_next (_gsi))
{
new_stmt = gsi_stmt (epilogue_gsi);
+ if (is_gimple_debug (new_stmt))
+ continue;
gcc_assert (gimple_uid (new_stmt) > 0);
stmt_vinfo
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 930f47e0742..636ad59c001 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -5112,8 +5112,9 @@ vect_determine_precisions (vec_info *vinfo)
basic_block bb = bbs[nbbs - i - 1];
for (gimple_stmt_iterator si = gsi_last_bb (bb);
!gsi_end_p (si); gsi_prev ())
- vect_determine_stmt_precisions
- (vinfo, vinfo->lookup_stmt (gsi_stmt (si)));
+ if (!is_gimple_debug (gsi_stmt (si)))
+ vect_determine_stmt_precisions
+ (vinfo, vinfo->lookup_stmt (gsi_stmt (si)));
}
}
else
@@ -5478,6 +5479,8 @@ vect_pattern_recog (vec_info *vinfo)
basic_block bb = bbs[i];
for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next ())
{
+ if (is_gimple_debug (gsi_stmt (si)))
+ continue;
stmt_vec_info stmt_info = vinfo->lookup_stmt (gsi_stmt (si));
/* Scan over all generic vect_recog_xxx_pattern functions. */
for (j = 0; j < NUM_PATTERNS; j++)
@@ -5494,7 +5497,7 @@ vect_pattern_recog (vec_info *vinfo)
{
gimple *stmt = gsi_stmt (si);
stmt_vec_info stmt_info = bb_vinfo->lookup_stmt (stmt);
- if (stmt_info && !STMT_VINFO_VECTORIZABLE (stmt_info))
+ if (!stmt_info || !STMT_VINFO_VECTORIZABLE (stmt_info))
continue;
/* Scan over all generic vect_recog_xxx_pattern functions. */
diff --git
[PATCH] tree-optimization/95576 - fix compare-debug issue with SLP vectorization
The following avoids leading debug stmts in BB vectorizer regions.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
2020-06-10 Richard Biener
PR tree-optimization/95576
* tree-vect-slp.c (vect_slp_bb): Skip leading debug stmts.
* g++.dg/vect/pr95576.cc: New testcase.
---
gcc/testsuite/g++.dg/vect/pr95576.cc | 23 +++
gcc/tree-vect-slp.c | 9 -
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/vect/pr95576.cc
diff --git a/gcc/testsuite/g++.dg/vect/pr95576.cc
b/gcc/testsuite/g++.dg/vect/pr95576.cc
new file mode 100644
index 000..64e0e2dae7f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr95576.cc
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-additional-options "-O3 -fno-tree-forwprop -fcompare-debug" }
+
+struct S {
+ virtual ~S();
+ struct S *s;
+ virtual void m();
+ int f;
+ void *d;
+};
+
+struct T : S {
+ void m();
+};
+
+S::~S() {
+ if (s) {
+s->f = 0;
+s->d = __null;
+ }
+}
+
+void T::m() {}
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 0217a524f05..866ce8d3717 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -3316,7 +3316,12 @@ vect_slp_bb (basic_block bb)
{
gimple *stmt = gsi_stmt (gsi);
if (is_gimple_debug (stmt))
- continue;
+ {
+ /* Skip leading debug stmts. */
+ if (gsi_stmt (region_begin) == stmt)
+ gsi_next (_begin);
+ continue;
+ }
insns++;
if (gimple_location (stmt) != UNKNOWN_LOCATION)
@@ -3325,6 +3330,8 @@ vect_slp_bb (basic_block bb)
if (!vect_find_stmt_data_reference (NULL, stmt, ))
break;
}
+ if (gsi_end_p (region_begin))
+ break;
/* Skip leading unhandled stmts. */
if (gsi_stmt (region_begin) == gsi_stmt (gsi))
--
2.25.1
RE: [PATCH resend] rs6000, pr 94833: fix vec_first_match_index for nulls
Segher, Bill: I committed this patch to mainline and backported to GCC 9. I have looked at GCC 8. The functional issue is there, i.e. the vcmpnez is used instead of vcmpne. However the test case builtins-8-p9-runnable.c does not exist in GCC 8. The patch consists of the functional fix: --- a/gcc/config/rs6000/vsx.md +++ b/gcc/config/rs6000/vsx.md @@ -4803,8 +4803,8 @@ rtx cmp_result = gen_reg_rtx (mode); rtx not_result = gen_reg_rtx (mode); - emit_insn (gen_vcmpnez (cmp_result, operands[1], -operands[2])); + emit_insn (gen_vcmpne (cmp_result, operands[1], + operands[2])); emit_insn (gen_one_cmpl2 (not_result, cmp_result)); sh = GET_MODE_SIZE (GET_MODE_INNER (mode)) / 2; So, I am a bit unsure how to proceed. I think we need the functional change. But without applying the test cases fixes I don't feel that I am really backporting the patch as approved for backporting. I think we could reference the mainline commit, as we always do when backporting, but then note the test case fix was not included since the testcase does not exist. Would that be OK? Please let me know how you would like me to handle this issue. Carl Love On Thu, 2020-05-14 at 11:53 -0500, Segher Boessenkool wrote: > Hi! > > On Wed, May 13, 2020 at 10:14:24AM -0700, Carl Love wrote: > > The following patch fixes PR94833, vec_first_match_index does not > > function as described in its description. > > > > The builtin does not handle vector elements which are zero > > correctly. > > The following patch fixes the issue and adds additional test cases > > to > > verify the vec_first_match_index builtin and related builtins work > > correctly with elements that are zero. > > 2020-04-30 Carl Love > > > > PR target/94833 > > * config/rs6000/vsx.md (define_expand): Fix instruction > > generation for > > first_match_index_. > > * testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c (main): > > Add > > additional test cases with zero vector elements. > > Okay for trunk and all backports needed after a while. Thanks! > > > Segher
Re: [PATCH PR95523] aarch64:ICE in register_tuple_type，at config/aarch64/aarch64-sve-builtins.cc:3434
Hi,
"Zhanghaijian (A)" writes:
> This is a simple fix for pr95523.
> When registering the tuple type in register_tuple_type, the TYPE_ALIGN
> (tuple_type) will be changed by -fpack-struct=n.
> We need to maintain natural alignment in handle_arm_sve_h.
> Bootstrap and tested on aarch64 Linux platform. No new regression witnessed.
Sorry for dropping the ball on the bugzilla PR and not replying
to your comment there. For the record...
IMO it's a misfeature that -fpack-struct=N and “#pragma pack (1)”
override explicit alignments. E.g. it means that for:
struct __attribute__((packed)) s1 { __attribute__((aligned(2))) int x; };
#pragma pack (1)
struct s2 { __attribute__((aligned(2))) int x; };
s1 has alignment 2 but s2 has alignment 1. There's a comment about
this in stor-layout.c:
/* Should this be controlled by DECL_USER_ALIGN, too? */
if (mfa != 0)
SET_DECL_ALIGN (decl, MIN (DECL_ALIGN (decl), mfa));
I think the condition probably should have checked DECL_USER_ALIGN.
However, there's no telling whether anything now relies on the current
behaviour, and since this bug is about internally-defined structures,
it's probably not a good motivating example for changing the code above.
Also, I agree the patch is a clean way of avoiding the problem,
and is probably more robust than the DECL_USER_ALIGN thing would
have been.
Pushed to master, thanks.
Richard
Re: 1-800-GIT-HELP
On Jun 10 2020, Nathan Sidwell wrote: > needed 'origin' -- eg 'git merge origin master' That's an octopus merge. I don't think you want that. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: Automatic vs. manual 'ChangeLog' files updates for devel/ branches etc. (was: [PATCH] wwwdocs: Document devel/omp/gcc-10 branch)
On Wed, Jun 10, 2020 at 05:47:54PM +0200, Thomas Schwinge wrote: > The automated system runs once a day. I guess it's generally deemed > acceptable if the source code state (commits) and the 'ChangeLog' files > state is out of sync for one day. There surely must be some mechanism in > place to make sure that actual GCC tarball etc. releases do have complete > 'ChangeLog' files state. Martin has provided a way to generate patches for all the standard branches instead of checking them in. I think it would be really much better to have a way to ask either for a patch on stdout for the HEAD (i.e. only on that branch find last DATESTAMP bump and generate ChangeLog entries since then) or just apply the changes to the ChangeLog directly. At least that is something I'd prefer for gcc_release and e.g. the distro snapshots. For the topic branches, there is the additional problem that it is (semi)-regularly synced from mainline or some release branch and thus is getting the DATESTAMP update commits which are now normally used as the anchors for ChangeLog generation. The commits that are not from the tracked branch can be intermixed with the inherited commits though and so the question is what should we generate and into which files. Jakub
Re: 1-800-GIT-HELP
On Wed, Jun 10, 2020 at 8:38 AM Nathan Sidwell wrote: > > I'm trying to merge master to devel/c++-modules, as I usually do. But > there's an ICE creeping in, so I'm attempting incremental merges to > figure out what happened. > > I thought 'git merge --no-commit origin $HASH' would do that, pulling > $HASH out of master's commit history (git tree --first-parent). But > this doesn;t seem to be working. In particular I thought that if $HASH > matched the last master commit that I merged from, the result would be a > nop. It isn't :( > > How do I update my branch to some non-HEAD point on master? I have been using git to maintain non-trivial changes over a long period of time. These are what I do 1. Break my changes into as many small pieces as possible. Each commit should be built without any regressions. 2. Use "git rebase commit-on-master" to sync with commits on master on a daily/weekly/monthly basis. Small pieces make rebasing much easier. -- H.J.
Re: 1-800-GIT-HELP
On 6/10/20 11:38 AM, Nathan Sidwell wrote: I'm trying to merge master to devel/c++-modules, as I usually do. But there's an ICE creeping in, so I'm attempting incremental merges to figure out what happened. I thought 'git merge --no-commit origin $HASH' would do that, pulling $HASH out of master's commit history (git tree --first-parent). But this doesn;t seem to be working. In particular I thought that if $HASH matched the last master commit that I merged from, the result would be a nop. It isn't :( How do I update my branch to some non-HEAD point on master? Answering my own question, I should say 'git merge --no-commit $HASH' (omitting the 'origin'). When merging from a branch head, I found I needed 'origin' -- eg 'git merge origin master' nathan -- Nathan Sidwell
Re: [EXTERNAL] Re: [PATCH 5/6] rs6000, Add vector splat builtin support
On Tue, 2020-06-09 at 17:01 -0700, Carl Love wrote: > Segher: > > So I have been looking at the predicate definitions that I had > created. > > On Fri, 2020-06-05 at 16:28 -0500, Segher Boessenkool wrote: > > > +;; Return 1 if op is a 32-bit constant signed integer > > > +(define_predicate "s32bit_cint_operand" > > > + (and (match_code "const_int") > > > + (match_test "INTVAL (op) >= -2147483648 > > > + && INTVAL (op) <= 2147483647"))) > > > > There probably is a nicer way to write this than with big decimal > > numbers. (I'll not suggest one here because I'll just make a fool > > of > > myself with overflow or signed/unsigned etc. :-) ) > > > > > +;; Return 1 if op is a constant 32-bit signed or unsigned > > > integer > > > +(define_predicate "c32bit_cint_operand" > > > + (and (match_code "const_int") > > > + (match_test "((INTVAL (op) >> 32) == 0)"))) > > The more I look at the above two they really are the > same. Basically, > it boils down to ... can the value signed or unsigned fit in 32-bits > or > not? It seems like both of the above just need to test if the INTVAL > (op) has any bits above bits 0:31 set. So seems like (INTVAL (op) >> > 32) == 0) should be sufficient for both predicates, i.e. replace the > two with a single generic predicate "cint_32bit_operand". > > For starters, I tried changing the definition for s32bit_cint_operand > to: > > ; Return 1 if op is a 32-bit constant signed integer > > (define_predicate "s32bit_cint_operand" > > (and (match_code "const_int") > >(match_test "((INTVAL (op) >> 32) == 0)"))) Compare that to the other predicates (config/rs6000/predicates.md) Those have explicit checks against both ends of the valid range of values. i.e. ;; Return 1 if op is a signed 5-bit constant integer. (define_predicate "s5bit_cint_operand" (and (match_code "const_int") (match_test "INTVAL (op) >= -16 && INTVAL (op) <= 15"))) > > Unfortunately it doesn't seem to work for > > (define_insn > "xxspltiw_v4si" > [(set (match_operand:V4SI 0 "register_operand" > "=wa") > (unspec:V4SI [(match_operand:SI 1 "s32bit_cint_operand" > "n")] > UNSPEC_XXSPLTIW))] > > "TARGET_FUTURE" > > "xxspltiw > %x0,%1" > [(set_attr "type" "vecsimple")]) > > I get unrecongized insn. It seems like (INTVAL (op) >> 32) == 0) > should be true for any 32-bit integer signed or unsigned??? > > Any thoughts as to why this doesn't work? > > > > > This does not work for negative 32-bit numbers? In GCC the LHS > > expression is -1 for those... Not sure what it is for the C++11 we > > now > > require, but in C11 it is implementation-defined, so not good > > either. > > > > > +;; Return 1 if op is a constant 32-bit floating point value > > > +(define_predicate "f32bit_const_operand" > > > + (match_code "const_double")) > > > > Either the predicate name is misleading (if you do allow all > > const_double values), or there should be some test for the alloed > > values > > here. > > The predicate is used to check for a 32-bit float constant. Looking > thru the code not sure if const_double is a 64-bit float? I don't > think > that is what I want. I want a 32-bit floating point value, > const_float, const_real? Don't see a a const_float or anything that > looks like that? Not having much luck to see where const_double gets > defined to see what other definitions there are. > > I am assuming at this point in the compilation process, the constant > that is passed has type info (signed int, unsigned int, float) > associated with it from the front end parsing of the source code. > > Carl >
Automatic vs. manual 'ChangeLog' files updates for devel/ branches etc. (was: [PATCH] wwwdocs: Document devel/omp/gcc-10 branch)
Hi! On 2020-06-10T17:15:41+0200, Tobias Burnus wrote: > On 6/10/20 3:34 PM, Kwok Cheung Yeung wrote: >> This patch updates the previous entry on the website for >> devel/omp/gcc-9 to refer to the new branch instead. This removes >> references to the old branch, as new development should occur on the >> newer branch. > > Can you move the old entry to "Inactive Development Branches" instead? Right, please look up in the revision history of how we did that same procedure in the past, then do the same thing now, and it'll qualify "as obvious". ;-) >> + Please send patch emails with a short-hand [og10] tag in the >> subject line, and use ChangeLog.omp files. > > Do we still need ChangeLog.omp or not? Yes, I wanted to raise this question too -- and that's a general concern for any branches that are not handled by the automatic "commit log to 'ChangeLog' files" updating machinery. The automated system runs once a day. I guess it's generally deemed acceptable if the source code state (commits) and the 'ChangeLog' files state is out of sync for one day. There surely must be some mechanism in place to make sure that actual GCC tarball etc. releases do have complete 'ChangeLog' files state. However, if at an arbitrary point in time, a merge is done, for example, from releases/gcc-10 branch into devel/omp/gcc-10 branch, and then a tarball etc. release is done from the latter, that may not have complete 'ChangeLog' files state, if it wasn't complete at the time of the merge. So that's something to take care of when doing such merges -- or at tarball etc. release time, possibly manually running the updating machinery. > I got used to not adding it > on mainline. If it is needed, I probably need to add one for my last > commit. Indeed the other concern is that people will simply forget to update 'ChangeLog.omp' etc. files given that we're no longer doing it for the GCC main branches. Should we stop doing the manual updates on development branches, too, and extent the automated system to go over all branches? (I suppose, per GNU etc. policy, we cannot just have no 'ChangeLog.*' files on development branches, even if no tarball etc. releases are published from them.) Is it sufficient to keep "ChangeLog state" in the commit log (as we're now doing for GCC main branches), and then run the updating machinery on demand, say, before tarball etc. releases are published? Or should we simply live with the inconsistency between GCC main branches (automatic 'ChangeLog' files updates), and development branches (manual 'ChangeLog' files updates)? Anybody got any clever idea? And sorry, if that has been discussed before, I have not yet read up all the relevant emails. Grüße Thomas - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
1-800-GIT-HELP
I'm trying to merge master to devel/c++-modules, as I usually do. But there's an ICE creeping in, so I'm attempting incremental merges to figure out what happened. I thought 'git merge --no-commit origin $HASH' would do that, pulling $HASH out of master's commit history (git tree --first-parent). But this doesn;t seem to be working. In particular I thought that if $HASH matched the last master commit that I merged from, the result would be a nop. It isn't :( How do I update my branch to some non-HEAD point on master? nathan -- Nathan Sidwell
Re: [PATCH] wwwdocs: Document devel/omp/gcc-10 branch
On 6/10/20 3:34 PM, Kwok Cheung Yeung wrote: This patch updates the previous entry on the website for devel/omp/gcc-9 to refer to the new branch instead. This removes references to the old branch, as new development should occur on the newer branch. Can you move the old entry to "Inactive Development Branches" instead? + Please send patch emails with a short-hand [og10] tag in the subject line, and use ChangeLog.omp files. Do we still need ChangeLog.omp or not? I got used to not adding it on mainline. If it is needed, I probably need to add one for my last commit. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
RE: [IMPORTANT] ChangeLog related changes
Thanks both! Cheers, Tamar > -Original Message- > From: Martin Liška > Sent: Wednesday, June 10, 2020 2:41 PM > To: Tamar Christina ; Jonathan Wakely > > Cc: Jakub Jelinek ; g...@gcc.gnu.org; gcc-patches patc...@gcc.gnu.org> > Subject: Re: [IMPORTANT] ChangeLog related changes > > On 6/10/20 3:34 PM, Tamar Christina wrote: > > Hi All, > > > > Hello. > > > We've been wondering since we no longer list authors in the changelog > > (at least mklog doesn't generate it), > > You are right, it's preferred solution and it's documented here: > https://gcc.gnu.org/codingconventions.html#ChangeLogs > > ''' > a commit author and committer date stamp can be automatically deduced > from a git commit - we recommend to use it ''' > > but we miss to document that additional authors are automatically taken > from: > Co-Authored-By: > > I'll document that. > > Martin > > > How do we handle multi author patches nowadays? > > > > Tried searching for it on the website but couldn’t find anything. > > > > Thanks, > > Tamar > > > >> -Original Message- > >> From: Gcc-patches On Behalf Of > >> Martin Liška > >> Sent: Wednesday, June 10, 2020 8:38 AM > >> To: Jonathan Wakely > >> Cc: Jakub Jelinek ; g...@gcc.gnu.org; gcc-patches > >> > >> Subject: Re: [IMPORTANT] ChangeLog related changes > >> > >> On 6/9/20 10:29 PM, Jonathan Wakely wrote: > >>> OK, here's a proper patch for the changes you liked, dropping the > >>> changes to the Error exception type. > >>> > >>> pytest contrib/gcc-changelog/test_email.py passes. > >>> > >>> OK for master? > >> > >> I like it and I've just pushed the patch to master. > >> > >> Martin
Re: libstdc++: Extend memcmp optimization in std::lexicographical_compare
On 10/06/20 08:18 +0200, François Dumont via Libstdc++ wrote: On 09/06/20 10:53 pm, Jonathan Wakely wrote: This reminds me that I was going to extend the condition for using memcmp to also apply to unsigned integers with sizeof(T) > 1 on big endian targets. This illustrates what I tried to avoid in my original patch, the code duplication. I was calling __lexicographical_compare_aux1 because I didn't want to duplicate the code to compute __simple. Of course it can be expose as a template using. Not for C++98. I'm not very concerned about duplicating the boolean condition. I definitely prefer that to codegen changes that affect every user of lexicographical_compare just to benefit the handful of people using it with std::deque. If __lexicographical_compare_aux1 could be reused without changes, great, but it needed changes with consequences for more code than just deque iterators.
Re: [PATCH] Implement no_stack_protect attribute.
On 6/10/20 2:12 AM, Martin Liška wrote: PING^1 The exclusion changes are just what I was suggesting, thanks. Martin On 5/25/20 3:10 PM, Martin Liška wrote: On 5/21/20 4:53 PM, Martin Sebor wrote: On 5/21/20 5:28 AM, Martin Liška wrote: On 5/18/20 10:37 PM, Martin Sebor wrote: I know there are some somewhat complex cases the attribute exclusion mechanism isn't general enough to handle but this seems simple enough that it should work. Unless I'm missing something that makes it not feasible I would suggest to use it. Hi Martin. Do we have a better place where we check for attribute collision? If by collision you mean the same thing as the mutual exclusion I was talking about then that's done by creating an attribute_spec::exclusions array like for instance attr_cold_hot_exclusions in c-attribs.c and pointing to it from the attribute_spec entries for each of the mutually exclusive attributes in the attribute table. Everything else is handled automatically by decl_attributes. Martin Thanks, I'm sending updated version of the patch that utilizes the conflict detection. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin
[Ada] AI12-0364 Add a modular atomic arithmetic package
This new Ada 202x AI introduces a new package Modular_Arithmetic. Related discussion also suggested to rename the recently introduced Arithmetic package -> Integer_Arithmetic, for consistency, so this is done at the same time. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Arnaud Charlet gcc/ada/ * libgnat/s-aomoar.ads, libgnat/s-aomoar.adb: New files. * libgnat/s-atopar.ads: Move... * libgnat/s-aoinar.ads: Here. * libgnat/s-atopar.adb: Move... * libgnat/s-aoinar.adb: Here. * impunit.adb: Update list of runtime files. * Makefile.rtl (GNATRTL_NONTASKING_OBJS=): Adjust. patch.diff.gz Description: application/gzip
[PATCH] gcc-changelog: fix parse_git_name_status for renames.
Renamed files are listed in the following format:
M gcc/ada/Makefile.rtl
M gcc/ada/impunit.adb
R097gcc/ada/libgnat/s-atopar.adbgcc/ada/libgnat/s-aoinar.adb
R095gcc/ada/libgnat/s-atopar.adsgcc/ada/libgnat/s-aoinar.ads
A gcc/ada/libgnat/s-aomoar.adb
A gcc/ada/libgnat/s-aomoar.ads
So 'R' is followed by a percentage number.
Pushed to master.
contrib/ChangeLog:
* gcc-changelog/git_commit.py: Fix renamed files in
parse_git_name_status.
* gcc-changelog/test_email.py: Add test for it.
---
contrib/gcc-changelog/git_commit.py | 2 +-
contrib/gcc-changelog/test_email.py | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/contrib/gcc-changelog/git_commit.py
b/contrib/gcc-changelog/git_commit.py
index eac64887053..e868e028225 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -322,7 +322,7 @@ class GitCommit:
t = parts[0]
if t == 'A' or t == 'D' or t == 'M':
modified_files.append((parts[1], t))
-elif t == 'R':
+elif t.startswith('R'):
modified_files.append((parts[1], 'D'))
modified_files.append((parts[2], 'A'))
return modified_files
diff --git a/contrib/gcc-changelog/test_email.py
b/contrib/gcc-changelog/test_email.py
index c50687bc331..a185b85e838 100755
--- a/contrib/gcc-changelog/test_email.py
+++ b/contrib/gcc-changelog/test_email.py
@@ -20,6 +20,8 @@ import os
import tempfile
import unittest
+from git_commit import GitCommit
+
from git_email import GitEmail
import unidiff
@@ -29,6 +31,12 @@ script_path = os.path.dirname(os.path.realpath(__file__))
unidiff_supports_renaming = hasattr(unidiff.PatchedFile(), 'is_rename')
+NAME_STATUS1 = """
+M gcc/ada/impunit.adb'
+R097 gcc/ada/libgnat/s-atopar.adbgcc/ada/libgnat/s-aoinar.adb
+"""
+
+
class TestGccChangelog(unittest.TestCase):
def setUp(self):
self.patches = {}
@@ -337,3 +345,9 @@ class TestGccChangelog(unittest.TestCase):
email = self.from_patch_glob('0001-configure.patch')
assert not email.errors
assert len(email.changelog_entries) == 2
+
+def test_parse_git_name_status(self):
+modified_files = GitCommit.parse_git_name_status(NAME_STATUS1)
+assert len(modified_files) == 3
+assert modified_files[1] == ('gcc/ada/libgnat/s-atopar.adb', 'D')
+assert modified_files[2] == ('gcc/ada/libgnat/s-aoinar.adb', 'A')
--
2.26.2
[DOC] gcc-changelog: document additional authors
Hello. I've just pushed the documentation improvement about Co-Authored-By. Martin --- htdocs/codingconventions.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/htdocs/codingconventions.html b/htdocs/codingconventions.html index 6ae962ea..a08ddcbb 100644 --- a/htdocs/codingconventions.html +++ b/htdocs/codingconventions.html @@ -171,7 +171,8 @@ a large batch of changes. script automatically generates missing "New file." entries for files that are added in a commit changed files that are not mentioned in a ChangeLog file generate an error similarly for unchanged files that are mentioned in a ChangeLog file -a commit author and committer date stamp can be automatically deduced from a git commit - we recommend to use it +a commit author and committer date stamp can be automatically deduced from a git commit +(additional authors are taken from Co-Authored-By trailer) - we recommend to use it co_authored_by is added to each ChangeLog entry a PR component is checked against list of valid components ChangeLog files, DATESTAMP, BASE-VER and DEV-PHASE can be modified only separately from other files -- 2.26.2
Re: [IMPORTANT] ChangeLog related changes
On 6/10/20 3:34 PM, Tamar Christina wrote: Hi All, Hello. We've been wondering since we no longer list authors in the changelog (at least mklog doesn't generate it), You are right, it's preferred solution and it's documented here: https://gcc.gnu.org/codingconventions.html#ChangeLogs ''' a commit author and committer date stamp can be automatically deduced from a git commit - we recommend to use it ''' but we miss to document that additional authors are automatically taken from: Co-Authored-By: I'll document that. Martin How do we handle multi author patches nowadays? Tried searching for it on the website but couldn’t find anything. Thanks, Tamar -Original Message- From: Gcc-patches On Behalf Of Martin Liška Sent: Wednesday, June 10, 2020 8:38 AM To: Jonathan Wakely Cc: Jakub Jelinek ; g...@gcc.gnu.org; gcc-patches Subject: Re: [IMPORTANT] ChangeLog related changes On 6/9/20 10:29 PM, Jonathan Wakely wrote: OK, here's a proper patch for the changes you liked, dropping the changes to the Error exception type. pytest contrib/gcc-changelog/test_email.py passes. OK for master? I like it and I've just pushed the patch to master. Martin
Re: [IMPORTANT] ChangeLog related changes
On Wed, Jun 10, 2020 at 01:34:54PM +, Tamar Christina wrote: > Hi All, > > We've been wondering since we no longer list authors in the changelog (at > least mklog doesn't generate it), > How do we handle multi author patches nowadays? > > Tried searching for it on the website but couldn’t find anything. You can add Co-authored-by: name to your commit. If we don't already document it, we should. Marek
[Ada] Ada 202x AI12-0192 "requires late initialization"
Working on this AI it appeared that GNAT wasn't implementing the Ada 2012 notion of "require late initialization", so plug this hole and implement the new rule from AI12-0192 at the same time. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Arnaud Charlet gcc/ada/ * exp_ch3.adb (Build_Init_Statements): Implement the notion of "require late initialization".--- gcc/ada/exp_ch3.adb +++ gcc/ada/exp_ch3.adb @@ -2826,16 +2826,16 @@ package body Exp_Ch3 is --- function Build_Init_Statements (Comp_List : Node_Id) return List_Id is - Checks : constant List_Id := New_List; - Actions : List_Id := No_List; - Counter_Id : Entity_Id:= Empty; - Comp_Loc : Source_Ptr; - Decl : Node_Id; - Has_POC : Boolean; - Id : Entity_Id; - Parent_Stmts : List_Id; - Stmts: List_Id; - Typ : Entity_Id; + Checks : constant List_Id := New_List; + Actions: List_Id := No_List; + Counter_Id : Entity_Id:= Empty; + Comp_Loc : Source_Ptr; + Decl : Node_Id; + Has_Late_Init_Comp : Boolean; + Id : Entity_Id; + Parent_Stmts : List_Id; + Stmts : List_Id; + Typ: Entity_Id; procedure Increment_Counter (Loc : Source_Ptr); -- Generate an "increment by one" statement for the current counter @@ -2846,6 +2846,12 @@ package body Exp_Ch3 is -- creates a new defining Id, adds an object declaration and sets -- the Id generator for the next variant. + function Requires_Late_Initialization + (Decl : Node_Id; +Rec_Type : Entity_Id) return Boolean; + -- Return whether the given Decl requires late initialization, as + -- defined by 3.3.1 (8.1/5). + --- -- Increment_Counter -- --- @@ -2892,6 +2898,158 @@ package body Exp_Ch3 is Make_Integer_Literal (Loc, 0))); end Make_Counter; + -- + -- Requires_Late_Initialization -- + -- + + function Requires_Late_Initialization + (Decl : Node_Id; +Rec_Type : Entity_Id) return Boolean + is +References_Current_Instance : Boolean := False; +Has_Access_Discriminant : Boolean := False; +Has_Internal_Call : Boolean := False; + +function Find_Access_Discriminant + (N : Node_Id) return Traverse_Result; +-- Look for a name denoting an access discriminant + +function Find_Current_Instance + (N : Node_Id) return Traverse_Result; +-- Look for a reference to the current instance of the type + +function Find_Internal_Call + (N : Node_Id) return Traverse_Result; +-- Look for an internal protected function call + +-- +-- Find_Access_Discriminant -- +-- + +function Find_Access_Discriminant + (N : Node_Id) return Traverse_Result is +begin + if Is_Entity_Name (N) + and then Denotes_Discriminant (N) + and then Is_Access_Type (Etype (N)) + then + Has_Access_Discriminant := True; + return Abandon; + else + return OK; + end if; +end Find_Access_Discriminant; + +--- +-- Find_Current_Instance -- +--- + +function Find_Current_Instance + (N : Node_Id) return Traverse_Result is +begin + if Nkind (N) = N_Attribute_Reference + and then Is_Access_Type (Etype (N)) + and then Is_Entity_Name (Prefix (N)) + and then Is_Type (Entity (Prefix (N))) + then + References_Current_Instance := True; + return Abandon; + else + return OK; + end if; +end Find_Current_Instance; + + +-- Find_Internal_Call -- + + +function Find_Internal_Call (N : Node_Id) return Traverse_Result is + + function Call_Scope (N : Node_Id) return Entity_Id; + -- Return the scope enclosing a given call node N + + + -- Call_Scope -- + + +
[Ada] Don't build equivalent record aggregate if type has predicates
Building equivalent record aggregates when the type of the aggregate has predicate functions can result in Gigi crashes if the type hasn't been frozen yet. Since Build_Equivalent_Record_Aggregate is an optimization, it's ok to disable it when encountering aggregates with predicates. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Ghjuvan Lacambre gcc/ada/ * exp_ch3.adb (Build_Equivalent_Record_Aggregate): Return Empty if Etype of record component has predicates.--- gcc/ada/exp_ch3.adb +++ gcc/ada/exp_ch3.adb @@ -1211,6 +1211,17 @@ package body Exp_Ch3 is then Initialization_Warning (T); return Empty; + + -- We need to return empty if the type has predicates because + -- this would otherwise duplicate calls to the predicate + -- function. If the type hasn't been frozen before being + -- referenced in the current record, the extraneous call to + -- the predicate function would be inserted somewhere before + -- the predicate function is elaborated, which would result in + -- an invalid tree. + +elsif Has_Predicates (Etype (Comp)) then + return Empty; end if; elsif Is_Scalar_Type (Etype (Comp)) then
[Ada] Additional warnings on overlapping actuals of composite types
This patch enhances the warnings on overlapping actuals of composite types when only one of them is writable. If these parameters are passed by reference it is the case that assignment to one could have the undesirable effect of modifying the other inside the called subprogram. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Ed Schonberg gcc/ada/ * sem_warn.adb (Warn_On_Overlapping_Actuals): Add a warning when two actuals in a call overlap, both are composite types that may be passed by reference, and only one of them is writable.--- gcc/ada/sem_warn.adb +++ gcc/ada/sem_warn.adb @@ -3742,10 +3742,26 @@ package body Sem_Warn is -- If appropriate warning switch is set, we also report warnings on -- overlapping parameters that are record types or array types. + -- It is also worthwhile to warn on overlaps of composite objects when + -- only one of the formals is (in)-out. Note that the RM rule above is + -- a legality rule. We choose to implement this check as a warning to + -- avoid major incompatibilities with legacy code. We exclude internal + -- sources from the warning, because subprograms in Container libraries + -- would be affected by the warning. + + -- Note also that the rule in 6.4.1 (6.17/3), introduced by AI12-0324, + -- is potentially more expensive to verify, and is not yet implemented. + + if Is_Internal_Unit (Current_Sem_Unit) then + return; + end if; + Form1 := First_Formal (Subp); Act1 := First_Actual (N); while Present (Form1) and then Present (Act1) loop - if Is_Covered_Formal (Form1) then + if Is_Covered_Formal (Form1) +or else not Is_Elementary_Type (Etype (Act1)) + then Form2 := First_Formal (Subp); Act2 := First_Actual (N); while Present (Form2) and then Present (Act2) loop
[Ada] AI12-0311 New checks for language-defined units
This Ada 202x AI defines among other things new check names. Recognize them as no-ops for now. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Arnaud Charlet gcc/ada/ * snames.ads-tmpl (Name_Characters_Assertion_Check, Name_Containers_Assertion_Check, Name_Interfaces_Assertion_Check, Name_IO_Assertion_Check, Name_Numerics_Assertion_Check, Name_Strings_Assertion_Check, Name_System_Assertion_Check): New constants. * types.ads (Characters_Assertion_Check, Containers_Assertion_Check, Interfaces_Assertion_Check, IO_Assertion_Check, Numerics_Assertion_Check, Strings_Assertion_Check, System_Assertion_Check): New constants. (All_Checks): Update accordingly.--- gcc/ada/snames.ads-tmpl +++ gcc/ada/snames.ads-tmpl @@ -1206,21 +1206,28 @@ package Snames is Name_Alignment_Check: constant Name_Id := N + $; -- GNAT Name_Allocation_Check : constant Name_Id := N + $; Name_Atomic_Synchronization : constant Name_Id := N + $; -- GNAT + Name_Characters_Assertion_Check : constant Name_Id := N + $; + Name_Containers_Assertion_Check : constant Name_Id := N + $; Name_Discriminant_Check : constant Name_Id := N + $; Name_Division_Check : constant Name_Id := N + $; Name_Duplicated_Tag_Check : constant Name_Id := N + $; -- GNAT Name_Elaboration_Check : constant Name_Id := N + $; Name_Index_Check: constant Name_Id := N + $; + Name_Interfaces_Assertion_Check : constant Name_Id := N + $; + Name_IO_Assertion_Check : constant Name_Id := N + $; Name_Length_Check : constant Name_Id := N + $; + Name_Numerics_Assertion_Check : constant Name_Id := N + $; Name_Overflow_Check : constant Name_Id := N + $; Name_Predicate_Check: constant Name_Id := N + $; -- GNAT + Name_Program_Error_Check: constant Name_Id := N + $; Name_Range_Check: constant Name_Id := N + $; Name_Storage_Check : constant Name_Id := N + $; + Name_Strings_Assertion_Check: constant Name_Id := N + $; + Name_System_Assertion_Check : constant Name_Id := N + $; Name_Tag_Check : constant Name_Id := N + $; Name_Validity_Check : constant Name_Id := N + $; -- GNAT Name_Container_Checks : constant Name_Id := N + $; -- GNAT Name_Tampering_Check: constant Name_Id := N + $; -- GNAT - Name_Program_Error_Check: constant Name_Id := N + $; Name_Tasking_Check : constant Name_Id := N + $; Name_All_Checks : constant Name_Id := N + $; Last_Check_Name : constant Name_Id := N + $; --- gcc/ada/types.ads +++ gcc/ada/types.ads @@ -668,32 +668,39 @@ package Types is No_Check_Id : constant := 0; -- Check_Id value used to indicate no check - Access_Check : constant := 1; - Accessibility_Check: constant := 2; - Alignment_Check: constant := 3; - Allocation_Check : constant := 4; - Atomic_Synchronization : constant := 5; - Discriminant_Check : constant := 6; - Division_Check : constant := 7; - Duplicated_Tag_Check : constant := 8; - Elaboration_Check : constant := 9; - Index_Check: constant := 10; - Length_Check : constant := 11; - Overflow_Check : constant := 12; - Predicate_Check: constant := 13; - Range_Check: constant := 14; - Storage_Check : constant := 15; - Tag_Check : constant := 16; - Validity_Check : constant := 17; - Container_Checks : constant := 18; - Tampering_Check: constant := 19; - Program_Error_Check: constant := 20; - Tasking_Check : constant := 21; + Access_Check : constant := 1; + Accessibility_Check: constant := 2; + Alignment_Check: constant := 3; + Allocation_Check : constant := 4; + Atomic_Synchronization : constant := 5; + Characters_Assertion_Check : constant := 6; + Containers_Assertion_Check : constant := 7; + Discriminant_Check : constant := 8; + Division_Check : constant := 9; + Duplicated_Tag_Check : constant := 10; + Elaboration_Check : constant := 11; + Index_Check: constant := 12; + Interfaces_Assertion_Check : constant := 13; + IO_Assertion_Check : constant := 14; + Length_Check : constant := 15; + Numerics_Assertion_Check : constant := 16; + Overflow_Check : constant := 17; + Predicate_Check: constant := 18; + Program_Error_Check: constant := 19; + Range_Check: constant
[Ada] Implement AI12-0162 Memberships and Unchecked_Unions
This makes sure that the semantics specified by this AI is observed by using an expression with actions in order to insert the PE raise statement in a membership context with multiple choices. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * exp_ch4.adb (Expand_N_In): Use an expression with actions to insert the PE raise statement for the Unchecked_Union case.--- gcc/ada/exp_ch4.adb +++ gcc/ada/exp_ch4.adb @@ -6527,23 +6527,24 @@ package body Exp_Ch4 is goto Leave; --- Ada 2005 (AI-216): Program_Error is raised when evaluating --- a membership test if the subtype mark denotes a constrained --- Unchecked_Union subtype and the expression lacks inferable --- discriminants. +-- Ada 2005 (AI95-0216 amended by AI12-0162): Program_Error is +-- raised when evaluating an individual membership test if the +-- subtype mark denotes a constrained Unchecked_Union subtype +-- and the expression lacks inferable discriminants. elsif Is_Unchecked_Union (Base_Type (Typ)) and then Is_Constrained (Typ) and then not Has_Inferable_Discriminants (Lop) then - Insert_Action (N, - Make_Raise_Program_Error (Loc, - Reason => PE_Unchecked_Union_Restriction)); - - -- Prevent Gigi from generating incorrect code by rewriting the - -- test as False. What is this undocumented thing about ??? + Rewrite (N, + Make_Expression_With_Actions (Loc, + Actions=> + New_List (Make_Raise_Program_Error (Loc, + Reason => PE_Unchecked_Union_Restriction)), + Expression => + New_Occurrence_Of (Standard_False, Loc))); + Analyze_And_Resolve (N, Restyp); - Rewrite (N, New_Occurrence_Of (Standard_False, Loc)); goto Leave; end if;
[Ada] Add missing Sloc on new explicit dereferences
This makes sure that a Sloc is put on the dereferences inserted by the new procedure Copy_And_Maybe_Dereference. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * sem_util.adb (Copy_And_Maybe_Dereference): Temporarily copy the parent node of the original tree when dereferencing.--- gcc/ada/sem_util.adb +++ gcc/ada/sem_util.adb @@ -1355,6 +1355,9 @@ package body Sem_Util is begin if Is_Access_Type (Etype (New_N)) then +-- Copy the parent to have a proper Sloc on the dereference + +Set_Parent (New_N, Parent (N)); Insert_Explicit_Dereference (New_N); end if;
[Ada] Insert explicit dereferences when building actual subtype
This plugs the only loophole in the front-end through which implicit dereferences can reach the code generator without having being turned into explicit ones. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * sem_util.adb (Copy_And_Maybe_Dereference): New function. (Build_Access_Record_Constraint): Use it to copy the prefix. (Build_Actual_Array_Constraint): Likewise. (Build_Actual_Record_Constraint): Likewise.--- gcc/ada/sem_util.adb +++ gcc/ada/sem_util.adb @@ -1218,6 +1218,10 @@ package body Sem_Util is -- Similar to previous one, for discriminated components constrained -- by the discriminant of the enclosing object. + function Copy_And_Maybe_Dereference (N : Node_Id) return Node_Id; + -- Copy the subtree rooted at N and insert an explicit dereference if it + -- is of an access type. + --- -- Build_Actual_Array_Constraint -- --- @@ -1239,7 +1243,7 @@ package body Sem_Util is if Denotes_Discriminant (Old_Lo) then Lo := Make_Selected_Component (Loc, - Prefix => New_Copy_Tree (P), + Prefix => Copy_And_Maybe_Dereference (P), Selector_Name => New_Occurrence_Of (Entity (Old_Lo), Loc)); else @@ -1257,7 +1261,7 @@ package body Sem_Util is if Denotes_Discriminant (Old_Hi) then Hi := Make_Selected_Component (Loc, - Prefix => New_Copy_Tree (P), + Prefix => Copy_And_Maybe_Dereference (P), Selector_Name => New_Occurrence_Of (Entity (Old_Hi), Loc)); else @@ -1286,7 +1290,7 @@ package body Sem_Util is while Present (D) loop if Denotes_Discriminant (Node (D)) then D_Val := Make_Selected_Component (Loc, - Prefix => New_Copy_Tree (P), + Prefix => Copy_And_Maybe_Dereference (P), Selector_Name => New_Occurrence_Of (Entity (Node (D)), Loc)); else @@ -1322,13 +1326,13 @@ package body Sem_Util is D_Val := New_Copy_Tree (D); Set_Expression (D_Val, Make_Selected_Component (Loc, - Prefix => New_Copy_Tree (P), + Prefix => Copy_And_Maybe_Dereference (P), Selector_Name => New_Occurrence_Of (Entity (Expression (D)), Loc))); elsif Denotes_Discriminant (D) then D_Val := Make_Selected_Component (Loc, - Prefix => New_Copy_Tree (P), + Prefix => Copy_And_Maybe_Dereference (P), Selector_Name => New_Occurrence_Of (Entity (D), Loc)); else @@ -1342,6 +1346,21 @@ package body Sem_Util is return Constraints; end Build_Access_Record_Constraint; + + -- Copy_And_Maybe_Dereference -- + + + function Copy_And_Maybe_Dereference (N : Node_Id) return Node_Id is + New_N : constant Node_Id := New_Copy_Tree (N); + + begin + if Is_Access_Type (Etype (New_N)) then +Insert_Explicit_Dereference (New_N); + end if; + + return New_N; + end Copy_And_Maybe_Dereference; + -- Start of processing for Build_Actual_Subtype_Of_Component begin
[Ada] Remove obsolete code in Resolve_Call
This removes a block of code in Resolve_Call that inserts an explicit dereference for a call whose prefix is an access-to-subprogram type, but this processing is already done earlier in Analyze_Call. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * sem_ch4.adb (Analyze_Call): Use idiomatic condition. * sem_res.adb (Resolve_Call): Remove obsolete code.--- gcc/ada/sem_ch4.adb +++ gcc/ada/sem_ch4.adb @@ -1176,8 +1176,7 @@ package body Sem_Ch4 is -- type is an array, F (X) cannot be interpreted as an indirect call -- through the result of the call to F. - elsif Is_Access_Type (Etype (Nam)) - and then Ekind (Designated_Type (Etype (Nam))) = E_Subprogram_Type + elsif Is_Access_Subprogram_Type (Base_Type (Etype (Nam))) and then (not Name_Denotes_Function or else Nkind (N) = N_Procedure_Call_Statement --- gcc/ada/sem_res.adb +++ gcc/ada/sem_res.adb @@ -6141,26 +6141,6 @@ package body Sem_Res is end loop; end if; - if Is_Access_Subprogram_Type (Base_Type (Etype (Nam))) -and then not Is_Access_Subprogram_Type (Base_Type (Typ)) -and then Nkind (Subp) /= N_Explicit_Dereference -and then Present (Parameter_Associations (N)) - then - -- The prefix is a parameterless function call that returns an access - -- to subprogram. If parameters are present in the current call, add - -- add an explicit dereference. We use the base type here because - -- within an instance these may be subtypes. - - -- The dereference is added either in Analyze_Call or here. Should - -- be consolidated ??? - - Set_Is_Overloaded (Subp, False); - Set_Etype (Subp, Etype (Nam)); - Insert_Explicit_Dereference (Subp); - Nam := Designated_Type (Etype (Nam)); - Resolve (Subp, Nam); - end if; - -- Check that a call to Current_Task does not occur in an entry body if Is_RTE (Nam, RE_Current_Task) then
[Ada] Remove Determine_License
The routine Determine_License is brittle and doesn't e.g. handle
properly wide characters. Furthermore this is just a heuristic, which
isn't really needed, so remove it to simplify maintenance and remove
latent issues with wide characters.
Tested on x86_64-pc-linux-gnu, committed on trunk
2020-06-10 Arnaud Charlet
gcc/ada/
* scn.adb (Determine_License): Remove.--- gcc/ada/scn.adb
+++ gcc/ada/scn.adb
@@ -36,158 +36,11 @@ with Uintp;use Uintp;
package body Scn is
- use ASCII;
-
Used_As_Identifier : array (Token_Type) of Boolean;
-- Flags set True if a given keyword is used as an identifier (used to
-- make sure that we only post an error message for incorrect use of a
-- keyword as an identifier once for a given keyword).
- function Determine_License return License_Type;
- -- Scan header of file and check that it has an appropriate GNAT-style
- -- header with a proper license statement. Returns GPL, Unrestricted,
- -- or Modified_GPL depending on header. If none of these, returns Unknown.
-
- ---
- -- Determine_License --
- ---
-
- function Determine_License return License_Type is
- GPL_Found : Boolean := False;
- Result: License_Type;
-
- function Contains (S : String) return Boolean;
- -- See if current comment contains successive non-blank characters
- -- matching the contents of S. If so leave Scan_Ptr unchanged and
- -- return True, otherwise leave Scan_Ptr unchanged and return False.
-
- procedure Skip_EOL;
- -- Skip to line terminator character
-
- --
- -- Contains --
- --
-
- function Contains (S : String) return Boolean is
- CP : Natural;
- SP : Source_Ptr;
- SS : Source_Ptr;
-
- begin
- -- Loop to check characters. This loop is terminated by end of
- -- line, and also we need to check for the EOF case, to take
- -- care of files containing only comments.
-
- SP := Scan_Ptr;
- while Source (SP) /= CR and then
- Source (SP) /= LF and then
- Source (SP) /= EOF
- loop
-if Source (SP) = S (S'First) then
- SS := SP;
- CP := S'First;
-
- loop
- SS := SS + 1;
- CP := CP + 1;
-
- if CP > S'Last then
- return True;
- end if;
-
- while Source (SS) = ' ' loop
- SS := SS + 1;
- end loop;
-
- exit when Source (SS) /= S (CP);
- end loop;
-end if;
-
-SP := SP + 1;
- end loop;
-
- return False;
- end Contains;
-
- --
- -- Skip_EOL --
- --
-
- procedure Skip_EOL is
- begin
- while Source (Scan_Ptr) /= CR
- and then Source (Scan_Ptr) /= LF
- and then Source (Scan_Ptr) /= EOF
- loop
-Scan_Ptr := Scan_Ptr + 1;
- end loop;
- end Skip_EOL;
-
- -- Start of processing for Determine_License
-
- begin
- loop
- if Source (Scan_Ptr) /= '-'
- or else Source (Scan_Ptr + 1) /= '-'
- then
-if GPL_Found then
- Result := GPL;
- exit;
-else
- Result := Unknown;
- exit;
-end if;
-
- elsif Contains ("Asaspecialexception") then
-if GPL_Found then
- Result := Modified_GPL;
- exit;
-end if;
-
- elsif Contains ("GNUGeneralPublicLicense") then
-GPL_Found := True;
-
- elsif
- Contains
- ("ThisspecificationisadaptedfromtheAdaSemanticInterface")
- or else
- Contains
- ("ThisspecificationisderivedfromtheAdaReferenceManual")
- then
-Result := Unrestricted;
-exit;
- end if;
-
- Skip_EOL;
-
- Scanner.Check_End_Of_Line;
-
- if Source (Scan_Ptr) /= EOF then
-
--- We have to take into account a degenerate case when the source
--- file contains only comments and no Ada code.
-
-declare
- Physical : Boolean;
-
-begin
- Skip_Line_Terminators (Scan_Ptr, Physical);
-
- -- If we are at start of physical line, update scan pointers
- -- to reflect the start of the new line.
-
- if Physical then
- Current_Line_Start := Scan_Ptr;
- Start_Column := Scanner.Set_Start_Column;
- First_Non_Blank_Location := Scan_Ptr;
- end if;
-end;
- end if;
- end loop;
-
- return Result;
- end Determine_License;
-
[Ada] Fold Enum_Rep attribute in evaluation and not in expansion
Folding of Enum_Rep attribute was partly done in evaluation (for expressions like "Typ'Enum_Rep (Enum_Literal)") and partly in expansion (for expressions like "Enum_Literal'Enum_Rep". Moreover, some of the code in evaluation was dead and some of the code in expansion was violating internal assertions (when the Enum_Rep attribute was applied to expression like "T'Last (1)"). This was confusing and required the code for expansion to be duplicated in backends that define custom expansion (e.g. for GNATprove). This patch activates dead folding in evaluation and deconstructs buggy folding in expansion (which was duplicated between GNAT and GNATprove). Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Piotr Trojanek gcc/ada/ * exp_attr.adb (Expand_N_Attribute_Reference): Remove folding for Enum_Rep attribute. * exp_spark.adb (Expand_SPARK_N_Attribute_Reference): Remove duplicated code for folding Enum_Rep attribute. * sem_attr.adb (Eval_Attribute): Relax condition for folding Enum_Rep attribute; previously dead code is now executed when the attribute prefix is an enumeration literal; refine type in processing of Enum_Val.--- gcc/ada/exp_attr.adb +++ gcc/ada/exp_attr.adb @@ -3159,17 +3159,8 @@ package body Exp_Attr is Expr := Pref; end if; - -- If the expression is an enumeration literal, it is replaced by the - -- literal value. - - if Nkind (Expr) in N_Has_Entity - and then Ekind (Entity (Expr)) = E_Enumeration_Literal - then -Rewrite (N, - Make_Integer_Literal (Loc, Enumeration_Rep (Entity (Expr; - - -- If not constant-folded above, Enum_Type'Enum_Rep (X) or - -- X'Enum_Rep expands to + -- If not constant-folded, Enum_Type'Enum_Rep (X) or X'Enum_Rep + -- expands to --target-type (X) @@ -3185,7 +3176,7 @@ package body Exp_Attr is -- first convert to a small signed integer type in order not to lose -- the size information. - elsif Is_Enumeration_Type (Ptyp) then + if Is_Enumeration_Type (Ptyp) then Psiz := RM_Size (Base_Type (Ptyp)); if Psiz < 8 then --- gcc/ada/exp_spark.adb +++ gcc/ada/exp_spark.adb @@ -199,29 +199,6 @@ package body Exp_SPARK is Parameter_Associations => New_List (Expr))); Analyze_And_Resolve (N, Typ); - -- Whenever possible, replace a prefix which is an enumeration literal - -- by the corresponding literal value, just like it happens in the GNAT - -- expander. - - elsif Attr_Id = Attribute_Enum_Rep then - declare -Exprs : constant List_Id := Expressions (N); - begin -if Is_Non_Empty_List (Exprs) then - Expr := First (Exprs); -else - Expr := Prefix (N); -end if; - --- If the argument is a literal, expand it - -if Nkind (Expr) in N_Has_Entity - and then Ekind (Entity (Expr)) = E_Enumeration_Literal -then - Exp_Attr.Expand_N_Attribute_Reference (N); -end if; - end; - elsif Attr_Id = Attribute_Object_Size or else Attr_Id = Attribute_Size or else Attr_Id = Attribute_Value_Size --- gcc/ada/sem_attr.adb +++ gcc/ada/sem_attr.adb @@ -7719,7 +7719,11 @@ package body Sem_Attr is -- purpose, a string literal counts as an object (attributes of string -- literals can only appear in generated code). - if Is_Object_Reference (P) or else Nkind (P) = N_String_Literal then + if Is_Object_Reference (P) +or else Nkind (P) = N_String_Literal +or else (Is_Entity_Name (P) + and then Ekind (Entity (P)) = E_Enumeration_Literal) + then -- For Component_Size, the prefix is an array object, and we apply -- the attribute to the type of the object. This is allowed for both @@ -8533,7 +8537,7 @@ package body Sem_Attr is -- when Attribute_Enum_Val => Enum_Val : declare - Lit : Node_Id; + Lit : Entity_Id; begin -- We have something like Enum_Type'Enum_Val (23), so search for a
[Ada] Revert workaround for expansion of Enum_Rep in GNATprove mode
A workaround for a bug in expansion of Enum_Rep attribute in the GNATprove mode was to expand First and Last attributes. Now with handling of Enum_Rep fixed we don't need this workaround anymore. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Piotr Trojanek gcc/ada/ * exp_spark.adb (Expand_SPARK_N_Attribute_Reference): Remove expansion of First and Last attributes.--- gcc/ada/exp_spark.adb +++ gcc/ada/exp_spark.adb @@ -52,14 +52,13 @@ package body Exp_SPARK is --- procedure Expand_SPARK_N_Attribute_Reference (N : Node_Id); - -- Replace occurrences of System'To_Address by calls to - -- System.Storage_Elements.To_Address + -- Perform attribute-reference-specific expansion procedure Expand_SPARK_N_Freeze_Type (E : Entity_Id); -- Build the DIC procedure of a type when needed, if not already done procedure Expand_SPARK_N_Loop_Statement (N : Node_Id); - -- Perform loop statement-specific expansion + -- Perform loop-statement-specific expansion procedure Expand_SPARK_N_Object_Declaration (N : Node_Id); -- Perform object-declaration-specific expansion @@ -266,12 +265,6 @@ package body Exp_SPARK is Analyze_And_Resolve (N, Standard_Boolean); end if; - -- For attributes First and Last simply reuse the standard expansion - - elsif Attr_Id = Attribute_First -or else Attr_Id = Attribute_Last - then - Exp_Attr.Expand_N_Attribute_Reference (N); end if; end Expand_SPARK_N_Attribute_Reference;
[Ada] Ada_2020 AI12-0220: Pre/Postconditions on Access_To_Subprogram types
This patch implements AI12-0220, which adds contracts to
Access_To_Subprogram types so that pre/postconditions are applied to all
indirect calls through such an access type.
Tested on x86_64-pc-linux-gnu, committed on trunk
2020-06-10 Ed Schonberg
gcc/ada/
* einfo.ads (Access_Subprogram_Wrapper): New attribute of
Subprogram_Type entities. Denotes subprogram constructed for
Access_To_Subprogram types that include pre- and postconditions.
* einfo.adb: Subprogram bodies for Access_Subprogram_Wrapper.
* exp_ch6.adb (Expand_Call): An indirect call through an
Access_To_subprogram that includes contracts is rewritten as a
call to the corresponding Access_ ubprogram_Wrapper. Handle
derived types that inherit contract from parent.
* sem_prag.adb (Build_Access_Subprogram_Wrapper): Build
subprogram declaration for subprogram that incorporates the
contracts of an Access_To_Subprogram type declaration. Build
corresponding body and attach it to freeze actions for type.
* sem_util.ads, sem_util.adb (Is_Access_Subprogram_Wrapper):
Utility that uses signature of the subprogram to determine
whether it is a generated wrapper for an Access_To_Subprogram
type.--- gcc/ada/einfo.adb
+++ gcc/ada/einfo.adb
@@ -282,6 +282,7 @@ package body Einfo is
--SPARK_PragmaNode40
+ --Access_Subprogram_Wrapper Node41
--Original_Protected_Subprogram Node41
--SPARK_Aux_PragmaNode41
@@ -738,6 +739,12 @@ package body Einfo is
return Node30 (Implementation_Base_Type (Id));
end Access_Disp_Table_Elab_Flag;
+ function Access_Subprogram_Wrapper (Id : E) return E is
+ begin
+ pragma Assert (Ekind (Id) = E_Subprogram_Type);
+ return Node41 (Id);
+ end Access_Subprogram_Wrapper;
+
function Activation_Record_Component (Id : E) return E is
begin
pragma Assert (Ekind_In (Id, E_Constant,
@@ -3902,6 +3909,12 @@ package body Einfo is
Set_Node30 (Id, V);
end Set_Access_Disp_Table_Elab_Flag;
+ procedure Set_Access_Subprogram_Wrapper (Id : E; V : E) is
+ begin
+ pragma Assert (Ekind (Id) = E_Subprogram_Type);
+ Set_Node41 (Id, V);
+ end Set_Access_Subprogram_Wrapper;
+
procedure Set_Anonymous_Designated_Type (Id : E; V : E) is
begin
pragma Assert (Ekind (Id) = E_Variable);
@@ -11411,6 +11424,9 @@ package body Einfo is
=>
Write_Str ("SPARK_Aux_Pragma");
+ when E_Subprogram_Type =>
+Write_Str ("Access_Subprogram_Wrapper");
+
when others =>
Write_Str ("Field41??");
end case;
--- gcc/ada/einfo.ads
+++ gcc/ada/einfo.ads
@@ -372,6 +372,15 @@ package Einfo is
-- on attribute 'Position applied to an object of the type; it is used by
-- the IP routine to avoid performing this elaboration twice.
+--Access_Subprogram_Wrapper (Node41)
+-- Entity created for access_to_subprogram types that have pre/post
+-- conditions. Wrapper subprogram is created when analyzing corresponding
+-- aspect, and inherits said aspects. Body of subprogram includes code
+-- to check contracts, and a direct call to the designated subprogram.
+-- The body is part of the freeze actions for the type.
+-- The Subprogram_Type created for the Access_To_Subprogram carries the
+-- Access_Subprogram_Wrapper for use in the expansion of indirect calls.
+
--Activation_Record_Component (Node31)
-- Defined for E_Variable, E_Constant, E_Loop_Parameter, and formal
-- parameter entities. Used in Opt.Unnest_Subprogram_Mode, in which case
@@ -6721,6 +6730,7 @@ package Einfo is
--Extra_Accessibility_Of_Result (Node19)
--Directly_Designated_Type(Node20)
--Extra_Formals (Node28)
+ --Access_Subprogram_Wrapper (Node41)
--First_Formal(synth)
--First_Formal_With_Extras(synth)
--Last_Formal (synth)
@@ -7068,6 +7078,7 @@ package Einfo is
function Accept_Address (Id : E) return L;
function Access_Disp_Table (Id : E) return L;
function Access_Disp_Table_Elab_Flag (Id : E) return E;
+ function Access_Subprogram_Wrapper (Id : E) return E;
function Activation_Record_Component (Id : E) return E;
function Actual_Subtype (Id : E) return E;
function Address_Taken (Id : E) return B;
@@ -7775,6 +7786,7 @@ package Einfo is
procedure Set_Accept_Address (Id : E; V : L);
procedure Set_Access_Disp_Table (Id : E; V : L);
procedure Set_Access_Disp_Table_Elab_Flag (Id : E; V : E);
+ procedure Set_Access_Subprogram_Wrapper (Id : E; V : E);
[Ada] Improve code generated for dynamic discriminated aggregate
This changes the way some assignments of aggregates of dynamic discriminated record types are expanded by the front-end: they used to always give rise to the creation of a temporary, which is unnecessary if the by-copy semantics can be guaranteed. This also puts the treatment of qualified aggregates on par with that of unqualified aggregates in other cases. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * exp_aggr.adb (In_Place_Assign_OK): Do not necessarily return false for a type with discriminants. (Convert_To_Assignments): Use Parent_Node and Parent_Kind more consistently. In the in-place assignment case, first apply a discriminant check if need be, and be prepared for a rewritten aggregate as a result.--- gcc/ada/exp_aggr.adb +++ gcc/ada/exp_aggr.adb @@ -4283,12 +4283,9 @@ package body Exp_Aggr is -- Start of processing for In_Place_Assign_OK begin - -- By-copy semantic cannot be guaranteed for controlled objects or - -- objects with discriminants. + -- By-copy semantic cannot be guaranteed for controlled objects - if Needs_Finalization (Etype (N)) -or else Has_Discriminants (Etype (N)) - then + if Needs_Finalization (Etype (N)) then return False; elsif Is_Array and then Present (Component_Associations (N)) then @@ -4465,26 +4462,40 @@ package body Exp_Aggr is -- assignment. if Is_Limited_Type (Typ) -and then Nkind (Parent (N)) = N_Assignment_Statement +and then Parent_Kind = N_Assignment_Statement then - Target_Expr := New_Copy_Tree (Name (Parent (N))); - Insert_Actions (Parent (N), + Target_Expr := New_Copy_Tree (Name (Parent_Node)); + Insert_Actions (Parent_Node, Build_Record_Aggr_Code (N, Typ, Target_Expr)); - Rewrite (Parent (N), Make_Null_Statement (Loc)); + Rewrite (Parent_Node, Make_Null_Statement (Loc)); -- Do not declare a temporary to initialize an aggregate assigned to an -- identifier when in-place assignment is possible, preserving the -- by-copy semantic of aggregates. This avoids large stack usage and -- generates more efficient code. - elsif Nkind (Parent (N)) = N_Assignment_Statement -and then Nkind (Name (Parent (N))) = N_Identifier + elsif Parent_Kind = N_Assignment_Statement +and then Nkind (Name (Parent_Node)) = N_Identifier and then In_Place_Assign_OK (N) then - Target_Expr := New_Copy_Tree (Name (Parent (N))); - Insert_Actions (Parent (N), - Build_Record_Aggr_Code (N, Typ, Target_Expr)); - Rewrite (Parent (N), Make_Null_Statement (Loc)); + declare +Lhs : constant Node_Id := Name (Parent_Node); + begin +-- Apply discriminant check if required + +if Has_Discriminants (Etype (N)) then + Apply_Discriminant_Check (N, Etype (Lhs), Lhs); +end if; + +-- The check just above may have replaced the aggregate with a CE + +if Nkind_In (N, N_Aggregate, N_Extension_Aggregate) then + Target_Expr := New_Copy_Tree (Lhs); + Insert_Actions (Parent_Node, + Build_Record_Aggr_Code (N, Typ, Target_Expr)); + Rewrite (Parent_Node, Make_Null_Statement (Loc)); +end if; + end; else Temp := Make_Temporary (Loc, 'A', N);
[Ada] Disable unwanted warnings in Assertion_Policy(Ignore) mode
This patch fixes a bug where if pragma Assertion_Policy(Ignore) is in effect, if the only reference to a given declaration is in an Invariant, spurious warnings about unused entities are given. For example, if a compilation unit says "with X;", and the only reference to X is in an invariant, the compiler would warn that the with clause is useless. The effect of this patch is that we generate invariant-checking procedures in Assertion_Policy(Ignore) mode, but they are empty, and we don't call them. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Bob Duff gcc/ada/ * sem_prag.adb (Invariant): Remove the pragma removing code. It doesn't work to remove the pragma, because various flags are set during Build_Invariant_Procedure_Declaration and Build_Invariant_Procedure_Body that need to be set to avoid the spurious warnings. * exp_util.adb (Make_Invariant_Call): Avoid calling the invariant-checking procedure if the body is empty. This is an optimization.--- gcc/ada/exp_util.adb +++ gcc/ada/exp_util.adb @@ -2298,9 +2298,8 @@ package body Exp_Util is -- Generate: --Invariant (_object ()); - -- Note that the invariant procedure may have a null body if - -- assertions are disabled or Assertion_Policy Ignore is in - -- effect. + -- The invariant procedure has a null body if assertions are + -- disabled or Assertion_Policy Ignore is in effect. if not Has_Null_Body (Proc_Id) then Append_New_To (Comp_Checks, @@ -9360,19 +9359,22 @@ package body Exp_Util is function Make_Invariant_Call (Expr : Node_Id) return Node_Id is Loc : constant Source_Ptr := Sloc (Expr); Typ : constant Entity_Id := Base_Type (Etype (Expr)); - - Proc_Id : Entity_Id; - - begin pragma Assert (Has_Invariants (Typ)); - - Proc_Id := Invariant_Procedure (Typ); + Proc_Id : constant Entity_Id := Invariant_Procedure (Typ); pragma Assert (Present (Proc_Id)); + begin + -- The invariant procedure has a null body if assertions are disabled or + -- Assertion_Policy Ignore is in effect. In that case, generate a null + -- statement instead of a call to the invariant procedure. - return -Make_Procedure_Call_Statement (Loc, - Name => New_Occurrence_Of (Proc_Id, Loc), - Parameter_Associations => New_List (Relocate_Node (Expr))); + if Has_Null_Body (Proc_Id) then + return Make_Null_Statement (Loc); + else + return + Make_Procedure_Call_Statement (Loc, + Name => New_Occurrence_Of (Proc_Id, Loc), + Parameter_Associations => New_List (Relocate_Node (Expr))); + end if; end Make_Invariant_Call; --- gcc/ada/sem_prag.adb +++ gcc/ada/sem_prag.adb @@ -18607,20 +18607,6 @@ package body Sem_Prag is return; end if; --- If invariants should be ignored, delete the pragma and then --- return. We do this here, after checking for errors, and before --- generating anything that has a run-time effect. - -if Present (Check_Policy_List) - and then -(Policy_In_Effect (Name_Invariant) = Name_Ignore - and then - Policy_In_Effect (Name_Type_Invariant) = Name_Ignore) -then - Rewrite (N, Make_Null_Statement (Loc)); - return; -end if; - -- A pragma that applies to a Ghost entity becomes Ghost for the -- purposes of legality checks and removal of ignored Ghost code.
[Ada] Simplify detection of static membership choices
Membership test operators (i.e. "in" and "not in") have either the right operand present (when there is just one choice) or the list of alternatives present (when there are many choices), as documented in sinfo.ads. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Piotr Trojanek gcc/ada/ * sem_ch13.adb (All_Membership_Choices_Static): Assert an AST property documented in sinfo.ads and simplify an excessive condition.--- gcc/ada/sem_ch13.adb +++ gcc/ada/sem_ch13.adb @@ -844,11 +844,16 @@ package body Sem_Ch13 is function All_Membership_Choices_Static (Expr : Node_Id) return Boolean is pragma Assert (Nkind (Expr) in N_Membership_Test); begin - return ((Present (Right_Opnd (Expr)) - and then Is_Static_Choice (Right_Opnd (Expr))) -or else - (Present (Alternatives (Expr)) - and then All_Static_Choices (Alternatives (Expr; + pragma Assert +(Present (Right_Opnd (Expr)) + xor + Present (Alternatives (Expr))); + + if Present (Right_Opnd (Expr)) then + return Is_Static_Choice (Right_Opnd (Expr)); + else + return All_Static_Choices (Alternatives (Expr)); + end if; end All_Membership_Choices_Static;
[Ada] Fix incorrect insertion of post-call actions in if-expression
When post-call actions need to be inserted in an expression context, an N_Expression_With_Actions node is used. That's not done here for the condition of an if-expression, so the change adds this case. It also deals with the case of a call to a primitive of a tagged type written in prefixed notation, which was also missing. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Eric Botcazou gcc/ada/ * exp_ch6.adb (Insert_Post_Call_Actions): Deal with the context of an if-expression and with a call written in prefixed notation.--- gcc/ada/exp_ch6.adb +++ gcc/ada/exp_ch6.adb @@ -7810,12 +7810,15 @@ package body Exp_Ch6 is return; end if; - -- Cases where the call is not a member of a statement list. This - -- includes the case where the call is an actual in another function - -- call or indexing, i.e. an expression context as well. + -- Cases where the call is not a member of a statement list. This also + -- includes the cases where the call is an actual in another function + -- call, or is an index, or is an operand of an if-expression, i.e. is + -- in an expression context. if not Is_List_Member (N) -or else Nkind_In (Context, N_Function_Call, N_Indexed_Component) +or else Nkind_In (Context, N_Function_Call, + N_If_Expression, + N_Indexed_Component) then -- In Ada 2012 the call may be a function call in an expression -- (since OUT and IN OUT parameters are now allowed for such calls). @@ -7823,7 +7826,9 @@ package body Exp_Ch6 is -- but the constraint checks generated when subtypes of formal and -- actual don't match must be inserted in the form of assignments. - if Nkind (Original_Node (N)) = N_Function_Call then + if Nkind (N) = N_Function_Call + or else Nkind (Original_Node (N)) = N_Function_Call + then pragma Assert (Ada_Version >= Ada_2012); -- Functions with '[in] out' parameters are only allowed in Ada -- 2012.
[Ada] Classwide controlled obj not dispatching
Overriding dispatching primitives Initialize, Adjust or Finalize of a controlled type by means of a subprogram body that has no specification causes the frontend to initialize incorrectly the dispatch table slots of these primitives. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Javier Miranda gcc/ada/ * sem_ch3.adb (Analyze_Declarations): Adjust the machinery that takes care of late body overriding of initialize, adjust, finalize. Remove ASIS mode code.--- gcc/ada/sem_ch3.adb +++ gcc/ada/sem_ch3.adb @@ -2592,12 +2592,10 @@ package body Sem_Ch3 is -- Local variables Context : Node_Id := Empty; + Ctrl_Typ: Entity_Id := Empty; Freeze_From : Entity_Id := Empty; Next_Decl : Node_Id; - Body_Seen : Boolean := False; - -- Flag set when the first body [stub] is encountered - -- Start of processing for Analyze_Declarations begin @@ -2613,6 +2611,16 @@ package body Sem_Ch3 is Freeze_From := First_Entity (Current_Scope); end if; + -- Remember if the declaration we just processed is the full type + -- declaration of a controlled type (to handle late overriding of + -- initialize, adjust or finalize). + + if Nkind (Decl) = N_Full_Type_Declaration + and then Is_Controlled (Defining_Identifier (Decl)) + then +Ctrl_Typ := Defining_Identifier (Decl); + end if; + -- At the end of a declarative part, freeze remaining entities -- declared in it. The end of the visible declarations of package -- specification is not the end of a declarative part if private @@ -2758,19 +2766,17 @@ package body Sem_Ch3 is -- ??? A cleaner approach may be possible and/or this solution -- could be extended to general-purpose late primitives, TBD. -if not Body_Seen and then not Is_Body (Decl) then - Body_Seen := True; +if Present (Ctrl_Typ) then - if Nkind (Next_Decl) = N_Subprogram_Body then - Handle_Late_Controlled_Primitive (Next_Decl); - end if; + -- No need to continue searching for late body overriding if + -- the controlled type is already frozen. -else - -- In ASIS mode, if the next declaration is a body, complete - -- the analysis of declarations so far. - -- Is this still needed??? + if Is_Frozen (Ctrl_Typ) then + Ctrl_Typ := Empty; - Resolve_Aspects; + elsif Nkind (Next_Decl) = N_Subprogram_Body then + Handle_Late_Controlled_Primitive (Next_Decl); + end if; end if; Adjust_Decl;
[Ada] Incorrect accessibility checks on functions calls
This patch corrects an issue whereby the compiler would issue incorrect accessibility errors and checks for objects initialized by functions returning anonymous access types or type conversions where the operand is a function returning an anonymous access type. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Justin Squirek gcc/ada/ * exp_ch3.adb (Expand_N_Object_Declaration): Add condition to handle processing of objects initialized by a call to a function return an anonymous access type. * exp_ch6.adb, exp_ch6.ads (Has_Unconstrained_Access_Discriminants): Moved to sem_util.adb (Needs_Result_Accessibility_Level): Moved to sem_util.adb * sem_util.adb, sem_util.ads (Has_Unconstrained_Access_Discriminants): Moved from exp_ch6.adb (Needs_Result_Accessibility_Level): Moved from exp_ch6.adb * sem_res.adb (Valid_Conversion): Add condition for the special case where the operand of a conversion is the result of an anonymous access type--- gcc/ada/exp_ch3.adb +++ gcc/ada/exp_ch3.adb @@ -7178,21 +7178,32 @@ package body Exp_Ch3 is Chars => New_External_Name (Chars (Def_Id), Suffix => "L")); -Level_Expr : Node_Id; Level_Decl : Node_Id; +Level_Expr : Node_Id; begin Set_Ekind (Level, Ekind (Def_Id)); Set_Etype (Level, Standard_Natural); Set_Scope (Level, Scope (Def_Id)); -if No (Expr) then - - -- Set accessibility level of null +-- Set accessibility level of null +if No (Expr) then Level_Expr := Make_Integer_Literal (Loc, Scope_Depth (Standard_Standard)); +-- When the expression of the object is a function which returns +-- an anonymous access type the master of the call is the object +-- being initialized instead of the type. + +elsif Nkind (Expr) = N_Function_Call + and then Ekind (Etype (Name (Expr))) = E_Anonymous_Access_Type +then + Level_Expr := Make_Integer_Literal (Loc, + Object_Access_Level (Def_Id)); + +-- General case + else Level_Expr := Dynamic_Accessibility_Level (Expr); end if; --- gcc/ada/exp_ch6.adb +++ gcc/ada/exp_ch6.adb @@ -244,11 +244,6 @@ package body Exp_Ch6 is -- Expand simple return from function. In the case where we are returning -- from a function body this is called by Expand_N_Simple_Return_Statement. - function Has_Unconstrained_Access_Discriminants - (Subtyp : Entity_Id) return Boolean; - -- Returns True if the given subtype is unconstrained and has one or more - -- access discriminants. - procedure Insert_Post_Call_Actions (N : Node_Id; Post_Call : List_Id); -- Insert the Post_Call list previously produced by routine Expand_Actuals -- or Expand_Call_Helper into the tree. @@ -7772,32 +7767,6 @@ package body Exp_Ch6 is end if; end Freeze_Subprogram; - - -- Has_Unconstrained_Access_Discriminants -- - - - function Has_Unconstrained_Access_Discriminants - (Subtyp : Entity_Id) return Boolean - is - Discr : Entity_Id; - - begin - if Has_Discriminants (Subtyp) -and then not Is_Constrained (Subtyp) - then - Discr := First_Discriminant (Subtyp); - while Present (Discr) loop -if Ekind (Etype (Discr)) = E_Anonymous_Access_Type then - return True; -end if; - -Next_Discriminant (Discr); - end loop; - end if; - - return False; - end Has_Unconstrained_Access_Discriminants; - -- -- Insert_Post_Call_Actions -- -- @@ -9431,144 +9400,6 @@ package body Exp_Ch6 is return Requires_Transient_Scope (Func_Typ); end Needs_BIP_Alloc_Form; - -- - -- Needs_Result_Accessibility_Level -- - -- - - function Needs_Result_Accessibility_Level - (Func_Id : Entity_Id) return Boolean - is - Func_Typ : constant Entity_Id := Underlying_Type (Etype (Func_Id)); - - function Has_Unconstrained_Access_Discriminant_Component -(Comp_Typ : Entity_Id) return Boolean; - -- Returns True if any component of the type has an unconstrained access - -- discriminant. - - - - -- Has_Unconstrained_Access_Discriminant_Component -- - - - - function Has_Unconstrained_Access_Discriminant_Component -(Comp_Typ : Entity_Id) return Boolean - is -
[Ada] Fix assertion failure on functions with contracts
This commit fixes a bug introduced in a previous commit that attempted
to detect illegal body definitions on null procedures but did not
account for the fact that function definitions such as `function F
return Integer with Global => null is` could use the same path. This
would result in an assertion failure when Null_Present was called on the
function's specification.
Tested on x86_64-pc-linux-gnu, committed on trunk
2020-06-10 Ghjuvan Lacambre
gcc/ada/
* par-ch6.adb (P_Subprogram): Make sure the specification
belongs to a procedure.--- gcc/ada/par-ch6.adb
+++ gcc/ada/par-ch6.adb
@@ -960,9 +960,12 @@ package body Ch6 is
if Token = Tok_Is then
--- If the subprogram declaration already has a specification, we
--- can't define another.
-if Null_Present (Specification (Decl_Node)) then
+-- If the subprogram is a procedure and already has a
+-- specification, we can't define another.
+
+if Nkind (Specification (Decl_Node)) = N_Procedure_Specification
+ and then Null_Present (Specification (Decl_Node))
+then
Error_Msg_AP ("null procedure cannot have a body");
end if;
[Ada] Reject illegal bodies for null procedures
The `if Token = ...` condition was added when aspect specifications were
introduced in GNAT: aspect specification parsing is done as part of
subprogram declaration parsing, and so once aspect specifications are
parsed, it is necessary to re-start the subprogram declaration-parsing
autotmata. But re-starting the automata is only needed if the parsed
declaration doesn't already have a specification, otherwise there is a
syntax error in the parsed program. This commit thus makes GNAT reject
illegal subprogram declarations such as `procedure p is null is begin
...`.
Tested on x86_64-pc-linux-gnu, committed on trunk
2020-06-10 Ghjuvan Lacambre
gcc/ada/
* par-ch6.adb (P_Subprogram): Reject duplicate subprogram
declarations.--- gcc/ada/par-ch6.adb
+++ gcc/ada/par-ch6.adb
@@ -959,6 +959,13 @@ package body Ch6 is
-- the collected aspects, if any, to the body.
if Token = Tok_Is then
+
+-- If the subprogram declaration already has a specification, we
+-- can't define another.
+if Null_Present (Specification (Decl_Node)) then
+ Error_Msg_AP ("null procedure cannot have a body");
+end if;
+
Scan;
goto Subprogram_Body;
[Ada] Remove unreferenced GNATprove utility routine Get_Low_Bound
Routine Get_Low_Bound was added to frontend while moving utility routines from the GNATprove backend, but it is no longer referenced. (Surprisingly, there is no Get_High_Bound routine anywhere). Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-10 Piotr Trojanek gcc/ada/ * sem_aux.ads, sem_aux.adb (Get_Low_Bound): Remove.--- gcc/ada/sem_aux.adb +++ gcc/ada/sem_aux.adb @@ -485,19 +485,6 @@ package body Sem_Aux is return Id; end Get_Called_Entity; - --- - -- Get_Low_Bound -- - --- - - function Get_Low_Bound (E : Entity_Id) return Node_Id is - begin - if Ekind (E) = E_String_Literal_Subtype then - return String_Literal_Low_Bound (E); - else - return Type_Low_Bound (E); - end if; - end Get_Low_Bound; - -- -- Get_Rep_Item -- -- --- gcc/ada/sem_aux.ads +++ gcc/ada/sem_aux.ads @@ -165,9 +165,6 @@ package Sem_Aux is -- Obtain the entity of the entry, operator, or subprogram being invoked -- by call Call. - function Get_Low_Bound (E : Entity_Id) return Node_Id; - -- For an index subtype or string literal subtype, returns its low bound - function Get_Unary_Nkind (Op : Entity_Id) return Node_Kind; -- Op must be an entity with an Ekind of E_Operator. This function returns -- the Nkind value that would be used to construct a unary operator node
RE: [IMPORTANT] ChangeLog related changes
Hi All, We've been wondering since we no longer list authors in the changelog (at least mklog doesn't generate it), How do we handle multi author patches nowadays? Tried searching for it on the website but couldn’t find anything. Thanks, Tamar > -Original Message- > From: Gcc-patches On Behalf Of Martin > Liška > Sent: Wednesday, June 10, 2020 8:38 AM > To: Jonathan Wakely > Cc: Jakub Jelinek ; g...@gcc.gnu.org; gcc-patches patc...@gcc.gnu.org> > Subject: Re: [IMPORTANT] ChangeLog related changes > > On 6/9/20 10:29 PM, Jonathan Wakely wrote: > > OK, here's a proper patch for the changes you liked, dropping the > > changes to the Error exception type. > > > > pytest contrib/gcc-changelog/test_email.py passes. > > > > OK for master? > > I like it and I've just pushed the patch to master. > > Martin
[PATCH] wwwdocs: Document devel/omp/gcc-10 branch
Hello The devel/omp/gcc-10 branch was pushed yesterday as the new development branch for OpenMP/OpenACC/offloading functionality, based on the GCC 10 release. This patch updates the previous entry on the website for devel/omp/gcc-9 to refer to the new branch instead. This removes references to the old branch, as new development should occur on the newer branch. OK to push? Thanks, Kwok commit ef2aef1c8649a9620f0975a3fe5d4cadaa0b9d1e Author: Kwok Cheung Yeung Date: Wed Jun 10 06:08:06 2020 -0700 Document devel/omp/gcc-10 branch This also removes references to the old devel/omp/gcc-9 branch. diff --git a/htdocs/git.html b/htdocs/git.html index 8c28bc0..292a27a 100644 --- a/htdocs/git.html +++ b/htdocs/git.html @@ -280,15 +280,15 @@ in Git. Makarov mailto:vmaka...@redhat.com;>vmaka...@redhat.com. - https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;a=shortlog;h=refs/heads/devel/omp/gcc-9;>devel/omp/gcc-9 + https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;a=shortlog;h=refs/heads/devel/omp/gcc-10;>devel/omp/gcc-10 This branch is for collaborative development of https://gcc.gnu.org/wiki/OpenACC;>OpenACC and https://gcc.gnu.org/wiki/openmp;>OpenMP support and related functionality, such as https://gcc.gnu.org/wiki/Offloading;>offloading support (OMP: offloading and multi processing). - The branch is based on releases/gcc-9. - Please send patch emails with a short-hand [og9] tag in the + The branch is based on releases/gcc-10. + Please send patch emails with a short-hand [og10] tag in the subject line, and use ChangeLog.omp files. unified-autovect
[PATCH] tree-optimization/95576 - fix compare-debug issue with SLP vectorization
The following avoids leading debug stmts in BB vectorizer regions.
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
2020-06-10 Richard Biener
PR tree-optimization/95576
* tree-vect-slp.c (vect_slp_bb): Skip leading debug stmts.
* g++.dg/vect/pr95576.cc: New testcase.
---
gcc/testsuite/g++.dg/vect/pr95576.cc | 23 +++
gcc/tree-vect-slp.c | 7 ++-
2 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/vect/pr95576.cc
diff --git a/gcc/testsuite/g++.dg/vect/pr95576.cc
b/gcc/testsuite/g++.dg/vect/pr95576.cc
new file mode 100644
index 000..64e0e2dae7f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr95576.cc
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-additional-options "-O3 -fno-tree-forwprop -fcompare-debug" }
+
+struct S {
+ virtual ~S();
+ struct S *s;
+ virtual void m();
+ int f;
+ void *d;
+};
+
+struct T : S {
+ void m();
+};
+
+S::~S() {
+ if (s) {
+s->f = 0;
+s->d = __null;
+ }
+}
+
+void T::m() {}
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 0217a524f05..de82c198309 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -3316,7 +3316,12 @@ vect_slp_bb (basic_block bb)
{
gimple *stmt = gsi_stmt (gsi);
if (is_gimple_debug (stmt))
- continue;
+ {
+ /* Skip leading debug stmts. */
+ if (gsi_stmt (region_begin) == stmt)
+ gsi_next (_begin);
+ continue;
+ }
insns++;
if (gimple_location (stmt) != UNKNOWN_LOCATION)
--
2.26.2
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On 6/10/20 2:27 PM, Martin Liška wrote:
/home/marxin/Programming/testcases/vect-low.c: In function ‘v2df foo(v2df,
v2df, v2df, v2df)’:
/home/marxin/Programming/testcases/vect-low.c:3:6: error: BB 2 is missing an EH
edge
Ok, I was missing copying of the EH edges:
FOR_EACH_EDGE (e, ei, gimple_bb (old_stmt)->succs)
{
if (e->flags & EDGE_EH)
make_edge (gimple_bb (new_stmt), e->dest, e->flags);
}
Martin
[PATCH 2/7 V3] rs6000: lenload/lenstore optab support
V3: Update the define_expand as optab changes.
gcc/ChangeLog:
2020-MM-DD Kewen Lin
* config/rs6000/vsx.md (lenload): New define_expand.
(lenstore): Likewise.
---
gcc/config/rs6000/vsx.md | 32
1 file changed, 32 insertions(+)
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 2a28215ac5b..349da294877 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -5082,6 +5082,38 @@ (define_expand "stxvl"
operands[3] = gen_reg_rtx (DImode);
})
+;; Define optab for vector access with length vectorization exploitation.
+(define_expand "lenload"
+ [(match_operand:VEC_A 0 "vlogical_operand")
+ (match_operand:VEC_A 1 "memory_operand")
+ (match_operand:QI 2 "int_reg_operand")]
+ "TARGET_P9_VECTOR && TARGET_64BIT"
+{
+ rtx mem = XEXP (operands[1], 0);
+ mem = force_reg (DImode, mem);
+ rtx len = gen_lowpart (DImode, operands[2]);
+ rtx res = gen_reg_rtx (V16QImode);
+ emit_insn (gen_lxvl (res, mem, len));
+ emit_move_insn (operands[0], gen_lowpart (mode, res));
+ DONE;
+})
+
+(define_expand "lenstore"
+ [(match_operand:VEC_A 0 "memory_operand")
+ (match_operand:VEC_A 1 "vlogical_operand")
+ (match_operand:QI 2 "int_reg_operand")
+ ]
+ "TARGET_P9_VECTOR && TARGET_64BIT"
+{
+ rtx val = gen_reg_rtx (V16QImode);
+ emit_move_insn (val, gen_lowpart (V16QImode, operands[1]));
+ rtx mem = XEXP (operands[0], 0);
+ mem = force_reg (DImode, mem);
+ rtx len = gen_lowpart (DImode, operands[2]);
+ emit_insn (gen_stxvl (val, mem, len));
+ DONE;
+})
+
(define_insn "*stxvl"
[(set (mem:V16QI (match_operand:DI 1 "gpc_reg_operand" "b"))
(unspec:V16QI
--
[PATCH 1/7 V3] ifn/optabs: Support vector load/store with length
on 2020/6/10 下午5:22, Richard Sandiford wrote:
> "Kewen.Lin" writes:
>> @@ -2497,6 +2499,9 @@ expand_mask_load_optab_fn (internal_fn, gcall *stmt,
>> convert_optab optab)
>>
>>if (optab == vec_mask_load_lanes_optab)
>> icode = get_multi_vector_move (type, optab);
>> + else if (optab == lenload_optab)
>> +icode = convert_optab_handler (optab, TYPE_MODE (type),
>> + targetm.vectorize.length_mode);
>>else
>> icode = convert_optab_handler (optab, TYPE_MODE (type),
>> TYPE_MODE (TREE_TYPE (maskt)));
>
> I think lenload_optab should just be a direct optab, based only on
> the vector mode. It seems unlikely that targets would provide the
> “same” load with different length modes.
Good point! Yes, targets unlikely have this need.
>
>> @@ -2507,15 +2512,20 @@ expand_mask_load_optab_fn (internal_fn, gcall *stmt,
>> convert_optab optab)
>>target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>>create_output_operand ([0], target, TYPE_MODE (type));
>>create_fixed_operand ([1], mem);
>> - create_input_operand ([2], mask, TYPE_MODE (TREE_TYPE (maskt)));
>> + if (optab == lenload_optab)
>> +create_convert_operand_from ([2], mask,
>> targetm.vectorize.length_mode,
>> + TYPE_UNSIGNED (TREE_TYPE (maskt)));
>
> The mode argument should be TYPE_MODE (TREE_TYPE (maskt)) -- i.e. the
> arguments should specify the precision and signedness of the existing rtx.
>
Thanks for correcting this. I found I misunderstood its usage.
> Hopefully this means that we don't need the target hook at all.
>
New version v3 attached to get rid of length mode hook.
BR,
Kewen
---
gcc/ChangeLog:
2020-MM-DD Kewen Lin
* doc/md.texi (lenload@var{m}): Document.
(lenstore@var{m}): Likewise.
* internal-fn.c (len_load_direct): New macro.
(len_store_direct): Likewise.
(expand_len_load_optab_fn): Likewise.
(expand_len_store_optab_fn): Likewise.
(direct_len_load_optab_supported_p): Likewise.
(direct_len_store_optab_supported_p): Likewise.
(expand_mask_load_optab_fn): Add handlings for lenload_optab.
(expand_mask_store_optab_fn): Add handlings for lenstore_optab.
(internal_load_fn_p): Handle IFN_LEN_LOAD.
(internal_store_fn_p): Handle IFN_LEN_STORE.
(internal_fn_stored_value_index): Handle IFN_LEN_STORE.
* internal-fn.def (LEN_LOAD): New internal function.
(LEN_STORE): Likewise.
* optabs.def (lenload_optab, lenstore_optab): New optab.
---
gcc/doc/md.texi | 18 ++
gcc/internal-fn.c | 29 +
gcc/internal-fn.def | 6 ++
gcc/optabs.def | 2 ++
4 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 2c67c818da5..dd0d3ec203b 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5167,6 +5167,24 @@ mode @var{n}.
This pattern is not allowed to @code{FAIL}.
+@cindex @code{lenload@var{m}} instruction pattern
+@item @samp{lenload@var{m}}
+Perform a vector load with length from memory operand 1 of mode @var{m}
+into register operand 0. Length is provided in register operand 2 with
+appropriate mode which should afford the maximal required precision of
+any available lengths.
+
+This pattern is not allowed to @code{FAIL}.
+
+@cindex @code{lenstore@var{m}} instruction pattern
+@item @samp{lenstore@var{m}}
+Perform a vector store with length from register operand 1 of mode @var{m}
+into memory operand 0. Length is provided in register operand 2 with
+appropriate mode which should afford the maximal required precision of
+any available lengths.
+
+This pattern is not allowed to @code{FAIL}.
+
@cindex @code{vec_perm@var{m}} instruction pattern
@item @samp{vec_perm@var{m}}
Output a (variable) vector permutation. Operand 0 is the destination
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 5e9aa60721e..e85df5cbd92 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -104,10 +104,12 @@ init_internal_fns ()
#define load_lanes_direct { -1, -1, false }
#define mask_load_lanes_direct { -1, -1, false }
#define gather_load_direct { 3, 1, false }
+#define len_load_direct { -1, 2, false }
#define mask_store_direct { 3, 2, false }
#define store_lanes_direct { 0, 0, false }
#define mask_store_lanes_direct { 0, 0, false }
#define scatter_store_direct { 3, 1, false }
+#define len_store_direct { 3, 2, false }
#define unary_direct { 0, 0, true }
#define binary_direct { 0, 0, true }
#define ternary_direct { 0, 0, true }
@@ -2478,7 +2480,7 @@ expand_call_mem_ref (tree type, gcall *stmt, int index)
return fold_build2 (MEM_REF, type, addr, build_int_cst (alias_ptr_type, 0));
}
-/* Expand MASK_LOAD{,_LANES} call STMT using optab OPTAB. */
+/* Expand MASK_LOAD{,_LANES} and LEN_LOAD call STMT using optab OPTAB. */
static void
expand_mask_load_optab_fn
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On 6/10/20 12:50 PM, Richard Biener wrote:
with -fnon-call-exceptions should trigger it.
Thanks, that works!
We start with:
foo (v2df a, v2df b, v2df c, v2df d)
Eh tree:
1 try land:{1,} catch:{}
{
void * _1;
v2df _2;
v2df _8;
[local count: 1073741824]:
[LP 1] _8 = VEC_COND_EXPR ;
[local count: 1073741824]:
# _2 = PHI <{ 0.0, 0.0 }(4), _8(2)>
return _2;
[count: 0]:
: [LP 1]
_1 = __builtin_eh_pointer (1);
__cxa_begin_catch (_1);
__cxa_end_catch ();
goto ; [0.00%]
I tried to use:
maybe_clean_or_replace_eh_stmt (stmt, assign);
which does:
[local count: 1073741824]:
[LP 1] _12 = a_4(D) < b_5(D);
[local count: 1073741824]:
_8 = VEC_COND_EXPR <_12, c_6(D), d_7(D)>;
which requires to split the BB. But now I'm missing an edge:
/home/marxin/Programming/testcases/vect-low.c: In function ‘v2df foo(v2df,
v2df, v2df, v2df)’:
/home/marxin/Programming/testcases/vect-low.c:3:6: error: BB 2 is missing an EH
edge
Am I doing that correctly? Or do we have a better function for it?
Martin
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 10/06/20 13:13 +0100, Jonathan Wakely wrote: On 10/06/20 09:43 +0200, Martin Liška wrote: On 6/9/20 9:46 PM, Jonathan Wakely wrote: Is this worth adding to contrib/prepare-commit-msg? I like the idea and I would make it conditional based on an environment variable? Similarly to GCC_GIT_DIFF_FILE? With this patch you can use the gcc-config.use-mklog-hook config key instead of defining GCC_FORCE_MKLOG in the environment. My new behaviour is enabled when that variable is set to 'smart-amend' (either in the environment, or in your git config). WDYT? commit a58493ff8f3d36645c6d4fad2fca04e3db2d267c Author: Jonathan Wakely Date: Wed Jun 10 12:48:40 2020 +0100 contrib: Make prepare-commit-msg hook smarter With this change defining GCC_FORCE_MKLOG=smart-amend in the environment will cause the prepare-commit-msg hook to compare the log of a commit being amended with the staged changes, and not run mklog.py unnecessarily. This also allows defining gcc-config.use-mklog-hook in Git config instead of the GCC_FORCE_MKLOG environment variable, and allows GCC_FORCE_MKLOG=no to do nothing even if the config key is set. contrib/ChangeLog: * prepare-commit-msg: Do not generate a new ChangeLog template for amended commits where the existing log is still OK. Check the gcc-config.use-mklog-hook Git config key if the GCC_FORCE_MKLOG environment variable is empty. diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index 24f0783aae2..cfcf0e7afe5 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -30,7 +30,11 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi # Don't do anything unless requested to. -if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi +if [ -z "$GCC_FORCE_MKLOG" ]; then +GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0 +echo xxx $GCC_FORCE_MKLOG Oops, this line was left in while I was testing it by amending existing commits! Here's an updated patch without that line. commit cd16c99a5fe281fc60b2023fc302994580078ca1 Author: Jonathan Wakely Date: Wed Jun 10 12:48:40 2020 +0100 contrib: Make prepare-commit-msg hook smarter With this change defining GCC_FORCE_MKLOG=smart-amend in the environment will cause the prepare-commit-msg hook to compare the log of a commit being amended with the staged changes, and not run mklog.py unnecessarily. This also allows defining gcc-config.use-mklog-hook in Git config instead of the GCC_FORCE_MKLOG environment variable, and allows GCC_FORCE_MKLOG=no to do nothing even if the config key is set. contrib/ChangeLog: * prepare-commit-msg: Do not generate a new ChangeLog template for amended commits where the existing log is still OK. Check the gcc-config.use-mklog-hook Git config key if the GCC_FORCE_MKLOG environment variable is empty. contrib/ChangeLog: * prepare-commit-msg: diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index 24f0783aae2..34f94012eae 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -30,7 +30,10 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi # Don't do anything unless requested to. -if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi +if [ -z "$GCC_FORCE_MKLOG" ]; then +GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0 +fi +if [ -z "$GCC_FORCE_MKLOG" ] || [ $GCC_FORCE_MKLOG = no ]; then exit 0; fi if [ -z "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then # No source or "template" means new commit. @@ -49,6 +52,19 @@ elif [ $COMMIT_SOURCE = commit ]; then # otherwise, assume a new commit with -C. if [ $SHA1 = HEAD ]; then cmd="diff --cached HEAD^" + if [ "$GCC_FORCE_MKLOG" = "smart-amend" ]; then + # Check if the existing message still describes the staged changes. + f=$(mktemp /tmp/git-commit.XX) || exit 1 + git log -1 --pretty=email HEAD > $f + printf '\n---\n\n' >> $f + git $cmd >> $f + if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then + # Existing commit message is still OK for amended commit. + rm $f + exit 0 + fi + rm $f + fi else cmd="diff --cached" fi
[PATCH] Make {SLP_TREE,STMT_VINFO}_VEC_STMTS a vector of gimple *
This makes {SLP_TREE,STMT_VINFO}_VEC_STMTS a vector of gimple * and
not allocate a stmt_vec_info for vectorizer generated stmts since
this is now possible after removing the only use which was chaining
of vector stmts via STMT_VINFO_RELATED_STMT.
This also removes all stmt_vec_info allocations done for vector
stmts, the remaining ones are for stmts in the scalar IL and for
patterns which are not part of the IL. Thus after this the stmt
UIDs inside a basic-block are suitable for dominance checking
if you ignore (or lazy-fill) UIDs of zero of the vector stmts
inserted during transform. This property is ensured by a new
flag set when pattern analysis is complete.
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
Richard.
2020-06-10 Richard Biener
* tree-vectorizer.h (_slp_tree::vec_stmts): Make it a vector
of gimple * stmts.
(_stmt_vec_info::vec_stmts): Likewise.
(vec_info::stmt_vec_info_ro): New flag.
(vect_finish_replace_stmt): Adjust declaration.
(vect_finish_stmt_generation): Likewise.
(vectorizable_induction): Likewise.
(vect_transform_reduction): Likewise.
(vectorizable_lc_phi): Likewise.
* tree-vect-data-refs.c (vect_create_data_ref_ptr): Do not
allocate stmt infos for increments.
(vect_record_grouped_load_vectors): Adjust.
* tree-vect-loop.c (vect_create_epilog_for_reduction): Likewise.
(vectorize_fold_left_reduction): Likewise.
(vect_transform_reduction): Likewise.
(vect_transform_cycle_phi): Likewise.
(vectorizable_lc_phi): Likewise.
(vectorizable_induction): Likewise.
(vectorizable_live_operation): Likewise.
(vect_transform_loop): Likewise.
* tree-vect-patterns.c (vect_pattern_recog): Set stmt_vec_info_ro.
* tree-vect-slp.c (vect_get_slp_vect_def): Adjust.
(vect_get_slp_defs): Likewise.
(vect_transform_slp_perm_load): Likewise.
(vect_schedule_slp_instance): Likewise.
(vectorize_slp_instance_root_stmt): Likewise.
* tree-vect-stmts.c (vect_get_vec_defs_for_operand): Likewise.
(vect_finish_stmt_generation_1): Do not allocate a stmt info.
(vect_finish_replace_stmt): Do not return anything.
(vect_finish_stmt_generation): Likewise.
(vect_build_gather_load_calls): Adjust.
(vectorizable_bswap): Likewise.
(vectorizable_call): Likewise.
(vectorizable_simd_clone_call): Likewise.
(vect_create_vectorized_demotion_stmts): Likewise.
(vectorizable_conversion): Likewise.
(vectorizable_assignment): Likewise.
(vectorizable_shift): Likewise.
(vectorizable_operation): Likewise.
(vectorizable_scan_store): Likewise.
(vectorizable_store): Likewise.
(vectorizable_load): Likewise.
(vectorizable_condition): Likewise.
(vectorizable_comparison): Likewise.
(vect_transform_stmt): Likewise.
* tree-vectorizer.c (vec_info::vec_info): Initialize
stmt_vec_info_ro.
(vec_info::replace_stmt): Copy over stmt UID rather than
unsetting/setting a stmt info allocating a new UID.
(vec_info::set_vinfo_for_stmt): Assert !stmt_vec_info_ro.
---
gcc/tree-vect-data-refs.c | 8 +-
gcc/tree-vect-loop.c | 112 -
gcc/tree-vect-patterns.c | 3 +
gcc/tree-vect-slp.c | 47 ++--
gcc/tree-vect-stmts.c | 461 --
gcc/tree-vectorizer.c | 5 +-
gcc/tree-vectorizer.h | 26 +--
7 files changed, 282 insertions(+), 380 deletions(-)
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 7edd9ebe3b6..39d5a1b554c 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -4896,7 +4896,6 @@ vect_create_data_ref_ptr (vec_info *vinfo, stmt_vec_info
stmt_info,
aggr_ptr, loop, _gsi, insert_after,
_before_incr, _after_incr);
incr = gsi_stmt (incr_gsi);
- loop_vinfo->add_stmt (incr);
/* Copy the points-to information if it exists. */
if (DR_PTR_INFO (dr))
@@ -4926,7 +4925,6 @@ vect_create_data_ref_ptr (vec_info *vinfo, stmt_vec_info
stmt_info,
containing_loop, _gsi, insert_after, _before_incr,
_after_incr);
incr = gsi_stmt (incr_gsi);
- loop_vinfo->add_stmt (incr);
/* Copy the points-to information if it exists. */
if (DR_PTR_INFO (dr))
@@ -6407,7 +6405,7 @@ vect_transform_grouped_load (vec_info *vinfo,
stmt_vec_info stmt_info,
for each vector to the associated scalar statement. */
void
-vect_record_grouped_load_vectors (vec_info *vinfo, stmt_vec_info stmt_info,
+vect_record_grouped_load_vectors (vec_info *, stmt_vec_info stmt_info,
vec result_chain)
{
stmt_vec_info first_stmt_info = DR_GROUP_FIRST_ELEMENT (stmt_info);
@@ -6441,10 +6439,10 @@ vect_record_grouped_load_vectors
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 10/06/20 13:13 +0100, Jonathan Wakely wrote: On 10/06/20 09:43 +0200, Martin Liška wrote: On 6/9/20 9:46 PM, Jonathan Wakely wrote: Is this worth adding to contrib/prepare-commit-msg? I like the idea and I would make it conditional based on an environment variable? Similarly to GCC_GIT_DIFF_FILE? With this patch you can use the gcc-config.use-mklog-hook config key instead of defining GCC_FORCE_MKLOG in the environment. My new behaviour is enabled when that variable is set to 'smart-amend' (either in the environment, or in your git config). And this is another little patch that just avoids running 'git diff' twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect of generating the diff to pipe to mklog.py. commit e147d6164775d004d608501963befeb551732529 Author: Jonathan Wakely Date: Wed Jun 10 13:05:39 2020 +0100 contrib: Avoid redundant 'git diff' in prepare-commit-msg hook contrib/ChangeLog: * prepare-commit-msg: Use 'tee' to save the diff to a file instead of running 'git diff' twice. diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index cfcf0e7afe5..d27861f4cd6 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -76,7 +76,9 @@ fi # Save diff to a file if requested. if ! [ -z "$GCC_GIT_DIFF_FILE" ]; then - git $cmd > "$GCC_GIT_DIFF_FILE"; +tee="tee $GCC_GIT_DIFF_FILE" +else +tee="cat" fi -git $cmd | git gcc-mklog -c "$COMMIT_MSG_FILE" +git $cmd | $tee | git gcc-mklog -c "$COMMIT_MSG_FILE"
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 10/06/20 09:43 +0200, Martin Liška wrote: On 6/9/20 9:46 PM, Jonathan Wakely wrote: Is this worth adding to contrib/prepare-commit-msg? I like the idea and I would make it conditional based on an environment variable? Similarly to GCC_GIT_DIFF_FILE? With this patch you can use the gcc-config.use-mklog-hook config key instead of defining GCC_FORCE_MKLOG in the environment. My new behaviour is enabled when that variable is set to 'smart-amend' (either in the environment, or in your git config). WDYT? commit a58493ff8f3d36645c6d4fad2fca04e3db2d267c Author: Jonathan Wakely Date: Wed Jun 10 12:48:40 2020 +0100 contrib: Make prepare-commit-msg hook smarter With this change defining GCC_FORCE_MKLOG=smart-amend in the environment will cause the prepare-commit-msg hook to compare the log of a commit being amended with the staged changes, and not run mklog.py unnecessarily. This also allows defining gcc-config.use-mklog-hook in Git config instead of the GCC_FORCE_MKLOG environment variable, and allows GCC_FORCE_MKLOG=no to do nothing even if the config key is set. contrib/ChangeLog: * prepare-commit-msg: Do not generate a new ChangeLog template for amended commits where the existing log is still OK. Check the gcc-config.use-mklog-hook Git config key if the GCC_FORCE_MKLOG environment variable is empty. diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index 24f0783aae2..cfcf0e7afe5 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -30,7 +30,11 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi # Don't do anything unless requested to. -if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi +if [ -z "$GCC_FORCE_MKLOG" ]; then +GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0 +echo xxx $GCC_FORCE_MKLOG +fi +if [ -z "$GCC_FORCE_MKLOG" ] || [ $GCC_FORCE_MKLOG = no ]; then exit 0; fi if [ -z "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then # No source or "template" means new commit. @@ -49,6 +53,19 @@ elif [ $COMMIT_SOURCE = commit ]; then # otherwise, assume a new commit with -C. if [ $SHA1 = HEAD ]; then cmd="diff --cached HEAD^" + if [ "$GCC_FORCE_MKLOG" = "smart-amend" ]; then + # Check if the existing message still describes the staged changes. + f=$(mktemp /tmp/git-commit.XX) || exit 1 + git log -1 --pretty=email HEAD > $f + printf '\n---\n\n' >> $f + git $cmd >> $f + if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then + # Existing commit message is still OK for amended commit. + rm $f + exit 0 + fi + rm $f + fi else cmd="diff --cached" fi
Re: [PATCH] Add missing store in emission of asan_stack_free.
On Wed, Jun 10, 2020 at 01:14:59PM +0200, Martin Liška wrote: > >From 4d2e0b1e87b08ec21fd82144f00d364687030706 Mon Sep 17 00:00:00 2001 > From: Martin Liska > Date: Tue, 19 May 2020 16:57:56 +0200 > Subject: [PATCH] Add missing store in emission of asan_stack_free. > > gcc/ChangeLog: > > 2020-05-19 Martin Liska > > PR sanitizer/94910 > * asan.c (asan_emit_stack_protection): Emit > also **SavedFlagPtr(FakeStack) = 0 in order to release > a stack frame. Please adjust the ChangeLog too. Ok with that change. Jakub
Re: [PATCH] Add missing store in emission of asan_stack_free.
On 6/10/20 12:08 PM, Jakub Jelinek wrote:
On Wed, Jun 10, 2020 at 11:49:01AM +0200, Martin Liška wrote:
- store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
-BITS_PER_UNIT, true, RETURN_BEGIN);
+ {
+ /* Emit:
+ memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+ **SavedFlagPtr(FakeStack) = 0
SavedFlagPtr has two arguments, doesn't it?
Good point, I copied that from
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
which has it wrong. Fixed that.
+ */
+ store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+ BITS_PER_UNIT, true, RETURN_BEGIN);
+
+ unsigned HOST_WIDE_INT offset
+ = (1 << (use_after_return_class + 6));
+ offset -= GET_MODE_SIZE (ptr_mode);
So, mem here is a MEM into which we stored ASAN_STACK_RETIRED_MAGIC.
Ok, we should rather start from 'base'.
+ mem = adjust_address (mem, ptr_mode, offset);
This adds offset to it and changes mode to ptr_mode. So,
mem is now *(ptr_mode)(_mem + offset)
+ rtx addr = gen_reg_rtx (ptr_mode);
+ emit_move_insn (addr, mem);
We load that value.
+ mem = gen_rtx_MEM (ptr_mode, addr);
+ mem = adjust_address (mem, QImode, 0);
And here I'm lost why you do that. If you want to store a single
byte into what it points to, then why don't you just
mem = gen_rtx_MEM (QImode, addr);
instead of the above two lines?
Because I'm not so much familiar with RTL ;)
adjust_address will return a MEM like the above, with offset not adjusted
(as the addition is 0) and mode changed to QImode, but there is no reason
not to create it already in QImode.
All right.
What about this?
Martin
+ emit_move_insn (mem, const0_rtx);
+ }
else if (use_after_return_class >= 5
|| !set_storage_via_setmem (shadow_mem,
GEN_INT (sz),
--
2.26.2
Jakub
>From 4d2e0b1e87b08ec21fd82144f00d364687030706 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Tue, 19 May 2020 16:57:56 +0200
Subject: [PATCH] Add missing store in emission of asan_stack_free.
gcc/ChangeLog:
2020-05-19 Martin Liska
PR sanitizer/94910
* asan.c (asan_emit_stack_protection): Emit
also **SavedFlagPtr(FakeStack) = 0 in order to release
a stack frame.
---
gcc/asan.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index c9872f1b007..e015fa3ec9b 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1598,8 +1598,24 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
if (use_after_return_class < 5
&& can_store_by_pieces (sz, builtin_memset_read_str, ,
BITS_PER_UNIT, true))
- store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
- BITS_PER_UNIT, true, RETURN_BEGIN);
+ {
+ /* Emit:
+ memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+ **SavedFlagPtr(FakeStack, class_id) = 0
+ */
+ store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+ BITS_PER_UNIT, true, RETURN_BEGIN);
+
+ unsigned HOST_WIDE_INT offset
+ = (1 << (use_after_return_class + 6));
+ offset -= GET_MODE_SIZE (ptr_mode);
+ mem = gen_rtx_MEM (ptr_mode, base);
+ mem = adjust_address (mem, ptr_mode, offset);
+ rtx addr = gen_reg_rtx (ptr_mode);
+ emit_move_insn (addr, mem);
+ mem = gen_rtx_MEM (QImode, addr);
+ emit_move_insn (mem, const0_rtx);
+ }
else if (use_after_return_class >= 5
|| !set_storage_via_setmem (shadow_mem,
GEN_INT (sz),
--
2.26.2
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On Wed, Jun 10, 2020 at 10:51 AM Martin Liška wrote:
>
> On 6/9/20 3:42 PM, Richard Biener wrote:
> > On Mon, Jun 8, 2020 at 1:04 PM Martin Liška wrote:
> >>
> >> Hello.
> >>
> >> Thank you for the approval. There's the patch that defines 4 new
> >> DEF_INTERNAL_OPTAB_FN.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >> It also builds on ppc64le-linux-gnu.
> >>
> >> Ready to be installed?
> >
> > The ChangeLog refers to DEF_INTERNAL_OPTAB_CAN_FAIL which is no longer
> > there.
>
>
> Sure.
>
> >
> > Can you put the isel pass to a separate file please?
>
> Yes.
>
> >
> > So this is a first step towards sanitizing VEC_COND_EXPR. There were
> > followups
> > mentioned, namely a) enforcing that VEC_COND_EXPR constraint everywhere,
> > b) isel vector comparisons at the same time since expansion has a
> > vec_cond fallback
>
> I'm planning to work on the follow up.
>
> >
> > There's
> >
> > + /* ??? If we do not cleanup EH then we will ICE in
> > +verification. But in reality we have created wrong-code
> > +as we did not properly transition EH info and edges to
> > +the piecewise computations. */
> > + if (maybe_clean_eh_stmt (gsi_stmt (gsi))
> > + && gimple_purge_dead_eh_edges (bb))
> > + cfg_changed = true;
>
> Hm, I've tried to comment the code in both ISEL and expansion and I can't
> find a test-case
> that would trigger a verification error (in vect.exp and i386.exp). Can you
> come up with
> something that will trigger the code?
typedef double v2df __attribute__((vector_size(16)));
v2df foo (v2df a, v2df b, v2df c, v2df d)
{
try
{
v2df res = a < b ? c : d;
return res;
}
catch (...)
{
return (v2df){};
}
}
with -fnon-call-exceptions should trigger it.
> >
> > which of course is bad. It's the comparison that can throw and I guess
> > current
> > RTL expansion manages to cope by find_many_sub_bbs and friends. But we
> > need to get this correct on GIMPLE here. Note I find it odd this only
> > triggers
> > during ISEL - it should trigger during the lowering step which splits
> > the comparison
> > from the VEC_COND_EXPR. An appropriate fix at lowering time would be to
> > insert the VEC_COND_EXPR w/o the condition on the normal outgoing edge
> > and keep the comparison in place of the original VEC_COND_EXPR,
> > moving EH info from the VEC_COND_EXPR to the comparison.
>
> Ah ok, so it's about correction of EH..
>
> Martin
>
> >
> > I think we need to fix that before merging.
> >
> > Thanks,
> > Richard.
> >
> >> Thanks,
> >> Martin
>
Re: [PATCH] libsanitizer: use gnu++14
On Tue, Jun 9, 2020 at 9:16 PM Martin Liška wrote: > > On 6/9/20 6:32 PM, Richard Biener wrote: > > On Tue, Jun 9, 2020 at 10:09 AM Martin Liška wrote: > >> > >> On 6/8/20 4:53 PM, Martin Liška wrote: > >>> Hi. > >>> > >>> Thank you for the report. It's caused by fact that LLVM switch in > >>> 4d474e078ac7 > >>> to c++14. So that I suggest to use gnu++14. > >>> > >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >>> I also verified that abidiff is equal for all libsanitizer shared > >>> libraries. > >>> I'm going to install the patch if there are no objections. > >>> > >>> Thanks, > >>> Martin > >> > >> Installed as 942a384ef9f. > >> @Andreas: Can you please check the riscv64 build? > > > > Note we need to document (and configure test?) the build > > requirement for non-bootstrap and asan/ubsan bootstraps. > > My impression was that libsanitizer is always built with just built GCC? I guess you are right. > Note that similarly the run-time needed c++11 since: > > commit c5be964a423f952e2ec16e2152ae504639bf8f07 > Author: Kostya Serebryany > Date: Thu Nov 13 20:41:38 2014 + > > libsanitizer merge from upstream r221802 > > From-SVN: r217518 > > which was a time when GCC was likely built with c++98. > > > > > For now we only document the requirement of a C++11 > > host compiler. Also not sure whether using -std=gnu++1y > > would allow more released compilers to build the code? > > For example GCC 4.8.5 knows -std=gnu++1y but not > > -std=gnu++14 and GCC 4.8.3 is required for bootstrap anyway. > > Do we really care about these compilers as one typically (always?) > use newly built GCC? > > Martin > > > > > Richard. > > > >> Martin >
Re: [PATCH] Add missing store in emission of asan_stack_free.
On Wed, Jun 10, 2020 at 11:49:01AM +0200, Martin Liška wrote:
> - store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> - BITS_PER_UNIT, true, RETURN_BEGIN);
> + {
> + /* Emit:
> +memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
> +**SavedFlagPtr(FakeStack) = 0
SavedFlagPtr has two arguments, doesn't it?
> + */
> + store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> +BITS_PER_UNIT, true, RETURN_BEGIN);
> +
> + unsigned HOST_WIDE_INT offset
> + = (1 << (use_after_return_class + 6));
> + offset -= GET_MODE_SIZE (ptr_mode);
So, mem here is a MEM into which we stored ASAN_STACK_RETIRED_MAGIC.
> + mem = adjust_address (mem, ptr_mode, offset);
This adds offset to it and changes mode to ptr_mode. So,
mem is now *(ptr_mode)(_mem + offset)
> + rtx addr = gen_reg_rtx (ptr_mode);
> + emit_move_insn (addr, mem);
We load that value.
> + mem = gen_rtx_MEM (ptr_mode, addr);
> + mem = adjust_address (mem, QImode, 0);
And here I'm lost why you do that. If you want to store a single
byte into what it points to, then why don't you just
mem = gen_rtx_MEM (QImode, addr);
instead of the above two lines?
adjust_address will return a MEM like the above, with offset not adjusted
(as the addition is 0) and mode changed to QImode, but there is no reason
not to create it already in QImode.
> + emit_move_insn (mem, const0_rtx);
> + }
>else if (use_after_return_class >= 5
> || !set_storage_via_setmem (shadow_mem,
> GEN_INT (sz),
> --
> 2.26.2
>
Jakub
Re: [PATCH] Add missing store in emission of asan_stack_free.
On 6/10/20 10:42 AM, Jakub Jelinek wrote:
E.g. we just shouldn't reuse
MEMs (even after adjusting them) from different indirection levels because
we risk some attributes (alias set, MEM_EXPR, whatever else) will stay
around from the different indirection level.
All right, what about the updated patch? I must confess that RTL instruction
emission is not my strength.
Martin
>From 16e46a532c059930887bc30f82c3054a75a5a56d Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Tue, 19 May 2020 16:57:56 +0200
Subject: [PATCH] Add missing store in emission of asan_stack_free.
gcc/ChangeLog:
2020-05-19 Martin Liska
PR sanitizer/94910
* asan.c (asan_emit_stack_protection): Emit
also **SavedFlagPtr(FakeStack) = 0 in order to release
a stack frame.
---
gcc/asan.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index c9872f1b007..232341f5c4b 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1598,8 +1598,24 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
if (use_after_return_class < 5
&& can_store_by_pieces (sz, builtin_memset_read_str, ,
BITS_PER_UNIT, true))
- store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
- BITS_PER_UNIT, true, RETURN_BEGIN);
+ {
+ /* Emit:
+ memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+ **SavedFlagPtr(FakeStack) = 0
+ */
+ store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+ BITS_PER_UNIT, true, RETURN_BEGIN);
+
+ unsigned HOST_WIDE_INT offset
+ = (1 << (use_after_return_class + 6));
+ offset -= GET_MODE_SIZE (ptr_mode);
+ mem = adjust_address (mem, ptr_mode, offset);
+ rtx addr = gen_reg_rtx (ptr_mode);
+ emit_move_insn (addr, mem);
+ mem = gen_rtx_MEM (ptr_mode, addr);
+ mem = adjust_address (mem, QImode, 0);
+ emit_move_insn (mem, const0_rtx);
+ }
else if (use_after_return_class >= 5
|| !set_storage_via_setmem (shadow_mem,
GEN_INT (sz),
--
2.26.2
Re: [PATCH 1/7 V2] ifn/optabs: Support vector load/store with length
"Kewen.Lin" writes: > @@ -2497,6 +2499,9 @@ expand_mask_load_optab_fn (internal_fn, gcall *stmt, > convert_optab optab) > >if (optab == vec_mask_load_lanes_optab) > icode = get_multi_vector_move (type, optab); > + else if (optab == lenload_optab) > +icode = convert_optab_handler (optab, TYPE_MODE (type), > +targetm.vectorize.length_mode); >else > icode = convert_optab_handler (optab, TYPE_MODE (type), > TYPE_MODE (TREE_TYPE (maskt))); I think lenload_optab should just be a direct optab, based only on the vector mode. It seems unlikely that targets would provide the “same” load with different length modes. > @@ -2507,15 +2512,20 @@ expand_mask_load_optab_fn (internal_fn, gcall *stmt, > convert_optab optab) >target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE); >create_output_operand ([0], target, TYPE_MODE (type)); >create_fixed_operand ([1], mem); > - create_input_operand ([2], mask, TYPE_MODE (TREE_TYPE (maskt))); > + if (optab == lenload_optab) > +create_convert_operand_from ([2], mask, > targetm.vectorize.length_mode, > + TYPE_UNSIGNED (TREE_TYPE (maskt))); The mode argument should be TYPE_MODE (TREE_TYPE (maskt)) -- i.e. the arguments should specify the precision and signedness of the existing rtx. Hopefully this means that we don't need the target hook at all. Thanks, Richard
