Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI
On 2020-01-16, Richard Sandiford wrote: Szabolcs Nagy writes: this affects the linux kernel and technically a wrong code bug so this fix tries to be backportable (fixing all issues with -fpatchable-function-entry=N,M will likely require new option). Even for the backportable version, I think it would be better not to duplicate so much varasm stuff. Perhaps instead we could: (a) set a cfun->machine flag in aarch64_declare_function_name to say that we've assembled the label (b) override print_patchable_function_entry so that it prints a BTI if this flag is set and the usual other conditions are true As discussed off-list, it'd be good to avoid a second BTI after the nops if possible. print_patchable_function_entry should be able to find the first instruction using get_insns and next_real_insn, then remove it if it turns out to be a BTI. Thanks, Richard It'd be great to have some tests, e.g. 1. -fpatchable-function-entry=0 -mbranch-protection=bti 2. -fpatchable-function-entry=2 -mbranch-protection=bti I have updated clang to emit `.Lfunc_begin0: bti c; nop; nop` for case 2. The __patchable_function_entries entry points to .Lfunc_begin0 (bti c). (The change is not included in the llvm 10.0 branch.) I think we can address all except the SHF_WRITE issue in a backward compatible manner. For some items listed in https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html , the fixes require GNU as (https://sourceware.org/bugzilla/show_bug.cgi?id=25380 https://sourceware.org/bugzilla/show_bug.cgi?id=25381) and ld features (https://sourceware.org/ml/binutils/2019-11/msg00266.html). gcc/ChangeLog: 2020-01-16 Szabolcs Nagy PR target/92424 * config/aarch64/aarch64.c (aarch64_declare_function_name): Emit BTI c if the function label is followed by a patch area. From ac2a46bab60ecc80a453328b4749a951908c02c5 Mon Sep 17 00:00:00 2001 From: Szabolcs Nagy Date: Wed, 15 Jan 2020 12:23:40 + Subject: [PATCH] [AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI This is a minimal workaround that emits another BTI after the function label if that is followed by a patch area. So before this commit -fpatchable-function-entry=3,1 with bti generates .section __patchable_function_entries .8byte .LPFE .text .LPFE: nop foo: nop nop bti c // or paciasp ... and after this commit .section __patchable_function_entries .8byte .LPFE .text .LPFE: nop foo: bti c nop nop bti c // or paciasp ... There is a new bti insn in the middle of the patchable area unless M=0 (patch area is after the new bti) or M=N (patch area is before the label, no new bti). Note that .cfi_startproc and the asynchronous unwind table entry label comes after the patch area, but whatever effect that has on the newly inserted bti c, it already affected the insns in the patch area. Tested on aarch64-none-linux-gnu. gcc/ChangeLog: PR target/92424 * config/aarch64/aarch64.c (aarch64_declare_function_name): Emit BTI c if the function label is followed by a patch area. --- gcc/config/aarch64/aarch64.c | 33 + 1 file changed, 33 insertions(+) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 66e20becaf2..0394c274330 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -18079,6 +18079,39 @@ aarch64_declare_function_name (FILE *stream, const char* name, /* Don't forget the type directive for ELF. */ ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function"); ASM_OUTPUT_LABEL (stream, name); + + if (!aarch64_bti_enabled () + || cgraph_node::get (fndecl)->only_called_directly_p ()) +return; + + /* Copy logic from varasm.c assemble_start_function + to handle -fpatchable-function-entry=N,M with BTI. */ + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; + + tree patchable_function_entry_attr += lookup_attribute ("patchable_function_entry", DECL_ATTRIBUTES (fndecl)); + if (patchable_function_entry_attr) +{ + tree pp_val = TREE_VALUE (patchable_function_entry_attr); + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); + + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); + patch_area_entry = 0; + if (TREE_CHAIN (pp_val) != NULL_TREE) + { + tree patchable_function_entry_value2 + = TREE_VALUE (TREE_CHAIN (pp_val)); + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); + } +} + + if (patch_area_entry > patch_area_size) +patch_area_entry = 0; + + /* Emit a BTI c if a patch area comes after the function label. */ + if (patch_area_size > patch_area_entry) +asm_fprintf (stream, "\thint\t34 // bti c\n"); } /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
[PATCH] Remove assertion in get_info_about_necessary_edges (PR ipa/93166)
Bootstrapped/regtested on x86_64-linux and aarch64-linux. Feng --- 2020-01-19 Feng Xue PR ipa/93166 * ipa-cp.c (get_info_about_necessary_edges): Remove value check assertion.From 02e4bea314a0ca0a8befb85c64efcfe422d35cb8 Mon Sep 17 00:00:00 2001 From: Feng Xue Date: Sun, 19 Jan 2020 15:49:44 +0800 Subject: [PATCH] remove assert in get_info_about_necessary_edges --- gcc/ipa-cp.c | 3 - gcc/testsuite/g++.dg/pr93166.C | 208 + 2 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr93166.C diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 17da1d8e8a7..e762abb896d 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -4175,9 +4175,6 @@ get_info_about_necessary_edges (ipcp_value *val, cgraph_node *dest, hot |= cs->maybe_hot_p (); if (cs->caller != dest) non_self_recursive = true; - else if (src->val) - gcc_assert (values_equal_for_ipcp_p (src->val->value, - val->value)); } cs = get_next_cgraph_edge_clone (cs); } diff --git a/gcc/testsuite/g++.dg/pr93166.C b/gcc/testsuite/g++.dg/pr93166.C new file mode 100644 index 000..e9234ce7a0c --- /dev/null +++ b/gcc/testsuite/g++.dg/pr93166.C @@ -0,0 +1,208 @@ +// { dg-do compile } +// { dg-options "-shared -flto -O2 -fPIC -fvisibility=hidden" } + +namespace Qt { +enum DropAction {}; +} +class QObject; +struct QMetaObject { + static void activate(const QMetaObject *, void *); + enum Call {}; + struct { +const QMetaObject *superdata; +int *stringdata; +unsigned *data; +typedef void (*StaticMetacallFunction)(QObject *, Call, int, void **); +StaticMetacallFunction static_metacallrelatedMetaObjectsextradata; + } d; +}; +class QString; +struct QListData { + struct Data; + Data *d; +}; +template class QList { + union { +QListData p; +QListData::Data *d; + }; + +public: + ~QList(); +}; +class QStringList : QList {}; +template struct QScopedPointerDeleter; +class QObjectData; +template > +class QScopedPointer { +public: + ~QScopedPointer(); + QObjectData *d; +}; + +template struct FunctionPointer; +template +struct FunctionPointer { + typedef Obj Object; +}; + +class QObject { +public: + virtual ~QObject(); + virtual void disconnectNotify(); + template + void connect(typename FunctionPointer::Object *, Func1, + typename FunctionPointer::Object *, Func2); + QScopedPointer d_ptr; +}; +class QPaintDevicePrivate; +class QPaintDevice { +public: + virtual ~QPaintDevice(); + unsigned short painters; + QPaintDevicePrivate *reserved; +}; +class QWidgetData; +class QWidget : public QObject, QPaintDevice { + QWidgetData *data; +}; +class QFrame : public QWidget {}; +class QMenu; +class QMimeData; +class QAbstractScrollArea : public QFrame {}; +class QAbstractItemView : public QAbstractScrollArea {}; +class QTreeView : public QAbstractItemView {}; +class QTreeWidgetItem; +class QTreeWidget : public QTreeView {}; +class QSignalMapper; +class KActionCollection; +class MenuFile; +class MenuFolderInfo; +class MenuEntryInfo; +class MenuSeparatorInfo; +class TreeView : QTreeWidget { +public: + static const QMetaObject d; + static void qt_static_metacall(QObject *, QMetaObject::Call, int, void **); + void disableAction(); + bool dropMimeData(QTreeWidgetItem *, int, const QMimeData *, Qt::DropAction); + KActionCollection *m_ac; + QMenu *m_popupMenu; + int m_clipboard; + MenuFolderInfo *m_clipboardFolderInfo; + MenuEntryInfo *m_clipboardEntryInfo; + bool m_showHidden; + MenuFile *m_menuFile; + MenuFolderInfo *m_rootFolder; + MenuSeparatorInfo *m_separator; + QStringList m_newMenuIds; + QStringList m_newDirectoryList; + bool m_layoutDirty; + bool m_detailedMenuEntries; + bool m_detailedEntriesNamesFirst; + QStringList m_dropMimeTypes; + QSignalMapper *m_sortSignalMapper; +}; +struct { + int data[]; +} b; +unsigned c[]{}; +void TreeView::qt_static_metacall(QObject *p1, QMetaObject::Call, int, + void **p4) { + static_cast(p1)->dropMimeData( + 0, 0, 0, *reinterpret_cast(p4)); +} +const QMetaObject TreeView::d{&d, b.data, c, qt_static_metacall}; +void TreeView::disableAction() { QMetaObject::activate(&d, nullptr); } +template struct QScopedPointerDeleter; +class KXMLGUIClientPrivate; +class KXMLGUIClient { +public: + virtual void m_fn2(); + KXMLGUIClient(); + virtual ~KXMLGUIClient(); + KXMLGUIClientPrivate *const d; +}; +class KXMLGUIBuilderPrivate; +class KXMLGUIBuilder { +public: + virtual ~KXMLGUIBuilder(); + virtual QStringList customTags(); + KXMLGUIBuilderPrivate *const d; +}; + +class QMainWindow : public QWidget {}; +class KMainWindowPrivate; +class KMainWindow : public QMainWindow { + KMainWindowPrivate *const k_ptr; +}; +class KXmlGuiWindow : public KMainWindow, + KXMLGUIBuilder, + virtual KXMLGUIClient { +public: + KXmlGuiWindow(); +}; +class QSplitter
Fix two issues with multi-target speculative calls
Hi, this fixes two issues with the new multi-target speculation code which reproduce on Firefox. I can now build firefox with FDO locally but on Mozilla build bots it still fails with ICE in speculative_call_info. One problem is that speuclative code compares call_stmt and lto_stmt_uid in a way that may get unwanted effect when these gets out of sync. It does not make sense to have both non-zero so I added code clearing it and sanity check that it is kept this way. Other problem is cgraph_edge::make_direct not working well with multiple targets. In this case it removed one speuclative target and the indirect call leaving other targets in the tree. This is fixed by iterating across all targets and removing all except the good one (if it exists). I will see if I can reproduce the other issue. lto-bootstrapped/regtested x86_64-linux. I plan to commit it after bit of extra testing. 2020-01-19 Jan Hubicka PR lto/93318 * cgraph.c (cgraph_edge::resolve_speculation): Fix foramting. (cgraph_edge::make_direct): Remove all indirect targets. (cgraph_edge::redirect_call_stmt_to_callee): Use make_direct.. (cgraph_node::verify_node): Verify that only one call_stmt or lto_stmt_uid is set. * cgraphclones.c (cgraph_edge::clone): Set only one call_stmt or lto_stmt_uid. * lto-cgraph.c (lto_output_edge): Simplify streaming of stmt. (lto_output_ref): Simplify streaming of stmt. * lto-streamer-in.c (fixup_call_stmt_edges_1): Clear lto_stmt_uid. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 95b523d6be5..5fe2178e334 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1226,8 +1226,8 @@ cgraph_edge::resolve_speculation (cgraph_edge *edge, tree callee_decl) fprintf (dump_file, "Speculative call turned into direct call.\n"); edge = e2; e2 = tmp; - /* FIXME: If EDGE is inlined, we should scale up the frequencies and counts - in the functions inlined through it. */ + /* FIXME: If EDGE is inlined, we should scale up the frequencies +and counts in the functions inlined through it. */ } edge->count += e2->count; if (edge->num_speculative_call_targets_p ()) @@ -1263,11 +1263,52 @@ cgraph_edge::make_direct (cgraph_edge *edge, cgraph_node *callee) /* If we are redirecting speculative call, make it non-speculative. */ if (edge->speculative) { - edge = resolve_speculation (edge, callee->decl); + cgraph_edge *found = NULL; + cgraph_edge *direct, *next; + ipa_ref *ref; + + edge->speculative_call_info (direct, edge, ref); - /* On successful speculation just return the pre existing direct edge. */ - if (!edge->indirect_unknown_callee) -return edge; + /* Look all speculative targets and remove all but one corresponding +to callee (if it exists). +If there is only one target we can save one extra call to +speculative_call_info. */ + if (edge->num_speculative_call_targets_p () != 1) + for (direct = edge->caller->callees; direct; direct = next) + { + next = direct->next_callee; + if (direct->call_stmt == edge->call_stmt + && direct->lto_stmt_uid == edge->lto_stmt_uid) + { + direct->speculative_call_info (direct, edge, ref); + + /* Compare ref not direct->callee. Direct edge is possibly + inlined or redirected. */ + if (!ref->referred->semantically_equivalent_p (callee)) + edge = direct->resolve_speculation (direct, NULL); + else + { + gcc_checking_assert (!found); + found = direct; + } + } + } + else if (!ref->referred->semantically_equivalent_p (callee)) + edge = direct->resolve_speculation (direct, NULL); + else + found = direct; + + /* On successful speculation just remove the indirect edge and +return the pre existing direct edge. +It is important to not remove it and redirect because the direct +edge may be inlined or redirected. */ + if (found) + { + resolve_speculation (edge, callee->decl); + gcc_checking_assert (!found->speculative); + return found; + } + gcc_checking_assert (edge->speculative); } edge->indirect_unknown_callee = 0; @@ -1328,7 +1369,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) /* If there already is an direct call (i.e. as a result of inliner's substitution), forget about speculating. */ if (decl) - e = resolve_speculation (e, decl); + e = make_direct (e, cgraph_node::get (decl)); else { /* Expand speculation into GIMPLE code. */ @@ -3116,6 +3157,8 @@ cgraph_node::verify_node (void) basic_block this_block; gimple_stmt_iterator gsi; bool er
[wwwdocs] Remove specifics shared by all of GCC from the main Fortran page.
This was triggered by a reference to SVN and dates back to the very early days for GFortran when it was more of a separate projects. Pushed. Gerald --- htdocs/fortran/index.html | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/htdocs/fortran/index.html b/htdocs/fortran/index.html index 7b6aa805..b7a71de2 100644 --- a/htdocs/fortran/index.html +++ b/htdocs/fortran/index.html @@ -85,11 +85,8 @@ of the front end and run-time library development. We encourage everyone to contribute changes and help test GNU Fortran. GNU Fortran is developed on the mainline of GCC and has been part of the compiler collection -since the 4.0.0 release. We provide read access to our development -sources for everybody by anonymous SVN. If -you do not have SVN access (for instance if you are behind a -firewall prohibiting the SVN protocol), you might want to download -snapshots. +since the 4.0.0 release. + Contributions will be reviewed by at least one of the following people: -- 2.24.1
Re: [PATCH] Remove assertion in get_info_about_necessary_edges (PR ipa/93166)
> Bootstrapped/regtested on x86_64-linux and aarch64-linux. > > Feng > --- > 2020-01-19 Feng Xue > > PR ipa/93166 > * ipa-cp.c (get_info_about_necessary_edges): Remove value > check assertion. OK. Please next time write short description on the problem in email so one does not need to look up the PR log. Honza
Re: Fix two issues with multi-target speculative calls
> Hi, > this fixes two issues with the new multi-target speculation code which > reproduce > on Firefox. I can now build firefox with FDO locally but on Mozilla build > bots > it still fails with ICE in speculative_call_info. > > One problem is that speuclative code compares call_stmt and lto_stmt_uid in > a way that may get unwanted effect when these gets out of sync. It does not > make sense to have both non-zero so I added code clearing it and sanity check > that it is kept this way. > > Other problem is cgraph_edge::make_direct not working well with multiple > targets. In this case it removed one speuclative target and the indirect call > leaving other targets in the tree. > > This is fixed by iterating across all targets and removing all except the good > one (if it exists). > > I will see if I can reproduce the other issue. > > lto-bootstrapped/regtested x86_64-linux. I plan to commit it after bit of > extra > testing. > > 2020-01-19 Jan Hubicka > > PR lto/93318 > * cgraph.c (cgraph_edge::resolve_speculation): Fix foramting. > (cgraph_edge::make_direct): Remove all indirect targets. > (cgraph_edge::redirect_call_stmt_to_callee): Use make_direct.. > (cgraph_node::verify_node): Verify that only one call_stmt or > lto_stmt_uid is set. > * cgraphclones.c (cgraph_edge::clone): Set only one call_stmt or > lto_stmt_uid. > * lto-cgraph.c (lto_output_edge): Simplify streaming of stmt. > (lto_output_ref): Simplify streaming of stmt. > * lto-streamer-in.c (fixup_call_stmt_edges_1): Clear lto_stmt_uid. Hi, this is variant of patch I comitted. There was one wrong sanity check triggering ICE when speculation was resolved to different target then predicted. PR lto/93318 * cgraph.c (cgraph_edge::resolve_speculation): Fix foramting. (cgraph_edge::make_direct): Remove all indirect targets. (cgraph_edge::redirect_call_stmt_to_callee): Use make_direct.. (cgraph_node::verify_node): Verify that only one call_stmt or lto_stmt_uid is set. * cgraphclones.c (cgraph_edge::clone): Set only one call_stmt or lto_stmt_uid. * lto-cgraph.c (lto_output_edge): Simplify streaming of stmt. (lto_output_ref): Simplify streaming of stmt. * lto-streamer-in.c (fixup_call_stmt_edges_1): Clear lto_stmt_uid. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 95b523d6be5..c442f334fe2 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1226,8 +1226,8 @@ cgraph_edge::resolve_speculation (cgraph_edge *edge, tree callee_decl) fprintf (dump_file, "Speculative call turned into direct call.\n"); edge = e2; e2 = tmp; - /* FIXME: If EDGE is inlined, we should scale up the frequencies and counts - in the functions inlined through it. */ + /* FIXME: If EDGE is inlined, we should scale up the frequencies +and counts in the functions inlined through it. */ } edge->count += e2->count; if (edge->num_speculative_call_targets_p ()) @@ -1263,11 +1263,52 @@ cgraph_edge::make_direct (cgraph_edge *edge, cgraph_node *callee) /* If we are redirecting speculative call, make it non-speculative. */ if (edge->speculative) { - edge = resolve_speculation (edge, callee->decl); + cgraph_edge *found = NULL; + cgraph_edge *direct, *next; + ipa_ref *ref; + + edge->speculative_call_info (direct, edge, ref); - /* On successful speculation just return the pre existing direct edge. */ - if (!edge->indirect_unknown_callee) -return edge; + /* Look all speculative targets and remove all but one corresponding +to callee (if it exists). +If there is only one target we can save one extra call to +speculative_call_info. */ + if (edge->num_speculative_call_targets_p () != 1) + for (direct = edge->caller->callees; direct; direct = next) + { + next = direct->next_callee; + if (direct->call_stmt == edge->call_stmt + && direct->lto_stmt_uid == edge->lto_stmt_uid) + { + direct->speculative_call_info (direct, edge, ref); + + /* Compare ref not direct->callee. Direct edge is possibly + inlined or redirected. */ + if (!ref->referred->semantically_equivalent_p (callee)) + edge = direct->resolve_speculation (direct, NULL); + else + { + gcc_checking_assert (!found); + found = direct; + } + } + } + else if (!ref->referred->semantically_equivalent_p (callee)) + edge = direct->resolve_speculation (direct, NULL); + else + found = direct; + + /* On successful speculation just remove the indirect edge and +return the pre existing direct edge. +It is important to not remove it and redirect because the di
[wwwdocs] Adjustments of "regression hunting" instructions to the post-SVN world.
With Git a clone carries the whole repository, so remove instructions on obtaining a local copy of the repository and related instructions on SVN usage. On the way remove a web link for the contrib/reghunt scripts since those are in the repository anyway. Pushed. Gerald --- htdocs/bugs/reghunt.html | 54 +++- 1 file changed, 4 insertions(+), 50 deletions(-) diff --git a/htdocs/bugs/reghunt.html b/htdocs/bugs/reghunt.html index d9c92067..48d7d241 100644 --- a/htdocs/bugs/reghunt.html +++ b/htdocs/bugs/reghunt.html @@ -54,9 +54,8 @@ while the range is too large to investigate by hand: The first three steps are described below. They can be automated, -as can the framework for the binary search. The directory https://gcc.gnu.org/svn/gcc/trunk/contrib/reghunt/";> -contrib/reghunt in the GCC repository includes +as can the framework for the binary search. The directory +contrib/reghunt in the GCC repository includes scripts to do this work. There are several short cuts @@ -70,58 +69,13 @@ are simple to work around. Get GCC sources -Get a Local Copy of the GCC repository - -Using rsync to get a local copy of the GCC -repository is highly recommended for regression hunts. You'll -be checking out the tree used for the regression search over and over -again and won't want to affect access times for other GCC developers -who are using the real repository, and it will also be faster for -you. - -The full tree takes a lot of disk space, but it's possible to -exclude directories you won't need for your hunt. If you're already -using a local SVN repository via rsync, -you can make a cut-down version of it that leaves out directories you -don't need for the regression hunt. This makes SVN operations much -quicker, making it worthwhile even if the copy is on the same system. -It's particularly useful if you'll want to copy it to a system that is -low on available disk space. The following, for example, makes a -smaller copy of the repository that can be used for finding C and C++ -compile-time problems and takes only half the disk space as the full -repository. - - -cat <rsync_exclude ---exclude=gcc-svn/benchmarks ---exclude=gcc-svn/boehm-gcc ---exclude=gcc-svn/old-gcc ---exclude=gcc-svn/wwwdocs ---exclude=gcc-svn/gcc/libstdc++-v3 ---exclude=gcc-svn/gcc/gcc/ada ---exclude=gcc-svn/gcc/gcc/testsuite -EOF - -tar `cat rsync_exclude` -cf - gcc-svn | gzip > gcc-svn.tar.gz - - - Check Out a Working Copy -Check out a local working copy of -GCC from your local repository. If you are not using a local -repository, then check out a working copy using anonymous read-only SVN access. In any case, -use a new working copy that is separate from what you use for +Check out a working copy using Git. +Use a new working copy that is separate from what you use for development or other testing, since it is easy to end up with files in strange states. - Information about checking out specific dates, working with branches and tags, and -inspecting the commit logs is available at the https://gcc.gnu.org/wiki/SvnHelp";>SVN Help pages in the GCC -Wiki. - Branch and release dates -- 2.24.1
Re: [wwwdocs] Remove the last reference to svnwrite.html and retire it.
On Fri, 17 Jan 2020, Gerald Pfeifer wrote: > One step^Wfile at a time; next in line: svn.html. Let's make sure old links to svnwrite.html (externally or from our own archives, say) redirect. Pushed. Gerald - Log - commit e48a9f9ec49f5e3e50e5e614bcfecb9543747441 Author: Gerald Pfeifer Date: Sun Jan 19 14:09:23 2020 +0100 With svnwrite.html retired, redirect to gitwrite.html. diff --git a/htdocs/.htaccess b/htdocs/.htaccess index a28af129..e80d14e4 100644 --- a/htdocs/.htaccess +++ b/htdocs/.htaccess @@ -67,6 +67,7 @@ Redirect permanent /projects.html https://gcc.gnu.org/projects/ Redirect permanent /projects/cxx1z.html https://gcc.gnu.org/projects/cxx-status.html#cxx1z Redirect permanent /projects/web.html https://gcc.gnu.org/about.html Redirect permanent /reghunt-howto.html https://gcc.gnu.org/bugs/reghunt.html +Redirect permanent /svnwrite.html https://gcc.gnu.org/gitwrite.html Redirect permanent /thanks.html https://gcc.gnu.org/onlinedocs/gcc/Contributors.html Redirect permanent /timeline.html https://gcc.gnu.org/releases.html#timeline Redirect permanent /web.html https://gcc.gnu.org/about.html
[PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
To add x32 support to -mtls-dialect=gnu2, we need to replace DI with P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in GNU2 TLS address computation must be done in ptr_mode to support -maddress-mode=long. Also drop the "q" suffix from lea to support both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". Tested on Linux/x86-64. OK for master? Thanks. H.J. --- gcc/ PR target/93319 * config/i386/i386.c (legitimize_tls_address): Pass Pmode to gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... (@tls_dynamic_gnu2_64_): This. Replace DI with P. (*tls_dynamic_gnu2_lea_64): Renamed to ... (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. Remove the {q} suffix from lea. (*tls_dynamic_gnu2_call_64): Renamed to ... (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. (*tls_dynamic_gnu2_combine_64): Renamed to ... (*tls_dynamic_gnu2_combine_64_): This. Replace DI with P. Pass Pmode to gen_tls_dynamic_gnu2_64. gcc/testsuite/ PR target/93319 * gcc.target/i386/pr93319-1a.c: New test. * gcc.target/i386/pr93319-1b.c: Likewise. * gcc.target/i386/pr93319-1c.c: Likewise. * gcc.target/i386/pr93319-1d.c: Likewise. --- gcc/config/i386/i386.c | 31 +++-- gcc/config/i386/i386.md| 54 +++--- gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ 6 files changed, 99 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2c087a4a3e0..8c437dbe1f3 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); else emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); tp = get_thread_pointer (Pmode, true); - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); + + /* NB: Since thread pointer is in ptr_mode, make sure that +PLUS is done in ptr_mode. */ + if (Pmode != ptr_mode) + { + tp = lowpart_subreg (ptr_mode, tp, Pmode); + dest = lowpart_subreg (ptr_mode, dest, Pmode); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); + } + else + dest = gen_rtx_PLUS (Pmode, tp, dest); + dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) x = gen_rtx_ZERO_EXTEND (Pmode, x); @@ -10821,7 +10833,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) rtx tmp = ix86_tls_module_base (); if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (base, tmp)); + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); else emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic)); @@ -10864,7 +10876,18 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, dest, tp)); + /* NB: Since thread pointer is in ptr_mode, make sure that +PLUS is done in ptr_mode. */ + if (Pmode != ptr_mode) + { + tp = lowpart_subreg (ptr_mode, tp, Pmode); + dest = lowpart_subreg (ptr_mode, dest, Pmode); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); + } + else + dest = gen_rtx_PLUS (Pmode, tp, dest); + dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) x = gen_rtx_ZERO_EXTEND (Pmode, x); diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index c9d2f338fe9..d53684096c4 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -15185,14 +15185,14 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_32" emit_insn (gen_tls_dynamic_gnu2_32 (operands[5], operands[1], operands[2])); }) -(define_expand "tls_dynamic_gnu2_64" +(define_expand "@tls_dynamic_gnu2_64_" [(set (match_dup 2) - (unspec:DI [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_TLSDESC)) + (unspec:P
Re: [wwwdocs] Updates to contribute.html for git-friendly posting rules
Hi Richard, On Thu, 9 Jan 2020, Richard Earnshaw (lists) wrote: > The thread on gcc@ is now so long and complicated that this proposal > back at the start has dropped off the radar. With the switch now > imminent I'd like to re-propose this change, this time more formally. I wasn't sure *who* would best approve this since it's more a policy question than anything else, but let's unstall this... There's a general note I'd make, partly based on my going through some of our older web contents recently (and "decluttering" some of it), and some specific feedback. The general note is that we've had a tendency to be very specific in some of our policies (see the last hunk of https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01064.html for an example) which can make appear us not very inviting to new blood. That is, if all I wanted is to submit a simple patch for a typo somewhere, how would I feel about our set of instructions, and now this addition? Is there a way to make this more light weight or less complex/ optional for simple contributions? +Email subject lines If I interpret both Merriam-Webster and the OED correctly, "e-mail" is the preferrable spelling? +Your contribution email subject line will become the first line of the +commit message for your patch. ... around paragraphs (throughout). +Classifier + +The classifier identifies the type of contribution, for example a +patch, an RFC (request for comments) or a committed patch (where +approval is not necessary. The classifier should be written in upper +case and surrounded with square brackets. This is the only component +of the email subject line that will not appear in the commit itself. +The classifier may optionally contain a version number (vN) and +a series marker (N/M). Examples are: + + + [PATCH] - a single patch + [PATCH v2] - the second version of a single patch + [PATCH 3/7] - the third patch in a series of seven +patches + [RFC] - a point of discussion, may contain a patch + [COMMITTED] - a patch that has already been committed. + I see a lot of [C++], [aarch64], [fortran], [wwwdocs] ;-),... in our archives. Should this all really move into the remainder of the subject line/ first line of the commit message? I guess this is a key part change as part of your proposal? +Component tags Alternately we could use [PATCH,fortran], [committed,C++],... ? Actually, if we use PATCH, RFC,... for everything else, could COMMITTED be omitted? That feels like a bit of shouting (so if we keep that, at least make it lower case)? +A component tag is a short identifier that identifies the part of the +compiler being modified, this is important as it highlights to Full stop: "...modified. This is..." or, better "...modified. This highlights..." which is shorter. I believe this could benefit from some examples of overall subject lines. In fact, perhaps start with examples and describe the individual components? As for next steps, can you please mail the (updated) proposal to the gcc@ list? Some, even quite prominent contributors, do not follow gcc-patches at all (or not close) and this is bigger policy question that will be interesting to the broad group. Hope this helps, Gerald
Re: Copy list of development branches from svn.html into git.html
On Thu, 16 Jan 2020, Joseph Myers wrote: > This patch makes a start on making the branch information from > svn.html available in git.html. Thank you, Joseph. That allowed for https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01089.html which was, from what I can tell, the last reference to that part of svn.html. > * At an appropriate point svn.html and svnwrite.html should be > redirected to git.html and gitwrite.html. Until then they should > probably have warnings at the top that the SVN repository is now > read-only and superseded by the Git repository. svnwrite.html already is gone and the web server redirects. There's only few references to svn.html left in our tree which I expect to have resolved today or Monday, so we should be able to add that redirection and remove the old file beginning of next week¹. Gerald ¹ For the continental-European definition of week. :)
New repository location
Question: Is the new gcc git repository at gcc.gnu.org/git/gcc.git using the same location as the earlier git mirror did? I'm curious whether our repository on pike is still syncing with the new master, or whether we need to make some adjustments before we next rebase pu against master.
Re: New repository location
On Sun, Jan 19, 2020 at 6:33 AM Bill Schmidt wrote: > > Question: Is the new gcc git repository at gcc.gnu.org/git/gcc.git > using the same location as the earlier git mirror did? I'm curious > whether our repository on pike is still syncing with the new master, or > whether we need to make some adjustments before we next rebase pu > against master. > 2 repos are different. I renamed my old mirror and created a new one: https://gitlab.com/x86-gcc -- H.J.
Re: New repository location
I apologize, I sent this to the wrong mailing list, this had meant to be internal. But thank you very much for the information! It appears we have some adjustments to make. Thanks! Bill On 1/19/20 8:46 AM, H.J. Lu wrote: On Sun, Jan 19, 2020 at 6:33 AM Bill Schmidt wrote: Question: Is the new gcc git repository at gcc.gnu.org/git/gcc.git using the same location as the earlier git mirror did? I'm curious whether our repository on pike is still syncing with the new master, or whether we need to make some adjustments before we next rebase pu against master. 2 repos are different. I renamed my old mirror and created a new one: https://gitlab.com/x86-gcc
Add speculative call edges verifier and fix one extra bug
Hi, this patch implements verifier and fixes one bug where speculative calls produced by ipa-devirt ended up having num_speculative_call_targets = 0 instead of 1. lto-profilebootstrapped/regtested x86_64-linux, will commit it shortly. Honza PR lto/93318 * cgraph.c (cgraph_edge::make_speculative): Increase number of speculative targets. (verify_speculative_call): New function (cgraph_node::verify_node): Use it. * ipa-profile.c (ipa_profile): Fix formating; do not set number of speculations. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index c442f334fe2..187f6ed30ba 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1076,6 +1076,7 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count, e2->speculative_id = speculative_id; e2->target_prob = target_prob; e2->in_polymorphic_cdtor = in_polymorphic_cdtor; + indirect_info->num_speculative_call_targets++; count -= e2->count; symtab->call_edge_duplication_hooks (this, e2); ref = n->create_reference (n2, IPA_REF_ADDR, call_stmt); @@ -3148,6 +3149,128 @@ cgraph_edge::verify_corresponds_to_fndecl (tree decl) # pragma GCC diagnostic ignored "-Wformat-diag" #endif +/* Verify consistency of speculative call in NODE corresponding to STMT + and LTO_STMT_UID. If INDIRECT is set, assume that it is the indirect + edge of call sequence. Return true if error is found. + + This function is called to every component of indirect call (direct edges, + indirect edge and refs). To save duplicated work, do full testing only + in that case. */ +static bool +verify_speculative_call (struct cgraph_node *node, gimple *stmt, +unsigned int lto_stmt_uid, +struct cgraph_edge *indirect) +{ + if (indirect == NULL) +{ + for (indirect = node->indirect_calls; indirect; + indirect = indirect->next_callee) + if (indirect->call_stmt == stmt + && indirect->lto_stmt_uid == lto_stmt_uid) + break; + if (!indirect) + { + error ("missing indirect call in speculative call sequence"); + return true; + } + if (!indirect->speculative) + { + error ("indirect call in speculative call sequence has no " +"speculative flag"); + return true; + } + return false; +} + + /* Maximal number of targets. We probably will never want to have more than + this. */ + const unsigned int num = 256; + cgraph_edge *direct_calls[num]; + ipa_ref *refs[num]; + + for (unsigned int i = 0; i < num; i++) +{ + direct_calls[i] = NULL; + refs[i] = NULL; +} + + for (cgraph_edge *direct = node->callees; direct; + direct = direct->next_callee) +if (direct->call_stmt == stmt && direct->lto_stmt_uid == lto_stmt_uid) + { + if (!direct->speculative) + { + error ("direct call to %s in speculative call sequence has no " + "speculative flag", direct->callee->dump_name ()); + return true; + } + if (direct->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + if (direct_calls[direct->speculative_id]) + { + error ("duplicate direct call to %s in speculative call sequence " + "with speculative_uid %i", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + direct_calls[direct->speculative_id] = direct; + } + + ipa_ref *ref; + for (int i = 0; node->iterate_reference (i, ref); i++) +if (ref->speculative + && ref->stmt == stmt && ref->lto_stmt_uid == lto_stmt_uid) + { + if (ref->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + if (refs[ref->speculative_id]) + { + error ("duplicate reference %s in speculative call sequence " + "with speculative_uid %i", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + refs[ref->speculative_id] = ref; + } + + int num_targets = 0; + for (unsigned int i = 0 ; i < num ; i++) +{ + if (refs[i] && !direct_calls[i]) + { + error ("missing direct call for speculation %i", i); + return true; + } + if (!refs[i] && direct_calls[i]) + { + error ("missing ref for speculation %i", i); + return true; + } + if (refs[i] != NULL) + num_targets++; +} + + if (num_targets != indirect->num_s
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > GNU2 TLS address computation must be done in ptr_mode to support > -maddress-mode=long. Also drop the "q" suffix from lea to support > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". Please use "lea%z0" instead. > Tested on Linux/x86-64. OK for master? > > Thanks. > > H.J. > --- > gcc/ > > PR target/93319 > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > (*tls_dynamic_gnu2_lea_64): Renamed to ... > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > Remove the {q} suffix from lea. > (*tls_dynamic_gnu2_call_64): Renamed to ... > (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. > (*tls_dynamic_gnu2_combine_64): Renamed to ... > (*tls_dynamic_gnu2_combine_64_): This. Replace DI with P. > Pass Pmode to gen_tls_dynamic_gnu2_64. > > gcc/testsuite/ > > PR target/93319 > * gcc.target/i386/pr93319-1a.c: New test. > * gcc.target/i386/pr93319-1b.c: Likewise. > * gcc.target/i386/pr93319-1c.c: Likewise. > * gcc.target/i386/pr93319-1d.c: Likewise. > --- > gcc/config/i386/i386.c | 31 +++-- > gcc/config/i386/i386.md| 54 +++--- > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > 6 files changed, 99 insertions(+), 31 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2c087a4a3e0..8c437dbe1f3 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model > model, bool for_mov) >if (TARGET_GNU2_TLS) > { > if (TARGET_64BIT) > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > else > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > tp = get_thread_pointer (Pmode, true); > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > + > + /* NB: Since thread pointer is in ptr_mode, make sure that > +PLUS is done in ptr_mode. */ > + if (Pmode != ptr_mode) > + { > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > + } > + else > + dest = gen_rtx_PLUS (Pmode, tp, dest); > + dest = force_reg (Pmode, dest); Sholdn't we use tp = get_thread_pointer (ptr_mode, true); then? If Pmode == ptr_mode, then we have the same functionality, otherwise we don't have to subreg tp to ptr_mode. Can we use the same approach as in ix86_zero_extend_to_Pmode here, like: dest = gen_rtx_PLUS (ptr_mode, tp, dest); dest = force_reg (Pmode, convert_to_mode (Pmode, dest, 1)); Uros. > > if (GET_MODE (x) != Pmode) > x = gen_rtx_ZERO_EXTEND (Pmode, x); > @@ -10821,7 +10833,7 @@ legitimize_tls_address (rtx x, enum tls_model model, > bool for_mov) > rtx tmp = ix86_tls_module_base (); > > if (TARGET_64BIT) > - emit_insn (gen_tls_dynamic_gnu2_64 (base, tmp)); > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); > else > emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic)); > > @@ -10864,7 +10876,18 @@ legitimize_tls_address (rtx x, enum tls_model model, > bool for_mov) > >if (TARGET_GNU2_TLS) > { > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, dest, tp)); > + /* NB: Since thread pointer is in ptr_mode, make sure that > +PLUS is done in ptr_mode. */ > + if (Pmode != ptr_mode) > + { > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > + } > + else > + dest = gen_rtx_PLUS (Pmode, tp, dest); > + dest = force_reg (Pmode
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > GNU2 TLS address computation must be done in ptr_mode to support > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > Please use "lea%z0" instead. > > > Tested on Linux/x86-64. OK for master? > > > > Thanks. > > > > H.J. > > --- > > gcc/ > > > > PR target/93319 > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > > Remove the {q} suffix from lea. > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI with P. > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > gcc/testsuite/ > > > > PR target/93319 > > * gcc.target/i386/pr93319-1a.c: New test. > > * gcc.target/i386/pr93319-1b.c: Likewise. > > * gcc.target/i386/pr93319-1c.c: Likewise. > > * gcc.target/i386/pr93319-1d.c: Likewise. > > --- > > gcc/config/i386/i386.c | 31 +++-- > > gcc/config/i386/i386.md| 54 +++--- > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > 6 files changed, 99 insertions(+), 31 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > index 2c087a4a3e0..8c437dbe1f3 100644 > > --- a/gcc/config/i386/i386.c > > +++ b/gcc/config/i386/i386.c > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model > > model, bool for_mov) > >if (TARGET_GNU2_TLS) > > { > > if (TARGET_64BIT) > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > else > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > tp = get_thread_pointer (Pmode, true); > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > + > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > +PLUS is done in ptr_mode. */ Actually, thread_pointer is in Pmode, see the line just above your change. Also, dest is in Pmode, so why do we need all this subreg dance? Uros. > > + if (Pmode != ptr_mode) > > + { > > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > > + } > > + else > > + dest = gen_rtx_PLUS (Pmode, tp, dest); > > + dest = force_reg (Pmode, dest); > > Sholdn't we use > > tp = get_thread_pointer (ptr_mode, true); > > then? If Pmode == ptr_mode, then we have the same functionality, > otherwise we don't have to subreg tp to ptr_mode. > > Can we use the same approach as in ix86_zero_extend_to_Pmode here, like: > > dest = gen_rtx_PLUS (ptr_mode, tp, dest); > dest = force_reg (Pmode, convert_to_mode (Pmode, dest, 1)); > > Uros.
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > GNU2 TLS address computation must be done in ptr_mode to support > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > Please use "lea%z0" instead. > > > > > Tested on Linux/x86-64. OK for master? > > > > > > Thanks. > > > > > > H.J. > > > --- > > > gcc/ > > > > > > PR target/93319 > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > > > Remove the {q} suffix from lea. > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI with P. > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > gcc/testsuite/ > > > > > > PR target/93319 > > > * gcc.target/i386/pr93319-1a.c: New test. > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > --- > > > gcc/config/i386/i386.c | 31 +++-- > > > gcc/config/i386/i386.md| 54 +++--- > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > --- a/gcc/config/i386/i386.c > > > +++ b/gcc/config/i386/i386.c > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model > > > model, bool for_mov) > > >if (TARGET_GNU2_TLS) > > > { > > > if (TARGET_64BIT) > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > else > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > tp = get_thread_pointer (Pmode, true); > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > + > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > +PLUS is done in ptr_mode. */ > > Actually, thread_pointer is in Pmode, see the line just above your > change. Also, dest is in Pmode, so why do we need all this subreg > dance? dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by call *foo@TLSCALL(%rax) (gdb) bt #0 test () at lib.s:20 #1 0x00401075 in main () at main.c:13 (gdb) f 0 #0 test () at lib.s:20 20 addq %rax, %r12 (gdb) disass Dump of assembler code for function test: 0xf7fca120 <+0>: push %r12 0xf7fca122 <+2>: lea0x2ef7(%rip),%rax# 0xf7fcd020 0xf7fca129 <+9>: lea0xed0(%rip),%rdi# 0xf7fcb000 0xf7fca130 <+16>: mov%fs:0x0,%r12d 0xf7fca139 <+25>: callq *(%rax) => 0xf7fca13b <+27>: add%rax,%r12 ^^ Wrong address in R12. 0xf7fca13e <+30>: xor%eax,%eax 0xf7fca140 <+32>: mov(%r12),%esi 0xf7fca144 <+36>: callq 0xf7fca030 0xf7fca149 <+41>: mov%r12,%rax 0xf7fca14c <+44>: pop%r12 0xf7fca14e <+46>: retq End of assembler dump. (gdb) p/x $rax $6 = 0xfffc (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault. test () at lib.s:22 22 movl (%r12), %esi (gdb) p/x $r12 $7 = 0x1f7df96fc (gdb) > Uros. > > > > + if (Pmode != ptr_mode) > > > + { > > > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > > > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > > > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > > > + } > > > + else > > > + dest = gen_rtx_PLUS (Pmode, tp, dest); > > > +
[patch, fortran] Fix PR 85781, ICE on valid
Hello world, the attached patch fixes an ICE which could occur for empty substrings (see test case). In the spirit of "A patch that works beats an elegant idea every time" I simply set a substring known to be empty to (1:0) so that problems further down the road could be avoided. Regression-tested. OK for trunk? Regards Thomas 2020-01-19 Thomas König PR fortran/85781 * resolve.c (resolve_substring): If the substring is empty, set it to (1:0) explicitly. 2020-01-19 Thomas König PR fortran/85781 * gfortran.dg/substr_9.f90: New test. commit 476a7e195399d2dc76b22947dcbde59f36fe5748 Author: Thomas König Date: Sun Jan 19 19:14:41 2020 +0100 2020-01-19 Thomas König PR fortran/85781 * resolve.c (resolve_substring): If the substring is empty, set it to (1:0) explicitly. 2020-01-19 Thomas König PR fortran/85781 * gfortran.dg/substr_9.f90: New test. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index e840aec62f2..5f644da9bf2 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -5092,6 +5092,19 @@ resolve_substring (gfc_ref *ref, bool *equal_length) && compare_bound (ref->u.ss.end, ref->u.ss.length->length) == CMP_EQ && compare_bound_int (ref->u.ss.start, 1) == CMP_EQ) *equal_length = true; + +} + + /* Set empty substrings to (1:0) to avoid subsequent ICEs. */ + if (gfc_dep_compare_expr (ref->u.ss.start, ref->u.ss.end) == 1) +{ + locus loc; + loc = ref->u.ss.start->where; + gfc_free_expr (ref->u.ss.start); + ref->u.ss.start = gfc_get_int_expr (gfc_index_integer_kind, &loc, 1); + loc = ref->u.ss.end->where; + gfc_free_expr (ref->u.ss.end); + ref->u.ss.end = gfc_get_int_expr (gfc_index_integer_kind, &loc, 0); } return true; diff --git a/gcc/testsuite/gfortran.dg/substr_9.f90 b/gcc/testsuite/gfortran.dg/substr_9.f90 new file mode 100644 index 000..60557d0cc53 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/substr_9.f90 @@ -0,0 +1,7 @@ +! { dg-do compile } +! PR 85781 - this used to cause an ICE. +subroutine s(x) bind(c) + use iso_c_binding, only: c_char + character(kind=c_char), value :: x + print *, x(2:1) +end ! { dg-do compile } ! PR 85781 - this used to cause an ICE. subroutine s(x) bind(c) use iso_c_binding, only: c_char character(kind=c_char), value :: x print *, x(2:1) end
Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI
On 2020-01-19, Fāng-ruì Sòng wrote: On 2020-01-16, Richard Sandiford wrote: Szabolcs Nagy writes: this affects the linux kernel and technically a wrong code bug so this fix tries to be backportable (fixing all issues with -fpatchable-function-entry=N,M will likely require new option). Even for the backportable version, I think it would be better not to duplicate so much varasm stuff. Perhaps instead we could: (a) set a cfun->machine flag in aarch64_declare_function_name to say that we've assembled the label (b) override print_patchable_function_entry so that it prints a BTI if this flag is set and the usual other conditions are true As discussed off-list, it'd be good to avoid a second BTI after the nops if possible. print_patchable_function_entry should be able to find the first instruction using get_insns and next_real_insn, then remove it if it turns out to be a BTI. Thanks, Richard It'd be great to have some tests, e.g. 1. -fpatchable-function-entry=0 -mbranch-protection=bti 2. -fpatchable-function-entry=2 -mbranch-protection=bti I have updated clang to emit `.Lfunc_begin0: bti c; nop; nop` for case 2. The __patchable_function_entries entry points to .Lfunc_begin0 (bti c). (The change is not included in the llvm 10.0 branch.) I think we can address all except the SHF_WRITE issue in a backward compatible manner. For some items listed in https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html , the fixes require GNU as (https://sourceware.org/bugzilla/show_bug.cgi?id=25380 https://sourceware.org/bugzilla/show_bug.cgi?id=25381) and ld features (https://sourceware.org/ml/binutils/2019-11/msg00266.html). Hope someone can take a look at https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00271.html
[C++ PATCH] Simplify lambda parsing.
Since we removed the special parsing for C++11 lambdas, it's just been an open-coded copy of cp_parser_function_body. So let's call it instead. This avoids the need to change this code in my revised 33799 patch. Tested x86_64-pc-linux-gnu, applying to trunk. * parser.c (cp_parser_lambda_body): Use cp_parser_function_body. --- gcc/cp/parser.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 75e32fcebcb..98c1beb400f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -11141,23 +11141,11 @@ cp_parser_lambda_body (cp_parser* parser, tree lambda_expr) local_specialization_stack s (lss_copy); tree fco = lambda_function (lambda_expr); tree body = start_lambda_function (fco, lambda_expr); -matching_braces braces; -if (braces.require_open (parser)) - { - tree compound_stmt = begin_compound_stmt (0); - - /* Originally C++11 required us to peek for 'return expr'; and - process it specially here to deduce the return type. N3638 - removed the need for that. */ - - while (cp_lexer_next_token_is_keyword (parser->lexer, RID_LABEL)) - cp_parser_label_declaration (parser); - cp_parser_statement_seq_opt (parser, NULL_TREE); - braces.require_close (parser); - - finish_compound_stmt (compound_stmt); - } +/* Originally C++11 required us to peek for 'return expr'; and + process it specially here to deduce the return type. N3638 + removed the need for that. */ +cp_parser_function_body (parser, false); finish_lambda_function (body); } base-commit: 52354dadb80b60c3fd05fb1b5aa3feb15a98b8af -- 2.18.1
[C++ PATCH] PR c++/33799 - destroy return value, take 2.
This patch differs from the reverted patch for 33799 in that it adds the CLEANUP_STMT for the return value at the end of the function, and only if we've seen a cleanup that might throw, so it should not affect most C++11 code. Tested x86_64-pc-linux-gnu, applying to trunk. * cp-tree.h (current_retval_sentinel): New macro. (struct language_function): Add throwing_cleanup bitfield. * decl.c (cxx_maybe_build_cleanup): Set it. * except.c (maybe_set_retval_sentinel) (maybe_splice_retval_cleanup): New functions. * parser.c (cp_parser_compound_statement): Call maybe_splice_retval_cleanup. * typeck.c (check_return_expr): Call maybe_set_retval_sentinel. --- gcc/cp/cp-tree.h | 11 + gcc/cp/decl.c | 3 ++ gcc/cp/except.c | 72 +++ gcc/cp/parser.c | 4 ++ gcc/cp/typeck.c | 3 ++ gcc/testsuite/g++.dg/eh/return1.C | 20 - 6 files changed, 112 insertions(+), 1 deletion(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 3d76096b041..890d5a27350 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1904,6 +1904,7 @@ struct GTY(()) language_function { BOOL_BITFIELD can_throw : 1; BOOL_BITFIELD invalid_constexpr : 1; + BOOL_BITFIELD throwing_cleanup : 1; hash_table *x_named_labels; @@ -1954,6 +1955,13 @@ struct GTY(()) language_function { #define current_vtt_parm cp_function_chain->x_vtt_parm +/* A boolean flag to control whether we need to clean up the return value if a + local destructor throws. Only used in functions that return by value a + class with a destructor. Which 'tors don't, so we can use the same + field as current_vtt_parm. */ + +#define current_retval_sentinel current_vtt_parm + /* Set to 0 at beginning of a function definition, set to 1 if a return statement that specifies a return value is seen. */ @@ -6686,6 +6694,9 @@ extern tree begin_eh_spec_block (void); extern void finish_eh_spec_block (tree, tree); extern tree build_eh_type_type (tree); extern tree cp_protect_cleanup_actions (void); +extern void maybe_splice_retval_cleanup(tree); +extern tree maybe_set_retval_sentinel (void); + extern tree template_parms_to_args (tree); extern tree template_parms_level_to_args (tree); extern tree generic_targs_for (tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index e58fecc9de7..28a79029d92 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -17402,6 +17402,9 @@ cxx_maybe_build_cleanup (tree decl, tsubst_flags_t complain) && !mark_used (decl, complain) && !(complain & tf_error)) return error_mark_node; + if (cleanup && cfun && !expr_noexcept_p (cleanup, tf_none)) +cp_function_chain->throwing_cleanup = true; + return cleanup; } diff --git a/gcc/cp/except.c b/gcc/cp/except.c index 55b4b6af442..0b40234e228 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1325,4 +1325,76 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) } } +/* If the current function has a cleanup that might throw, and the return value + has a non-trivial destructor, return a MODIFY_EXPR to set + current_retval_sentinel so that we know that the return value needs to be + destroyed on throw. Otherwise, returns NULL_TREE. */ + +tree +maybe_set_retval_sentinel () +{ + if (processing_template_decl) +return NULL_TREE; + tree retval = DECL_RESULT (current_function_decl); + if (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (retval))) +return NULL_TREE; + if (!cp_function_chain->throwing_cleanup) +return NULL_TREE; + + if (!current_retval_sentinel) +{ + /* Just create the temporary now, maybe_splice_retval_cleanup +will do the rest. */ + current_retval_sentinel = create_temporary_var (boolean_type_node); + DECL_INITIAL (current_retval_sentinel) = boolean_false_node; + pushdecl_outermost_localscope (current_retval_sentinel); +} + + return build2 (MODIFY_EXPR, boolean_type_node, +current_retval_sentinel, boolean_true_node); +} + +/* COMPOUND_STMT is the STATEMENT_LIST for the current function body. If + current_retval_sentinel was set in this function, wrap the body in a + CLEANUP_STMT to destroy the return value on throw. */ + +void +maybe_splice_retval_cleanup (tree compound_stmt) +{ + /* If need_retval_cleanup set current_retval_sentinel, wrap the function body + in a CLEANUP_STMT to handle destroying the return value. */ + if (!DECL_CONSTRUCTOR_P (current_function_decl) + && !DECL_DESTRUCTOR_P (current_function_decl) + && current_retval_sentinel) +{ + location_t loc = DECL_SOURCE_LOCATION (current_function_decl); + + /* Add a DECL_EXPR for current_retval_sentinel. */ + tree_stmt_iterator iter = tsi_start (compound_stmt); +
[committed] testsuite: Make use of effective-target march_option for cris
With march_option in place, here's the rest. (And yes, that cris-linux line in the last context goes away in the patchset putting that target down.) gcc/testsuite: * gcc.dg/torture/pr26515.c (cris*-*-*): Conditionalize -march=v10 option on target ! march_option. * gcc.target/cris/asm-v10.S, gcc.target/cris/inasm-v10.c, gcc.target/cris/sync-1-v10.c: Similar. --- gcc/testsuite/gcc.dg/torture/pr26515.c | 2 +- gcc/testsuite/gcc.target/cris/asm-v10.S| 2 +- gcc/testsuite/gcc.target/cris/inasm-v10.c | 2 +- gcc/testsuite/gcc.target/cris/sync-1-v10.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gcc/testsuite/gcc.dg/torture/pr26515.c b/gcc/testsuite/gcc.dg/torture/pr26515.c index a051e2e..ff765ba 100644 --- a/gcc/testsuite/gcc.dg/torture/pr26515.c +++ b/gcc/testsuite/gcc.dg/torture/pr26515.c @@ -1,4 +1,4 @@ -/* { dg-options "-march=v10" { target cris*-*-* } } */ +/* { dg-options "-march=v10" { target { cris*-*-* && { ! march_option } } } } */ struct i { long long i_size; diff --git a/gcc/testsuite/gcc.target/cris/asm-v10.S b/gcc/testsuite/gcc.target/cris/asm-v10.S index c85ebe2..8bb0c29 100644 --- a/gcc/testsuite/gcc.target/cris/asm-v10.S +++ b/gcc/testsuite/gcc.target/cris/asm-v10.S @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-DOTHER_ISA=10 -march=v10" } */ +/* { dg-options "-DOTHER_ISA=10 -march=v10" { target { ! march_option } } } */ /* Check that -march=v10 is also recognized. */ diff --git a/gcc/testsuite/gcc.target/cris/inasm-v10.c b/gcc/testsuite/gcc.target/cris/inasm-v10.c index 75379b3..774cd03 100644 --- a/gcc/testsuite/gcc.target/cris/inasm-v10.c +++ b/gcc/testsuite/gcc.target/cris/inasm-v10.c @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-DOTHER_ISA=10 -march=v10" } */ +/* { dg-options "-DOTHER_ISA=10 -march=v10" { target { ! march_option } } } */ /* Check that -march=v10 is also recognized. */ diff --git a/gcc/testsuite/gcc.target/cris/sync-1-v10.c b/gcc/testsuite/gcc.target/cris/sync-1-v10.c index 6c8dd1a..861fc8c 100644 --- a/gcc/testsuite/gcc.target/cris/sync-1-v10.c +++ b/gcc/testsuite/gcc.target/cris/sync-1-v10.c @@ -1,5 +1,5 @@ /* Check that we can assemble both base atomic variants. */ /* { dg-do assemble } */ -/* { dg-options "-O2 -march=v10" } */ +/* { dg-options "-O2 -march=v10" { target { ! march_option } } } */ /* { dg-additional-options "-mno-unaligned-atomic-may-use-library" { target cris*-*-linux* } } */ #include "sync-1.c" -- 2.11.0 brgds, H-P
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu wrote: > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > Please use "lea%z0" instead. > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > Thanks. > > > > > > > > H.J. > > > > --- > > > > gcc/ > > > > > > > > PR target/93319 > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > > > > Remove the {q} suffix from lea. > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI with P. > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > gcc/testsuite/ > > > > > > > > PR target/93319 > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > --- > > > > gcc/config/i386/i386.c | 31 +++-- > > > > gcc/config/i386/i386.md| 54 +++--- > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > --- a/gcc/config/i386/i386.c > > > > +++ b/gcc/config/i386/i386.c > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model > > > > model, bool for_mov) > > > >if (TARGET_GNU2_TLS) > > > > { > > > > if (TARGET_64BIT) > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > else > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > + > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > +PLUS is done in ptr_mode. */ > > > > Actually, thread_pointer is in Pmode, see the line just above your > > change. Also, dest is in Pmode, so why do we need all this subreg > > dance? > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > call *foo@TLSCALL(%rax) It looks to me that we have Pmode/ptr_mode mixup in legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we need to perform all calculations in ptr_mode, since get_thread_pointer in effect always returns ptr_mode (SImode) value, and also the above call always returns ptr_mode (SImode) value. In case of -maddress-mode=long, the value would be zero-extended to Pmode. Uros.
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak wrote: > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu wrote: > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > Thanks. > > > > > > > > > > H.J. > > > > > --- > > > > > gcc/ > > > > > > > > > > PR target/93319 > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in > > > > > ptr_mode. > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > > > > > Remove the {q} suffix from lea. > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI with P. > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI with > > > > > P. > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > PR target/93319 > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > --- > > > > > gcc/config/i386/i386.c | 31 +++-- > > > > > gcc/config/i386/i386.md| 54 > > > > > +++--- > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > --- a/gcc/config/i386/i386.c > > > > > +++ b/gcc/config/i386/i386.c > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum > > > > > tls_model model, bool for_mov) > > > > >if (TARGET_GNU2_TLS) > > > > > { > > > > > if (TARGET_64BIT) > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > else > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > + > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > +PLUS is done in ptr_mode. */ > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > dance? > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > call *foo@TLSCALL(%rax) > > It looks to me that we have Pmode/ptr_mode mixup in > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > need to perform all calculations in ptr_mode, since get_thread_pointer > in effect always returns ptr_mode (SImode) value, and also the above > call always returns ptr_mode (SImode) value. In case of > -maddress-mode=long, the value would be zero-extended to Pmode. > The problem is with tls_dynamic_gnu2_64 which generates call *foo@TLSCALL(%rax) It always returns a 32-bit index. I can change get_thread_pointer if needed. Here is the updated patch with "lea%z0" and updated comments. -- H.J. From 6be0be4176337744ba89d7ce0496b7fb3de175bb Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Sat, 18 Jan 2020 10:20:29 -0800 Subject: [PATCH] x32: Add x32 support to -mtls-dialect=gnu2 To add x32 support to -mtls-dialect=gnu2, we need to replace DI with P in GNU2 TLS patt
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu wrote: > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak wrote: > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu wrote: > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS > > > > > > in > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > Thanks. > > > > > > > > > > > > H.J. > > > > > > --- > > > > > > gcc/ > > > > > > > > > > > > PR target/93319 > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in > > > > > > ptr_mode. > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with P. > > > > > > Remove the {q} suffix from lea. > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI with > > > > > > P. > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI > > > > > > with P. > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > PR target/93319 > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > --- > > > > > > gcc/config/i386/i386.c | 31 +++-- > > > > > > gcc/config/i386/i386.md| 54 > > > > > > +++--- > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > --- a/gcc/config/i386/i386.c > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum > > > > > > tls_model model, bool for_mov) > > > > > >if (TARGET_GNU2_TLS) > > > > > > { > > > > > > if (TARGET_64BIT) > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > else > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > + > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > +PLUS is done in ptr_mode. */ > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > dance? > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > call *foo@TLSCALL(%rax) > > > > It looks to me that we have Pmode/ptr_mode mixup in > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > need to perform all calculations in ptr_mode, since get_thread_pointer > > in effect always returns ptr_mode (SImode) value, and also the above > > call always returns ptr_mode (SImode) value. In case of > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > The problem is with tls_dynamic_gnu2_64 which generates > > call *foo@TLSCALL(%rax) > > It always returns a 32-bit index. I can change get_thread_pointer if > needed. > > Here is the updated patch with "lea%z0" and updated comments
[C++ PATCH] PR c++/93324 - ICE with -Wall on constexpr if.
This is a crash with constexpr if, when trying to see if the call in the if-statement is std::is_constant_evaluated. cp_get_callee_fndecl_nofold can return NULL_TREE and fndecl_built_in_p doesn't expect to get a null tree, so check FNDECL first. Bootstrapped/regtested on x86_64-linux, ok for trunk? * semantics.c (is_std_constant_evaluated_p): Check fndecl. * g++.dg/cpp1z/constexpr-if33.C: New test. --- gcc/cp/semantics.c | 4 ++-- gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C | 16 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 3669b247e34..9051a2863e0 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -734,8 +734,8 @@ is_std_constant_evaluated_p (tree fn) return false; tree fndecl = cp_get_callee_fndecl_nofold (fn); - if (fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, -BUILT_IN_FRONTEND)) + if (fndecl && fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, + BUILT_IN_FRONTEND)) return true; if (!decl_in_std_namespace_p (fndecl)) diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C new file mode 100644 index 000..e5ef659932b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C @@ -0,0 +1,16 @@ +// PR c++/93324 - ICE with -Wall on constexpr if. +// { dg-do compile { target c++17 } } +// { dg-options "-Wall" } + +struct { + template + static constexpr bool a() { return 0; } +} e; + +template +void d() +{ + auto c(e); + using b = decltype(c); + if constexpr (b::a<2>()); +} base-commit: bcfc2227c556f2801a657ce3007374732baa8333 -- 2.24.1
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak wrote: > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu wrote: > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak wrote: > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu wrote: > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu wrote: > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI > > > > > > > with > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, > > > > > > > PLUS in > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > H.J. > > > > > > > --- > > > > > > > gcc/ > > > > > > > > > > > > > > PR target/93319 > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode > > > > > > > to > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in > > > > > > > ptr_mode. > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to > > > > > > > ... > > > > > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI with > > > > > > > P. > > > > > > > Remove the {q} suffix from lea. > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI > > > > > > > with P. > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_combine_64_): This. Replace DI > > > > > > > with P. > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > PR target/93319 > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > --- > > > > > > > gcc/config/i386/i386.c | 31 +++-- > > > > > > > gcc/config/i386/i386.md| 54 > > > > > > > +++--- > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum > > > > > > > tls_model model, bool for_mov) > > > > > > >if (TARGET_GNU2_TLS) > > > > > > > { > > > > > > > if (TARGET_64BIT) > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > > else > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, > > > > > > > dest)); > > > > > > > + > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure > > > > > > > that > > > > > > > +PLUS is done in ptr_mode. */ > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > dance? > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > It looks to me that we have Pmode/ptr_mode mixup in > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > > need to perform all calculations in ptr_mode, since get_thread_pointer > > > in effect always returns ptr_mode (SImode) value, and also the above > > > call always returns
Re: [C++ PATCH] PR c++/93324 - ICE with -Wall on constexpr if.
On Sun, Jan 19, 2020 at 03:34:48PM -0500, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > * semantics.c (is_std_constant_evaluated_p): Check fndecl. > > * g++.dg/cpp1z/constexpr-if33.C: New test. > --- > gcc/cp/semantics.c | 4 ++-- > gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C | 16 > 2 files changed, 18 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > index 3669b247e34..9051a2863e0 100644 > --- a/gcc/cp/semantics.c > +++ b/gcc/cp/semantics.c > @@ -734,8 +734,8 @@ is_std_constant_evaluated_p (tree fn) > return false; > >tree fndecl = cp_get_callee_fndecl_nofold (fn); > - if (fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, > - BUILT_IN_FRONTEND)) > + if (fndecl && fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, > +BUILT_IN_FRONTEND)) > return true; > >if (!decl_in_std_namespace_p (fndecl)) Shouldn't it instead do if (fndecl == NULL_TREE) return false; before the fndecl_built_in_p check? While decl_in_std_namespace_p apparently will return false for NULL argument, relying on that is werid. Jakub
Re: [C++ PATCH v2] PR c++/93324 - ICE with -Wall on constexpr if.
On Sun, Jan 19, 2020 at 10:00:42PM +0100, Jakub Jelinek wrote: > On Sun, Jan 19, 2020 at 03:34:48PM -0500, Marek Polacek wrote: > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > * semantics.c (is_std_constant_evaluated_p): Check fndecl. > > > > * g++.dg/cpp1z/constexpr-if33.C: New test. > > --- > > gcc/cp/semantics.c | 4 ++-- > > gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C | 16 > > 2 files changed, 18 insertions(+), 2 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C > > > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > > index 3669b247e34..9051a2863e0 100644 > > --- a/gcc/cp/semantics.c > > +++ b/gcc/cp/semantics.c > > @@ -734,8 +734,8 @@ is_std_constant_evaluated_p (tree fn) > > return false; > > > >tree fndecl = cp_get_callee_fndecl_nofold (fn); > > - if (fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, > > -BUILT_IN_FRONTEND)) > > + if (fndecl && fndecl_built_in_p (fndecl, > > CP_BUILT_IN_IS_CONSTANT_EVALUATED, > > + BUILT_IN_FRONTEND)) > > return true; > > > >if (!decl_in_std_namespace_p (fndecl)) > > Shouldn't it instead do > if (fndecl == NULL_TREE) > return false; > before the fndecl_built_in_p check? > > While decl_in_std_namespace_p apparently will return false for NULL > argument, relying on that is werid. Can do that, too. -- >8 -- This is a crash with constexpr if, when trying to see if the call in the if-statement is std::is_constant_evaluated. cp_get_callee_fndecl_nofold can return NULL_TREE and fndecl_built_in_p doesn't expect to get a null tree, so check FNDECL first. Bootstrapped/regtested on x86_64-linux, ok for trunk? * semantics.c (is_std_constant_evaluated_p): Check fndecl. * g++.dg/cpp1z/constexpr-if33.C: New test. --- gcc/cp/semantics.c | 3 +++ gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C | 16 2 files changed, 19 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 3669b247e34..3b88f1520bc 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -734,6 +734,9 @@ is_std_constant_evaluated_p (tree fn) return false; tree fndecl = cp_get_callee_fndecl_nofold (fn); + if (fndecl == NULL_TREE) +return false; + if (fndecl_built_in_p (fndecl, CP_BUILT_IN_IS_CONSTANT_EVALUATED, BUILT_IN_FRONTEND)) return true; diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C new file mode 100644 index 000..e5ef659932b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if33.C @@ -0,0 +1,16 @@ +// PR c++/93324 - ICE with -Wall on constexpr if. +// { dg-do compile { target c++17 } } +// { dg-options "-Wall" } + +struct { + template + static constexpr bool a() { return 0; } +} e; + +template +void d() +{ + auto c(e); + using b = decltype(c); + if constexpr (b::a<2>()); +} base-commit: bcfc2227c556f2801a657ce3007374732baa8333 -- 2.24.1
[PATCH] Manually handle recursiveness in prepare_block_for_update
From: Andrew Pinski Reported as PR 93321, prepare_block_for_update with some huge recusive inlining can go past the stack limit. The loop at the end, could be transformed such that the last iteration goes back to the begining of the function instead of the call. This reduces the stack usage and speeds up slightly the function. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. ChangeLog: * tree-into-ssa.c (prepare_block_for_update): Manaually sibcall optimize to self. --- gcc/tree-into-ssa.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c index c27bf2ce121..6e139c3b056 100644 --- a/gcc/tree-into-ssa.c +++ b/gcc/tree-into-ssa.c @@ -2616,6 +2616,7 @@ prepare_block_for_update (basic_block bb, bool insert_phi_p) edge e; edge_iterator ei; +again: mark_block_for_update (bb); /* Process PHI nodes marking interesting those that define or use @@ -2695,10 +2696,17 @@ prepare_block_for_update (basic_block bb, bool insert_phi_p) } /* Now visit all the blocks dominated by BB. */ - for (son = first_dom_son (CDI_DOMINATORS, bb); - son; - son = next_dom_son (CDI_DOMINATORS, son)) -prepare_block_for_update (son, insert_phi_p); + for (son = first_dom_son (CDI_DOMINATORS, bb); son; ) +{ + basic_block next = next_dom_son (CDI_DOMINATORS, son); + if (!next) + { + bb = son; + goto again; + } + prepare_block_for_update (son, insert_phi_p); + son = next; +} } -- 2.17.1
Re: [PATCH] Clean up references to Subversion in documentation sources.
On 1/13/20 9:12 AM, Joseph Myers wrote: On Mon, 13 Jan 2020, Sandra Loosemore wrote: On 1/13/20 7:02 AM, Eric S. Raymond wrote: Clean up references to SVN in in the GCC docs, redirecting to Git documentation as appropriate. This is OK, although the set of changes for the libstdc++ manual like this gave me pause: I think you'll need to commit this for Eric (using --author= to set the git author, whenever you commit a patch for someone else). The libstdc++ maintainers can probably handle regenerating the HTML version of the libstdc++ documentation. I've pushed this patch to mainline and the gcc8 and gcc9 branches now. -Sandra
[PATCH Coroutines]Fix false warning message about missing return
Hi, The patch sets current_function_returns_value flag in templates for all co_return/co_yield/co_await cases, as well as for ramp function. This fixes false warning message for case like the added, or various cases in cppcoro. Bootstrap and test on X86_64, is it OK? Thanks, bin gcc/cp 2020-01-20 Bin Cheng * coroutines.cc (finish_co_await_expr): Set return value flag. (finish_co_yield_expr, morph_fn_to_coro): Ditto. gcc/testsuite 2020-01-20 Bin Cheng * g++.dg/coroutines/co-return-warning-1.C: New test. 0001-Fix-false-warning-messages-about-missing-return-in-c.patch Description: Binary data
Re: [OpenACC] libgomp.texi — document acc_*_async and acc_*_finalize(_async) functions
On 1/8/20 9:07 AM, Tobias Burnus wrote: When looking at libgomp.texi the other day, I saw that the acc_*_async variants and the acc_*_finalize functions of OpenACC 2.5 were not documented. Hence, this patch adds them. Those are part of OpenACC 2.5, hence, I updated the @ref (but referenced to OpenACC 2.6 instead). Possible variants: (a) update all acc_* calls to OpenACC 2.6 @refs (b) defer updating the @ref until the OpenACC version is bumped from 2.0 (alias 201306) to OpenACC 2.6 (alias 201711). [Cf. OG9 branch's 7a22697197b85931d9fda66e8b0f75171ea13b43] (c) Independent of the @ref: write the variable-type declarations for Fortran en bloc after all the "subroutine" as they are the same – especially useful for acc_copyout* which has 8 variants. That's how OpenACC 2.7's spec does it. Regarding (c): If one goes for that change, does one keep the "INTERFACE" string in the table for each "subroutine" line? And what do to about the variable-declaration lines? Adding a single "ARGUMENTS" before the first of those (i.e. in the "a" line)? Comments, suggestions, approval? From a docs point of view, this patch looks OK to me as-is, but I don't really have enough state on what is being documented here to comment on the content and organization questions you've raised. :-( -Sandra
Re: [OpenACC] bump version for 2.6 plus libgomp.texi update — document acc_attach/acc_detach, acc_*_async, acc_*_finalize(_async) functions
On 1/10/20 9:34 AM, Tobias Burnus wrote: I believe except for bugs and known omissions (e.g. PR93225+93226), the GCC 10 trunk implementation is complete – and the version number can be bumped from 2.0 (alias 201306) to OpenACC 2.6 (alias 201711). That's what this patch does (i.e. applying the previously mentioned OG9 patch). It also includes the previous patch, i.e. the addition of the missing acc_*_async and acc_*_finalize prototypes. Additionally, I added the missing documentation for acc_attach/acc_detach. — And I did not include the following wording, which the OG9 patch added: "This list has not yet been updated for the OpenACC specification in version 2.6." OK for the trunk? My only comment on this patch relates to this hunk: diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 4cf8b3a5c24..2ef9c22da66 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -546,10 +546,8 @@ status} and @ref{Fortran 2018 status} sections of the documentation. Additionally, the GNU Fortran compilers supports the OpenMP specification (version 4.0 and most of the features of the 4.5 version, @url{http://openmp.org/@/wp/@/openmp-specifications/}). -There also is initial support for the OpenACC specification (targeting -version 2.0, @uref{http://www.openacc.org/}). -Note that this is an experimental feature, incomplete, and subject to -change in future versions of GCC. See +There also is support for the OpenACC specification (targeting +version 2.6, @uref{http://www.openacc.org/}). See @uref{https://gcc.gnu.org/wiki/OpenACC} for more information. @node Varying Length Character Strings I happen to have noticed a couple weeks ago that this language about OpenACC support being experimental appears in multiple places in the gfortran manual, including in the description of the -fopenacc command-line option. The same disclaimer for that option in the main GCC manual was removed years ago, so unless the Fortran support is much more broken than the C/C++ support, I think it ought to be removed from the Fortran manual as well. It looks like there are 3 instances in gfortran.texi and 1 in invoke texi. The other documentation changes in this patch look trivial to me, but again I'm not the right person to review for content. -Sandra
[PATCH Coroutines] Add error messages for missing methods of awaitable class
Hi This patch adds lookup_awaitable_member, it outputs error messages when any of the await_ready/suspend/resume functions are missing in awaitable class. This patch also add some error check on return value of build_co_await since we may failed to build co_await_expr. Bootstrap and test on X86_64, is it OK? Regards JunMa gcc/cp 2020-01-20 Jun Ma * coroutines.cc (lookup_awaitable_member): Lookup an awaitable member. (build_co_await): Use lookup_awaitable_member instead of lookup_member. (finish_co_yield_expr, finish_co_await_expr): Add error check on return value of build_co_await. gcc/testsuite 2020-01-20 Jun Ma * g++.dg/coroutines/coro1-missing-await-method.C: New test. From 3979b29dcdb1000bbc819f69c1dc3ce7616f87cd Mon Sep 17 00:00:00 2001 From: "liangbin.mj" Date: Thu, 21 Nov 2019 08:51:22 +0800 Subject: [PATCH] to #23584419 Add some error messages when can not find method of awaitable class. --- gcc/cp/coroutines.cc | 49 --- .../coroutines/coro1-missing-await-method.C | 21 .../coroutines/coro1-ret-int-yield-int.h | 9 3 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index d3aacd7ad3b..49e509f4384 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -505,6 +505,23 @@ lookup_promise_method (tree fndecl, tree member_id, location_t loc, return pm_memb; } +/* Lookup an Awaitable member, which should be await_ready, await_suspend + or await_resume */ + +static tree +lookup_awaitable_member (tree await_type, tree member_id, location_t loc) +{ + tree aw_memb += lookup_member (await_type, member_id, +/*protect*/ 1, /*want_type*/ 0, tf_warning_or_error); + if (aw_memb == NULL_TREE || aw_memb == error_mark_node) +{ + error_at (loc, "no member named %qE in %qT", member_id, await_type); + return error_mark_node; +} + return aw_memb; +} + /* Here we check the constraints that are common to all keywords (since the presence of a coroutine keyword makes the function into a coroutine). */ @@ -643,25 +660,18 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) /* Check for required awaitable members and their types. */ tree awrd_meth -= lookup_member (o_type, coro_await_ready_identifier, -/* protect */ 1, /*want_type=*/0, tf_warning_or_error); - += lookup_awaitable_member (o_type, coro_await_ready_identifier, loc); if (!awrd_meth || awrd_meth == error_mark_node) return error_mark_node; - tree awsp_meth -= lookup_member (o_type, coro_await_suspend_identifier, -/* protect */ 1, /*want_type=*/0, tf_warning_or_error); - += lookup_awaitable_member (o_type, coro_await_suspend_identifier, loc); if (!awsp_meth || awsp_meth == error_mark_node) return error_mark_node; /* The type of the co_await is the return type of the awaitable's - co_resume(), so we need to look that up. */ + await_resume(), so we need to look that up. */ tree awrs_meth -= lookup_member (o_type, coro_await_resume_identifier, -/* protect */ 1, /*want_type=*/0, tf_warning_or_error); - += lookup_awaitable_member (o_type, coro_await_resume_identifier, loc); if (!awrs_meth || awrs_meth == error_mark_node) return error_mark_node; @@ -800,9 +810,11 @@ finish_co_await_expr (location_t kw, tree expr) /* Now we want to build co_await a. */ tree op = build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT); - TREE_SIDE_EFFECTS (op) = 1; - SET_EXPR_LOCATION (op, kw); - + if (op != error_mark_node) +{ + TREE_SIDE_EFFECTS (op) = 1; + SET_EXPR_LOCATION (op, kw); +} return op; } @@ -864,10 +876,11 @@ finish_co_yield_expr (location_t kw, tree expr) promise transform_await(). */ tree op = build_co_await (kw, yield_call, CO_YIELD_SUSPEND_POINT); - - op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); - TREE_SIDE_EFFECTS (op) = 1; - + if (op != error_mark_node) +{ + op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); + TREE_SIDE_EFFECTS (op) = 1; +} return op; } diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C new file mode 100644 index 000..c1869e0654c --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C @@ -0,0 +1,21 @@ +// { dg-additional-options "-fsyntax-only -w" } +#include "coro.h" + +#define MISSING_AWAIT_READY +#define MISSING_AWAIT_SUSPEND +#define MISSING_AWAIT_RESUME +#include "coro1-ret-int-yield-int.h" + +coro1 +bar0 () // { dg-error {no member named 'await_suspend' in 'coro1::suspend_always_prt'} } +{ + co_await coro1::suspend_never_prt{}; // { dg-error {n
[PATCH Coroutines]Fix ICE when co_awaiting on void type
Hi, This simple patch skips calling complete_type_or_else for void type, which fixes the corresponding ICE. Thanks, bin gcc/cp 2020-01-20 Bin Cheng * coroutines.cc (build_co_await): Skip getting complete type for void. gcc/testsuite 2020-01-20 Bin Cheng * g++.dg/coroutines/co-await-void_type.C: New test. 0001-Fix-ICE-when-co_awaiting-on-void-type.patch Description: Binary data
[PATCH][amdgcn] Add runtime ISA check for amdgcn offloading
Hi, this patch implements a runtime ISA check for amdgcn offloading. The check verifies that the ISA of the GPU to which we try to offload matches the ISA for which the code to be offloaded has been compiled. If it detects a mismatch, it emits an error message which contains a hint at the correct compilation parameters for the GPU. For instance: "libgomp: GCN fatal error: GCN code object ISA 'gfx906' does not match GPU ISA 'gfx900'. Try to recompile with '-foffload=-march=gfx900'." or "libgomp: GCN fatal error: GCN code object ISA 'gfx900' does not match agent ISA 'gfx803'. Try to recompile with '-foffload=-march=fiji'." (By the way, the names that we use for the ISAs are a bit inconsistent. Perhaps we should just use the gfx-names for all ISAs everywhere?.) Without this patch, the user only gets an confusing error message from the HSA runtime which fails to load the GCN object code. I have checked that the code does not lead to any regressions when running the test suite correctly, i.e. with the "-foffload=-march=..." option given to the compiler matching the architecture of the GPU. It seems difficult to implement an automated test that triggers an ISA mismatch. I have tested manually (for different combinations of the compilation flags and offloading GPU ISAs) that the runtime ISA check produces the expected error messages. Is it ok to commit this patch to the master branch? Frederik From 27981f9c93d1efed6d943dae4ea0c52147c02d5b Mon Sep 17 00:00:00 2001 From: Frederik Harwath Date: Mon, 20 Jan 2020 07:45:43 +0100 Subject: [PATCH] Add runtime ISA check for amdgcn offloading When executing code that uses amdgcn GPU offloading, the ISA of the GPU must match the ISA for which the code has been compiled. So far, the libgomp amdgcn plugin did not attempt to verify this. In case of a mismatch, the user is confronted with an unhelpful error message produced by the HSA runtime. This commit implements a runtime ISA check. In the case of a ISA mismatch, the execution is aborted with a clear error message and a hint at the correct compilation parameters for the GPU on which the execution has been attempted. libgomp/ * plugin/plugin-gcn.c (EF_AMDGPU_MACH): New enum. (EF_AMDGPU_MACH_MASK): New constant. (gcn_isa): New typedef. (gcn_gfx801_s): New constant. (gcn_gfx803_s): New constant. (gcn_gfx900_s): New constant. (gcn_gfx906_s): New constant. (gcn_isa_name_len): New constant. (elf_gcn_isa_field): New function. (isa_hsa_name): New function. (isa_gcc_name): New function. (isa_code): New function. (struct agent_info): Add field "device_isa" ... (GOMP_OFFLOAD_init_device): ... and init from here, failing if device has unknown ISA; adapt init of "gfx900_p" to use new constants. (isa_matches_agent): New function ... (create_and_finalize_hsa_program): ... used from here to check that the GPU ISA and the code-object ISA match. --- libgomp/plugin/plugin-gcn.c | 127 +++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c index 16ce251f3a5..14f4a707a7c 100644 --- a/libgomp/plugin/plugin-gcn.c +++ b/libgomp/plugin/plugin-gcn.c @@ -396,6 +396,88 @@ struct gcn_image_desc struct global_var_info *global_variables; }; +/* This enum mirrors the corresponding LLVM enum's values for all ISAs that we + support. + See https://llvm.org/docs/AMDGPUUsage.html#amdgpu-ef-amdgpu-mach-table */ + +typedef enum { + EF_AMDGPU_MACH_AMDGCN_GFX801 = 0x028, + EF_AMDGPU_MACH_AMDGCN_GFX803 = 0x02a, + EF_AMDGPU_MACH_AMDGCN_GFX900 = 0x02c, + EF_AMDGPU_MACH_AMDGCN_GFX906 = 0x02f, +} EF_AMDGPU_MACH; + +const static int EF_AMDGPU_MACH_MASK = 0x00ff; +typedef EF_AMDGPU_MACH gcn_isa; + +const static char* gcn_gfx801_s = "gfx801"; +const static char* gcn_gfx803_s = "gfx803"; +const static char* gcn_gfx900_s = "gfx900"; +const static char* gcn_gfx906_s = "gfx906"; +const static int gcn_isa_name_len = 6; + +static int +elf_gcn_isa_field (Elf64_Ehdr *image) +{ + return image->e_flags & EF_AMDGPU_MACH_MASK; +} + +/* Returns the name that the HSA runtime uses for the ISA or NULL if we do not + support the ISA. */ + +static const char* +isa_hsa_name (int isa) { + switch(isa) +{ +case EF_AMDGPU_MACH_AMDGCN_GFX801: + return gcn_gfx801_s; +case EF_AMDGPU_MACH_AMDGCN_GFX803: + return gcn_gfx803_s; +case EF_AMDGPU_MACH_AMDGCN_GFX900: + return gcn_gfx900_s; +case EF_AMDGPU_MACH_AMDGCN_GFX906: + return gcn_gfx906_s; +} + return NULL; +} + +/* Returns the user-facing name that GCC uses to identify the architecture (e.g. + with -march) or NULL if we do not support the ISA. + Keep in sync with /gcc/config/gcn/gcn.{c,opt}. */ + +static const char* +isa_gcc_name (int isa) { + switch(isa) +{ +case EF_AMDGPU_MACH_AMDGCN_GFX801: + return "carrizo"; +case EF_AMDGPU_MACH_AMDGCN_GFX803: + return "fiji"; +default: + return isa_hsa
Re: [PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
On Sun, Jan 19, 2020 at 10:00 PM H.J. Lu wrote: > > On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak wrote: > > > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu wrote: > > > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak wrote: > > > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak > > > > > > wrote: > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu > > > > > > > wrote: > > > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI > > > > > > > > with > > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, > > > > > > > > PLUS in > > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to > > > > > > > > support > > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), > > > > > > > > %rax". > > > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > H.J. > > > > > > > > --- > > > > > > > > gcc/ > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass > > > > > > > > Pmode to > > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in > > > > > > > > ptr_mode. > > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to > > > > > > > > ... > > > > > > > > (@tls_dynamic_gnu2_64_): This. Replace DI with P. > > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_lea_64_): This. Replace DI > > > > > > > > with P. > > > > > > > > Remove the {q} suffix from lea. > > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_call_64_): This. Replace DI > > > > > > > > with P. > > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_combine_64_): This. Replace > > > > > > > > DI with P. > > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > > --- > > > > > > > > gcc/config/i386/i386.c | 31 +++-- > > > > > > > > gcc/config/i386/i386.md| 54 > > > > > > > > +++--- > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum > > > > > > > > tls_model model, bool for_mov) > > > > > > > >if (TARGET_GNU2_TLS) > > > > > > > > { > > > > > > > > if (TARGET_64BIT) > > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, > > > > > > > > x)); > > > > > > > > else > > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, > > > > > > > > dest)); > > > > > > > > + > > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure > > > > > > > > that > > > > > > > > +PLUS is done in ptr_mode. */ > > > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > > dance? > > > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > > > call *f