[pushed] c++: Avoid unnecessary empty class copy [94175].
A simple empty class copy is still simple when wrapped in a TARGET_EXPR, so
we need to strip that as well. This change also exposed some unnecessary
copies in return statements, which when returning by invisible reference led
to >>, which gimplify_return_expr didn't
like. So we also need to strip the _REF when we eliminate the INIT_EXPR.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog
2020-03-19 Jason Merrill
PR c++/94175
* cp-gimplify.c (simple_empty_class_p): Look through
SIMPLE_TARGET_EXPR_P.
(cp_gimplify_expr) [MODIFY_EXPR]: Likewise.
[RETURN_EXPR]: Avoid producing 'return *retval;'.
* call.c (build_call_a): Strip TARGET_EXPR from empty class arg.
* cp-tree.h (SIMPLE_TARGET_EXPR_P): Check that TARGET_EXPR_INITIAL
is non-null.
---
gcc/cp/cp-tree.h | 1 +
gcc/cp/call.c | 4
gcc/cp/cp-gimplify.c | 13 -
gcc/testsuite/g++.dg/abi/empty30.C | 14 ++
4 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/abi/empty30.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 757cdd8168a..0783b3114f2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5145,6 +5145,7 @@ more_aggr_init_expr_args_p (const
aggr_init_expr_arg_iterator *iter)
the initializer has void type, it's doing something more complicated. */
#define SIMPLE_TARGET_EXPR_P(NODE) \
(TREE_CODE (NODE) == TARGET_EXPR \
+ && TARGET_EXPR_INITIAL (NODE) \
&& !VOID_TYPE_P (TREE_TYPE (TARGET_EXPR_INITIAL (NODE
/* True if EXPR expresses direct-initialization of a TYPE. */
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1715acc0ec3..65a3ea35dee 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -392,6 +392,10 @@ build_call_a (tree function, int n, tree *argarray)
if (is_empty_class (TREE_TYPE (arg))
&& simple_empty_class_p (TREE_TYPE (arg), arg, INIT_EXPR))
{
+ while (TREE_CODE (arg) == TARGET_EXPR)
+ /* We're disconnecting the initializer from its target,
+don't create a temporary. */
+ arg = TARGET_EXPR_INITIAL (arg);
tree t = build0 (EMPTY_CLASS_EXPR, TREE_TYPE (arg));
arg = build2 (COMPOUND_EXPR, TREE_TYPE (t), arg, t);
CALL_EXPR_ARG (function, i) = arg;
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 87c7e394b01..aa80384e1a4 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -603,6 +603,10 @@ simple_empty_class_p (tree type, tree op, tree_code code)
{
if (TREE_CODE (op) == COMPOUND_EXPR)
return simple_empty_class_p (type, TREE_OPERAND (op, 1), code);
+ if (SIMPLE_TARGET_EXPR_P (op)
+ && TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
+/* The TARGET_EXPR is itself a simple copy, look through it. */
+return simple_empty_class_p (type, TARGET_EXPR_INITIAL (op), code);
return
(TREE_CODE (op) == EMPTY_CLASS_EXPR
|| code == MODIFY_EXPR
@@ -740,6 +744,11 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
else if (simple_empty_class_p (TREE_TYPE (op0), op1, code))
{
+ while (TREE_CODE (op1) == TARGET_EXPR)
+ /* We're disconnecting the initializer from its target,
+don't create a temporary. */
+ op1 = TARGET_EXPR_INITIAL (op1);
+
/* Remove any copies of empty classes. Also drop volatile
variables on the RHS to avoid infinite recursion from
gimplify_expr trying to load the value. */
@@ -754,6 +763,9 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
gimplify_expr (_OPERAND (*expr_p, 0), pre_p, post_p,
is_gimple_lvalue, fb_lvalue);
*expr_p = TREE_OPERAND (*expr_p, 0);
+ if (code == RETURN_EXPR && REFERENCE_CLASS_P (*expr_p))
+ /* Avoid 'return *;' */
+ *expr_p = TREE_OPERAND (*expr_p, 0);
}
/* P0145 says that the RHS is sequenced before the LHS.
gimplify_modify_expr gimplifies the RHS before the LHS, but that
@@ -924,7 +936,6 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
|| TREE_CODE (TREE_OPERAND (*expr_p, 0)) == MODIFY_EXPR))
{
expr_p = _OPERAND (*expr_p, 0);
- code = TREE_CODE (*expr_p);
/* Avoid going through the INIT_EXPR case, which can
degrade INIT_EXPRs into AGGR_INIT_EXPRs. */
goto modify_expr_case;
diff --git a/gcc/testsuite/g++.dg/abi/empty30.C
b/gcc/testsuite/g++.dg/abi/empty30.C
new file mode 100644
index 000..f10d2034e36
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/empty30.C
@@ -0,0 +1,14 @@
+// PR c++/94175
+// { dg-do link }
+
+struct A {};
+extern A a;
+
+int i;
+__attribute
Re: [PATCH PR94026] combine missed opportunity to simplify comparisons with zero
On Thu, Mar 19, 2020 at 01:43:40AM +, Yangfei (Felix) wrote: > 2. Given that the patterns for ubfx and ubfiz are already not simple, I am > afraid the pattern we got by combining the three would be much complex. > And even more complex when further merged with insn 14 here in order to > make sure that we are doing a equality comparison with zero. It will be just as simple as with the other approach: > > Another approach: > > > > Trying 7 -> 9: > > 7: r99:SI=r103:SI>>0x8 > > REG_DEAD r103:SI > > 9: cc:CC_NZ=cmp(r99:SI&0x6,0) > > REG_DEAD r99:SI > > Failed to match this instruction: > > (set (reg:CC_NZ 66 cc) > > (compare:CC_NZ (and:SI (lshiftrt:SI (reg:SI 103) > > (const_int 8 [0x8])) > > (const_int 6 [0x6])) > > (const_int 0 [0]))) > > > > This can be recognised as just that "tst" insn, no? But combine (or > > simplify-rtx) should get rid of the shift here, just the "and" is simpler > > after all (it > > just needs to change the constant for that). > > No, this does not mean an equality comparison with zero. I have mentioned > this in my previous mail. This should be simplified to (set (reg:CC_NZ 66 cc) (compare:CC_NZ (and:SI (reg:SI 103) (const_int 1536)) (const_int 0))) (but it isn't), and that is just *and3nr_compare0, which is a "tst" instruction. If this is fixed (in simplify-rtx.c), it will work as you want. Segher
Re: Re: Re: [PATCH, rs6000] Add command line and builtin compatibility
Hi Carl,
On Wed, Mar 18, 2020 at 04:19:18PM -0700, Carl Love wrote:
> > Yes, but only for this fprnd vs. 2.06 (vsx) situation. Like we
> > already
> > have:
> >
> > if (TARGET_DIRECT_MOVE && !TARGET_VSX)
> > {
> > if (rs6000_isa_flags_explicit & OPTION_MASK_DIRECT_MOVE)
> > error ("%qs requires %qs", "-mdirect-move", "-mvsx");
> > rs6000_isa_flags &= ~OPTION_MASK_DIRECT_MOVE;
> > }
> >
> > (and many other cases there), we could do this there as well (so,
> > don't
> > allow -mvsx (maybe via a -mcpu= etc.) at the same time as -mno-
> > fprnd).
>
> I redid the patch to try and make it more general. It looks to me like
> TARGET_VSX is set for Power 7 and newer.
By default, yes. But you can use -mno-vsx, and you can use -mvsx with
older CPUs as well (but you really really really shouldn't).
> I setup a test similar to the
> example checking TARGET_VSX. So if you are on a Power 7 then -mvsx is
> set for you, i.e. the user would not have to explicitly use the option.
> My objection to the error message in the example is that the user
> wouldn't necessarily know what processor or ISA is implied by -mvsx.
> So in my error message I called out the processor number. We could do
> it based on ISA. I figure the user is more likely to know the
> processor version then the ISA level supported by the processor so went
> with the processor number in the patch. Thoughts?
>
> gcc -mno-fprnd -g -mcpu=power7 -c vsx-builtin-3.c
> cc1: error: ‘-mno-fprnd’ not compatible with Power 7 and newer
I think it should say
error ("%qs requires %qs", "-mvsx", "-mfprnd");
(It's easier to not use negatives, and, this is more consistent with
other such tests).
> * gcc/config/rs6000/rs6000.c (altivec_resolve_overloaded_builtin):
> Add check for TARGET_FRND for Power 7 or newer.
(It's in a different function now, and, TARGET_FPRND).
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index ac9455e3b7c..5c72a863dbf 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3716,6 +3716,14 @@ rs6000_option_override_internal (bool global_init_p)
>rs6000_isa_flags &= ~OPTION_MASK_CRYPTO;
> }
>
> + if (!TARGET_FPRND && TARGET_VSX)
> +{
> + if (rs6000_isa_flags_explicit & OPTION_MASK_FPRND)
> + /* TARGET_VSX = 1 implies Power 7 and newer */
> + error ("%qs not compatible with Power 7 and newer", "-mno-fprnd");
> + rs6000_isa_flags &= ~OPTION_MASK_FPRND;
> +}
Please make such changes if you agree. Either way, okay for trunk.
Thank you, and sorry the review took so long.
Segher
Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)
On 3/18/20 9:07 PM, Jason Merrill wrote:
On 3/12/20 6:38 PM, Martin Sebor wrote:
...
After a lot more trial and error I discovered
most_specialized_partial_spec in pt.c with whose help I have been able
to get templates to work the way I think they should (at least the cases
I've tested do).
Besides fixing the original problem that motivated it, the attached
patch also corrects how template specializations are handled: the first
declaration of either a primary template or its specialization (either
explicit or partial) determines the class-key in subsequent uses of
the type or its instantiations. To do this for uses of first-time
template instantiations such as in the declaration of s1 in the test
case above, class_decl_loc_t::diag_mismatched_tags looks up the template
(either the primary or the partial specialization) in the CLASS2LOC map
and uses it and its class-key as the guide when issuing diagnostics.
This also means that the first instance of every template needs to
have a record in the CLASS2LOC map (it also needs it to compare its
class-key to subsequent redeclarations of the template).
This has been unexpectedly difficult. A big part of it is that I've
never before worked with templates in the front-end so I had to learn
how they're organized (I'm far from having mastered it). What's made
the learning curve especially steep, besides the sparse documentation,
is the problems hinted at in the paragraph below below. This whole
area could really stand to be documented in some sort of a writeup:
a high-level overview of how templates are put together (i.e., what
hangs off what in the tree representation) and what APIs to use to
work with them.
The revised patch has been tested on x86_64-linux.
...
+ FIXME: Relying on cp_parser_declares_only_class_p to diffeerentiate
"differentiate"
+ declarations of a class from its uses doesn't work for type aliases
+ (as in using T = class C;). */
Good point. Perhaps we could pass flags to
cp_parser_declares_only_class_p and have it return false if
CP_PARSER_FLAGS_TYPENAME_OPTIONAL, since that is set for an alias but
not for a normal type-specifier.
I wondered if there was a way to identify that we're dealing with
an alias. CP_PARSER_FLAGS_TYPENAME_OPTIONAL is set not just for
those but also for template declarations (in
cp_parser_single_declaration) but I was able to make it work by
tweaking cp_parser_type_specifier. It doesn't feel very clean
(it seems like either the bit or all of cp_parser_flags could be
a member of the parser class or some subobject of it) but it does
the job. Thanks for pointing me in the right direction!
+ DECL corresponsds. IS_USE should be set when DECL refers to a class
"corresponds"
+ for (tree t = decl_type;
+ t != NULL_TREE;
+ t = CLASSTYPE_USE_TEMPLATE (t)
+ ? TREE_TYPE (CLASSTYPE_TI_TEMPLATE (t)) : NULL_TREE
+ )
+ {
+ if (same_type_ignoring_top_level_qualifiers_p (t, decl_type))
Looks like on the first iteration, t == decl_type, so this will always
be true, and this loop has no effect, right?
Right.
+ if (tree spec = most_specialized_partial_spec (ret,
tsubst_flags_t ()))
tf_none instead of tsubst_flags_t ().
+ if (!decl_p && !def_p && TREE_CODE (decl) == TEMPLATE_DECL)
{
+ /* When TYPE is the use of an implicit specialization of a
previously
+ declared template set TYPE_DECL to the type of the primary template
+ for the specialization and look it up in CLASS2LOC below. For uses
+ of explicit or partial specializations TYPE_DECL already points to
+ the declaration of the specialization. */
+ type_decl = specialization_of (type_decl);
Here shouldn't is_use be true?
If it were set to true here we would find the partial specialization
corresponding to its specialization in the use when what we want is
the latter. As a result, for the following:
templatestruct S;
template struct S;
extern class S s1; // expect -Wmismatched-tags
extern struct S s2;
we'd end up with a warning for s2 pointing to the instantiation of
s1 as the "guiding declaration:"
z.C:5:15: warning: ‘template struct S’ declared with a
mismatched class-key ‘struct’ [-Wmismatched-tags]
5 | extern struct S s2;
| ^~~
z.C:5:15: note: remove the class-key or replace it with ‘class’
z.C:4:15: note: ‘template struct S’ first declared as
‘class’ here
4 | extern class S s1; // expect -Wmismatched-tags
| ^~~
By setting it to false here we record the type of the instantiation
instead along with the class-key so that we can diagnose the mismatch
between the partial and its use.
+ /* Save the current function before changing it below. */
+ tree save_func = current_function_decl;
Let's change this (and the restoration at the bottom of the function) to
a temp_override sentinel.
Attached is a revision with the updates above, rested on x86_64. To
check
Re: [PATCH 0/6] aarch64: Implement TImode comparisons
Hi Richard,
> Any compare can be done in at most 2 instructions:
>
> void doit(void);
> void f(long long a)
> {
> if (a <= 1)
> doit();
> }
>
> f:
> cmp r0, #2
> sbcs r3, r1, #0
> blt .L4
> Well, this one requires that you be able to add 1 to an input and for that
> input to not overflow. But you're right that I should be using this sequence
> for LT (not LE).
And for GE. For LE and GT swap operands and condition. You can safely
increment the immediate since small immediates that fit CMP have no
chance of overflowing and large immediates have to be split off anyway
(and then are treated like register variants).
Cheers,
Wilco
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On 3/19/20 2:06 PM, Patrick Palka via Gcc-patches wrote:
On Thu, 19 Mar 2020, Marek Polacek wrote:
On Thu, Mar 19, 2020 at 01:06:35PM -0400, Patrick Palka via Gcc-patches wrote:
On Thu, 19 Mar 2020, Patrick Palka wrote:
This patch adds a check to detect changing the active union member during
initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to in
cxx_eval_store_expression is in the process of being initialized, which seems to
work well.
If we can't rely on CONSTRUCTOR_NO_CLEARING to be set iff a CONSTRUCTOR
is in the process of being initialized, then here's an alternative patch
for consideration, that detects this UB in an indirect way and after the
fact.
Yeah, I'm not sure if that would work well, especially in C++20 where we
sometimes don't clear it:
/* The result of a constexpr function must be completely initialized.
However, in C++20, a constexpr constructor doesn't necessarily have
to initialize all the fields, so we don't clear CONSTRUCTOR_NO_CLEARING
in order to detect reading an unitialized object in constexpr instead
of value-initializing it. (reduced_constant_expression_p is expected to
take care of clearing the flag.) */
if (TREE_CODE (result) == CONSTRUCTOR
&& (cxx_dialect < cxx2a
|| !DECL_CONSTRUCTOR_P (fun)))
clear_no_implicit_zero (result);
and rely on reduced_constant_expression_p to clear it.
I see, thanks. Here's a reproducer for the issue you pointed out, which
is a valid testcase but gets rejected with the proposed patch:
union U
{
int x;
char y;
};
constexpr bool
baz ()
{
U u;
u.x = 3;
u.y = 7;
return (u.y == 7);
}
static_assert (baz ());
CONSTRUCTOR_NO_CLEARING is set for 'u' and is not cleared after its
constructor returns, and so the check yields a false positive for the
assignment to u.y. That's unfortunate...
We should probably clear the flag when we assign to u.x because once we
give a value to one union member, the union has a value.
Jason
Re: [PATCH] c-family: Tighten vector handling in type_for_mode [PR94072]
On Thu, 19 Mar 2020, Richard Sandiford wrote: > In this PR we had a 512-bit VECTOR_TYPE whose mode is XImode > (an integer mode used for four 128-bit vectors). When trying > to expand a zero constant for it, we hit code in expand_expr_real_1 > that tries to use the associated integer type instead. The code used > type_for_mode (XImode, 1) to get this integer type. > > However, the c-family implementation of type_for_mode checks for > any registered built-in type that matches the mode and has the > right signedness. This meant that it could return a built-in > vector type when given an integer mode (particularly if, as here, > the vector type isn't supported by the current subtarget and so > TYPE_MODE != TYPE_MODE_RAW). The expand code would then cycle > endlessly trying to use this "new" type instead of the original > vector type. > > The search loop is probably too lax in other ways -- e.g. it could > return records that just happen to have the right mode -- but this > seems like a safe, incremental improvement. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On Thu, 19 Mar 2020, Jason Merrill wrote:
> On 3/19/20 12:35 PM, Patrick Palka wrote:
> > This patch adds a check to detect changing the active union member during
> > initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
> > proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to
> > in
> > cxx_eval_store_expression is in the process of being initialized, which
> > seems to
> > work well.
>
> > In order for this check to work reliably, we also have to adjust
> > cxx_eval_bare_aggregate to set the active union member before processing the
> > initializer.
> >
> > Does this look OK to commit after testing?
>
> That makes sense. OK.
Thanks. The issue Marek points out unfortunately makes this approach of
using CONSTRUCTOR_NO_CLEARING unreliable and regresses other testcases,
and I'm afraid I don't know how to salvage this approach...
>
> > gcc/cp/ChangeLog:
> >
> > PR c++/94066
> > * constexpr.c (cxx_eval_bare_aggregate): When constructing a union,
> > always set the active union member before evaluating the initializer.
> > Relax assertion that verifies the index of the constructor element
> > we're
> > initializing hasn't been changed.
> > (cxx_eval_store_expression): Diagnose changing the active union member
> > while the union is in the process of being initialized.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/94066
> > * g++.dg/cpp1y/pr94066.C: New test.
> > * g++.dg/cpp1y/pr94066-2.C: New test.
> > * g++.dg/cpp1y/pr94066-3.C: New test.
> > ---
> > gcc/cp/constexpr.c | 25 -
> > gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++
> > gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 18 ++
> > gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 ++
> > 4 files changed, 79 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
> >
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 192face9a3a..97fe5572f71 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -3751,6 +3751,11 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> > tree t,
> > /* If we built a new CONSTRUCTOR, attach it now so that other
> >initializers can refer to it. */
> > CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
> > + else if (TREE_CODE (type) == UNION_TYPE)
> > + /* If we're constructing a union, set the active union member now so
> > + that we can later detect if the initializer attempts to activate
> > + another member. */
> > + CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
> > tree elt = cxx_eval_constant_expression (_ctx, value,
> >lval,
> >non_constant_p, overflow_p);
> > @@ -3784,7 +3789,12 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
> > tree t,
> > }
> > else
> > {
> > - if (new_ctx.ctor != ctx->ctor)
> > + if (TREE_CODE (type) == UNION_TYPE
> > + && (*p)->last().index != index)
> > + /* The initializer may have erroneously changed the active union
> > + member we were initializing. */
> > + gcc_assert (*non_constant_p);
> > + else if (new_ctx.ctor != ctx->ctor)
> > {
> > /* We appended this element above; update the value. */
> > gcc_assert ((*p)->last().index == index);
> > @@ -4647,6 +4657,19 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
> > tree t,
> > index);
> > *non_constant_p = true;
> > }
> > + else if (TREE_CODE (t) == MODIFY_EXPR
> > + && CONSTRUCTOR_NO_CLEARING (*valp))
> > + {
> > + /* Diagnose changing the active union member while the union
> > +is in the process of being initialized. */
> > + if (!ctx->quiet)
> > + error_at (cp_expr_loc_or_input_loc (t),
> > + "change of the active member of a union "
> > + "from %qD to %qD during initialization",
> > + CONSTRUCTOR_ELT (*valp, 0)->index,
> > + index);
> > + *non_constant_p = true;
> > + }
> > /* Changing active member. */
> > vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
> > no_zero_init = true;
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > new file mode 100644
> > index 000..1c00b650961
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/94066
> > +// { dg-do compile { target c++14 } }
> > +
> > +struct A { long x; };
> > +
> > +union U;
Re: [PATCH] c++: template keyword in a typename-specifier [PR94057]
On 3/19/20 5:28 PM, Marek Polacek wrote:
Consider
template class A {
template class B {
void fn(typename A::B);
};
};
which is rejected with
error: 'typename A::B' names 'template template class
A::B', which is not a type
whereas clang/icc/msvc accept it.
"typename A::B" is a typename-specifier. Sadly, our comments
don't mention it anywhere, because the typename-specifier wasn't in C++11;
it was only added to the language in N1376. Instead, we handle it as
an elaborated-type-specifier (not a problem thus far). So we get to
cp_parser_nested_name_specifier_opt which has a loop that breaks if we
don't see a < or ::, but that means we can -- tentatively -- parse even
B which is not a nested-name-specifier (it doesn't end with a ::).
Even though we're parsing B tentatively, we issue an error in
cp_parser_class_name -> make_typename_type, but here we should not. In
fact, we probably shouldn't have parsed "B" at all. Fixed by the
cp_parser_class_name hunk.
I think this should compile because [temp.names]/4 says: "In a qualified-id
used as the name in a typename-specifier, elaborated-type-specifier,
using-declaration, or class-or-decltype, an optional keyword template
appearing at the top level is ignored.", added in DR 1710. Also see
DR 1812.
Looks good, but please add tests for the other contexts mentioned in
that passage.
This issue on its own is not a significant problem or a regression.
However, in C++20, the typename here becomes optional, and so this test
is rejected in C++20, but accepted in C++17:
template class A {
template class B {
void fn(A::B);
};
};
Here we morph A::B into a typename-specifier, but that happens
in cp_parser_simple_type_specifier and we never handle it as above.
To fake the template keyword I'm afraid we need to use cp_parser_template_id
with template_keyword_p=true as in the patch below. The tricky thing
is to avoid breaking concepts.
Does this approach make sense? Should these tests be accepted because
of DR 1710 or am I off base here?
Apologies for the verbosity, but I felt it necessary.
Verbosity isn't a problem. :)
Bootstrapped/regtested on x86_64-linux, built Boost/cmcstl2, ok for trunk?
PR c++/94057 - template keyword in a typename-specifier.
* parser.c (cp_parser_simple_type_specifier): Assume that a <
following a qualified-id in a typename-specifier begins
a template argument list.
(cp_parser_class_name): Complain only if not parsing tentatively.
* g++.dg/template/dependent-name5.C: Update dg-error.
* g++.dg/template/dependent-name7.C: New test.
* g++.dg/template/dependent-name8.C: New test.
* g++.dg/template/dependent-name9.C: New test.
---
gcc/cp/parser.c | 32 +--
.../g++.dg/template/dependent-name5.C | 2 --
.../g++.dg/template/dependent-name7.C | 9 ++
.../g++.dg/template/dependent-name8.C | 9 ++
.../g++.dg/template/dependent-name9.C | 9 ++
5 files changed, 57 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name7.C
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name8.C
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name9.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index cbd5510a8fb..f4175955992 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -18113,6 +18113,33 @@ cp_parser_simple_type_specifier (cp_parser* parser,
}
}
}
+ /* DR 1812: A < following a qualified-id in a typename-specifier
+could safely be assumed to begin a template argument list, so
+the template keyword should be optional. */
+ else if (parser->scope
+ && qualified_p
+ && typename_p
+ && cp_lexer_next_token_is (parser->lexer, CPP_TEMPLATE_ID))
+ {
+ cp_parser_parse_tentatively (parser);
+
+ type = cp_parser_template_id (parser,
+ /*template_keyword_p=*/true,
+ /*check_dependency_p=*/true,
+ none_type,
+ /*is_declaration=*/false);
+ /* This is handled below, so back off. */
+ if (type && concept_check_p (type))
+ cp_parser_simulate_error (parser);
+
+ if (!cp_parser_parse_definitely (parser))
+ type = NULL_TREE;
+ else if (TREE_CODE (type) == TEMPLATE_ID_EXPR)
+ type = make_typename_type (parser->scope, type, typename_type,
+ /*complain=*/tf_error);
+ else if (TREE_CODE (type) != TYPE_DECL)
+ type = NULL_TREE;
+ }
/* Otherwise, look for a type-name. */
if (!type)
@@ -23636,8 +23663,9 @@ cp_parser_class_name (cp_parser *parser,
&& decl !=
Re: [PATCH] c: Fix up cfun->function_end_locus from the C FE [PR94029]
On Thu, 19 Mar 2020, Jakub Jelinek via Gcc-patches wrote: > The second instead propagates the location_t from the parsing of the > outermost compound statement (the function body) to finish_function. > > Both patches successfully bootstrapped/regtested on x86_64-linux and > i686-linux, ok for trunk (which one)? The second patch is OK. -- Joseph S. Myers jos...@codesourcery.com
Fix cgraph_node::function_symbol availability computation [PR94202]
Hi,
this fixes ICE in inliner cache sanity check which is caused by very old
bug in visibility calculation in cgraph_node::function_symbol and
cgraph_node::function_or_virtual_thunk_symbol.
In the testcase there is indirect call to a thunk. At begining we correctly
see that its body as AVAIL_AVAILABLE but later we inline into the thunk and
this turns it to AVAIL_INTERPOSABLE.
This is because function_symbol incorrectly overwrites availability parameter
by availability of the alias used in the call within thunk, which is a local
alias.
Bootstrap/regtest x86_64-linux in progress, plan to commit if succesfull.
gcc/ChangeLog:
2020-03-19 Jan Hubicka
PR ipa/94202
* cgraph.c (cgraph_node::function_symbol): Fix availability computation.
(cgraph_node::function_or_virtual_thunk_symbol): Likewise.
gcc/testsuite/ChangeLog:
2020-03-19 Jan Hubicka
PR ipa/94202
* g++.dg/torture/pr94202.C: New test.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 9f0774f227f..b41dea1fcca 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3788,16 +3788,13 @@ cgraph_node::function_symbol (enum availability
*availability,
while (node->thunk.thunk_p)
{
+ enum availability a;
+
ref = node;
node = node->callees->callee;
- if (availability)
- {
- enum availability a;
- a = node->get_availability (ref);
- if (a < *availability)
- *availability = a;
- }
- node = node->ultimate_alias_target (availability, ref);
+ node = node->ultimate_alias_target (availability ? : NULL, ref);
+ if (availability && a < *availability)
+ *availability = a;
}
return node;
}
@@ -3818,16 +3815,13 @@ cgraph_node::function_or_virtual_thunk_symbol
while (node->thunk.thunk_p && !node->thunk.virtual_offset_p)
{
+ enum availability a;
+
ref = node;
node = node->callees->callee;
- if (availability)
- {
- enum availability a;
- a = node->get_availability (ref);
- if (a < *availability)
- *availability = a;
- }
- node = node->ultimate_alias_target (availability, ref);
+ node = node->ultimate_alias_target (availability ? : NULL, ref);
+ if (availability && a < *availability)
+ *availability = a;
}
return node;
}
diff --git a/gcc/testsuite/g++.dg/torture/pr94202.C
b/gcc/testsuite/g++.dg/torture/pr94202.C
new file mode 100644
index 000..ab077368f9f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr94202.C
@@ -0,0 +1,21 @@
+struct S1 {
+ virtual ~S1();
+ virtual void v();
+};
+struct S2: S1 {};
+struct S3: S1, S2 { void v(); };
+struct S4: S3 { void v(); };
+void S4::v() { S3::v(); }
+struct R {
+ S1 * m;
+ void f(S2 * x) {
+static_cast(x)->v();
+x->v();
+m = x;
+ }
+};
+void f() {
+ R r;
+ r.f(new S4);
+ r.f(new S3);
+}
[PATCH] c++: template keyword in a typename-specifier [PR94057]
Consider
template class A {
template class B {
void fn(typename A::B);
};
};
which is rejected with
error: 'typename A::B' names 'template template class
A::B', which is not a type
whereas clang/icc/msvc accept it.
"typename A::B" is a typename-specifier. Sadly, our comments
don't mention it anywhere, because the typename-specifier wasn't in C++11;
it was only added to the language in N1376. Instead, we handle it as
an elaborated-type-specifier (not a problem thus far). So we get to
cp_parser_nested_name_specifier_opt which has a loop that breaks if we
don't see a < or ::, but that means we can -- tentatively -- parse even
B which is not a nested-name-specifier (it doesn't end with a ::).
Even though we're parsing B tentatively, we issue an error in
cp_parser_class_name -> make_typename_type, but here we should not. In
fact, we probably shouldn't have parsed "B" at all. Fixed by the
cp_parser_class_name hunk.
I think this should compile because [temp.names]/4 says: "In a qualified-id
used as the name in a typename-specifier, elaborated-type-specifier,
using-declaration, or class-or-decltype, an optional keyword template
appearing at the top level is ignored.", added in DR 1710. Also see
DR 1812.
This issue on its own is not a significant problem or a regression.
However, in C++20, the typename here becomes optional, and so this test
is rejected in C++20, but accepted in C++17:
template class A {
template class B {
void fn(A::B);
};
};
Here we morph A::B into a typename-specifier, but that happens
in cp_parser_simple_type_specifier and we never handle it as above.
To fake the template keyword I'm afraid we need to use cp_parser_template_id
with template_keyword_p=true as in the patch below. The tricky thing
is to avoid breaking concepts.
Does this approach make sense? Should these tests be accepted because
of DR 1710 or am I off base here?
Apologies for the verbosity, but I felt it necessary.
Bootstrapped/regtested on x86_64-linux, built Boost/cmcstl2, ok for trunk?
PR c++/94057 - template keyword in a typename-specifier.
* parser.c (cp_parser_simple_type_specifier): Assume that a <
following a qualified-id in a typename-specifier begins
a template argument list.
(cp_parser_class_name): Complain only if not parsing tentatively.
* g++.dg/template/dependent-name5.C: Update dg-error.
* g++.dg/template/dependent-name7.C: New test.
* g++.dg/template/dependent-name8.C: New test.
* g++.dg/template/dependent-name9.C: New test.
---
gcc/cp/parser.c | 32 +--
.../g++.dg/template/dependent-name5.C | 2 --
.../g++.dg/template/dependent-name7.C | 9 ++
.../g++.dg/template/dependent-name8.C | 9 ++
.../g++.dg/template/dependent-name9.C | 9 ++
5 files changed, 57 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name7.C
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name8.C
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name9.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index cbd5510a8fb..f4175955992 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -18113,6 +18113,33 @@ cp_parser_simple_type_specifier (cp_parser* parser,
}
}
}
+ /* DR 1812: A < following a qualified-id in a typename-specifier
+could safely be assumed to begin a template argument list, so
+the template keyword should be optional. */
+ else if (parser->scope
+ && qualified_p
+ && typename_p
+ && cp_lexer_next_token_is (parser->lexer, CPP_TEMPLATE_ID))
+ {
+ cp_parser_parse_tentatively (parser);
+
+ type = cp_parser_template_id (parser,
+ /*template_keyword_p=*/true,
+ /*check_dependency_p=*/true,
+ none_type,
+ /*is_declaration=*/false);
+ /* This is handled below, so back off. */
+ if (type && concept_check_p (type))
+ cp_parser_simulate_error (parser);
+
+ if (!cp_parser_parse_definitely (parser))
+ type = NULL_TREE;
+ else if (TREE_CODE (type) == TEMPLATE_ID_EXPR)
+ type = make_typename_type (parser->scope, type, typename_type,
+ /*complain=*/tf_error);
+ else if (TREE_CODE (type) != TYPE_DECL)
+ type = NULL_TREE;
+ }
/* Otherwise, look for a type-name. */
if (!type)
@@ -23636,8 +23663,9 @@ cp_parser_class_name (cp_parser *parser,
&& decl != error_mark_node
&& !is_overloaded_fn (decl))
{
- decl = make_typename_type (scope, decl, typename_type,
-/*complain=*/tf_error);
+ tsubst_flags_t
Re: [stage1][PATCH] Provide hint for misspelled -fdump-foo options.
On Thu, 2020-03-19 at 14:52 +0100, Martin Liška wrote:
> Hi.
Hi Martin.
> The patch is about basic hint support for -fdump-foo options where
> one can newly get something like:
>
> $ ./xgcc -B. /tmp/foo.c -fdump-ipa-ynline -c
> cc1: error: unrecognized command-line option ‘-fdump-ipa-ynline’; did you
> mean ‘-fdump-ipa-inline’?
> $ ./xgcc -B. /tmp/foo.c -fdump-tree-switchlowe -c
> cc1: error: unrecognized command-line option ‘-fdump-tree-switchlowe’; did
> you mean ‘-fdump-tree-switchlower1’?
> $ ./xgcc -B. /tmp/foo.c -fdump-rtl-sched -c
> cc1: error: unrecognized command-line option ‘-fdump-rtl-sched’; did you mean
> ‘-fdump-rtl-sched1’?
>
> I also considered the same support for --completion option but it's more
> problematic
> as the --completion is handled in driver. In driver we do not instantiate
> pass_manager
> where we have listed all passes (and their corresponding dump options).
This seems like a reasonable restriction; instantiating passes doesn't
seem like something we should be doing from within the driver (what
about things like dumps for arch-specific passes?)
Thinking aloud: would it make sense to move --completion to the
compiler to get around this?
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed in next stage1?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2020-03-19 Martin Liska
>
> * dumpfile.c (dump_switch_p): Change return type
> and print option suggestion.
Strictly speaking this is gcc::dump_manager::dump_switch_p.
[...]
> -int
> +void
> gcc::dump_manager::
> dump_switch_p (const char *arg)
> {
> @@ -1896,8 +1897,22 @@ dump_switch_p (const char *arg)
> for (i = 0; i < m_extra_dump_files_in_use; i++)
>any |= dump_switch_p_1 (arg, _extra_dump_files[i], true);
>
> -
> - return any;
> + if (!any)
> +{
> + char *s;
> + auto_vec candidates;
> + for (size_t i = TDI_none + 1; i != TDI_end; i++)
> + candidates.safe_push (dump_files[i].swtch);
> + for (size_t i = 0; i < m_extra_dump_files_in_use; i++)
> + candidates.safe_push (m_extra_dump_files[i].swtch);
If I'm reading this right, this covers the simplest cases, but misses
various valid options.
You did indeed describe it as "basic hint support", so fair enough, I
suppose.
What about the glob and "-all" variants?
Also, parse_dump_option does various things here.
Should this be integrated into option_proposer in opt-suggestions.c?
But maybe that's overkill if we're not going to do completion.
> + const char *hint = candidates_list_and_hint (arg, s, candidates);
> + if (hint)
> + error ("unrecognized command-line option %<-fdump-%s%>; "
> +"did you mean %<-fdump-%s%>?", arg, hint);
> + else
> + error ("unrecognized command-line option %<-fdump-%s%>", arg);
> + XDELETEVEC (s);
> +}
> }
candidates_list_and_hint builds a list of space-separated candidates
and writes it back to s (for showing to the user). The above code
ignores this, which is probably good given that everything has an
implicit prefix of "-fdump-" which wouldn't be in those listed strings.
I don't like the way the patch is building that string and then doing
nothing with it. At first I thought it would be cleaner to convert
candidates_list_and_hint's "char *" param to a "char **out_str",
and support it being NULL, in which case not to build the candidatate
list.
But even simpler would be to use spellcheck.h's:
extern const char *
find_closest_string (const char *target,
const auto_vec *candidates);
to get the hint without building the candidate list.
Why not just do that?
A random other thought: the spellcheck code makes use of overall
lengths to decide if a candidate is a meaningful suggestion, and I
wonder if it's an issue that we're implicitly stripping off and then
re-adding that "-fdump-" prefix. Perhaps not an issue.
[...]
> diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-22.c
> b/gcc/testsuite/gcc.dg/spellcheck-options-22.c
> new file mode 100644
> index 000..b0ddae2e78e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/spellcheck-options-22.c
> @@ -0,0 +1,3 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-ipa-ynline" } */
> +/* { dg-error "unrecognized command-line option '-fdump-ipa-ynline'; did you
> mean '-fdump-ipa-inline'?" "" { target *-*-* } 0 } */
There are various ways in which gcc::dump_manager::dump_switch_p's
"any" can be set, so it would be good to have test coverage of each of
them - but I'm not sure how feasible that is.
What about the "-all" variants?
Maybe that's overkill though.
Hope this is constructive
Dave
[PATCH] include/dwarf2.h: Sync with binutils
DW_CIE_VERSION is unused by GCC and has been removed from binutils by commit 66f8b2cbbb675ccbcae56e2bdb6dae485878ec00 Author: Andrew Burgess Date: Mon Nov 4 12:27:45 2019 + gas: Add --gdwarf-cie-version command line flag * dwarf2.h (DW_CIE_VERSION): Delete. --- include/dwarf2.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/dwarf2.h b/include/dwarf2.h index 0b6facfd4cf..882453dce08 100644 --- a/include/dwarf2.h +++ b/include/dwarf2.h @@ -316,7 +316,6 @@ enum dwarf_location_list_entry_type #define DW_CIE_ID0x #define DW64_CIE_ID 0xULL -#define DW_CIE_VERSION 1 #define DW_CFA_extended 0 -- 2.25.1
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On 3/19/20 12:35 PM, Patrick Palka wrote:
This patch adds a check to detect changing the active union member during
initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to in
cxx_eval_store_expression is in the process of being initialized, which seems to
work well.
In order for this check to work reliably, we also have to adjust
cxx_eval_bare_aggregate to set the active union member before processing the
initializer.
Does this look OK to commit after testing?
That makes sense. OK.
gcc/cp/ChangeLog:
PR c++/94066
* constexpr.c (cxx_eval_bare_aggregate): When constructing a union,
always set the active union member before evaluating the initializer.
Relax assertion that verifies the index of the constructor element we're
initializing hasn't been changed.
(cxx_eval_store_expression): Diagnose changing the active union member
while the union is in the process of being initialized.
gcc/testsuite/ChangeLog:
PR c++/94066
* g++.dg/cpp1y/pr94066.C: New test.
* g++.dg/cpp1y/pr94066-2.C: New test.
* g++.dg/cpp1y/pr94066-3.C: New test.
---
gcc/cp/constexpr.c | 25 -
gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++
gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 18 ++
gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 ++
4 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 192face9a3a..97fe5572f71 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3751,6 +3751,11 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
/* If we built a new CONSTRUCTOR, attach it now so that other
initializers can refer to it. */
CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+ else if (TREE_CODE (type) == UNION_TYPE)
+ /* If we're constructing a union, set the active union member now so
+ that we can later detect if the initializer attempts to activate
+ another member. */
+ CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
tree elt = cxx_eval_constant_expression (_ctx, value,
lval,
non_constant_p, overflow_p);
@@ -3784,7 +3789,12 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
}
else
{
- if (new_ctx.ctor != ctx->ctor)
+ if (TREE_CODE (type) == UNION_TYPE
+ && (*p)->last().index != index)
+ /* The initializer may have erroneously changed the active union
+ member we were initializing. */
+ gcc_assert (*non_constant_p);
+ else if (new_ctx.ctor != ctx->ctor)
{
/* We appended this element above; update the value. */
gcc_assert ((*p)->last().index == index);
@@ -4647,6 +4657,19 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
index);
*non_constant_p = true;
}
+ else if (TREE_CODE (t) == MODIFY_EXPR
+ && CONSTRUCTOR_NO_CLEARING (*valp))
+ {
+ /* Diagnose changing the active union member while the union
+is in the process of being initialized. */
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ CONSTRUCTOR_ELT (*valp, 0)->index,
+ index);
+ *non_constant_p = true;
+ }
/* Changing active member. */
vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
no_zero_init = true;
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
new file mode 100644
index 000..1c00b650961
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
@@ -0,0 +1,19 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr A foo(U *up);
+
+union U {
+ U() = default;
+ A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+ up->y = 11; // { dg-error "'U::a' to 'U::y'" }
+ return {42};
+}
+
+extern constexpr U u = {};
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
new file mode 100644
index 000..6bf1ec81885
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
@@ -0,0 +1,18 @@
+// PR c++/94066
+// { dg-do
Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
On 3/19/20 12:50 PM, Patrick Palka wrote:
On Thu, 19 Mar 2020, Jason Merrill wrote:
On 3/18/20 3:26 PM, Patrick Palka wrote:
+ if (map)
+{
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, map);
+ pp_cxx_right_bracket (pp);
+}
Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping rather
than duplicate them here.
Like this?
-- >8 --
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern. Move
the "[with ]" bits to here from ...
(pp_cxx_atomic_constraint): ... here.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the dependent form of the constraint
and the
preferably substituted parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 12 ---
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 36 +++--
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14
4 files changed, 33 insertions(+), 30 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..15d8b7eb609 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,9 +2878,13 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+
for (tree p = map; p; p = TREE_CHAIN (p))
{
tree parm = TREE_VALUE (p);
@@ -2903,6 +2907,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree
map)
if (TREE_CHAIN (p) != NULL_TREE)
pp_cxx_separate_with (pp, ';');
}
+
+ pp_cxx_right_bracket (pp);
}
void
@@ -2915,12 +2921,8 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp, tree t)
tree map = ATOMIC_CONSTR_MAP (t);
if (map && map != error_mark_node)
{
- pp_cxx_whitespace (pp);
- pp_cxx_left_bracket (pp);
- pp->translate_string ("with");
pp_cxx_whitespace (pp);
Is there a reason not to move the initial whitespace as well, like in
dump_substitution? OK with that change.
pp_cxx_parameter_mapping (pp, map);
- pp_cxx_right_bracket (pp);
}
}
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..9f19e9bbaa4 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t
loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
-return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
-return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,19 @@ print_concept_check_info (diagnostic_context *context,
tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
-check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n",
check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On Thu, Mar 19, 2020 at 12:56 PM Richard Biener wrote: > > On March 19, 2020 5:51:14 PM GMT+01:00, "H.J. Lu" wrote: > >On Thu, Mar 19, 2020 at 9:00 AM Martin Liška wrote: > >> > >> On 3/19/20 4:50 PM, H.J. Lu wrote: > >> > I like it and I will take case of binutils side. > >> > > >> > Thanks. > >> > >> Great. I've just installed the 2 patches to master. > >> > > > >Here is the binutils patch: > > > >https://sourceware.org/pipermail/binutils/2020-March/110295.html > > If the plugin advertises v2 support but the LTO IL files lack symtab_ext data > what happens? Did you mean symbol_type == LDST_UNKNOWN? In that case, you get the text symbol for everything. -- H.J.
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On March 19, 2020 5:51:14 PM GMT+01:00, "H.J. Lu" wrote: >On Thu, Mar 19, 2020 at 9:00 AM Martin Liška wrote: >> >> On 3/19/20 4:50 PM, H.J. Lu wrote: >> > I like it and I will take case of binutils side. >> > >> > Thanks. >> >> Great. I've just installed the 2 patches to master. >> > >Here is the binutils patch: > >https://sourceware.org/pipermail/binutils/2020-March/110295.html If the plugin advertises v2 support but the LTO IL files lack symtab_ext data what happens? Richard.
[PATCH d] Committed merge upstream dmd d1a606599
Fixes long standing regression in the D front-end implemention, adds a
new field to allow retrieving a list of all content imports from the
code generator, and fixes a recently introduced test to use old-style
syntax for contracts.
Bootstrapped and tested on x86_64-linux-gnu, and committed to trunk.
Regards
Iain.
---
gcc/d/dmd/MERGE| 2 +-
gcc/d/dmd/dclass.c | 1 -
gcc/d/dmd/expressionsem.c | 1 +
gcc/d/dmd/module.h | 1 +
.../gdc.test/compilable/imports/pr9471a.d | 2 ++
.../gdc.test/compilable/imports/pr9471b.d | 5 +
.../gdc.test/compilable/imports/pr9471c.d | 18 ++
.../gdc.test/compilable/imports/pr9471d.d | 1 +
gcc/testsuite/gdc.test/compilable/pr9471.d | 6 ++
gcc/testsuite/gdc.test/runnable/traits.d | 4 ++--
10 files changed, 37 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
create mode 100644 gcc/testsuite/gdc.test/compilable/pr9471.d
diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 6cbc4e37819..a421448a287 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-b061bd744cb4eb94a7118581387d988d4ec25e97
+d1a606599e7c2bea8fda8bf5e3ddceb486ae69ac
The first line of this file holds the git revision number of the last
merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/dclass.c b/gcc/d/dmd/dclass.c
index bbe2f8a9d72..4609d6a9f54 100644
--- a/gcc/d/dmd/dclass.c
+++ b/gcc/d/dmd/dclass.c
@@ -395,7 +395,6 @@ void ClassDeclaration::semantic(Scope *sc)
}
else if (symtab && !scx)
{
-semanticRun = PASSsemanticdone;
return;
}
semanticRun = PASSsemantic;
diff --git a/gcc/d/dmd/expressionsem.c b/gcc/d/dmd/expressionsem.c
index 781bd3ea5fd..fed36cf9242 100644
--- a/gcc/d/dmd/expressionsem.c
+++ b/gcc/d/dmd/expressionsem.c
@@ -2370,6 +2370,7 @@ public:
return setError();
}
+sc->_module->contentImportedFiles.push(name);
if (global.params.verbose)
message("file %.*s\t(%s)", (int)se->len, (char *)se->string,
name);
if (global.params.moduleDeps != NULL)
diff --git a/gcc/d/dmd/module.h b/gcc/d/dmd/module.h
index 4a20356db89..f4253d32657 100644
--- a/gcc/d/dmd/module.h
+++ b/gcc/d/dmd/module.h
@@ -76,6 +76,7 @@ public:
unsigned numlines; // number of lines in source file
int isDocFile; // if it is a documentation input file, not D source
bool isPackageFile; // if it is a package.d
+Strings contentImportedFiles; // array of files whose content was imported
int needmoduleinfo;
int selfimports;// 0: don't know, 1: does not, 2: does
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
b/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
new file mode 100644
index 000..79b78e1e52a
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
@@ -0,0 +1,2 @@
+import imports.pr9471c;
+class AggregateDeclaration : ScopeDsymbol { }
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
b/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
new file mode 100644
index 000..a46a12c496f
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
@@ -0,0 +1,5 @@
+import imports.pr9471a;
+class ClassDeclaration : AggregateDeclaration
+{
+void isBaseOf();
+}
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
b/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
new file mode 100644
index 000..d80a61480ce
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
@@ -0,0 +1,18 @@
+import imports.pr9471b;
+
+struct Array(T)
+{
+static if (is(typeof(T.opCmp))) { }
+}
+alias ClassDeclarations = Array!ClassDeclaration;
+
+class Dsymbol
+{
+void addObjcSymbols(ClassDeclarations);
+}
+
+class ScopeDsymbol : Dsymbol
+{
+import imports.pr9471d;
+void importScope();
+}
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
b/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
new file mode 100644
index 000..187b9083294
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
@@ -0,0 +1 @@
+// Module needs to be imported to trigger bug.
diff --git a/gcc/testsuite/gdc.test/compilable/pr9471.d
b/gcc/testsuite/gdc.test/compilable/pr9471.d
new file mode 100644
index 000..37ff32e4957
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/pr9471.d
@@ -0,0 +1,6 @@
+// PERMUTE_ARGS:
+// EXTRA_FILES: imports/pr9471a.d imports/pr9471b.d imports/pr9471c.d
imports/pr9471d.d
+import imports.pr9471a;
+import imports.pr9471b;
+
+static assert (__traits(getVirtualIndex,
Re: [PATCH, d] Committed merge with upstream dmd e9420cfbf
On 15/03/2020 14:32, Rainer Orth wrote:
> Hi Ian,
>
>> This patch merges the D front-end implementation with dmd upstream e9420cfbf.
> [...]
>> Bootstrapped and tested on x86_64-linux-gnu, and committed to trunk.
>
> this merge introduced a regression on Solaris (SPARC and x86):
>
> +UNRESOLVED: gdc.test/runnable/traits.d compilation failed to produce
> executable
> +UNRESOLVED: gdc.test/runnable/traits.d -shared-libphobos compilation
> failed to produce executable
>
> runnable/traits.d:1256:23: error: statement expected to be { }, not (^M
> runnable/traits.d:1256:29: error: found 'out' when expecting ';' following
> statement^M
> runnable/traits.d:1256:32: error: declaration expected, not '('^M
> runnable/traits.d:1256:38: error: no identifier for declarator r^M
> runnable/traits.d:1256:38: error: declaration expected, not '=='^M
> runnable/traits.d:1257:1: error: unrecognized declaration^M
>
> I suspect the changes need to be moved to their own file in
> gdc.test/fail_compilation.
>
The backported test used a newer syntax for contracts that's not in the C++
port of the D front-end. I've amended this and committed the fix.
Iain.
[PATCH v2][ARM][GCC][7x]: MVE vreinterpretq and vuninitializedq intrinsics.
Hello Kyrill, This patch addresses all the comments in patch version v2. (version v2) https://gcc.gnu.org/pipermail/gcc-patches/2019-November/534351.html Hello, This patch supports following MVE ACLE intrinsics. vreinterpretq_s16_s32, vreinterpretq_s16_s64, vreinterpretq_s16_s8, vreinterpretq_s16_u16, vreinterpretq_s16_u32, vreinterpretq_s16_u64, vreinterpretq_s16_u8, vreinterpretq_s32_s16, vreinterpretq_s32_s64, vreinterpretq_s32_s8, vreinterpretq_s32_u16, vreinterpretq_s32_u32, vreinterpretq_s32_u64, vreinterpretq_s32_u8, vreinterpretq_s64_s16, vreinterpretq_s64_s32, vreinterpretq_s64_s8, vreinterpretq_s64_u16, vreinterpretq_s64_u32, vreinterpretq_s64_u64, vreinterpretq_s64_u8, vreinterpretq_s8_s16, vreinterpretq_s8_s32, vreinterpretq_s8_s64, vreinterpretq_s8_u16, vreinterpretq_s8_u32, vreinterpretq_s8_u64, vreinterpretq_s8_u8, vreinterpretq_u16_s16, vreinterpretq_u16_s32, vreinterpretq_u16_s64, vreinterpretq_u16_s8, vreinterpretq_u16_u32, vreinterpretq_u16_u64, vreinterpretq_u16_u8, vreinterpretq_u32_s16, vreinterpretq_u32_s32, vreinterpretq_u32_s64, vreinterpretq_u32_s8, vreinterpretq_u32_u16, vreinterpretq_u32_u64, vreinterpretq_u32_u8, vreinterpretq_u64_s16, vreinterpretq_u64_s32, vreinterpretq_u64_s64, vreinterpretq_u64_s8, vreinterpretq_u64_u16, vreinterpretq_u64_u32, vreinterpretq_u64_u8, vreinterpretq_u8_s16, vreinterpretq_u8_s32, vreinterpretq_u8_s64, vreinterpretq_u8_s8, vreinterpretq_u8_u16, vreinterpretq_u8_u32, vreinterpretq_u8_u64, vreinterpretq_s32_f16, vreinterpretq_s32_f32, vreinterpretq_u16_f16, vreinterpretq_u16_f32, vreinterpretq_u32_f16, vreinterpretq_u32_f32, vreinterpretq_u64_f16, vreinterpretq_u64_f32, vreinterpretq_u8_f16, vreinterpretq_u8_f32, vreinterpretq_f16_f32, vreinterpretq_f16_s16, vreinterpretq_f16_s32, vreinterpretq_f16_s64, vreinterpretq_f16_s8, vreinterpretq_f16_u16, vreinterpretq_f16_u32, vreinterpretq_f16_u64, vreinterpretq_f16_u8, vreinterpretq_f32_f16, vreinterpretq_f32_s16, vreinterpretq_f32_s32, vreinterpretq_f32_s64, vreinterpretq_f32_s8, vreinterpretq_f32_u16, vreinterpretq_f32_u32, vreinterpretq_f32_u64, vreinterpretq_f32_u8, vreinterpretq_s16_f16, vreinterpretq_s16_f32, vreinterpretq_s64_f16, vreinterpretq_s64_f32, vreinterpretq_s8_f16, vreinterpretq_s8_f32, vuninitializedq_u8, vuninitializedq_u16, vuninitializedq_u32, vuninitializedq_u64, vuninitializedq_s8, vuninitializedq_s16, vuninitializedq_s32, vuninitializedq_s64, vuninitializedq_f16, vuninitializedq_f32 and vuninitializedq. Please refer to M-profile Vector Extension (MVE) intrinsics [1] for more details. [1] https://developer.arm.com/architectures/instruction-sets/simd-isas/helium/mve-intrinsics Regression tested on arm-none-eabi and found no regressions. Ok for trunk? Thanks, Srinath. gcc/ChangeLog: 2020-03-19 Srinath Parvathaneni * config/arm/arm_mve.h (vreinterpretq_s16_s32): Define macro. (vreinterpretq_s16_s64): Likewise. (vreinterpretq_s16_s8): Likewise. (vreinterpretq_s16_u16): Likewise. (vreinterpretq_s16_u32): Likewise. (vreinterpretq_s16_u64): Likewise. (vreinterpretq_s16_u8): Likewise. (vreinterpretq_s32_s16): Likewise. (vreinterpretq_s32_s64): Likewise. (vreinterpretq_s32_s8): Likewise. (vreinterpretq_s32_u16): Likewise. (vreinterpretq_s32_u32): Likewise. (vreinterpretq_s32_u64): Likewise. (vreinterpretq_s32_u8): Likewise. (vreinterpretq_s64_s16): Likewise. (vreinterpretq_s64_s32): Likewise. (vreinterpretq_s64_s8): Likewise. (vreinterpretq_s64_u16): Likewise. (vreinterpretq_s64_u32): Likewise. (vreinterpretq_s64_u64): Likewise. (vreinterpretq_s64_u8): Likewise. (vreinterpretq_s8_s16): Likewise. (vreinterpretq_s8_s32): Likewise. (vreinterpretq_s8_s64): Likewise. (vreinterpretq_s8_u16): Likewise. (vreinterpretq_s8_u32): Likewise. (vreinterpretq_s8_u64): Likewise. (vreinterpretq_s8_u8): Likewise. (vreinterpretq_u16_s16): Likewise. (vreinterpretq_u16_s32): Likewise. (vreinterpretq_u16_s64): Likewise. (vreinterpretq_u16_s8): Likewise. (vreinterpretq_u16_u32): Likewise. (vreinterpretq_u16_u64): Likewise. (vreinterpretq_u16_u8): Likewise. (vreinterpretq_u32_s16): Likewise. (vreinterpretq_u32_s32): Likewise. (vreinterpretq_u32_s64): Likewise. (vreinterpretq_u32_s8): Likewise. (vreinterpretq_u32_u16): Likewise. (vreinterpretq_u32_u64): Likewise. (vreinterpretq_u32_u8): Likewise. (vreinterpretq_u64_s16): Likewise. (vreinterpretq_u64_s32): Likewise. (vreinterpretq_u64_s64): Likewise. (vreinterpretq_u64_s8): Likewise. (vreinterpretq_u64_u16): Likewise. (vreinterpretq_u64_u32): Likewise. (vreinterpretq_u64_u8): Likewise. (vreinterpretq_u8_s16): Likewise.
[PATCH v2][ARM][GCC][6x]:MVE ACLE vaddq intrinsics using arithmetic plus operator.
Hello Kyrill,
This patch addresses all the comments in patch version v2.
(version v2) https://gcc.gnu.org/pipermail/gcc-patches/2019-November/534349.html
Hello,
This patch supports following MVE ACLE vaddq intrinsics. The RTL patterns for
this intrinsics
are added using arithmetic "plus" operator.
vaddq_s8, vaddq_s16, vaddq_s32, vaddq_u8, vaddq_u16, vaddq_u32, vaddq_f16,
vaddq_f32.
Please refer to M-profile Vector Extension (MVE) intrinsics [1] for more
details.
[1]
https://developer.arm.com/architectures/instruction-sets/simd-isas/helium/mve-intrinsics
Regression tested on arm-none-eabi and found no regressions.
Ok for trunk?
Thanks,
Srinath.
gcc/ChangeLog:
2020-03-19 Srinath Parvathaneni
Andre Vieira
Mihail Ionescu
* config/arm/arm_mve.h (vaddq_s8): Define macro.
(vaddq_s16): Likewise.
(vaddq_s32): Likewise.
(vaddq_u8): Likewise.
(vaddq_u16): Likewise.
(vaddq_u32): Likewise.
(vaddq_f16): Likewise.
(vaddq_f32): Likewise.
(__arm_vaddq_s8): Define intrinsic.
(__arm_vaddq_s16): Likewise.
(__arm_vaddq_s32): Likewise.
(__arm_vaddq_u8): Likewise.
(__arm_vaddq_u16): Likewise.
(__arm_vaddq_u32): Likewise.
(__arm_vaddq_f16): Likewise.
(__arm_vaddq_f32): Likewise.
(vaddq): Define polymorphic variant.
* config/arm/iterators.md (VNIM): Define mode iterator for common types
Neon, IWMMXT and MVE.
(VNINOTM): Likewise.
* config/arm/mve.md (mve_vaddq): Define RTL pattern.
(mve_vaddq_f): Define RTL pattern.
* config/arm/neon.md (add3): Rename to addv4hf3 RTL pattern.
(addv8hf3_neon): Define RTL pattern.
* config/arm/vec-common.md (add3): Modify standard add RTL pattern
to support MVE.
(addv8hf3): Define standard RTL pattern for MVE and Neon.
(add3): Modify existing standard add RTL pattern for Neon and
IWMMXT.
gcc/testsuite/ChangeLog:
2020-03-19 Srinath Parvathaneni
Andre Vieira
Mihail Ionescu
* gcc.target/arm/mve/intrinsics/vaddq_f16.c: New test.
* gcc.target/arm/mve/intrinsics/vaddq_f32.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_s16.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_s32.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_s8.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_u16.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_u32.c: Likewise.
* gcc.target/arm/mve/intrinsics/vaddq_u8.c: Likewise.
### Attachment also inlined for ease of reply###
diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index
5ea42bd6a5bd98d5c77a0e7da3464ba6b431770b..55c256910bb7f4c616ea592be699f7f4fc3f17f7
100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -1898,6 +1898,14 @@ typedef struct { uint8x16_t val[4]; } uint8x16x4_t;
#define vstrwq_scatter_shifted_offset_p_u32(__base, __offset, __value, __p)
__arm_vstrwq_scatter_shifted_offset_p_u32(__base, __offset, __value, __p)
#define vstrwq_scatter_shifted_offset_s32(__base, __offset, __value)
__arm_vstrwq_scatter_shifted_offset_s32(__base, __offset, __value)
#define vstrwq_scatter_shifted_offset_u32(__base, __offset, __value)
__arm_vstrwq_scatter_shifted_offset_u32(__base, __offset, __value)
+#define vaddq_s8(__a, __b) __arm_vaddq_s8(__a, __b)
+#define vaddq_s16(__a, __b) __arm_vaddq_s16(__a, __b)
+#define vaddq_s32(__a, __b) __arm_vaddq_s32(__a, __b)
+#define vaddq_u8(__a, __b) __arm_vaddq_u8(__a, __b)
+#define vaddq_u16(__a, __b) __arm_vaddq_u16(__a, __b)
+#define vaddq_u32(__a, __b) __arm_vaddq_u32(__a, __b)
+#define vaddq_f16(__a, __b) __arm_vaddq_f16(__a, __b)
+#define vaddq_f32(__a, __b) __arm_vaddq_f32(__a, __b)
#endif
__extension__ extern __inline void
@@ -12341,6 +12349,48 @@ __arm_vstrwq_scatter_shifted_offset_u32 (uint32_t *
__base, uint32x4_t __offset,
__builtin_mve_vstrwq_scatter_shifted_offset_uv4si ((__builtin_neon_si *)
__base, __offset, __value);
}
+__extension__ extern __inline int8x16_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+__arm_vaddq_s8 (int8x16_t __a, int8x16_t __b)
+{
+ return __a + __b;
+}
+
+__extension__ extern __inline int16x8_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+__arm_vaddq_s16 (int16x8_t __a, int16x8_t __b)
+{
+ return __a + __b;
+}
+
+__extension__ extern __inline int32x4_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+__arm_vaddq_s32 (int32x4_t __a, int32x4_t __b)
+{
+ return __a + __b;
+}
+
+__extension__ extern __inline uint8x16_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+__arm_vaddq_u8 (uint8x16_t __a, uint8x16_t __b)
+{
+ return __a + __b;
+}
+
+__extension__ extern __inline uint16x8_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On Thu, 19 Mar 2020, Marek Polacek wrote:
> On Thu, Mar 19, 2020 at 01:06:35PM -0400, Patrick Palka via Gcc-patches wrote:
> > On Thu, 19 Mar 2020, Patrick Palka wrote:
> >
> > > This patch adds a check to detect changing the active union member during
> > > initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as
> > > a
> > > proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning
> > > to in
> > > cxx_eval_store_expression is in the process of being initialized, which
> > > seems to
> > > work well.
> >
> > If we can't rely on CONSTRUCTOR_NO_CLEARING to be set iff a CONSTRUCTOR
> > is in the process of being initialized, then here's an alternative patch
> > for consideration, that detects this UB in an indirect way and after the
> > fact.
>
> Yeah, I'm not sure if that would work well, especially in C++20 where we
> sometimes don't clear it:
>
> /* The result of a constexpr function must be completely initialized.
>
> However, in C++20, a constexpr constructor doesn't necessarily have
> to initialize all the fields, so we don't clear CONSTRUCTOR_NO_CLEARING
> in order to detect reading an unitialized object in constexpr instead
> of value-initializing it. (reduced_constant_expression_p is expected to
> take care of clearing the flag.) */
> if (TREE_CODE (result) == CONSTRUCTOR
> && (cxx_dialect < cxx2a
> || !DECL_CONSTRUCTOR_P (fun)))
> clear_no_implicit_zero (result);
>
> and rely on reduced_constant_expression_p to clear it.
I see, thanks. Here's a reproducer for the issue you pointed out, which
is a valid testcase but gets rejected with the proposed patch:
union U
{
int x;
char y;
};
constexpr bool
baz ()
{
U u;
u.x = 3;
u.y = 7;
return (u.y == 7);
}
static_assert (baz ());
CONSTRUCTOR_NO_CLEARING is set for 'u' and is not cleared after its
constructor returns, and so the check yields a false positive for the
assignment to u.y. That's unfortunate...
RE: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension (CDE): enable the feature
Hi Dennis,
> -Original Message-
> From: Dennis Zhang
> Sent: 19 March 2020 14:03
> To: Kyrylo Tkachov ; gcc-patches@gcc.gnu.org
> Cc: nd ; Richard Earnshaw ;
> Ramana Radhakrishnan
> Subject: Re: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension
> (CDE): enable the feature
>
> Hi Kyrylo,
>
> >
> >From: Kyrylo Tkachov
> >Sent: Wednesday, March 18, 2020 9:04 AM
> >To: Dennis Zhang; gcc-patches@gcc.gnu.org
> >Cc: nd; Richard Earnshaw; Ramana Radhakrishnan
> >Subject: RE: [PATCH][Arm][1/3] Support for Arm Custom Datapath
> >Extension (CDE): enable the feature
> >
> >Hi Dennis,
> >
> >> -Original Message-
> >> From: Dennis Zhang
> >> Sent: 12 March 2020 12:06
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: nd ; Richard Earnshaw ;
> >> Ramana Radhakrishnan ; Kyrylo
> Tkachov
> >>
> >> Subject: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension
> >> (CDE): enable the feature
> >>
> >> Hi all,
> >>
> >> This patch is part of a series that adds support for the ARMv8.m
> >> Custom Datapath Extension.
> >> This patch defines the options cdecp0-cdecp7 for CLI to enable the
> >> CDE on corresponding coprocessor 0-7.
> >> It also adds new check-effective for CDE feature.
> >>
> >> ISA has been announced at
> >> https://developer.arm.com/architectures/instruction-sets/custom-
> >> instructions
> >>
> >> Regtested and bootstrapped.
> >>
> >> Is it OK to commit please?
> >
> >Can you please rebase this patch on top of the recent MVE commits?
> >It currently doesn't apply cleanly to trunk.
> >Thanks,
> >Kyrill
>
> The rebase patches is as attached.
> Is it OK to commit?
Ok, with a few fixes...
diff --git a/gcc/testsuite/gcc.target/arm/pragma_cde.c
b/gcc/testsuite/gcc.target/arm/pragma_cde.c
new file mode 100644
index 000..97643a08405
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pragma_cde.c
@@ -0,0 +1,98 @@
+/* Test for CDE #prama target macros. */
+/* { dg-do compile } */
Typo in "pragma" in the comment.
+# A series of routines are created to 1) check if a given architecture is
+# effective (check_effective_target_*_ok) and then 2) give the corresponding
+# flags that enable the architecture (add_options_for_*).
+# The series includes:
+# arm_v8m_main_cde: Armv8-m CDE (Custom Datapath Extension).
+# arm_v8m_main_cde_fp: Armv8-m CDE with FP registers.
+# arm_v8_1m_main_cde_mve: Armv8.1-m CDE with MVE.
+# Usage:
+# /* { dg-require-effective-target arm_v8m_main_cde_ok } */
+# /* { dg-add-options arm_v8m_main_cde } */
+# The tests are valid for Arm.
+
+foreach { armfunc armflag armdef } {
New effective target checks need to be documented in doc/invoke.texi
Ok with those changes.
Kyrill
>
> Thanks
> Dennis
Re: [PATCH] Fix PR90332 by extending half size vector mode
Hi! On Thu, Mar 19, 2020 at 09:18:06AM +0100, Richard Biener wrote: > On Wed, Mar 18, 2020 at 8:34 PM Segher Boessenkool > wrote: > > We don't have ops on short integer types, either, for similar reasons. > > How do you represent two vector input shuffles? The usual > way is (vec_select (vec_concat ...))) which requires a _larger_ > vector mode for the concat. Which you don't have ops on either. Yes, we have double length modes for this, and it is painful as well. And we also have a few half-length modes. >From rs6000-modes.def: /* VMX/VSX. */ VECTOR_MODES (INT, 16); /* V16QI V8HI V4SI V2DI */ VECTOR_MODE (INT, TI, 1); /* V1TI */ VECTOR_MODES (FLOAT, 16); /* V8HF V4SF V2DF */ /* Two VMX/VSX vectors (for permute, select, concat, etc.) */ VECTOR_MODES (INT, 32); /* V32QI V16HI V8SI V4DI */ VECTOR_MODES (FLOAT, 32); /* V16HF V8SF V4DF */ /* Half VMX/VSX vector (for internal use) */ VECTOR_MODE (FLOAT, SF, 2); /* V2SF */ VECTOR_MODE (INT, SI, 2); /* V2SI */ > It's also not different to those large integer modes you need > but do not have ops on. > > So I think the argument is somewhat moot, but yes. The point is that as soon as you allow some computations in any mode, it snowballs to having to support all computations in those modes, but even more importantly having to do it in movM as well. Not good. Segher
Re: [PATCH v2] generate EH info for volatile asm statements (PR93981)
Hello, On Thu, 19 Mar 2020, J.W. Jagersma via Gcc-patches wrote: > I just realized that changing all outputs to in+out would generate > worse code for *every* single asm that has any outputs. Under -fnon-call-exception only. And I'm not sure what you mean, the only effect of the additional 'in+' part that wasn't there before should be that some instructions setting those operands to values before the asm aren't deleted. If there are none, the code should come out the same. > Whereas changing outputs to a temporary will not generate any extra code > if there is no local try/catch block. I think that is something that > should be considered. But it will also disallow values to be given out of the asm in the exception case. (See my other mail). I think that's more worthwhile than some superflous moves (that, if they turn out to be really useless could be removed after reload). Ciao, Michael.
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On Thu, Mar 19, 2020 at 01:06:35PM -0400, Patrick Palka via Gcc-patches wrote: > On Thu, 19 Mar 2020, Patrick Palka wrote: > > > This patch adds a check to detect changing the active union member during > > initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a > > proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning > > to in > > cxx_eval_store_expression is in the process of being initialized, which > > seems to > > work well. > > If we can't rely on CONSTRUCTOR_NO_CLEARING to be set iff a CONSTRUCTOR > is in the process of being initialized, then here's an alternative patch > for consideration, that detects this UB in an indirect way and after the > fact. Yeah, I'm not sure if that would work well, especially in C++20 where we sometimes don't clear it: /* The result of a constexpr function must be completely initialized. However, in C++20, a constexpr constructor doesn't necessarily have to initialize all the fields, so we don't clear CONSTRUCTOR_NO_CLEARING in order to detect reading an unitialized object in constexpr instead of value-initializing it. (reduced_constant_expression_p is expected to take care of clearing the flag.) */ if (TREE_CODE (result) == CONSTRUCTOR && (cxx_dialect < cxx2a || !DECL_CONSTRUCTOR_P (fun))) clear_no_implicit_zero (result); and rely on reduced_constant_expression_p to clear it. Marek
Re: [PATCH] c++: Reject changing active member of union during initialization [PR94066]
On Thu, 19 Mar 2020, Patrick Palka wrote:
> This patch adds a check to detect changing the active union member during
> initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
> proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to
> in
> cxx_eval_store_expression is in the process of being initialized, which seems
> to
> work well.
If we can't rely on CONSTRUCTOR_NO_CLEARING to be set iff a CONSTRUCTOR
is in the process of being initialized, then here's an alternative patch
for consideration, that detects this UB in an indirect way and after the
fact.
-- >8 --
This patch adds checks to detect whether an active member of a union has been
changed during initialization of the union.
It add checks in three places, one in cxx_eval_bare_aggregate and two in
cxx_eval_store_expression (for preeval and !preeval). These checks are indirect
and only detect this UB after the whole initializer has been evaluated and not
at e.g. the exact assignment in the initializer through which the active union
member gets changed, so the source locations attached to the error messages are
suboptimal.
gcc/cp/ChangeLog:
PR c++/94066
* constexpr.c (cxx_eval_bare_aggregate): Check whether the active
member of the union has been changed during evaluation of its
initializer, and diagnose this if so.
(cxx_eval_store_expression): Likewise for both preeval and !preeval
cases.
gcc/testsuite/ChangeLog:
PR c++/94066
* g++.dg/cpp1y/pr94066.C: New test.
* g++.dg/cpp1y/pr94066-2.C: New test.
* g++.dg/cpp1y/pr94066-3.C: New test.
---
gcc/cp/constexpr.c | 45 +-
gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++
gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 21
gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 +++
4 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 192face9a3a..bcb6b758980 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3784,7 +3784,19 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
}
else
{
- if (new_ctx.ctor != ctx->ctor)
+ if (TREE_CODE (type) == UNION_TYPE
+ && vec_safe_length (*p)
+ && (**p)[0].index != index)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ (**p)[0].index);
+ *non_constant_p = true;
+ }
+ else if (new_ctx.ctor != ctx->ctor)
{
/* We appended this element above; update the value. */
gcc_assert ((*p)->last().index == index);
@@ -4647,6 +4659,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
index);
*non_constant_p = true;
}
+ else if (preeval && TREE_CODE (t) == INIT_EXPR)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ CONSTRUCTOR_ELT (*valp, 0)->index);
+ *non_constant_p = true;
+ }
/* Changing active member. */
vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
no_zero_init = true;
@@ -4761,6 +4783,27 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
if (ctors->is_empty())
/* The hash table might have moved since the get earlier. */
valp = ctx->global->values.get (object);
+ else if (TREE_CODE (t) == INIT_EXPR)
+ {
+ tree ctor = ctors->last();
+ if (TREE_CODE (TREE_TYPE (ctor)) == UNION_TYPE)
+ {
+ tree index = TREE_OPERAND (target, 1);
+ gcc_assert (CONSTRUCTOR_NELTS (ctor));
+ gcc_assert (TREE_CODE (index) == FIELD_DECL
+ && DECL_CONTEXT (index) == TREE_TYPE (ctor));
+ if (CONSTRUCTOR_ELT (ctor, 0)->index != index)
+ {
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ index,
+ CONSTRUCTOR_ELT (ctor, 0)->index);
+ *non_constant_p = true;
Re: [PATCH v2] generate EH info for volatile asm statements (PR93981)
Hello,
On Wed, 18 Mar 2020, J.W. Jagersma via Gcc-patches wrote:
> > Well, it's both: on the exception path the compiler has to assume that the
> > the value wasn't changed (so that former defines are regarded as dead) or
> > that it already has changed (so that the effects the throwing
> > "instruction" had on the result (if any) aren't lost). The easiest for
> > this is to regard the result place as also being an input.
>
> The way I see it, there are two options: either you use the outputs
> when an exception is thrown, or you don't.
Assuming by "use the outputs" you mean "transform them implicitely to
in-out operands", not "the source code uses the output operands after the
asm on except and no-except paths".
> The first option is more or less what my first patch did, but it was
> incomplete. Changing each output to in+out would make that work
> correctly.
Right.
> The second is what I have implemented now, each output is assigned via
> a temporary which is then assigned back to the variable bound to this
> output. On exception, this temporary is discarded. However this is
> not possible for asms that write directly to memory, so those cases are
> treated like option #1.
Right again, somewhat. Except that the determination of which outputs are
going into memory is a fuzzy notion until reload/LRA (which is very late
in the compile pipeline). You can infer some things from the constraint
letters, but gimple might still put things into memory (e.g. when the
output place is address taken), even though the constraint only allows a
register (in which case reloads will be generated), or place something
into a register, even though the constraint only allows memory (again,
reloads will be generated).
Some of these reloads will be done early in the gimple pipeline to help
optimizations (they basically look like the insns that you generate for
copy-out), some of them will be generated only very late.
> I think the second option is easier on optimization since any previous
> assignments can be removed from the normal code path, and registers
> don't need to be loaded with known-valid values before the asm.
True (easier to optimizations) but immaterial (see below).
> The first option is more consistent since all outputs are treated the
> same, but more dangerous, as the asm may write incomplete values before
> throwing.
You have to assume that the author of the asm and its surrounding code is
written by a knowledgable person, so if the asm possibly writes partially
to outputs and then throws, then the output must not be accessed on the
exception path. If the asm does not write partially, then the author can
access it. So, what can or cannot be accessed on the exception path
really is an inherent property of the contents of the asm.
Now, your second case tries to create additional guarantees: it makes it
so that for some operands the user can depend on certain behaviour, namely
that the old value before the asm was entered is available on the except
path. As you can't ensure this for all operands (those in memory are the
problem) you want to tighten the definition to only include the operands
where you can guarantee it, but this is fairly brittle, as above, because
some decisions are taken very late.
There's another case to consider: assume I'm writing an asm that writes
meaningful values to an output before and after a potential exception is
thrown, ala this:
asm (" mov %2, %0
xyz %2, %1
mov $0, %0" : "=r" (status), "+r" (a) : "r" (b));
Assume 'xyz' can fault depending on input. The intention above would be
that on the exception path 'status' would contain a meaningful value (here
the value of input b, and on the non-except path would contain zero.
Your proposal of copyin/out for register values would make the above
impossible. (Basically you wouldn't be able to output any reliable
information out of the asm in the except path).
Given that, and the complication of determining what the safe-for-copy-out
operands really are, and the fact that being easier on optimizations in
connection with asms and -fnon-call-exceptions isn't a high priority it
seems best to opt for the option that is guaranteed to work correctly in
most cases, and in fact allows more freedom in using the asm.
Ciao,
Michael.
Re: [PATCH v2] generate EH info for volatile asm statements (PR93981)
On 2020-03-19 00:56, Segher Boessenkool wrote: > On Tue, Mar 17, 2020 at 03:32:34PM +, Michael Matz wrote: >> On Mon, 16 Mar 2020, Richard Sandiford wrote: >>> Similarly for non-call exceptions on other statements. It sounds like >>> what you're describing requires the corresponding definition to happen >>> for memory outputs regardless of whether the asm throws or not, so that >>> the memory appears to change on both excecution paths. Otherwise, the >>> compiler would be able to assume that the memory operand still has its >>> original value in the exception handler. >> >> Well, it's both: on the exception path the compiler has to assume that the >> the value wasn't changed (so that former defines are regarded as dead) or >> that it already has changed (so that the effects the throwing >> "instruction" had on the result (if any) aren't lost). The easiest for >> this is to regard the result place as also being an input. >> >> (If broadened to all instructions under -fnon-call-exceptions, and not >> just to asms will have quite a bad effect on optimization capabilities, >> but I believe with enough force it's already possible now to construct >> miscompiling testcases with the right mixtures of return types and ABIs) > > It's a tradeoff: do we want this to work for almost no one and get PRs > that we cannot solve, or do we generate slightly worse assembler code > for -fnon-call-exceptions? I don't think this is a difficult decision > to make, considering that you already get pretty bad performance with > that flag (if indeed it works correctly at all). I just realized that changing all outputs to in+out would generate worse code for *every* single asm that has any outputs. Whereas changing outputs to a temporary will not generate any extra code if there is no local try/catch block. I think that is something that should be considered. Then again, if the consensus is that outputs should become in+out, I will implement that. Then there is the issue of those debug stmts being inserted after asms. I can either patch the code that inserts these (in tree-into-ssa.c) as I did earlier, or change the verify routines to ignore debug stmts and the end of a basic block. Which would be preferable?
RE: [PATCH, GCC, Arm]: Fix no_cond issue introduced by MVE
Hi Andre, > -Original Message- > From: Andre Vieira (lists) > Sent: 19 March 2020 16:52 > To: Christophe Lyon ; Kyrylo Tkachov > > Cc: gcc-patches@gcc.gnu.org; Kyrill Tkachov > Subject: [PATCH, GCC, Arm]: Fix no_cond issue introduced by MVE > > Hi, > > This was a matter of mistaken logic in (define_attr "conds" ..). This was > setting the conds attribute for any neon instruction to no_cond which was > messing up code generation. > > Bootsrapped and regression tested arm-linux-gnueabihf. > > Is this OK for trunk? Ok. Thanks, Kyrill > > 2020-03-19 Andre Vieira > > * config/arm/arm.md (define_attr "conds"): Fix logic for neon and mve. > > On 17/03/2020 11:55, Christophe Lyon via Gcc-patches wrote: > > On Mon, 16 Mar 2020 at 18:41, Kyrylo Tkachov > wrote: > >> Hi Srinath, > >> > >> I've pushed the first three of the patches for you to master: > >> [ARM][GCC][3/x]: MVE ACLE intrinsics framework patch. > >> [ARM][GCC][2/x]: MVE ACLE intrinsics framework patch. > >> [ARM][GCC][1/x]: MVE ACLE intrinsics framework patch. > >> > >> For this first one I adjusted the add_options directives for mve to > >> actually > add armv8.1-m.main+mve or armv8.1-m.main+mve.fp to the march line so > that the tests don't appear as UNSUPPORTED when running the testsuite > without any explicit options specified. > >> Please keep an eye out for any fallout in the coming days (though > >> I've smoked tested the patches myself before committing) > >> > > Hi, > > > > I've noticed regressions on arm-none-linux-gnueabihf --with-mode arm > > --with-cpu cortex-a9 --with-fpu neon-fp16 (--with-mode arm is > > important, or force -marm when running the tests) > > FAIL: gcc.target/arm/neon-cond-1.c execution test > > FAIL: gfortran.dg/array_constructor_7.f90 -O3 -g execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O0 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O1 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O2 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -fomit-frame-pointer > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > > test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -g execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -Os execution test > > > > Christophe > > > >> Thanks, > >> Kyrill > >> > >> -Original Message- > >> From: Gcc-patches On Behalf Of > >> Srinath Parvathaneni > >> Sent: 16 March 2020 10:53 > >> To: Kyrill Tkachov ; > >> gcc-patches@gcc.gnu.org > >> Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework > patch. > >> > >> Hi Kyrill, > >> > >>> This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. > >> I have bootstrapped this patch on arm-none-linux-gnueabihf and found > no issues. > >> There is problem with git commit rights, could you commit this patch on > my behalf. > >> > >> Regards > >> SRI > >> > >> From: Kyrill Tkachov > >> Sent: 12 March 2020 11:15 > >> To: Srinath Parvathaneni ; > >> gcc-patches@gcc.gnu.org > >> Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework > patch. > >> > >> Hi Srinath, > >> > >> On 3/10/20 6:19 PM, Srinath Parvathaneni wrote: > >>> Hello Kyrill, > >>> > >>> This patch addresses all the comments in patch version v2. > >>> (version v2) > >>> https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540415.html > >>> > >>> > >>> > >>> Hello, > >>> > >>> This patch creates the required framework for MVE ACLE intrinsics. > >>> > >>> The following changes are done in this patch to support MVE ACLE > >>> intrinsics. > >>> > >>> Header file arm_mve.h is added to source code, which contains the > >>> definitions of MVE ACLE intrinsics and different data types used in > >>> MVE. Machine description file mve.md is also added which contains > >>> the RTL patterns defined for MVE. > >>> > >>> A new reigster "p0" is added which is used in by MVE predicated > >>> patterns. A new register class "VPR_REG" > >>> is added and its contents are defined in REG_CLASS_CONTENTS. > >>> > >>> The vec-common.md file is modified to support the standard move > >>> patterns. The prefix of neon functions which are also used by MVE is > >>> changed from "neon_" to "simd_". > >>> eg: neon_immediate_valid_for_move changed to > >>> simd_immediate_valid_for_move. > >>> > >>> In the patch standard patterns mve_move, mve_store and move_load > for > >>> MVE are added and neon.md and vfp.md files are modified to support > >>> this common patterns. > >>> > >>> Please refer to Arm reference manual [1] for more details. > >>> > >>> [1] https://developer.arm.com/docs/ddi0553/latest > >>> > >>> Regression tested on target arm-none-eabi and armeb-none-eabi and > >>> found no regressions. > >>> > >>> Ok for trunk? > >> > >> This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. > >> > >> Thanks, > >> > >> Kyrill > >> > >> > >>> Thanks, > >>> Srinath > >>> > >>>
[PATCH, GCC, Arm]: Fix no_cond issue introduced by MVE
Hi, This was a matter of mistaken logic in (define_attr "conds" ..). This was setting the conds attribute for any neon instruction to no_cond which was messing up code generation. Bootsrapped and regression tested arm-linux-gnueabihf. Is this OK for trunk? 2020-03-19 Andre Vieira * config/arm/arm.md (define_attr "conds"): Fix logic for neon and mve. On 17/03/2020 11:55, Christophe Lyon via Gcc-patches wrote: On Mon, 16 Mar 2020 at 18:41, Kyrylo Tkachov wrote: Hi Srinath, I've pushed the first three of the patches for you to master: [ARM][GCC][3/x]: MVE ACLE intrinsics framework patch. [ARM][GCC][2/x]: MVE ACLE intrinsics framework patch. [ARM][GCC][1/x]: MVE ACLE intrinsics framework patch. For this first one I adjusted the add_options directives for mve to actually add armv8.1-m.main+mve or armv8.1-m.main+mve.fp to the march line so that the tests don't appear as UNSUPPORTED when running the testsuite without any explicit options specified. Please keep an eye out for any fallout in the coming days (though I've smoked tested the patches myself before committing) Hi, I've noticed regressions on arm-none-linux-gnueabihf --with-mode arm --with-cpu cortex-a9 --with-fpu neon-fp16 (--with-mode arm is important, or force -marm when running the tests) FAIL: gcc.target/arm/neon-cond-1.c execution test FAIL: gfortran.dg/array_constructor_7.f90 -O3 -g execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -O0 execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -O1 execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -O2 execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -g execution test FAIL: gfortran.dg/complex_intrinsic_5.f90 -Os execution test Christophe Thanks, Kyrill -Original Message- From: Gcc-patches On Behalf Of Srinath Parvathaneni Sent: 16 March 2020 10:53 To: Kyrill Tkachov ; gcc-patches@gcc.gnu.org Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework patch. Hi Kyrill, This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. I have bootstrapped this patch on arm-none-linux-gnueabihf and found no issues. There is problem with git commit rights, could you commit this patch on my behalf. Regards SRI From: Kyrill Tkachov Sent: 12 March 2020 11:15 To: Srinath Parvathaneni ; gcc-patches@gcc.gnu.org Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework patch. Hi Srinath, On 3/10/20 6:19 PM, Srinath Parvathaneni wrote: Hello Kyrill, This patch addresses all the comments in patch version v2. (version v2) https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540415.html Hello, This patch creates the required framework for MVE ACLE intrinsics. The following changes are done in this patch to support MVE ACLE intrinsics. Header file arm_mve.h is added to source code, which contains the definitions of MVE ACLE intrinsics and different data types used in MVE. Machine description file mve.md is also added which contains the RTL patterns defined for MVE. A new reigster "p0" is added which is used in by MVE predicated patterns. A new register class "VPR_REG" is added and its contents are defined in REG_CLASS_CONTENTS. The vec-common.md file is modified to support the standard move patterns. The prefix of neon functions which are also used by MVE is changed from "neon_" to "simd_". eg: neon_immediate_valid_for_move changed to simd_immediate_valid_for_move. In the patch standard patterns mve_move, mve_store and move_load for MVE are added and neon.md and vfp.md files are modified to support this common patterns. Please refer to Arm reference manual [1] for more details. [1] https://developer.arm.com/docs/ddi0553/latest Regression tested on target arm-none-eabi and armeb-none-eabi and found no regressions. Ok for trunk? This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. Thanks, Kyrill Thanks, Srinath gcc/ChangeLog: 2020-03-06 Andre Vieira Mihail Ionescu Srinath Parvathaneni * config.gcc (arm_mve.h): Include mve intrinsics header file. * config/arm/aout.h (p0): Add new register name for MVE predicated cases. * config/arm-builtins.c (ARM_BUILTIN_SIMD_LANE_CHECK): Define macro common to Neon and MVE. (ARM_BUILTIN_NEON_LANE_CHECK): Renamed to ARM_BUILTIN_SIMD_LANE_CHECK. (arm_init_simd_builtin_types): Disable poly types for MVE. (arm_init_neon_builtins): Move a check to arm_init_builtins function. (arm_init_builtins): Use ARM_BUILTIN_SIMD_LANE_CHECK instead of ARM_BUILTIN_NEON_LANE_CHECK. (mve_dereference_pointer): Add function. (arm_expand_builtin_args): Call to mve_dereference_pointer when MVE is enabled.
Re: [PATCH] c++: Fix spelling of non-static
On Thu, Mar 19, 2020 at 10:45:01AM -0600, Martin Sebor via Gcc-patches wrote:
> On 3/19/20 9:48 AM, Marek Polacek via Gcc-patches wrote:
> > I was looking at DR 296 and noticed that we say "nonstatic" instead of
> > "non-static", which is the version the standard uses. So this patch
> > fixes the spelling throughout the front end. Did not check e.g.
> > non-dependent or any other.
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> If this is the spelling we want to standardize on, would you mind
> adding a check to -Wformat-diag as well so these misspellings get
> diagnosed in diagnostic messages? We might also want to add
> a mention of it to the Spelling section of the GCC Coding
> Conventions.
Good point, I can definitely add it. For now I'm putting this aside though.
> Alternatively, please try to remember to CC me when you commit
> the patch in stage1 and I'll add that myself.
>
> Thanks
> Martin
>
> PS The hyphenated form makes sense to me when applied to keywords
> or standard terms like non-inline or non-static. I'm not sure it's
> necessary or even widespread in other already established terms
> like nonnegative, and it's even explicitly discouraged in the GCC
> Coding Conventions for nonzero.
>
> >
> > * decl.c (grok_op_properties): Fix spelling of non-static.
> > * typeck.c (build_class_member_access_expr): Likewise.
> >
> > * g++.dg/other/operator1.C: Adjust expected message.
> > * g++.dg/overload/operator2.C: Likewise.
> > * g++.dg/template/error30.C: Likewise.
> > * g++.old-deja/g++.jason/operator.C: Likewise.
> > ---
> > gcc/cp/call.c | 2 +-
> > gcc/cp/class.c | 8
> > gcc/cp/cxx-pretty-print.c | 2 +-
> > gcc/cp/decl.c | 2 +-
> > gcc/cp/init.c | 10 +-
> > gcc/cp/search.c | 6 +++---
> > gcc/cp/typeck.c | 2 +-
> > gcc/testsuite/g++.dg/other/operator1.C | 2 +-
> > gcc/testsuite/g++.dg/overload/operator2.C | 4 ++--
> > gcc/testsuite/g++.dg/template/error30.C | 2 +-
> > gcc/testsuite/g++.old-deja/g++.jason/operator.C | 4 ++--
> > 11 files changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> > index 1715acc0ec3..db396f428a4 100644
> > --- a/gcc/cp/call.c
> > +++ b/gcc/cp/call.c
> > @@ -8671,7 +8671,7 @@ build_over_call (struct z_candidate *cand, int flags,
> > tsubst_flags_t complain)
> > (DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path
> > flags |= LOOKUP_NONVIRTUAL;
> > - /* [class.mfct.nonstatic]: If a nonstatic member function of a class
> > + /* [class.mfct.non-static]: If a non-static member function of a
> > class
> > X is called for an object that is not of type X, or of a type
> > derived from X, the behavior is undefined.
> > diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> > index 5340799fdd3..fb2ef202629 100644
> > --- a/gcc/cp/class.c
> > +++ b/gcc/cp/class.c
> > @@ -3661,7 +3661,7 @@ check_field_decls (tree t, tree *access_decls,
> > {
> > /* ARM $12.6.2: [A member initializer list] (or, for an
> > aggregate, initialization by a brace-enclosed list) is the
> > -only way to initialize nonstatic const and reference
> > +only way to initialize non-static const and reference
> > members. */
> > TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
> > TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
> > @@ -3784,7 +3784,7 @@ check_field_decls (tree t, tree *access_decls,
> > {
> > /* ARM $12.6.2: [A member initializer list] (or, for an
> > aggregate, initialization by a brace-enclosed list) is the
> > -only way to initialize nonstatic const and reference
> > +only way to initialize non-static const and reference
> > members. */
> > TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
> > TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
> > @@ -3799,7 +3799,7 @@ check_field_decls (tree t, tree *access_decls,
> > | CLASSTYPE_READONLY_FIELDS_NEED_INIT (type));
> > }
> > - /* Core issue 80: A nonstatic data member is required to have a
> > + /* Core issue 80: A non-static data member is required to have a
> > different name from the class iff the class has a
> > user-declared constructor. */
> > if (constructor_name_p (DECL_NAME (field), t)
> > @@ -8104,7 +8104,7 @@ resolve_address_of_overloaded_function (tree
> > target_type,
> > member functions match targets of type "pointer-to-member
> > function;" the function type of the pointer to member is used to
> > select the member function from the set of overloaded member
> > - functions. If a nonstatic member function
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On Thu, Mar 19, 2020 at 9:00 AM Martin Liška wrote: > > On 3/19/20 4:50 PM, H.J. Lu wrote: > > I like it and I will take case of binutils side. > > > > Thanks. > > Great. I've just installed the 2 patches to master. > Here is the binutils patch: https://sourceware.org/pipermail/binutils/2020-March/110295.html -- H.J.
Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
On Thu, 19 Mar 2020, Jason Merrill wrote:
> On 3/18/20 3:26 PM, Patrick Palka wrote:
> > + if (map)
> > +{
> > + pp_cxx_whitespace (pp);
> > + pp_cxx_left_bracket (pp);
> > + pp->translate_string ("with");
> > + pp_cxx_whitespace (pp);
> > + pp_cxx_parameter_mapping (pp, map);
> > + pp_cxx_right_bracket (pp);
> > +}
>
> Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping rather
> than duplicate them here.
Like this?
-- >8 --
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern. Move
the "[with ]" bits to here from ...
(pp_cxx_atomic_constraint): ... here.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the dependent form of the constraint
and the
preferably substituted parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 12 ---
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 36 +++--
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14
4 files changed, 33 insertions(+), 30 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..15d8b7eb609 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,9 +2878,13 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+
for (tree p = map; p; p = TREE_CHAIN (p))
{
tree parm = TREE_VALUE (p);
@@ -2903,6 +2907,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree
map)
if (TREE_CHAIN (p) != NULL_TREE)
pp_cxx_separate_with (pp, ';');
}
+
+ pp_cxx_right_bracket (pp);
}
void
@@ -2915,12 +2921,8 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp, tree t)
tree map = ATOMIC_CONSTR_MAP (t);
if (map && map != error_mark_node)
{
- pp_cxx_whitespace (pp);
- pp_cxx_left_bracket (pp);
- pp->translate_string ("with");
pp_cxx_whitespace (pp);
pp_cxx_parameter_mapping (pp, map);
- pp_cxx_right_bracket (pp);
}
}
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..9f19e9bbaa4 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t
loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
-return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
-return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,19 @@ print_concept_check_info (diagnostic_context *context,
tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
-check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n",
check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the satisfaction of %qE", expr);
+ if (map)
+{
+ tree subst_map = tsubst_parameter_mapping (map, args, tf_none,
NULL_TREE);
+ pp_cxx_whitespace (pp);
+
Re: [PATCH 0/6] aarch64: Implement TImode comparisons
On 3/19/20 8:47 AM, Wilco Dijkstra wrote:
> Hi Richard,
>
> Thanks for these patches - yes TI mode expansions can certainly be improved!
> So looking at your expansions for signed compares, why not copy the optimal
> sequence from 32-bit Arm?
>
> Any compare can be done in at most 2 instructions:
>
> void doit(void);
> void f(long long a)
> {
> if (a <= 1)
> doit();
> }
>
> f:
> cmp r0, #2
> sbcsr3, r1, #0
> blt .L4
Well, this one requires that you be able to add 1 to an input and for that
input to not overflow. But you're right that I should be using this sequence
for LT (not LE).
I'll have another look.
r~
Re: [PATCH] c++: Fix spelling of non-static
On 3/19/20 9:48 AM, Marek Polacek via Gcc-patches wrote:
I was looking at DR 296 and noticed that we say "nonstatic" instead of
"non-static", which is the version the standard uses. So this patch
fixes the spelling throughout the front end. Did not check e.g.
non-dependent or any other.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
If this is the spelling we want to standardize on, would you mind
adding a check to -Wformat-diag as well so these misspellings get
diagnosed in diagnostic messages? We might also want to add
a mention of it to the Spelling section of the GCC Coding
Conventions.
Alternatively, please try to remember to CC me when you commit
the patch in stage1 and I'll add that myself.
Thanks
Martin
PS The hyphenated form makes sense to me when applied to keywords
or standard terms like non-inline or non-static. I'm not sure it's
necessary or even widespread in other already established terms
like nonnegative, and it's even explicitly discouraged in the GCC
Coding Conventions for nonzero.
* decl.c (grok_op_properties): Fix spelling of non-static.
* typeck.c (build_class_member_access_expr): Likewise.
* g++.dg/other/operator1.C: Adjust expected message.
* g++.dg/overload/operator2.C: Likewise.
* g++.dg/template/error30.C: Likewise.
* g++.old-deja/g++.jason/operator.C: Likewise.
---
gcc/cp/call.c | 2 +-
gcc/cp/class.c | 8
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/decl.c | 2 +-
gcc/cp/init.c | 10 +-
gcc/cp/search.c | 6 +++---
gcc/cp/typeck.c | 2 +-
gcc/testsuite/g++.dg/other/operator1.C | 2 +-
gcc/testsuite/g++.dg/overload/operator2.C | 4 ++--
gcc/testsuite/g++.dg/template/error30.C | 2 +-
gcc/testsuite/g++.old-deja/g++.jason/operator.C | 4 ++--
11 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1715acc0ec3..db396f428a4 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8671,7 +8671,7 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
(DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path
flags |= LOOKUP_NONVIRTUAL;
- /* [class.mfct.nonstatic]: If a nonstatic member function of a class
+ /* [class.mfct.non-static]: If a non-static member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 5340799fdd3..fb2ef202629 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -3661,7 +3661,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3784,7 +3784,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3799,7 +3799,7 @@ check_field_decls (tree t, tree *access_decls,
| CLASSTYPE_READONLY_FIELDS_NEED_INIT (type));
}
- /* Core issue 80: A nonstatic data member is required to have a
+ /* Core issue 80: A non-static data member is required to have a
different name from the class iff the class has a
user-declared constructor. */
if (constructor_name_p (DECL_NAME (field), t)
@@ -8104,7 +8104,7 @@ resolve_address_of_overloaded_function (tree target_type,
member functions match targets of type "pointer-to-member
function;" the function type of the pointer to member is used to
select the member function from the set of overloaded member
- functions. If a nonstatic member function is selected, the
+ functions. If a non-static member function is selected, the
reference to the overloaded function name is required to have the
form of a pointer to member as described in 5.3.1.
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..05c56775adc 100644
--- a/gcc/cp/cxx-pretty-print.c
+++
Re: [PATCH v2] generate EH info for volatile asm statements (PR93981)
Hello, On Wed, 18 Mar 2020, Segher Boessenkool wrote: > > > Similarly for non-call exceptions on other statements. It sounds like > > > what you're describing requires the corresponding definition to happen > > > for memory outputs regardless of whether the asm throws or not, so that > > > the memory appears to change on both excecution paths. Otherwise, the > > > compiler would be able to assume that the memory operand still has its > > > original value in the exception handler. > > > > Well, it's both: on the exception path the compiler has to assume that the > > the value wasn't changed (so that former defines are regarded as dead) or > > that it already has changed (so that the effects the throwing > > "instruction" had on the result (if any) aren't lost). The easiest for > > this is to regard the result place as also being an input. > > > > (If broadened to all instructions under -fnon-call-exceptions, and not > > just to asms will have quite a bad effect on optimization capabilities, > > but I believe with enough force it's already possible now to construct > > miscompiling testcases with the right mixtures of return types and ABIs) > > It's a tradeoff: do we want this to work for almost no one and get PRs > that we cannot solve, or do we generate slightly worse assembler code > for -fnon-call-exceptions? I don't think this is a difficult decision > to make, considering that you already get pretty bad performance with > that flag (if indeed it works correctly at all). Oh, I wasn't advocating doing anything else than you suggested (i.e. make all operands in-out), I merely pointed out that the inherent problem here is not really specific to asms. Ciao, Michael.
[PATCH] c++: Reject changing active member of union during initialization [PR94066]
This patch adds a check to detect changing the active union member during
initialization of the union. It uses the CONSTRUCTOR_NO_CLEARING flag as a
proxy for whether the non-empty CONSTRUCTOR of UNION_TYPE we're assigning to in
cxx_eval_store_expression is in the process of being initialized, which seems to
work well.
In order for this check to work reliably, we also have to adjust
cxx_eval_bare_aggregate to set the active union member before processing the
initializer.
Does this look OK to commit after testing?
gcc/cp/ChangeLog:
PR c++/94066
* constexpr.c (cxx_eval_bare_aggregate): When constructing a union,
always set the active union member before evaluating the initializer.
Relax assertion that verifies the index of the constructor element we're
initializing hasn't been changed.
(cxx_eval_store_expression): Diagnose changing the active union member
while the union is in the process of being initialized.
gcc/testsuite/ChangeLog:
PR c++/94066
* g++.dg/cpp1y/pr94066.C: New test.
* g++.dg/cpp1y/pr94066-2.C: New test.
* g++.dg/cpp1y/pr94066-3.C: New test.
---
gcc/cp/constexpr.c | 25 -
gcc/testsuite/g++.dg/cpp1y/pr94066-2.C | 19 +++
gcc/testsuite/g++.dg/cpp1y/pr94066-3.C | 18 ++
gcc/testsuite/g++.dg/cpp1y/pr94066.C | 18 ++
4 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr94066.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 192face9a3a..97fe5572f71 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3751,6 +3751,11 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
/* If we built a new CONSTRUCTOR, attach it now so that other
initializers can refer to it. */
CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
+ else if (TREE_CODE (type) == UNION_TYPE)
+ /* If we're constructing a union, set the active union member now so
+ that we can later detect if the initializer attempts to activate
+ another member. */
+ CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
tree elt = cxx_eval_constant_expression (_ctx, value,
lval,
non_constant_p, overflow_p);
@@ -3784,7 +3789,12 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree
t,
}
else
{
- if (new_ctx.ctor != ctx->ctor)
+ if (TREE_CODE (type) == UNION_TYPE
+ && (*p)->last().index != index)
+ /* The initializer may have erroneously changed the active union
+ member we were initializing. */
+ gcc_assert (*non_constant_p);
+ else if (new_ctx.ctor != ctx->ctor)
{
/* We appended this element above; update the value. */
gcc_assert ((*p)->last().index == index);
@@ -4647,6 +4657,19 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
tree t,
index);
*non_constant_p = true;
}
+ else if (TREE_CODE (t) == MODIFY_EXPR
+ && CONSTRUCTOR_NO_CLEARING (*valp))
+ {
+ /* Diagnose changing the active union member while the union
+is in the process of being initialized. */
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_input_loc (t),
+ "change of the active member of a union "
+ "from %qD to %qD during initialization",
+ CONSTRUCTOR_ELT (*valp, 0)->index,
+ index);
+ *non_constant_p = true;
+ }
/* Changing active member. */
vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
no_zero_init = true;
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
new file mode 100644
index 000..1c00b650961
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-2.C
@@ -0,0 +1,19 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr A foo(U *up);
+
+union U {
+ U() = default;
+ A a = foo(this); int y;
+};
+
+constexpr A foo(U *up) {
+ up->y = 11; // { dg-error "'U::a' to 'U::y'" }
+ return {42};
+}
+
+extern constexpr U u = {};
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
new file mode 100644
index 000..6bf1ec81885
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr94066-3.C
@@ -0,0 +1,18 @@
+// PR c++/94066
+// { dg-do compile { target c++14 } }
+
+struct A { long x; };
+
+union U;
+constexpr int
Re: [PATCH] c++: Fix spelling of non-static
On 3/19/20 11:48 AM, Marek Polacek wrote:
I was looking at DR 296 and noticed that we say "nonstatic" instead of
"non-static", which is the version the standard uses. So this patch
fixes the spelling throughout the front end. Did not check e.g.
non-dependent or any other.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
OK for stage 1; at this point I'd prefer not to make unnecessary changes
to diagnostic messages that will break translations in gcc/po/*
* decl.c (grok_op_properties): Fix spelling of non-static.
* typeck.c (build_class_member_access_expr): Likewise.
* g++.dg/other/operator1.C: Adjust expected message.
* g++.dg/overload/operator2.C: Likewise.
* g++.dg/template/error30.C: Likewise.
* g++.old-deja/g++.jason/operator.C: Likewise.
---
gcc/cp/call.c | 2 +-
gcc/cp/class.c | 8
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/decl.c | 2 +-
gcc/cp/init.c | 10 +-
gcc/cp/search.c | 6 +++---
gcc/cp/typeck.c | 2 +-
gcc/testsuite/g++.dg/other/operator1.C | 2 +-
gcc/testsuite/g++.dg/overload/operator2.C | 4 ++--
gcc/testsuite/g++.dg/template/error30.C | 2 +-
gcc/testsuite/g++.old-deja/g++.jason/operator.C | 4 ++--
11 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1715acc0ec3..db396f428a4 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8671,7 +8671,7 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
(DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path
flags |= LOOKUP_NONVIRTUAL;
- /* [class.mfct.nonstatic]: If a nonstatic member function of a class
+ /* [class.mfct.non-static]: If a non-static member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 5340799fdd3..fb2ef202629 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -3661,7 +3661,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3784,7 +3784,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3799,7 +3799,7 @@ check_field_decls (tree t, tree *access_decls,
| CLASSTYPE_READONLY_FIELDS_NEED_INIT (type));
}
- /* Core issue 80: A nonstatic data member is required to have a
+ /* Core issue 80: A non-static data member is required to have a
different name from the class iff the class has a
user-declared constructor. */
if (constructor_name_p (DECL_NAME (field), t)
@@ -8104,7 +8104,7 @@ resolve_address_of_overloaded_function (tree target_type,
member functions match targets of type "pointer-to-member
function;" the function type of the pointer to member is used to
select the member function from the set of overloaded member
- functions. If a nonstatic member function is selected, the
+ functions. If a non-static member function is selected, the
reference to the overloaded function name is required to have the
form of a pointer to member as described in 5.3.1.
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..05c56775adc 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -1532,7 +1532,7 @@ pp_cxx_parameter_declaration_clause (cxx_pretty_printer
*pp, tree t)
}
bool first = true;
- /* Skip artificial parameter for nonstatic member functions. */
+ /* Skip artificial parameter for non-static member functions. */
if (TREE_CODE (t) == METHOD_TYPE)
types = TREE_CHAIN (types);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 319b7ee5c1c..31025cd5edc 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -14388,7 +14388,7 @@ grok_op_properties (tree decl, bool complain)
Re: [stage1][PATCH] Add gcc_assert that _options are not dirty modified.
On 3/19/20 9:32 AM, Martin Liška wrote:
On 3/19/20 10:09 AM, Jakub Jelinek wrote:
I mean, optimize for the !flag_checking case...
Sure, I transformed both situations into heap memory allocation.
In gcc/c-family/c-attribs.c I faced maybe uninitialized warning when
I only assigned (and checked) the variable in flag_checking context.
I was mostly just curious about what was being checked and how so
I might be misunderstanding something but it looks to me like
the code generated by the loop below will be a very long series
(of hundreds?) of if statements, each doing the same thing but
each hardcoding different options names. If that's correct, is
there an easy way to turn that repetitive series into a loop to
keep code (and string) size growth to a minimum?
Also, since the function prints output and the caller then aborts
on failure, would calling internal_error for the first mismatch
instead be more in keeping with how internal errors with additional
output are reported?
One last question/suggestion: if the efficiency of the checking is
at all a concern, would calling memcmp on the arrays first and only
looping to find the position of the mismatch, be viable? (I have no
idea if changes to some of the options are acceptable; if they are
this wouldn't work).
Martin
PS Since the function doesn't modify the option arrays it could
declare them const.
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
print "#endif"
print ""
+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) &&
!defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1,
gcc_options *ptr2)"
+print "{"
+print " bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+ name = var_name(flags[i]);
+ if (name == "")
+ continue;
+
+ if (name in checked_options)
+ continue;
+ checked_options[name]++
+
+ print " if (ptr1->x_" name " != ptr2->x_" name ")"
+ print " {"
+ print "if (result)"
+ print " fprintf (stderr, \"Error: global_options are modified in
local context:\\n\");"
+ print "fprintf (stderr, \" " name " (%ld/%ld)\\n\", (long
int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+ print "result = false;"
+ print " }"
+}
+
+print " return result;"
+print "}"
+print "#endif"
+print "#endif"
+
# All of the optimization switches gathered together so they can be
saved and restored.
# This will allow attribute((cold)) to turn on space optimization.
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On 3/19/20 4:50 PM, H.J. Lu wrote: I like it and I will take case of binutils side. Thanks. Great. I've just installed the 2 patches to master. Martin
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On Thu, Mar 19, 2020 at 8:46 AM Richard Biener
wrote:
>
> On Thu, Mar 19, 2020 at 4:00 PM Martin Liška wrote:
> >
> > On 3/19/20 10:12 AM, Richard Biener wrote:
> > > On Wed, Mar 18, 2020 at 9:52 AM Martin Liška wrote:
> > >>
> > >> On 3/18/20 12:27 AM, Jan Hubicka wrote:
> > Hi.
> >
> > There's updated version of the patch.
> > Changes from the previous version:
> > - comment added to ld_plugin_symbol
> > - new section renamed to ext_symtab
> > - assert added for loop iterations in produce_symtab and
> > produce_symtab_extension
> > >>> Hi,
> > >>> I hope this is last version of the patch.
> > >>
> > >> Hello.
> > >>
> > >> Yes.
> > >>
> >
> > 2020-03-12 Martin Liska
> >
> > * lto-section-in.c: Add extension_symtab.
> > >>> ext_symtab :)
> > >>
> > >> Fixed.
> > >>
> > diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
> > index c17dd69dbdd..78b015be696 100644
> > --- a/gcc/lto-section-in.c
> > +++ b/gcc/lto-section-in.c
> > @@ -54,7 +54,8 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
> > "mode_table",
> > "hsa",
> > "lto",
> > - "ipa_sra"
> > + "ipa_sra",
> > + "ext_symtab"
> > >>> I would move ext_symtab next to symtab so the sections remains at least
> > >>> bit reasonably ordered.
> > >>
> > >> Ok, I'll adjust it and I will send a separate patch where we bump
> > >> LTO_major_version.
> > >>
> >
> > +/* Write extension information for symbols (symbol type, section
> > flags). */
> > +
> > +static void
> > +write_symbol_extension_info (tree t)
> > +{
> > + unsigned char c;
> > >>> Do we still use vertical whitespace after decls per GNU coding style?
> > >>
> > >> Dunno. This seems to me like a nit.
> > >>
> > diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> > index 25bf6c468f7..4f82b439360 100644
> > --- a/gcc/lto-streamer.h
> > +++ b/gcc/lto-streamer.h
> > @@ -236,6 +236,7 @@ enum lto_section_type
> > LTO_section_ipa_hsa,
> > LTO_section_lto,
> > LTO_section_ipa_sra,
> > + LTO_section_symtab_extension,
> > >>> I guess symtab_ext to match the actual section name?
> > >>
> > >> No. See e.g. LTO_section_jump_functions - "jmpfuncs". We want to have
> > >> more descriptive
> > >> enum names.
> > >>
> > LTO_N_SECTION_TYPES /* Must be last. */
> > };
> >
> > diff --git a/include/lto-symtab.h b/include/lto-symtab.h
> > index 0ce0de10121..47f0ff27df8 100644
> > --- a/include/lto-symtab.h
> > +++ b/include/lto-symtab.h
> > @@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
> > GCCPV_HIDDEN
> > };
> >
> > +enum gcc_plugin_symbol_type
> > +{
> > + GCCST_UNKNOWN,
> > + GCCST_FUNCTION,
> > + GCCST_VARIABLE,
> > +};
> > +
> > +enum gcc_plugin_symbol_section_flags
> > +{
> > + GCCSSS_BSS = 1
> > +};
> > >>>
> > >>> Probably comments here?
> > >>
> > >> No. There are just shadow copy of enum types from plugin-api.h which
> > >> are documented.
> > >>
> > +
> > #endif /* GCC_LTO_SYMTAB_H */
> > +/* Parse an entry of the IL symbol table. The data to be parsed is
> > pointed
> > + by P and the result is written in ENTRY. The slot number is stored
> > in SLOT.
> > + Returns the address of the next entry. */
> > +
> > +static char *
> > +parse_table_entry_extension (char *p, struct ld_plugin_symbol *entry)
> > +{
> > + unsigned char t;
> > + enum ld_plugin_symbol_type symbol_types[] =
> > +{
> > + LDST_UNKNOWN,
> > + LDST_FUNCTION,
> > + LDST_VARIABLE,
> > +};
> > +
> > + t = *p;
> > + check (t <= 3, LDPL_FATAL, "invalid symbol type found");
> > + entry->symbol_type = symbol_types[t];
> > + p++;
> > + entry->section_flags = *p;
> > + p++;
> > +
> > + return p;
> > +}
> > >>>
> > >>> I think we have chance to make some plan for future extensions without
> > >>> introducing too many additional sections.
> > >>>
> > >>> Currently there are 2 bytes per entry, while only 3 bits are actively
> > >>> used of them. If we invent next flag to pass we can use unused bits
> > >>> however we need a way to indicate to plugin that the bit is defined.
> > >>> This could be done by a simple version byte at the beggining of
> > >>> ext_symtab section which will be 0 now and once we define extra bits we
> > >>> bump it up to 1.
> > >>
> > >> I like the suggested change, it can help us in the future.
> > >>
> > >>>
> > >>> It is not that important given that even empty file results in 2k LTO
> > >>> object file, but I think it would be nicer in longer run.
> > + /* This is for compatibility with older ABIs. */
> > >>> Perhaps say here
[PATCH] c++: Fix spelling of non-static
I was looking at DR 296 and noticed that we say "nonstatic" instead of
"non-static", which is the version the standard uses. So this patch
fixes the spelling throughout the front end. Did not check e.g.
non-dependent or any other.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
* decl.c (grok_op_properties): Fix spelling of non-static.
* typeck.c (build_class_member_access_expr): Likewise.
* g++.dg/other/operator1.C: Adjust expected message.
* g++.dg/overload/operator2.C: Likewise.
* g++.dg/template/error30.C: Likewise.
* g++.old-deja/g++.jason/operator.C: Likewise.
---
gcc/cp/call.c | 2 +-
gcc/cp/class.c | 8
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/decl.c | 2 +-
gcc/cp/init.c | 10 +-
gcc/cp/search.c | 6 +++---
gcc/cp/typeck.c | 2 +-
gcc/testsuite/g++.dg/other/operator1.C | 2 +-
gcc/testsuite/g++.dg/overload/operator2.C | 4 ++--
gcc/testsuite/g++.dg/template/error30.C | 2 +-
gcc/testsuite/g++.old-deja/g++.jason/operator.C | 4 ++--
11 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1715acc0ec3..db396f428a4 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8671,7 +8671,7 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
(DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path
flags |= LOOKUP_NONVIRTUAL;
- /* [class.mfct.nonstatic]: If a nonstatic member function of a class
+ /* [class.mfct.non-static]: If a non-static member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 5340799fdd3..fb2ef202629 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -3661,7 +3661,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3784,7 +3784,7 @@ check_field_decls (tree t, tree *access_decls,
{
/* ARM $12.6.2: [A member initializer list] (or, for an
aggregate, initialization by a brace-enclosed list) is the
-only way to initialize nonstatic const and reference
+only way to initialize non-static const and reference
members. */
TYPE_HAS_COMPLEX_COPY_ASSIGN (t) = 1;
TYPE_HAS_COMPLEX_MOVE_ASSIGN (t) = 1;
@@ -3799,7 +3799,7 @@ check_field_decls (tree t, tree *access_decls,
| CLASSTYPE_READONLY_FIELDS_NEED_INIT (type));
}
- /* Core issue 80: A nonstatic data member is required to have a
+ /* Core issue 80: A non-static data member is required to have a
different name from the class iff the class has a
user-declared constructor. */
if (constructor_name_p (DECL_NAME (field), t)
@@ -8104,7 +8104,7 @@ resolve_address_of_overloaded_function (tree target_type,
member functions match targets of type "pointer-to-member
function;" the function type of the pointer to member is used to
select the member function from the set of overloaded member
- functions. If a nonstatic member function is selected, the
+ functions. If a non-static member function is selected, the
reference to the overloaded function name is required to have the
form of a pointer to member as described in 5.3.1.
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..05c56775adc 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -1532,7 +1532,7 @@ pp_cxx_parameter_declaration_clause (cxx_pretty_printer
*pp, tree t)
}
bool first = true;
- /* Skip artificial parameter for nonstatic member functions. */
+ /* Skip artificial parameter for non-static member functions. */
if (TREE_CODE (t) == METHOD_TYPE)
types = TREE_CHAIN (types);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 319b7ee5c1c..31025cd5edc 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -14388,7 +14388,7 @@ grok_op_properties (tree decl, bool complain)
|| operator_code == ARRAY_REF
|| operator_code == NOP_EXPR)
{
- error_at (loc, "%qD must be a nonstatic member function", decl);
+ error_at (loc, "%qD must be a non-static member
Re: [stage1][PATCH] Add gcc_assert that _options are not dirty modified.
On Thu, Mar 19, 2020 at 04:32:05PM +0100, Martin Liška wrote:
> + gcc_options *saved_global_options = NULL;
> + if (flag_checking)
> + {
> + saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));
XNEW (gcc_options) please.
> + p->saved_global_options = (gcc_options *) xmalloc (sizeof
> (gcc_options));
Ditto.
> + *p->saved_global_options = global_options;
Jakub
Re: [PATCH 0/6] aarch64: Implement TImode comparisons
Hi Richard,
Thanks for these patches - yes TI mode expansions can certainly be improved!
So looking at your expansions for signed compares, why not copy the optimal
sequence from 32-bit Arm?
Any compare can be done in at most 2 instructions:
void doit(void);
void f(long long a)
{
if (a <= 1)
doit();
}
f:
cmp r0, #2
sbcsr3, r1, #0
blt .L4
bx lr
.L4:
b doit
Cheers,
Wilco
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On Thu, Mar 19, 2020 at 4:00 PM Martin Liška wrote:
>
> On 3/19/20 10:12 AM, Richard Biener wrote:
> > On Wed, Mar 18, 2020 at 9:52 AM Martin Liška wrote:
> >>
> >> On 3/18/20 12:27 AM, Jan Hubicka wrote:
> Hi.
>
> There's updated version of the patch.
> Changes from the previous version:
> - comment added to ld_plugin_symbol
> - new section renamed to ext_symtab
> - assert added for loop iterations in produce_symtab and
> produce_symtab_extension
> >>> Hi,
> >>> I hope this is last version of the patch.
> >>
> >> Hello.
> >>
> >> Yes.
> >>
>
> 2020-03-12 Martin Liska
>
> * lto-section-in.c: Add extension_symtab.
> >>> ext_symtab :)
> >>
> >> Fixed.
> >>
> diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
> index c17dd69dbdd..78b015be696 100644
> --- a/gcc/lto-section-in.c
> +++ b/gcc/lto-section-in.c
> @@ -54,7 +54,8 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
> "mode_table",
> "hsa",
> "lto",
> - "ipa_sra"
> + "ipa_sra",
> + "ext_symtab"
> >>> I would move ext_symtab next to symtab so the sections remains at least
> >>> bit reasonably ordered.
> >>
> >> Ok, I'll adjust it and I will send a separate patch where we bump
> >> LTO_major_version.
> >>
>
> +/* Write extension information for symbols (symbol type, section
> flags). */
> +
> +static void
> +write_symbol_extension_info (tree t)
> +{
> + unsigned char c;
> >>> Do we still use vertical whitespace after decls per GNU coding style?
> >>
> >> Dunno. This seems to me like a nit.
> >>
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 25bf6c468f7..4f82b439360 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -236,6 +236,7 @@ enum lto_section_type
> LTO_section_ipa_hsa,
> LTO_section_lto,
> LTO_section_ipa_sra,
> + LTO_section_symtab_extension,
> >>> I guess symtab_ext to match the actual section name?
> >>
> >> No. See e.g. LTO_section_jump_functions - "jmpfuncs". We want to have
> >> more descriptive
> >> enum names.
> >>
> LTO_N_SECTION_TYPES /* Must be last. */
> };
>
> diff --git a/include/lto-symtab.h b/include/lto-symtab.h
> index 0ce0de10121..47f0ff27df8 100644
> --- a/include/lto-symtab.h
> +++ b/include/lto-symtab.h
> @@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
> GCCPV_HIDDEN
> };
>
> +enum gcc_plugin_symbol_type
> +{
> + GCCST_UNKNOWN,
> + GCCST_FUNCTION,
> + GCCST_VARIABLE,
> +};
> +
> +enum gcc_plugin_symbol_section_flags
> +{
> + GCCSSS_BSS = 1
> +};
> >>>
> >>> Probably comments here?
> >>
> >> No. There are just shadow copy of enum types from plugin-api.h which
> >> are documented.
> >>
> +
> #endif /* GCC_LTO_SYMTAB_H */
> +/* Parse an entry of the IL symbol table. The data to be parsed is
> pointed
> + by P and the result is written in ENTRY. The slot number is stored
> in SLOT.
> + Returns the address of the next entry. */
> +
> +static char *
> +parse_table_entry_extension (char *p, struct ld_plugin_symbol *entry)
> +{
> + unsigned char t;
> + enum ld_plugin_symbol_type symbol_types[] =
> +{
> + LDST_UNKNOWN,
> + LDST_FUNCTION,
> + LDST_VARIABLE,
> +};
> +
> + t = *p;
> + check (t <= 3, LDPL_FATAL, "invalid symbol type found");
> + entry->symbol_type = symbol_types[t];
> + p++;
> + entry->section_flags = *p;
> + p++;
> +
> + return p;
> +}
> >>>
> >>> I think we have chance to make some plan for future extensions without
> >>> introducing too many additional sections.
> >>>
> >>> Currently there are 2 bytes per entry, while only 3 bits are actively
> >>> used of them. If we invent next flag to pass we can use unused bits
> >>> however we need a way to indicate to plugin that the bit is defined.
> >>> This could be done by a simple version byte at the beggining of
> >>> ext_symtab section which will be 0 now and once we define extra bits we
> >>> bump it up to 1.
> >>
> >> I like the suggested change, it can help us in the future.
> >>
> >>>
> >>> It is not that important given that even empty file results in 2k LTO
> >>> object file, but I think it would be nicer in longer run.
> + /* This is for compatibility with older ABIs. */
> >>> Perhaps say here that this ABI defined only "int def;"
> >>
> >> Good point.
> >>
> >>>
> >>> The patch look good to me. Thanks for the work!
> >>
> >> Thanks. I'm sending updated patch that I've just tested on lto.exp and
> >> both binutils master and HJ's branch that utilizes the new API.
> >
> > @@ -495,10 +560,16 @@ write_resolution (void)
> >
> > /*
Re: [stage1][PATCH] Add gcc_assert that _options are not dirty modified.
On 3/19/20 10:09 AM, Jakub Jelinek wrote:
I mean, optimize for the !flag_checking case...
Sure, I transformed both situations into heap memory allocation.
In gcc/c-family/c-attribs.c I faced maybe uninitialized warning when
I only assigned (and checked) the variable in flag_checking context.
Martin
>From a336c110cbefda2a1febddc56e0fd8289bb08c94 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Tue, 10 Dec 2019 19:41:08 +0100
Subject: [PATCH] Add gcc_assert that _options are not dirty modified.
gcc/ChangeLog:
2020-03-17 Martin Liska
PR tree-optimization/92860
* opth-gen.awk: Generate new function gcc_options_check. Include
known exceptions.
gcc/c-family/ChangeLog:
2020-03-17 Martin Liska
PR tree-optimization/92860
* c-attribs.c (handle_optimize_attribute):
Save global options before parsing of an optimization attribute.
* c-pragma.c (opt_stack): Add saved_global_options field.
(handle_pragma_push_options): Save current global_options.
(handle_pragma_pop_options): Check current options.
---
gcc/c-family/c-attribs.c | 12
gcc/c-family/c-pragma.c | 11 +++
gcc/opth-gen.awk | 35 +++
3 files changed, 58 insertions(+)
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..4ac44128407 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4448,6 +4448,13 @@ handle_optimize_attribute (tree *node, tree name, tree args,
/* If we previously had some optimization options, use them as the
default. */
+ gcc_options *saved_global_options = NULL;
+ if (flag_checking)
+ {
+ saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));
+ *saved_global_options = global_options;
+ }
+
if (old_opts)
cl_optimization_restore (_options,
TREE_OPTIMIZATION (old_opts));
@@ -4459,6 +4466,11 @@ handle_optimize_attribute (tree *node, tree name, tree args,
/* Restore current options. */
cl_optimization_restore (_options, _opts);
+ if (saved_global_options != NULL)
+ {
+ gcc_assert (gcc_options_check (saved_global_options, _options));
+ free (saved_global_options);
+ }
}
return NULL_TREE;
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 7c35741745b..94a1c486fc1 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1003,6 +1003,7 @@ struct GTY(()) opt_stack {
tree target_strings;
tree optimize_binary;
tree optimize_strings;
+ gcc_options *saved_global_options;
};
static GTY(()) struct opt_stack * options_stack;
@@ -1028,6 +1029,11 @@ handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
options_stack = p;
/* Save optimization and target flags in binary format. */
+ if (flag_checking)
+{
+ p->saved_global_options = (gcc_options *) xmalloc (sizeof (gcc_options));
+ *p->saved_global_options = global_options;
+}
p->optimize_binary = build_optimization_node (_options);
p->target_binary = build_target_option_node (_options);
@@ -1079,6 +1085,11 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
p->optimize_binary);
optimization_current_node = p->optimize_binary;
}
+ if (flag_checking)
+{
+ gcc_assert (gcc_options_check (p->saved_global_options, _options));
+ free (p->saved_global_options);
+}
current_target_pragma = p->target_strings;
current_optimize_pragma = p->optimize_strings;
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
print "#endif"
print ""
+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1, gcc_options *ptr2)"
+print "{"
+print " bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+ name = var_name(flags[i]);
+ if (name == "")
+ continue;
+
+ if (name in checked_options)
+ continue;
+ checked_options[name]++
+
+ print " if (ptr1->x_" name " != ptr2->x_" name ")"
+ print " {"
+ print "if (result)"
+ print " fprintf (stderr, \"Error: global_options are modified in local context:\\n\");"
+ print "fprintf (stderr, \" " name " (%ld/%ld)\\n\", (long int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+ print "result = false;"
+ print " }"
+}
+
+print " return result;"
+print "}"
+print "#endif"
+print "#endif"
+
# All of the optimization switches gathered together so they can be saved and restored.
# This will allow attribute((cold)) to turn on space optimization.
--
2.25.1
Fix inliner ICE on alias with flatten attribute [PR92372]
Hi,
inliner ICEs upon trying to flatten alias. This patch fixes the ICE
and also adds a warning that flattens on aliases makes no sense.
Testing x86_64-linux in progress, intend to commit it after it finishes
if there are no complains.
gcc/ChangeLog:
2020-03-19 Jan Hubicka
PR ipa/92372
* cgraphunit.c (process_function_and_variable_attributes):
* ipa-inline.c (ipa_inline):
gcc/testsuite/ChangeLog:
2020-03-19 Jan Hubicka
PR ipa/92372
* gcc.c-torture/pr92372.c: New test.
* gcc.dg/attr-flatten-1.c: New test.
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index fd586366bb9..d7ed405bf2c 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -851,6 +851,14 @@ process_function_and_variable_attributes (cgraph_node
*first,
node = symtab->next_function (node))
{
tree decl = node->decl;
+
+ if (node->alias
+ && lookup_attribute ("flatten", DECL_ATTRIBUTES (decl)))
+ {
+ warning_at (DECL_SOURCE_LOCATION (node->decl), OPT_Wattributes,
+ "%"
+ " attribute attribute is ignored on aliases");
+ }
if (DECL_PRESERVE_P (decl))
node->mark_force_output ();
else if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (decl)))
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 6b6ba9aa4b6..302ce16a646 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -2634,6 +2634,9 @@ ipa_inline (void)
{
node = order[i];
if (node->definition
+ /* Do not try to flatten aliases. These may happen for example when
+creating local aliases. */
+ && !node->alias
&& lookup_attribute ("flatten",
DECL_ATTRIBUTES (node->decl)) != NULL)
order[j--] = order[i];
diff --git a/gcc/testsuite/gcc.c-torture/pr92372.c
b/gcc/testsuite/gcc.c-torture/pr92372.c
new file mode 100644
index 000..72a13bb16f4
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/pr92372.c
@@ -0,0 +1,16 @@
+int fn2(int);
+int fn3(int);
+
+__attribute__((flatten))
+int fn1(int p1)
+{
+ int a = fn2(p1);
+ return fn3(a);
+}
+__attribute__((flatten))
+int fn4(int p1)
+{
+ int j = fn2(p1);
+ return fn3(j);
+}
+
diff --git a/gcc/testsuite/gcc.dg/attr-flatten-1.c
b/gcc/testsuite/gcc.dg/attr-flatten-1.c
new file mode 100644
index 000..b2895e1287c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-flatten-1.c
@@ -0,0 +1,18 @@
+/* { dg-require-alias "" } */
+int fn2(int);
+int fn3(int);
+
+__attribute__((flatten))
+int fn1(int p1)
+{
+ int a = fn2(p1);
+ return fn3(a);
+}
+__attribute__((flatten))
+__attribute__((alias("fn1"))) /* { dg-warning "ignored" } */
+int fn4(int p1);
+int
+test ()
+{
+ return fn4(1);
+}
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On 3/19/20 10:12 AM, Richard Biener wrote:
On Wed, Mar 18, 2020 at 9:52 AM Martin Liška wrote:
On 3/18/20 12:27 AM, Jan Hubicka wrote:
Hi.
There's updated version of the patch.
Changes from the previous version:
- comment added to ld_plugin_symbol
- new section renamed to ext_symtab
- assert added for loop iterations in produce_symtab and
produce_symtab_extension
Hi,
I hope this is last version of the patch.
Hello.
Yes.
2020-03-12 Martin Liska
* lto-section-in.c: Add extension_symtab.
ext_symtab :)
Fixed.
diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
index c17dd69dbdd..78b015be696 100644
--- a/gcc/lto-section-in.c
+++ b/gcc/lto-section-in.c
@@ -54,7 +54,8 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
"mode_table",
"hsa",
"lto",
- "ipa_sra"
+ "ipa_sra",
+ "ext_symtab"
I would move ext_symtab next to symtab so the sections remains at least
bit reasonably ordered.
Ok, I'll adjust it and I will send a separate patch where we bump
LTO_major_version.
+/* Write extension information for symbols (symbol type, section flags). */
+
+static void
+write_symbol_extension_info (tree t)
+{
+ unsigned char c;
Do we still use vertical whitespace after decls per GNU coding style?
Dunno. This seems to me like a nit.
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 25bf6c468f7..4f82b439360 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -236,6 +236,7 @@ enum lto_section_type
LTO_section_ipa_hsa,
LTO_section_lto,
LTO_section_ipa_sra,
+ LTO_section_symtab_extension,
I guess symtab_ext to match the actual section name?
No. See e.g. LTO_section_jump_functions - "jmpfuncs". We want to have more
descriptive
enum names.
LTO_N_SECTION_TYPES /* Must be last. */
};
diff --git a/include/lto-symtab.h b/include/lto-symtab.h
index 0ce0de10121..47f0ff27df8 100644
--- a/include/lto-symtab.h
+++ b/include/lto-symtab.h
@@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
GCCPV_HIDDEN
};
+enum gcc_plugin_symbol_type
+{
+ GCCST_UNKNOWN,
+ GCCST_FUNCTION,
+ GCCST_VARIABLE,
+};
+
+enum gcc_plugin_symbol_section_flags
+{
+ GCCSSS_BSS = 1
+};
Probably comments here?
No. There are just shadow copy of enum types from plugin-api.h which
are documented.
+
#endif /* GCC_LTO_SYMTAB_H */
+/* Parse an entry of the IL symbol table. The data to be parsed is pointed
+ by P and the result is written in ENTRY. The slot number is stored in SLOT.
+ Returns the address of the next entry. */
+
+static char *
+parse_table_entry_extension (char *p, struct ld_plugin_symbol *entry)
+{
+ unsigned char t;
+ enum ld_plugin_symbol_type symbol_types[] =
+{
+ LDST_UNKNOWN,
+ LDST_FUNCTION,
+ LDST_VARIABLE,
+};
+
+ t = *p;
+ check (t <= 3, LDPL_FATAL, "invalid symbol type found");
+ entry->symbol_type = symbol_types[t];
+ p++;
+ entry->section_flags = *p;
+ p++;
+
+ return p;
+}
I think we have chance to make some plan for future extensions without
introducing too many additional sections.
Currently there are 2 bytes per entry, while only 3 bits are actively
used of them. If we invent next flag to pass we can use unused bits
however we need a way to indicate to plugin that the bit is defined.
This could be done by a simple version byte at the beggining of
ext_symtab section which will be 0 now and once we define extra bits we
bump it up to 1.
I like the suggested change, it can help us in the future.
It is not that important given that even empty file results in 2k LTO
object file, but I think it would be nicer in longer run.
+ /* This is for compatibility with older ABIs. */
Perhaps say here that this ABI defined only "int def;"
Good point.
The patch look good to me. Thanks for the work!
Thanks. I'm sending updated patch that I've just tested on lto.exp and
both binutils master and HJ's branch that utilizes the new API.
@@ -495,10 +560,16 @@ write_resolution (void)
/* Version 2 of API supports IRONLY_EXP resolution that is
accepted by GCC-4.7 and newer. */
- if (get_symbols_v2)
-get_symbols_v2 (info->handle, symtab->nsyms, syms);
+ if (get_symbols_v4)
+ get_symbols_v4 (info->handle, symtab->nsyms, syms);
else
-get_symbols (info->handle, symtab->nsyms, syms);
+ {
+ clear_new_symtab_flags (symtab);
can you instead just avoid parsing the ext symtab?
Yes, I simplified the changes and I bet we'll only need one new hook
get_symbols_v2.
Then we can base parsing of the external symtab on that.
+ if (get_symbols_v2)
+ get_symbols_v2 (info->handle, symtab->nsyms, syms);
+ else
+ get_symbols (info->handle, symtab->nsyms, syms);
+ }
I guess this also points to the fact that LDs symbol resolution
can't tell GCC it chose "BSS" (from a non-IL object) or that
it chose a variable or function.
@@ -296,6 +300,8 @@ parse_table_entry (char
Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
On 3/18/20 3:26 PM, Patrick Palka wrote:
+ if (map)
+{
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, map);
+ pp_cxx_right_bracket (pp);
+}
Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping
rather than duplicate them here.
Jason
Re: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension (CDE): enable the feature
Hi Kyrylo, > >From: Kyrylo Tkachov >Sent: Wednesday, March 18, 2020 9:04 AM >To: Dennis Zhang; gcc-patches@gcc.gnu.org >Cc: nd; Richard Earnshaw; Ramana Radhakrishnan >Subject: RE: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension >(CDE): enable the feature > >Hi Dennis, > >> -Original Message- >> From: Dennis Zhang >> Sent: 12 March 2020 12:06 >> To: gcc-patches@gcc.gnu.org >> Cc: nd ; Richard Earnshaw ; >> Ramana Radhakrishnan ; Kyrylo Tkachov >> >> Subject: [PATCH][Arm][1/3] Support for Arm Custom Datapath Extension >> (CDE): enable the feature >> >> Hi all, >> >> This patch is part of a series that adds support for the ARMv8.m >> Custom Datapath Extension. >> This patch defines the options cdecp0-cdecp7 for CLI to enable the CDE >> on corresponding coprocessor 0-7. >> It also adds new check-effective for CDE feature. >> >> ISA has been announced at >> https://developer.arm.com/architectures/instruction-sets/custom- >> instructions >> >> Regtested and bootstrapped. >> >> Is it OK to commit please? > >Can you please rebase this patch on top of the recent MVE commits? >It currently doesn't apply cleanly to trunk. >Thanks, >Kyrill The rebase patches is as attached. Is it OK to commit? Thanks Dennis arm-m-cde-cli-20200318.patch Description: arm-m-cde-cli-20200318.patch
Re: [PATCH] c-family: Tighten vector handling in type_for_mode [PR94072]
"H.J. Lu" writes:
> On Thu, Mar 19, 2020 at 4:10 AM Richard Sandiford
> wrote:
>>
>> In this PR we had a 512-bit VECTOR_TYPE whose mode is XImode
>> (an integer mode used for four 128-bit vectors). When trying
>> to expand a zero constant for it, we hit code in expand_expr_real_1
>> that tries to use the associated integer type instead. The code used
>> type_for_mode (XImode, 1) to get this integer type.
>>
>> However, the c-family implementation of type_for_mode checks for
>> any registered built-in type that matches the mode and has the
>> right signedness. This meant that it could return a built-in
>> vector type when given an integer mode (particularly if, as here,
>> the vector type isn't supported by the current subtarget and so
>> TYPE_MODE != TYPE_MODE_RAW). The expand code would then cycle
>> endlessly trying to use this "new" type instead of the original
>> vector type.
>>
>> The search loop is probably too lax in other ways -- e.g. it could
>> return records that just happen to have the right mode -- but this
>> seems like a safe, incremental improvement.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>>
>> Richard
>>
>>
>> 2020-03-18 Richard Sandiford
>>
>> gcc/c-family/
>> PR middle-end/94072
>> * c-common.c (c_common_type_for_mode): Before using a registered
>> built-in type, check that the vectorness of the type matches
>> the vectorness of the mode.
>>
>> gcc/testsuite/
>> PR middle-end/94072
>> * gcc.target/aarch64/pr94072.c: New test.
>> ---
>> gcc/c-family/c-common.c| 11 +++
>> gcc/testsuite/gcc.target/aarch64/pr94072.c | 9 +
>> 2 files changed, 16 insertions(+), 4 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94072.c
>>
>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>> index 25020bf1415..8e5a9243827 100644
>> --- a/gcc/c-family/c-common.c
>> +++ b/gcc/c-family/c-common.c
>> @@ -2387,10 +2387,13 @@ c_common_type_for_mode (machine_mode mode, int
>> unsignedp)
>> }
>>
>>for (t = registered_builtin_types; t; t = TREE_CHAIN (t))
>> -if (TYPE_MODE (TREE_VALUE (t)) == mode
>> - && !!unsignedp == !!TYPE_UNSIGNED (TREE_VALUE (t)))
>> - return TREE_VALUE (t);
>> -
>> +{
>> + tree type = TREE_VALUE (t);
>> + if (TYPE_MODE (type) == mode
>> + && VECTOR_TYPE_P (type) == VECTOR_MODE_P (mode)
>> + && !!unsignedp == !!TYPE_UNSIGNED (type))
>> + return type;
>> +}
>>return NULL_TREE;
>> }
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> b/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> new file mode 100644
>> index 000..2aa72eb7a16
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-options "-msve-vector-bits=512" } */
>> +
>> +#pragma GCC target "+nosve"
>> +
>> +void
>> +foo (void)
>> +{
>> + (int __attribute__ ((__vector_size__ (64{};
>> +}
>
> Shouldn't this test also be enabled for AVX512F?
The test already worked on 512-bit vector targets (including 512-bit SVE).
The problem was when the SVE length was set to 512 bits but SVE itself
wasn't enabled.
There are already more extensive tests for:
int __attribute__ ((__vector_size__ (64)))
and similar vectors in gcc.dg and gcc.c-torture, so I don't think
making this generic would really add much.
Thanks,
Richard
[stage1][PATCH] Provide hint for misspelled -fdump-foo options.
Hi.
The patch is about basic hint support for -fdump-foo options where
one can newly get something like:
$ ./xgcc -B. /tmp/foo.c -fdump-ipa-ynline -c
cc1: error: unrecognized command-line option ‘-fdump-ipa-ynline’; did you mean
‘-fdump-ipa-inline’?
$ ./xgcc -B. /tmp/foo.c -fdump-tree-switchlowe -c
cc1: error: unrecognized command-line option ‘-fdump-tree-switchlowe’; did you
mean ‘-fdump-tree-switchlower1’?
$ ./xgcc -B. /tmp/foo.c -fdump-rtl-sched -c
cc1: error: unrecognized command-line option ‘-fdump-rtl-sched’; did you mean
‘-fdump-rtl-sched1’?
I also considered the same support for --completion option but it's more
problematic
as the --completion is handled in driver. In driver we do not instantiate
pass_manager
where we have listed all passes (and their corresponding dump options).
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed in next stage1?
Thanks,
Martin
gcc/ChangeLog:
2020-03-19 Martin Liska
* dumpfile.c (dump_switch_p): Change return type
and print option suggestion.
* dumpfile.h: Change return type.
* opts-global.c (handle_common_deferred_options):
Move error into dump_switch_p function.
gcc/testsuite/ChangeLog:
2020-03-19 Martin Liska
* gcc.dg/spellcheck-options-22.c: New test.
---
gcc/dumpfile.c | 21 +---
gcc/dumpfile.h | 2 +-
gcc/opts-global.c| 3 +--
gcc/testsuite/gcc.dg/spellcheck-options-22.c | 3 +++
4 files changed, 23 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-22.c
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 468ffab6cce..e820f72edc3 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h" /* for "current_pass". */
#include "optinfo-emit-json.h"
#include "stringpool.h" /* for get_identifier. */
+#include "opts.h"
/* If non-NULL, return one past-the-end of the matching SUBPART of
the WHOLE string. */
@@ -1874,7 +1875,7 @@ dump_switch_p_1 (const char *arg, struct dump_file_info *dfi, bool doglob)
return 1;
}
-int
+void
gcc::dump_manager::
dump_switch_p (const char *arg)
{
@@ -1896,8 +1897,22 @@ dump_switch_p (const char *arg)
for (i = 0; i < m_extra_dump_files_in_use; i++)
any |= dump_switch_p_1 (arg, _extra_dump_files[i], true);
-
- return any;
+ if (!any)
+{
+ char *s;
+ auto_vec candidates;
+ for (size_t i = TDI_none + 1; i != TDI_end; i++)
+ candidates.safe_push (dump_files[i].swtch);
+ for (size_t i = 0; i < m_extra_dump_files_in_use; i++)
+ candidates.safe_push (m_extra_dump_files[i].swtch);
+ const char *hint = candidates_list_and_hint (arg, s, candidates);
+ if (hint)
+ error ("unrecognized command-line option %<-fdump-%s%>; "
+ "did you mean %<-fdump-%s%>?", arg, hint);
+ else
+ error ("unrecognized command-line option %<-fdump-%s%>", arg);
+ XDELETEVEC (s);
+}
}
/* Parse ARG as a -fopt-info switch and store flags, optgroup_flags
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 840ae4d55d5..00e175a4737 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -691,7 +691,7 @@ public:
char *
get_dump_file_name (struct dump_file_info *dfi, int part = -1) const;
- int
+ void
dump_switch_p (const char *arg);
/* Start a dump for PHASE. Store user-supplied dump flags in
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index c658805470e..b1a8429dc3c 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -385,8 +385,7 @@ handle_common_deferred_options (void)
break;
case OPT_fdump_:
- if (!g->get_dumps ()->dump_switch_p (opt->arg))
- error ("unrecognized command-line option %<-fdump-%s%>", opt->arg);
+ g->get_dumps ()->dump_switch_p (opt->arg);
break;
case OPT_fopt_info_:
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-22.c b/gcc/testsuite/gcc.dg/spellcheck-options-22.c
new file mode 100644
index 000..b0ddae2e78e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-22.c
@@ -0,0 +1,3 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-ipa-ynline" } */
+/* { dg-error "unrecognized command-line option '-fdump-ipa-ynline'; did you mean '-fdump-ipa-inline'?" "" { target *-*-* } 0 } */
Re: [PATCH] middle-end/94188 fix fold of addr expression generation
On Thu, 19 Mar 2020 at 13:34, Richard Biener wrote: > > On Thu, 19 Mar 2020, Christophe Lyon wrote: > > > On Wed, 18 Mar 2020 at 20:30, Richard Biener wrote: > > > > > > On March 18, 2020 6:20:29 PM GMT+01:00, Maxim Kuvyrkov > > > wrote: > > > > > > > >> On 17 Mar 2020, at 17:40, Richard Biener wrote: > > > >> > > > >> > > > >> This adds a missing type conversion to build_fold_addr_expr and > > > >adjusts > > > >> fallout - build_fold_addr_expr was used as a convenience to build an > > > >> ADDR_EXPR but some callers do not expect the result to be simplified > > > >> to something else. > > > >> > > > >> Bootstrapped on x86_64-unknown-linux-gnu, testin in progress. > > > >> > > > >> This is the 3rd or 4th attempt and I hope to have catched all fallout > > > > > > > >> with this. I think it's inevitable we fix the mistake in > > > >> build_fold_addr_expr. > > > >> > > > >> Richard. > > > >> > > > >> 2020-03-17 Richard Biener > > > >> > > > >> PR middle-end/94188 > > > >> * fold-const.c (build_fold_addr_expr): Convert address to > > > >> correct type. > > > >> * asan.c (maybe_create_ssa_name): Strip useless type conversions. > > > >> * gimple-fold.c (gimple_fold_stmt_to_constant_1): Use build1 > > > >> to build the ADDR_EXPR which we don't really want to simplify. > > > >> * tree-ssa-dom.c (record_equivalences_from_stmt): Likewise. > > > >> * tree-ssa-loop-im.c (gather_mem_refs_stmt): Likewise. > > > >> * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Likewise. > > > >> (simplify_builtin_call): Strip useless type conversions. > > > >> * tree-ssa-strlen.c (new_strinfo): Likewise. > > > >> > > > >> * gcc.dg/pr94188.c: New testcase. > > > > > > > >Hi Richard, > > > > > > > >This breaks Linux kernel build on 32-bit ARM: > > > > > > > >00:01:29 ./include/linux/string.h:333:9: internal compiler error: in > > > >gen_movsi, at config/arm/arm.md:6291 > > > >00:01:29 make[2]: *** [sound/drivers/serial-u16550.o] Error 1 > > > > > > > >Would you please investigate? Let me know if you need any help > > > >reproducing the problem. > > > > > > Please file a bug report with preprocessed source and instructions how to > > > configure a cross to reproduce this. > > > > > > The change has caused more fallout than I expected... > > > > > > > I think this commit is also causing regressions in gfortran, that's > > probably easier to reproduce for you? > > gfortran.dg/char_expr_3.f90 -O1 (internal compiler error) > > gfortran.dg/char_expr_3.f90 -O2 (internal compiler error) > > gfortran.dg/char_expr_3.f90 -O3 -fomit-frame-pointer > > -funroll-loops -fpeel-loops -ftracer -finline-functions (internal > > compiler error) > > gfortran.dg/char_expr_3.f90 -O3 -g (internal compiler error) > > > > seen on cross arm-none-linux-gnueabi[hf] > > Can't reproduce those on master (which has seen two fixes already). > Ha, it's possible this has been fixed, then. Sorry my validations are bit delayed these days. Christophe > Richard. > > > Christophe > > > > > Thanks, > > > Richard. > > > > > > >Kernel’s build line is (assuming cross-compilation): > > > >make CC=/path/to/arm-linux-gnueabihf-gcc ARCH=arm > > > >CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc allyesconfig > > > > > > > >Regards, > > > > > > > >-- > > > >Maxim Kuvyrkov > > > >https://www.linaro.org > > > > > > > -- > Richard Biener > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] c-family: Tighten vector handling in type_for_mode [PR94072]
On Thu, Mar 19, 2020 at 4:10 AM Richard Sandiford
wrote:
>
> In this PR we had a 512-bit VECTOR_TYPE whose mode is XImode
> (an integer mode used for four 128-bit vectors). When trying
> to expand a zero constant for it, we hit code in expand_expr_real_1
> that tries to use the associated integer type instead. The code used
> type_for_mode (XImode, 1) to get this integer type.
>
> However, the c-family implementation of type_for_mode checks for
> any registered built-in type that matches the mode and has the
> right signedness. This meant that it could return a built-in
> vector type when given an integer mode (particularly if, as here,
> the vector type isn't supported by the current subtarget and so
> TYPE_MODE != TYPE_MODE_RAW). The expand code would then cycle
> endlessly trying to use this "new" type instead of the original
> vector type.
>
> The search loop is probably too lax in other ways -- e.g. it could
> return records that just happen to have the right mode -- but this
> seems like a safe, incremental improvement.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>
> Richard
>
>
> 2020-03-18 Richard Sandiford
>
> gcc/c-family/
> PR middle-end/94072
> * c-common.c (c_common_type_for_mode): Before using a registered
> built-in type, check that the vectorness of the type matches
> the vectorness of the mode.
>
> gcc/testsuite/
> PR middle-end/94072
> * gcc.target/aarch64/pr94072.c: New test.
> ---
> gcc/c-family/c-common.c| 11 +++
> gcc/testsuite/gcc.target/aarch64/pr94072.c | 9 +
> 2 files changed, 16 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94072.c
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 25020bf1415..8e5a9243827 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -2387,10 +2387,13 @@ c_common_type_for_mode (machine_mode mode, int
> unsignedp)
> }
>
>for (t = registered_builtin_types; t; t = TREE_CHAIN (t))
> -if (TYPE_MODE (TREE_VALUE (t)) == mode
> - && !!unsignedp == !!TYPE_UNSIGNED (TREE_VALUE (t)))
> - return TREE_VALUE (t);
> -
> +{
> + tree type = TREE_VALUE (t);
> + if (TYPE_MODE (type) == mode
> + && VECTOR_TYPE_P (type) == VECTOR_MODE_P (mode)
> + && !!unsignedp == !!TYPE_UNSIGNED (type))
> + return type;
> +}
>return NULL_TREE;
> }
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr94072.c
> b/gcc/testsuite/gcc.target/aarch64/pr94072.c
> new file mode 100644
> index 000..2aa72eb7a16
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr94072.c
> @@ -0,0 +1,9 @@
> +/* { dg-options "-msve-vector-bits=512" } */
> +
> +#pragma GCC target "+nosve"
> +
> +void
> +foo (void)
> +{
> + (int __attribute__ ((__vector_size__ (64{};
> +}
Shouldn't this test also be enabled for AVX512F?
--
H.J.
Re: [PATCH] middle-end/94188 fix fold of addr expression generation
On Thu, 19 Mar 2020, Christophe Lyon wrote: > On Wed, 18 Mar 2020 at 20:30, Richard Biener wrote: > > > > On March 18, 2020 6:20:29 PM GMT+01:00, Maxim Kuvyrkov > > wrote: > > > > > >> On 17 Mar 2020, at 17:40, Richard Biener wrote: > > >> > > >> > > >> This adds a missing type conversion to build_fold_addr_expr and > > >adjusts > > >> fallout - build_fold_addr_expr was used as a convenience to build an > > >> ADDR_EXPR but some callers do not expect the result to be simplified > > >> to something else. > > >> > > >> Bootstrapped on x86_64-unknown-linux-gnu, testin in progress. > > >> > > >> This is the 3rd or 4th attempt and I hope to have catched all fallout > > > > > >> with this. I think it's inevitable we fix the mistake in > > >> build_fold_addr_expr. > > >> > > >> Richard. > > >> > > >> 2020-03-17 Richard Biener > > >> > > >> PR middle-end/94188 > > >> * fold-const.c (build_fold_addr_expr): Convert address to > > >> correct type. > > >> * asan.c (maybe_create_ssa_name): Strip useless type conversions. > > >> * gimple-fold.c (gimple_fold_stmt_to_constant_1): Use build1 > > >> to build the ADDR_EXPR which we don't really want to simplify. > > >> * tree-ssa-dom.c (record_equivalences_from_stmt): Likewise. > > >> * tree-ssa-loop-im.c (gather_mem_refs_stmt): Likewise. > > >> * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Likewise. > > >> (simplify_builtin_call): Strip useless type conversions. > > >> * tree-ssa-strlen.c (new_strinfo): Likewise. > > >> > > >> * gcc.dg/pr94188.c: New testcase. > > > > > >Hi Richard, > > > > > >This breaks Linux kernel build on 32-bit ARM: > > > > > >00:01:29 ./include/linux/string.h:333:9: internal compiler error: in > > >gen_movsi, at config/arm/arm.md:6291 > > >00:01:29 make[2]: *** [sound/drivers/serial-u16550.o] Error 1 > > > > > >Would you please investigate? Let me know if you need any help > > >reproducing the problem. > > > > Please file a bug report with preprocessed source and instructions how to > > configure a cross to reproduce this. > > > > The change has caused more fallout than I expected... > > > > I think this commit is also causing regressions in gfortran, that's > probably easier to reproduce for you? > gfortran.dg/char_expr_3.f90 -O1 (internal compiler error) > gfortran.dg/char_expr_3.f90 -O2 (internal compiler error) > gfortran.dg/char_expr_3.f90 -O3 -fomit-frame-pointer > -funroll-loops -fpeel-loops -ftracer -finline-functions (internal > compiler error) > gfortran.dg/char_expr_3.f90 -O3 -g (internal compiler error) > > seen on cross arm-none-linux-gnueabi[hf] Can't reproduce those on master (which has seen two fixes already). Richard. > Christophe > > > Thanks, > > Richard. > > > > >Kernel’s build line is (assuming cross-compilation): > > >make CC=/path/to/arm-linux-gnueabihf-gcc ARCH=arm > > >CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc allyesconfig > > > > > >Regards, > > > > > >-- > > >Maxim Kuvyrkov > > >https://www.linaro.org > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [Patch, comitted] libgomp/testsuite: ignore blank-line output for function-not-offloaded.c
On 19/03/2020 11:47, Tobias Burnus wrote: This error only appears for C++ as the reason seems to be that there are two unresolved symbols: "foo" and "__gxx_personality_v0". Those error messages are separated by an empty line. The blank lines are a feature of using the LLVM linker. GNU bintils doesn't emit blank lines, but LLD does. The GCC testsuite has a test that turns off blank line checking for specific targets (i.e. amdgcn). That test handles both regular and offload configurations. https://gcc.gnu.org/pipermail/gcc-patches/2018-September/506073.html Possibly libgomp could use the same, but if this is the only case then perhaps not. Andrew
Re: [PATCH] middle-end/94188 fix fold of addr expression generation
On Wed, 18 Mar 2020 at 20:30, Richard Biener wrote: > > On March 18, 2020 6:20:29 PM GMT+01:00, Maxim Kuvyrkov > wrote: > > > >> On 17 Mar 2020, at 17:40, Richard Biener wrote: > >> > >> > >> This adds a missing type conversion to build_fold_addr_expr and > >adjusts > >> fallout - build_fold_addr_expr was used as a convenience to build an > >> ADDR_EXPR but some callers do not expect the result to be simplified > >> to something else. > >> > >> Bootstrapped on x86_64-unknown-linux-gnu, testin in progress. > >> > >> This is the 3rd or 4th attempt and I hope to have catched all fallout > > > >> with this. I think it's inevitable we fix the mistake in > >> build_fold_addr_expr. > >> > >> Richard. > >> > >> 2020-03-17 Richard Biener > >> > >> PR middle-end/94188 > >> * fold-const.c (build_fold_addr_expr): Convert address to > >> correct type. > >> * asan.c (maybe_create_ssa_name): Strip useless type conversions. > >> * gimple-fold.c (gimple_fold_stmt_to_constant_1): Use build1 > >> to build the ADDR_EXPR which we don't really want to simplify. > >> * tree-ssa-dom.c (record_equivalences_from_stmt): Likewise. > >> * tree-ssa-loop-im.c (gather_mem_refs_stmt): Likewise. > >> * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Likewise. > >> (simplify_builtin_call): Strip useless type conversions. > >> * tree-ssa-strlen.c (new_strinfo): Likewise. > >> > >> * gcc.dg/pr94188.c: New testcase. > > > >Hi Richard, > > > >This breaks Linux kernel build on 32-bit ARM: > > > >00:01:29 ./include/linux/string.h:333:9: internal compiler error: in > >gen_movsi, at config/arm/arm.md:6291 > >00:01:29 make[2]: *** [sound/drivers/serial-u16550.o] Error 1 > > > >Would you please investigate? Let me know if you need any help > >reproducing the problem. > > Please file a bug report with preprocessed source and instructions how to > configure a cross to reproduce this. > > The change has caused more fallout than I expected... > I think this commit is also causing regressions in gfortran, that's probably easier to reproduce for you? gfortran.dg/char_expr_3.f90 -O1 (internal compiler error) gfortran.dg/char_expr_3.f90 -O2 (internal compiler error) gfortran.dg/char_expr_3.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) gfortran.dg/char_expr_3.f90 -O3 -g (internal compiler error) seen on cross arm-none-linux-gnueabi[hf] Christophe > Thanks, > Richard. > > >Kernel’s build line is (assuming cross-compilation): > >make CC=/path/to/arm-linux-gnueabihf-gcc ARCH=arm > >CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc allyesconfig > > > >Regards, > > > >-- > >Maxim Kuvyrkov > >https://www.linaro.org >
[Patch,comitted] libgomp/testsuite: ignore blank-line output for function-not-offloaded.c
The libgomp.c-c++-common/function-not-offloaded.c is supposed
to fail on "target offload_device_nonshared_as";
producing "unresolved symbol foo" and tons of additional
(fatal) errors by collect, ld, lto1, mkoffload, lto-wrapper etc.
On NVidia, the existing "dg-excess-errors" works, but
with AMDGCN this is insufficient as one still gets a FAIL
"1 blank line(s) in output"
That's solved by adding "dg-allow-blank-lines-in-output 1".
(I think the argument – or at least its value – is ignored
but "1" is used everywhere.)
This error only appears for C++ as the reason seems to be that
there are two unresolved symbols: "foo" and "__gxx_personality_v0".
Those error messages are separated by an empty line.
Hence, this change could be undone as soon as cc1plus and a minimal
libstdc++ (libsupc++) is available for AMDGCN; however, for the
purpose of the test, the "dg-allow-blank-lines-in-output" can
remain even after __gxx_personality_v0 is available.(*)
Committed as r10-7275-gbb83e069ebadf0a724298f80a65b1775eff9cfab
Cheers,
Tobias
(*) The lack of __gxx_personality_v0 shows also up in the following
tests, i.e. that issue is already covered:
libgomp.c++/for-15.C
libgomp.c++/for-24.C
libgomp.oacc-c++/../libgomp.oacc-c-c++-common/routine-1.c
libgomp.oacc-c++/pr71959.C
libgomp.oacc-c++/routine-1-auto.C
libgomp.oacc-c++/routine-1-template-auto.C
libgomp.oacc-c++/routine-1-template-trailing-return-type.C
libgomp.oacc-c++/routine-1-template.C
libgomp.oacc-c++/routine-1-trailing-return-type.C
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
commit bb83e069ebadf0a724298f80a65b1775eff9cfab
Author: Tobias Burnus
Date: Thu Mar 19 11:42:49 2020 +0100
libgomp/testsuite: ignore blank-line output for function-not-offloaded.c
* testsuite/libgomp.c-c++-common/function-not-offloaded.c: Add
dg-allow-blank-lines-in-output.
diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 9a1065fef4e..4d3a4f32ece 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-19 Tobias Burnus
+
+ * testsuite/libgomp.c-c++-common/function-not-offloaded.c: Add
+ dg-allow-blank-lines-in-output.
+
2020-03-18 Julian Brown
Tobias Burnus
diff --git a/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c b/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
index 9e59ef8864e..f01a64e72c0 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
@@ -1,5 +1,6 @@
/* { dg-do link } */
/* { dg-excess-errors "unresolved symbol foo, lto1, mkoffload and lto-wrapper fatal errors" { target offload_device_nonshared_as } } */
+/* { dg-allow-blank-lines-in-output 1 } */
/* { dg-additional-sources "function-not-offloaded-aux.c" } */
#pragma omp declare target
[committed] c++: Fix up handling of captured vars in lambdas in OpenMP clauses [PR93931]
Hi!
Without the parser.c change we were ICEing on the testcase, because while the
uses of the captured vars inside of the constructs were replaced with capture
proxy decls, we didn't do that for decls in OpenMP clauses.
With that fixed, we don't ICE anymore, but the testcase is miscompiled and FAILs
at runtime. This is because the capture proxy decls have DECL_VALUE_EXPR and
during gimplification we were gimplifying those to their DECL_VALUE_EXPRs.
That is fine for shared vars, but for privatized ones we must not do that.
So that is what the cp-gimplify.c changes do. Had to add a DECL_CONTEXT check
before calling is_capture_proxy because some VAR_DECLs don't have DECL_CONTEXT
set (yet) and is_capture_proxy relies on that being non-NULL always.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2020-03-19 Jakub Jelinek
PR c++/93931
* parser.c (cp_parser_omp_var_list_no_open): Call process_outer_var_ref
on outer_automatic_var_p decls.
* cp-gimplify.c (cxx_omp_disregard_value_expr): Return true also for
capture proxy decls.
* testsuite/libgomp.c++/pr93931.C: New test.
--- gcc/cp/parser.c.jj 2020-03-19 10:20:28.621837665 +0100
+++ gcc/cp/parser.c 2020-03-19 11:17:25.204683731 +0100
@@ -34059,6 +34059,8 @@ cp_parser_omp_var_list_no_open (cp_parse
token->location);
}
}
+ if (outer_automatic_var_p (decl))
+ decl = process_outer_var_ref (decl, tf_warning_or_error);
if (decl == error_mark_node)
;
else if (kind != 0)
--- gcc/cp/cp-gimplify.c.jj 2020-03-19 10:20:28.276842716 +0100
+++ gcc/cp/cp-gimplify.c2020-03-19 11:41:01.101925656 +0100
@@ -2260,12 +2260,17 @@ cxx_omp_finish_clause (tree c, gimple_se
bool
cxx_omp_disregard_value_expr (tree decl, bool shared)
{
- return !shared
-&& VAR_P (decl)
-&& DECL_HAS_VALUE_EXPR_P (decl)
-&& DECL_ARTIFICIAL (decl)
-&& DECL_LANG_SPECIFIC (decl)
-&& DECL_OMP_PRIVATIZED_MEMBER (decl);
+ if (shared)
+return false;
+ if (VAR_P (decl)
+ && DECL_HAS_VALUE_EXPR_P (decl)
+ && DECL_ARTIFICIAL (decl)
+ && DECL_LANG_SPECIFIC (decl)
+ && DECL_OMP_PRIVATIZED_MEMBER (decl))
+return true;
+ if (VAR_P (decl) && DECL_CONTEXT (decl) && is_capture_proxy (decl))
+return true;
+ return false;
}
/* Fold expression X which is used as an rvalue if RVAL is true. */
--- libgomp/testsuite/libgomp.c++/pr93931.C.jj 2020-03-19 11:43:51.839420252
+0100
+++ libgomp/testsuite/libgomp.c++/pr93931.C 2020-03-19 11:38:22.097258893
+0100
@@ -0,0 +1,120 @@
+// PR c++/93931
+// { dg-do run }
+// { dg-options "-O2 -std=c++14" }
+
+extern "C" void abort ();
+
+void
+sink (int )
+{
+ int *volatile p;
+ p =
+ (*p)++;
+}
+
+int
+foo ()
+{
+ int r = 0;
+ [] () {
+#pragma omp parallel for reduction(+ : r)
+for (int i = 0; i < 1024; ++i)
+ r += i;
+ } ();
+ return r;
+}
+
+int
+bar ()
+{
+ int l = 0;
+ [] () {
+#pragma omp parallel for lastprivate (l)
+for (int i = 0; i < 1024; ++i)
+ l = i;
+ } ();
+ return l;
+}
+
+void
+baz ()
+{
+ int f = 18;
+ [] () {
+#pragma omp parallel for firstprivate (f)
+for (int i = 0; i < 1024; ++i)
+ {
+ sink (f);
+ f += 3;
+ sink (f);
+ if (f != 23)
+ abort ();
+ sink (f);
+ f -= 7;
+ sink (f);
+ }
+ } ();
+ if (f != 18)
+abort ();
+}
+
+int
+qux ()
+{
+ int r = 0;
+ [&] () {
+#pragma omp parallel for reduction(+ : r)
+for (int i = 0; i < 1024; ++i)
+ r += i;
+ } ();
+ return r;
+}
+
+int
+corge ()
+{
+ int l = 0;
+ [&] () {
+#pragma omp parallel for lastprivate (l)
+for (int i = 0; i < 1024; ++i)
+ l = i;
+ } ();
+ return l;
+}
+
+void
+garply ()
+{
+ int f = 18;
+ [&] () {
+#pragma omp parallel for firstprivate (f)
+for (int i = 0; i < 1024; ++i)
+ {
+ sink (f);
+ f += 3;
+ sink (f);
+ if (f != 23)
+ abort ();
+ sink (f);
+ f -= 7;
+ sink (f);
+ }
+ } ();
+ if (f != 18)
+abort ();
+}
+
+int
+main ()
+{
+ if (foo () != 1024 * 1023 / 2)
+abort ();
+ if (bar () != 1023)
+abort ();
+ baz ();
+ if (qux () != 1024 * 1023 / 2)
+abort ();
+ if (corge () != 1023)
+abort ();
+ garply ();
+}
Jakub
[PATCH] c-family: Tighten vector handling in type_for_mode [PR94072]
In this PR we had a 512-bit VECTOR_TYPE whose mode is XImode
(an integer mode used for four 128-bit vectors). When trying
to expand a zero constant for it, we hit code in expand_expr_real_1
that tries to use the associated integer type instead. The code used
type_for_mode (XImode, 1) to get this integer type.
However, the c-family implementation of type_for_mode checks for
any registered built-in type that matches the mode and has the
right signedness. This meant that it could return a built-in
vector type when given an integer mode (particularly if, as here,
the vector type isn't supported by the current subtarget and so
TYPE_MODE != TYPE_MODE_RAW). The expand code would then cycle
endlessly trying to use this "new" type instead of the original
vector type.
The search loop is probably too lax in other ways -- e.g. it could
return records that just happen to have the right mode -- but this
seems like a safe, incremental improvement.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2020-03-18 Richard Sandiford
gcc/c-family/
PR middle-end/94072
* c-common.c (c_common_type_for_mode): Before using a registered
built-in type, check that the vectorness of the type matches
the vectorness of the mode.
gcc/testsuite/
PR middle-end/94072
* gcc.target/aarch64/pr94072.c: New test.
---
gcc/c-family/c-common.c| 11 +++
gcc/testsuite/gcc.target/aarch64/pr94072.c | 9 +
2 files changed, 16 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94072.c
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 25020bf1415..8e5a9243827 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -2387,10 +2387,13 @@ c_common_type_for_mode (machine_mode mode, int
unsignedp)
}
for (t = registered_builtin_types; t; t = TREE_CHAIN (t))
-if (TYPE_MODE (TREE_VALUE (t)) == mode
- && !!unsignedp == !!TYPE_UNSIGNED (TREE_VALUE (t)))
- return TREE_VALUE (t);
-
+{
+ tree type = TREE_VALUE (t);
+ if (TYPE_MODE (type) == mode
+ && VECTOR_TYPE_P (type) == VECTOR_MODE_P (mode)
+ && !!unsignedp == !!TYPE_UNSIGNED (type))
+ return type;
+}
return NULL_TREE;
}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94072.c
b/gcc/testsuite/gcc.target/aarch64/pr94072.c
new file mode 100644
index 000..2aa72eb7a16
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr94072.c
@@ -0,0 +1,9 @@
+/* { dg-options "-msve-vector-bits=512" } */
+
+#pragma GCC target "+nosve"
+
+void
+foo (void)
+{
+ (int __attribute__ ((__vector_size__ (64{};
+}
Re: [PATCH] phiopt: Avoid -fcompare-debug bug in phiopt [PR94211]
On Thu, 19 Mar 2020, Jakub Jelinek wrote:
> Hi!
>
> Two years ago, I've added support for up to 2 simple preparation statements
> in value_replacement, but the
> - && estimate_num_insns (assign, _time_weights)
> + && estimate_num_insns (bb_seq (middle_bb), _time_weights)
> change, meant that we compute the cost of all those statements rather than
> just the single assign that has been the single supported non-debug
> statement in the bb before, doesn't do what I thought would do, gimple_seq
> is just gimple * and thus it can't be really overloaded depending on whether
> we pass a single gimple * or a whole sequence. Which means in the last
> two years it doesn't count all the statements, but only the first one.
> With -g that happens to be a DEBUG_STMT, or it could be e.g. the first
> preparation statement which could be much cheaper than the actual assign.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
OK.
I wonder if we should make gimple vs gimple_seq type safe via a
struct gimple_seq { gimple *first }; wrapper type ...
Thanks,
Richard.
> 2020-03-19 Jakub Jelinek
>
> PR tree-optimization/94211
> * tree-ssa-phiopt.c (value_replacement): Use estimate_num_insns_seq
> instead of estimate_num_insns for bb_seq (middle_bb). Rename
> emtpy_or_with_defined_p variable to empty_or_with_defined_p, adjust
> all uses.
>
> * gcc.dg/pr94211.c: New test.
>
> --- gcc/tree-ssa-phiopt.c.jj 2020-03-16 23:49:29.853404202 +0100
> +++ gcc/tree-ssa-phiopt.c 2020-03-18 19:42:22.583225152 +0100
> @@ -1056,7 +1056,7 @@ value_replacement (basic_block cond_bb,
>gimple *cond;
>edge true_edge, false_edge;
>enum tree_code code;
> - bool emtpy_or_with_defined_p = true;
> + bool empty_or_with_defined_p = true;
>
>/* If the type says honor signed zeros we cannot do this
> optimization. */
> @@ -1075,7 +1075,7 @@ value_replacement (basic_block cond_bb,
> {
> if (gimple_code (stmt) != GIMPLE_PREDICT
> && gimple_code (stmt) != GIMPLE_NOP)
> - emtpy_or_with_defined_p = false;
> + empty_or_with_defined_p = false;
> continue;
> }
>/* Now try to adjust arg0 or arg1 according to the computation
> @@ -1085,7 +1085,7 @@ value_replacement (basic_block cond_bb,
>&& jump_function_from_stmt (, stmt))
> || (lhs == arg1
> && jump_function_from_stmt (, stmt)))
> - emtpy_or_with_defined_p = false;
> + empty_or_with_defined_p = false;
> }
>
>cond = last_stmt (cond_bb);
> @@ -1137,7 +1137,7 @@ value_replacement (basic_block cond_bb,
>/* If the middle basic block was empty or is defining the
>PHI arguments and this is a single phi where the args are different
>for the edges e0 and e1 then we can remove the middle basic block. */
> - if (emtpy_or_with_defined_p
> + if (empty_or_with_defined_p
> && single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)),
>e0, e1) == phi)
> {
> @@ -1255,7 +1255,7 @@ value_replacement (basic_block cond_bb,
>&& profile_status_for_fn (cfun) != PROFILE_ABSENT
>&& EDGE_PRED (middle_bb, 0)->probability < profile_probability::even ()
>/* If assign is cheap, there is no point avoiding it. */
> - && estimate_num_insns (bb_seq (middle_bb), _time_weights)
> + && estimate_num_insns_seq (bb_seq (middle_bb), _time_weights)
>>= 3 * estimate_num_insns (cond, _time_weights))
> return 0;
>
> --- gcc/testsuite/gcc.dg/pr94211.c.jj 2020-03-18 16:30:34.562427467 +0100
> +++ gcc/testsuite/gcc.dg/pr94211.c2020-03-18 16:30:19.998640206 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/94211 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcompare-debug" } */
> +
> +long
> +foo (long a, long b)
> +{
> + if (__builtin_expect (b == 1, 1))
> +return a;
> + int e = a + 1;
> + return a / b;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH][RFC] API extension for binutils (type of symbols).
On Wed, Mar 18, 2020 at 9:52 AM Martin Liška wrote:
>
> On 3/18/20 12:27 AM, Jan Hubicka wrote:
> >> Hi.
> >>
> >> There's updated version of the patch.
> >> Changes from the previous version:
> >> - comment added to ld_plugin_symbol
> >> - new section renamed to ext_symtab
> >> - assert added for loop iterations in produce_symtab and
> >> produce_symtab_extension
> > Hi,
> > I hope this is last version of the patch.
>
> Hello.
>
> Yes.
>
> >>
> >> 2020-03-12 Martin Liska
> >>
> >> * lto-section-in.c: Add extension_symtab.
> > ext_symtab :)
>
> Fixed.
>
> >> diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
> >> index c17dd69dbdd..78b015be696 100644
> >> --- a/gcc/lto-section-in.c
> >> +++ b/gcc/lto-section-in.c
> >> @@ -54,7 +54,8 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
> >> "mode_table",
> >> "hsa",
> >> "lto",
> >> - "ipa_sra"
> >> + "ipa_sra",
> >> + "ext_symtab"
> > I would move ext_symtab next to symtab so the sections remains at least
> > bit reasonably ordered.
>
> Ok, I'll adjust it and I will send a separate patch where we bump
> LTO_major_version.
>
> >>
> >> +/* Write extension information for symbols (symbol type, section flags).
> >> */
> >> +
> >> +static void
> >> +write_symbol_extension_info (tree t)
> >> +{
> >> + unsigned char c;
> > Do we still use vertical whitespace after decls per GNU coding style?
>
> Dunno. This seems to me like a nit.
>
> >> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> >> index 25bf6c468f7..4f82b439360 100644
> >> --- a/gcc/lto-streamer.h
> >> +++ b/gcc/lto-streamer.h
> >> @@ -236,6 +236,7 @@ enum lto_section_type
> >> LTO_section_ipa_hsa,
> >> LTO_section_lto,
> >> LTO_section_ipa_sra,
> >> + LTO_section_symtab_extension,
> > I guess symtab_ext to match the actual section name?
>
> No. See e.g. LTO_section_jump_functions - "jmpfuncs". We want to have more
> descriptive
> enum names.
>
> >> LTO_N_SECTION_TYPES /* Must be last. */
> >> };
> >>
> >> diff --git a/include/lto-symtab.h b/include/lto-symtab.h
> >> index 0ce0de10121..47f0ff27df8 100644
> >> --- a/include/lto-symtab.h
> >> +++ b/include/lto-symtab.h
> >> @@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
> >> GCCPV_HIDDEN
> >> };
> >>
> >> +enum gcc_plugin_symbol_type
> >> +{
> >> + GCCST_UNKNOWN,
> >> + GCCST_FUNCTION,
> >> + GCCST_VARIABLE,
> >> +};
> >> +
> >> +enum gcc_plugin_symbol_section_flags
> >> +{
> >> + GCCSSS_BSS = 1
> >> +};
> >
> > Probably comments here?
>
> No. There are just shadow copy of enum types from plugin-api.h which
> are documented.
>
> >> +
> >> #endif /* GCC_LTO_SYMTAB_H */
> >> +/* Parse an entry of the IL symbol table. The data to be parsed is pointed
> >> + by P and the result is written in ENTRY. The slot number is stored in
> >> SLOT.
> >> + Returns the address of the next entry. */
> >> +
> >> +static char *
> >> +parse_table_entry_extension (char *p, struct ld_plugin_symbol *entry)
> >> +{
> >> + unsigned char t;
> >> + enum ld_plugin_symbol_type symbol_types[] =
> >> +{
> >> + LDST_UNKNOWN,
> >> + LDST_FUNCTION,
> >> + LDST_VARIABLE,
> >> +};
> >> +
> >> + t = *p;
> >> + check (t <= 3, LDPL_FATAL, "invalid symbol type found");
> >> + entry->symbol_type = symbol_types[t];
> >> + p++;
> >> + entry->section_flags = *p;
> >> + p++;
> >> +
> >> + return p;
> >> +}
> >
> > I think we have chance to make some plan for future extensions without
> > introducing too many additional sections.
> >
> > Currently there are 2 bytes per entry, while only 3 bits are actively
> > used of them. If we invent next flag to pass we can use unused bits
> > however we need a way to indicate to plugin that the bit is defined.
> > This could be done by a simple version byte at the beggining of
> > ext_symtab section which will be 0 now and once we define extra bits we
> > bump it up to 1.
>
> I like the suggested change, it can help us in the future.
>
> >
> > It is not that important given that even empty file results in 2k LTO
> > object file, but I think it would be nicer in longer run.
> >> + /* This is for compatibility with older ABIs. */
> > Perhaps say here that this ABI defined only "int def;"
>
> Good point.
>
> >
> > The patch look good to me. Thanks for the work!
>
> Thanks. I'm sending updated patch that I've just tested on lto.exp and
> both binutils master and HJ's branch that utilizes the new API.
@@ -495,10 +560,16 @@ write_resolution (void)
/* Version 2 of API supports IRONLY_EXP resolution that is
accepted by GCC-4.7 and newer. */
- if (get_symbols_v2)
-get_symbols_v2 (info->handle, symtab->nsyms, syms);
+ if (get_symbols_v4)
+ get_symbols_v4 (info->handle, symtab->nsyms, syms);
else
-get_symbols (info->handle, symtab->nsyms, syms);
+ {
+ clear_new_symtab_flags (symtab);
can you instead just avoid parsing the ext symtab?
+ if
Re: [stage1][PATCH] Add gcc_assert that _options are not dirty modified.
On Thu, Mar 19, 2020 at 09:56:00AM +0100, Martin Liška wrote: > I'm planning to work on the problematic options in next stage1 and this > patch will help me to catch another violations. > > Ready to be installed in next stage1? Isn't that costly even for the !flag_checking case? struct gcc_options is over 5KB now. So even just that gcc_options saved_global_options = global_options; is a fair amount of work. Can't you instead: gcc_options saved_global_options; if (flag_checking) saved_global_options = global_options; ? Or for the opt_stack case have a pointer to the options and XNEW/XDELETE it instead of having it directly in opt_stack? I mean, optimize for the !flag_checking case... Jakub
[PATCH] phiopt: Avoid -fcompare-debug bug in phiopt [PR94211]
Hi!
Two years ago, I've added support for up to 2 simple preparation statements
in value_replacement, but the
- && estimate_num_insns (assign, _time_weights)
+ && estimate_num_insns (bb_seq (middle_bb), _time_weights)
change, meant that we compute the cost of all those statements rather than
just the single assign that has been the single supported non-debug
statement in the bb before, doesn't do what I thought would do, gimple_seq
is just gimple * and thus it can't be really overloaded depending on whether
we pass a single gimple * or a whole sequence. Which means in the last
two years it doesn't count all the statements, but only the first one.
With -g that happens to be a DEBUG_STMT, or it could be e.g. the first
preparation statement which could be much cheaper than the actual assign.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2020-03-19 Jakub Jelinek
PR tree-optimization/94211
* tree-ssa-phiopt.c (value_replacement): Use estimate_num_insns_seq
instead of estimate_num_insns for bb_seq (middle_bb). Rename
emtpy_or_with_defined_p variable to empty_or_with_defined_p, adjust
all uses.
* gcc.dg/pr94211.c: New test.
--- gcc/tree-ssa-phiopt.c.jj2020-03-16 23:49:29.853404202 +0100
+++ gcc/tree-ssa-phiopt.c 2020-03-18 19:42:22.583225152 +0100
@@ -1056,7 +1056,7 @@ value_replacement (basic_block cond_bb,
gimple *cond;
edge true_edge, false_edge;
enum tree_code code;
- bool emtpy_or_with_defined_p = true;
+ bool empty_or_with_defined_p = true;
/* If the type says honor signed zeros we cannot do this
optimization. */
@@ -1075,7 +1075,7 @@ value_replacement (basic_block cond_bb,
{
if (gimple_code (stmt) != GIMPLE_PREDICT
&& gimple_code (stmt) != GIMPLE_NOP)
- emtpy_or_with_defined_p = false;
+ empty_or_with_defined_p = false;
continue;
}
/* Now try to adjust arg0 or arg1 according to the computation
@@ -1085,7 +1085,7 @@ value_replacement (basic_block cond_bb,
&& jump_function_from_stmt (, stmt))
|| (lhs == arg1
&& jump_function_from_stmt (, stmt)))
- emtpy_or_with_defined_p = false;
+ empty_or_with_defined_p = false;
}
cond = last_stmt (cond_bb);
@@ -1137,7 +1137,7 @@ value_replacement (basic_block cond_bb,
/* If the middle basic block was empty or is defining the
PHI arguments and this is a single phi where the args are different
for the edges e0 and e1 then we can remove the middle basic block. */
- if (emtpy_or_with_defined_p
+ if (empty_or_with_defined_p
&& single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)),
e0, e1) == phi)
{
@@ -1255,7 +1255,7 @@ value_replacement (basic_block cond_bb,
&& profile_status_for_fn (cfun) != PROFILE_ABSENT
&& EDGE_PRED (middle_bb, 0)->probability < profile_probability::even ()
/* If assign is cheap, there is no point avoiding it. */
- && estimate_num_insns (bb_seq (middle_bb), _time_weights)
+ && estimate_num_insns_seq (bb_seq (middle_bb), _time_weights)
>= 3 * estimate_num_insns (cond, _time_weights))
return 0;
--- gcc/testsuite/gcc.dg/pr94211.c.jj 2020-03-18 16:30:34.562427467 +0100
+++ gcc/testsuite/gcc.dg/pr94211.c 2020-03-18 16:30:19.998640206 +0100
@@ -0,0 +1,12 @@
+/* PR tree-optimization/94211 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+long
+foo (long a, long b)
+{
+ if (__builtin_expect (b == 1, 1))
+return a;
+ int e = a + 1;
+ return a / b;
+}
Jakub
[stage1][PATCH] Add gcc_assert that _options are not dirty modified.
Hi.
As seen in the mentioned PR we do have issues related to modification
of global_options in context of #pragma GCC optimize/target and
the corresponding function attributes. The patch brings a sanity check
that these context related option modifications should not affect global
state. The patch lists known limitations (mentioned in the PR).
So far I bootstrapped and tested the patch on x86_64-linux-gnu and
ppc64le-linux-gnu.
I'm planning to work on the problematic options in next stage1 and this
patch will help me to catch another violations.
Ready to be installed in next stage1?
Thanks,
Martin
gcc/ChangeLog:
2020-03-17 Martin Liska
PR tree-optimization/92860
* opth-gen.awk: Generate new function gcc_options_check. Include
known exceptions.
gcc/c-family/ChangeLog:
2020-03-17 Martin Liska
PR tree-optimization/92860
* c-attribs.c (handle_optimize_attribute):
Save global options before parsing of an optimization attribute.
* c-pragma.c (opt_stack): Add saved_global_options field.
(handle_pragma_push_options): Save current global_options.
(handle_pragma_pop_options): Check current options.
---
gcc/c-family/c-attribs.c | 3 +++
gcc/c-family/c-pragma.c | 4
gcc/opth-gen.awk | 35 +++
3 files changed, 42 insertions(+)
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..c99b1256186 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4448,6 +4448,7 @@ handle_optimize_attribute (tree *node, tree name, tree args,
/* If we previously had some optimization options, use them as the
default. */
+ gcc_options saved_global_options = global_options;
if (old_opts)
cl_optimization_restore (_options,
TREE_OPTIMIZATION (old_opts));
@@ -4459,6 +4460,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
/* Restore current options. */
cl_optimization_restore (_options, _opts);
+ if (flag_checking)
+ gcc_assert (gcc_options_check (_global_options, _options));
}
return NULL_TREE;
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 7c35741745b..8b3b4f218ba 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1003,6 +1003,7 @@ struct GTY(()) opt_stack {
tree target_strings;
tree optimize_binary;
tree optimize_strings;
+ gcc_options saved_global_options;
};
static GTY(()) struct opt_stack * options_stack;
@@ -1028,6 +1029,7 @@ handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
options_stack = p;
/* Save optimization and target flags in binary format. */
+ p->saved_global_options = global_options;
p->optimize_binary = build_optimization_node (_options);
p->target_binary = build_target_option_node (_options);
@@ -1079,6 +1081,8 @@ handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
p->optimize_binary);
optimization_current_node = p->optimize_binary;
}
+ if (flag_checking)
+gcc_assert (gcc_options_check (>saved_global_options, _options));
current_target_pragma = p->target_strings;
current_optimize_pragma = p->optimize_strings;
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 856a69168a5..586213da3d3 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -119,6 +119,41 @@ print "#endif"
print "#endif"
print ""
+print "#if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)"
+print "#ifndef GENERATOR_FILE"
+print "static inline bool gcc_options_check (gcc_options *ptr1, gcc_options *ptr2)"
+print "{"
+print " bool result = true;"
+
+# all these options are mentioned in PR92860
+checked_options["flag_merge_constants"]++
+checked_options["param_max_fields_for_field_sensitive"]++
+checked_options["flag_omit_frame_pointer"]++
+checked_options["unroll_only_small_loops"]++
+
+for (i = 0; i < n_opts; i++) {
+ name = var_name(flags[i]);
+ if (name == "")
+ continue;
+
+ if (name in checked_options)
+ continue;
+ checked_options[name]++
+
+ print " if (ptr1->x_" name " != ptr2->x_" name ")"
+ print " {"
+ print "if (result)"
+ print " fprintf (stderr, \"Error: global_options are modified in local context:\\n\");"
+ print "fprintf (stderr, \" " name " (%ld/%ld)\\n\", (long int)ptr1->x_" name ", (long int)ptr2->x_" name ");"
+ print "result = false;"
+ print " }"
+}
+
+print " return result;"
+print "}"
+print "#endif"
+print "#endif"
+
# All of the optimization switches gathered together so they can be saved and restored.
# This will allow attribute((cold)) to turn on space optimization.
[PATCH] c: Fix up cfun->function_end_locus from the C FE [PR94029]
Hi!
On the following testcase we ICE because while
DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus
= c_parser_peek_token (parser)->location;
and similarly DECL_SOURCE_LOCATION (fndecl) is set from some token's
location, the end is set as:
/* Store the end of the function, so that we get good line number
info for the epilogue. */
cfun->function_end_locus = input_location;
and the thing is that input_location is only very rarely set in the C FE
(the primary spot that changes it is the cb_line_change/fe_file_change).
Which means, e.g. for pretty much all C functions that are on a single line,
function_start_locus column is > than function_end_locus column, and the
testcase even has smaller line in function_end_locus because cb_line_change
isn't performed while parsing multi-line arguments of a function-like macro.
Attached are two possible fixes to achieve what the C++ FE does, in
particular that cfun->function_end_locus is the locus of the closing } of
the function. The first one updates input_location when we see a closing }
of a compound statement (though any, not just the function body) and thus
input_location in the finish_function call is what we need.
The second instead propagates the location_t from the parsing of the
outermost compound statement (the function body) to finish_function.
Both patches successfully bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk (which one)?
Jakub
2020-03-19 Jakub Jelinek
PR gcov-profile/94029
* c-parser.c (c_parser_compound_statement_nostart): Set
input_location to the locus of closing CPP_CLOSE_BRACE.
* gcc.misc-tests/gcov-pr94029.c: New test.
--- gcc/c/c-parser.c.jj 2020-03-18 12:47:20.749483242 +0100
+++ gcc/c/c-parser.c2020-03-18 13:12:25.182276999 +0100
@@ -5631,7 +5631,9 @@ c_parser_compound_statement_nostart (c_p
location_t label_loc = UNKNOWN_LOCATION; /* Quiet warning. */
if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
{
- add_debug_begin_stmt (c_parser_peek_token (parser)->location);
+ c_token *token = c_parser_peek_token (parser);
+ add_debug_begin_stmt (token->location);
+ c_parser_set_source_position_from_token (token);
c_parser_consume_token (parser);
return;
}
@@ -5804,6 +5806,7 @@ c_parser_compound_statement_nostart (c_p
}
if (last_label)
error_at (label_loc, "label at end of compound statement");
+ c_parser_set_source_position_from_token (c_parser_peek_token (parser));
c_parser_consume_token (parser);
/* Restore the value we started with. */
mark_valid_location_for_stdc_pragma (save_valid_for_pragma);
--- gcc/testsuite/gcc.misc-tests/gcov-pr94029.c.jj 2020-03-18
13:20:10.371407269 +0100
+++ gcc/testsuite/gcc.misc-tests/gcov-pr94029.c 2020-03-18 13:20:46.736870237
+0100
@@ -0,0 +1,14 @@
+/* PR gcov-profile/94029 */
+/* { dg-options "-ftest-coverage" } */
+/* { dg-do compile } */
+
+#define impl_test(name) void test_##name() { }
+impl_test(t1
+) impl_test(t2)
+
+int main()
+{
+ return 0;
+}
+
+/* { dg-final { run-gcov remove-gcda gcov-pr94029.c } } */
2020-03-19 Jakub Jelinek
PR gcov-profile/94029
* c-tree.h (finish_function): Add location_t argument defaulted to
input_location.
* c-parser.c (c_parser_compound_statement): Add endlocp argument and
set it to the locus of closing } if non-NULL.
(c_parser_compound_statement_nostart): Return locus of closing }.
(c_parser_parse_rtl_body): Likewise.
(c_parser_declaration_or_fndef): Propagate locus of closing } to
finish_function.
* c-decl.c (finish_function): Add end_loc argument, use it instead of
input_location to set function_end_locus.
* gcc.misc-tests/gcov-pr94029.c: New test.
--- gcc/c/c-tree.h.jj 2020-03-17 22:31:56.586909614 +0100
+++ gcc/c/c-tree.h 2020-03-18 13:54:56.207538538 +0100
@@ -580,7 +580,7 @@ extern bool c_check_switch_jump_warnings
location_t, location_t);
extern void finish_decl (tree, location_t, tree, tree, tree);
extern tree finish_enum (tree, tree, tree);
-extern void finish_function (void);
+extern void finish_function (location_t = input_location);
extern tree finish_struct (location_t, tree, tree, tree,
class c_struct_parse_info *);
extern tree c_simulate_enum_decl (location_t, const char *,
--- gcc/c/c-parser.c.jj 2020-03-18 13:36:22.006021310 +0100
+++ gcc/c/c-parser.c2020-03-18 13:57:45.576034113 +0100
@@ -1487,8 +1487,8 @@ static struct c_expr c_parser_braced_ini
static void c_parser_initelt (c_parser *, struct obstack *);
static void c_parser_initval (c_parser *, struct c_expr *,
struct obstack *);
-static tree c_parser_compound_statement (c_parser *);
-static void c_parser_compound_statement_nostart (c_parser *);
+static tree c_parser_compound_statement
Re: [PATCH] Fix PR90332 by extending half size vector mode
On Wed, Mar 18, 2020 at 8:34 PM Segher Boessenkool wrote: > > On Wed, Mar 18, 2020 at 10:12:00PM +0800, Kewen.Lin wrote: > > > Btw, why not implement the neccessary vector init patterns? > > > > Power doesn't support 64bit vector size, it looks a bit hacky and > > confusing to introduce this kind of mode just for some optab requirement, > > but I admit the optab hack can immediately make it work. :) > > But it opens up all kinds of other problems. To begin with, how is a > short vector mapped to a "real" vector? > > We don't have ops on short integer types, either, for similar reasons. How do you represent two vector input shuffles? The usual way is (vec_select (vec_concat ...))) which requires a _larger_ vector mode for the concat. Which you don't have ops on either. It's also not different to those large integer modes you need but do not have ops on. So I think the argument is somewhat moot, but yes. Richard. > > Segher
[PATCH 2/2] ipa/94217 simplify offsetted address build
This avoids using build_ref_for_offset and build_fold_addr_expr
where type mixup easily results in something not IP invariant.
Bootstrap and regtest on x86_64-unknown-linux-gnu in progress.
Richard.
2020-03-19 Richard Biener
PR ipa/94217
* ipa-cp.c (ipa_get_jf_ancestor_result): Avoid build_fold_addr_expr
and build_ref_for_offset.
---
gcc/ipa-cp.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 27c020b8199..1c17010e369 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1352,11 +1352,13 @@ ipa_get_jf_ancestor_result (struct ipa_jump_func
*jfunc, tree input)
gcc_checking_assert (TREE_CODE (input) != TREE_BINFO);
if (TREE_CODE (input) == ADDR_EXPR)
{
- tree t = TREE_OPERAND (input, 0);
- t = build_ref_for_offset (EXPR_LOCATION (t), t,
- ipa_get_jf_ancestor_offset (jfunc), false,
- ptr_type_node, NULL, false);
- return build_fold_addr_expr (t);
+ gcc_checking_assert (is_gimple_ip_invariant_address (input));
+ poly_int64 off = ipa_get_jf_ancestor_offset (jfunc);
+ if (known_eq (off, 0))
+ return input;
+ return build1 (ADDR_EXPR, TREE_TYPE (input),
+fold_build2 (MEM_REF, TREE_TYPE (TREE_TYPE (input)),
+ input, build_int_cst (ptr_type_node, off)));
}
else
return NULL_TREE;
--
2.16.4
[PATCH 1/2] middle-end/94216 fix another build_fold_addr_expr use
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2020-03-19 Richard Biener
PR middle-end/94216
* fold-const.c (fold_binary_loc): Avoid using
build_fold_addr_expr when we really want an ADDR_EXPR.
* g++.dg/torture/pr94216.C: New testcase.
---
gcc/fold-const.c | 2 +-
gcc/testsuite/g++.dg/torture/pr94216.C | 45 ++
2 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/torture/pr94216.C
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3ab1a9adcdf..92679142f04 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -10284,7 +10284,7 @@ fold_binary_loc (location_t loc, enum tree_code code,
tree type,
if (!base)
return NULL_TREE;
return fold_build2 (MEM_REF, type,
- build_fold_addr_expr (base),
+ build1 (ADDR_EXPR, TREE_TYPE (arg0), base),
int_const_binop (PLUS_EXPR, arg1,
size_int (coffset)));
}
diff --git a/gcc/testsuite/g++.dg/torture/pr94216.C
b/gcc/testsuite/g++.dg/torture/pr94216.C
new file mode 100644
index 000..e67239de98d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr94216.C
@@ -0,0 +1,45 @@
+// { dg-do compile }
+// { dg-additional-options "-g" }
+
+template struct A { typedef int _Type[_Nm]; };
+template struct B {
+typename A<_Nm>::_Type _M_elems;
+void operator[](int) { int a = *_M_elems; }
+};
+class C {
+struct D {
+ using type = int *;
+};
+
+public:
+using pointer = D::type;
+};
+class F {
+public:
+using pointer = C::pointer;
+F(pointer);
+};
+struct G {
+int data;
+};
+template struct H {
+using dimensions_t = B;
+dimensions_t dimensions;
+G mem;
+};
+template
+H alloc_view(int, DimT, AlignT, Allocator) {
+H b;
+b.dimensions[0];
+return b;
+}
+namespace memory {
+template using DynMdView = H<6>;
+}
+class I {
+I();
+memory::DynMdView m_view;
+F m_memory;
+};
+int c, d, e;
+I::I() : m_view(alloc_view<6>(c, d, e, [] {})), m_memory(_view.mem.data) {}
--
2.16.4
[PATCH 4/6] aarch64: Simplify @ccmp operands
The first two arguments were "reversed", in that operand 0 was not
the output, but the input cc_reg. Remove operand 0 entirely, since
we can get the input cc_reg from within the operand 3 comparison
expression. This moves the output operand to index 0.
* config/aarch64/aarch64.md (@ccmpcc): New expander; remove
operand 0; change operand 3 from match_operator to match_operand.
(*ccmpcc): Rename from @ccmp; swap numbers of operand 0 & 1.
(@ccmp, *ccmp): Likewise.
(@ccmpcc_rev, *ccmpcc_rev): Likewise.
(@ccmp_rev, *ccmp_rev): Likewise.
* config/aarch64/aarch64.c (aarch64_gen_compare_reg): Update to match.
(aarch64_gen_ccmp_next): Likewise.
---
gcc/config/aarch64/aarch64.c | 21 +-
gcc/config/aarch64/aarch64.md | 76 +--
2 files changed, 74 insertions(+), 23 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 619357fa210..16ff40fc267 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2349,7 +2349,7 @@ aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
rtx x_hi = operand_subword (x, 1, 0, TImode);
rtx y_hi = operand_subword (y, 1, 0, TImode);
- emit_insn (gen_ccmpccdi (cc_reg, cc_reg, x_hi, y_hi,
+ emit_insn (gen_ccmpccdi (cc_reg, x_hi, y_hi,
gen_rtx_EQ (cc_mode, cc_reg, const0_rtx),
GEN_INT (AARCH64_EQ)));
}
@@ -20445,7 +20445,7 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn
**gen_seq, rtx prev,
machine_mode op_mode, cmp_mode, cc_mode = CCmode;
int unsignedp = TYPE_UNSIGNED (TREE_TYPE (treeop0));
insn_code icode;
- struct expand_operand ops[6];
+ struct expand_operand ops[5];
int aarch64_cond;
push_to_sequence (*prep_seq);
@@ -20484,8 +20484,8 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn
**gen_seq, rtx prev,
icode = code_for_ccmp (cc_mode, cmp_mode);
- op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
- op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
+ op0 = prepare_operand (icode, op0, 1, op_mode, cmp_mode, unsignedp);
+ op1 = prepare_operand (icode, op1, 2, op_mode, cmp_mode, unsignedp);
if (!op0 || !op1)
{
end_sequence ();
@@ -20517,15 +20517,14 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn
**gen_seq, rtx prev,
aarch64_cond = AARCH64_INVERSE_CONDITION_CODE (aarch64_cond);
}
- create_fixed_operand ([0], XEXP (prev, 0));
- create_fixed_operand ([1], target);
- create_fixed_operand ([2], op0);
- create_fixed_operand ([3], op1);
- create_fixed_operand ([4], prev);
- create_fixed_operand ([5], GEN_INT (aarch64_cond));
+ create_fixed_operand ([0], target);
+ create_fixed_operand ([1], op0);
+ create_fixed_operand ([2], op1);
+ create_fixed_operand ([3], prev);
+ create_fixed_operand ([4], GEN_INT (aarch64_cond));
push_to_sequence (*gen_seq);
- if (!maybe_expand_insn (icode, 6, ops))
+ if (!maybe_expand_insn (icode, 5, ops))
{
end_sequence ();
return NULL_RTX;
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 0fe41117640..12213176103 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -495,11 +495,24 @@
""
"")
-(define_insn "@ccmp"
- [(set (match_operand:CC_ONLY 1 "cc_register" "")
+(define_expand "@ccmp"
+ [(set (match_operand:CC_ONLY 0 "cc_register")
+ (if_then_else:CC_ONLY
+ (match_operand 3 "aarch64_comparison_operator")
+ (compare:CC_ONLY
+ (match_operand:GPI 1 "aarch64_reg_or_zero")
+ (match_operand:GPI 2 "aarch64_ccmp_operand"))
+ (unspec:CC_ONLY
+ [(match_operand 4 "immediate_operand")]
+ UNSPEC_NZCV)))]
+ ""
+)
+
+(define_insn "*ccmp"
+ [(set (match_operand:CC_ONLY 0 "cc_register" "")
(if_then_else:CC_ONLY
(match_operator 4 "aarch64_comparison_operator"
- [(match_operand 0 "cc_register" "")
+ [(match_operand 1 "cc_register" "")
(const_int 0)])
(compare:CC_ONLY
(match_operand:GPI 2 "aarch64_reg_or_zero" "rZ,rZ,rZ")
@@ -515,11 +528,24 @@
[(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
)
-(define_insn "@ccmp"
- [(set (match_operand:CCFP_CCFPE 1 "cc_register" "")
+(define_expand "@ccmp"
+ [(set (match_operand:CCFP_CCFPE 0 "cc_register")
+ (if_then_else:CCFP_CCFPE
+ (match_operand 3 "aarch64_comparison_operator")
+ (compare:CCFP_CCFPE
+ (match_operand:GPF 1 "register_operand")
+ (match_operand:GPF 2 "register_operand"))
+ (unspec:CCFP_CCFPE
+ [(match_operand 4 "immediate_operand")]
+ UNSPEC_NZCV)))]
+ ""
+)
+
+(define_insn "*ccmp"
+ [(set (match_operand:CCFP_CCFPE 0 "cc_register" "")
(if_then_else:CCFP_CCFPE
(match_operator 4 "aarch64_comparison_operator"
-
[PATCH 2/6] aarch64: Adjust result of aarch64_gen_compare_reg
Return the entire comparison expression, not just the cc_reg.
This will allow the routine to adjust the comparison code as
needed for TImode comparisons.
Note that some users were passing e.g. EQ to aarch64_gen_compare_reg
and then using gen_rtx_NE. Pass the proper code in the first place.
* config/aarch64/aarch64.c (aarch64_gen_compare_reg): Return
the final comparison for code & cc_reg.
(aarch64_gen_compare_reg_maybe_ze): Likewise.
(aarch64_expand_compare_and_swap): Update to match -- do not
build the final comparison here, but PUT_MODE as necessary.
(aarch64_split_compare_and_swap): Use prebuilt comparison.
* config/aarch64/aarch64-simd.md (aarch64_cmdi): Likewise.
(aarch64_cmdi): Likewise.
(aarch64_cmtstdi): Likewise.
* config/aarch64/aarch64-speculation.cc
(aarch64_speculation_establish_tracker): Likewise.
* config/aarch64/aarch64.md (cbranch4, cbranch4): Likewise.
(mod3, abs2): Likewise.
(cstore4, cstore4): Likewise.
(cmov6, cmov6): Likewise.
(movcc, movcc, movcc): Likewise.
(cc): Likewise.
(ffs2): Likewise.
(cstorecc4): Remove redundant "".
---
gcc/config/aarch64/aarch64.c | 26 +++---
gcc/config/aarch64/aarch64-simd.md| 18 ++---
gcc/config/aarch64/aarch64-speculation.cc | 5 +-
gcc/config/aarch64/aarch64.md | 96 ++-
4 files changed, 63 insertions(+), 82 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c90de65de12..619357fa210 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2328,7 +2328,7 @@ emit_set_insn (rtx x, rtx y)
}
/* X and Y are two things to compare using CODE. Emit the compare insn and
- return the rtx for register 0 in the proper mode. */
+ return the rtx for the CCmode comparison. */
rtx
aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
{
@@ -2359,7 +2359,7 @@ aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
cc_reg = gen_rtx_REG (cc_mode, CC_REGNUM);
emit_set_insn (cc_reg, gen_rtx_COMPARE (cc_mode, x, y));
}
- return cc_reg;
+ return gen_rtx_fmt_ee (code, VOIDmode, cc_reg, const0_rtx);
}
/* Similarly, but maybe zero-extend Y if Y_MODE < SImode. */
@@ -2382,7 +2382,7 @@ aarch64_gen_compare_reg_maybe_ze (RTX_CODE code, rtx x,
rtx y,
cc_mode = CC_SWPmode;
cc_reg = gen_rtx_REG (cc_mode, CC_REGNUM);
emit_set_insn (cc_reg, t);
- return cc_reg;
+ return gen_rtx_fmt_ee (code, VOIDmode, cc_reg, const0_rtx);
}
}
@@ -18506,7 +18506,8 @@ aarch64_expand_compare_and_swap (rtx operands[])
emit_insn (gen_aarch64_compare_and_swap_lse (mode, rval, mem,
newval, mod_s));
- cc_reg = aarch64_gen_compare_reg_maybe_ze (NE, rval, oldval, mode);
+ x = aarch64_gen_compare_reg_maybe_ze (EQ, rval, oldval, mode);
+ PUT_MODE (x, SImode);
}
else if (TARGET_OUTLINE_ATOMICS)
{
@@ -18517,7 +18518,8 @@ aarch64_expand_compare_and_swap (rtx operands[])
rval = emit_library_call_value (func, NULL_RTX, LCT_NORMAL, r_mode,
oldval, mode, newval, mode,
XEXP (mem, 0), Pmode);
- cc_reg = aarch64_gen_compare_reg_maybe_ze (NE, rval, oldval, mode);
+ x = aarch64_gen_compare_reg_maybe_ze (EQ, rval, oldval, mode);
+ PUT_MODE (x, SImode);
}
else
{
@@ -18529,13 +18531,13 @@ aarch64_expand_compare_and_swap (rtx operands[])
emit_insn (GEN_FCN (code) (rval, mem, oldval, newval,
is_weak, mod_s, mod_f));
cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
+ x = gen_rtx_EQ (SImode, cc_reg, const0_rtx);
}
if (r_mode != mode)
rval = gen_lowpart (mode, rval);
emit_move_insn (operands[1], rval);
- x = gen_rtx_EQ (SImode, cc_reg, const0_rtx);
emit_insn (gen_rtx_SET (bval, x));
}
@@ -18610,10 +18612,8 @@ aarch64_split_compare_and_swap (rtx operands[])
if (strong_zero_p)
x = gen_rtx_NE (VOIDmode, rval, const0_rtx);
else
-{
- rtx cc_reg = aarch64_gen_compare_reg_maybe_ze (NE, rval, oldval, mode);
- x = gen_rtx_NE (VOIDmode, cc_reg, const0_rtx);
-}
+x = aarch64_gen_compare_reg_maybe_ze (NE, rval, oldval, mode);
+
x = gen_rtx_IF_THEN_ELSE (VOIDmode, x,
gen_rtx_LABEL_REF (Pmode, label2), pc_rtx);
aarch64_emit_unlikely_jump (gen_rtx_SET (pc_rtx, x));
@@ -18626,8 +18626,7 @@ aarch64_split_compare_and_swap (rtx operands[])
{
/* Emit an explicit compare instruction, so that we can correctly
track the condition codes. */
- rtx cc_reg = aarch64_gen_compare_reg (NE, scratch, const0_rtx);
- x = gen_rtx_NE (GET_MODE (cc_reg), cc_reg, const0_rtx);
+ x = aarch64_gen_compare_reg (NE,
[PATCH 6/6] aarch64: Implement TImode comparisons
Use ccmp to perform all TImode comparisons branchless.
* config/aarch64/aarch64.c (aarch64_gen_compare_reg): Expand all of
the comparisons for TImode, not just NE.
* config/aarch64/aarch64.md (cbranchti4, cstoreti4): New.
---
gcc/config/aarch64/aarch64.c | 182 +++---
gcc/config/aarch64/aarch64.md | 28 ++
2 files changed, 196 insertions(+), 14 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d7899dad759..911dc1c91cd 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2363,32 +2363,186 @@ rtx
aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
{
machine_mode cmp_mode = GET_MODE (x);
- machine_mode cc_mode;
rtx cc_reg;
if (cmp_mode == TImode)
{
- gcc_assert (code == NE);
-
- cc_mode = CCmode;
- cc_reg = gen_rtx_REG (cc_mode, CC_REGNUM);
-
rtx x_lo = operand_subword (x, 0, 0, TImode);
- rtx y_lo = operand_subword (y, 0, 0, TImode);
- emit_set_insn (cc_reg, gen_rtx_COMPARE (cc_mode, x_lo, y_lo));
-
rtx x_hi = operand_subword (x, 1, 0, TImode);
- rtx y_hi = operand_subword (y, 1, 0, TImode);
- emit_insn (gen_ccmpccdi (cc_reg, x_hi, y_hi,
- gen_rtx_EQ (cc_mode, cc_reg, const0_rtx),
- GEN_INT (aarch64_nzcv_codes[AARCH64_NE])));
+ rtx y_lo, y_hi, tmp;
+
+ if (y == const0_rtx)
+ {
+ y_lo = y_hi = y;
+ switch (code)
+ {
+ case EQ:
+ case NE:
+ /* For equality, IOR the two halves together. If this gets
+used for a branch, we expect this to fold to cbz/cbnz;
+otherwise it's no larger than cmp+ccmp below. Beware of
+the compare-and-swap post-reload split and use cmp+ccmp. */
+ if (!can_create_pseudo_p ())
+ break;
+ tmp = gen_reg_rtx (DImode);
+ emit_insn (gen_iordi3 (tmp, x_hi, x_lo));
+ emit_insn (gen_cmpdi (tmp, const0_rtx));
+ cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
+ goto done;
+
+ case LT:
+ case GE:
+ /* Check only the sign bit. Choose to expose this detail,
+lest something later tries to use a COMPARE in a way
+that doesn't correspond. This is "tst". */
+ cc_reg = gen_rtx_REG (CC_NZmode, CC_REGNUM);
+ tmp = gen_rtx_AND (DImode, x_hi, GEN_INT (INT64_MIN));
+ tmp = gen_rtx_COMPARE (CC_NZmode, tmp, const0_rtx);
+ emit_set_insn (cc_reg, tmp);
+ code = (code == LT ? NE : EQ);
+ goto done;
+
+ case LE:
+ case GT:
+ /* For GT, (x_hi >= 0) && ((x_hi | x_lo) != 0),
+and of course the inverse for LE. */
+ emit_insn (gen_cmpdi (x_hi, const0_rtx));
+
+ tmp = gen_reg_rtx (DImode);
+ emit_insn (gen_iordi3 (tmp, x_hi, x_lo));
+
+ /* Combine the two terms:
+(GE ? (compare tmp 0) : EQ),
+so that the whole term is true for NE, false for EQ. */
+ cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
+ emit_insn (gen_ccmpccdi
+(cc_reg, tmp, const0_rtx,
+ gen_rtx_GE (VOIDmode, cc_reg, const0_rtx),
+ GEN_INT (aarch64_nzcv_codes[AARCH64_EQ])));
+
+ /* The result is entirely within the Z bit. */
+ code = (code == GT ? NE : EQ);
+ goto done;
+
+ default:
+ break;
+ }
+ }
+ else
+ {
+ y_lo = operand_subword (y, 0, 0, TImode);
+ y_hi = operand_subword (y, 1, 0, TImode);
+ }
+
+ cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
+ switch (code)
+ {
+ case EQ:
+ case NE:
+ /* For EQ, (x_lo == y_lo) && (x_hi == y_hi). */
+ emit_insn (gen_cmpdi (x_lo, y_lo));
+ emit_insn (gen_ccmpccdi (cc_reg, x_hi, y_hi,
+ gen_rtx_EQ (VOIDmode, cc_reg, const0_rtx),
+ GEN_INT (aarch64_nzcv_codes[AARCH64_NE])));
+ break;
+
+ case LEU:
+ case GTU:
+ std::swap (x_lo, y_lo);
+ std::swap (x_hi, y_hi);
+ code = swap_condition (code);
+ /* fall through */
+
+ case LTU:
+ case GEU:
+ /* For LTU, (x - y), as double-word arithmetic. */
+ emit_insn (gen_cmpdi (x_lo, y_lo));
+ /* The ucmp*_carryinC pattern uses zero_extend, and so cannot
+take the constant 0 we allow elsewhere. Force to reg now
+and allow combine to eliminate via simplification. */
+ x_hi = force_reg (DImode, x_hi);
+ y_hi = force_reg (DImode, y_hi);
+ emit_insn (gen_ucmpdi3_carryinC(x_hi, y_hi));
+ /* The result is entirely within
[PATCH 5/6] aarch64: Improve nzcv argument to ccmp
Currently we use %k to interpret an aarch64_cond_code value.
This interpretation is done via an array, aarch64_nzcv_codes.
The rtl is neither hindered nor harmed by using the proper
nzcv value itself, so index the array earlier than later.
This makes it easier to compare the rtl to the assembly.
It is slightly confusing in that aarch64_nzcv_codes has
values of nzcv which produce the inverse of the code that
is the index. Invert those values.
* config/aarch64/aarch64.c (AARCH64_CC_{NZCV}): Move up.
(aarch64_nzcv_codes): Move up; reverse values of even/odd entries.
(aarch64_gen_compare_reg): Use aarch64_nzcv_codes in
gen_ccmpccdi generation.
(aarch64_print_operand): Remove case 'k'.
(aarch64_gen_ccmp_next): Invert condition for !AND, remove
inversion for AND; use aarch64_nzcv_codes.
* config/aarch64/aarch64.md (*ccmpcc): Remove %k from
all alternatives.
(*ccmpcc_rev, *ccmp, *ccmp_rev): Likewise.
---
gcc/config/aarch64/aarch64.c | 81 +++
gcc/config/aarch64/aarch64.md | 16 +++
2 files changed, 42 insertions(+), 55 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 16ff40fc267..d7899dad759 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1270,6 +1270,36 @@ aarch64_cc;
#define AARCH64_INVERSE_CONDITION_CODE(X) ((aarch64_cc) (((int) X) ^ 1))
+/* N Z C V. */
+#define AARCH64_CC_V 1
+#define AARCH64_CC_C (1 << 1)
+#define AARCH64_CC_Z (1 << 2)
+#define AARCH64_CC_N (1 << 3)
+
+/*
+ * N Z C V flags for ccmp. Indexed by aarch64_cond_code.
+ * These are the flags to make the given code be *true*.
+ */
+static const int aarch64_nzcv_codes[] =
+{
+ AARCH64_CC_Z,/* EQ, Z == 1. */
+ 0, /* NE, Z == 0. */
+ AARCH64_CC_C,/* CS, C == 1. */
+ 0, /* CC, C == 0. */
+ AARCH64_CC_N,/* MI, N == 1. */
+ 0, /* PL, N == 0. */
+ AARCH64_CC_V,/* VS, V == 1. */
+ 0, /* VC, V == 0. */
+ AARCH64_CC_C,/* HI, C == 1 && Z == 0. */
+ 0, /* LS, !(C == 1 && Z == 0). */
+ 0, /* GE, N == V. */
+ AARCH64_CC_V,/* LT, N != V. */
+ 0, /* GT, Z == 0 && N == V. */
+ AARCH64_CC_V,/* LE, !(Z == 0 && N == V). */
+ 0, /* AL, Any. */
+ 0/* NV, Any. */
+};
+
struct aarch64_branch_protect_type
{
/* The type's name that the user passes to the branch-protection option
@@ -2351,7 +2381,7 @@ aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
rtx y_hi = operand_subword (y, 1, 0, TImode);
emit_insn (gen_ccmpccdi (cc_reg, x_hi, y_hi,
gen_rtx_EQ (cc_mode, cc_reg, const0_rtx),
- GEN_INT (AARCH64_EQ)));
+ GEN_INT (aarch64_nzcv_codes[AARCH64_NE])));
}
else
{
@@ -9302,33 +9332,6 @@ aarch64_const_vec_all_in_range_p (rtx vec,
return true;
}
-/* N Z C V. */
-#define AARCH64_CC_V 1
-#define AARCH64_CC_C (1 << 1)
-#define AARCH64_CC_Z (1 << 2)
-#define AARCH64_CC_N (1 << 3)
-
-/* N Z C V flags for ccmp. Indexed by AARCH64_COND_CODE. */
-static const int aarch64_nzcv_codes[] =
-{
- 0, /* EQ, Z == 1. */
- AARCH64_CC_Z,/* NE, Z == 0. */
- 0, /* CS, C == 1. */
- AARCH64_CC_C,/* CC, C == 0. */
- 0, /* MI, N == 1. */
- AARCH64_CC_N, /* PL, N == 0. */
- 0, /* VS, V == 1. */
- AARCH64_CC_V, /* VC, V == 0. */
- 0, /* HI, C ==1 && Z == 0. */
- AARCH64_CC_C,/* LS, !(C == 1 && Z == 0). */
- AARCH64_CC_V,/* GE, N == V. */
- 0, /* LT, N != V. */
- AARCH64_CC_Z, /* GT, Z == 0 && N == V. */
- 0, /* LE, !(Z == 0 && N == V). */
- 0, /* AL, Any. */
- 0/* NV, Any. */
-};
-
/* Print floating-point vector immediate operand X to F, negating it
first if NEGATE is true. Return true on success, false if it isn't
a constant we can handle. */
@@ -9416,7 +9419,6 @@ sizetochar (int size)
(32-bit or 64-bit).
'0': Print a normal operand, if it's a general register,
then we assume DImode.
- 'k': Print NZCV for conditional compare instructions.
'A': Output address constant representing the first
argument of X, specifying a relocation offset
if appropriate.
@@ -9866,22 +9868,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
output_addr_const (asm_out_file, x);
break;
-case 'k':
- {
- HOST_WIDE_INT cond_code;
-
- if (!CONST_INT_P (x))
- {
- output_operand_lossage ("invalid operand for '%%%c'", code);
- return;
- }
-
- cond_code = INTVAL (x);
- gcc_assert (cond_code >= 0 && cond_code <= AARCH64_NV);
-
[PATCH 1/6] aarch64: Add ucmp_*_carryinC patterns for all usub_*_carryinC
Use xzr for the output when we only require the flags output. This will be used shortly for TImode comparisons. * config/aarch64/aarch64.md (ucmp3_carryinC): New. (*ucmp3_carryinC_z1): New. (*ucmp3_carryinC_z2): New. (*ucmp3_carryinC): New. --- gcc/config/aarch64/aarch64.md | 50 +++ 1 file changed, 50 insertions(+) diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index c7c4d1dd519..fcc1ddafaec 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -3439,6 +3439,18 @@ "" ) +(define_expand "ucmp3_carryinC" + [(set (reg:CC CC_REGNUM) +(compare:CC + (zero_extend: +(match_operand:GPI 0 "register_operand")) + (plus: +(zero_extend: + (match_operand:GPI 1 "register_operand")) +(ltu: (reg:CC CC_REGNUM) (const_int 0)] + "" +) + (define_insn "*usub3_carryinC_z1" [(set (reg:CC CC_REGNUM) (compare:CC @@ -3456,6 +3468,19 @@ [(set_attr "type" "adc_reg")] ) +(define_insn "*ucmp3_carryinC_z1" + [(set (reg:CC CC_REGNUM) + (compare:CC + (const_int 0) + (plus: + (zero_extend: + (match_operand:GPI 0 "register_operand" "r")) + (match_operand: 1 "aarch64_borrow_operation" ""] + "" + "sbcs\\tzr, zr, %0" + [(set_attr "type" "adc_reg")] +) + (define_insn "*usub3_carryinC_z2" [(set (reg:CC CC_REGNUM) (compare:CC @@ -3471,6 +3496,17 @@ [(set_attr "type" "adc_reg")] ) +(define_insn "*ucmp3_carryinC_z2" + [(set (reg:CC CC_REGNUM) + (compare:CC + (zero_extend: + (match_operand:GPI 0 "register_operand" "r")) + (match_operand: 1 "aarch64_borrow_operation" "")))] + "" + "sbcs\\tzr, %0, zr" + [(set_attr "type" "adc_reg")] +) + (define_insn "*usub3_carryinC" [(set (reg:CC CC_REGNUM) (compare:CC @@ -3489,6 +3525,20 @@ [(set_attr "type" "adc_reg")] ) +(define_insn "*ucmp3_carryinC" + [(set (reg:CC CC_REGNUM) + (compare:CC + (zero_extend: + (match_operand:GPI 0 "register_operand" "r")) + (plus: + (zero_extend: + (match_operand:GPI 1 "register_operand" "r")) + (match_operand: 2 "aarch64_borrow_operation" ""] + "" + "sbcs\\tzr, %0, %1" + [(set_attr "type" "adc_reg")] +) + (define_expand "sub3_carryinV" [(parallel [(set (reg:CC_V CC_REGNUM) -- 2.20.1
[PATCH 3/6] aarch64: Accept 0 as first argument to compares
While cmp (extended register) and cmp (immediate) uses , cmp (shifted register) uses . So we can perform cmp xzr, x0. For ccmp, we only have as an input. * config/aarch64/aarch64.md (cmp): For operand 0, use aarch64_reg_or_zero. Shuffle reg/reg to last alternative and accept Z. (@ccmpcc): For operand 0, use aarch64_reg_or_zero and Z. (@ccmpcc_rev): Likewise. --- gcc/config/aarch64/aarch64.md | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 29dfd6df30c..0fe41117640 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -502,7 +502,7 @@ [(match_operand 0 "cc_register" "") (const_int 0)]) (compare:CC_ONLY - (match_operand:GPI 2 "register_operand" "r,r,r") + (match_operand:GPI 2 "aarch64_reg_or_zero" "rZ,rZ,rZ") (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")) (unspec:CC_ONLY [(match_operand 5 "immediate_operand")] @@ -542,7 +542,7 @@ [(match_operand 5 "immediate_operand")] UNSPEC_NZCV) (compare:CC_ONLY - (match_operand:GPI 2 "register_operand" "r,r,r") + (match_operand:GPI 2 "aarch64_reg_or_zero" "rZ,rZ,rZ") (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn"] "" "@ @@ -4009,14 +4009,14 @@ (define_insn "cmp" [(set (reg:CC CC_REGNUM) - (compare:CC (match_operand:GPI 0 "register_operand" "rk,rk,rk") - (match_operand:GPI 1 "aarch64_plus_operand" "r,I,J")))] + (compare:CC (match_operand:GPI 0 "aarch64_reg_or_zero" "rk,rk,rkZ") + (match_operand:GPI 1 "aarch64_plus_operand" "I,J,rZ")))] "" "@ - cmp\\t%0, %1 cmp\\t%0, %1 - cmn\\t%0, #%n1" - [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] + cmn\\t%0, #%n1 + cmp\\t%0, %1" + [(set_attr "type" "alus_imm,alus_imm,alus_sreg")] ) (define_insn "fcmp" -- 2.20.1
[PATCH 0/6] aarch64: Implement TImode comparisons
This is attacking case 3 of PR 94174.
The existing ccmp optimization happens at the gimple level,
which means that rtl expansion of TImode stuff cannot take
advantage. But we can to even better than the existing
ccmp optimization.
This expansion is similar size to our current branchful
expansion, but all straight-line code. I will assume in
general that the branch predictor will work better with
fewer branches.
E.g.
- 10: b7f800a3tbnzx3, #63, 24 <__subvti3+0x24>
- 14: eb02003fcmp x1, x2
- 18: 5400010cb.gt38 <__subvti3+0x38>
- 1c: 54000140b.eq44 <__subvti3+0x44> // b.none
- 20: d65f03c0ret
- 24: eb01005fcmp x2, x1
- 28: 548cb.gt38 <__subvti3+0x38>
- 2c: 54a1b.ne20 <__subvti3+0x20> // b.any
- 30: eb9fcmp x4, x0
- 34: 5469b.ls20 <__subvti3+0x20> // b.plast
- 38: a9bf7bfdstp x29, x30, [sp, #-16]!
- 3c: 910003fdmov x29, sp
- 40: 9400bl 0
- 44: eb04001fcmp x0, x4
- 48: 5488b.hi38 <__subvti3+0x38> // b.pmore
- 4c: d65f03c0ret
+ 10: b7f800e3tbnzx3, #63, 2c <__subvti3+0x2c>
+ 14: eb01005fcmp x2, x1
+ 18: 1a9fb7e2csetw2, ge // ge = tcont
+ 1c: fa400080ccmpx4, x0, #0x0, eq // eq = none
+ 20: 7a40a844ccmpw2, #0x0, #0x4, ge // ge = tcont
+ 24: 54e0b.eq40 <__subvti3+0x40> // b.none
+ 28: d65f03c0ret
+ 2c: eb01005fcmp x2, x1
+ 30: 1a9fc7e2csetw2, le
+ 34: fa400081ccmpx4, x0, #0x1, eq // eq = none
+ 38: 7a40d844ccmpw2, #0x0, #0x4, le
+ 3c: 5460b.eq28 <__subvti3+0x28> // b.none
+ 40: a9bf7bfdstp x29, x30, [sp, #-16]!
+ 44: 910003fdmov x29, sp
+ 48: 9400bl 0
So one less insn, but 2 branches instead of 6.
As for the specific case of the PR,
void test_int128(__int128 a, uint64_t l)
{
if ((__int128_t)a - l <= 1)
doit();
}
0: eb02subsx0, x0, x2
4: da1f0021sbc x1, x1, xzr
8: f13fcmp x1, #0x0
- c: 544db.le14
- 10: d65f03c0ret
- 14: 5461b.ne20 // b.any
- 18: f100041fcmp x0, #0x1
- 1c: 54a8b.hi10 // b.pmore
+ c: 1a9fc7e1csetw1, le
+ 10: fa410801ccmpx0, #0x1, #0x1, eq // eq = none
+ 14: 7a40d824ccmpw1, #0x0, #0x4, le
+ 18: 5441b.ne20 // b.any
+ 1c: d65f03c0ret
20: 1400b 0
r~
Richard Henderson (6):
aarch64: Add ucmp_*_carryinC patterns for all usub_*_carryinC
aarch64: Adjust result of aarch64_gen_compare_reg
aarch64: Accept 0 as first argument to compares
aarch64: Simplify @ccmp operands
aarch64: Improve nzcv argument to ccmp
aarch64: Implement TImode comparisons
gcc/config/aarch64/aarch64.c | 304 --
gcc/config/aarch64/aarch64-simd.md| 18 +-
gcc/config/aarch64/aarch64-speculation.cc | 5 +-
gcc/config/aarch64/aarch64.md | 280 ++--
4 files changed, 429 insertions(+), 178 deletions(-)
--
2.20.1
Re: [PATCH] rs6000: Check -+0 and NaN for smax/smin generation
Jiufu Guo writes: Backported to GCC 9, preapproved by Segher. Thanks, Jiufu > Segher Boessenkool writes: > >> Hi! >> >> On Thu, Mar 05, 2020 at 10:46:58AM +0800, Jiufu Guo wrote: >>> PR93709 mentioned regressions on maxlocval_4.f90 and minlocval_f.f90 which >>> relates to max of '-inf' and 'nan'. This regression occur on P9 which has >>> new instruction 'xsmaxcdp/xsmincdp'. >>> The similar issue also could be find on `a < b ? b : a` which is also >>> generated as `xsmaxcdp` under -O2 for P9. This instruction `xsmaxcdp` >>> more like C/C++ semantic (a>b?a:b). A testcase is added for this issue. >>> >>> The following patch improve code to check -+0 and NaN before 'smax/smin' to >>> be generated for those cases. >> >>> - else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond)) >>> + /* Only when -fno-signed-zeros and -ffinite_math_only are in effect, >>> + `op0 < op1 ? op1 : op0` works like `op1 > op0 ? op1 : op0` which >>> + could use smax; >>> + `op0 > op1 ? op1 : op0` works like `op1 < op0 ? op1 : op0` which >>> + could use smin. */ >>> + else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond) >>> + && (flag_finite_math_only && !flag_signed_zeros)) >>> max_p = !max_p; >> >> I know I asked for it, but should this use HONOR_NANS (compare_mode) >> instead? Infinities will work fine? Just NaNs and zeros won't. > HONOR_NANS(mode) is `MODE_HAS_NANS (mode) && !flag_finite_math_only`. > We know the mode is SF or DF. Both maybe ok for current code. > > I agree with you HONOR_NANS would be better, it is more generic for > front-end, gimple and rtl. And rs6000_emit_p9_fp_minmax maybe called > without checking mode in future code, in this case HONOR_NANS is > better. > > I updated the code as: > ``` > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index f34e1ba70c6..b057f689b56 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -14836,7 +14836,11 @@ rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx > true_cond, rtx false_cond) >if (rtx_equal_p (op0, true_cond) && rtx_equal_p (op1, false_cond)) > ; > > - else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, >false_cond)) >+ /* Only when NaNs and signed-zeros are not in effect, smax could be >+ used for `op0 < op1 ? op1 : op0`, and smin could be used for >+ `op0 > op1 ? op1 : op0`. */ >+ else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond) >+ && !HONOR_NANS (compare_mode) && >!HONOR_SIGNED_ZEROS(compare_mode)) > max_p = !max_p; > >else > ``` > This code works fine. I'm going to submit it. > > Thanks! > Jiufu Guo > >> >> Okay for trunk with that change (if it works :-) ) Thanks! >> >> >> Segher
