Re: [PATCH] i386: Add register source to movddup
On Sun, 14 Oct 2018, Alexander Monakov wrote: > > I doubt this is a correct fix, and I think the issue merits more > investigation. > Please see comment #5 in the PR. Sorry, it seems I was misunderstanding how constraints interact with cost calculation. I withdraw my objection to the patch. Alexander
[PATCH, moxie] Adjust divide-by-zero testcase for moxie
This patch adjusts one of the c-torture tests to account for the possible lack of divide-by-zero exceptions on certain moxie targets. Committed. gcc/testsuite/ 2018-10-14 Anthony Green * gcc.c-torture/execute/20101011-1.c: Adjust for moxie. Index: gcc/testsuite/gcc.c-torture/execute/20101011-1.c === --- gcc/testsuite/gcc.c-torture/execute/20101011-1.c(revision 265146) +++ gcc/testsuite/gcc.c-torture/execute/20101011-1.c(working copy) @@ -97,6 +97,9 @@ /* This presently doesn't raise SIGFPE even on csky-linux-gnu, much less bare metal. See the implementation of __divsi3 in libgcc. */ # define DO_TEST 0 +#elif defined (__moxie__) + /* Not all moxie configurations may raise exceptions. */ +# define DO_TEST 0 #else # define DO_TEST 1 #endif
[Patch, fortran] PR87566 - ICE with class(*) and select
Tobias started this patch and I finished it in answering a question
that he had about a problem with the gimplifier. Along the way, I
tried the associate version of the select type test case and found
that it failed in a different way. The chunk in resolve_assoc_var
fixes that.
Bootstrapped and regtested on FC28/x86_64 - OK for trunk?
On checking to see if any other associate problems had been fixed, I
noticed, as had Dominique, that PR83146 was fixed. I committed the
testcase to trunk as revision 265148 to make sure that it remained so.
Paul
2018-10-14 Paul Thomas
Tobias Burnus
PR fortran/87566
* resolve.c (resolve_assoc_var): Add missing array spec for
class associate names.
(resolve_select_type): Handle case where last typed component
of the selector has a different type to the expression.
* trans-expr.c (gfc_find_and_cut_at_last_class_ref): Replace
call to gfc_expr_to_initialize with call to gfc_copy_expr.
(gfc_conv_class_to_class): Guard assignment to 'len' field
against case where zero constant is supplied.
2018-10-14 Paul Thomas
Tobias Burnus
PR fortran/87566
* gfortran.dg/select_type_44.f90: New test.
* gfortran.dg/associate_42.f90: New test.
Index: gcc/fortran/resolve.c
===
*** gcc/fortran/resolve.c (revision 264948)
--- gcc/fortran/resolve.c (working copy)
*** resolve_assoc_var (gfc_symbol* sym, bool
*** 8675,8680
--- 8675,8692
if (as->corank != 0)
sym->attr.codimension = 1;
}
+ else if (sym->ts.type == BT_CLASS && (!CLASS_DATA (sym)->as || sym->assoc->rankguessed))
+ {
+ if (!CLASS_DATA (sym)->as)
+ CLASS_DATA (sym)->as = gfc_get_array_spec ();
+ as = CLASS_DATA (sym)->as;
+ as->rank = target->rank;
+ as->type = AS_DEFERRED;
+ as->corank = gfc_get_corank (target);
+ CLASS_DATA (sym)->attr.dimension = 1;
+ if (as->corank != 0)
+ CLASS_DATA (sym)->attr.codimension = 1;
+ }
}
else
{
*** resolve_select_type (gfc_code *code, gfc
*** 8875,8883
if (code->expr2)
{
! if (code->expr1->symtree->n.sym->attr.untyped)
! code->expr1->symtree->n.sym->ts = code->expr2->ts;
! selector_type = CLASS_DATA (code->expr2)->ts.u.derived;
if (code->expr2->rank && CLASS_DATA (code->expr1)->as)
CLASS_DATA (code->expr1)->as->rank = code->expr2->rank;
--- 8887,8910
if (code->expr2)
{
! gfc_ref *ref2 = NULL;
! for (ref = code->expr2->ref; ref != NULL; ref = ref->next)
! if (ref->type == REF_COMPONENT
! && ref->u.c.component->ts.type == BT_CLASS)
! ref2 = ref;
!
! if (ref2)
! {
! if (code->expr1->symtree->n.sym->attr.untyped)
! code->expr1->symtree->n.sym->ts = ref->u.c.component->ts;
! selector_type = CLASS_DATA (ref2->u.c.component)->ts.u.derived;
! }
! else
! {
! if (code->expr1->symtree->n.sym->attr.untyped)
! code->expr1->symtree->n.sym->ts = code->expr2->ts;
! selector_type = CLASS_DATA (code->expr2)->ts.u.derived;
! }
if (code->expr2->rank && CLASS_DATA (code->expr1)->as)
CLASS_DATA (code->expr1)->as->rank = code->expr2->rank;
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c (revision 264948)
--- gcc/fortran/trans-expr.c (working copy)
*** gfc_find_and_cut_at_last_class_ref (gfc_
*** 394,400
e->ref = NULL;
}
! base_expr = gfc_expr_to_initialize (e);
/* Restore the original tail expression. */
if (class_ref)
--- 394,400
e->ref = NULL;
}
! base_expr = gfc_copy_expr (e);
/* Restore the original tail expression. */
if (class_ref)
*** gfc_conv_class_to_class (gfc_se *parmse,
*** 1131,1137
/* Return the len component, except in the case of scalarized array
references, where the dynamic type cannot change. */
! if (!elemental && full_array && copyback)
gfc_add_modify (&parmse->post, tmp,
fold_convert (TREE_TYPE (tmp), ctree));
}
--- 1131,1138
/* Return the len component, except in the case of scalarized array
references, where the dynamic type cannot change. */
! if (!elemental && full_array && copyback
! && (UNLIMITED_POLY (e) || VAR_P (tmp)))
gfc_add_modify (&parmse->post, tmp,
fold_convert (TREE_TYPE (tmp), ctree));
}
Index: gcc/testsuite/gfortran.dg/associate_42.f90
===
*** gcc/testsuite/gfortran.dg/associate_42.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/associate_42.f90 (working copy)
***
*** 0
--- 1,41
+ ! { dg-do run }
+ !
+ ! Tests the fix for a bug that was found in the course of fixing PR87566.
+ !
+ ! Contributed by Paul Thomas
+ !
+ call AddArray
+ contains
+ subroutine AddA
Update my email address
Hi, The company (Alibaba) finished signing copyright assignment with FSF, so this patch updates my email address as attached. Thanks, bin email-address.txt Description: Binary data
[PATCH PR87022]Check all bits in dist-vector rather than the fisrt in loop distribution
Hi, This patch fixes PR87022. The root cause is the original code checks the first bit in dist vector for zero and we still do that after enabling loop nest distribution. For the test case, the first bit is for outer loop while the dependence happens in the inner loop, as a result, the direction of dependence is not correctly reverted. This patch fixes the issue by checking all bits in dist vector. Bootstrap and test on x86_64, is it OK? Thanks, bin 2018-10-14 Bin Cheng PR tree-optimization/87022 * tree-loop-distribution.c (pg_add_dependence_edges): Check all bits in dist vector rather than the first one. 2018-10-14 Bin Cheng PR tree-optimization/87022 * gcc.dg/tree-ssa/pr87022.c: New test. pr87022.txt Description: Binary data
Re: [PATCH/RFC] Add "User Experience Guidelines" to gccint.texi
On 10/12/18, David Malcolm wrote:
> Here's a proposed "User Experience Guidelines" section for our
> internals manual
>
> It's a mixture of proposed policy, together with notes on how to
> implement the recommendations.
>
> Thoughts?
I have no comments on the actual contents of the patch, just that it's
a good idea to have such a section in general, and I hope it's
approved!
Eric
>
> gcc/ChangeLog:
> * Makefile.in (TEXI_GCCINT_FILES): Add ux.texi.
> * doc/gccint.texi: Include ux.texi and use it in top-level menu.
> * doc/ux.texi: New file.
> ---
> gcc/Makefile.in | 2 +-
> gcc/doc/gccint.texi | 2 +
> gcc/doc/ux.texi | 455
>
> 3 files changed, 458 insertions(+), 1 deletion(-)
> create mode 100644 gcc/doc/ux.texi
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 70efab7..3f05e95 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3176,7 +3176,7 @@ TEXI_GCCINT_FILES = gccint.texi gcc-common.texi
> gcc-vers.texi \
>gnu.texi gpl_v3.texi fdl.texi contrib.texi languages.texi \
>sourcebuild.texi gty.texi libgcc.texi cfg.texi tree-ssa.texi \
>loop.texi generic.texi gimple.texi plugins.texi optinfo.texi \
> - match-and-simplify.texi poly-int.texi
> + match-and-simplify.texi ux.texi poly-int.texi
>
> TEXI_GCCINSTALL_FILES = install.texi install-old.texi fdl.texi
> \
>gcc-common.texi gcc-vers.texi
> diff --git a/gcc/doc/gccint.texi b/gcc/doc/gccint.texi
> index 1a1af41..2554b31 100644
> --- a/gcc/doc/gccint.texi
> +++ b/gcc/doc/gccint.texi
> @@ -125,6 +125,7 @@ Additional tutorial information is linked to from
> * LTO:: Using Link-Time Optimization.
>
> * Match and Simplify:: How to write expression simplification patterns for
> GIMPLE and GENERIC
> +* User Experience Guidelines:: Guidelines for implementing diagnostics and
> options.
> * Funding:: How to help assure funding for free software.
> * GNU Project:: The GNU Project and GNU/Linux.
>
> @@ -162,6 +163,7 @@ Additional tutorial information is linked to from
> @include plugins.texi
> @include lto.texi
> @include match-and-simplify.texi
> +@include ux.texi
>
> @include funding.texi
> @include gnu.texi
> diff --git a/gcc/doc/ux.texi b/gcc/doc/ux.texi
> new file mode 100644
> index 000..87ff599
> --- /dev/null
> +++ b/gcc/doc/ux.texi
> @@ -0,0 +1,455 @@
> +@c Copyright (C) 2018 Free Software Foundation, Inc.
> +@c Free Software Foundation, Inc.
> +@c This is part of the GCC manual.
> +@c For copying conditions, see the file gcc.texi.
> +
> +@node User Experience Guidelines
> +@chapter User Experience Guidelines
> +@cindex User Experience Guidelines
> +@cindex Guidelines, User Experience
> +
> +To borrow a slogan from
> + @uref{https://elm-lang.org/blog/compilers-as-assistants, Elm},
> +
> +@quotation
> +@strong{Compilers should be assistants, not adversaries.} A compiler
> should
> +not just detect bugs, it should then help you understand why there is a
> bug.
> +It should not berate you in a robot voice, it should give you specific
> hints
> +that help you write better code. Ultimately, a compiler should make
> +programming faster and more fun!
> +@author Evan Czaplicki
> +@end quotation
> +
> +This chapter provides guidelines on how to implement diagnostics and
> +command-line options in ways that we hope achieve the above ideal.
> +
> +@menu
> +* Guidelines for Diagnostics:: How to implement diagnostics.
> +* Guidelines for Options:: Guidelines for command-line options.
> +@end menu
> +
> +
> +@node Guidelines for Diagnostics
> +@cindex Guidelines for Diagnostics
> +@cindex Diagnostics, Guidelines for
> +
> +@section Guidelines for Diagnostics
> +
> +@subsection Talk in terms of the user's code
> +
> +Diagnostics should be worded in terms of the user's source code, and the
> +source language, rather than GCC's own implementation details.
> +
> +@subsection Diagnostics are ``actionable''
> +
> +A good diagnostic is @emph{actionable}: it should assist the user in
> +taking action.
> +
> +Consider what an end-user will want to do when encountering a diagnostic.
> +
> +Given an error, an end-user will think: ``How do I fix this?''
> +
> +Given a warning, an end-user will want to review the warning and think:
> +
> +@itemize @bullet
> +@item
> +``Is this a ``true'' result?''
> +@item
> +``Do I care?''
> +@item
> +if they decide it's genuine: ``How do I fix this?''
> +@end itemize
> +
> +A good diagnostic provides pertinent information to allow the user to
> +easily answer the above questions.
> +
> +@subsection The user's attention is important
> +
> +A perfect compiler would issue a warning on every aspect of the user's
> +source code that ought to be fixed, and issue no other warnings.
> +Naturally, this ideal is impossible to achieve.
> +
> +Warnings should have a good ``signal:noise ratio'': we should have few
> +``false positive
Re: [PATCH 02/14] Add D frontend (GDC) implementation.
[Sorry if this turns out to do be a dup]
Iain Buclaw writes:
> On 18 September 2018 at 02:33, Iain Buclaw wrote:
>> This patch adds the D front-end implementation, the only part of the
>> compiler that interacts with GCC directly, and being the parts that I
>> maintain, is something that I can talk about more directly.
>>
>> For the actual code generation pass, that converts the front-end AST
>> to GCC trees, most parts use a separate Visitor interfaces to do a
>> certain kind of lowering, for instance, types.cc builds *_TYPE trees
>> from AST Type's. The Visitor class is part of the DMD front-end, and
>> is defined in dfrontend/visitor.h.
>>
>> There are also a few interfaces which have their headers in the DMD
>> frontend, but are implemented here because they do something that
>> requires knowledge of the GCC backend (d-target.cc), does something
>> that may not be portable, or differ between D compilers
>> (d-frontend.cc) or are a thin wrapper around something that is managed
>> by GCC (d-diagnostic.cc).
>>
>> Many high level operations result in generation of calls to D runtime
>> library functions (runtime.def), all with require some kind of runtime
>> type information (typeinfo.cc). The compiler also generates functions
>> for registering/deregistering compiled modules with the D runtime
>> library (modules.cc).
>>
>> As well as the D language having it's own built-in functions
>> (intrinsics.cc), we also expose GCC builtins to D code via a
>> `gcc.builtins' module (d-builtins.cc), and give special treatment to a
>> number of UDAs that could be applied to functions (d-attribs.cc).
>>
>>
>> That is roughly the high level jist of how things are currently organized.
>>
>> ftp://ftp.gdcproject.org/patches/v4/02-v4-d-frontend-gdc.patch
>>
>
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00587.html
>
> During the last round, the last comment given was that the reviewer
> wasn't going to dive deep into this code, as it's essentially
> converting between the different representations and is code that I'd
> be maintaining.
>
> As this is code that other gcc maintainers will be potentially looking
> after as well, I'd like any glaring problems to be dealt with
> immediately.
I won't claim that I dived deep into the code either, since with a patch
of this size that would take weeks. But FWIW I read it through trying
to scan for anything that stood out.
I think the patch is OK besides the below. The comments are in patch
order and so mix very trivial stuff with things that seem more fundamental.
I think the only real blocker is the point below about translations.
> +/* Define attributes that are mutually exclusive with one another. */
> +static const struct attribute_spec::exclusions attr_noreturn_exclusions[] =
> +{
> + ATTR_EXCL ("noreturn", true, true, true),
Why does noreturn exclude itself? Probably worth a comment if deliberate.
> +tree
> +build_attributes (Expressions *eattrs)
> +{
> [...]
> + tree list = build_tree_list (get_identifier (name), args);
> + attribs = chainon (attribs, list);
Too many spaces before "chainon".
> +static tree
> +handle_const_attribute (tree *node, tree ARG_UNUSED (name),
> + tree ARG_UNUSED (args), int ARG_UNUSED (flags),
> + bool * ARG_UNUSED (no_add_attrs))
> +{
> + tree type = TREE_TYPE (*node);
> +
> + /* See FIXME comment on noreturn in c_common_attribute_table. */
Seems strange to be referencing c-family code here, especially since
there's no corresponding comment for handle_noreturn_attribute and
also no FIXME comment in the D version of the table. Think we should
just drop the comment above.
It's unfortunate that there's so much cut-&-paste with c-attribs.c,
but there again, the current split between target-independent code
(attribs.*) and frontend code (c-attribs.*) isn't very clear-cut,
so I don't think this should hold up acceptance.
I'm going to review it as new code though, so:
> +/* Handle a "malloc" attribute; arguments as in
> + struct attribute_spec.handler. */
> +
> +tree
> +handle_malloc_attribute (tree *node, tree ARG_UNUSED (name),
> + tree ARG_UNUSED (args), int ARG_UNUSED (flags),
> + bool * ARG_UNUSED (no_add_attrs))
> +{
> + if (TREE_CODE (*node) == FUNCTION_DECL
> + && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (*node
> +DECL_IS_MALLOC (*node) = 1;
> + else
> +gcc_unreachable ();
> +
> + return NULL_TREE;
> +}
> +
> +/* Handle a "pure" attribute; arguments as in
> + struct attribute_spec.handler. */
> +
> +static tree
> +handle_pure_attribute (tree *node, tree ARG_UNUSED (name),
> +tree ARG_UNUSED (args), int ARG_UNUSED (flags),
> +bool * ARG_UNUSED (no_add_attrs))
> +{
> + if (TREE_CODE (*node) == FUNCTION_DECL)
> +DECL_PURE_P (*node) = 1;
> + else
> +gcc_unreachable ();
> +
> + return NULL_TREE;
> +}
> +
> +/* Handle a "no vops" attribute; arguments as in
> +
Re: [PATCH/RFC] Add "User Experience Guidelines" to gccint.texi
On 10/12/2018 09:43 AM, David Malcolm wrote:
Here's a proposed "User Experience Guidelines" section for our
internals manual
It's a mixture of proposed policy, together with notes on how to
implement the recommendations.
Thoughts?
To improve consistency among diagnostic messages it's important
to have a set of guidelines in place. Thank you for taking
the first step!
gcc/ChangeLog:
* Makefile.in (TEXI_GCCINT_FILES): Add ux.texi.
* doc/gccint.texi: Include ux.texi and use it in top-level menu.
* doc/ux.texi: New file.
---
gcc/Makefile.in | 2 +-
gcc/doc/gccint.texi | 2 +
gcc/doc/ux.texi | 455
3 files changed, 458 insertions(+), 1 deletion(-)
create mode 100644 gcc/doc/ux.texi
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 70efab7..3f05e95 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3176,7 +3176,7 @@ TEXI_GCCINT_FILES = gccint.texi gcc-common.texi
gcc-vers.texi \
gnu.texi gpl_v3.texi fdl.texi contrib.texi languages.texi \
sourcebuild.texi gty.texi libgcc.texi cfg.texi tree-ssa.texi \
loop.texi generic.texi gimple.texi plugins.texi optinfo.texi \
-match-and-simplify.texi poly-int.texi
+match-and-simplify.texi ux.texi poly-int.texi
TEXI_GCCINSTALL_FILES = install.texi install-old.texi fdl.texi \
gcc-common.texi gcc-vers.texi
diff --git a/gcc/doc/gccint.texi b/gcc/doc/gccint.texi
index 1a1af41..2554b31 100644
--- a/gcc/doc/gccint.texi
+++ b/gcc/doc/gccint.texi
@@ -125,6 +125,7 @@ Additional tutorial information is linked to from
* LTO:: Using Link-Time Optimization.
* Match and Simplify:: How to write expression simplification patterns for
GIMPLE and GENERIC
+* User Experience Guidelines:: Guidelines for implementing diagnostics and
options.
* Funding:: How to help assure funding for free software.
* GNU Project:: The GNU Project and GNU/Linux.
@@ -162,6 +163,7 @@ Additional tutorial information is linked to from
@include plugins.texi
@include lto.texi
@include match-and-simplify.texi
+@include ux.texi
@include funding.texi
@include gnu.texi
diff --git a/gcc/doc/ux.texi b/gcc/doc/ux.texi
new file mode 100644
index 000..87ff599
--- /dev/null
+++ b/gcc/doc/ux.texi
@@ -0,0 +1,455 @@
+@c Copyright (C) 2018 Free Software Foundation, Inc.
+@c Free Software Foundation, Inc.
+@c This is part of the GCC manual.
+@c For copying conditions, see the file gcc.texi.
+
+@node User Experience Guidelines
+@chapter User Experience Guidelines
+@cindex User Experience Guidelines
+@cindex Guidelines, User Experience
+
+To borrow a slogan from
+ @uref{https://elm-lang.org/blog/compilers-as-assistants, Elm},
+
+@quotation
+@strong{Compilers should be assistants, not adversaries.} A compiler should
+not just detect bugs, it should then help you understand why there is a bug.
+It should not berate you in a robot voice, it should give you specific hints
+that help you write better code. Ultimately, a compiler should make
+programming faster and more fun!
+@author Evan Czaplicki
+@end quotation
+
+This chapter provides guidelines on how to implement diagnostics and
+command-line options in ways that we hope achieve the above ideal.
+
+@menu
+* Guidelines for Diagnostics:: How to implement diagnostics.
+* Guidelines for Options:: Guidelines for command-line options.
+@end menu
+
+
+@node Guidelines for Diagnostics
+@cindex Guidelines for Diagnostics
+@cindex Diagnostics, Guidelines for
+
+@section Guidelines for Diagnostics
+
+@subsection Talk in terms of the user's code
+
+Diagnostics should be worded in terms of the user's source code, and the
+source language, rather than GCC's own implementation details.
+
+@subsection Diagnostics are ``actionable''
+
+A good diagnostic is @emph{actionable}: it should assist the user in
+taking action.
+
+Consider what an end-user will want to do when encountering a diagnostic.
+
+Given an error, an end-user will think: ``How do I fix this?''
+
+Given a warning, an end-user will want to review the warning and think:
+
+@itemize @bullet
+@item
+``Is this a ``true'' result?''
Sounds like you mean a true positive here but I'm not 100% sure
so expanding on this a bit it might be helpful -- maybe simply
phrasing it as "is this a real problem?"
+@item
+``Do I care?''
+@item
+if they decide it's genuine: ``How do I fix this?''
+@end itemize
+
+A good diagnostic provides pertinent information to allow the user to
+easily answer the above questions.
+
+@subsection The user's attention is important
+
+A perfect compiler would issue a warning on every aspect of the user's
+source code that ought to be fixed, and issue no other warnings.
+Naturally, this ideal is impossible to achieve.
+
+Warnings should have a good ``signal:noise ratio'': we should have few
+``false positives'' (falsely issuing a warning when no warning is
+warranted) and few ``fals
Re: [PATCH] i386: Add register source to movddup
On Sat, Oct 13, 2018 at 11:54 PM H.J. Lu wrote:
>
> Add register source to movddup so that IRA will allow register source
> for *vec_dupv2di when SSE3 is enabled.
>
> gcc/
>
> PR target/87599
> * config/i386/sse.md (*vec_dupv2di): Add register source to
> movddup.
>
> gcc/testsuite/
>
> PR target/87599
> * gcc.target/i386/pr87599.c: New test.
OK with a small scan-string fix, described below.
Uros.
> ---
> gcc/config/i386/sse.md | 2 +-
> gcc/testsuite/gcc.target/i386/pr87599.c | 12
> 2 files changed, 13 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr87599.c
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index af4d80d8c9b..4b2193e0462 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -18185,7 +18185,7 @@
> (define_insn "*vec_dupv2di"
>[(set (match_operand:V2DI 0 "register_operand" "=x,v,v,x")
> (vec_duplicate:V2DI
> - (match_operand:DI 1 "nonimmediate_operand" " 0,Yv,m,0")))]
> + (match_operand:DI 1 "nonimmediate_operand" " 0,Yv,vm,0")))]
>"TARGET_SSE"
>"@
> punpcklqdq\t%0, %0
> diff --git a/gcc/testsuite/gcc.target/i386/pr87599.c
> b/gcc/testsuite/gcc.target/i386/pr87599.c
> new file mode 100644
> index 000..abbeb7a41af
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr87599.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-march=corei7 -O2" } */
> +/* { dg-final { scan-assembler-times "punpcklqdq\[ \\t\]+%xmm\[0-9\],\[
> \\t\]+%xmm\[0-9\]" 1 } } */
You need to scan for %xmm\[0-9\]+, otherwise xmm10 is already out of luck.
> +
> +#include
> +
> +__m128i
> +foo (long long val)
> +{
> + __m128i rval = {val, val};
> + return rval;
> +}
> --
> 2.17.2
>
Re: [PATCH] i386: Also disable AVX512IFMA/AVX5124FMAPS/AVX5124VNNIW
On Sat, Oct 13, 2018 at 11:54 PM H.J. Lu wrote:
>
> Also disable AVX512IFMA, AVX5124FMAPS and AVX5124VNNIW when disabling
> AVX512F.
>
> gcc/
>
> PR target/87572
> * common/config/i386/i386-common.c (OPTION_MASK_ISA_AVX512F_UNSET):
> Add OPTION_MASK_ISA_AVX512IFMA_UNSET,
> OPTION_MASK_ISA_AVX5124FMAPS_UNSET and
> OPTION_MASK_ISA_AVX5124VNNIW_UNSET.
>
> gcc/testsuite/
>
> PR target/87572
> * gcc.target/i386/pr87572.c: New test.
LGTM.
Thanks,
Uros.
> gcc/common/config/i386/i386-common.c| 8 ++--
> gcc/testsuite/gcc.target/i386/pr87572.c | 10 ++
> 2 files changed, 16 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr87572.c
>
> diff --git a/gcc/common/config/i386/i386-common.c
> b/gcc/common/config/i386/i386-common.c
> index 3b5312d7250..36ef999df83 100644
> --- a/gcc/common/config/i386/i386-common.c
> +++ b/gcc/common/config/i386/i386-common.c
> @@ -194,8 +194,12 @@ along with GCC; see the file COPYING3. If not see
>(OPTION_MASK_ISA_AVX512F | OPTION_MASK_ISA_AVX512CD_UNSET \
> | OPTION_MASK_ISA_AVX512PF_UNSET | OPTION_MASK_ISA_AVX512ER_UNSET \
> | OPTION_MASK_ISA_AVX512DQ_UNSET | OPTION_MASK_ISA_AVX512BW_UNSET \
> - | OPTION_MASK_ISA_AVX512VL_UNSET | OPTION_MASK_ISA_AVX512VBMI2_UNSET \
> - | OPTION_MASK_ISA_AVX512VNNI_UNSET |
> OPTION_MASK_ISA_AVX512VPOPCNTDQ_UNSET \
> + | OPTION_MASK_ISA_AVX512VL_UNSET | OPTION_MASK_ISA_AVX512IFMA_UNSET \
> + | OPTION_MASK_ISA_AVX5124FMAPS_UNSET \
> + | OPTION_MASK_ISA_AVX5124VNNIW_UNSET \
> + | OPTION_MASK_ISA_AVX512VBMI2_UNSET \
> + | OPTION_MASK_ISA_AVX512VNNI_UNSET \
> + | OPTION_MASK_ISA_AVX512VPOPCNTDQ_UNSET \
> | OPTION_MASK_ISA_AVX512BITALG_UNSET)
> #define OPTION_MASK_ISA_AVX512CD_UNSET OPTION_MASK_ISA_AVX512CD
> #define OPTION_MASK_ISA_AVX512PF_UNSET OPTION_MASK_ISA_AVX512PF
> diff --git a/gcc/testsuite/gcc.target/i386/pr87572.c
> b/gcc/testsuite/gcc.target/i386/pr87572.c
> new file mode 100644
> index 000..ea1beb78f5c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr87572.c
> @@ -0,0 +1,10 @@
> +/* PR target/82483 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mavx512ifma -mno-sse2 -w -Wno-psabi" } */
> +
> +typedef long long __m512i __attribute__((__vector_size__(64)));
> +__m512i
> +foo (__m512i c, __m512i d, __m512i e, int b)
> +{
> + return __builtin_ia32_vpmadd52huq512_maskz (c, d, e, b); /* { dg-error
> "incompatible types" } */
> +}
> --
> 2.17.2
>
[PATCH] asm non-code template parts (alternative to asm inline)
Hello,
This is an alternative proposal to the "asm inline" feature.
Kernel developers have reported suboptimal optimization where use of asm
statements such as
asm("ud2\n"
".pushsection foo\n"
...
".popsection\n" : : ...)
impacts inlining decisions badly, since GCC assumes cost of the asm to be
high, even though it emits just one instruction to the text section. I'd
like to point out that branch range optimization is also negatively affected.
I suggest we give asm writers a way to mark portions of the asm template
that should be ignored in cost estimation. This is a more fine-grained
mechanism compared to 'asm inline', and it also helps branch range optimization.
Specifically, I propose that in Extended asms, percent-backtick (%`) is
recognized as such region boundary. Percent sign is of course always special
in Extended asms, and backtick sign is not claimed by any backend.
For Basic asms, no similar mechanism is necessary since they are antithetical
to efficiency in the first place.
Kernels developers can then use this extension via
[if gcc-9 or compatible]
#define ASM_NONTEXT_BEGIN "%`\n"
[else]
#define ASM_NONTEXT_BEGIN "\n"
[endif]
#define ASM_NONTEXT_END ASM_NONTEXT_BEGIN
asm("ud2\n"
ASM_NONTEXT_BEGIN
".pushsection foo\n"
...
".popsection\n"
ASM_NONTEXT_END : : ...)
How does this look?
* doc/extend.texi (Extended Asm): Document %` in template.
(Size of an Asm): Document intended use of %`.
* final.c (asm_insn_count): Adjust.
(asm_str_count): Add argument to distinguish basic and extended asms.
In extended asms, ignore separators inside of %` ... %`.
(output_asm_insn): Handle %`.
* rtl.h (asm_str_count): Adjust prototype.
* tree-inline.c (estimate_num_insns): Adjust.
* config/arm/arm.c (arm_rtx_costs_internal): Adjust.
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index cfe6a8e5bb8..798d310061c 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8613,6 +8613,11 @@ generates multiple assembler instructions.
Outputs @samp{@{}, @samp{|}, and @samp{@}} characters (respectively)
into the assembler code. When unescaped, these characters have special
meaning to indicate multiple assembler dialects, as described below.
+
+@item %`
+Signifies a boundary of a region where instruction separators are not
+counted towards its cost (@pxref{Size of an asm}). Must be followed by
+a whitespace character.
@end table
@subsubheading Multiple assembler dialects in @code{asm} templates
@@ -9821,7 +9826,7 @@ does this by counting the number of instructions in the
pattern of the
@code{asm} and multiplying that by the length of the longest
instruction supported by that processor. (When working out the number
of instructions, it assumes that any occurrence of a newline or of
-whatever statement separator character is supported by the assembler --
+whatever statement separator character is supported by the assembler ---
typically @samp{;} --- indicates the end of an instruction.)
Normally, GCC's estimate is adequate to ensure that correct
@@ -9832,6 +9837,15 @@ space in the object file than is needed for a single
instruction.
If this happens then the assembler may produce a diagnostic saying that
a label is unreachable.
+Likewise, it is possible for GCC to significantly over-estimate the
+number of instructions in an @code{asm}, resulting in suboptimal decisions
+when the estimate is used during inlining and branch range optimization.
+This can happen if the @code{asm} template has many lines that do not
+correspond to instructions, for example when the @samp{.pushsection}
+directive is used to emit auxiliary data in a non-code section.
+For Extended @code{asm} statements, you can improve the estimate by
+wrapping the non-code portion in @samp{%` ... %`} delimiters.
+
@node Alternate Keywords
@section Alternate Keywords
@cindex alternate keywords
diff --git a/gcc/final.c b/gcc/final.c
index 6943c073d9b..dc474d7f6e1 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1408,29 +1408,40 @@ static int
asm_insn_count (rtx body)
{
const char *templ;
+ bool basic = GET_CODE (body) == ASM_INPUT;
- if (GET_CODE (body) == ASM_INPUT)
+ if (basic)
templ = XSTR (body, 0);
else
templ = decode_asm_operands (body, NULL, NULL, NULL, NULL, NULL);
- return asm_str_count (templ);
+ return asm_str_count (templ, basic);
}
/* Return the number of machine instructions likely to be generated for the
- inline-asm template. */
+ inline-asm template. BASIC indicates if it is used in a basic asm. */
int
-asm_str_count (const char *templ)
+asm_str_count (const char *templ, bool basic)
{
int count = 1;
+ bool in_backticks = false;
if (!*templ)
return 0;
for (; *templ; templ++)
-if (IS_ASM_LOGICAL_LINE_SEPARATOR (*templ, templ)
- || *templ == '\n')
- count++;
+if (*templ == '%' && !basic)
+ {
+
Re: [PATCH] i386: Add register source to movddup
On Sun, 14 Oct 2018, Uros Bizjak wrote:
> > +/* { dg-final { scan-assembler-times "punpcklqdq\[ \\t\]+%xmm\[0-9\],\[
> > \\t\]+%xmm\[0-9\]" 1 } } */
>
> You need to scan for %xmm\[0-9\]+, otherwise xmm10 is already out of luck.
I think it would be preferable to scan for 'punpcklqdq xmm0, xmm0' exactly,
because the result (rval) must be in xmm0 for function return anyway.
Alexander
> > +
> > +#include
> > +
> > +__m128i
> > +foo (long long val)
> > +{
> > + __m128i rval = {val, val};
> > + return rval;
> > +}
> > --
> > 2.17.2
> >
>
Re: [PATCH] i386: Add register source to movddup
On 10/14/18, Alexander Monakov wrote:
> On Sun, 14 Oct 2018, Uros Bizjak wrote:
>> > +/* { dg-final { scan-assembler-times "punpcklqdq\[ \\t\]+%xmm\[0-9\],\[
>> > \\t\]+%xmm\[0-9\]" 1 } } */
>>
>> You need to scan for %xmm\[0-9\]+, otherwise xmm10 is already out of
>> luck.
>
> I think it would be preferable to scan for 'punpcklqdq xmm0, xmm0' exactly,
> because the result (rval) must be in xmm0 for function return anyway.
I checked in
/* { dg-final { scan-assembler-times "punpcklqdq\[
\\t\]+%xmm\[0-9\]+,\[ \\t\]+%xmm\[0-9\]+" 1 } } */
It covers 'punpcklqdq xmm0, xmm0' and 'punpcklqdq xmm10, xmm0' .
There should be
only one punpcklqdq.
> Alexander
>
>> > +
>> > +#include
>> > +
>> > +__m128i
>> > +foo (long long val)
>> > +{
>> > + __m128i rval = {val, val};
>> > + return rval;
>> > +}
>> > --
>> > 2.17.2
>> >
>>
>
--
H.J.
Re: Relocation (= move+destroy)
On Sat, 13 Oct 2018, Marc Glisse wrote:
+ template
+struct __is_trivially_relocatable
+: is_trivially_move_constructible<_Tp> { };
Oups, this part is wrong, sorry, it is supposed to be "is_trivial" instead
of "is_trivially_move_constructible", to match what is done elsewhere in
this file. Using is_trivially_move_constructible instead (like libc++) is
a separate issue, and in the particular case of relocation would need to
be combined with is_trivially_destructible.
I'll retest with is_trivial, but I would be very surprised if that broke
anything in the testsuite.
--
Marc Glisse
Re: [PATCH 02/14] Add D frontend (GDC) implementation.
On 14 October 2018 at 17:29, Richard Sandiford
wrote:
> [Sorry if this turns out to do be a dup]
>
> Iain Buclaw writes:
>> On 18 September 2018 at 02:33, Iain Buclaw wrote:
>>> This patch adds the D front-end implementation, the only part of the
>>> compiler that interacts with GCC directly, and being the parts that I
>>> maintain, is something that I can talk about more directly.
>>>
>>> For the actual code generation pass, that converts the front-end AST
>>> to GCC trees, most parts use a separate Visitor interfaces to do a
>>> certain kind of lowering, for instance, types.cc builds *_TYPE trees
>>> from AST Type's. The Visitor class is part of the DMD front-end, and
>>> is defined in dfrontend/visitor.h.
>>>
>>> There are also a few interfaces which have their headers in the DMD
>>> frontend, but are implemented here because they do something that
>>> requires knowledge of the GCC backend (d-target.cc), does something
>>> that may not be portable, or differ between D compilers
>>> (d-frontend.cc) or are a thin wrapper around something that is managed
>>> by GCC (d-diagnostic.cc).
>>>
>>> Many high level operations result in generation of calls to D runtime
>>> library functions (runtime.def), all with require some kind of runtime
>>> type information (typeinfo.cc). The compiler also generates functions
>>> for registering/deregistering compiled modules with the D runtime
>>> library (modules.cc).
>>>
>>> As well as the D language having it's own built-in functions
>>> (intrinsics.cc), we also expose GCC builtins to D code via a
>>> `gcc.builtins' module (d-builtins.cc), and give special treatment to a
>>> number of UDAs that could be applied to functions (d-attribs.cc).
>>>
>>>
>>> That is roughly the high level jist of how things are currently organized.
>>>
>>> ftp://ftp.gdcproject.org/patches/v4/02-v4-d-frontend-gdc.patch
>>>
>>
>> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00587.html
>>
>> During the last round, the last comment given was that the reviewer
>> wasn't going to dive deep into this code, as it's essentially
>> converting between the different representations and is code that I'd
>> be maintaining.
>>
>> As this is code that other gcc maintainers will be potentially looking
>> after as well, I'd like any glaring problems to be dealt with
>> immediately.
>
> I won't claim that I dived deep into the code either, since with a patch
> of this size that would take weeks. But FWIW I read it through trying
> to scan for anything that stood out.
>
> I think the patch is OK besides the below. The comments are in patch
> order and so mix very trivial stuff with things that seem more fundamental.
> I think the only real blocker is the point below about translations.
>
Having a quick look at the fix-ups, I would say all look reasonable.
>> +/* Define attributes that are mutually exclusive with one another. */
>> +static const struct attribute_spec::exclusions attr_noreturn_exclusions[] =
>> +{
>> + ATTR_EXCL ("noreturn", true, true, true),
>
> Why does noreturn exclude itself? Probably worth a comment if deliberate.
>
That does seem wrong indeed, so I'll remove it.
>> +static tree
>> +handle_const_attribute (tree *node, tree ARG_UNUSED (name),
>> + tree ARG_UNUSED (args), int ARG_UNUSED (flags),
>> + bool * ARG_UNUSED (no_add_attrs))
>> +{
>> + tree type = TREE_TYPE (*node);
>> +
>> + /* See FIXME comment on noreturn in c_common_attribute_table. */
>
> Seems strange to be referencing c-family code here, especially since
> there's no corresponding comment for handle_noreturn_attribute and
> also no FIXME comment in the D version of the table. Think we should
> just drop the comment above.
>
> It's unfortunate that there's so much cut-&-paste with c-attribs.c,
> but there again, the current split between target-independent code
> (attribs.*) and frontend code (c-attribs.*) isn't very clear-cut,
> so I don't think this should hold up acceptance.
>
Attributes handlers present only for built-in functions would have
been initially copied from LTO if memory serves right.
> I'm going to review it as new code though, so:
>
All suggestions seem reasonable to me.
>> + /* For now, we need to tell the D frontend what platform is being
>> targeted.
>> + This should be removed once the frontend has been fixed. */
>> + if (strcmp (ident, "linux") == 0)
>> +global.params.isLinux = true;
>> + else if (strcmp (ident, "OSX") == 0)
>> +global.params.isOSX = true;
>> + else if (strcmp (ident, "Windows") == 0)
>> +global.params.isWindows = true;
>> + else if (strcmp (ident, "FreeBSD") == 0)
>> +global.params.isFreeBSD = true;
>> + else if (strcmp (ident, "OpenBSD") == 0)
>> +global.params.isOpenBSD = true;
>> + else if (strcmp (ident, "Solaris") == 0)
>> +global.params.isSolaris = true;
>> + else if (strcmp (ident, "X86_64") == 0)
>> +global.params.is64bit = true;
>
> Probably worth adding a comment to say why only
Extend usage of C++11 direct init in __debug::vector
This patch extend usage of C++11 direct initialization in
__debug::vector and makes some calls to operator - more consistent.
Note that I also rewrote following expression in erase method:
- return begin() + (__first.base() - cbegin().base());
+ return { _Base::begin() + (__first.base() - _Base::cbegin()), this };
The latter version was building 2 safe iterators and incrementing 1 with
the additional debug check inherent to such an operation whereas the new
version just build 1 safe iterator with directly the expected offset.
2018-10-15 François Dumont
* include/debug/vector (vector<>::cbegin()): Use C++11 direct
initialization.
(vector<>::cend()): Likewise.
(vector<>::emplace(const_iterator, _Args&&...)): Likewise and use
consistent iterator comparison.
(vector<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(vector<>::insert(const_iterator, _InputIterator, _InputIterator)):
Likewise.
(vector<>::erase(const_iterator)): Likewise.
(vector<>::erase(const_iterator, const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index ff9f5f47c24..c11ddbb7048 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -328,11 +328,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return const_iterator(_Base::begin(), this); }
+ { return { _Base::begin(), this }; }
const_iterator
cend() const noexcept
- { return const_iterator(_Base::end(), this); }
+ { return { _Base::end(), this }; }
const_reverse_iterator
crbegin() const noexcept
@@ -521,7 +521,7 @@ namespace __debug
{
__glibcxx_check_insert(__position);
bool __realloc = this->_M_requires_reallocation(this->size() + 1);
- difference_type __offset = __position.base() - _Base::begin();
+ difference_type __offset = __position.base() - _Base::cbegin();
_Base_iterator __res = _Base::emplace(__position.base(),
std::forward<_Args>(__args)...);
if (__realloc)
@@ -529,7 +529,7 @@ namespace __debug
else
this->_M_invalidate_after_nth(__offset);
this->_M_update_guaranteed_capacity();
- return iterator(__res, this);
+ return { __res, this };
}
#endif
@@ -542,7 +542,8 @@ namespace __debug
{
__glibcxx_check_insert(__position);
bool __realloc = this->_M_requires_reallocation(this->size() + 1);
- difference_type __offset = __position.base() - _Base::begin();
+ difference_type __offset
+ = __position.base() - __position._M_get_sequence()->_M_base().begin();
_Base_iterator __res = _Base::insert(__position.base(), __x);
if (__realloc)
this->_M_invalidate_all();
@@ -577,7 +578,7 @@ namespace __debug
else
this->_M_invalidate_after_nth(__offset);
this->_M_update_guaranteed_capacity();
- return iterator(__res, this);
+ return { __res, this };
}
#else
void
@@ -623,7 +624,7 @@ namespace __debug
else
this->_M_invalidate_after_nth(__offset);
this->_M_update_guaranteed_capacity();
- return iterator(__res, this);
+ return { __res, this };
}
#else
template
@@ -661,7 +662,8 @@ namespace __debug
#endif
{
__glibcxx_check_erase(__position);
- difference_type __offset = __position.base() - _Base::begin();
+ difference_type __offset
+ = __position.base() - __position._M_get_sequence()->_M_base().begin();
_Base_iterator __res = _Base::erase(__position.base());
this->_M_invalidate_after_nth(__offset);
return iterator(__res, this);
@@ -680,7 +682,8 @@ namespace __debug
if (__first.base() != __last.base())
{
- difference_type __offset = __first.base() - _Base::begin();
+ difference_type __offset =
+ __first.base() - __first._M_get_sequence()->_M_base().begin();
_Base_iterator __res = _Base::erase(__first.base(),
__last.base());
this->_M_invalidate_after_nth(__offset);
@@ -688,7 +691,7 @@ namespace __debug
}
else
#if __cplusplus >= 201103L
- return begin() + (__first.base() - cbegin().base());
+ return { _Base::begin() + (__first.base() - _Base::cbegin()), this };
#else
return __first;
#endif
