Re: PR 89864 - gcc fails to build/bootstrap with XCode 10.2
Hi Eric,
Thanks for working on this!
> On 4 Apr 2019, at 04:00, Erik Schnetter wrote:
>
> Fixinclude the header file that incorrectly uses the C-only
> _Atomic keyword when compiled as C++. Apply the same work-around for two
> GCC source files that transitively use this header file.
1/
If the fix-include is doing the right thing, then there should be no need for a
workaround in any GCC file including it.
Perhaps the wrap needs to be around the (SDK) header(s) that is/are
including ?
(so if is including , then the wrap needs to be in
that
file, not in the GCC files that use it).
2/
I’m somewhat unhappy that this will hack around the problem and silently do
something that can’t work properly in any case that we started to use the
field/struct. What didn’t work about your proposed substitution of the
appropriate
c++ stuff?
thanks
Iain
>
> Index: fixincludes/inclhack.def
> ===
> --- fixincludes/inclhack.def (revision 270127)
> +++ fixincludes/inclhack.def (working copy)
> @@ -1298,6 +1298,36 @@ fix = {
> };
>
> /*
> + * macOS 10.14.4 uses the C _Atomic keyword in C++ code.
> + */
> +fix = {
> +hackname = darwin_ucred__Atomic;
> +mach = "*-*-darwin18.5.*";
> +files = sys/ucred.h;
> +select= "_Atomic";
> +
> +c_fix = wrap;
> +
> +c_fix_arg = <<- _EOArg_
> +
> + #if defined(__cplusplus) && __cplusplus >= 201103L
> + # define _Atomic volatile
> + #endif
> +
> + _EOArg_;
> +
> +c_fix_arg = <<- _EOArg_
> +
> + #if defined(__cplusplus) && __cplusplus >= 201103L
> + # undef _Atomic
> + #endif
> +
> + _EOArg_;
> +
> +test_text = "#include \n";
> +};
> +
> +/*
> * For the AAB_darwin7_9_long_double_funcs fix to be useful,
> * you have to not use "" includes.
> */
> Index: gcc/config/darwin-driver.c
> ===
> --- gcc/config/darwin-driver.c (revision 270127)
> +++ gcc/config/darwin-driver.c (working copy)
> @@ -27,7 +27,15 @@ along with GCC; see the file COPYING3.
> #include "diagnostic-core.h"
>
> #ifndef CROSS_DIRECTORY_STRUCTURE
> +#if defined(__cplusplus) && __cplusplus >= 201103L
> +// This is safe because it applies only to struct ucred, which is
> +// not actually used by gcc.
> +#define _Atomic volatile
> +#endif
> #include
> +#if defined(__cplusplus) && __cplusplus >= 201103L
> +#undef _Atomic
> +#endif
> #include "xregex.h"
>
> static char *
> Index: libsanitizer/sanitizer_common/sanitizer_mac.cc
> ===
> --- libsanitizer/sanitizer_common/sanitizer_mac.cc (revision 270127)
> +++ libsanitizer/sanitizer_common/sanitizer_mac.cc (working copy)
> @@ -67,7 +67,13 @@ extern "C" {
> #include
> #include
> #include
> +#if defined(__cplusplus) && __cplusplus >= 201103L
> +# define _Atomic volatile
> +#endif
> #include
> +#if defined(__cplusplus) && __cplusplus >= 201103L
> +# undef _Atomic
> +#endif
> #include
> #include
> #include
>
>
> --
> Erik Schnetter
> http://www.perimeterinstitute.ca/personal/eschnetter/
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On Wed, 3 Apr 2019 at 20:24, Ian Lance Taylor wrote: > > On Wed, Apr 3, 2019 at 6:19 AM Christophe Lyon > wrote: > > > > Thanks, here is what I have committed as r270126. > > (skip one test on short_enums targets, skip the other one on > > non-short_enums targets) > > I noticed that your testsuite/ChangeLog entry is marked 2019-04-13, > but it should perhaps be 2019-04-03. > Oops, sorry. I've just fixed it, thanks Christophe > Ian
[PATCH] backport r257541, r259936, r260294, r260623, r261098, r261333, r268585.
From: Xiong Hu Luo
These patches are followed changes for r25 on testcases
vsx-vector-6*.c. backport them to update file names and fix regressions
for GCC7 on power9.
Regression tested on power7-be, power8-be, power8-le, power9.
gcc/ChangeLog:
2019-04-03 Xiong Hu Luo
backport from trunk r260623.
2018-05-23 Segher Boessenkool
* doc/sourcebuild.texi (Endianness): New subsubsection.
gcc/testsuite/ChangeLog:
2019-04-03 Xiong Hu Luo
backport from trunk r257541.
2018-02-07 Will Schmidt
* gcc.target/powerpc/vsx-vector-6-le.c: Update CPU target.
* gcc.target/powerpc/vsx-vector-6-le.p9.c: New.
backport from trunk r259936.
2018-05-04 Carl Love
* gcc.target/powerpc/vsx-vector-6.h (foo): Add test for vec_max,
vec_trunc.
* gcc.target/powerpc/vsx-vector-6-le.c (dg-final): Update xvcmpeqdp,
xvcmpgtdp, xvcmpgedp counts. Add xxsel counts.
* gcc.target/powerpc/vsx-vector-6-be.c (dg-final): Update xvcmpgtdp,
xvcmpgedp counts. Add xxsel counts.
backport from trunk r260294.
2018-05-16 Carl Love
* gcc.target/powerpc/vsx-vector-6-be.c: Remove file.
* gcc.target/powerpc/vsx-vector-6-be.p7.c: New test file.
* gcc.target/powerpc/vsx-vector-6-be.p8.c: New test file.
* gcc.target/powerpc/vsx-vector-6-le.c (dg-final): Update counts for
xvcmpeqdp., xvcmpgtdp., xvcmpgedp., xxlxor, xvrdpi.
backport from trunk r260623.
2018-05-23 Segher Boessenkool
* lib/target-supports.exp (check_effective_target_be): New.
(check_effective_target_le): New.
backport from part of trunk r261097.
2018-06-01 Carl Love
* gcc.target/powerpc/altivec-7-be.c: Delete file.
* gcc.target/powerpc/altivec-7-le.c: Delete file.
* gcc.target/powerpc/vsx-7-be.c: Remove file.
backport from trunk r261098.
2018-06-01 Carl Love
Commit 260294 on 2018-05-16 by Carl Love was supposed to add the
following files.
* gcc.target/powerpc/vsx-vector-6-be.p7.c: New test file.
* gcc.target/powerpc/vsx-vector-6-be.p8.c: New test file.
backport from trunk r261333.
2018-06-08 Carl Love
* gcc.target/powerpc/vsx-vector-6-be.p7.c: Rename this file to
vsx-vector-6.p7.c.
* gcc.target/powerpc/vsx-vector-6-le.p9.c: Rename this file to
vsx-vector-6.p9.c.
* gcc.target/powerpc/vsx-vector-6-be.p8.c: Move instruction counts
for BE system that are different then for an LE system from this file
into vsx-vector-6-le.c using be target qualifier. Remove this file.
* gcc.target/powerpc/vsx-vector-6-le.c: Add le qualifiers as needed for
the various instruction counts. Rename file to vsx-vector-6.p8.c.
backport from trunk r268585.
2019-02-06 Bill Seurer
* gcc.target/powerpc/vsx-vector-6.p7.c: Update instruction
counts and target.
* gcc.target/powerpc/vsx-vector-6.p8.c: Update instruction
counts and target.
* gcc.target/powerpc/vsx-vector-6.p9.c: Update instruction
counts and target.
---
gcc/doc/sourcebuild.texi | 10
gcc/testsuite/gcc.target/powerpc/altivec-7-be.c| 30
gcc/testsuite/gcc.target/powerpc/altivec-7-le.c| 37 ---
gcc/testsuite/gcc.target/powerpc/vsx-7-be.c| 50
gcc/testsuite/gcc.target/powerpc/vsx-vector-6-be.c | 31 -
gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c | 32 -
gcc/testsuite/gcc.target/powerpc/vsx-vector-6.h| 14 +-
gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c | 42 +
gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c | 54 ++
gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p9.c | 39
gcc/testsuite/lib/target-supports.exp | 16 +++
11 files changed, 173 insertions(+), 182 deletions(-)
delete mode 100644 gcc/testsuite/gcc.target/powerpc/altivec-7-be.c
delete mode 100644 gcc/testsuite/gcc.target/powerpc/altivec-7-le.c
delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-7-be.c
delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-be.c
delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p9.c
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index c7bb4b7..f0e9bb8 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1273,6 +1273,16 @@ By convention, keywords ending in @code{_nocache} can
also include options
specified for the particular test in an earlier @code{dg-options} or
@code{dg-add-options} direct
[PATCH][MIPS] Fix pr89623 Can't build mips-wrs-vxworks cross-compiler
Hi, The MIPS target run out of Mask in mips.opt, we are stage4, this patch retrieve loongson-ext that haven't used yet for now. In next stage1, I will rewrite those part use HOST_WIDE_INT or same thing like that. Ok for commit ? 2019-04-04 Chenghua Xu gcc/ PR target/89623 * config/mips/mips.opt (LOONGSON_EXT2): Use Var instead of Mask. From a7671686bb820a6be896e6c75f5d2dd23dc1441f Mon Sep 17 00:00:00 2001 From: Chenghua Xu Date: Thu, 4 Apr 2019 16:11:50 +0800 Subject: [PATCH] [MIPS] Set loongson-ext2 options to Var instead of Mask. 2019-04-04 Chenghua Xu gcc/ PR target/89623 * config/mips/mips.opt (LOONGSON_EXT2): Use Var instead of Mask. --- gcc/config/mips/mips.opt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index f3702c4..817a482 100644 --- a/gcc/config/mips/mips.opt +++ b/gcc/config/mips/mips.opt @@ -473,5 +473,5 @@ Target Report Mask(LOONGSON_EXT) Use Loongson EXTension (EXT) instructions. mloongson-ext2 -Target Report Mask(LOONGSON_EXT2) +Target Report Var(TARGET_LOONGSON_EXT2) Use Loongson EXTension R2 (EXT2) instructions. -- 1.8.3.1
Re: [PATCH] PR libstdc++/87431 re-adjust never-valueless optimizations
On 03/04/19 23:27 +0100, Jonathan Wakely wrote: On 03/04/19 23:32 +0300, Antony Polukhin wrote: Looks good. Covers most of the use cases. Please consider adding filesystem::path, pair, tuple, string_view?, string_view is trivially_copyable, and < 256 bytes, so works automatically. error_code, list, deque (myabe all the other containers), optional, variant itself (for cases when variant holds another variant). I hadn't thought about error_code and recursive variants, I'll check those can be made to work. error_code is trivially copyasble and small, so handled automatically.
Re: [PATCH][RFC] Fix PRs 88882, 89905, 89892 (and more?)
On Apr 3, 2019, Richard Biener wrote: > My solution is to instead of dropping the debug binds still > move them to the destination but then reset them. This solves > the wrong-debug issue. *nod*, yeah, that's probably the best we can do without major surgery. > But it of course possibly degrades debug information for the > other paths into the destination block. If we could find a condition that took control flow through the forwarding path vs other paths, we could try to preserve them with a ternary expression that uses an unbound debug temp as the reset value, the closest to a conditional debug bind we could have right now. But I guess it would make no difference once we attempted to resolve the expression and found a subexpression to be reset. > I'm less sure what would be the correct action for DEBUG_BEGIN > stmts (the patch contines to drop them on the floor, leaving > reset debug-binds possibly surrounded by wrong stmt context?). Dropping them is the best we can do now. > Not sure what else debug stmt kinds we have and what the proper > action for those would be. Inline entry point markers, also to be dropped for now. > Somewhere I've written that debug-stmts living on edges would > also solve the issue on the GIMPLE (and RTL) side, not sure > if we can make sense of those in the end though. Once you go down that path, I guess you'll soon find a need for control flow graphs within edges, at which point it gets really ugly. I'm slightly more hopeful about identifying guarding conditions to introduce conditional debug stmts. > Having stmts permanently on edges is also sth new on GIMPLE so I'm > staying away from that at this moment... *firm nod* :-) > I've noted that for the specific case where there is > (before the next DEBUG_BEGIN_STMT? before the next real stmt?) > (debug-) definitions of the debug vars we reset in the destination > block then dropping the debug stmt might be better and avoids > the debug-info quality degadation. If there's any instruction or view that would be reached by the earlier bind (the one that precedes the one we'd drop or reset), it would get wrong debug information if we were to drop the bind rather than reset it. If there isn't, whatever happens to the bind will have no effect. This suggests to me that resetting it is the right thing to do. Now, if we were to try to duplicate the logic that decides whether the bind might possibly be observable, in order to drop it on the floor instead of resetting it, we should look for another bind of the same variable before any real stmt or debug marker. If we find one, it would be ok to drop the bind instead of resetting it. I suppose we might even make this part of the debug bind resetting logic, or introduce separate but reusable functionality for this sort of cleanup. But didn't we have something to that effect already? I vaguely recall Jakub implemented something that would drop binds that could not be observed, and that it became a lot less effective after adjusting it to deal with view markers. > I guess that at least looking at all PHI nodes of the destination > might be a good idea because those defs happen before the "new" debug > reset. /me mumbles something about PHI debug binds ;-) > 2019-04-03 Richard Biener > PR debug/89892 > PR debug/89905 > * tree-cfgcleanup.c (remove_forwarder_block): Always move > debug bind stmts but reset them if they are not valid at the > destination. > * gcc.dg/guality/pr89892.c: New testcase. > * gcc.dg/guality/pr89905.c: Likewise. LGTM, thanks. Any reason to mention PR 2 in the Subject: but not in the ChangeLog? -- Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain EngineerFree Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
Make FMA code cope with redundant negates (PR89956)
This patch fixes a case in which, due to forced missed optimisations
in earlier passes, we have:
_1 = a * b
_2 = -_1
_3 = -_1
_4 = _2 + _3
and treated _4 as two FNMA candidates, once via _2 and once via _3.
Tested on aarch64-linux-gnu. OK to install?
Richard
2019-04-04 Richard Sandiford
gcc/
PR tree-optimization/89956
* tree-ssa-math-opts.c (convert_mult_to_fma): Protected against
multiple negates of the same value.
gcc/testsuite/
PR tree-optimization/89956
* gfortran.dg/pr89956.f90: New test.
Index: gcc/tree-ssa-math-opts.c
===
--- gcc/tree-ssa-math-opts.c2019-04-04 08:34:52.217937275 +0100
+++ gcc/tree-ssa-math-opts.c2019-04-04 09:11:12.733814338 +0100
@@ -3094,6 +3094,7 @@ convert_mult_to_fma (gimple *mul_stmt, t
&& (tree_to_shwi (TYPE_SIZE (type))
<= PARAM_VALUE (PARAM_AVOID_FMA_MAX_BITS)));
bool defer = check_defer;
+ bool seen_negate_p = false;
/* Make sure that the multiplication statement becomes dead after
the transformation, thus that all uses are transformed to FMAs.
This means we assume that an FMA operation has the same cost
@@ -3127,6 +3128,12 @@ convert_mult_to_fma (gimple *mul_stmt, t
ssa_op_iter iter;
use_operand_p usep;
+ /* If (due to earlier missed optimizations) we have two
+negates of the same value, treat them as equivalent
+to a single negate with multiple uses. */
+ if (seen_negate_p)
+ return false;
+
result = gimple_assign_lhs (use_stmt);
/* Make sure the negate statement becomes dead with this
@@ -3145,7 +3152,7 @@ convert_mult_to_fma (gimple *mul_stmt, t
if (gimple_bb (use_stmt) != gimple_bb (mul_stmt))
return false;
- negate_p = true;
+ negate_p = seen_negate_p = true;
}
tree cond, else_value, ops[3];
Index: gcc/testsuite/gfortran.dg/pr89956.f90
===
--- /dev/null 2019-03-08 11:40:14.606883727 +
+++ gcc/testsuite/gfortran.dg/pr89956.f90 2019-04-04 09:11:12.733814338
+0100
@@ -0,0 +1,16 @@
+! { dg-options "-O3 -fno-tree-forwprop -fno-tree-pre -fno-tree-dominator-opts
-fno-code-hoisting -ffast-math" }
+
+module de
+contains
+ function zu (az, xx) result (q3)
+real :: az, xx, q3
+
+q3 = 1.0 - lz (az, xx) - lz (xx, az)
+ end function zu
+
+ function lz (ho, gh) result (ye)
+real :: ho, gh, ye
+
+ye = sqrt (ho) - ho * gh
+ end function lz
+end module de
Re: PR 89864 - gcc fails to build/bootstrap with XCode 10.2
On Thu, Apr 4, 2019 at 3:11 AM Iain Sandoe wrote: > Hi Eric, > > Thanks for working on this! > > > On 4 Apr 2019, at 04:00, Erik Schnetter wrote: > > > > Fixinclude the header file that incorrectly uses the C-only > > _Atomic keyword when compiled as C++. Apply the same work-around for two > > GCC source files that transitively use this header file. > > 1/ > If the fix-include is doing the right thing, then there should be no need > for a > workaround in any GCC file including it. > > Perhaps the wrap needs to be around the (SDK) header(s) that is/are > including ? > > (so if is including , then the wrap needs to > be in that > file, not in the GCC files that use it). > Okay, that's good to know. I will update the patch accordingly. 2/ > I’m somewhat unhappy that this will hack around the problem and silently do > something that can’t work properly in any case that we started to use the > field/struct. What didn’t work about your proposed substitution of the > appropriate > c++ stuff? > What doesn't work was that this requires changing . If only can be changed, then it is not possible to replace _Atomic ulong by _Atomic(ulong) (adding parentheses). I'll try fixincluding both files. -erik -- Erik Schnetter http://www.perimeterinstitute.ca/personal/eschnetter/
Re: PR 89864 - gcc fails to build/bootstrap with XCode 10.2
Hi Erik, > On 4 Apr 2019, at 12:56, Erik Schnetter wrote: > > On Thu, Apr 4, 2019 at 3:11 AM Iain Sandoe wrote: > What doesn't work was that this requires changing . If only > can be changed, then it is not possible to replace _Atomic > ulong by _Atomic(ulong) (adding parentheses). I'll try fixincluding both > files. That might be what’s needed (although both fixes really need to be conditional on the same guard). Please note the additional comments in the PR (this is potentially an ABI breaking change, but then I suppose so is the SDK change). We should at minimum test that the size and alignment of the types are the same! It’s somewhat of an additional problem that we can’t easily check for the SDK version to base fixes on known broken SDKs - it’s not really an issue of the OS revision (we’re relying on the match case + the OS version being unique ‘enough’). again, thanks for working on this, Iain
Re: [PATCH][RFC] Fix PRs 88882, 89905, 89892 (and more?)
On Thu, 4 Apr 2019, Alexandre Oliva wrote:
> On Apr 3, 2019, Richard Biener wrote:
>
> > My solution is to instead of dropping the debug binds still
> > move them to the destination but then reset them. This solves
> > the wrong-debug issue.
>
> *nod*, yeah, that's probably the best we can do without major surgery.
>
> > But it of course possibly degrades debug information for the
> > other paths into the destination block.
>
> If we could find a condition that took control flow through the
> forwarding path vs other paths, we could try to preserve them with a
> ternary expression that uses an unbound debug temp as the reset value,
> the closest to a conditional debug bind we could have right now. But I
> guess it would make no difference once we attempted to resolve the
> expression and found a subexpression to be reset.
>
> > I'm less sure what would be the correct action for DEBUG_BEGIN
> > stmts (the patch contines to drop them on the floor, leaving
> > reset debug-binds possibly surrounded by wrong stmt context?).
>
> Dropping them is the best we can do now.
>
> > Not sure what else debug stmt kinds we have and what the proper
> > action for those would be.
>
> Inline entry point markers, also to be dropped for now.
>
> > Somewhere I've written that debug-stmts living on edges would
> > also solve the issue on the GIMPLE (and RTL) side, not sure
> > if we can make sense of those in the end though.
>
> Once you go down that path, I guess you'll soon find a need for control
> flow graphs within edges, at which point it gets really ugly.
>
> I'm slightly more hopeful about identifying guarding conditions to
> introduce conditional debug stmts.
>
> > Having stmts permanently on edges is also sth new on GIMPLE so I'm
> > staying away from that at this moment...
>
> *firm nod* :-)
>
>
> > I've noted that for the specific case where there is
> > (before the next DEBUG_BEGIN_STMT? before the next real stmt?)
> > (debug-) definitions of the debug vars we reset in the destination
> > block then dropping the debug stmt might be better and avoids
> > the debug-info quality degadation.
>
> If there's any instruction or view that would be reached by the earlier
> bind (the one that precedes the one we'd drop or reset), it would get
> wrong debug information if we were to drop the bind rather than reset
> it. If there isn't, whatever happens to the bind will have no effect.
> This suggests to me that resetting it is the right thing to do.
Hmm. I was thinking of
void foo(int n)
{
if (n == 0)
return;
int i = 0;
do
{
++i;
}
while (i < n);
}
where before CCP we have
:
# DEBUG BEGIN_STMT
if (n_2(D) == 0)
goto ; [INV]
else
goto ; [INV]
:
# DEBUG BEGIN_STMT
i_3 = 0;
# DEBUG i => i_3
:
# i_1 = PHI
# DEBUG i => i_1
# DEBUG BEGIN_STMT
# DEBUG BEGIN_STMT
i_4 = i_1 + 1;
# DEBUG i => i_4
if (i_4 < n_2(D))
goto ; [INV]
else
goto ; [INV]
:
# DEBUG BEGIN_STMT
// predicted unlikely by early return (on trees) predictor.
goto ; [INV]
:
return;
and then CCP propagates out i_3 = 0 resulting in
# DEBUG i => 0
and bb 4 degrading into a forwarder block CFG cleanup removes. Now
before the patch we'd drop the above but afterwards we end up with
:
# i_1 = PHI <0(2), i_4(4)>
# DEBUG i => NULL
# DEBUG i => i_1
# DEBUG BEGIN_STMT
# DEBUG BEGIN_STMT
i_4 = i_1 + 1;
# DEBUG i => i_4
if (i_4 < n_2(D))
goto ; [INV]
else
goto ; [INV]
that might be OK(?) and in gdb I can't find it doing any harm. What
I suggested was to, for example, notice that there's a PHI defining
'i' in the destination and thus it would be safe to drop the debug-bind
(it's now associated with a "wrong" stmt/view since we dropped the
DEBUG BEGIN_STMT as well?). Similarly we immediately arrive at
# DEBUG i => i_1 and thus dopping would be OK even if the actual
IV were replaced by another one?
Since I've now done the above experiment in gdb I feel safer with
the idea btw - at least this kind of simple cases do not regress
visibly ;)
> Now, if we were to try to duplicate the logic that decides whether the
> bind might possibly be observable, in order to drop it on the floor
> instead of resetting it, we should look for another bind of the same
> variable before any real stmt or debug marker. If we find one, it would
> be ok to drop the bind instead of resetting it. I suppose we might even
> make this part of the debug bind resetting logic, or introduce separate
> but reusable functionality for this sort of cleanup.
Hmm, but with the BEGIN_STMT markers still in place the views would
make the different values observable, no?
> But didn't we have
> something to that effect already? I vaguely recall Jakub implemented
> something that would drop binds that could not be observed, and that it
> became a lot less effective after adjusting it to deal with view
> markers.
>
> > I guess that at least looking at all PHI nodes o
Re: [PATCH] Fix a missed case in predicate analysis of the late uninit pass
Richard Biener writes:
> On Mon, Apr 1, 2019 at 5:36 PM Vladislav Ivanishin wrote:
>>
>> Hi!
>>
>> This is a fairly trivial change fixing a false negative in
>> -Wmaybe-uninitialized. I am pretty sure this is simply an overlooked
>> case (is_value_included_in() is not meant to deal with the case where
>> both compare codes are NE_EXPRs, neither does it need to, since their
>> handling is trivial).
>>
>> In a nutshell, what happens is values of v restricted by (v != 2) are
>> incorrectly considered a subset of values of v restricted by (v != 1).
>> As if "v != 2, therefore v != 1".
>>
>> This is by no means a gcc-9 regression; I'll ping the patch once stage4
>> is over, if needed.
>>
>> This came up when I was experimenting with moving the uninit passes
>> around. On mainline, the late uninit pass runs very late, so reliably
>> triggering the affected path is a bit tricky. So I created a GIMPLE
>> test (it reproduces the behavior precisely, but might be fragile
>> w.r.t. future versions of the textual representation) and then with a
>> hint from Alexander managed to produce a simple C test. [By the way,
>> the first take was to insert an asm with a lot of newlines to prevent
>> the dom pass from rewriting the CFG due to high cost of duplicating
>> instructions. This didn't work out; I think the dom pass does not
>> respect sizes of inline asms. I plan to create a testcase and file a
>> bug later.]
>>
>> I ran regression testing on x86_64-pc-linux-gnu and saw no new
>> regressions modulo a handful of flaky UNRESOLVED tests under
>> gcc.dg/tree-prof. `BOOT_CFLAGS="-O -Wno-error=maybe-uninitialized
>> -Wmaybe-uninitialized" bootstrap` also succeeded producing no new
>> warnings.
>>
>> OK for stage1?
>
> Hum. While definitely two NE_EXPR do not work correctly I'd like
> to see a positive test since LT_EXPR doesn't work either?
Right, thanks. The case that was not covered well is actually when
cond2 == NE_EXPR (arbitrary cond1). I created 2 new tests: uninit-26.c
demonstrates a false negative, and uninit-27-gimple.c a false positive
with trunk.
> Specifically the code falls through to test is_value_included_in which
> seems to assume code1 == code2.
The function is_value_included_in itself only cares about one condition
code (I'll expound on this below). I agree though that the second part
of the comment seems to assume that.
Please take a look at the updated patch. The rationale is as follows.
When we have 2 potentially comparable predicates in
is_pred_expr_subset_of, there are basically two things we want to check.
1) Whether two ranges with identical condition codes are nested. This
is done straightforwardly with is_value_included_in.
2) Whether X CMPC VAL1 is nested in X != VAL2. Which is easy: this
happens iff the set (a.k.a "range") {X: X CMPC VAL1 is true} doesn't
contain ("cover") VAL2, in other words iff VAL2 CMPC VAL1 is false.
Only, the logic of 2) is faulty when X CMPC VAL1 is not a range bounded
on one end (this happens when, and only when CMPC is NE_EXPR; the range
is then unbounded on both ends and can only be nested in X != VAL2, if
VAL1 == VAL2).
3) Whether X != VAL1 is nested in X CMPC VAL2, but that is always false
unless CMPC is NE_EXPR, which is already considered.
> To me it looks like is_value_includeds comment should be clarified to
> say
>
> /* Returns true if all values X satisfying X CMPC VAL satisfy
>X CMPC BOUNDARY. */
This is indeed more clear than the current comment, and the meaning is
the same. I suggest however to not discuss nestedness of ranges in this
comment at all.
> That is, "all values in the range" in the current comment is
> under-specified since VAL is just a single value.
The range is implied, since only one CMPC is passed. (If CMPC is
NE_EXPR, however, then I guess the range would only be known in the
caller, but the function does not work correctly for this purpose --
hence the patch to not call it then.) But is_value_included_in actually
only cares about one value and one set anyway, as the name suggests.
So I think the second part of the comment is supposed to help to think
about applying this function for its main use-case (this function is
used twice, actually: in the case we are discussing there is a range
whose nestedness the calling code is testing, in the other case there is
just a constant), whereas the first part simply states what the function
does. I'd say, the second part of the comment should be rewritten or
discarded, while the first should be kept. OTOH, it might have been
helpful to the person who wrote this code.
> So I wonder what testcases regress if we remove the && code2 != NE_EXPR
> case? That way we see what the intention was. A patch should then
> change that to
>
> if ((code1 != code2)
> || !( && code2 == NE_EXPR))
>return false;
>
> to explicitely spell out what case was meant here.
make check-gcc RUNTESTFLAGS='dg.exp=uninit*' gives one regression:
gcc.dg/uninit-pred-9_b.c bogus war
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly). The solution makes
sure equivalent forms of array initializers such as for char[5]:
"\0\1\2"
"\0\1\2\0"
{ 0, 1, 2 }
{ 0, 1, 2, 0 }
{ 0, 1, 2, 0, 0 }
are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:
""
"\0"
"\0\0\0\0"
{ }
{ 0 }
{ 0, 0, 0, 0, 0 }
and for these of struct { char a[5], b[5], c[5]; }:
{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
{ { 0 }, { } }
{ "" }
Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.
I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct. (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).
You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
triviality of other operations.
Done (I think).
I wouldn't worry about trying to omit user-defined constexpr ctors.
The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.
The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):
g++.dg/spellcheck-macro-ordering-2.C
g++.dg/cpp0x/auto52.C
g++.dg/cpp1y/auto-neg1.C
g++.dg/other/crash-9.C
You probably need to check zero_init_p to properly handle pointers to
data members, where a null value is integer -1; given
struct A { int i; };
constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };
we don't want to discard the initializers because they look like
zeros, as then digest_init will add back -1s.
I added it but it wasn't doing the right thing. It mangled { } and
{ 0 } differently:
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
because the zeros didn't get removed. They need to be mangled the same,
don't they?
I've added three tests to exercise this (array51.C, nontype-class16.C,
and mangle72.C). They pass without the zero_init_p() calls but some
fail with it (depending on where it's added). Please check to see
that the tests really do exercise what they should.
If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).
Hmm, let me poke at this a bit.
+ unsigned n = TREE_STRING_LENGTH (value);
+ const char *str = TREE_STRING_POINTER (value);
+ /* Count the number of trailing nuls and subtract them from
+ STRSIZE because they don't need to be mangled. */
+ tree strsizenode = TYPE_SIZE_UNIT (TREE_TYPE (value));
+ unsigned strsize = tree_to_uhwi (strsizenode);
+ if (strsize > n)
+ strsize = n;
Why not just use TREE_STRING_LENGTH?
The length reflects the size of the string literal, including
any trailing nuls (like in "x\0y\0\0"). We only want to mangle
the leading part up to (but not including) the first trailing
nul.
Yes, I meant why look at TYPE_SIZE_UNIT at all? TREE_STRING_LENGTH
seems like the right starting value of strsize.
Jason
Re: [PATCH, GCC, AARCH64] Add GNU note section with BTI and PAC.
Hi Richard
On 03/04/2019 11:28, Richard Henderson wrote:
> On 4/3/19 5:19 PM, Sudakshina Das wrote:
>> + /* PT_NOTE header: namesz, descsz, type.
>> + namesz = 4 ("GNU\0")
>> + descsz = 16 (Size of the program property array)
>> + type = 5 (NT_GNU_PROPERTY_TYPE_0). */
>> + assemble_align (POINTER_SIZE);
>> + assemble_integer (GEN_INT (4), 4, 32, 1);
>> + assemble_integer (GEN_INT (16), 4, 32, 1);
>
> So, it's 16 only if POINTER_SIZE == 64.
>
> I think ROUND_UP (12, POINTER_BYTES) is what you want here.
>
Ah yes. I have made that change now.
Thanks
Sudi
>
> r~
>
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 9d0292d64f20939ccedd7ab56027aa1282826b23..5e8b34ded03c78493f868e38647bf57c2da5187c 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -83,7 +83,7 @@
#define GNU_USER_TARGET_D_CRITSEC_SIZE 48
-#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+#define TARGET_ASM_FILE_END aarch64_file_end_indicate_exec_stack
/* Uninitialized common symbols in non-PIE executables, even with
strong definitions in dependent shared libraries, will resolve
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b38505b0872688634b2d3f625ab8d313e89cfca0..83b8ef84808c19fa1214fa06c32957936f5eb520 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18744,6 +18744,57 @@ aarch64_stack_protect_guard (void)
return NULL_TREE;
}
+/* Implement TARGET_ASM_FILE_END for AArch64. This adds the AArch64 GNU NOTE
+ section at the end if needed. */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1U << 1)
+void
+aarch64_file_end_indicate_exec_stack ()
+{
+ file_end_indicate_exec_stack ();
+
+ unsigned feature_1_and = 0;
+ if (aarch64_bti_enabled ())
+feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+
+ if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
+feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+
+ if (feature_1_and)
+{
+ /* Generate .note.gnu.property section. */
+ switch_to_section (get_section (".note.gnu.property",
+ SECTION_NOTYPE, NULL));
+
+ /* PT_NOTE header: namesz, descsz, type.
+ namesz = 4 ("GNU\0")
+ descsz = 16 (Size of the program property array)
+ [(12 + padding) * Number of array elements]
+ type = 5 (NT_GNU_PROPERTY_TYPE_0). */
+ assemble_align (POINTER_SIZE);
+ assemble_integer (GEN_INT (4), 4, 32, 1);
+ assemble_integer (GEN_INT (ROUND_UP (12, POINTER_BYTES)), 4, 32, 1);
+ assemble_integer (GEN_INT (5), 4, 32, 1);
+
+ /* PT_NOTE name. */
+ assemble_string ("GNU", 4);
+
+ /* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
+ type = GNU_PROPERTY_AARCH64_FEATURE_1_AND
+ datasz = 4
+ data = feature_1_and. */
+ assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 1);
+ assemble_integer (GEN_INT (4), 4, 32, 1);
+ assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+
+ /* Pad the size of the note to the required alignment. */
+ assemble_align (POINTER_SIZE);
+}
+}
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_PAC
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_BTI
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_AND
/* Target-specific selftests. */
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index a8c60412e310a4f322372f334ae5314f426d310e..5a556b08ed15679b25676a11fe9c7a64641ee671 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -61,3 +61,4 @@ lab2:
}
/* { dg-final { scan-assembler-times "hint\t34" 1 } } */
/* { dg-final { scan-assembler-times "hint\t36" 12 } } */
+/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
index e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77ea91537030640 100644
--- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
@@ -4,7 +4,9 @@
int
f (int a, ...)
{
- /* { dg-final { scan-assembler-not "str" } } */
+ /* Fails on aarch64*-*-linux* if configured with
+--enable-standard-branch-protection because of the GNU NOTE section. */
+ /* { dg-final { scan-assembler-not "str" { target { ! aarch64*-*-linux* } || { ! default_branch_protection } } } } */
return a;
}
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On 4/4/19 8:57 AM, Jason Merrill wrote:
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly). The solution makes
sure equivalent forms of array initializers such as for char[5]:
"\0\1\2"
"\0\1\2\0"
{ 0, 1, 2 }
{ 0, 1, 2, 0 }
{ 0, 1, 2, 0, 0 }
are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:
""
"\0"
"\0\0\0\0"
{ }
{ 0 }
{ 0, 0, 0, 0, 0 }
and for these of struct { char a[5], b[5], c[5]; }:
{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
{ { 0 }, { } }
{ "" }
Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.
I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct. (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).
You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
triviality of other operations.
Done (I think).
I wouldn't worry about trying to omit user-defined constexpr ctors.
The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.
The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):
g++.dg/spellcheck-macro-ordering-2.C
g++.dg/cpp0x/auto52.C
g++.dg/cpp1y/auto-neg1.C
g++.dg/other/crash-9.C
You probably need to check zero_init_p to properly handle pointers to
data members, where a null value is integer -1; given
struct A { int i; };
constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };
we don't want to discard the initializers because they look like
zeros, as then digest_init will add back -1s.
I added it but it wasn't doing the right thing. It mangled { } and
{ 0 } differently:
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
because the zeros didn't get removed. They need to be mangled the same,
don't they?
I've added three tests to exercise this (array51.C, nontype-class16.C,
and mangle72.C). They pass without the zero_init_p() calls but some
fail with it (depending on where it's added). Please check to see
that the tests really do exercise what they should.
If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).
Hmm, let me poke at this a bit.
Here's a test case showing that mangling null member pointers
as zero leads to conflicts:
struct A { int i; };
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
// both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0E
void f (Y) { }
void g (Y) { }
This happens both with trunk and with my patch. They probably
need to mangle as negative one, don't you think? (There's
a comment saying mangling them as zeros is intentional but not
why.)
+ unsigned n = TREE_STRING_LENGTH (value);
+ const char *str = TREE_STRING_POINTER (value);
+ /* Count the number of trailing nuls and subtract them from
+ STRSIZE because they don't need to be mangled. */
+ tree strsizenode = TYPE_SIZE_UNIT (TREE_TYPE (value));
+ unsigned strsize = tree_to_uhwi (strsizenode);
+ if (strsize > n)
+ strsize = n;
Why not just use TREE_STRING_LENGTH?
The length reflects the size of the string literal, including
any trailing nuls (like in "x\0y\0\0"). We only want to mangle
the leading part up to (but not including) the first trailing
nul.
Yes, I meant why look at TYPE_SIZE_UNIT at all? TREE_STRING_LENGTH
seems like the right starting value of strsize.
Ah. Let me see about changing that.
Martin
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On 4/4/19 12:29 PM, Martin Sebor wrote:
On 4/4/19 8:57 AM, Jason Merrill wrote:
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly). The solution makes
sure equivalent forms of array initializers such as for char[5]:
"\0\1\2"
"\0\1\2\0"
{ 0, 1, 2 }
{ 0, 1, 2, 0 }
{ 0, 1, 2, 0, 0 }
are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:
""
"\0"
"\0\0\0\0"
{ }
{ 0 }
{ 0, 0, 0, 0, 0 }
and for these of struct { char a[5], b[5], c[5]; }:
{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
{ { 0 }, { } }
{ "" }
Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.
I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct. (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).
You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
triviality of other operations.
Done (I think).
I wouldn't worry about trying to omit user-defined constexpr ctors.
The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.
The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):
g++.dg/spellcheck-macro-ordering-2.C
g++.dg/cpp0x/auto52.C
g++.dg/cpp1y/auto-neg1.C
g++.dg/other/crash-9.C
You probably need to check zero_init_p to properly handle pointers
to data members, where a null value is integer -1; given
struct A { int i; };
constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };
we don't want to discard the initializers because they look like
zeros, as then digest_init will add back -1s.
I added it but it wasn't doing the right thing. It mangled { } and
{ 0 } differently:
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
because the zeros didn't get removed. They need to be mangled the same,
don't they?
I've added three tests to exercise this (array51.C,
nontype-class16.C, and mangle72.C). They pass without the
zero_init_p() calls but some
fail with it (depending on where it's added). Please check to see
that the tests really do exercise what they should.
If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).
Hmm, let me poke at this a bit.
Here's a test case showing that mangling null member pointers
as zero leads to conflicts:
struct A { int i; };
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
// both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0E
void f (Y) { }
void g (Y) { }
This happens both with trunk and with my patch. They probably
need to mangle as negative one, don't you think? (There's
a comment saying mangling them as zeros is intentional but not
why.)
Hmm. If we leave out f, then g mangles as 0 and &A::i, which is fine.
Need to figure out why creating a B before the declaration of g breaks that.
The rationale for mangling as 0 would have been to reflect what you
would write in the source: (int A::*)0 does give a null member pointer.
But then we really can't use that representation 0 for anything else, we
have to use a symbolic representation. This patch doesn't need to fix
that.
Jason
Re: [PATCH] Fix PR83033
On 29/03/2019 11:01, Andrea Corallo wrote: > Hi all, > simple patch addressing minor style issue into > gcc/config/aarch64/cortex-a57-fma-steering.c. > > make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap > > Okay for trunk? > > Bests > Andrea > > > 2019-03-29 Andrea Corallo > > PR target/83033 > * config/aarch64/cortex-a57-fma-steering.c > (fma_forest): Fix missing copy constructor. > (fma_root_node): Likewise. > (func_fma_steering): Likewise. > These should be commented, even if it's as simple as "Prohibit copy construction." R. > > 83033.patch > > diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c > b/gcc/config/aarch64/cortex-a57-fma-steering.c > index f2da03a..a390a62 100644 > --- a/gcc/config/aarch64/cortex-a57-fma-steering.c > +++ b/gcc/config/aarch64/cortex-a57-fma-steering.c > @@ -114,6 +114,8 @@ public: >void dispatch (); > > private: > + fma_forest (const fma_forest &); > + >/* The list of roots that form this forest. */ >std::list *m_roots; > > @@ -180,6 +182,8 @@ public: >void dump_info (fma_forest *); > > private: > + fma_root_node (const fma_root_node &); > + >/* The forest this node belonged to when it was created. */ >fma_forest *m_forest; > }; > @@ -203,6 +207,7 @@ public: >void execute_fma_steering (); > > private: > + func_fma_steering (const func_fma_steering &); >void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node > *), > void (*) (fma_forest *, fma_node *), bool); >void analyze (); >
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On 4/4/19 10:50 AM, Jason Merrill wrote:
On 4/4/19 12:29 PM, Martin Sebor wrote:
On 4/4/19 8:57 AM, Jason Merrill wrote:
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly). The solution makes
sure equivalent forms of array initializers such as for char[5]:
"\0\1\2"
"\0\1\2\0"
{ 0, 1, 2 }
{ 0, 1, 2, 0 }
{ 0, 1, 2, 0, 0 }
are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:
""
"\0"
"\0\0\0\0"
{ }
{ 0 }
{ 0, 0, 0, 0, 0 }
and for these of struct { char a[5], b[5], c[5]; }:
{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
{ { 0 }, { } }
{ "" }
Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.
I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct. (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).
You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
triviality of other operations.
Done (I think).
I wouldn't worry about trying to omit user-defined constexpr ctors.
The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.
The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):
g++.dg/spellcheck-macro-ordering-2.C
g++.dg/cpp0x/auto52.C
g++.dg/cpp1y/auto-neg1.C
g++.dg/other/crash-9.C
You probably need to check zero_init_p to properly handle pointers
to data members, where a null value is integer -1; given
struct A { int i; };
constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };
we don't want to discard the initializers because they look like
zeros, as then digest_init will add back -1s.
I added it but it wasn't doing the right thing. It mangled { } and
{ 0 } differently:
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
because the zeros didn't get removed. They need to be mangled the
same,
don't they?
I've added three tests to exercise this (array51.C,
nontype-class16.C, and mangle72.C). They pass without the
zero_init_p() calls but some
fail with it (depending on where it's added). Please check to see
that the tests really do exercise what they should.
If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).
Hmm, let me poke at this a bit.
Here's a test case showing that mangling null member pointers
as zero leads to conflicts:
struct A { int i; };
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
// both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0E
void f (Y) { }
void g (Y) { }
This happens both with trunk and with my patch. They probably
need to mangle as negative one, don't you think? (There's
a comment saying mangling them as zeros is intentional but not
why.)
Hmm. If we leave out f, then g mangles as 0 and &A::i, which is fine.
Need to figure out why creating a B before the declaration of g breaks
that.
Let me know if you want me to start looking into this or if you
are on it yourself.
The rationale for mangling as 0 would have been to reflect what you
would write in the source: (int A::*)0 does give a null member pointer.
But then we really can't use that representation 0 for anything else, we
have to use a symbolic representation. This patch doesn't need to fix
that.
Okay. It doesn't look like it would be hard to change but I'm
happy to leave it for later.
Attached is a patch that just simplifies the STRING_CST traversal.
I suspect I got confused into thinking that TREE_STRING_LENGTH is
the length of the string literal as opposed to its size, even
after all this time working with GCC strings. I wonder if
the macro could be renamed to something like TREE_STRING_SIZE.
I also added more tests for member pointers, including those to
member functions and managed to trigger another ICE in the process:
struct A { void (A::*p)(); };
template struct X { };
X x; // ICE in find_substitution during mangling
I opened bug 89974 for it even though the patch happens to avoid
it because it treats a null member function pointer as an empty
initializer list (i.e., { }). The added test exposes the mangling
problems with pointers to data members. I xfailed them.
Martin
P
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On 4/4/19 2:30 PM, Martin Sebor wrote:
On 4/4/19 10:50 AM, Jason Merrill wrote:
On 4/4/19 12:29 PM, Martin Sebor wrote:
On 4/4/19 8:57 AM, Jason Merrill wrote:
On 4/3/19 10:34 PM, Martin Sebor wrote:
On 4/1/19 11:27 AM, Jason Merrill wrote:
On 3/31/19 10:17 PM, Martin Sebor wrote:
To fix PR 89833, a P1 regression, the attached patch tries to
handle string literals as C++ 2a non-type template arguments
by treating them the same as brace enclosed initializer lists
(where the latter are handled correctly). The solution makes
sure equivalent forms of array initializers such as for char[5]:
"\0\1\2"
"\0\1\2\0"
{ 0, 1, 2 }
{ 0, 1, 2, 0 }
{ 0, 1, 2, 0, 0 }
are treated as the same, both for overloading and mangling.
Ditto for the following equivalent forms:
""
"\0"
"\0\0\0\0"
{ }
{ 0 }
{ 0, 0, 0, 0, 0 }
and for these of struct { char a[5], b[5], c[5]; }:
{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
{ { 0 }, { } }
{ "" }
Since this is not handled correctly by the current code (see PR
89876 for a test case) the patch also fixes that.
I'm not at all confident the handling of classes with user-defined
constexpr ctors is 100% correct. (I use triviality to constrain
the solution for strings but that was more of an off-the-cuff guess
than a carefully considered decision).
You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
triviality of other operations.
Done (I think).
I wouldn't worry about trying to omit user-defined constexpr ctors.
The g++.dg/abi/mangle71.C
test is all I've got in terms of verifying it works correctly.
I'm quite sure the C++ 2a testing could stand to be beefed up.
The patch passes x86_64-linux bootstrap and regression tests.
There are a few failures in check-c++-all tests that don't look
related to the changes (I see them with an unpatched GCC as well):
g++.dg/spellcheck-macro-ordering-2.C
g++.dg/cpp0x/auto52.C
g++.dg/cpp1y/auto-neg1.C
g++.dg/other/crash-9.C
You probably need to check zero_init_p to properly handle pointers
to data members, where a null value is integer -1; given
struct A { int i; };
constexpr A::* pm = &A::i;
int A::* pma[] = { pm, pm };
we don't want to discard the initializers because they look like
zeros, as then digest_init will add back -1s.
I added it but it wasn't doing the right thing. It mangled { } and
{ 0 } differently:
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
because the zeros didn't get removed. They need to be mangled the
same,
don't they?
I've added three tests to exercise this (array51.C,
nontype-class16.C, and mangle72.C). They pass without the
zero_init_p() calls but some
fail with it (depending on where it's added). Please check to see
that the tests really do exercise what they should.
If you think a zero_init_p() check really is needed I will need some
guidance where (a test case would be great).
Hmm, let me poke at this a bit.
Here's a test case showing that mangling null member pointers
as zero leads to conflicts:
struct A { int i; };
typedef int A::*pam_t;
struct B { pam_t a[2]; };
template struct Y { };
// both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0E
void f (Y) { }
void g (Y) { }
This happens both with trunk and with my patch. They probably
need to mangle as negative one, don't you think? (There's
a comment saying mangling them as zeros is intentional but not
why.)
Hmm. If we leave out f, then g mangles as 0 and &A::i, which is fine.
Need to figure out why creating a B before the declaration of g breaks
that.
Let me know if you want me to start looking into this or if you
are on it yourself.
Sure, go ahead.
The rationale for mangling as 0 would have been to reflect what you
would write in the source: (int A::*)0 does give a null member
pointer. But then we really can't use that representation 0 for
anything else, we have to use a symbolic representation. This patch
doesn't need to fix that.
Okay. It doesn't look like it would be hard to change but I'm
happy to leave it for later.
Feel free to fix it in a followup patch, along with investigating the above.
Attached is a patch that just simplifies the STRING_CST traversal.
OK.
Jason
Re: [Patch, fortran] PR89904 - ICE in gfortran starting with r270045
Hi Harald, OK for trunk (and affected backports)? Yes, OK. Thanks! (For cases like this, it often makes sense to wait a week or so before backporting something, to see if anything comes up). Regards Thomas
[committed][PR rtl-optimization/89399] Safely access the source/destination of sets
As noted in the BZ ree was incorrectly looking at SET_SRC/SET_DEST of
PATTERN of various insns.
In the case where we're looking at something in the candidate list, we
know that all the insns passed the single_set test. So when digging
into something from the candidate list, we should call single_set to
extract the set, then use SET_SRC/SET_DEST to look at the appropriate
parts of the set. That's precisely what this patch does.
Other places within ree.c use get_sub_rtx, but that seems to be for the
insn we want to change rather than items on the candidate list.
get_sub_rtx uses PATTERN (insn) internally, but does so in a reasonably
safe manner.
transform_ifelse also uses PATTERN (insn), but AFAICT only does so for
conditional moves which passed the is_cond_copy_insn test which includes
a single_set test. So those are safe as well.
Bootstrapped and regression tested on a variety of platforms.
Installing on the trunk.
Jeff
commit 289d708e113e0d579eb78f9b5e63fb6146288cf5
Author: Jeff Law
Date: Thu Apr 4 14:49:53 2019 -0600
PR rtl-optimization/89399
* ree.c (combine_set_extension): Use single_set rather than
digging into PATTERN for items on the candidate list.
(combine_reaching_defs): Likewise.
PR rtl-optimization/89399
* gcc.c-torture/compile/pr89399.c: New test.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 47eeed65ab7..f57c2f9c193 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2019-04-04 Jeff Law
+
+ PR rtl-optimization/89399
+ * ree.c (combine_set_extension): Use single_set rather than
+ digging into PATTERN for items on the candidate list.
+ (combine_reaching_defs): Likewise.
+
2019-04-04 Richard Sandiford
PR rtl-optimization/46590
diff --git a/gcc/ree.c b/gcc/ree.c
index e23e607a5e1..104f8dbf435 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -320,7 +320,7 @@ combine_set_extension (ext_cand *cand, rtx_insn *curr_insn,
rtx *orig_set)
rtx orig_src = SET_SRC (*orig_set);
machine_mode orig_mode = GET_MODE (SET_DEST (*orig_set));
rtx new_set;
- rtx cand_pat = PATTERN (cand->insn);
+ rtx cand_pat = single_set (cand->insn);
/* If the extension's source/destination registers are not the same
then we need to change the original load to reference the destination
@@ -778,10 +778,14 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat,
ext_state *state)
/* If the destination operand of the extension is a different
register than the source operand, then additional restrictions
are needed. Note we have to handle cases where we have nested
- extensions in the source operand. */
+ extensions in the source operand.
+
+ Candidate insns are known to be single_sets, via the test in
+ find_removable_extensions. So we continue to use single_set here
+ rather than get_sub_rtx. */
+ rtx set = single_set (cand->insn);
bool copy_needed
-= (REGNO (SET_DEST (PATTERN (cand->insn)))
- != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn);
+= (REGNO (SET_DEST (set)) != REGNO (get_extended_src_reg (SET_SRC (set;
if (copy_needed)
{
/* Considering transformation of
@@ -816,8 +820,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat,
ext_state *state)
if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
return false;
- machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn)));
- rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)));
+ machine_mode dst_mode = GET_MODE (SET_DEST (set));
+ rtx src_reg = get_extended_src_reg (SET_SRC (set));
/* Ensure we can use the src_reg in dst_mode (needed for
the (set (reg1) (reg2)) insn mentioned above). */
@@ -852,9 +856,9 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat,
ext_state *state)
|| !REG_P (SET_DEST (*dest_sub_rtx)))
return false;
- rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (PATTERN (cand->insn))),
+ rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (set)),
REGNO (SET_DEST (*dest_sub_rtx)));
- if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn
+ if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (set)))
return false;
/* On RISC machines we must make sure that changing the mode of SRC_REG
@@ -877,10 +881,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat,
ext_state *state)
/* The destination register of the extension insn must not be
used or set between the def_insn and cand->insn exclusive. */
- if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn, cand->insn)
- || reg_set_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn, cand->insn))
+ if (reg_used_between_p (SET_DEST (set), def_insn,
Re: [PATCH] avoid an other ICE due to invalid built-in call (PR 89934)
On 4/3/19 1:27 PM, Martin Sebor wrote: > PR 89934 reports yet another ICE due to a call to a library > built-in function with invalid arguments. The attached patch > does the bare minimum to avoid it. I will commit it to trunk > and to GCC 8 later this week if there are no objections. > > Martin > > PS There have been many of these reports (at least three in > the wrestrict pass alone, and quite a few elsewhere). Simply > avoiding the ICEs as this patch does and as all the others > before it have done as well does nothing to help solve > the underlying problem in user code: the invalid calls will > end up with undefined behavior if they are ever made at runtime. > For example, the one in this bug report will most likely cause > data corruption or buffer overflow without ever being detected > by _FORTIFY_SOURCE. That seems far worse than failing to compile > the code (either with an ICE or with some > other hard error). > > For GCC 10 I think a more robust solution should be put in place > not just to prevent these invalid built-in calls from tripping > up the middle-end, but also to avoid the undefined behavior at > runtime. The calls could either be rejected with an error as > Clang does, or they could be replaced them with __builtin_trap > or perhaps __builtin_unreachable, depending on some command > line option. > > > gcc-89934.diff > > PR middle-end/89934 - ICE on a call with fewer arguments to strncpy declared > without prototype > > gcc/ChangeLog: > > PR middle-end/89934 > * gimple-ssa-warn-restrict.c (builtin_access::builtin_access): Bail > out if the number of arguments is less than expected. > > gcc/testsuite/ChangeLog: > > PR middle-end/89934 > * gcc.dg/Wrestrict-19.c: New test. > * gcc.dg/Wrestrict-5.c: Add comment. Remove unused code. OK. WRT the larger issue. Assume we have the infrastructure bits to transform the bogus calls into a __builtin_trap or __builtin_unreachable (which I think we can easily lift from elsewhere) and a flag to allow us to determine which style we want. (we can build and prototype that and use the result in the erroneous-path-isolation pass). The question is when do we want to detect the bogus calls. There are some that we'd likely want to catch early, such as the wrong number of arguments like we see in this case. That feels like a pass over the IL right after gimplification. Other cases are more interesting. Do we try to detect when a propagation makes the call invalid as soon as it happens, or do we defer to the erroneous path isolation pass?Or do we end up doing both as there may be cases where analysis is nontrivial. Anyway, I think we can start working down this path for gcc-10. jeff
Re: [PATCH] Remove usage of apostrophes in error and warning messages (PR translation/89935).
On 4/3/19 4:17 AM, Martin Liška wrote: > Hi. > > The patch addresses wrong usage of apostrophes in error and warning > messages. It's follow up of what I did couple of weeks ago. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > contrib/ChangeLog: > > 2019-04-03 Martin Liska > > PR translation/89935 > * check-internal-format-escaping.py: Properly detect wrong > apostrophes. > > gcc/ChangeLog: > > 2019-04-03 Martin Liska > > PR translation/89935 > * collect-utils.c (collect_execute): Use %< and %>, or %qs in > order to wrap keywords or arguments. > * collect2.c (main): Likewise. > (scan_prog_file): Likewise. > (scan_libraries): Likewise. > * common/config/riscv/riscv-common.c > (riscv_subset_list::parsing_subset_version): Likewise. > (riscv_subset_list::parse_std_ext): Likewise. > * config/aarch64/aarch64.c (aarch64_override_options_internal): > Likewise. > * config/arm/arm.c (arm_option_override): Likewise. > * config/cris/cris.c (cris_print_operand): Likewise. > * config/darwin-c.c (darwin_pragma_options): Likewise. > (darwin_pragma_unused): Likewise. > (darwin_pragma_ms_struct): Likewise. > * config/ft32/ft32.c (ft32_print_operand): Likewise. > * config/i386/i386.c (print_reg): Likewise. > (ix86_print_operand): Likewise. > * config/i386/xm-djgpp.h: Likewise. > * config/iq2000/iq2000.c (iq2000_print_operand): Likewise. > * config/m32c/m32c.c (m32c_option_override): Likewise. > * config/msp430/msp430.c (msp430_option_override): Likewise. > * config/nds32/nds32.c (nds32_option_override): Likewise. > * config/nvptx/mkoffload.c (main): Likewise. > * config/rx/rx.c (rx_print_operand): Likewise. > (valid_psw_flag): Likewise. > * config/vms/vms-c.c (vms_pragma_member_alignment): Likewise. > (vms_pragma_nomember_alignment): Likewise. > (vms_pragma_extern_model): Likewise. > * lto-wrapper.c (compile_offload_image): Likewise. > * omp-offload.c (oacc_parse_default_dims): Likewise. > * symtab.c (symtab_node::verify_base): Likewise. > * tlink.c (recompile_files): Likewise. > (start_tweaking): Likewise. > * tree-profile.c (parse_profile_filter): Likewise. > > gcc/objc/ChangeLog: > > 2019-04-03 Martin Liska > > * objc-act.c (objc_add_property_declaration): Use %< and %>, or %qs in > order to wrap keywords or arguments. > (objc_add_synthesize_declaration_for_property): Likewise. > --- > contrib/check-internal-format-escaping.py | 2 +- > gcc/collect-utils.c | 2 +- > gcc/collect2.c| 10 +- > gcc/common/config/riscv/riscv-common.c| 12 ++-- > gcc/config/aarch64/aarch64.c | 4 ++-- > gcc/config/arm/arm.c | 2 +- > gcc/config/cris/cris.c| 2 +- > gcc/config/darwin-c.c | 24 +++ > gcc/config/ft32/ft32.c| 2 +- > gcc/config/i386/i386.c| 4 ++-- > gcc/config/i386/xm-djgpp.h| 4 ++-- > gcc/config/iq2000/iq2000.c| 2 +- > gcc/config/m32c/m32c.c| 2 +- > gcc/config/msp430/msp430.c| 18 - > gcc/config/nds32/nds32.c | 6 +++--- > gcc/config/nvptx/mkoffload.c | 2 +- > gcc/config/rx/rx.c| 7 --- > gcc/config/vms/vms-c.c| 15 +++--- > gcc/lto-wrapper.c | 4 ++-- > gcc/objc/objc-act.c | 18 +++-- > gcc/omp-offload.c | 2 +- > gcc/symtab.c | 2 +- > gcc/tlink.c | 4 ++-- > gcc/tree-profile.c| 2 +- > 24 files changed, 80 insertions(+), 72 deletions(-) > > OK jeff
Re: C++ PATCH for c++/87145 - bogus error converting class type in template argument list
On Thu, Mar 28, 2019 at 03:54:30PM -0400, Jason Merrill wrote:
> On 3/20/19 4:12 PM, Marek Polacek wrote:
> > The fix for 77656 caused us to call convert_nontype_argument even for
> > value-dependent arguments, to perform the conversion in order to avoid
> > a bogus warning.
> >
> > In this case, the argument is Pod{N}. The call to
> > build_converted_constant_expr
> > in convert_nontype_argument produces Pod::operator Enum(&{N}). It doesn't
> > crash
> > because we're in a template and build_address no longer crashes on
> > CONSTRUCTORs
> > in a template.
>
> Yeah, we shouldn't be preserving lower level codes like this ADDR_EXPR; we
> should probably return an IMPLICIT_CONV_EXPR rather than make the call
> explicit.
I'm having trouble with this. Do we want build_converted_constant_expr_internal
to build an IMPLICIT_CONV_EXPR in a template, much like
perform_implicit_conversion? That seems to break a lot of stuff, and I'm
nervous about that at this stage :/ We could probably handle this specially
in convert_nontype_argument. Maybe build an IMPLICIT_CONV_EXPR for
value-dependent CONSTRUCTORs?
Marek
Re: Make FMA code cope with redundant negates (PR89956)
On 4/4/19 4:43 AM, Richard Sandiford wrote: > This patch fixes a case in which, due to forced missed optimisations > in earlier passes, we have: > > _1 = a * b > _2 = -_1 > _3 = -_1 > _4 = _2 + _3 > > and treated _4 as two FNMA candidates, once via _2 and once via _3. > > Tested on aarch64-linux-gnu. OK to install? > > Richard > > > 2019-04-04 Richard Sandiford > > gcc/ > PR tree-optimization/89956 > * tree-ssa-math-opts.c (convert_mult_to_fma): Protected against > multiple negates of the same value. > > gcc/testsuite/ > PR tree-optimization/89956 > * gfortran.dg/pr89956.f90: New test. OK jeff
[PATCH] avoid more ICE due to bad built-in calls declared without a prototype (PR 89911, 89957)
Attached is yet another patch to avoid ICE due to middle-end
assumptions about the sanity of calls to built-ins, this time
for strnlen. It fixes two unsafe assumptions:
1) The -Wstringop-overflow checker for unterminated constant char
arrays assumes that strnlen is called with exactly two arguments.
When the function is declared without a prototype and called with
no arguments the code aborts. This is PR 89911 (P1).
2) The wide_int min/max values of get_range_info() called on
the strnlen bound have the same precision as PTRDIFF_MAX.
That's not so when strnlen is declared without a prototype
and called with an int128_t argument in some range. Rather
than handling this case, wi::ltu_p() helpfully aborts instead.
This is PR 89957 that I exposed while testing the fix above.
The trivial patch avoids both of these assumptions. It's been
tested on x86_64-linux. Similar to the patch for PR 89934, I
will commit it later this week unless there are objections.
Martin
Patch for PR 89934 for reference:
https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00149.html
PR middle-end/89957 - ICE calling strnlen with an int128_t bound in a known range
PR middle-end/89911 - [9 Regression] ICE in get_attr_nonstring_decl
gcc/ChangeLog:
PR middle-end/89957
PR middle-end/89911
* builtins.c (expand_builtin_strnlen): Make sure wi::ltu_p operands
have the same precision since the function crashes otherwise.
* calls.c (maybe_warn_nonstring_arg): Avoid assuming strnlen() call
has non-zero arguments.
gcc/testsuite/ChangeLog:
PR middle-end/89957
PR middle-end/89911
* gcc.dg/Wstringop-overflow-13.c: New test.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 270149)
+++ gcc/builtins.c (working copy)
@@ -3151,7 +3151,7 @@ expand_builtin_strnlen (tree exp, rtx target, mach
return NULL_RTX;
if (!TREE_NO_WARNING (exp)
- && wi::ltu_p (wi::to_wide (maxobjsize), min)
+ && wi::ltu_p (wi::to_wide (maxobjsize, min.get_precision ()), min)
&& warning_at (loc, OPT_Wstringop_overflow_,
"%K%qD specified bound [%wu, %wu] "
"exceeds maximum object size %E",
Index: gcc/calls.c
===
--- gcc/calls.c (revision 270149)
+++ gcc/calls.c (working copy)
@@ -1555,7 +1555,10 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
if (TREE_NO_WARNING (exp) || !warn_stringop_overflow)
return;
+ /* Avoid clearly invalid calls (more checking done below). */
unsigned nargs = call_expr_nargs (exp);
+ if (!nargs)
+return;
/* The bound argument to a bounded string function like strncpy. */
tree bound = NULL_TREE;
Index: gcc/testsuite/gcc.dg/Wstringop-overflow-13.c
===
--- gcc/testsuite/gcc.dg/Wstringop-overflow-13.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-overflow-13.c (working copy)
@@ -0,0 +1,40 @@
+/* PR middle-end/89957 - ICE calling strnlen with an int128_t bound
+ in a known range
+ PR middle-end/89911 - ICE on a call with no arguments to strnlen
+ declared with no prototype
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern size_t strnlen ();
+
+size_t f0 (void)
+{
+ return strnlen (); /* { dg-warning "too few arguments to built-in function 'strnlen'" } */
+}
+
+size_t f1 (const char *s)
+{
+ return strnlen (s); /* { dg-warning "too few arguments to built-in function 'strnlen'" } */
+}
+
+size_t f2 (const char *s)
+{
+ return strnlen (s, s); /* { dg-warning "\\\[-Wint-conversion]" } */
+}
+
+#if __SIZEOF_INT128__ == 16
+
+size_t fi128 (const char *s, __int128_t n)
+{
+ if (n < 0)
+ n = 0;
+
+ /* PR middle-end/89957 */
+ return strnlen (s, n); /* { dg-warning "\\\[-Wbuiltin-declaration-mismatch]" "int128" { target int128 } } */
+}
+
+#endif
+
+/* { dg-prune-output "\\\[-Wint-conversion]" } */
Re: [PATCH] avoid more ICE due to bad built-in calls declared without a prototype (PR 89911, 89957)
On 4/4/19 3:42 PM, Martin Sebor wrote: > Attached is yet another patch to avoid ICE due to middle-end > assumptions about the sanity of calls to built-ins, this time > for strnlen. It fixes two unsafe assumptions: > > 1) The -Wstringop-overflow checker for unterminated constant char > arrays assumes that strnlen is called with exactly two arguments. > When the function is declared without a prototype and called with > no arguments the code aborts. This is PR 89911 (P1). > > 2) The wide_int min/max values of get_range_info() called on > the strnlen bound have the same precision as PTRDIFF_MAX. > That's not so when strnlen is declared without a prototype > and called with an int128_t argument in some range. Rather > than handling this case, wi::ltu_p() helpfully aborts instead. > This is PR 89957 that I exposed while testing the fix above. > > The trivial patch avoids both of these assumptions. It's been > tested on x86_64-linux. Similar to the patch for PR 89934, I > will commit it later this week unless there are objections. > > Martin > > Patch for PR 89934 for reference: > https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00149.html > > gcc-89911.diff > > PR middle-end/89957 - ICE calling strnlen with an int128_t bound in a known > range > PR middle-end/89911 - [9 Regression] ICE in get_attr_nonstring_decl > > gcc/ChangeLog: > > PR middle-end/89957 > PR middle-end/89911 > * builtins.c (expand_builtin_strnlen): Make sure wi::ltu_p operands > have the same precision since the function crashes otherwise. > * calls.c (maybe_warn_nonstring_arg): Avoid assuming strnlen() call > has non-zero arguments. > > gcc/testsuite/ChangeLog: > > PR middle-end/89957 > PR middle-end/89911 > * gcc.dg/Wstringop-overflow-13.c: New test. OK jeff
Re: PING [PATCH] fix ICE in __builtin_has_attribute (PR 88383 and 89288)
On 3/21/19 3:59 PM, Martin Sebor wrote: > On 3/19/19 9:33 PM, Jeff Law wrote: >> On 3/19/19 8:22 PM, Joseph Myers wrote: >>> On Tue, 19 Mar 2019, Jeff Law wrote: >>> I'll note that our documentation clearly states that attributes can be applied to functions, variables, labels, enums, statements and types. >>> >>> A key thing here is that they can be applied to fields - that is, >>> they can >>> be properties of a FIELD_DECL. Referring to a field either requires an >>> expression referring to that field, or some other custom syntax that >>> uses >>> both a type name and a field name (like in __builtin_offsetof). >>> >> Thanks for chiming in, your opinions on this kind of thing are greatly >> appreciated. >> >> Understood WRT applying to fields, and I think that's consistent with >> some of what Jakub has expressed elsewhere -- specifically that we >> should consider allowing COMPONENT_REFs as the exception, returning the >> attributes of the associated FIELD_DECL in that case. >> >> Is there a need to call out BIT_FIELD_REFs where we can't actually get >> to the underlying DECL? And if so, how do we do that in the end user >> documentation that is clear and consistent? > > Is it possible to see a BIT_FIELD_REF here? I couldn't find a way. Unsure. It was a generic concern -- I don't have a motivating example. From very light reading in the front-ends, you're more likely to get one from the C++ front-end rather the C front-end. It may also be able to get a BIT_FIELD_REF from some OMP constructs. In general it looks like the front-ends aren't generating them often, preferring to stick with COMPONENT_REF instead. > >> >> One of the big worries I've got here is that documenting when an >> expression as an argument to __builtin_has_attribute (or any attribute >> query) can be expected to work. It's always easier on our end users to >> loosen semantics of extensions over time than it is to tighten them. > > I wonder if a part of the issue isn't due to a mismatch between > the C terminology and what GCC uses internally. Every argument > to the built-in that's not a type (and every argument to attribute > copy which doesn't accept types) must be a C expression: My hesitation has been based more on the core concept that we don't attach attributes to expressions, only types and decls. Thus asking if an expression has any given attribute doesn't make much sense at first glance. In the copy attribute case the docs were pretty clear when and how it applied to expressions -- specifically stating that we're going to look at the underlying type and pull the attributes from there. Now that in turn raises its own set of issues. If the front-ends were to change the type of an expression (and they have in the past, particularly for COMPONENT_REF/INDIRECT_REF to avoid subsequent nop-casts), then we have to worry about the inconsistencies in behavior that's going to expose to the user. Another case where that could potentially happen would be LTO. I believe LTO will canonicalize/merge types to some degree and I don't offhand know the rules there and inconsistencies could be exposed to the user. Given that I think the front-end code that changed types of COMPONENT_REF/INDIRECT_REF to avoid subsequent nop-conversions is no longer in the tree and that (in theory) type merging in LTO should be considering attributes I went ahead and pushed those concerns aside for the copy attribute patch. I'll be doing the same when re-re-re-reviewing the __builtin_has_attribute patch :-) jeff
Re: [PATCH][MIPS] Fix pr89623 Can't build mips-wrs-vxworks cross-compiler
On 4/4/19 3:00 AM, Paul Hua wrote: > Hi, > > The MIPS target run out of Mask in mips.opt, we are stage4, this > patch retrieve loongson-ext that haven't used yet for now. In next > stage1, I will rewrite those part use HOST_WIDE_INT or same thing like > that. > > Ok for commit ? > > 2019-04-04 Chenghua Xu > > gcc/ > PR target/89623 > * config/mips/mips.opt (LOONGSON_EXT2): Use Var instead of Mask. > OK jeff
C/C++ PATCH for c++/89973 - -Waddress-of-packed-member ICE with invalid conversion
We ICE here because rhstype is null. Since we're looking to see if it's
a pointer type, we can just return NULL_TREE if it's null.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-04-04 Marek Polacek
PR c++/89973 - -Waddress-of-packed-member ICE with invalid conversion.
* c-warn.c (check_address_or_pointer_of_packed_member): Check the type
of RHS.
* g++.dg/warn/Waddress-of-packed-member2.C: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 4785887c1de..05ea2bf8719 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -2769,7 +2769,7 @@ check_address_or_pointer_of_packed_member (tree type,
tree rhs)
rhs = TREE_TYPE (rhs);/* Pointer type. */
rhs = TREE_TYPE (rhs);/* Function type. */
rhstype = TREE_TYPE (rhs);
- if (!POINTER_TYPE_P (rhstype))
+ if (!rhstype || !POINTER_TYPE_P (rhstype))
return NULL_TREE;
rvalue = true;
}
diff --git gcc/testsuite/g++.dg/warn/Waddress-of-packed-member2.C
gcc/testsuite/g++.dg/warn/Waddress-of-packed-member2.C
new file mode 100644
index 000..ec6edab2f41
--- /dev/null
+++ gcc/testsuite/g++.dg/warn/Waddress-of-packed-member2.C
@@ -0,0 +1,7 @@
+// PR c++/89973
+// { dg-do compile { target c++14 } }
+
+constexpr int a(); // { dg-warning "used but never defined" }
+
+template
+constexpr void *b = a(); // { dg-error "invalid conversion" }
RFA: Fix uninitialized memory use in sched_macro_fuse_insns
sched_macro_fuse_insns uses the value in condreg1 without
checking the return value of targetm.fixed_condition_code_regs. As
this variables
is not initialized anywhere, this leads to constructing cc_reg_1 with
an undefined value,
and then using that in reg_referenced_p, if TARGET_FIXED_CONDITION_CODE_REGS
has the default value as defined in target.def (hook_bool_uintp_uintp_false).
The attached patch fixes this by checking the return value of
targetm.fixed_condition_code_regs. Bootstrapped & regtested on
x86_64-pc-linux-gnu .
2019-04-04 Joern Rennecke
* sched-deps.c (sched_macro_fuse_insns): Check return value of
targetm.fixed_condition_code_regs.
Index: sched-deps.c
===
--- sched-deps.c(revision 270146)
+++ sched-deps.c(working copy)
@@ -2857,14 +2857,16 @@ sched_macro_fuse_insns (rtx_insn *insn)
{
unsigned int condreg1, condreg2;
rtx cc_reg_1;
- targetm.fixed_condition_code_regs (&condreg1, &condreg2);
- cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
- if (reg_referenced_p (cc_reg_1, PATTERN (insn))
- && modified_in_p (cc_reg_1, prev))
+ if (targetm.fixed_condition_code_regs (&condreg1, &condreg2))
{
- if (targetm.sched.macro_fusion_pair_p (prev, insn))
- SCHED_GROUP_P (insn) = 1;
- return;
+ cc_reg_1 = gen_rtx_REG (CCmode, condreg1);
+ if (reg_referenced_p (cc_reg_1, PATTERN (insn))
+ && modified_in_p (cc_reg_1, prev))
+ {
+ if (targetm.sched.macro_fusion_pair_p (prev, insn))
+ SCHED_GROUP_P (insn) = 1;
+ return;
+ }
}
}
[C++ PATCH] PR c++/86986 - ICE with TTP with parameter pack.
Three separate issues were breaking this testcase. One, we were trying to
look at the type of a template template parameter to see if it's a valid
non-type template parameter. Two, we were treating a parameter pack named
in the type of a template parameter pack of a TTP pack as being one of the
packs expanded by the outer pack. Three, we weren't supplying all the
necessary levels of template arguments when TTP matching.
Tested x86_64-pc-linux-gnu, applying to trunk.
* pt.c (coerce_template_parameter_pack): Only look at the type of a
non-type parameter pack.
(fixed_parameter_pack_p_1): Don't recurse into the type of a
non-type parameter pack.
(coerce_template_template_parms): Call add_outermost_template_args.
---
gcc/cp/pt.c| 14 +++---
gcc/testsuite/g++.dg/cpp0x/variadic-ttp9.C | 10 ++
gcc/cp/ChangeLog | 9 +
3 files changed, 30 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-ttp9.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9cb29d22ca3..20647be587a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5116,7 +5116,13 @@ fixed_parameter_pack_p_1 (tree parm, struct
find_parameter_pack_data *ppd)
tree vec = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm));
for (int i = 0; i < TREE_VEC_LENGTH (vec); ++i)
-fixed_parameter_pack_p_1 (TREE_VALUE (TREE_VEC_ELT (vec, i)), ppd);
+{
+ tree p = TREE_VALUE (TREE_VEC_ELT (vec, i));
+ if (template_parameter_pack_p (p))
+ /* Any packs in the type are expanded by this parameter. */;
+ else
+ fixed_parameter_pack_p_1 (p, ppd);
+}
}
/* PARM is a template parameter pack. Return any parameter packs used in
@@ -7554,6 +7560,7 @@ coerce_template_template_parms (tree parm_parms,
args and the converted args. If that succeeds, A is at least as
specialized as P, so they match.*/
tree pargs = template_parms_level_to_args (parm_parms);
+ pargs = add_outermost_template_args (outer_args, pargs);
++processing_template_decl;
pargs = coerce_template_parms (arg_parms, pargs, NULL_TREE, tf_none,
/*require_all*/true, /*use_default*/true);
@@ -8184,8 +8191,9 @@ coerce_template_parameter_pack (tree parms,
int j, len = TREE_VEC_LENGTH (packed_parms);
for (j = 0; j < len; ++j)
{
- tree t = TREE_TYPE (TREE_VEC_ELT (packed_parms, j));
- if (invalid_nontype_parm_type_p (t, complain))
+ tree t = TREE_VEC_ELT (packed_parms, j);
+ if (TREE_CODE (t) == PARM_DECL
+ && invalid_nontype_parm_type_p (TREE_TYPE (t), complain))
return error_mark_node;
}
/* We don't know how many args we have yet, just
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-ttp9.C
b/gcc/testsuite/g++.dg/cpp0x/variadic-ttp9.C
new file mode 100644
index 000..63e3f2684aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-ttp9.C
@@ -0,0 +1,10 @@
+// PR c++/86986
+// { dg-do compile { target c++11 } }
+
+template
+struct X {
+template class...>
+struct Y { };
+};
+
+using type = X::Y<>;
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 729b7732565..55a083fab6f 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,12 @@
+2019-04-04 Jason Merrill
+
+ PR c++/86986 - ICE with TTP with parameter pack.
+ * pt.c (coerce_template_parameter_pack): Only look at the type of a
+ non-type parameter pack.
+ (fixed_parameter_pack_p_1): Don't recurse into the type of a
+ non-type parameter pack.
+ (coerce_template_template_parms): Call add_outermost_template_args.
+
2019-04-04 Martin Sebor
PR c++/89974
base-commit: 7395ef5fd901285c705689bbfa3da70c70a5b237
--
2.20.1
[C++ PATCH] PR c++/89966 - error with non-type auto tparm.
My patch for PR 86932 broke this testcase by passing tf_partial to
coerce_template_template_parms, which prevented do_auto_deduction from
actually replacing the auto.
Tested x86_64-pc-linux-gnu, applying to trunk.
* pt.c (do_auto_deduction): Clear tf_partial.
---
gcc/cp/pt.c | 4
gcc/testsuite/g++.dg/cpp1z/nontype-auto15.C | 6 ++
gcc/cp/ChangeLog| 5 +
3 files changed, 15 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto15.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 20647be587a..40d954d1e8a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -27504,6 +27504,10 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
if (init && undeduced_auto_decl (init))
return type;
+ /* We may be doing a partial substitution, but we still want to replace
+ auto_node. */
+ complain &= ~tf_partial;
+
if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
/* C++17 class template argument deduction. */
return do_class_deduction (type, tmpl, init, flags, complain);
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto15.C
b/gcc/testsuite/g++.dg/cpp1z/nontype-auto15.C
new file mode 100644
index 000..d94885d19ea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto15.C
@@ -0,0 +1,6 @@
+// PR c++/89966
+// { dg-do compile { target c++17 } }
+
+template < auto a0 >
+void f0() { }
+void f0_call() { f0< sizeof(int) >(); }
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 55a083fab6f..ad4dba46bfe 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-04 Jason Merrill
+
+ PR c++/89966 - error with non-type auto tparm.
+ * pt.c (do_auto_deduction): Clear tf_partial.
+
2019-04-04 Jason Merrill
PR c++/86986 - ICE with TTP with parameter pack.
base-commit: 80d040496626199eb71219593cc941bb7fe22eca
--
2.20.1
[C++ PATCH] PR c++/89948 - ICE with break in statement-expr.
If 'jump_target' is local to this function and still set at the end, we can't
correctly evaluate it, and we don't need to try.
* constexpr.c (cxx_eval_statement_list): Jumping out of a
statement-expr is non-constant.
Tested x86_64-pc-linux-gnu, applying to trunk.
---
gcc/cp/constexpr.c| 9 +
gcc/testsuite/g++.dg/ext/stmtexpr23.C | 10 ++
gcc/cp/ChangeLog | 6 ++
3 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/ext/stmtexpr23.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 53854a8acd4..0ce5618a2fc 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4153,6 +4153,15 @@ cxx_eval_statement_list (const constexpr_ctx *ctx, tree
t,
if (returns (jump_target) || breaks (jump_target))
break;
}
+ if (*jump_target && jump_target == &local_target)
+{
+ /* We aren't communicating the jump to our caller, so give up. We don't
+need to support evaluation of jumps out of statement-exprs. */
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_loc (r, input_location),
+ "statement is not a constant expression");
+ *non_constant_p = true;
+}
return r;
}
diff --git a/gcc/testsuite/g++.dg/ext/stmtexpr23.C
b/gcc/testsuite/g++.dg/ext/stmtexpr23.C
new file mode 100644
index 000..c6aaea76f23
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/stmtexpr23.C
@@ -0,0 +1,10 @@
+// PR c++/89948
+// { dg-options "" }
+
+void f ()
+{
+ int a = 0;
+ for (;;)
+for (;a=+({break;1;});)
+ {}
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index ad4dba46bfe..6bfd26c6a2e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2019-04-04 Jason Merrill
+
+ PR c++/89948 - ICE with break in statement-expr.
+ * constexpr.c (cxx_eval_statement_list): Jumping out of a
+ statement-expr is non-constant.
+
2019-04-04 Jason Merrill
PR c++/89966 - error with non-type auto tparm.
base-commit: 1a08779b89cee120afe057ba013ad7e92e62798f
--
2.20.1
Re: C++ PATCH for c++/87145 - bogus error converting class type in template argument list
On 4/4/19 5:18 PM, Marek Polacek wrote:
On Thu, Mar 28, 2019 at 03:54:30PM -0400, Jason Merrill wrote:
On 3/20/19 4:12 PM, Marek Polacek wrote:
The fix for 77656 caused us to call convert_nontype_argument even for
value-dependent arguments, to perform the conversion in order to avoid
a bogus warning.
In this case, the argument is Pod{N}. The call to build_converted_constant_expr
in convert_nontype_argument produces Pod::operator Enum(&{N}). It doesn't crash
because we're in a template and build_address no longer crashes on CONSTRUCTORs
in a template.
Yeah, we shouldn't be preserving lower level codes like this ADDR_EXPR; we
should probably return an IMPLICIT_CONV_EXPR rather than make the call
explicit.
I'm having trouble with this. Do we want build_converted_constant_expr_internal
to build an IMPLICIT_CONV_EXPR in a template, much like
perform_implicit_conversion? That seems to break a lot of stuff, and I'm
nervous about that at this stage :/ We could probably handle this specially
in convert_nontype_argument. Maybe build an IMPLICIT_CONV_EXPR for
value-dependent CONSTRUCTORs?
That sounds reasonable.
Jason
Re: C/C++ PATCH for c++/89973 - -Waddress-of-packed-member ICE with invalid conversion
On 4/4/19 7:20 PM, Marek Polacek wrote: We ICE here because rhstype is null. Since we're looking to see if it's a pointer type, we can just return NULL_TREE if it's null. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-04-04 Marek Polacek PR c++/89973 - -Waddress-of-packed-member ICE with invalid conversion. * c-warn.c (check_address_or_pointer_of_packed_member): Check the type of RHS. OK. Jason
Re: [PATCH] handle strings as template arguments (PR 47488, 89833, 89876)
On Thu, Apr 4, 2019 at 11:30 AM Martin Sebor wrote:
>
> On 4/4/19 10:50 AM, Jason Merrill wrote:
> > On 4/4/19 12:29 PM, Martin Sebor wrote:
> >> On 4/4/19 8:57 AM, Jason Merrill wrote:
> >>> On 4/3/19 10:34 PM, Martin Sebor wrote:
> On 4/1/19 11:27 AM, Jason Merrill wrote:
> > On 3/31/19 10:17 PM, Martin Sebor wrote:
> >> To fix PR 89833, a P1 regression, the attached patch tries to
> >> handle string literals as C++ 2a non-type template arguments
> >> by treating them the same as brace enclosed initializer lists
> >> (where the latter are handled correctly). The solution makes
> >> sure equivalent forms of array initializers such as for char[5]:
> >>
> >>"\0\1\2"
> >>"\0\1\2\0"
> >>{ 0, 1, 2 }
> >>{ 0, 1, 2, 0 }
> >>{ 0, 1, 2, 0, 0 }
> >>
> >> are treated as the same, both for overloading and mangling.
> >> Ditto for the following equivalent forms:
> >>
> >>""
> >>"\0"
> >>"\0\0\0\0"
> >>{ }
> >>{ 0 }
> >>{ 0, 0, 0, 0, 0 }
> >>
> >> and for these of struct { char a[5], b[5], c[5]; }:
> >>
> >>{ {0,0,0,0,0}, {0,0,0,0}, {0,0,0} }
> >>{ { 0 }, { } }
> >>{ "" }
> >>
> >> Since this is not handled correctly by the current code (see PR
> >> 89876 for a test case) the patch also fixes that.
> >>
> >> I'm not at all confident the handling of classes with user-defined
> >> constexpr ctors is 100% correct. (I use triviality to constrain
> >> the solution for strings but that was more of an off-the-cuff guess
> >> than a carefully considered decision).
> >
> > You could use TYPE_HAS_TRIVIAL_DFLT since we don't care about the
> > triviality of other operations.
>
> Done (I think).
>
> >
> > I wouldn't worry about trying to omit user-defined constexpr ctors.
> >
> >> The g++.dg/abi/mangle71.C
> >> test is all I've got in terms of verifying it works correctly.
> >> I'm quite sure the C++ 2a testing could stand to be beefed up.
> >>
> >> The patch passes x86_64-linux bootstrap and regression tests.
> >> There are a few failures in check-c++-all tests that don't look
> >> related to the changes (I see them with an unpatched GCC as well):
> >>
> >>g++.dg/spellcheck-macro-ordering-2.C
> >>g++.dg/cpp0x/auto52.C
> >>g++.dg/cpp1y/auto-neg1.C
> >>g++.dg/other/crash-9.C
> >
> > You probably need to check zero_init_p to properly handle pointers
> > to data members, where a null value is integer -1; given
> >
> > struct A { int i; };
> >
> > constexpr A::* pm = &A::i;
> > int A::* pma[] = { pm, pm };
> >
> > we don't want to discard the initializers because they look like
> > zeros, as then digest_init will add back -1s.
>
> I added it but it wasn't doing the right thing. It mangled { } and
> { 0 } differently:
>
> typedef int A::*pam_t;
> struct B { pam_t a[2]; };
> template struct Y { };
>
> void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0E
> void f (Y) { } // _Z1f1YIXtl1BtlA2_M1AiLS2_0ELS2_0E
>
> because the zeros didn't get removed. They need to be mangled the
> same,
> don't they?
>
> I've added three tests to exercise this (array51.C,
> nontype-class16.C, and mangle72.C). They pass without the
> zero_init_p() calls but some
> fail with it (depending on where it's added). Please check to see
> that the tests really do exercise what they should.
>
> If you think a zero_init_p() check really is needed I will need some
> guidance where (a test case would be great).
> >>>
> >>> Hmm, let me poke at this a bit.
> >>
> >> Here's a test case showing that mangling null member pointers
> >> as zero leads to conflicts:
> >>
> >>struct A { int i; };
> >>typedef int A::*pam_t;
> >>struct B { pam_t a[2]; };
> >>template struct Y { };
> >>
> >>// both mangle as _Z2.31YIXtl1BtlA2_M1AiLS2_0ELS2_0E
> >>void f (Y) { }
> >>void g (Y) { }
> >>
> >> This happens both with trunk and with my patch. They probably
> >> need to mangle as negative one, don't you think? (There's
> >> a comment saying mangling them as zeros is intentional but not
> >> why.)
> >
> > Hmm. If we leave out f, then g mangles as 0 and &A::i, which is fine.
> > Need to figure out why creating a B before the declaration of g breaks
> > that.
>
> Let me know if you want me to start looking into this or if you
> are on it yourself.
>
> >
> > The rationale for mangling as 0 would have been to reflect what you
> > would write in the source: (int A::*)0 does give a null member pointer.
> > But then we really can't use that representation 0 for anything else, we
> > have to use a symbolic representation. This patch doesn't need to fix
> > that.
>
> Okay.
