Re: [PATCHv4] Vimrc config with GNU formatting
On Wed, Sep 17, 2014 at 09:08:44PM +0400, Yury Gribov wrote:
[ Use a proper mime type and target-disposition inline, as contribute.html
tells you. Or use a saner email client; since you're using git, try git
send-email perhaps? Thanks. ]
> index f7c7e38..ee2321f 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -867,6 +867,11 @@ mail-report-with-warnings.log: warning.log
> chmod +x $@
> echo If you really want to send e-mail, run ./$@ now
>
> +# Local Vim config
> +
> +vimrc:
> + (cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc; $(LN_S)
> contrib/vimrc .lvimrc)
> +
This is another target than what the doc (in the script itself) mentions.
It is not marked as phony. It should not _be_ phony; the two files should
be separate targets. Why make links instead of copies? A user will likely
want to edit his config. The way you use ";" is wrong (it continues if there
is an error). You don't need the "cd" anyway, come to that.
It's pretty silly to have a makefile target that only copies a file (that
is never used by the makefile itself); just tell in the doc where to copy
the file.
> --- /dev/null
> +++ b/contrib/vimrc
> @@ -0,0 +1,45 @@
> +" Code formatting settings for Vim.
> +"
> +" To enable this for GCC files by default, install thinca's vim-localrc
> +" plugin and do
> +" $ make .local.vimrc
No, we should *not* advertise an "enough rope" solution without mentioning
it *will* kill you. Or not mention it at all. Esp. since your next option
has all the same functionality and more.
> +" Or install Markus Braun's localvimrc and do
> +" $ make .lvimrc
As said before, these targets won't work.
> +" Or if you dislike plugins, add autocmd in your ~/.vimrc:
> +" :au BufNewFile,BufReadPost path/to/gcc/* :so path/to/gcc/contrib/vimrc
There are many more reasons than just "dislike of plugins" to prefer
something like this. For one thing, many Vim users will have many
similar statements in their config _already_.
> +" Or just source file manually every time if you are masochist:
> +" :so path/to/gcc/contrib/vimrc
How is that masochist? Typing that cino by hand though, now that would
qualify ;-)
Your list reads as a recommendation, telling the reader to only do the
latter options if they are desperate/stupid/etc. While it really is
the other way around: only lazy people or people who cannot configure
their editor themselves want the do-stuff-behind-your-back solution.
Just keep things neutral please.
> +setlocal cindent
> +setlocal shiftwidth=2
> +setlocal softtabstop=2
> +setlocal cinoptions=>2s,n-s,{s,^-s,:s,=s,g0,f0,hs,p2s,t0,+s,(0,u0,w1,m0
If you write this as absolute numbers instead of as shift widths, you do not
need to force sw and sts settings down people's throat. It might also be
easier to read? Well I doubt that, but it will be slightly shorter at least.
> +setlocal textwidth=79
The coding conventions say maximum line length is 80. 'tw' is a user
preference as well, and this one is almost as annoying as cindent, if
not more.
> +setlocal formatoptions-=ro formatoptions+=cql
Yet another user preference. Also mostly the default, except "l" -- which
won't do anything if tw=0 as it should be. And you do not enable "t" (also
on by default), so you do not want to wrap text anyway? Confused now.
Segher
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Thu, 18 Sep 2014, Janne Blomqvist wrote:
> On Thu, Sep 18, 2014 at 12:57 AM, Hans-Peter Nilsson
> wrote:
> > 'k so we'll track the regressions in a PR.
>
> Do you prefer to tack on to 62768 or a new PR?
Hijacking 62768 for the purposes of reporting a regression for
its fix would not be proper.
> Note that the r215338 fix only applies when using
> INQUIRE(...NAME=...), so I don't think manually disabling
> HAVE_TTYNAME{_R} helps in finding the cause of the regressions, while
> I haven't gone through every testcase you mentioned none of the few
> ones I did check inquired for the name. So there must be something
> else. Can you check where it's actually failing? Is it failing the
> testcase (call abort() ) or is there a segfault etc.?
Will tell in a new PR, unless I see a really obvious fix.
> I suggest trying
> e.g. gfortran.dg/inquire.f90 which is quite a simple testcase.
Thanks.
brgds, H-P
[jit] Add sphinx-based documentation for libgccjit
I've written documentation for the JIT API, in the form of a tutorial together with a more in-depth set of topic guides. I greatly prefer to use Sphinx over Texinfo, both for the ease of editing, and the quality of the generated HTML; I already use it for both the Python bindings to libgccjit, and for gcc-python-plugin. There seems to be some existing precedent for using Sphinx within GCC, albeit experimental, for Ada: https://gcc.gnu.org/ml/gcc/2014-05/msg00286.html Hence I've used Sphinx for these docs. It's trivial to build texinfo and info files from it (assuming you have sphinx installed). Prebuilt HTML of the documentation can be seen at: https://dmalcolm.fedorapeople.org/gcc/libgccjit-api-docs/ (using the default sphinx settings). --- gcc/jit/ChangeLog.jit| 20 ++ gcc/jit/docs/Makefile| 153 + gcc/jit/docs/conf.py | 242 ++ gcc/jit/docs/examples/install-hello-world.c | 103 ++ gcc/jit/docs/examples/tut01-square.c | 87 + gcc/jit/docs/examples/tut02-sum-of-squares.c | 152 + gcc/jit/docs/index.rst | 57 gcc/jit/docs/intro/install.rst | 179 +++ gcc/jit/docs/intro/sum-of-squares.png| Bin 0 -> 22839 bytes gcc/jit/docs/intro/tutorial01.rst| 349 gcc/jit/docs/intro/tutorial02.rst| 376 ++ gcc/jit/docs/topics/contexts.rst | 305 ++ gcc/jit/docs/topics/expressions.rst | 463 +++ gcc/jit/docs/topics/functions.rst| 288 + gcc/jit/docs/topics/locations.rst| 65 gcc/jit/docs/topics/objects.rst | 86 + gcc/jit/docs/topics/results.rst | 48 +++ gcc/jit/docs/topics/types.rst| 209 18 files changed, 3182 insertions(+) create mode 100644 gcc/jit/docs/Makefile create mode 100644 gcc/jit/docs/conf.py create mode 100644 gcc/jit/docs/examples/install-hello-world.c create mode 100644 gcc/jit/docs/examples/tut01-square.c create mode 100644 gcc/jit/docs/examples/tut02-sum-of-squares.c create mode 100644 gcc/jit/docs/index.rst create mode 100644 gcc/jit/docs/intro/install.rst create mode 100644 gcc/jit/docs/intro/sum-of-squares.png create mode 100644 gcc/jit/docs/intro/tutorial01.rst create mode 100644 gcc/jit/docs/intro/tutorial02.rst create mode 100644 gcc/jit/docs/topics/contexts.rst create mode 100644 gcc/jit/docs/topics/expressions.rst create mode 100644 gcc/jit/docs/topics/functions.rst create mode 100644 gcc/jit/docs/topics/locations.rst create mode 100644 gcc/jit/docs/topics/objects.rst create mode 100644 gcc/jit/docs/topics/results.rst create mode 100644 gcc/jit/docs/topics/types.rst diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit index 651b285..b42038e 100644 --- a/gcc/jit/ChangeLog.jit +++ b/gcc/jit/ChangeLog.jit @@ -1,3 +1,23 @@ +2014-09-17 David Malcolm + + * docs/Makefile: New file. + * docs/conf.py: New file. + * docs/examples/install-hello-world.c: New file. + * docs/examples/tut01-square.c: New file. + * docs/examples/tut02-sum-of-squares.c: New file. + * docs/index.rst: New file. + * docs/intro/install.rst: New file. + * docs/intro/sum-of-squares.png: New file. + * docs/intro/tutorial01.rst: New file. + * docs/intro/tutorial02.rst: New file. + * docs/topics/contexts.rst: New file. + * docs/topics/expressions.rst: New file. + * docs/topics/functions.rst: New file. + * docs/topics/locations.rst: New file. + * docs/topics/objects.rst: New file. + * docs/topics/results.rst: New file. + * docs/topics/types.rst: New file. + 2014-09-11 David Malcolm * TODO.rst (Initial Release): Update for addition of myself as diff --git a/gcc/jit/docs/Makefile b/gcc/jit/docs/Makefile new file mode 100644 index 000..7d20702 --- /dev/null +++ b/gcc/jit/docs/Makefile @@ -0,0 +1,153 @@ +# Makefile for Sphinx documentation +# + +# You can set these variables from the command line. +SPHINXOPTS= +SPHINXBUILD = sphinx-build +PAPER = +BUILDDIR = _build + +# Internal variables. +PAPEROPT_a4 = -D latex_paper_size=a4 +PAPEROPT_letter = -D latex_paper_size=letter +ALLSPHINXOPTS = -d $(BUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . +# the i18n builder cannot share the environment and doctrees with the others +I18NSPHINXOPTS = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . + +.PHONY: help clean html dirhtml singlehtml pickle json htmlhelp qthelp devhelp epub latex latexpdf text man changes linkcheck doctest gettext + +help: + @echo "Please use \`make ' where is one of" + @echo " html to make standalone HTML files" + @echo " dirhtmlto make HTML files named index.html in directories" + @echo " singlehtml to make a sin
Re: [PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
Am 17.09.2014 um 15:14 schrieb Matthias Klose:
> Am 17.09.2014 um 00:03 schrieb James Greenhalgh:
>> If you have any other suggestions, or if "=&r" is actually correct and
>> I am misreading the documentation please let me know.
>
> with this patch I see a lot of ICEs in the testsuite for test cases built with
> -O3 (and a build defaulting to -fstack-protector-strong by default), all of
> the
> form:
>
> Executing on host: /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc
> -B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret
> -fdia
> gnostics-color=never -O3 -fomit-frame-pointer -funroll-all-loops
> -finline-functions -w -c -o 900116-1.o
> /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/g
> cc/testsuite/gcc.c-torture/compile/900116-1.c(timeout = 300)
> spawn /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc
> -B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret
> -fdiagnostics-color
> =never -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions -w -c -o
> 900116-1.o /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-
> torture/compile/900116-1.c
> /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:
> In function 'zloop':
> /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1:
> error: insn does not satisfy its constraints:
> (insn 228 225 230 9 (parallel [
> (set (reg:DI 1 x1 [279])
> (unspec:DI [
> (mem/v/f/c:DI (plus:DI (reg/f:DI 31 sp)
> (const_int 24 [0x18])) [4 D.2626+0 S8 A64])
> (mem/v/f/c:DI (reg/f:DI 2 x2 [277]) [4
> __stack_chk_guard+0 S8 A64])
> ] UNSPEC_SP_TEST))
> (clobber (reg:DI 2 x2 [320]))
> ])
> /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14
> 741 {stack_protect_test_di}
> (expr_list:REG_DEAD (reg/f:DI 13 x13 [277])
> (expr_list:REG_UNUSED (reg:DI 2 x2 [320])
> (nil
> /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1:
> internal compiler error: in copyprop_hardreg_forward_1, at regcprop.c:775
> Please submit a full bug report,
> with preprocessed source if appropriate.
>
> for now only tested with the 4.9 linaro branch, now testing with trunk.
seen with trunk r215323 as well, after disabling itm (--disable-libitm) which
currently doesn't seem to build.
Matthias
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Thu, Sep 18, 2014 at 12:57 AM, Hans-Peter Nilsson wrote:
> On Thu, 18 Sep 2014, Janne Blomqvist wrote:
>> On Wed, Sep 17, 2014 at 11:36 PM, Hans-Peter Nilsson
>> wrote:
>> > On the other hand, the tree *is* broken for some ports; I'd
>> > prefer regressions to that. So, unless you're onto this, how do
>> > you feel about me committing the posted patch and opening a PR
>> > for the regressions?
>>
>> I committed
>>
>> Index: inquire.c
>> ===
>> --- inquire.c (revision 215337)
>> +++ inquire.c (working copy)
>> @@ -92,9 +92,9 @@ inquire_via_unit (st_parameter_inquire *
>>else if (u->unit_number == options.stderr_unit)
>> fstrcpy (iqp->name, iqp->name_len, "CONERR$", sizeof("CONERR$"));
>>else
>> - fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
>> + cf_strcpy (iqp->name, iqp->name_len, u->filename);
>> #else
>> -fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
>> +cf_strcpy (iqp->name, iqp->name_len, u->filename);
>> #endif
>> }
>>
>> as obvious (r215338).
>
> (Bah, without the indentation fixed...)
Fixed in r215340. :)
> 'k so we'll track the regressions in a PR.
Do you prefer to tack on to 62768 or a new PR?
Note that the r215338 fix only applies when using
INQUIRE(...NAME=...), so I don't think manually disabling
HAVE_TTYNAME{_R} helps in finding the cause of the regressions, while
I haven't gone through every testcase you mentioned none of the few
ones I did check inquired for the name. So there must be something
else. Can you check where it's actually failing? Is it failing the
testcase (call abort() ) or is there a segfault etc.? I suggest trying
e.g. gfortran.dg/inquire.f90 which is quite a simple testcase.
--
Janne Blomqvist
Re: [PATCH] RTEMS: Update contrib/config-list.mk
On 09/17/14 09:10, Joel Sherrill wrote: Is there anyone else from GCC who needs to approve this? I think you're OK is all we need here. As RTEMS maintainer for GCC, I am ok with it. Then it's good to go :-) Thanks, jeff
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Thu, 18 Sep 2014, Janne Blomqvist wrote:
> On Wed, Sep 17, 2014 at 11:36 PM, Hans-Peter Nilsson
> wrote:
> > On the other hand, the tree *is* broken for some ports; I'd
> > prefer regressions to that. So, unless you're onto this, how do
> > you feel about me committing the posted patch and opening a PR
> > for the regressions?
>
> I committed
>
> Index: inquire.c
> ===
> --- inquire.c (revision 215337)
> +++ inquire.c (working copy)
> @@ -92,9 +92,9 @@ inquire_via_unit (st_parameter_inquire *
>else if (u->unit_number == options.stderr_unit)
> fstrcpy (iqp->name, iqp->name_len, "CONERR$", sizeof("CONERR$"));
>else
> - fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
> + cf_strcpy (iqp->name, iqp->name_len, u->filename);
> #else
> -fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
> +cf_strcpy (iqp->name, iqp->name_len, u->filename);
> #endif
> }
>
> as obvious (r215338).
(Bah, without the indentation fixed...)
'k so we'll track the regressions in a PR.
brgds, H-P
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, Sep 17, 2014 at 11:36 PM, Hans-Peter Nilsson wrote:
> On Wed, 17 Sep 2014, Hans-Peter Nilsson wrote:
>> On Wed, 17 Sep 2014, Hans-Peter Nilsson wrote:
>> > On Wed, 17 Sep 2014, Janne Blomqvist wrote:
>> > > Oops, I forgot to update some parts in an #ifdef branch that isn't
>> > > taken on my target. I'll try to find time to fix it later tonight. If
>> > > you're in a hurry, just replace
>> > >
>> > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
>> > >
>> > > with
>> > >
>> > > cf_strcpy (iqp->name, iqp->name_len, u->filename);
>> > >
>> > > in inquire.c.
>> >
>> > Thanks, build completes and I'll commit the following as obvious
>> > if there are no regressions.
>>
>> Since there are 25 related regressions, not committed.
>> There must be something else amiss too.
>
> On the other hand, the tree *is* broken for some ports; I'd
> prefer regressions to that. So, unless you're onto this, how do
> you feel about me committing the posted patch and opening a PR
> for the regressions?
I committed
Index: inquire.c
===
--- inquire.c (revision 215337)
+++ inquire.c (working copy)
@@ -92,9 +92,9 @@ inquire_via_unit (st_parameter_inquire *
else if (u->unit_number == options.stderr_unit)
fstrcpy (iqp->name, iqp->name_len, "CONERR$", sizeof("CONERR$"));
else
- fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
+ cf_strcpy (iqp->name, iqp->name_len, u->filename);
#else
-fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len);
+cf_strcpy (iqp->name, iqp->name_len, u->filename);
#endif
}
as obvious (r215338).
--
Janne Blomqvist
Re: [C PATCH] Better diagnostics for C++ comments in C90 (PR c/61854)
On Wed, 17 Sep 2014, Marek Polacek wrote: > Sure, updated. > > Bootstrap in progress, regtested on x86_64-linux, ok for trunk? > > 2014-09-17 Marek Polacek > > PR c/61854 > libcpp/ > * init.c (struct lang_flags): Remove cplusplus_comments. > (cpp_set_lang): Likewise. > (post_options): Likewise. > * lex.c (_cpp_lex_direct): Disallow C++ style comments in C90/C94. > testsuite/ > * gcc.dg/cpp/pr61854-1.c: New test. > * gcc.dg/cpp/pr61854-2.c: New test. > * gcc.dg/cpp/pr61854-3.c: New test. > * gcc.dg/cpp/pr61854-3.h: New test. > * gcc.dg/cpp/pr61854-4.c: New test. > * gcc.dg/cpp/pr61854-5.c: New test. > * gcc.dg/cpp/pr61854-6.c: New test. > * gcc.dg/cpp/pr61854-7.c: New test. > * gcc.dg/cpp/pr61854-c90.c: New test. > * gcc.dg/cpp/pr61854-c94.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC PATCH] -fsanitize=vptr instrumentation
On 09/17/2014 02:02 PM, Jason Merrill wrote: On 09/17/2014 11:40 AM, Jakub Jelinek wrote: And for the last one, should we before dynamic_cast verify the object passed to dynamic_cast has the expected vptr? Perhaps we should just add checking to the dynamic_cast code. I'm not sure if that would slow it down enough to merit a separate entry point for checking. There is some checking I want to do in dynamic_cast, but I think we also want to verify that the argument has the type expected. Jason
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, 17 Sep 2014, Hans-Peter Nilsson wrote: > On Wed, 17 Sep 2014, Hans-Peter Nilsson wrote: > > On Wed, 17 Sep 2014, Janne Blomqvist wrote: > > > Oops, I forgot to update some parts in an #ifdef branch that isn't > > > taken on my target. I'll try to find time to fix it later tonight. If > > > you're in a hurry, just replace > > > > > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > > > > > > with > > > > > > cf_strcpy (iqp->name, iqp->name_len, u->filename); > > > > > > in inquire.c. > > > > Thanks, build completes and I'll commit the following as obvious > > if there are no regressions. > > Since there are 25 related regressions, not committed. > There must be something else amiss too. On the other hand, the tree *is* broken for some ports; I'd prefer regressions to that. So, unless you're onto this, how do you feel about me committing the posted patch and opening a PR for the regressions? brgds, H-P
Re: ptx preliminary address space fixes [4/4]
On 09/11/2014 01:41 PM, Richard Biener wrote:
On Thu, Sep 11, 2014 at 12:12 PM, Bernd Schmidt wrote:
This one isn't a wrong-code issue, just a missed optimization. The strlen
optimizations need to be made to look through ADDR_SPACE_CONVERT_EXPR to
work on ptx.
Bootstrapped and tested together with the other patches on x86_64-linux.
Ok?
Did you try adding ADDR_SPACE_CONVERT_EXPR to the tree codes
handled in gimple_assign_cast_p?
I did now (full test on x86_64, and also tested with ptx), and that also
appears to work. Ok?
Bernd
diff --git a/gcc/gimple.h b/gcc/gimple.h
index ec41585..8d4032b 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -2384,9 +2384,10 @@ gimple_assign_cast_p (const_gimple s)
if (is_gimple_assign (s))
{
enum tree_code sc = gimple_assign_rhs_code (s);
- return CONVERT_EXPR_CODE_P (sc)
- || sc == VIEW_CONVERT_EXPR
- || sc == FIX_TRUNC_EXPR;
+ return (CONVERT_EXPR_CODE_P (sc)
+ || sc == VIEW_CONVERT_EXPR
+ || sc == FIX_TRUNC_EXPR
+ || sc == ADDR_SPACE_CONVERT_EXPR);
}
return false;
[GCOV] shared library awareness
I've committed this patch, last in my current series. It teaches the gcov
machinery about shared objects that are also built with coverage information.
The basic idea is to chain gcov_root structures together onto a single global
chain -- checking the GCOV_VERSION number so that only compatible gcov modules
get chained.
When __gcov_rest or __gcov_dump are called, all compatible modules are cleared
or dumped. The existing atexit machinery is used to dump the current module
only when it is exited/unloaded.
nathan
2014-09-17 Nathan sidwell
* Makefile.in (LIBGCOV_INTERFACE): Add _gcov_dump from ...
(LIBGCOV_DRIVER): ... here.
* libgcov-driver.c (gcov_master): New.
(gcov_exit): Remove from master chain.
(__gcov_init): Add to master chain if version compatible. Don't
clear the version.
* libgcov_interface (__gcov_flust): Call gcov_dump_int.
(gcov_reset_int): Clear master chain, if compatible.
(gcov_dump_int): New internal interface. Dump master chain, if
compatible.
(gcov_dump): Alias for gcov_dump_int.
* libgcov.h (struct gcov_root): Add next and prev fields.
(struct gcov_master): New struct.
(__gcov_master): New.
(gcov_dump_int): Declare.
Index: Makefile.in
===
--- Makefile.in (revision 215334)
+++ Makefile.in (working copy)
@@ -858,9 +858,10 @@ LIBGCOV_PROFILER = _gcov_interval_profil
_gcov_one_value_profiler _gcov_indirect_call_profiler \
_gcov_average_profiler _gcov_ior_profiler \
_gcov_indirect_call_profiler_v2 _gcov_time_profiler
-LIBGCOV_INTERFACE = _gcov_flush _gcov_fork _gcov_execl _gcov_execlp\
+LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork \
+ _gcov_execl _gcov_execlp\
_gcov_execle _gcov_execv _gcov_execvp _gcov_execve _gcov_reset
-LIBGCOV_DRIVER = _gcov _gcov_dump
+LIBGCOV_DRIVER = _gcov
libgcov-merge-objects = $(patsubst %,%$(objext),$(LIBGCOV_MERGE))
libgcov-profiler-objects = $(patsubst %,%$(objext),$(LIBGCOV_PROFILER))
Index: libgcov-driver.c
===
--- libgcov-driver.c(revision 215334)
+++ libgcov-driver.c(working copy)
@@ -777,13 +777,23 @@ __gcov_dump_one (struct gcov_root *root)
root->run_counted = 1;
}
-/* Per-program/shared-object gcov state. */
+/* Per-dynamic-object gcov state. */
struct gcov_root __gcov_root;
+/* Exactly one of these will be live in the process image. */
+struct gcov_master __gcov_master =
+ {GCOV_VERSION, 0};
+
static void
gcov_exit (void)
{
__gcov_dump_one (&__gcov_root);
+ if (__gcov_root.next)
+__gcov_root.next->prev = __gcov_root.prev;
+ if (__gcov_root.prev)
+__gcov_root.prev->next = __gcov_root.next;
+ else
+__gcov_master.root = __gcov_root.next;
}
/* Add a new object file onto the bb chain. Invoked automatically
@@ -797,12 +807,21 @@ __gcov_init (struct gcov_info *info)
if (gcov_version (info, info->version, 0))
{
if (!__gcov_root.list)
-atexit (gcov_exit);
+ {
+ /* Add to master list and at exit function. */
+ if (gcov_version (NULL, __gcov_master.version, ""))
+ {
+ __gcov_root.next = __gcov_master.root;
+ if (__gcov_master.root)
+ __gcov_master.root->prev = &__gcov_root;
+ __gcov_master.root = &__gcov_root;
+ }
+ atexit (gcov_exit);
+ }
info->next = __gcov_root.list;
__gcov_root.list = info;
}
- info->version = 0;
}
#endif /* !IN_GCOV_TOOL */
#endif /* L_gcov */
Index: libgcov-interface.c
===
--- libgcov-interface.c (revision 215334)
+++ libgcov-interface.c (working copy)
@@ -85,7 +85,7 @@ __gcov_flush (void)
init_mx_once ();
__gthread_mutex_lock (&__gcov_flush_mx);
- __gcov_dump_one (&__gcov_root);
+ __gcov_dump_int ();
__gcov_reset_int ();
__gthread_mutex_unlock (&__gcov_flush_mx);
@@ -132,8 +132,16 @@ gcov_clear (const struct gcov_info *list
void
__gcov_reset_int (void)
{
- gcov_clear (__gcov_root.list);
- __gcov_root.dumped = 0;
+ struct gcov_root *root;
+
+ /* If we're compatible with the master, iterate over everything,
+ otherise just do us. */
+ for (root = __gcov_master.version == GCOV_VERSION
+? __gcov_master.root : &__gcov_root; root; root = root->next)
+{
+ gcov_clear (root->list);
+ root->dumped = 0;
+}
}
ALIAS_void_fn (__gcov_reset_int, __gcov_reset);
@@ -145,11 +153,19 @@ ALIAS_void_fn (__gcov_reset_int, __gcov_
so far, in order to collect profile in region of interest. */
void
-__gcov_dump (void)
+__gcov_dump_int (void)
{
- __gcov_dump_one (&__gcov_root);
+ struct gcov_root *root;
+
+ /* If we're compatible wit
[patch] avoid ICE due to NULL pointer dereference in ipa-comdats.c
Hi,
I got an ICE while building libstdc++ of a cross compiler x86 to aarch64. I
have a testcase that ICEs on current GCC trunk. I was trying to painfully
reduce it with creduce, and it is still several thousand lines of c++.
Frustrated
that it does not reduce anymore, I decided to have a look with gdb at why the
compiler was iceing: the code dereferences a NULL pointer that we get by looking
up the value of a symbol in a map. Around that place, there is another pattern
that first makes sure that the pointer we get from the map is non NULL: I copied
that code around and that seemed to have solved the ICE.
Regtested on x86-64-linux, and also checked that my aarch64 cross compilers are
now building correctly libstdc++.
Ok to commit?
Thanks,
Sebastian
>From f5934ecdee5d7e8e143310c21906d5099b9e7d23 Mon Sep 17 00:00:00 2001
From: Sebastian Pop
Date: Wed, 17 Sep 2014 14:04:20 -0500
Subject: [PATCH] avoid dereferencing a NULL pointer
2014-09-17 Sebastian Pop
* ipa-comdats.c (ipa_comdats): Check that the value in the map is non
NULL before dereferencing it.
---
gcc/ipa-comdats.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
index b270d97..57e8239 100644
--- a/gcc/ipa-comdats.c
+++ b/gcc/ipa-comdats.c
@@ -317,7 +317,13 @@ ipa_comdats (void)
&& !symbol->alias
&& symbol->real_symbol_p ())
{
- tree group = *map.get (symbol);
+ tree group = NULL;
+ /* Get current lattice value of SYMBOL. */
+ tree *val = map.get (symbol);
+ if (val)
+ group = *val;
+ else
+ continue;
if (group == error_mark_node)
continue;
--
1.9.1
Fix i386 FP_TRAPPING_EXCEPTIONS
The i386 sfp-machine.h defines FP_TRAPPING_EXCEPTIONS in a way that is
always wrong: it treats a set bit as indicating the exception is
trapping, when actually a set bit (both for 387 and SSE floating
point) indicates it is masked, and a clear bit indicates it is
trapping. This patch fixes this bug.
Bootstrapped with no regressions on x86_64-unknown-linux-gnu. OK to
commit?
Note to ia64 maintainers: it would be a good idea to add a definition
of FP_TRAPPING_EXCEPTIONS for ia64, and I expect the new test to fail
on ia64 until you do so.
libgcc:
2014-09-17 Joseph Myers
* config/i386/sfp-machine.h (FP_TRAPPING_EXCEPTIONS): Treat clear
bits not set bits as indicating trapping exceptions.
gcc/testsuite:
2014-09-17 Joseph Myers
* gcc.dg/torture/float128-exact-underflow.c: New test.
Index: gcc/testsuite/gcc.dg/torture/float128-exact-underflow.c
===
--- gcc/testsuite/gcc.dg/torture/float128-exact-underflow.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/float128-exact-underflow.c (revision 0)
@@ -0,0 +1,41 @@
+/* Test that exact underflow in __float128 signals the underflow
+ exception if trapping is enabled, but does not raise the flag
+ otherwise. */
+
+/* { dg-do run { target i?86-*-*gnu* x86_64-*-*gnu* ia64-*-*gnu* } } */
+/* { dg-options "-D_GNU_SOURCE" } */
+/* { dg-require-effective-target fenv_exceptions } */
+
+#include
+#include
+#include
+#include
+
+volatile sig_atomic_t caught_sigfpe;
+sigjmp_buf buf;
+
+static void
+handle_sigfpe (int sig)
+{
+ caught_sigfpe = 1;
+ siglongjmp (buf, 1);
+}
+
+int
+main (void)
+{
+ volatile __float128 a = 0x1p-16382q, b = 0x1p-2q;
+ volatile __float128 r;
+ r = a * b;
+ if (fetestexcept (FE_UNDERFLOW))
+abort ();
+ if (r != 0x1p-16384q)
+abort ();
+ feenableexcept (FE_UNDERFLOW);
+ signal (SIGFPE, handle_sigfpe);
+ if (sigsetjmp (buf, 1) == 0)
+r = a * b;
+ if (!caught_sigfpe)
+abort ();
+ exit (0);
+}
Index: libgcc/config/i386/sfp-machine.h
===
--- libgcc/config/i386/sfp-machine.h(revision 215323)
+++ libgcc/config/i386/sfp-machine.h(working copy)
@@ -60,7 +60,7 @@
__sfp_handle_exceptions (_fex); \
} while (0);
-#define FP_TRAPPING_EXCEPTIONS ((_fcw >> FP_EX_SHIFT) & FP_EX_ALL)
+#define FP_TRAPPING_EXCEPTIONS ((~_fcw >> FP_EX_SHIFT) & FP_EX_ALL)
#define FP_ROUNDMODE (_fcw & FP_RND_MASK)
#endif
--
Joseph S. Myers
jos...@codesourcery.com
Re: [committed] Fix up run_all.sh
On Sep 17, 2014, at 12:16 PM, Jakub Jelinek wrote: > On Wed, Sep 17, 2014 at 12:13:51PM -0700, Mike Stump wrote: >> On Sep 17, 2014, at 11:25 AM, Jakub Jelinek wrote: >>> - if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count; then >>> + if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count 2>/dev/null; >>> then >> >> So, I can’t help but think we should just do a mkdir -p for this and be done >> with it. > > mkdir -p does not fail if the directory exists, which is exactly what this > code relies on for the synchronization. Oh, yeah, right. :-) Never mind.
Re: [committed] Fix up run_all.sh
On Wed, Sep 17, 2014 at 12:13:51PM -0700, Mike Stump wrote: > On Sep 17, 2014, at 11:25 AM, Jakub Jelinek wrote: > > - if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count; then > > + if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count 2>/dev/null; > > then > > So, I can’t help but think we should just do a mkdir -p for this and be done > with it. mkdir -p does not fail if the directory exists, which is exactly what this code relies on for the synchronization. Jakub
Re: [C PATCH] Better diagnostics for C++ comments in C90 (PR c/61854)
On Tue, Sep 16, 2014 at 10:32:51PM +, Joseph S. Myers wrote:
> This is getting closer, but it looks like you still treat it as a line
> comment when being skipped for C90, when actually it's not safe to treat
> it like that; you have to produce a '/' preprocessing token and continue
> tokenizing the rest of the line. Consider the following code:
>
> int i = 0
> #if 0
> // /*
> #else
> // */
> +1
> #endif
> ;
>
> For C90 i gets value 0. With // comments it gets value 1.
Aha. I made a thinko, hopefully fixed now. I added two new tests
(it's definitely good to have more testing in this area!) based on
your code, thanks for that.
> > + /* In C89/C94, C++ style comments are forbidden. */
> > + else if ((CPP_OPTION (pfile, lang) == CLK_STDC89
> > + || CPP_OPTION (pfile, lang) == CLK_STDC94))
> > + {
> > + /* But don't be confused about // immediately followed by *. */
> > + if (buffer->cur[1] == '*'
> > + || pfile->state.in_directive)
>
> And this comment needs updating to reflect that it's not just //* where //
> can appear in valid C90 code in a way incompatible with treating it as a
> comment.
Sure, updated.
Bootstrap in progress, regtested on x86_64-linux, ok for trunk?
2014-09-17 Marek Polacek
PR c/61854
libcpp/
* init.c (struct lang_flags): Remove cplusplus_comments.
(cpp_set_lang): Likewise.
(post_options): Likewise.
* lex.c (_cpp_lex_direct): Disallow C++ style comments in C90/C94.
testsuite/
* gcc.dg/cpp/pr61854-1.c: New test.
* gcc.dg/cpp/pr61854-2.c: New test.
* gcc.dg/cpp/pr61854-3.c: New test.
* gcc.dg/cpp/pr61854-3.h: New test.
* gcc.dg/cpp/pr61854-4.c: New test.
* gcc.dg/cpp/pr61854-5.c: New test.
* gcc.dg/cpp/pr61854-6.c: New test.
* gcc.dg/cpp/pr61854-7.c: New test.
* gcc.dg/cpp/pr61854-c90.c: New test.
* gcc.dg/cpp/pr61854-c94.c: New test.
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-1.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-1.c
index e69de29..364a511 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-1.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-1.c
@@ -0,0 +1,15 @@
+/* PR c/61854 */
+/* { dg-do run } */
+/* { dg-options "-std=c89" } */
+
+int
+main (void)
+{
+ int i = 1 //**/ 2
+ ;
+ int j = 1 //**/ 2
+ ;
+ if (i != 0 || j != 0)
+__builtin_abort ();
+ return 0;
+}
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-2.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-2.c
index e69de29..883db21 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-2.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-2.c
@@ -0,0 +1,15 @@
+/* PR c/61854 */
+/* { dg-do run } */
+/* { dg-options "-std=gnu89" } */
+
+int
+main (void)
+{
+ int i = 1 //**/ 2
+ ;
+ int j = 1 //**/ 2
+ ;
+ if (i != 1 || j != 1)
+__builtin_abort ();
+ return 0;
+}
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.c
index e69de29..916c12e 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.c
@@ -0,0 +1,6 @@
+/* PR c/61854 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=c89" } */
+
+#include "pr61854-3.h"
+int i;
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.h
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.h
index e69de29..fd798bd 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.h
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-3.h
@@ -0,0 +1,4 @@
+#pragma GCC system_header
+// X
+// Y
+// Z
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-4.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-4.c
index e69de29..5cfa5a7 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-4.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-4.c
@@ -0,0 +1,16 @@
+/* PR c/61854 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=c89" } */
+
+void
+foo (void)
+{
+#if 0
+ // Do not error here.
+#endif
+#if 1
+ // But error here.
+#endif
+ /* { dg-error "C\\+\\+ style comments are not allowed in ISO C90" "comments"
{ target *-*-*} 12 } */
+ /* { dg-error "reported only once" "" { target *-*-*} 12 } */
+}
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-5.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-5.c
index e69de29..a7628dc 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-5.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-5.c
@@ -0,0 +1,15 @@
+/* PR c/61854 */
+/* { dg-do run } */
+/* { dg-options "-std=c89" } */
+
+#define h(x) #x
+#define s(x) h(x)
+#define foo //
+
+int
+main (void)
+{
+ if (__builtin_memcmp (s(foo), "//", 3) != 0)
+__builtin_abort ();
+ return 0;
+}
diff --git gcc/gcc/testsuite/gcc.dg/cpp/pr61854-6.c
gcc/gcc/testsuite/gcc.dg/cpp/pr61854-6.c
index e69de29..1b60cc3 100644
--- gcc/gcc/testsuite/gcc.dg/cpp/pr61854-6.c
+++ gcc/gcc/testsuite/gcc.dg/cpp/pr61854-6.c
@@ -0,0 +1,19 @@
+/* PR c/61854 */
+/* { dg-do run } */
+/* { dg-options "-std=gnu89" } */
+
+int
+main (void)
+{
+ int i = 0
+#if 0
+// /*
+#else
+// */
++1
+#endif
+;
+ if (i != 1)
+__builtin_abort ();
+ return 0;
+}
diff -
Re: [committed] Fix up run_all.sh
On Sep 17, 2014, at 11:25 AM, Jakub Jelinek wrote: > - if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count; then > + if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count 2>/dev/null; > then So, I can’t help but think we should just do a mkdir -p for this and be done with it.
Re: [PATCH, i386, Pointer Bounds Checker 30/x] Size relocation
On Wed, Sep 17, 2014 at 8:35 PM, Ilya Enkovich wrote:
> On 16 Sep 12:22, Uros Bizjak wrote:
>> On Tue, Sep 16, 2014 at 11:37 AM, Ilya Enkovich
>> wrote:
>> > 2014-09-16 13:08 GMT+04:00 Uros Bizjak :
>> >>
>> >> Can x86_64_immediate_operand predicate be used here?
>> >
>> > I think it cannot be used because of TLS symbols not counting as immediate.
>>
>> OK, please introduce a new predicate, similar to
>> x86_64_immediate_operand, perhaps x86_64_immediate_size_operand, so we
>> can add some comments there. This will also help to macroize the insn,
>> x86_64_immediate_operand has !TARGET_64BIT shortcut for this case.
>>
>> Uros.
>
> I don't see how new predicate would help to macroize insn. Single template
> may look as following patch.
You put early return for !TARGET_64BITS. Please see
x86_64_immediate_operand predicate.
So,
/* Here comes comment. */
(define_predicate "x86_64_immediate_size_operand"
(match_code "symbol_ref")
{
if (!TARGET_64BIT)
return true;
/* Comment here explaining these conditions. */
return (ix86_cmodel == CM_SMALL || ix86_cmodel == CM_KERNEL);
}
And then in the pattern itself:
if (x86_64_immediate_size_operand (operands[1], VOIDmode)
return "mov{l}\t{%1@SIZE, %k0|%k0, %1@SIZE}";
else
return "movabs{q}\t{%1@SIZE, %0|%0, %1@SIZE}";
Uros.
Re: [PATCH, i386, Pointer Bounds Checker 30/x] Size relocation
On 16 Sep 12:22, Uros Bizjak wrote:
> On Tue, Sep 16, 2014 at 11:37 AM, Ilya Enkovich
> wrote:
> > 2014-09-16 13:08 GMT+04:00 Uros Bizjak :
> >>
> >> Can x86_64_immediate_operand predicate be used here?
> >
> > I think it cannot be used because of TLS symbols not counting as immediate.
>
> OK, please introduce a new predicate, similar to
> x86_64_immediate_operand, perhaps x86_64_immediate_size_operand, so we
> can add some comments there. This will also help to macroize the insn,
> x86_64_immediate_operand has !TARGET_64BIT shortcut for this case.
>
> Uros.
I don't see how new predicate would help to macroize insn. Single template may
look as following patch.
Thanks,
Ilya
--
2014-09-17 Ilya Enkovich
* config/i386/i386.md (UNSPEC_SIZEOF): New.
(move_size_reloc_): New.
* config/i386/predicates.md (symbol_operand): New.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 2c367b2..628a58b 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -79,6 +79,7 @@
UNSPEC_PLTOFF
UNSPEC_MACHOPIC_OFFSET
UNSPEC_PCREL
+ UNSPEC_SIZEOF
;; Prologue support
UNSPEC_STACK_ALLOC
@@ -18554,6 +18555,24 @@
"bndstx\t{%2, %3|%3, %2}"
[(set_attr "type" "mpxst")])
+(define_insn "move_size_reloc_"
+ [(set (match_operand:SWI48 0 "register_operand" "=r")
+ (unspec:SWI48
+[(match_operand:SWI48 1 "symbol_operand")]
+UNSPEC_SIZEOF))]
+ "TARGET_MPX"
+{
+ /* We have to use movabs in case object's size may exceed
+ 31 bits. */
+ if (ix86_cmodel == CM_MEDIUM || ix86_cmodel == CM_LARGE
+ || ix86_cmodel == CM_MEDIUM_PIC || ix86_cmodel == CM_LARGE_PIC)
+return "movabs{q}\t{%1@SIZE, %0|%0, %1@SIZE}";
+ else
+return "mov{l}\t{%1@SIZE, %k0|%k0, %1@SIZE}";
+}
+ [(set_attr "type" "imov")
+ (set_attr "mode" "")])
+
(include "mmx.md")
(include "sse.md")
(include "sync.md")
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index cd542b7..3e2d471 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -124,6 +124,10 @@
(match_test "TARGET_64BIT")
(match_test "REGNO (op) > BX_REG")))
+;; Return true if VALUE is symbol reference
+(define_predicate "symbol_operand"
+ (match_code "symbol_ref"))
+
;; Return true if VALUE can be stored in a sign extended immediate field.
(define_predicate "x86_64_immediate_operand"
(match_code "const_int,symbol_ref,label_ref,const")
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, 17 Sep 2014, Hans-Peter Nilsson wrote: > On Wed, 17 Sep 2014, Janne Blomqvist wrote: > > Oops, I forgot to update some parts in an #ifdef branch that isn't > > taken on my target. I'll try to find time to fix it later tonight. If > > you're in a hurry, just replace > > > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > > > > with > > > > cf_strcpy (iqp->name, iqp->name_len, u->filename); > > > > in inquire.c. > > Thanks, build completes and I'll commit the following as obvious > if there are no regressions. Since there are 25 related regressions, not committed. There must be something else amiss too. (Maybe you can find it on your platform by artificially disabling HAVE_TTYNAME && HAVE_TTYNAME_R.) +gfortran.sum gfortran.dg/f2003_inquire_1.f03 +gfortran.sum gfortran.dg/inquire.f90 +gfortran.sum gfortran.dg/inquire_13.f90 +gfortran.sum gfortran.dg/inquire_15.f90 +gfortran.sum gfortran.dg/inquire_16.f90 +gfortran.sum gfortran.dg/inquire_6.f90 +gfortran.sum gfortran.dg/inquire_7.f90 +gfortran.sum gfortran.dg/inquire_9.f90 +gfortran.sum gfortran.dg/inquire_size.f90 +gfortran.sum gfortran.dg/large_unit_1.f90 +gfortran.sum gfortran.dg/large_unit_2.f90 +gfortran.sum gfortran.dg/negative_unit.f +gfortran.sum gfortran.dg/negative_unit_int8.f +gfortran.sum gfortran.dg/open_negative_unit_1.f90 +gfortran.sum gfortran.dg/pr20950.f +gfortran.sum gfortran.dg/streamio_10.f90 +gfortran.sum gfortran.dg/streamio_16.f90 +gfortran.sum gfortran.dg/streamio_3.f90 +gfortran.sum gfortran.dg/streamio_8.f90 +gfortran.sum gfortran.dg/unf_io_convert_4.f90 +gfortran.sum gfortran.fortran-torture/execute/inquire_1.f90 +gfortran.sum gfortran.fortran-torture/execute/inquire_2.f90 +gfortran.sum gfortran.fortran-torture/execute/inquire_3.f90 +gfortran.sum gfortran.fortran-torture/execute/inquire_4.f90 +gfortran.sum gfortran.fortran-torture/execute/inquire_5.f90 brgds, H-P
[committed] Fix up run_all.sh
Hi! I've committed following fix as obvious, to avoid thousands of mkdir: cannot create directory `/usr/src/gcc/obj084/gcc/testsuite/ada/acats-parallel/10': File exists lines in output when checking acats in parallel. 2014-09-17 Jakub Jelinek * ada/acats/run_all.sh: Redirect mkdir stderr to /dev/null. --- gcc/testsuite/ada/acats/run_all.sh.jj 2014-09-15 20:19:15.0 +0200 +++ gcc/testsuite/ada/acats/run_all.sh 2014-09-17 19:46:34.921752227 +0200 @@ -268,7 +268,7 @@ for chapter in $chapters; do if [ $par_countm -eq 1 ]; then as_fn_arith $par_count + 1 par_count=$as_val - if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count; then + if mkdir $GCC_RUNTEST_PARALLELIZE_DIR/$par_count 2>/dev/null; then par_last=1 else par_last= Jakub
Re: [PATCH] Fix -fcompare-debug failures caused by fixup_noreturn_call (PR debug/63284)
On September 17, 2014 8:20:41 PM CEST, Jakub Jelinek wrote:
>Hi!
>
>The following testcase fails on 4.9 branch (shows
>undesirable difference in *.optimized dump on the trunk, but
>doesn't report -fcompare-debug failure there, and in 4.8 is latent).
>The problem is that if we have a noreturn stmt followed by only
>debug stmt, fixup_noreturn_call will try to split_block after the
>noreturn call, but with -g0 there are never debug stmts and thus
>we would not split the block; that can lead to different order of bb
>predecessors, different order of PHI args etc.
>
>As nothing after noreturn call really matters, this patch if there are
>only debug stmts after noreturn call removes the debug stmts instead of
>the undesirable splitting of the block.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and
>on
>the 4.9 branch, ok for trunk/4.9 and eventually 4.8 too?
OK everywhere.
Thanks,
Richard.
>
>2014-09-17 Jakub Jelinek
>
> PR debug/63284
> * tree-cfgcleanup.c (fixup_noreturn_call): Don't split block
> if there are only debug stmts after the noreturn call, instead
> remove the debug stmts.
>
> * gcc.dg/pr63284.c: New test.
>
>--- gcc/tree-cfgcleanup.c.jj 2014-09-01 13:33:11.0 +0200
>+++ gcc/tree-cfgcleanup.c 2014-09-17 11:01:53.448165423 +0200
>@@ -565,7 +565,20 @@ fixup_noreturn_call (gimple stmt)
>
> /* First split basic block if stmt is not last. */
> if (stmt != gsi_stmt (gsi_last_bb (bb)))
>-split_block (bb, stmt);
>+{
>+ if (stmt == gsi_stmt (gsi_last_nondebug_bb (bb)))
>+ {
>+/* Don't split if there are only debug stmts
>+ after stmt, that can result in -fcompare-debug
>+ failures. Remove the debug stmts instead,
>+ they should be all unreachable anyway. */
>+gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>+for (gsi_next (&gsi); !gsi_end_p (gsi); )
>+ gsi_remove (&gsi, true);
>+ }
>+ else
>+ split_block (bb, stmt);
>+}
>
> changed |= remove_fallthru_edge (bb->succs);
>
>--- gcc/testsuite/gcc.dg/pr63284.c.jj 2014-09-17 10:56:59.699677715
>+0200
>+++ gcc/testsuite/gcc.dg/pr63284.c 2014-09-17 10:56:39.0 +0200
>@@ -0,0 +1,42 @@
>+/* PR debug/63284 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fcompare-debug" } */
>+
>+int a[10], *b, *d, c, f;
>+int fn2 (void);
>+void fn3 (void);
>+void fn4 (int);
>+
>+static int
>+fn1 (int x)
>+{
>+ int e = a[0];
>+ if (e)
>+return 1;
>+ if (b)
>+switch (x)
>+ {
>+ case 1:
>+if (d)
>+ e = fn2 ();
>+else
>+ fn3 ();
>+break;
>+ case 0:
>+if (d)
>+ {
>+fn3 ();
>+if (c)
>+ fn4 (1);
>+ }
>+else
>+ fn4 (0);
>+ }
>+ return e;
>+}
>+
>+void
>+fn6 (void)
>+{
>+ f = fn1 (0);
>+}
>
> Jakub
[PATCH] Fix -fcompare-debug failures caused by fixup_noreturn_call (PR debug/63284)
Hi!
The following testcase fails on 4.9 branch (shows
undesirable difference in *.optimized dump on the trunk, but
doesn't report -fcompare-debug failure there, and in 4.8 is latent).
The problem is that if we have a noreturn stmt followed by only
debug stmt, fixup_noreturn_call will try to split_block after the
noreturn call, but with -g0 there are never debug stmts and thus
we would not split the block; that can lead to different order of bb
predecessors, different order of PHI args etc.
As nothing after noreturn call really matters, this patch if there are
only debug stmts after noreturn call removes the debug stmts instead of
the undesirable splitting of the block.
Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and on
the 4.9 branch, ok for trunk/4.9 and eventually 4.8 too?
2014-09-17 Jakub Jelinek
PR debug/63284
* tree-cfgcleanup.c (fixup_noreturn_call): Don't split block
if there are only debug stmts after the noreturn call, instead
remove the debug stmts.
* gcc.dg/pr63284.c: New test.
--- gcc/tree-cfgcleanup.c.jj2014-09-01 13:33:11.0 +0200
+++ gcc/tree-cfgcleanup.c 2014-09-17 11:01:53.448165423 +0200
@@ -565,7 +565,20 @@ fixup_noreturn_call (gimple stmt)
/* First split basic block if stmt is not last. */
if (stmt != gsi_stmt (gsi_last_bb (bb)))
-split_block (bb, stmt);
+{
+ if (stmt == gsi_stmt (gsi_last_nondebug_bb (bb)))
+ {
+ /* Don't split if there are only debug stmts
+after stmt, that can result in -fcompare-debug
+failures. Remove the debug stmts instead,
+they should be all unreachable anyway. */
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ for (gsi_next (&gsi); !gsi_end_p (gsi); )
+ gsi_remove (&gsi, true);
+ }
+ else
+ split_block (bb, stmt);
+}
changed |= remove_fallthru_edge (bb->succs);
--- gcc/testsuite/gcc.dg/pr63284.c.jj 2014-09-17 10:56:59.699677715 +0200
+++ gcc/testsuite/gcc.dg/pr63284.c 2014-09-17 10:56:39.0 +0200
@@ -0,0 +1,42 @@
+/* PR debug/63284 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int a[10], *b, *d, c, f;
+int fn2 (void);
+void fn3 (void);
+void fn4 (int);
+
+static int
+fn1 (int x)
+{
+ int e = a[0];
+ if (e)
+return 1;
+ if (b)
+switch (x)
+ {
+ case 1:
+if (d)
+ e = fn2 ();
+else
+ fn3 ();
+break;
+ case 0:
+if (d)
+ {
+fn3 ();
+if (c)
+ fn4 (1);
+ }
+else
+ fn4 (0);
+ }
+ return e;
+}
+
+void
+fn6 (void)
+{
+ f = fn1 (0);
+}
Jakub
Re: [PATCH, i386, Pointer Bounds Checker 31/x] Pointer Bounds Checker builtins for i386 target
On Wed, Sep 17, 2014 at 6:31 PM, Ilya Enkovich wrote:
>> >> I don't like the way arguments are prepared. For the case above,
>> >> bnd_ldx should have index_register_operand predicate in its pattern,
>> >> and this predicate (and its mode) should be checked in the expander
>> >> code. There are many examples of argument expansion in
>> >> ix86_expand_builtin function, including how Pmode is handled.
>> >>
>> >> Also, please see how target is handled there. Target can be null, so
>> >> REG_P predicate will crash.
>> >>
>> >> You should also select insn patterns depending on BNDmode, not
>> >> TARGET_64BIT.
>> >>
>> >> Please use assign_386_stack_local so stack slots can be shared.
>> >> SLOT_TEMP is intended for short-lived temporaries, you can introduce
>> >> new slots if you need more live values at once.
>> >>
>> >> Uros.
>> >
>> > Thanks for comments! Here is a new version in which I addressed all your
>> > concerns.
>>
>> Unfortunately, it doesn't. The patch only fixed one instance w.r.t to
>> target handling, the one I referred as an example. You still have
>> unchecked target, at least in IX86_BUILTIN_BNDMK.
>>
>> However, you have a general problems in your builtin expansion code,
>> so please look at how other builtins are handled. E.g.:
>>
>> if (optimize || !target
>> || GET_MODE (target) != tmode
>> || !register_operand(target, tmode))
>> target = gen_reg_rtx (tmode);
>>
>> also, here is an example how input operands are prepared:
>>
>> op0 = expand_normal (arg0);
>> op1 = expand_normal (arg1);
>> op2 = expand_normal (arg2);
>> if (!register_operand (op0, Pmode))
>> op0 = ix86_zero_extend_to_Pmode (op0);
>> if (!register_operand (op1, SImode))
>> op1 = copy_to_mode_reg (SImode, op1);
>> if (!register_operand (op2, SImode))
>> op2 = copy_to_mode_reg (SImode, op2);
>>
>> So, Pmode is handled in a special way, even when x32 is not considered.
>>
>> BTW: I wonder if word_mode is needed here, Pmode can be SImode with
>> address prefix (x32).
>>
>> Inside the expanders, please use expand_simple_binop and expand_unop
>> on RTX, not tree expressions. Again, please see many examples.
>
> Thank you for additional explanations. Hope this time I answer your concerns
> correctly :)
Yes, this version is MUCH better. There are further comments down the code.
> 2014-09-17 Ilya Enkovich
>
> * config/i386/i386-builtin-types.def (BND): New.
> (ULONG): New.
> (BND_FTYPE_PCVOID_ULONG): New.
> (VOID_FTYPE_BND_PCVOID): New.
> (VOID_FTYPE_PCVOID_PCVOID_BND): New.
> (BND_FTYPE_PCVOID_PCVOID): New.
> (BND_FTYPE_PCVOID): New.
> (BND_FTYPE_BND_BND): New.
> (PVOID_FTYPE_PVOID_PVOID_ULONG): New.
> (PVOID_FTYPE_PCVOID_BND_ULONG): New.
> (ULONG_FTYPE_VOID): New.
> (PVOID_FTYPE_BND): New.
> * config/i386/i386.c: Include tree-chkp.h, rtl-chkp.h.
> (ix86_builtins): Add
> IX86_BUILTIN_BNDMK, IX86_BUILTIN_BNDSTX,
> IX86_BUILTIN_BNDLDX, IX86_BUILTIN_BNDCL,
> IX86_BUILTIN_BNDCU, IX86_BUILTIN_BNDRET,
> IX86_BUILTIN_BNDNARROW, IX86_BUILTIN_BNDINT,
> IX86_BUILTIN_SIZEOF, IX86_BUILTIN_BNDLOWER,
> IX86_BUILTIN_BNDUPPER.
> (builtin_isa): Add leaf_p and nothrow_p fields.
> (def_builtin): Initialize leaf_p and nothrow_p.
> (ix86_add_new_builtins): Handle leaf_p and nothrow_p
> flags.
> (bdesc_mpx): New.
> (bdesc_mpx_const): New.
> (ix86_init_mpx_builtins): New.
> (ix86_init_builtins): Call ix86_init_mpx_builtins.
> (ix86_emit_cmove): New.
> (ix86_emit_move_max): New.
> (ix86_expand_builtin): Expand IX86_BUILTIN_BNDMK,
> IX86_BUILTIN_BNDSTX, IX86_BUILTIN_BNDLDX,
> IX86_BUILTIN_BNDCL, IX86_BUILTIN_BNDCU,
> IX86_BUILTIN_BNDRET, IX86_BUILTIN_BNDNARROW,
> IX86_BUILTIN_BNDINT, IX86_BUILTIN_SIZEOF,
> IX86_BUILTIN_BNDLOWER, IX86_BUILTIN_BNDUPPER.
> * config/i386/i386.h (ix86_stack_slot): Added SLOT_BND_STORED.
..
> + /* We need to move bounds to memory before any computations. */
> + if (!MEM_P (op1))
> + {
> + m1 = assign_386_stack_local (BNDmode, SLOT_TEMP);
> + emit_move_insn (m1, op1);
> + }
> + else
> + m1 = op1;
No negative conditions, please. Just swap the arms of if sentence. It
is much more readable.
> +
> + /* Generate mem expression to be used for access to LB and UB. */
> + m1h1 = gen_rtx_MEM (Pmode, XEXP (m1, 0));
> + m1h2 = gen_rtx_MEM (Pmode, plus_constant (Pmode, XEXP (m1, 0),
> + GET_MODE_SIZE (Pmode)));
Please use adjust_address instead of manually producing MEMs.
> +
> + t1 = gen_reg_rtx (Pmode);
> +
> + /* Compute LB. */
> + emit_move_insn (t1, m1h1);
> + ix86_emit_move_max (t1, lb);
> + emit_move_
Re: [RFC PATCH] -fsanitize=vptr instrumentation
On 09/17/2014 11:40 AM, Jakub Jelinek wrote:
build_base_path seems to be used in lots of places though, apparently
including member access, etc. The ubsan library right now has just these
const char *TypeCheckKinds[] = {
"load of", "store to", "reference binding to", "member access within",
"member call on", "constructor call on", "downcast of", "downcast of"
};
reasons for the runtime diagnostics (constructor call on, reference
binding to, load of and store to meant for other diagnostics), if what the
vptr-5.C testcase does for the pointer comparison? is not one of these, we'd
need to coordinate with upstream addition of other kinds (but,
build_base_path would need to be told what action it is, or it would need to
be instrumented in the callers of there like build_static_cast_1).
Suggestions on what the other kinds should be?
Well, it's a conversion to virtual base; I suppose we could use "member
access within" until the library is updated.
:(. Well, for NULL one could argue that it was never a pointer to an object
and never will be, but as it could be non-NULL pointer refering to an out of
life object (e.g. deleted), I guess we have to stop instrumenting any
"member accesses" if it is surrounded by ADDR_EXPR, right?
That's my interpretation. It might be worth discussing with the clang
folks what their rationale is.
Based on the DR597 resolution, I guess the only cases -fsanitize=vptr
can instrument are those listed in those bullets there:
- the pointer is used to access a non-static data member or call a non-static
member function of the object, or
- the pointer is implicitly converted (4.10 [conv.ptr]) to a pointer to a
virtual base class type, or
- the pointer is used as the operand of a static_cast (5.2.9
[expr.static.cast]) except when the conversion is to pointer to cv void, or to
pointer to cv void and subsequently to pointer to cv char or pointer to cv
unsigned char, or
- the pointer is used as the operand of a dynamic_cast (5.2.7
[expr.dynamic.cast])...
the first bullet is supposedly instrumented in the patch (except we
instrument ADDR_EXPR we shouldn't), the second, is that what vptr-5.C
above is about?
Yes.
the third one is partially the build_static_cast_1,
but we only instrument downcasts, shouldn't we instrument upcasts too,
or static_cast conversions to say POD types other than to
void/char pointers?
I guess so. This passage is specifically about references to storage
either before or after the lifetime of an object, so it's not entirely
clear that it applies here, but I think it does.
And for the last one, should we before dynamic_cast verify the object
passed to dynamic_cast has the expected vptr?
Perhaps we should just add checking to the dynamic_cast code. I'm not
sure if that would slow it down enough to merit a separate entry point
for checking.
Jason
Re: Stream ODR types
Hi,
this patch renames types reported by Wodr during LTO bootstrap.
Bootrapping/regtesting in progress, OK if it passes?
Honza
* tree-ssa-ccp.c (prop_value_d): Rename to ...
(ccp_prop_value_t): ... this one to avoid ODR violation; update uses.
* ipa-prop.c (struct type_change_info): Rename to ...
(prop_type_change_infoprop_type_change_info): ... this; update uses.
* ggc-page.c (globals): Rename to ...
(static struct ggc_globals): ... this; update uses.
* tree-ssa-loop-im.c (mem_ref): Rename to ...
(im_mem_ref): ... this; update uses.
* ggc-common.c (loc_descriptor): Rename to ...
(ggc_loc_descriptor): ... this; update uses.
* lra-eliminations.c (elim_table): Rename to ...
(lra_elim_table): ... this; update uses.
* bitmap.c (output_info): Rename to ...
(bitmap_output_info): ... this; update uses.
* gcse.c (expr): Rename to ...
(gcse_expr) ... this; update uses.
(occr): Rename to ...
(gcse_occr): .. this; update uses.
* tree-ssa-copy.c (prop_value_d): Rename to ...
(prop_value_t): ... this.
* predict.c (block_info_def): Rename to ...
(block_info): ... this; update uses.
(edge_info_def): Rename to ...
(edge_info): ... this; update uses.
* profile.c (bb_info): Rename to ...
(bb_profile_info): ... this; update uses.
* alloc-pool.c (output_info): Rename to ...
(pool_output_info): ... this; update uses.
Index: tree-ssa-ccp.c
===
--- tree-ssa-ccp.c (revision 215328)
+++ tree-ssa-ccp.c (working copy)
@@ -166,7 +166,7 @@ typedef enum
VARYING
} ccp_lattice_t;
-struct prop_value_d {
+struct ccp_prop_value_t {
/* Lattice value. */
ccp_lattice_t lattice_val;
@@ -180,24 +180,22 @@ struct prop_value_d {
widest_int mask;
};
-typedef struct prop_value_d prop_value_t;
-
/* Array of propagated constant values. After propagation,
CONST_VAL[I].VALUE holds the constant value for SSA_NAME(I). If
the constant is held in an SSA name representing a memory store
(i.e., a VDEF), CONST_VAL[I].MEM_REF will contain the actual
memory reference used to store (i.e., the LHS of the assignment
doing the store). */
-static prop_value_t *const_val;
+static ccp_prop_value_t *const_val;
static unsigned n_const_val;
-static void canonicalize_value (prop_value_t *);
+static void canonicalize_value (ccp_prop_value_t *);
static bool ccp_fold_stmt (gimple_stmt_iterator *);
/* Dump constant propagation value VAL to file OUTF prefixed by PREFIX. */
static void
-dump_lattice_value (FILE *outf, const char *prefix, prop_value_t val)
+dump_lattice_value (FILE *outf, const char *prefix, ccp_prop_value_t val)
{
switch (val.lattice_val)
{
@@ -236,10 +234,10 @@ dump_lattice_value (FILE *outf, const ch
/* Print lattice value VAL to stderr. */
-void debug_lattice_value (prop_value_t val);
+void debug_lattice_value (ccp_prop_value_t val);
DEBUG_FUNCTION void
-debug_lattice_value (prop_value_t val)
+debug_lattice_value (ccp_prop_value_t val)
{
dump_lattice_value (stderr, "", val);
fprintf (stderr, "\n");
@@ -272,10 +270,10 @@ extend_mask (const wide_int &nonzero_bit
4- Initial values of variables that are not GIMPLE registers are
considered VARYING. */
-static prop_value_t
+static ccp_prop_value_t
get_default_value (tree var)
{
- prop_value_t val = { UNINITIALIZED, NULL_TREE, 0 };
+ ccp_prop_value_t val = { UNINITIALIZED, NULL_TREE, 0 };
gimple stmt;
stmt = SSA_NAME_DEF_STMT (var);
@@ -343,10 +341,10 @@ get_default_value (tree var)
/* Get the constant value associated with variable VAR. */
-static inline prop_value_t *
+static inline ccp_prop_value_t *
get_value (tree var)
{
- prop_value_t *val;
+ ccp_prop_value_t *val;
if (const_val == NULL
|| SSA_NAME_VERSION (var) >= n_const_val)
@@ -366,7 +364,7 @@ get_value (tree var)
static inline tree
get_constant_value (tree var)
{
- prop_value_t *val;
+ ccp_prop_value_t *val;
if (TREE_CODE (var) != SSA_NAME)
{
if (is_gimple_min_invariant (var))
@@ -387,7 +385,7 @@ get_constant_value (tree var)
static inline void
set_value_varying (tree var)
{
- prop_value_t *val = &const_val[SSA_NAME_VERSION (var)];
+ ccp_prop_value_t *val = &const_val[SSA_NAME_VERSION (var)];
val->lattice_val = VARYING;
val->value = NULL_TREE;
@@ -413,7 +411,7 @@ set_value_varying (tree var)
For other constants, make sure to drop TREE_OVERFLOW. */
static void
-canonicalize_value (prop_value_t *val)
+canonicalize_value (ccp_prop_value_t *val)
{
enum machine_mode mode;
tree type;
@@ -451,7 +449,7 @@ canonicalize_value (prop_value_t *val)
/* Return whether the lattice transition is valid. */
static bool
-valid_lattice_transition (prop_value_t old_val, prop_value_t new_va
Re: [C++14 PATCH] Minor cleanup, don't run tests for which compilation is enough.
Applied, thanks. Jason
Re: [PATCH 1/2, x86] Add palignr support for AVX2.
The test in pr62128 is exactly TEST 22 from gcc.dg/torture/vshuf-v32qi.c. It will check if the pattern is correct or not. Resubmitting patch looks good as current mail thread is already too complicated. On Wed, Sep 17, 2014 at 6:49 PM, H.J. Lu wrote: > On Wed, Sep 17, 2014 at 6:01 AM, Evgeny Stupachenko > wrote: >> It fix "gcc.target/i386/pr52252-atom.c" in core-avx2 make check and pr62128. >> > > I suggest you resubmit the patch as a bug fix for pr62128 with > testcases from pr62128 as well as gcc.target/i386/pr52252-atom.c. > > > -- > H.J.
[PATCHv4] Vimrc config with GNU formatting
On 09/16/2014 08:38 PM, Yury Gribov wrote:
Hi all,
This is the third version of the patch. A list of changes since last
version:
* move config to contrib so that it's _not_ enabled by default (current
score is 2/1 in favor of no Vim config by default)
* update Makefile.in to make .local.vimrc if developer asks for it
* disable autoformatting for flex files
* fix filtering of non-GNU sources (libsanitizer)
* added some small fixes in cinoptions based on feedback from community
As noted by Richard, the config does not do a good job of formatting
unbound {} blocks e.g.
void
foo ()
{
int x;
{
// I'm an example of bad bad formatting
}
}
but it seems to be the best we can get with Vim's cindent
(and I don't think anyone seriously considers writing a custom indentexpr).
Ok to commit?
-Y
New vesion with support for another popular local .vimrc plugin.
-Y
commit fa7d6c68b0eb876aef6c22e4c88aa82db05e
Author: Yury Gribov
Date: Thu Sep 4 16:55:44 2014 +0400
2014-09-17 Laurynas Biveinis
Yury Gribov
Vim config with GNU formatting.
contrib/
* vimrc: New file.
/
* .gitignore: Added .local.vimrc and .lvimrc.
* Makefile.tpl (.local.vimrc): New target.
* Makefile.in: Regenerate.
diff --git a/.gitignore b/.gitignore
index e9b56be..ab97ac6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,9 @@ POTFILES
TAGS
TAGS.sub
+.local.vimrc
+.lvimrc
+
.gdbinit
.gdb_history
diff --git a/Makefile.in b/Makefile.in
index d6105b3..1be75b7 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -2384,6 +2384,11 @@ mail-report-with-warnings.log: warning.log
chmod +x $@
echo If you really want to send e-mail, run ./$@ now
+# Local Vim config
+
+vimrc:
+ (cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc; $(LN_S) contrib/vimrc .lvimrc)
+
# Installation targets.
.PHONY: install uninstall
diff --git a/Makefile.tpl b/Makefile.tpl
index f7c7e38..ee2321f 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -867,6 +867,11 @@ mail-report-with-warnings.log: warning.log
chmod +x $@
echo If you really want to send e-mail, run ./$@ now
+# Local Vim config
+
+vimrc:
+ (cd $(srcdir); $(LN_S) contrib/vimrc .local.vimrc; $(LN_S) contrib/vimrc .lvimrc)
+
# Installation targets.
.PHONY: install uninstall
diff --git a/contrib/vimrc b/contrib/vimrc
new file mode 100644
index 000..88e77a6
--- /dev/null
+++ b/contrib/vimrc
@@ -0,0 +1,45 @@
+" Code formatting settings for Vim.
+"
+" To enable this for GCC files by default, install thinca's vim-localrc
+" plugin and do
+" $ make .local.vimrc
+" Or install Markus Braun's localvimrc and do
+" $ make .lvimrc
+" Or if you dislike plugins, add autocmd in your ~/.vimrc:
+" :au BufNewFile,BufReadPost path/to/gcc/* :so path/to/gcc/contrib/vimrc
+" Or just source file manually every time if you are masochist:
+" :so path/to/gcc/contrib/vimrc
+"
+" Copyright (C) 2014 Free Software Foundation, Inc.
+"
+" This program is free software; you can redistribute it and/or modify
+" it under the terms of the GNU General Public License as published by
+" the Free Software Foundation; either version 3 of the License, or
+" (at your option) any later version.
+"
+" This program is distributed in the hope that it will be useful,
+" but WITHOUT ANY WARRANTY; without even the implied warranty of
+" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+" GNU General Public License for more details.
+"
+" You should have received a copy of the GNU General Public License
+" along with this program. If not, see
[PATCH, rs6000] Improve power8 fusion peepholes
This patch is an intermediate step of what I want to do to improve power8
fusion.
In the current trunk, the fusion support for gpr loads is done by a peephole2
to find the addis followed by the load instruction where the only consumer of
the addis instruction is the load, and it rewrites the addis to use the
register that will be loaded, and emits the two separate instructions. There
is then a normal peephole that recognizes the addis/load combination, and makes
sure they are emited together, along with a comment, to make tracking of the
fusion attempts easier. The problem is things like the second scheduler pass
will move things around, and often times move the addis away from the load.
This means the normal peephole pass won't see the two instructions.
This patch creates a new insn that combines the two parts, so that the
scheduler2 pass won't split up the two insns. In doing static analysis, a lot
more fused pairs are generated. For instance, 400.perlbench generates more
than 11,300 more load fusion with these patches, 403.gcc generates 23,000 more
load fusions, and 416.gamess generates 39,000 more load fusions.
However, when spec 2006 is run on a power8, you don't actually see much of a
performance difference with these patches. In digging into it, the main place
where fusion occurs is in referencing static/global variables. The spec 2006
suite does not tend to have that much static/global data, so the linker
optimizes most of the addis instructions to be nops, and the load index
register is adjusted to use r2. These optimizations should help much larger
code bases that do have a lot more static/global data.
I've done bootstraps on both a big endian power7 and a little endian power8
with no regressions. Are these patches ok to install in the trunk, and the
4.8/4.9 branches?
2014-09-16 Michael Meissner
* config/rs6000/predicates.md (fusion_gpr_mem_load): Move testing
for base_reg_operand to be common between LO_SUM and PLUS.
(fusion_gpr_mem_combo): New predicate to match a fused address
that combines the addis and memory offset address.
* config/rs6000/rs6000-protos.h (fusion_gpr_load_p): Change
calling signature.
(emit_fusion_gpr_load): Likewise.
* config/rs6000/rs6000.c (fusion_gpr_load_p): Change calling
signature to pass each argument separately, rather than
using an operands array. Rewrite the insns found by peephole2 to
be a single insn, rather than hoping the insns will still be
together when the peephole pass is done. Drop being called via a
normal peephole.
(emit_fusion_gpr_load): Change calling signature to be called from
the fusion_gpr_load_ insns with a combined memory address
instead of the peephole pass passing the addis and offset
separately.
* config/rs6000/rs6000.md (UNSPEC_FUSION_GPR): New unspec for GPR
fusion.
(power8 fusion peephole): Drop support for doing power8 via a
normal peephole that was created by the peephole2 pass.
(power8 fusion peephole2): Create a new insn with the fused
address, so that the fused operation is kept together after
register allocation is done.
(fusion_gpr_load_): Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/predicates.md
===
--- gcc/config/rs6000/predicates.md (revision 215301)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1797,7 +1797,7 @@ (define_predicate "fusion_gpr_addis"
(define_predicate "fusion_gpr_mem_load"
(match_code "mem,sign_extend,zero_extend")
{
- rtx addr;
+ rtx addr, base, offset;
/* Handle sign/zero extend. */
if (GET_CODE (op) == ZERO_EXTEND
@@ -1827,24 +1827,79 @@ (define_predicate "fusion_gpr_mem_load"
}
addr = XEXP (op, 0);
+ if (GET_CODE (addr) != PLUS && GET_CODE (addr) != LO_SUM)
+return 0;
+
+ base = XEXP (addr, 0);
+ if (!base_reg_operand (base, GET_MODE (base)))
+return 0;
+
+ offset = XEXP (addr, 1);
+
if (GET_CODE (addr) == PLUS)
+return satisfies_constraint_I (offset);
+
+ else if (GET_CODE (addr) == LO_SUM)
{
- rtx base = XEXP (addr, 0);
- rtx offset = XEXP (addr, 1);
+ if (TARGET_XCOFF || (TARGET_ELF && TARGET_POWERPC64))
+ return small_toc_ref (offset, GET_MODE (offset));
- return (base_reg_operand (base, GET_MODE (base))
- && satisfies_constraint_I (offset));
+ else if (TARGET_ELF && !TARGET_POWERPC64)
+ return CONSTANT_P (offset);
}
- else if (GET_CODE (addr) == LO_SUM)
+ return 0;
+})
+
+;; Match a GPR load (lbz, lhz, lwz, ld) that uses a combined address in the
+;; memory field with both the addis and the memory offset. Sign extension
+;; is not handled here, since lha and
Re: [PATCH, i386, Pointer Bounds Checker 31/x] Pointer Bounds Checker builtins for i386 target
On 17 Sep 11:42, Uros Bizjak wrote:
> On Wed, Sep 17, 2014 at 10:11 AM, Ilya Enkovich
> wrote:
> > On 16 Sep 12:02, Uros Bizjak wrote:
> >>
> >> Hm, can this patch be compiled as part of the series? The expanders
> >> refer to various gen_bnd patterns that I don't see. Also, I don't see
> >> BND mode introduced.
> >
> > Hi,
> >
> > Here is a patch from the series that introduces modes and instructions:
> > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00880.html. It needs update
> > in bndldx expander as you suggested.
> >
> >>
> >> Anyway, some general observations:
> >>
> >> > +case IX86_BUILTIN_BNDLDX:
> >> > + arg0 = CALL_EXPR_ARG (exp, 0);
> >> > + arg1 = CALL_EXPR_ARG (exp, 1);
> >> > +
> >> > + op0 = expand_normal (arg0);
> >> > + op1 = expand_normal (arg1);
> >> > +
> >> > + op0 = force_reg (Pmode, op0);
> >> > + op1 = force_reg (Pmode, op1);
> >> > +
> >> > + /* Avoid registers which connot be used as index. */
> >> > + if (!index_register_operand (op1, Pmode))
> >> > + {
> >> > + rtx temp = gen_reg_rtx (Pmode);
> >> > + emit_move_insn (temp, op1);
> >> > + op1 = temp;
> >> > + }
> >> > +
> >> > + /* If op1 was a register originally then it may have
> >> > +mode other than Pmode. We need to extend in such
> >> > +case because bndldx may work only with Pmode regs. */
> >> > + if (GET_MODE (op1) != Pmode)
> >> > + op1 = ix86_zero_extend_to_Pmode (op1);
> >> > +
> >> > + if (REG_P (target))
> >> > + emit_insn (TARGET_64BIT
> >> > + ? gen_bnd64_ldx (target, op0, op1)
> >> > + : gen_bnd32_ldx (target, op0, op1));
> >> > + else
> >> > + {
> >> > + rtx temp = gen_reg_rtx (BNDmode);
> >> > + emit_insn (TARGET_64BIT
> >> > +? gen_bnd64_ldx (temp, op0, op1)
> >> > +: gen_bnd32_ldx (temp, op0, op1));
> >> > + emit_move_insn (target, temp);
> >> > + }
> >> > + return target;
> >>
> >> I don't like the way arguments are prepared. For the case above,
> >> bnd_ldx should have index_register_operand predicate in its pattern,
> >> and this predicate (and its mode) should be checked in the expander
> >> code. There are many examples of argument expansion in
> >> ix86_expand_builtin function, including how Pmode is handled.
> >>
> >> Also, please see how target is handled there. Target can be null, so
> >> REG_P predicate will crash.
> >>
> >> You should also select insn patterns depending on BNDmode, not
> >> TARGET_64BIT.
> >>
> >> Please use assign_386_stack_local so stack slots can be shared.
> >> SLOT_TEMP is intended for short-lived temporaries, you can introduce
> >> new slots if you need more live values at once.
> >>
> >> Uros.
> >
> > Thanks for comments! Here is a new version in which I addressed all your
> > concerns.
>
> Unfortunately, it doesn't. The patch only fixed one instance w.r.t to
> target handling, the one I referred as an example. You still have
> unchecked target, at least in IX86_BUILTIN_BNDMK.
>
> However, you have a general problems in your builtin expansion code,
> so please look at how other builtins are handled. E.g.:
>
> if (optimize || !target
> || GET_MODE (target) != tmode
> || !register_operand(target, tmode))
> target = gen_reg_rtx (tmode);
>
> also, here is an example how input operands are prepared:
>
> op0 = expand_normal (arg0);
> op1 = expand_normal (arg1);
> op2 = expand_normal (arg2);
> if (!register_operand (op0, Pmode))
> op0 = ix86_zero_extend_to_Pmode (op0);
> if (!register_operand (op1, SImode))
> op1 = copy_to_mode_reg (SImode, op1);
> if (!register_operand (op2, SImode))
> op2 = copy_to_mode_reg (SImode, op2);
>
> So, Pmode is handled in a special way, even when x32 is not considered.
>
> BTW: I wonder if word_mode is needed here, Pmode can be SImode with
> address prefix (x32).
>
> Inside the expanders, please use expand_simple_binop and expand_unop
> on RTX, not tree expressions. Again, please see many examples.
Thank you for additional explanations. Hope this time I answer your concerns
correctly :)
>
> Please use emit_move_insn instead of
>
> emit_insn (gen_move_insn (m1, op1));
>
> As a wish, can you perhaps write a generic cmove expander to be also
> used in other places?
I added ix86_emit_cmove as a general form of ix86_emit_move_max.
>
> > +static void
> > +ix86_emit_move_max (rtx dst, rtx src)
>
> Uros.
>
Thanks,
Ilya
--
2014-09-17 Ilya Enkovich
* config/i386/i386-builtin-types.def (BND): New.
(ULONG): New.
(BND_FTYPE_PCVOID_ULONG): New.
(VOID_FTYPE_BND_PCVOID): New.
(VOID_FTYPE_PCVOID_PCVOID_BND): New.
(BND_FTYPE_PCVOID_PCVOID): New.
(BND_FTYPE_PCVOID): New.
(BND_FTYPE_BND_BND): New.
(PVOID_FTYPE_PVOID_PVOID_ULONG): New.
(PVOID_F
Re: [PATCH 2/2] Add some more test cases for fentry and pg
> Well, so why does Yo're right. It's actually not supported. I'll use the method you suggested earlier. -Andi
Fix issues with ODR type comparsions
Hi,
this patch fixes some issues with ODR type comparsions I noticed while looking
into different applications. There are some additional warnings output now
instead of ICE because comparsions is done recursively when given ODR type is
known
to have violation in it.
More informative warnings are output on mismatch in number of fileds and some
extra cases.
Bootstrapped/regtested x86_64-linux, LTO bootstrapped and tested on
Firefox/libreoffice
libreoffice currently fails on libiberty ELF handling bug though.
Honza
* ipa-devirt.c (type_pair, default_hashset_traits): New types.
(odr_types_equivalent_p): Use pair hash.
(odr_subtypes_equivalent_p): Likewise, do structural compare
on ODR types that may be mismatched.
(warn_odr): Support warning when only one field is given.
(odr_types_equivalent_p): Strenghten comparsions made;
support VOIDtype.
(add_type_duplicate): Update VISITED hash set.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 215196)
+++ ipa-devirt.c(working copy)
@@ -136,8 +136,44 @@ along with GCC; see the file COPYING3.
#include "intl.h"
#include "hash-map.h"
+/* Hash based set of pairs of types. */
+typedef struct
+{
+ tree first;
+ tree second;
+} type_pair;
+
+struct pair_traits : default_hashset_traits
+{
+ static hashval_t
+ hash (type_pair p)
+ {
+return TYPE_UID (p.first) ^ TYPE_UID (p.second);
+ }
+ static bool
+ is_empty (type_pair p)
+ {
+return p.first == NULL;
+ }
+ static bool
+ is_deleted (type_pair p ATTRIBUTE_UNUSED)
+{
+ return false;
+}
+ static bool
+ equal (const type_pair &a, const type_pair &b)
+{
+ return a.first==b.first && a.second == b.second;
+}
+ static void
+ mark_empty (type_pair &e)
+{
+ e.first = NULL;
+}
+};
+
static bool odr_types_equivalent_p (tree, tree, bool, bool *,
- hash_set *);
+ hash_set *);
static bool odr_violation_reported = false;
@@ -471,7 +507,7 @@ set_type_binfo (tree type, tree binfo)
/* Compare T2 and T2 based on name or structure. */
static bool
-odr_subtypes_equivalent_p (tree t1, tree t2, hash_set *visited)
+odr_subtypes_equivalent_p (tree t1, tree t2, hash_set
*visited)
{
bool an1, an2;
@@ -489,29 +525,39 @@ odr_subtypes_equivalent_p (tree t1, tree
if (an1 != an2 || an1)
return false;
- /* For types where we can not establish ODR equivalency (either by ODR names
- or by virtual tables), recurse and deeply compare. */
- if ((!odr_type_p (t1) || !odr_type_p (t2))
- && (TREE_CODE (t1) != RECORD_TYPE || TREE_CODE (t2) != RECORD_TYPE
- || !TYPE_BINFO (t1) || !TYPE_BINFO (t2)
- || !polymorphic_type_binfo_p (TYPE_BINFO (t1))
- || !polymorphic_type_binfo_p (TYPE_BINFO (t2
+ /* For ODR types be sure to compare their names. */
+ if ((odr_type_p (t1) && !odr_type_p (t2))
+ || (TREE_CODE (t1) == RECORD_TYPE && TREE_CODE (t2) == RECORD_TYPE
+ && TYPE_BINFO (t1) && TYPE_BINFO (t2)
+ && polymorphic_type_binfo_p (TYPE_BINFO (t1))
+ && polymorphic_type_binfo_p (TYPE_BINFO (t2
{
- if (TREE_CODE (t1) != TREE_CODE (t2))
- return false;
- if ((TYPE_NAME (t1) == NULL_TREE) != (TYPE_NAME (t2) == NULL_TREE))
- return false;
- if (TYPE_NAME (t1) && DECL_NAME (TYPE_NAME (t1)) != DECL_NAME (TYPE_NAME
(t2)))
- return false;
- /* This should really be a pair hash, but for the moment we do not need
-100% reliability and it would be better to compare all ODR types so
-recursion here is needed only for component types. */
- if (visited->add (t1))
- return true;
- return odr_types_equivalent_p (t1, t2, false, NULL, visited);
+ if (!types_same_for_odr (t1, t2))
+return false;
+ /* Limit recursion: If subtypes are ODR types and we know
+ that they are same, be happy. */
+ if (!get_odr_type (t1, true)->odr_violated)
+return true;
}
- return types_same_for_odr (t1, t2);
+ /* Component types, builtins and possibly vioalting ODR types
+ have to be compared structurally. */
+ if (TREE_CODE (t1) != TREE_CODE (t2))
+return false;
+ if ((TYPE_NAME (t1) == NULL_TREE) != (TYPE_NAME (t2) == NULL_TREE))
+return false;
+ if (TYPE_NAME (t1) && DECL_NAME (TYPE_NAME (t1)) != DECL_NAME (TYPE_NAME
(t2)))
+return false;
+
+ type_pair pair={t1,t2};
+ if (TYPE_UID (t1) > TYPE_UID (t2))
+{
+ pair.first = t2;
+ pair.second = t1;
+}
+ if (visited->add (pair))
+return true;
+ return odr_types_equivalent_p (t1, t2, false, NULL, visited);
}
/* Compare two virtual tables, PREVAILING and VTABLE and output ODR
@@ -646,16 +692,25 @@ warn_odr (tree t1, tree t2, tree st1, tr
"type %qT violates one definition rule",
Re: [PATCH] Add header guard to several header files.
On Wed, 17 Sep 2014, Kito Cheng wrote: > Updated patch OK except for the changes to target-def.h and target-hooks-macros.h. (Those aren't exactly normal headers that could reasonably be included more than once in a source file; they have dependencies on where they get included and what's defined before/after inclusion. So while I suspect the include guards would not cause any problems in those headers, it's not obvious they're desirable either.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, 17 Sep 2014, Janne Blomqvist wrote: > > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:41: error: 'gfc_unit' > > has no member named 'file' > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > > ^ > > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:50: error: 'gfc_unit' > > has no member named 'file_len' > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > > ^ > > make[3]: *** [inquire.lo] Error 1 > > > > brgds, H-P > > Oops, I forgot to update some parts in an #ifdef branch that isn't > taken on my target. I'll try to find time to fix it later tonight. If > you're in a hurry, just replace > > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > > with > > cf_strcpy (iqp->name, iqp->name_len, u->filename); > > in inquire.c. Thanks, build completes and I'll commit the following as obvious if there are no regressions. (The indentation change is correct; lining up with the "else". Note the closing brace.) * libgfortran/io/inquire.c (inquire_via_unit) [!HAVE_TTYNAME && !HAVE_TTYNAME_R && !__MINGW32__]: Adjust for last commit. Index: libgfortran/io/inquire.c === --- libgfortran/io/inquire.c(revision 215321) +++ libgfortran/io/inquire.c(working copy) @@ -94,7 +94,7 @@ inquire_via_unit (st_parameter_inquire * else fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); #else -fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); + cf_fstrcpy (iqp->name, iqp->name_len, u->filename); #endif } brgds, H-P
Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)
On Wed, 17 Sep 2014, Alan Lawrence wrote: > We've just noticed this patch causes an ICE in > gcc.c-torture/execute/scal-to-vec1.c at -O3 when running with -fPIC on > arm-none-linux-gnueabi and arm-none-linux-gnueabihf; test logs: Which part causes the ICE? The arm_hard_regno_mode_ok change relating to modes assigned to CC_REGNUM, the arm_regno_class change relating to PC_REGNUM, or something else? Either of those would indicate something very strange going on in LRA (maybe something else needs to change somewhere as well to stop attempts to use CC_REGNUM or PC_REGNUM inappropriately?). -- Joseph S. Myers jos...@codesourcery.com
Re: [kyukhin/gomp4-offload] DESTDIR issues
Il 17/09/2014 16:09, Jakub Jelinek ha scritto: >> > Is the offloading compiler built together with GCC or previously? If >> > the latter, what's the difference between the offloading compiler and >> > say gmp? Setting the LD_LIBRARY_PATH would be the responsibility of >> > whoever builds GCC; it would be the same here for the COMPILER_PATH. > Right now previously, but (usually) from the very same sources, so I think > it will be usually (but not always?) desirable to build them together, perhaps > not in a single toplevel configure and single toplevel make. If it's from the same sources, that probably rules out a single toplevel configure/make. Is it a problem if a newer GCC is compiled with older headers from the offloading compiler? In any case, I would go with a configure option. Paolo
Re: [PATCH] RTEMS: Update contrib/config-list.mk
On 9/17/2014 10:41 AM, Sebastian Huber wrote: > On 09/17/2014 04:45 PM, Jan-Benedict Glaw wrote: >> On Wed, 2014-09-17 15:37:32 +0200, Sebastian >> Huber wrote: contrib/ChangeLog 2014-09-17 Sebastian Huber * config-list.mk (LIST): Add arm-rtems. Add nios2-rtems. Remove extra option from powerpc-rtems. >> What's the rationale for removing --enable-threads=yes here, as well >> as the specific version number? > The version number can be arbitrary. All the other RTEMS targets in > this list omit it. > > The --enable-threads=yes is not specific to PowerPC for RTEMS. So all > RTEMS targets should have it or none. With a recent commit this option > is superfluous. > I was noticing that at least the v850 is not represented by any target. Other than that, it looks like all the RTEMS targets except that the or1k (which is not in the FSF tree yet) are included. I didn't review against the list of *-elf targets though. What is the rationale for inclusion on the list? Should v850-elf and v850-rtems also be added? And is this the input to your buildbot? :) -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherr...@oarcorp.comOn-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available(256) 722-9985
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On Wed, Sep 17, 2014 at 05:46:45PM +0200, Andi Kleen wrote:
> On Wed, Sep 17, 2014 at 04:51:33PM +0200, Jakub Jelinek wrote:
> > On Wed, Sep 17, 2014 at 04:39:00PM +0200, Andi Kleen wrote:
> > > Ok, this should fix it:
> > >
> > > I'll commit it as obvious after testing unless there are objections.
> >
> > This isn't sufficient.
> > If -mfentry isn't compatible with -m32 -fpic, supposedly you need
>
> -mfentry works fine with -m32 -fpic as far as I know.
>
> [my recent pending patch adding some extensions didn't support it
> though, but that's not tested here]
Well, so why does
make -k check-gcc
RUNTESTFLAGS='--target_board=unix\{-m32,-m32/-fpic,-m64,-m64/-fpic\}
i386.exp=fentry*.c'
fail for -m32/-fpic then on x86_64-linux (current trunk)?
/usr/src/gcc/gcc/testsuite/gcc.target/i386/fentry-override.c:1:0: sorry,
unimplemented: -mfentry isn't supported for 32-bit in combination with -fpic
/usr/src/gcc/gcc/testsuite/gcc.target/i386/fentry.c:1:0: sorry, unimplemented:
-mfentry isn't supported for 32-bit in combination with -fpic
Jakub
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On Wed, Sep 17, 2014 at 04:51:33PM +0200, Jakub Jelinek wrote: > On Wed, Sep 17, 2014 at 04:39:00PM +0200, Andi Kleen wrote: > > Ok, this should fix it: > > > > I'll commit it as obvious after testing unless there are objections. > > This isn't sufficient. > If -mfentry isn't compatible with -m32 -fpic, supposedly you need -mfentry works fine with -m32 -fpic as far as I know. [my recent pending patch adding some extensions didn't support it though, but that's not tested here] -Andi
Re: [PATCH] RTEMS: Update contrib/config-list.mk
On 09/17/2014 04:45 PM, Jan-Benedict Glaw wrote: On Wed, 2014-09-17 15:37:32 +0200, Sebastian Huber wrote: >contrib/ChangeLog >2014-09-17 Sebastian Huber > >* config-list.mk (LIST): Add arm-rtems. >Add nios2-rtems. Remove extra option from powerpc-rtems. What's the rationale for removing --enable-threads=yes here, as well as the specific version number? The version number can be arbitrary. All the other RTEMS targets in this list omit it. The --enable-threads=yes is not specific to PowerPC for RTEMS. So all RTEMS targets should have it or none. With a recent commit this option is superfluous. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [RFC PATCH] -fsanitize=vptr instrumentation
On Wed, Sep 17, 2014 at 10:27:02AM -0400, Jason Merrill wrote:
> On 09/16/2014 10:56 AM, Jakub Jelinek wrote:
> >vptr-5.C is one Jason mailed me yesterday, clang++ doesn't instrument this
> >and g++ right now doesn't either, build_static_cast_1 certainly isn't called
> >in that case, and I must say I have no idea what should be checked there,
> >where etc.
>
> What needs to be checked is conversion (in this case implicit) to a virtual
> base; if the vptr doesn't point to a vtable that has the appropriate vbase
> offset, we should complain.
>
> virtual base conversions are implemented in build_base_path under if
> (virtual_access).
build_base_path seems to be used in lots of places though, apparently
including member access, etc. The ubsan library right now has just these
const char *TypeCheckKinds[] = {
"load of", "store to", "reference binding to", "member access within",
"member call on", "constructor call on", "downcast of", "downcast of"
};
reasons for the runtime diagnostics (constructor call on, reference
binding to, load of and store to meant for other diagnostics), if what the
vptr-5.C testcase does for the pointer comparison? is not one of these, we'd
need to coordinate with upstream addition of other kinds (but,
build_base_path would need to be told what action it is, or it would need to
be instrumented in the callers of there like build_static_cast_1).
Suggestions on what the other kinds should be?
> >vptr-6.C shows where the this optimization is performed and where it isn't
> >(clang++ has 10 instrumentations in T::h and 1 in S::l, g++ has fewer than
> >that, but not 0 in T::h (1 in S::l is right and needed I think)).
>
> I agree that 0 is enough for T::h and 1 for S::l.
>
> >I hope all of f[1-6] is invalid, I really don't see how we could instrument
> >member accesses otherwise (we'd need to limit to not taking address of it);
> >NULL pointer shouldn't point at a valid object.
>
> I don't see anything in the standard saying that these are undefined, only
> that trying to access the (non-)object pointed to is undefined. It would be
> undefined if a conversion to virtual base were involved, i.e.
>
> struct V: virtual R { };
>
> // undefined if p doesn't point to a V because of the conversion to
> // virtual base R
> int* f7 (V* p) { return &p->r; }
>
> These conditions were loosened in C++11 by DRs 597 and 1531; before that it
> was reasonable to regard f[1-6] as undefined, and perhaps clang is using the
> earlier interpretation.
:(. Well, for NULL one could argue that it was never a pointer to an object
and never will be, but as it could be non-NULL pointer refering to an out of
life object (e.g. deleted), I guess we have to stop instrumenting any
"member accesses" if it is surrounded by ADDR_EXPR, right?
So, while we'll instrument
T *p;
...
int i = p->a;
we won't be able to instrument
int *ip = &p->a;
int i = *ip;
Based on the DR597 resolution, I guess the only cases -fsanitize=vptr
can instrument are those listed in those bullets there:
- the pointer is used to access a non-static data member or call a non-static
member function of the object, or
- the pointer is implicitly converted (4.10 [conv.ptr]) to a pointer to a
virtual base class type, or
- the pointer is used as the operand of a static_cast (5.2.9
[expr.static.cast]) except when the conversion is to pointer to cv void, or to
pointer to cv void and subsequently to pointer to cv char or pointer to cv
unsigned char, or
- the pointer is used as the operand of a dynamic_cast (5.2.7
[expr.dynamic.cast])...
the first bullet is supposedly instrumented in the patch (except we
instrument ADDR_EXPR we shouldn't), the second, is that what vptr-5.C
above is about?, the third one is partially the build_static_cast_1,
but we only instrument downcasts, shouldn't we instrument upcasts too,
or static_cast conversions to say POD types other than to
void/char pointers?
And for the last one, should we before dynamic_cast verify the object
passed to dynamic_cast has the expected vptr?
> >+ TREE_SIDE_EFFECTS (cond) = 1;
> ...
> >+ TREE_SIDE_EFFECTS (hash) = 1;
>
> Why do you need to set TREE_SIDE_EFFECTS on these?
I guess I can try to remove those, at some point I had in the patch
TRUTH_AND_EXPR instead of TRUTH_ANDIF_EXPR for the downcasts, and
some TREE_SIDE_EFFECTS I've added until I've noticed the missing IF.
> >+ if (current_function_decl == NULL_TREE
> >+ || lookup_attribute ("no_sanitize_undefined",
> >+ DECL_ATTRIBUTES (current_function_decl)))
> >+return NULL_TREE;
>
>
[C++ Patch] PR 62232
Hi,
clang recently, in 3.5.0 if I remember correctly, stopped
-Wnon-virtual-dtor warning for final classes. I think it makes sense to
do the same.
Tested x86_64-linux.
Thanks,
Paolo.
7
/cp
2014-09-17 Paolo Carlini
PR c++/62232
* class.c (finish_struct_1): Do not -Wnon-virtual-dtor warn
for final class types.
/testsuite
2014-09-17 Paolo Carlini
PR c++/62232
* g++.dg/cpp0x/Wdtor1.C: New.
Index: cp/class.c
===
--- cp/class.c (revision 215325)
+++ cp/class.c (working copy)
@@ -6506,7 +6506,8 @@ finish_struct_1 (tree t)
/* This warning does not make sense for Java classes, since they
cannot have destructors. */
if (!TYPE_FOR_JAVA (t) && warn_nonvdtor
- && TYPE_POLYMORPHIC_P (t) && accessible_nvdtor_p (t))
+ && TYPE_POLYMORPHIC_P (t) && accessible_nvdtor_p (t)
+ && !CLASSTYPE_FINAL (t))
warning (OPT_Wnon_virtual_dtor,
"%q#T has virtual functions and accessible"
" non-virtual destructor", t);
Index: testsuite/g++.dg/cpp0x/Wdtor1.C
===
--- testsuite/g++.dg/cpp0x/Wdtor1.C (revision 0)
+++ testsuite/g++.dg/cpp0x/Wdtor1.C (working copy)
@@ -0,0 +1,13 @@
+// PR c++/62232
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wnon-virtual-dtor" }
+
+class base
+{
+protected:
+ ~base () {}
+ virtual void foo (){};
+};
+class derive final : public base
+{
+};
Re: [PATCH] RTEMS: Update contrib/config-list.mk
Is there anyone else from GCC who needs to approve this? As RTEMS maintainer for GCC, I am ok with it. --joel On 9/17/2014 8:37 AM, Sebastian Huber wrote: > contrib/ChangeLog > 2014-09-17 Sebastian Huber > > * config-list.mk (LIST): Add arm-rtems. > Add nios2-rtems. Remove extra option from powerpc-rtems. > --- > contrib/config-list.mk | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/contrib/config-list.mk b/contrib/config-list.mk > index 4345487..056fbf0 100644 > --- a/contrib/config-list.mk > +++ b/contrib/config-list.mk > @@ -17,7 +17,7 @@ LIST = aarch64-elf aarch64-linux-gnu \ >arc-elf32OPT-with-cpu=arc600 arc-elf32OPT-with-cpu=arc700 \ >arc-linux-uclibcOPT-with-cpu=arc700 arceb-linux-uclibcOPT-with-cpu=arc700 \ >arm-wrs-vxworks arm-netbsdelf \ > - arm-linux-androideabi arm-uclinux_eabi arm-eabi \ > + arm-linux-androideabi arm-uclinux_eabi arm-eabi arm-rtems \ >arm-symbianelf avr-rtems avr-elf \ >bfin-elf bfin-uclinux bfin-linux-uclibc bfin-rtems bfin-openbsd \ >c6x-elf c6x-uclinux cr16-elf cris-elf cris-linux crisv32-elf crisv32-linux > \ > @@ -48,13 +48,13 @@ LIST = aarch64-elf aarch64-linux-gnu \ >moxie-uclinux moxie-rtems \ >msp430-elf \ >nds32le-elf nds32be-elf \ > - nios2-elf nios2-linux-gnu \ > + nios2-elf nios2-linux-gnu nios2-rtems \ >pdp11-aout picochip-elfOPT-enable-obsolete \ >powerpc-darwin8 \ >powerpc-darwin7 powerpc64-darwin powerpc-freebsd6 powerpc-netbsd \ >powerpc-eabispe powerpc-eabisimaltivec powerpc-eabisim ppc-elf \ >powerpc-eabialtivec powerpc-xilinx-eabi powerpc-eabi \ > - powerpc-rtems4.11OPT-enable-threads=yes powerpc-linux_spe \ > + powerpc-rtems powerpc-linux_spe \ >powerpc-linux_paired powerpc64-linux_altivec \ >powerpc-wrs-vxworks powerpc-wrs-vxworksae powerpc-lynxos powerpcle-elf \ >powerpcle-eabisim powerpcle-eabi rs6000-ibm-aix4.3 rs6000-ibm-aix5.1.0 \ -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherr...@oarcorp.comOn-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available(256) 722-9985
Re: Fix pr61848, linux kernel miscompile
On Tue, Sep 16, 2014 at 01:03:50PM +, Joseph S. Myers wrote: > The point of testsuite additions is to verify the visible changes in Understood. A long time ago I worked on mil-spec systems.. > > Come to think of it, what if I decline to make any testsuite > > additions? > > Then the patch isn't ready for review. Documentation and testcases are > the first thing I look at when reviewing C front-end changes; the > testcases are the primary evidence that the patch does what it's meant to > do, and without them I won't generally try to review the code changes. I gave you two testcases. I happen to know how nice it is to have testcases when reviewing patches. Yes, they weren't testsuite patches. Now I'd normally just grumble a bit to myself and comply with a request for a testsuite patch, but I'm a little annoyed. The testsuite isn't my personal itch. I could have just bisected until finding Jan's patch, and asked him nicely to please fix his breakage. This is what most people do, and pointing the finger at a particular patch is useful. I went further and analysed exactly why the patch was at fault. I could have stopped there. The breakage wasn't really affecting me, and it wasn't my bug after all. Instead I thought I'd have a go at fixing the problem, without bothering Jan: The loss of section attribute looked quite easy to fix. So I threw together a patch, verified it worked, then knowing that there is more than one merge_decls in the gcc source code, fixed the same problem for C++. At that point I found Andrew's bugzilla. Andrew is competent, I could have just attached the patch I had to his bugzilla and left him to it. However it occurred to me that Jan's changes might have broken something else, so I poked around and found some other fields that had been moved to symtab_node. No big deal, I fixed that breakage as well. (I'm not sure I have them all actually. symtab_node fields and varpool_node fields are not clearly demarcated into those private to cgraph and those that matter externally.) I could have stopped there too. The final piece was wondering why Jan had added an extra test on DECL_SECTION, seeing that gcc has not reported an error on a changed section attribute for a very long time, and noticing that tls model wasn't generally merged. This is obviously where I went wrong. Silly me. I should have just pointed out these problems and asked you as C front end maintainer to please think about fixing them. That way you would have written the patch, and presumably the testsuite, and everyone would be happy! In fact, maybe I should have just avoided all this effort and just asked Jan to please fix his bug. BTW, in my patch submission at https://gcc.gnu.org/ml/gcc-patches/2014-09/msg01146.html I said "trunk does not honour the last section attribute". What I really should have said was "trunk, with a straight-forward fix for complete loss of section attribute, does not honour the last section attribute". -- Alan Modra Australia Development Lab, IBM
Re: [PATCH] RTEMS thread model configuration
Thanks for the ping.
I updated the date on the ChangeLog and committed this.
--joel
On 9/17/2014 8:26 AM, Sebastian Huber wrote:
Ping^2.
On 02/05/14 10:46, Sebastian Huber wrote:
Ping.
On 2014-04-18 12:11, Sebastian Huber wrote:
From: Sebastian Huber
The command line to build a GCC for RTEMS contained virtually always a
'--enable-threads'. This patch helps to avoid this extra configuration
command line parameter and makes the GCC build a bit more user friendly
for RTEMS.
This patch should be applied to GCC 4.9 branch and master.
2014-04-18 Sebastian Huber
* config.gcc (*-*-rtems*): Default to 'rtems' thread model.
Enable selection of 'posix' or no thread model.
---
gcc/config.gcc | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 3c55c88..93d5994 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -791,7 +791,13 @@ case ${target} in
;;
*-*-rtems*)
case ${enable_threads} in
-yes) thread_file='rtems' ;;
+"" | yes | rtems) thread_file='rtems' ;;
+posix) thread_file='posix' ;;
+no) ;;
+*)
+ echo 'Unknown thread configuration for RTEMS'
+ exit 1
+ ;;
esac
tmake_file="${tmake_file} t-rtems"
extra_options="${extra_options} rtems.opt"
--
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherr...@oarcorp.comOn-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available(256) 722-9985
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On Wed, Sep 17, 2014 at 04:39:00PM +0200, Andi Kleen wrote:
> Ok, this should fix it:
>
> I'll commit it as obvious after testing unless there are objections.
This isn't sufficient.
If -mfentry isn't compatible with -m32 -fpic, supposedly you need
something like (untested):
/* { dg-do compile { target { { *-*-linux* } && { nonpic || ! { ia32 } } } } }
*/
or similar (dunno about -mx32 -fpic, whether it is supposed to work or not).
Otherwise, if somebody tests (and some people do) with
make check-gcc
RUNTESTFLAGS='--target_board=unix\{-m32,-m32/-fpic,-m64,-m64/-fpic\}'
or similar, there could be failures.
Just test your patch with
make check-gcc
RUNTESTFLAGS='--target_board=unix\{-m32,-m32/-fpic,-m64,-m64/-fpic\}
i386.exp=fentry*.c'
in addition to normal testing...
> --- a/gcc/testsuite/gcc.target/i386/fentry-override.c
> +++ b/gcc/testsuite/gcc.target/i386/fentry-override.c
> @@ -1,5 +1,5 @@
> /* Test -mfentry override */
> -/* { dg-do compile } */
> +/* { dg-do compile { target { *-*-linux* } } } */
> /* { dg-options "-mfentry" } */
> /* { dg-final { scan-assembler-not "__fentry__" } } */
> /* Origin: Andi Kleen */
> diff --git a/gcc/testsuite/gcc.target/i386/fentry.c
> b/gcc/testsuite/gcc.target/i386/fentry.c
> index bd3db13..888 100644
> --- a/gcc/testsuite/gcc.target/i386/fentry.c
> +++ b/gcc/testsuite/gcc.target/i386/fentry.c
> @@ -1,5 +1,5 @@
> /* Test -mfentry */
> -/* { dg-do compile } */
> +/* { dg-do compile { target { *-*-linux* } } } */
> /* { dg-options "-fprofile -mfentry" } */
> /* { dg-final { scan-assembler "__fentry__" } } */
> /* Origin: Andi Kleen */
>
>
> --
> a...@linux.intel.com -- Speaking for myself only.
Jakub
Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern)
> On Sep 17, 2014, at 7:43 AM, James Greenhalgh > wrote: > > >> On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote: >> "=&r" is correct for an early-clobbered scratch. >> >> R. > > In that case... > > How is the attached patch for trunk? I've bootstrapped it on AArch64 > with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS > without seeing any issues. > > OK? > > Thanks, > James > > --- > gcc/ > > 2014-09-15 James Greenhalgh > > * config/aarch64/aarch64.md (stack_protect_test_): Mark > scratch register as an output to placate register renaming. > > gcc/testsuite/ > > 2014-09-15 James Greenhalgh > > * gcc.target/aarch64/stack_protector_set_1.c: New. > * gcc.target/aarch64/stack_protector_set_2.c: Likewise. There is nothing aarch64 specific about this testcase so I would place them under gcc.dg and add the extra marker which says this testcase requires stack protector. And maybe even use compile instead of just assemble too. Thanks, Andrew > <0001-Re-PATCH-AArch64-Add-constraint-letter-for-stack_pro.patch>
Re: [PATCH 1/2, x86] Add palignr support for AVX2.
On Wed, Sep 17, 2014 at 6:01 AM, Evgeny Stupachenko wrote: > It fix "gcc.target/i386/pr52252-atom.c" in core-avx2 make check and pr62128. > I suggest you resubmit the patch as a bug fix for pr62128 with testcases from pr62128 as well as gcc.target/i386/pr52252-atom.c. -- H.J.
Re: [PATCH] RTEMS: Update contrib/config-list.mk
On Wed, 2014-09-17 15:37:32 +0200, Sebastian Huber wrote: > contrib/ChangeLog > 2014-09-17 Sebastian Huber > > * config-list.mk (LIST): Add arm-rtems. > Add nios2-rtems. Remove extra option from powerpc-rtems. What's the rationale for removing --enable-threads=yes here, as well as the specific version number? Don't get me wrong: I don't question this change, though it's not obvious to me; I'd probably just add powerpc-rtems additionally? MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: "Debugging is twice as hard as writing the code in the first place. the second : Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan signature.asc Description: Digital signature
Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern)
On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote:
> "=&r" is correct for an early-clobbered scratch.
>
> R.
In that case...
How is the attached patch for trunk? I've bootstrapped it on AArch64
with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS
without seeing any issues.
OK?
Thanks,
James
---
gcc/
2014-09-15 James Greenhalgh
* config/aarch64/aarch64.md (stack_protect_test_): Mark
scratch register as an output to placate register renaming.
gcc/testsuite/
2014-09-15 James Greenhalgh
* gcc.target/aarch64/stack_protector_set_1.c: New.
* gcc.target/aarch64/stack_protector_set_2.c: Likewise.
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c60038a9015d614f40f6d9e3fd228ad3e2b247a8..f15a516bb0559c86bea7512f91d60dc179ec9149 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4031,7 +4031,7 @@ (define_insn "stack_protect_test_"
(unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")
(match_operand:PTR 2 "memory_operand" "m")]
UNSPEC_SP_TEST))
- (clobber (match_scratch:PTR 3 "&r"))]
+ (clobber (match_scratch:PTR 3 "=&r"))]
""
"ldr\t%3, %x1\;ldr\t%0, %x2\;eor\t%0, %3, %0"
[(set_attr "length" "12")
diff --git a/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c
new file mode 100644
index 000..df0d26b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c
@@ -0,0 +1,15 @@
+/* { dg-do assemble } */
+/* { dg-options "-fstack-protector-strong -O1 -frename-registers" } */
+
+extern int bar (const char *s, int *argc);
+extern int baz (const char *s);
+
+char
+foo (const char *s)
+{
+ int argc;
+ int ret;
+ if ( !bar (s, &argc))
+ret = baz (s);
+ return *s;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c
new file mode 100644
index 000..b94a2d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c
@@ -0,0 +1,17 @@
+/* { dg-do assemble } */
+/* { dg-options "-fstack-protector-strong -O1 -frename-registers" } */
+
+typedef unsigned int uint32_t;
+struct ctx
+{
+ uint32_t A;
+};
+
+void *
+buffer_copy (const struct ctx *ctx, void *resbuf)
+{
+ uint32_t buffer[4];
+ buffer[0] = (ctx->A);
+ __builtin_memcpy (resbuf, buffer, 16);
+ return resbuf;
+}
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On Wed, Sep 17, 2014 at 04:32:21PM +0200, Andi Kleen wrote:
> On Wed, Sep 17, 2014 at 03:42:37PM +0200, Dominique Dhumieres wrote:
> >
> > On darwin I get
> >
> > FAIL: gcc.target/i386/fentry-override.c (test for excess errors)
> > UNRESOLVED: gcc.target/i386/fentry-override.c scan-assembler-not __fentry__
> > FAIL: gcc.target/i386/fentry.c (test for excess errors)
> > UNRESOLVED: gcc.target/i386/fentry.c scan-assembler __fentry__
> >
> > with -m32. The error is
> >
> > sorry, unimplemented: -mfentry isn't supported for 32-bit in combination
> > with -fpic
> >
> > With -m64 I get
> >
> > FAIL: gcc.target/i386/fentry.c scan-assembler __fentry__
> >
> > with no string 'entry' in the fentry.s file.
>
> __fentry__ doesn't exist on all x86 targets.
> I'm not sure how to mark a test to be Linux only. I'll just remove it.
Ok, this should fix it:
I'll commit it as obvious after testing unless there are objections.
diff --git a/gcc/testsuite/gcc.target/i386/fentry-override.c
b/gcc/testsuite/gcc.target/i386/fentry-override.c
index 3771f19..1b158b3 100644
--- a/gcc/testsuite/gcc.target/i386/fentry-override.c
+++ b/gcc/testsuite/gcc.target/i386/fentry-override.c
@@ -1,5 +1,5 @@
/* Test -mfentry override */
-/* { dg-do compile } */
+/* { dg-do compile { target { *-*-linux* } } } */
/* { dg-options "-mfentry" } */
/* { dg-final { scan-assembler-not "__fentry__" } } */
/* Origin: Andi Kleen */
diff --git a/gcc/testsuite/gcc.target/i386/fentry.c
b/gcc/testsuite/gcc.target/i386/fentry.c
index bd3db13..888 100644
--- a/gcc/testsuite/gcc.target/i386/fentry.c
+++ b/gcc/testsuite/gcc.target/i386/fentry.c
@@ -1,5 +1,5 @@
/* Test -mfentry */
-/* { dg-do compile } */
+/* { dg-do compile { target { *-*-linux* } } } */
/* { dg-options "-fprofile -mfentry" } */
/* { dg-final { scan-assembler "__fentry__" } } */
/* Origin: Andi Kleen */
--
a...@linux.intel.com -- Speaking for myself only.
[PING] Enhance array types debug info. for Ada
Ping for https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00206.html Adding a few maintainers in copy... Thanks in advance! -- Pierre-Marie de Rodat
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On Wed, Sep 17, 2014 at 03:42:37PM +0200, Dominique Dhumieres wrote: > > On darwin I get > > FAIL: gcc.target/i386/fentry-override.c (test for excess errors) > UNRESOLVED: gcc.target/i386/fentry-override.c scan-assembler-not __fentry__ > FAIL: gcc.target/i386/fentry.c (test for excess errors) > UNRESOLVED: gcc.target/i386/fentry.c scan-assembler __fentry__ > > with -m32. The error is > > sorry, unimplemented: -mfentry isn't supported for 32-bit in combination with > -fpic > > With -m64 I get > > FAIL: gcc.target/i386/fentry.c scan-assembler __fentry__ > > with no string 'entry' in the fentry.s file. __fentry__ doesn't exist on all x86 targets. I'm not sure how to mark a test to be Linux only. I'll just remove it. -Andi
Re: [RFC PATCH] -fsanitize=vptr instrumentation
On 09/16/2014 10:56 AM, Jakub Jelinek wrote:
vptr-5.C is one Jason mailed me yesterday, clang++ doesn't instrument this
and g++ right now doesn't either, build_static_cast_1 certainly isn't called
in that case, and I must say I have no idea what should be checked there,
where etc.
What needs to be checked is conversion (in this case implicit) to a
virtual base; if the vptr doesn't point to a vtable that has the
appropriate vbase offset, we should complain.
virtual base conversions are implemented in build_base_path under if
(virtual_access).
vptr-6.C shows where the this optimization is performed and where it isn't
(clang++ has 10 instrumentations in T::h and 1 in S::l, g++ has fewer than
that, but not 0 in T::h (1 in S::l is right and needed I think)).
I agree that 0 is enough for T::h and 1 for S::l.
I hope all of f[1-6] is invalid, I really don't see how we could instrument
member accesses otherwise (we'd need to limit to not taking address of it);
NULL pointer shouldn't point at a valid object.
I don't see anything in the standard saying that these are undefined,
only that trying to access the (non-)object pointed to is undefined. It
would be undefined if a conversion to virtual base were involved, i.e.
struct V: virtual R { };
// undefined if p doesn't point to a V because of the conversion to
// virtual base R
int* f7 (V* p) { return &p->r; }
These conditions were loosened in C++11 by DRs 597 and 1531; before that
it was reasonable to regard f[1-6] as undefined, and perhaps clang is
using the earlier interpretation.
+ TREE_SIDE_EFFECTS (cond) = 1;
...
+ TREE_SIDE_EFFECTS (hash) = 1;
Why do you need to set TREE_SIDE_EFFECTS on these?
+ if (current_function_decl == NULL_TREE
+ || lookup_attribute ("no_sanitize_undefined",
+ DECL_ATTRIBUTES (current_function_decl)))
+return NULL_TREE;
When would this be called outside a function? If for namespace-scope
variable initializers, I'd think we do want instrumentation.
+ /* T t; t.foo (); doesn't need instrumentation, if the type is known. */
+ if (is_addr
+ && TREE_CODE (op) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (op, 0))
+ && same_type_p (type,
+ TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op, 0)
+return NULL_TREE;
You might want to use resolves_to_fixed_type_p in the optimizations.
Jason
Re: [kyukhin/gomp4-offload] DESTDIR issues
On Wed, Sep 17, 2014 at 04:04:25PM +0200, Paolo Bonzini wrote: > Il 17/09/2014 15:31, Jakub Jelinek ha scritto: > > It seems building of the host compiler requires the offloading compiler > > to be installed directly in the prefix, which is something really > > undesirable e.g. for distro builds where things are installed with > > non-empty $(DESTDIR). > > Is the offloading compiler built together with GCC or previously? If > the latter, what's the difference between the offloading compiler and > say gmp? Setting the LD_LIBRARY_PATH would be the responsibility of > whoever builds GCC; it would be the same here for the COMPILER_PATH. Right now previously, but (usually) from the very same sources, so I think it will be usually (but not always?) desirable to build them together, perhaps not in a single toplevel configure and single toplevel make. Jakub
Re: [PATCH 5/5] add libcc1 [gcc-5/changes.html]
On Sun, 14 Sep 2014 17:07:05 +0200, Manuel López-Ibáñez wrote: > What happened with this? I don't see any libcc1 in the gcc repository > and this patch was never committed. It was discussed internally and the patches are going to be updated, rebased and later checked in. Thanks, Jan
Re: [gomp4] various OpenACC/PTX built-ins and a reduction tweak
On 09/17/2014 01:49 AM, Jakub Jelinek wrote:
> On Wed, Sep 17, 2014 at 10:44:12AM +0200, Tobias Burnus wrote:
>> Cesar Philippidis wrote:
>>> The patch introduces the following OpenACC/PTX-specific built-ins:
>> ...
>>
>> It is not completely clear how they are supposed to get used. Should the
>> user call them directly in some cases? Or are they only used internally?
>>
>> acc_on_device sounds like a function which would be in C/C++ made available
>> to the user via #define acc_on_device __builtin_acc_on_device.
>
> And not just providing acc_on_device prototype in some header? Without
> looking at the OpenACC standard, it sounds like this function could be
> similar to omp_is_initial_device, so can and should be handled supposedly
> similarly.
All of the functions are internal except for acc_on_device. We do have a
proper interface for it, but it's part of the libgomp runtime. Should I
remove acc_on_device from this patch until the runtime is ready?
>> However, the rest looks as if it should rather be an internal function
>> instead of a builtin. Or should the user really ever call the builtin
>> directly?
>
> GOMP_* functions are builtins and not internal functions too, all those
> functions are library functions, while the user typically doesn't call them
> directly, they still are implemented in the library. Internal functions are
> used for something that doesn't have a library implementation and is not
> something user can call directly.
All of the GOACC_* functions introduced in this patch are internal
functions without library functions, although that may change for
GOACC_get_thread_num and GOACC_get_num_threads. Is there a better way to
handle internal functions or is it correct as-is? I'm not that familiar
with built-in functions.
>> Regarding Fortran: Builtins aren't directly available to the user. You have
>> to
>> wrap them into an intrinsic to make them available. If they have to be made
>> available via a module (e.g. via "module acc) - you have to create a virtual
>> module, which provides the intrinsic. If you don't want to convert the whole
>> module, you could create an auxiliar module (e.g. acc_internal_) which
>> provides
>> only those bits - and then include it ("use,intrinsic :: ...") it in the
>> main module - written in normal Fortran.
>
> For the user callable fortran functions, for OpenMP libgomp just provides
> *_ entrypoints to * functions. Perhaps acc_on_device_ could be provided
> too.
Cesar
Re: [PATCHv3] Vimrc config with GNU formatting
Sure. If you have the localrc thing installed, anyone who can write files you can read can make your vim do *anything* (and I mean *anything*). I thought that modern versions of localrc run .local.vimrc scripts in a sandbox? Ok, looks like Markus Braun's http://www.vim.org/scripts/script.php?script_id=441 does this. Thinca's plugin currently indeed allows arbitrary code execution. -Y
Re: [patch] gcc fstack-protector-explicit
On Wed, Jul 2, 2014 at 2:58 PM, Marcos Díaz wrote: > On Tue, Jul 1, 2014 at 6:34 PM, Daniel Gutson > wrote: >> On Tue, Jul 1, 2014 at 2:25 PM, Jeff Law wrote: >>> On 03/19/14 08:06, Marcos Díaz wrote: Well, finally I have the assignment, could you please review this patch? >>> >>> Thanks. >>> >>> My first thought was that if we've marked the function with an explicit >>> static protector attribute, then it ought to be protected regardless of any >>> flags. Is there some reason to require the -fstack-protect-explicit? >> >> They can work separately, since the logic is: >> >> if NOT stack-protect-explicit >>a function can be protected by the current logic OR it has the attribute >>(a function may be not automatically protected with the current logic) >> ELSE // stack-protect-explicit >>only functions marked with the attribute will be protected. >> > If there isn't any stack-protect flag (strong, common or explicit) the > attribute has no effect >> IOW, when no stack-protect-explicit, the functions may not be >> protected due to current logic, so the attribute acts as an override >> to request protection. >> >>> >>> The patch itself is relatively simple and I don't see anything that looks >>> terribly wrong at first glance. I think we just need to make sure we're on >>> the same page WRT needing the -fstack-protect-explicit flag. >>> >>> jeff >>> >>> >> >> >> >> -- >> >> Daniel F. Gutson >> Chief Engineering Officer, SPD >> >> >> San Lorenzo 47, 3rd Floor, Office 5 >> >> Córdoba, Argentina >> >> >> Phone: +54 351 4217888 / +54 351 4218211 >> >> Skype: dgutson > > > > -- > __ > > > Marcos Díaz > > Software Engineer > > > San Lorenzo 47, 3rd Floor, Office 5 > > Córdoba, Argentina > > > Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452 > > Skype: markdiaz22 ping -- __ Marcos Díaz Software Engineer San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452 Skype: markdiaz22
Re: [PATCHv3] Vimrc config with GNU formatting
On 09/17/2014 05:01 PM, Segher Boessenkool wrote: You can make Vim automatically adapt settings, but you cannot make the Vim user adapt to that. How about making Vim user adapt to GNU coding style though? Not that I want to start flame again. Sure. If you have the localrc thing installed, anyone who can write files you can read can make your vim do *anything* (and I mean *anything*). I thought that modern versions of localrc run .local.vimrc scripts in a sandbox? -Y
Re: [kyukhin/gomp4-offload] DESTDIR issues
Il 17/09/2014 15:31, Jakub Jelinek ha scritto: > It seems building of the host compiler requires the offloading compiler > to be installed directly in the prefix, which is something really > undesirable e.g. for distro builds where things are installed with > non-empty $(DESTDIR). Is the offloading compiler built together with GCC or previously? If the latter, what's the difference between the offloading compiler and say gmp? Setting the LD_LIBRARY_PATH would be the responsibility of whoever builds GCC; it would be the same here for the COMPILER_PATH. > Either we can do something like the following patch, i.e. look > at -I$(DESTDIR)/$(target_dir)/include first and fall back to > -I$(target_dir)/include, which fixed the build for me, but unfortunately > it violates GNU conventions: > https://www.gnu.org/prep/standards/html_node/DESTDIR.html > "DESTDIR should be supported only in the install* and uninstall* targets, as > those are the only targets where it is useful." > while this patch uses it during build. I agree with the coding standards. The right way to do it could be a configure option, if it's really not possible to leave it to whoever builds GCC. Paolo
Re: [PATCH 1/n] OpenMP 4.0 offloading infrastructure
On Mon, Sep 15, 2014 at 08:52:27PM +0400, Ilya Verbin wrote:
> This patch contains necessary changes for the build system to support
> offloading.
> It adds 2 new options for configure:
> * --enable-as-accelerator-for=ARG is intended for the offload target compiler.
> * --enable-offload-targets=LIST is intended for the offload host compiler.
> Some more info:
> https://gcc.gnu.org/wiki/Offloading#Building_host_and_accel_compilers
>
> Bootstrapped and regtested on i686-linux and x86_64-linux. Is it OK for
> trunk?
Looks mostly ok, just some nits. But see the patch I've just posted,
perhaps we want to tweak the --enable-offload-targets arguments. And
I'd strongly prefer if the offloading patches are committed after everything
has been reviewed, so what will be acked please stash away for later batch
commits.
> --- /dev/null
> +++ b/libgcc/ompstuff.c
> @@ -0,0 +1,79 @@
> +/* FIXME: Including auto-host is incorrect, but until we have
> + identified the set of defines that need to go into auto-target.h,
> + this will have to do. */
> +#include "auto-host.h"
> +#undef pid_t
> +#undef rlim_t
> +#undef ssize_t
> +#undef vfork
crtstuff.c undefs here also caddr_t, any reason not to do that too?
> +#include "tconfig.h"
> +#include "tsystem.h"
> +#include "coretypes.h"
> +#include "tm.h"
> +#include "libgcc_tm.h"
> +
> +#define OFFLOAD_FUNC_TABLE_SECTION_NAME "__gnu_offload_funcs"
> +#define OFFLOAD_VAR_TABLE_SECTION_NAME "__gnu_offload_vars"
> +
> +#ifdef CRT_BEGIN
> +
> +#if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> +void *_omp_func_table[0]
> + __attribute__ ((__used__, visibility ("hidden"),
> + section (OFFLOAD_FUNC_TABLE_SECTION_NAME))) = { };
> +void *_omp_var_table[0]
> + __attribute__ ((__used__, visibility ("hidden"),
> + section (OFFLOAD_VAR_TABLE_SECTION_NAME))) = { };
> +#endif
Does this mean that if HAVE_GAS_HIDDEN is not defined, you don't
define _omp_*_table at all and offloading will fail?
I wonder if it just should avoid visibility ("hidden") if it isn't
supported.
Jakub
Re: [PATCH 2/2] Add some more test cases for fentry and pg
On darwin I get FAIL: gcc.target/i386/fentry-override.c (test for excess errors) UNRESOLVED: gcc.target/i386/fentry-override.c scan-assembler-not __fentry__ FAIL: gcc.target/i386/fentry.c (test for excess errors) UNRESOLVED: gcc.target/i386/fentry.c scan-assembler __fentry__ with -m32. The error is sorry, unimplemented: -mfentry isn't supported for 32-bit in combination with -fpic With -m64 I get FAIL: gcc.target/i386/fentry.c scan-assembler __fentry__ with no string 'entry' in the fentry.s file. TIA Dominique
[PATCH] RTEMS: Update contrib/config-list.mk
contrib/ChangeLog 2014-09-17 Sebastian Huber * config-list.mk (LIST): Add arm-rtems. Add nios2-rtems. Remove extra option from powerpc-rtems. --- contrib/config-list.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/config-list.mk b/contrib/config-list.mk index 4345487..056fbf0 100644 --- a/contrib/config-list.mk +++ b/contrib/config-list.mk @@ -17,7 +17,7 @@ LIST = aarch64-elf aarch64-linux-gnu \ arc-elf32OPT-with-cpu=arc600 arc-elf32OPT-with-cpu=arc700 \ arc-linux-uclibcOPT-with-cpu=arc700 arceb-linux-uclibcOPT-with-cpu=arc700 \ arm-wrs-vxworks arm-netbsdelf \ - arm-linux-androideabi arm-uclinux_eabi arm-eabi \ + arm-linux-androideabi arm-uclinux_eabi arm-eabi arm-rtems \ arm-symbianelf avr-rtems avr-elf \ bfin-elf bfin-uclinux bfin-linux-uclibc bfin-rtems bfin-openbsd \ c6x-elf c6x-uclinux cr16-elf cris-elf cris-linux crisv32-elf crisv32-linux \ @@ -48,13 +48,13 @@ LIST = aarch64-elf aarch64-linux-gnu \ moxie-uclinux moxie-rtems \ msp430-elf \ nds32le-elf nds32be-elf \ - nios2-elf nios2-linux-gnu \ + nios2-elf nios2-linux-gnu nios2-rtems \ pdp11-aout picochip-elfOPT-enable-obsolete \ powerpc-darwin8 \ powerpc-darwin7 powerpc64-darwin powerpc-freebsd6 powerpc-netbsd \ powerpc-eabispe powerpc-eabisimaltivec powerpc-eabisim ppc-elf \ powerpc-eabialtivec powerpc-xilinx-eabi powerpc-eabi \ - powerpc-rtems4.11OPT-enable-threads=yes powerpc-linux_spe \ + powerpc-rtems powerpc-linux_spe \ powerpc-linux_paired powerpc64-linux_altivec \ powerpc-wrs-vxworks powerpc-wrs-vxworksae powerpc-lynxos powerpcle-elf \ powerpcle-eabisim powerpcle-eabi rs6000-ibm-aix4.3 rs6000-ibm-aix5.1.0 \ -- 1.8.4.5
[kyukhin/gomp4-offload] DESTDIR issues
Hi! It seems building of the host compiler requires the offloading compiler to be installed directly in the prefix, which is something really undesirable e.g. for distro builds where things are installed with non-empty $(DESTDIR). Right now it seems that for building all that is needed is main_target_image.h. Either we can do something like the following patch, i.e. look at -I$(DESTDIR)/$(target_dir)/include first and fall back to -I$(target_dir)/include, which fixed the build for me, but unfortunately it violates GNU conventions: https://www.gnu.org/prep/standards/html_node/DESTDIR.html "DESTDIR should be supported only in the install* and uninstall* targets, as those are the only targets where it is useful." while this patch uses it during build. Another option would be to have two different modes of building --enable-offload-targets= compilers, one where you really have all the offloading target compilers installed (with empty DESTDIR, or initially with non-empty DESTDIR, then somehow (package manager, etc.) copied to the right prefix), and another one where for each offloading target you'd additionally specify the build directory containing non-installed tree (dunno, could be e.g. --enable-offload-targets=x86_64-intelmic-linux-gnu=/some/path,ptx-none=/some/other/path format?) and let the host compiler's build look into the other build trees for what it needs during building and/or testing (for building main_target_image.h should be supposedly all that is needed, for testing perhaps more). Thoughts on this? 2014-09-17 Jakub Jelinek * plugin/Makefile.am (libgomp_plugin_mic_la_CPPFLAGS): Use $(DESTDIR)$(target_dir)/include path before trying $(target_dir)/include. * plugin/Makefile.in: Regenerated. --- liboffloadmic/plugin/Makefile.am2014-09-16 18:16:17.961415565 +0200 +++ liboffloadmic/plugin/Makefile.am2014-09-17 14:40:26.990566860 +0200 @@ -47,7 +47,7 @@ target_dir = $(libdir)/gcc/$(accel_targe if PLUGIN_HOST toolexeclib_LTLIBRARIES = libgomp-plugin-mic.la libgomp_plugin_mic_la_SOURCES = libgomp-plugin-mic.cpp - libgomp_plugin_mic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -DMIC_LIB_PATH=\"$(target_dir)\" -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_dir) -I$(target_dir)/include + libgomp_plugin_mic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -DMIC_LIB_PATH=\"$(target_dir)\" -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_dir) -I$(DESTDIR)$(target_dir)/include -I$(target_dir)/include libgomp_plugin_mic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic -version-info 1:0:0 else # PLUGIN_TARGET plugin_includedir = $(libsubincludedir) --- liboffloadmic/plugin/Makefile.in2014-09-16 18:16:17.961415565 +0200 +++ liboffloadmic/plugin/Makefile.in2014-09-17 14:54:50.555096069 +0200 @@ -277,7 +277,7 @@ libsubincludedir = $(libdir)/gcc/$(targe target_dir = $(libdir)/gcc/$(accel_target)/$(gcc_version) @PLUGIN_HOST_TRUE@toolexeclib_LTLIBRARIES = libgomp-plugin-mic.la @PLUGIN_HOST_TRUE@libgomp_plugin_mic_la_SOURCES = libgomp-plugin-mic.cpp -@PLUGIN_HOST_TRUE@libgomp_plugin_mic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -DMIC_LIB_PATH=\"$(target_dir)\" -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_dir) -I$(target_dir)/include +@PLUGIN_HOST_TRUE@libgomp_plugin_mic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -DMIC_LIB_PATH=\"$(target_dir)\" -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_dir) -I$(DESTDIR)$(target_dir)/include -I$(target_dir)/include @PLUGIN_HOST_TRUE@libgomp_plugin_mic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic -version-info 1:0:0 @PLUGIN_HOST_FALSE@plugin_includedir = $(libsubincludedir) @PLUGIN_HOST_FALSE@plugin_include_HEADERS = main_target_image.h Jakub
Re: [PATCH] RTEMS thread model configuration
Ping^2.
On 02/05/14 10:46, Sebastian Huber wrote:
Ping.
On 2014-04-18 12:11, Sebastian Huber wrote:
From: Sebastian Huber
The command line to build a GCC for RTEMS contained virtually always a
'--enable-threads'. This patch helps to avoid this extra configuration
command line parameter and makes the GCC build a bit more user friendly
for RTEMS.
This patch should be applied to GCC 4.9 branch and master.
2014-04-18 Sebastian Huber
* config.gcc (*-*-rtems*): Default to 'rtems' thread model.
Enable selection of 'posix' or no thread model.
---
gcc/config.gcc | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 3c55c88..93d5994 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -791,7 +791,13 @@ case ${target} in
;;
*-*-rtems*)
case ${enable_threads} in
-yes) thread_file='rtems' ;;
+"" | yes | rtems) thread_file='rtems' ;;
+posix) thread_file='posix' ;;
+no) ;;
+*)
+ echo 'Unknown thread configuration for RTEMS'
+ exit 1
+ ;;
esac
tmake_file="${tmake_file} t-rtems"
extra_options="${extra_options} rtems.opt"
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [PATCH] Add header guard to several header files.
Updated patch On Wed, Sep 17, 2014 at 9:17 PM, Kito Cheng wrote: > Updated patch > > On Mon, Sep 8, 2014 at 9:46 PM, Kito Cheng wrote: >> >> ping! >> >> On Tue, Sep 2, 2014 at 12:37 AM, Kito Cheng wrote: >> > Hi Joseph: >> > >> > Thanks for your review, I've reverted the part of gsyslimits.h, >> > here is updated patch and ChangeLog :) >> > >> > bootstrap ok for x86_64 >> > >> > 2014-09-01 Kito Cheng >> > >> > except.h: Fix header guard. >> > addresses.h: Add missing header guard. >> > cfghooks.h: Likewise. >> > collect-utils.h: Likewise. >> > collect2-aix.h: Likewise. >> > conditions.h: Likewise. >> > cselib.h: Likewise. >> > dwarf2asm.h: Likewise. >> > graphds.h: Likewise. >> > graphite-scop-detection.h: Likewise. >> > gsyms.h: Likewise. >> > hw-doloop.h: Likewise. >> > incpath.h: Likewise. >> > ipa-inline.h: Likewise. >> > ipa-ref.h: Likewise. >> > ira-int.h: Likewise. >> > ira.h: Likewise. >> > lra-int.h: Likewise. >> > lra.h: Likewise. >> > lto-section-names.h: Likewise. >> > read-md.h: Likewise. >> > reload.h: Likewise. >> > rtl-error.h: Likewise. >> > sdbout.h: Likewise. >> > target-def.h: Likewise. >> > target-hooks-macros.h: Likewise. >> > targhooks.h: Likewise. >> > tree-affine.h: Likewise. >> > xcoff.h: Likewise. >> > xcoffout.h: Likewise. >> > >> > On Mon, Sep 1, 2014 at 11:56 PM, Joseph S. Myers >> > wrote: >> >> On Mon, 1 Sep 2014, Kito Cheng wrote: >> >> >> >>> gsyslimits.h: Likewise. >> >> >> >> This is incorrect. This is a very special header file, installed as >> >> part >> >> of the implementation of ; it certainly can't use any >> >> user-namespace identifiers, and it's probably not safe for it to have >> >> header guards at all. >> >> >> >> -- >> >> Joseph S. Myers >> >> jos...@codesourcery.com > > From a4c4d11680a223c9b9b5ab565234f31a2c8e5cb3 Mon Sep 17 00:00:00 2001 From: Kito Cheng Date: Fri, 22 Aug 2014 17:34:49 +0800 Subject: [PATCH] Add header guard to several header files. 2014-09-01 Kito Cheng except.h: Fix header guard. addresses.h: Add missing header guard. cfghooks.h: Likewise. collect-utils.h: Likewise. collect2-aix.h: Likewise. conditions.h: Likewise. cselib.h: Likewise. dwarf2asm.h: Likewise. graphds.h: Likewise. graphite-scop-detection.h: Likewise. gsyms.h: Likewise. hw-doloop.h: Likewise. incpath.h: Likewise. ipa-inline.h: Likewise. ipa-ref.h: Likewise. ira-int.h: Likewise. ira.h: Likewise. lra-int.h: Likewise. lra.h: Likewise. lto-section-names.h: Likewise. read-md.h: Likewise. reload.h: Likewise. rtl-error.h: Likewise. sdbout.h: Likewise. target-def.h: Likewise. target-hooks-macros.h: Likewise. targhooks.h: Likewise. tree-affine.h: Likewise. xcoff.h: Likewise. xcoffout.h: Likewise. --- gcc/addresses.h | 5 + gcc/cfghooks.h| 4 gcc/collect-utils.h | 5 + gcc/collect2-aix.h| 4 gcc/conditions.h | 5 + gcc/cselib.h | 5 + gcc/dwarf2asm.h | 4 gcc/except.h | 5 +++-- gcc/graphds.h | 5 + gcc/graphite-scop-detection.h | 4 gcc/gsyms.h | 4 gcc/hw-doloop.h | 5 + gcc/incpath.h | 5 + gcc/ipa-inline.h | 5 + gcc/ipa-ref.h | 5 + gcc/ira-int.h | 5 + gcc/ira.h | 5 + gcc/lra-int.h | 5 + gcc/lra.h | 5 + gcc/lto-section-names.h | 5 + gcc/read-md.h | 5 + gcc/reload.h | 4 gcc/rtl-error.h | 5 + gcc/sdbout.h | 5 + gcc/target-def.h | 5 + gcc/target-hooks-macros.h | 5 + gcc/targhooks.h | 5 + gcc/tree-affine.h | 5 + gcc/xcoff.h | 5 + gcc/xcoffout.h| 4 30 files changed, 141 insertions(+), 2 deletions(-) diff --git a/gcc/addresses.h b/gcc/addresses.h index e323b58..3f0089a 100644 --- a/gcc/addresses.h +++ b/gcc/addresses.h @@ -21,6 +21,9 @@ along with GCC; see the file COPYING3. If not see MODE_BASE_REG_REG_CLASS, MODE_BASE_REG_CLASS and BASE_REG_CLASS. Arguments as for the MODE_CODE_BASE_REG_CLASS macro. */ +#ifndef GCC_ADDRESSES_H +#define GCC_ADDRESSES_H + static inline enum reg_class base_reg_class (enum machine_mode mode ATTRIBUTE_UNUSED, addr_space_t as ATTRIBUTE_UNUSED, @@ -82,3 +85,5 @@ regno_ok_for_base_p (unsigned regno, enum machine_mode mode, addr_space_t as, return ok_for_base_p_1 (regno, mode, as, outer_code, index_code); } + +#endif /* GCC_ADDRESSES_H */ diff --git a/gcc/cfghooks.h b/gcc/cfghooks.h index 8ff808c..1b8587a 100644 --- a/gcc/cfghooks.h +++ b/gcc/cfghooks.h @@ -18,6 +18,9 @@ You should have received a copy of the GNU Ge
Re: [PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
Am 17.09.2014 um 00:03 schrieb James Greenhalgh:
> If you have any other suggestions, or if "=&r" is actually correct and
> I am misreading the documentation please let me know.
with this patch I see a lot of ICEs in the testsuite for test cases built with
-O3 (and a build defaulting to -fstack-protector-strong by default), all of the
form:
Executing on host: /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc
-B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret -fdia
gnostics-color=never -O3 -fomit-frame-pointer -funroll-all-loops
-finline-functions -w -c -o 900116-1.o /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/g
cc/testsuite/gcc.c-torture/compile/900116-1.c(timeout = 300)
spawn /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc
-B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret
-fdiagnostics-color
=never -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions -w -c -o
900116-1.o /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-
torture/compile/900116-1.c
/home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:
In function 'zloop':
/home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1:
error: insn does not satisfy its constraints:
(insn 228 225 230 9 (parallel [
(set (reg:DI 1 x1 [279])
(unspec:DI [
(mem/v/f/c:DI (plus:DI (reg/f:DI 31 sp)
(const_int 24 [0x18])) [4 D.2626+0 S8 A64])
(mem/v/f/c:DI (reg/f:DI 2 x2 [277]) [4
__stack_chk_guard+0 S8 A64])
] UNSPEC_SP_TEST))
(clobber (reg:DI 2 x2 [320]))
])
/home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14
741 {stack_protect_test_di}
(expr_list:REG_DEAD (reg/f:DI 13 x13 [277])
(expr_list:REG_UNUSED (reg:DI 2 x2 [320])
(nil
/home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1:
internal compiler error: in copyprop_hardreg_forward_1, at regcprop.c:775
Please submit a full bug report,
with preprocessed source if appropriate.
for now only tested with the 4.9 linaro branch, now testing with trunk.
Matthias
--- - 2014-09-17 13:13:05.245022015 +
+++ test-summary2014-09-17 03:02:55.916771634 +
@@ -1,4 +1,4 @@
-Results for 4.9.1 (Ubuntu/Linaro 4.9.1-14ubuntu2) testsuite on
aarch64-unknown-linux-gnu
+Results for 4.9.1 (Ubuntu/Linaro 4.9.1-14ubuntu2.1) testsuite on
aarch64-unknown-linux-gnu
LAST_UPDATED: Fri Sep 12 17:12:16 UTC 2014 (revision 215228)
Native configuration is aarch64-unknown-linux-gnu
@@ -78,19 +78,16 @@
# of expected passes 116
# of unexpected failures 6
-/build/buildd/gcc-4.9-4.9.1/build/./gcc/gccgo version 4.9.1 (Ubuntu/Linaro
4.9.1-14ubuntu2)
+/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/./gcc/gccgo version 4.9.1
(Ubuntu/Linaro 4.9.1-14ubuntu2.1)
=== libgomp tests ===
Running target unix
-WARNING: program timed out.
-FAIL: libgomp.graphite/force-parallel-6.c execution test
=== libgomp Summary for unix ===
-# of expected passes 3240
-# of unexpected failures 1
+# of expected passes 3241
# of unsupported tests 36
Running target unix/-fno-stack-protector
@@ -102,8 +99,7 @@
=== libgomp Summary ===
-# of expected passes 6481
-# of unexpected failures 1
+# of expected passes 6482
# of unsupported tests 72
=== libitm tests ===
@@ -198,14 +194,29 @@
# of expected failures 82
# of unsupported tests 504
Target: aarch64-linux-gnu
-gcc version 4.9.1 (Ubuntu/Linaro 4.9.1-14ubuntu2)
+gcc version 4.9.1 (Ubuntu/Linaro 4.9.1-14ubuntu2.1)
=== g++ tests ===
Running target unix
+FAIL: g++.dg/opt/unroll1.C -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/opt/unroll1.C -std=gnu++98 (test for excess errors)
+UNRESOLVED: g++.dg/opt/unroll1.C -std=gnu++98 compilation failed to produce
executable
+FAIL: g++.dg/opt/unroll1.C -std=gnu++11 (internal compiler error)
+FAIL: g++.dg/opt/unroll1.C -std=gnu++11 (test for excess errors)
+UNRESOLVED: g++.dg/opt/unroll1.C -std=gnu++11 compilation failed to produce
executable
+FAIL: g++.dg/opt/unroll1.C -std=gnu++1y (internal compiler error)
+FAIL: g++.dg/opt/unroll1.C -std=gnu++1y (test for excess errors)
+UNRESOLVED: g++.dg/opt/unroll1.C -std=gnu++1y compilation failed to produce
executable
XPASS: g++.dg/tls/thread_local-order2.C -std=c++11 execution test
XPASS: g++.dg/tls/thread_local-order2.C -std=c++1y execution test
+FAIL: c-c++-common/torture/vector-shift.c -O3 -fomit-frame-pointer
-funroll-loops (internal compiler error)
+FAIL: c-c++-common/torture/vector-shift.c -O3 -fomit-frame-pointer
-funroll-loops (test for excess errors)
+UNRESOLVED: c-c++-common/torture/vector-shift.c -O3 -fomit-frame-pointer
-funroll-loops comp
Re: [gomp4] Offload option handling
On Wed, Sep 17, 2014 at 3:42 PM, Jakub Jelinek wrote: > I don't like the == in there. Doesn't , being a target triplet > or something like that, always have to start with alphanumeric character, > and options always have to start with -? Thus, can't you decide from the > first character after -ftarget-options= whether it is for all or specific > target? If the character after = is whitespace or -, it will be for > all targets and > -ftarget-options= > form, otherwise it must be > -ftarget-options== > form. Yes, I think it would be better. This patch is just a prototype to show the main idea, so I'd like to hear if you agree with the approach. On Wed, Sep 17, 2014 at 3:19 PM, Bernd Schmidt wrote: > I have no objections to supporting a -ftarget-options switch. I had posted a > patch a while ago that looked somewhat similar, but also contained an > automatic translation step from things like -march=x86_64 to a generic > -foffload-abi=lp64. I still think such a mechanism is desirable. I'm going to apply your patch on '-ftarget-options' stuff and check if it works together. -- Best regards, Andrey Turetskiy
Re: [PATCHv3] Vimrc config with GNU formatting
On Tue, Sep 16, 2014 at 05:58:58PM -0400, Trevor Saunders wrote: > fwiw, I think enabling it by default especially when that really means > enable it if you've enabled the localrc plugin makes sense. Enabling it by default means enabling it for all users. That is a really really bad plan; many of the options this script sets are user preferences. You can make Vim automatically adapt settings, but you cannot make the Vim user adapt to that. Of course Vim won't use this script by default anyway. It would be nice if there was some "modeline for this whole subtree" mechanism, but there is not. > I don't see > how you can enable the localrc plugin and then complain when people use > it for its designed purpose. Sure. If you have the localrc thing installed, anyone who can write files you can read can make your vim do *anything* (and I mean *anything*). It is a security disaster, that is, there is no security at all. It runs any script in the path from the file you open up to the root. There is no confirmation asked, no whitelist, no blacklist, no nothing. And no sandboxing either, of course. Did I mention /tmp? And writing to files? Or just opening a shell. The possibilities are endless! We should not encourage people to install this. Running it is reckless; telling other people to run it is irresponsible. > However something in contrib/ is probably > easier for new people to find than something on the wiki or something so > better than doing nothing :) Yup, just a bunch of recommended settings somewhere easy to find in contrib/ should be quite helpful to many people. Segher
Re: [PATCH 1/2, x86] Add palignr support for AVX2.
It fix "gcc.target/i386/pr52252-atom.c" in core-avx2 make check and pr62128. On Tue, Sep 16, 2014 at 6:51 PM, H.J. Lu wrote: > On Tue, Sep 16, 2014 at 6:15 AM, Evgeny Stupachenko > wrote: >> PING 2 >> >> On Mon, Sep 8, 2014 at 2:03 PM, Evgeny Stupachenko >> wrote: >>> PING >>> >>> On Wed, Aug 27, 2014 at 7:50 PM, Evgeny Stupachenko >>> wrote: The rotate insn appeared right after expand. I've done it similar to define_insn_and_split "*avx_vperm_broadcast_". I don't see any potential losses on splitting that after reload. On Tue, Aug 26, 2014 at 8:29 PM, Richard Henderson wrote: > On 08/26/2014 05:59 AM, Evgeny Stupachenko wrote: >> +(define_insn_and_split "avx2_rotate_perm" >> + [(set (match_operand:V_256 0 "register_operand" "=&x") >> + (vec_select:V_256 >> + (match_operand:V_256 1 "register_operand" "x") >> + (match_parallel 2 "palignr_operand" >> + [(match_operand 3 "const_int_operand" "n")])))] >> + "TARGET_AVX2" >> + "#" >> + "&& reload_completed" >> + [(const_int 0)] > > Why are you waiting until after reload to expand this? It's only the > vec_select parallel that determines which direction the palignr should be > done. > > This seems like something you could do during permutation expansion. > > > > Assuming your change is triggered today without any additional changes > you should include some testcases. For now, it doesn't show if it does > anything useful. > > -- > H.J.
Re: [ping*3] fix build failure of x86_64-mingw32, missing crtbegin/crtend.o
On Sep 17, 2014, at 12:55 , Kai Tietz wrote: >> We probably could twist our configuration scripts to experiment the -w64- >> variant as well. Might take a bit of time though. > > Well, would be interesting. Yes, I agree. I'll open an internal discussion to pursue this track. > As you are using for 64-bit anyway > mingw-w64's runtime, it doesn't make here any difference AFAICS. The > -pc- vs. the -w64- varaint just controls the enabling of some > additional features. So said, -pc- is legacy version. Understood. > So, patch is ok. Please apply patch to trunk. Will do. Thanks again for your feedback, Cheers, Olivier
Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)
We've just noticed this patch causes an ICE in
gcc.c-torture/execute/scal-to-vec1.c at -O3 when running with -fPIC on
arm-none-linux-gnueabi and arm-none-linux-gnueabihf; test logs:
spawn /tmp/alan/buildarm-none-linux-gnueabi/obj/gcc4/gcc/xgcc -B/tmp/alan/builda
rm-none-linux-gnueabi/obj/gcc4/gcc/ /work/alan/src/gcc/gcc/testsuite/gcc.c-tortu
re/execute/scal-to-vec1.c -fno-diagnostics-show-caret -fdiagnostics-color=never
-O3 -fomit-frame-pointer -w -lm -fPIC -o ./scal-to-vec1.exe
/work/alan/src/gcc/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c: In functi
on 'main':
/work/alan/src/gcc/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c:86:1: inte
rnal compiler error: in split_reg, at lra-constraints.c:4805
0x8b4cc7 split_reg
/work/alan/src/gcc/gcc/lra-constraints.c:4805
0x8b4f52 split_if_necessary
/work/alan/src/gcc/gcc/lra-constraints.c:4901
0x8b7d2c inherit_in_ebb
/work/alan/src/gcc/gcc/lra-constraints.c:5395
0x8b8786 lra_inheritance()
/work/alan/src/gcc/gcc/lra-constraints.c:5560
0x8abd84 lra(_IO_FILE*)
/work/alan/src/gcc/gcc/lra.c:2218
0x8705f6 do_reload
/work/alan/src/gcc/gcc/ira.c:5310
0x8705f6 execute
/work/alan/src/gcc/gcc/ira.c:5469
Complete list (for both arm-none-linux-gnueabi and arm-none-linux-gnueabihf,
both -fPIC):
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer (internal
compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer (test for
excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer
-funroll-loops (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer
-funroll-loops (test for excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions (test for excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -g (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c -O3 -g (test for excess errors)
Passing without -fPIC.
--Alan
Joseph S. Myers wrote:
PR 60606 reports an ICE on ARM when declaring a register variable in
register pc. Discussion in that bug suggests this usage should be
considered invalid and give an error. It turns out similar ICEs also
occur (after errors) for other cases of invalid register variables.
This patch fixes at least the original bug and others I observed in
the course of fixing it (there may well be other cases of registers,
on ARM and elsewhere, that still create such ICEs). To make pc
invalid for register variables, arm_regno_class is changed to return
NO_REGS for PC_REGNUM (which is consistent with REG_CLASS_CONTENTS).
Testing the scope of the bug showed similar issues for cc in Thumb
mode; that is made invalid by making arm_hard_regno_mode_ok disallow
it for non-MODE_CC modes (i.e. modes that might apply to a variable).
To avoid ICEs after errors, make_decl_rtl is make to clear
DECL_ASSEMBLER_NAME and DECL_HARD_REGISTER when a hard register
specification turns out to be invalid. cfgexpand.c:expand_one_var is
then made to call expand_one_error_var in this case, consistent with
other cases of erroneous variables.
Tested with no regressions for cross to arm-none-eabi (it also fixes
failures of gcc.dg/noncompile/920507-1.c, which is PR 61330). OK to
commit?
2014-08-22 Joseph Myers
PR target/60606
PR target/61330
* varasm.c (make_decl_rtl): Clear DECL_ASSEMBLER_NAME and
DECL_HARD_REGISTER and return for invalid register specifications.
* cfgexpand.c (expand_one_var): If expand_one_hard_reg_var clears
DECL_HARD_REGISTER, call expand_one_error_var.
* config/arm/arm.c (arm_hard_regno_mode_ok): Do not allow
CC_REGNUM with non-MODE_CC modes.
(arm_regno_class): Return NO_REGS for PC_REGNUM.
2014-08-22 Joseph Myers
PR target/60606
PR target/61330
* gcc.dg/torture/pr60606-1.c, gcc.target/arm/pr60606-2.c,
gcc.target/arm/pr60606-3.c, gcc.target/arm/pr60606-4.c: New tests.
Index: gcc/cfgexpand.c
===
--- gcc/cfgexpand.c (revision 214225)
+++ gcc/cfgexpand.c (working copy)
@@ -1307,7 +1307,12 @@ expand_one_var (tree var, bool toplevel, bool real
else if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var))
{
if (really_expand)
-expand_one_hard_reg_var (var);
+ {
+ expand_one_hard_reg_var (var);
+ if (!DECL_HARD_REGISTER (var))
+ /* Invalid register specification. */
+ expand_one_error_var (var);
+ }
}
else if (use_register_for_decl (var))
{
Index: gcc/testsuite/gcc.dg/torture/pr60606-1.c
===
--- gcc/testsuite/gcc.dg/tort
[PATCH] Fix PR63152
The following fixes PR63152 zeroing the data field only for allocatables, not
pointers. The benefit of the patch is a small speedup, and it avoids that code
starts to rely on behavior that is undefined in the standard. With this patch,
something like
INTEGER, DIMENSION(:), POINTER :: foo
IF (ASSOCIATED(foo)) ...
will be detected by valgrind as undefined behavior.
tested on x86_64-unknown-linux-gnu.
OK for trunk ?
JoostIndex: gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90
===
--- gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90 (revision 215321)
+++ gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90 (working copy)
@@ -11,6 +11,8 @@ end module global
program oh_no_not_pr15908_again
character(12), dimension(:), pointer :: ptr
+ nullify(ptr)
+
call a (ptr, 12)
if (.not.associated (ptr) ) call abort ()
if (any (ptr.ne."abc")) call abort ()
Index: gcc/testsuite/gfortran.dg/pr63152.f90
===
--- gcc/testsuite/gfortran.dg/pr63152.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/pr63152.f90 (revision 0)
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+! PR 63152 : needless init of local pointer arrays
+!
+! Contributed by Joost VandeVondele
+ SUBROUTINE S1()
+ INTEGER, POINTER, DIMENSION(:) :: v
+ INTERFACE
+SUBROUTINE foo(v)
+ INTEGER, POINTER, DIMENSION(:) :: v
+END SUBROUTINE
+ END INTERFACE
+ CALL foo(v)
+ END SUBROUTINE S1
+! { dg-final { scan-tree-dump-times "= 0B" 0 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }
Index: gcc/fortran/trans-array.c
===
--- gcc/fortran/trans-array.c (revision 215321)
+++ gcc/fortran/trans-array.c (working copy)
@@ -8647,8 +8647,8 @@ gfc_trans_deferred_array (gfc_symbol * s
type = TREE_TYPE (descriptor);
}
- /* NULLIFY the data pointer. */
- if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save)
+ /* NULLIFY the data pointer, for non-saved allocatables. */
+ if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save && sym->attr.allocatable)
gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node);
gfc_restore_backend_locus (&loc);
gcc/fortran/ChangeLog:
2014-09-17 Joost VandeVondele
PR fortran/63152
* trans-array.c (gfc_trans_deferred_array): Only nullify allocatables.
gcc/testsuite/ChangeLog:
2014-09-17 Joost VandeVondele
PR fortran/63152
* gfortran.dg/auto_char_dummy_array_1.f90: Fix undefined behavior.
* gfortran.dg/pr63152.f90: New test.
Re: [gomp4] Offload option handling
On Wed, Sep 17, 2014 at 03:16:52PM +0400, Andrey Turetskiy wrote: > Hi, > This patch (attached) contains the prototype of mechanism for passing > options to offload target compiler. > If one need to pass additional options for target compiler, one may > add option ‘–ftarget-options==’ to host > compiler (target name can be skipped, that will append specified > options for every target). > > Options preparing takes place in lto-wrapper in several stages: > 1. Read (host) options from .gnu.target_lto_.opts section for every > object file and merge them as during lto > 2. Remove CL_TARGET, CL_DRIVER and CL_LTO options > 3. For every offload target: search for option ‘–ftarget-options’ with > argument corresponding to target triple, parse it and merge with > remained host options > > Here is an example of option merging: > ./install/bin/gcc -fopenmp -c a.c -O0 -mno-sse -ftarget-options=="-O2 -msse" I don't like the == in there. Doesn't , being a target triplet or something like that, always have to start with alphanumeric character, and options always have to start with -? Thus, can't you decide from the first character after -ftarget-options= whether it is for all or specific target? If the character after = is whitespace or -, it will be for all targets and -ftarget-options= form, otherwise it must be -ftarget-options== form. Jakub
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, Sep 17, 2014 at 2:22 PM, Hans-Peter Nilsson wrote: > On Wed, 17 Sep 2014, Janne Blomqvist wrote: >> On Tue, Sep 16, 2014 at 11:17 AM, FX wrote: >> >>> 2014-09-05 Janne Blomqvist >> >>> >> >>>PR libfortran/62768 >> >>>* io/io.h (gfc_unit): Store C string for the filename. >> >>>* io/close.c (st_close): Use gfc_unit.filename. >> >>>* io/inquire.c (inquire_via_unit): Likewise. >> >>>* io/open.c (new_unit): Likewise. >> >>>(already_open): Likewise, unlink file before freeing filename. >> >>>* io/unit.c (init_units): Likewise. >> >>>(close_unit_1): Likewise. >> >>>(filename_from_unit): Likewise. >> >>>* io/unix.c (compare_file_filename): Likewise. >> >>>(find_file0): Likewise. >> >>>(delete_file): Likewise. >> > >> > OK, if you add a runtime testcase. >> >> Thanks for the review, committed as r215307. > > Something went wrong. For cris-elf: > > libtool: compile: /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc > -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc > -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem > /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem > /tmp/hpautotest-gcc1/gcc/newlib/libc/include > -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris > -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys > -L/tmp/hpautotest-gcc1/gcc/libgloss/cris > -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ > -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem > /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem > /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -DHAVE_CONFIG_H -I. > -I/tmp/hpautotest-gcc1/gcc/libgfortran > -iquote/tmp/hpautotest-gcc1/gcc/libgfortran/io > -I/tmp/hpautotest-gcc1/gcc/libgfortran/../gcc > -I/tmp/hpautotest-gcc1/gcc/libgfortran/../gcc/config -I../.././gcc > -I/tmp/hpautotest-gcc1/gcc/libgfortran/../libgcc -I../libgcc -std=gnu11 -Wall > -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra > -Wwrite-strings -fcx-fortran-rules -ffunction-sections -fdata-sections -g -O2 > -MT inquire.lo -MD -MP -MF .deps/inquire.Tpo -c > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c -o inquire.o > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c: In function > 'inquire_via_unit': > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:41: error: 'gfc_unit' > has no member named 'file' > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > ^ > /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:50: error: 'gfc_unit' > has no member named 'file_len' > fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); > ^ > make[3]: *** [inquire.lo] Error 1 > > brgds, H-P Oops, I forgot to update some parts in an #ifdef branch that isn't taken on my target. I'll try to find time to fix it later tonight. If you're in a hurry, just replace fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); with cf_strcpy (iqp->name, iqp->name_len, u->filename); in inquire.c. -- Janne Blomqvist
Re: [PR libfortran/62768] Handle filenames with embedded nulls
On Wed, 17 Sep 2014, Janne Blomqvist wrote: > On Tue, Sep 16, 2014 at 11:17 AM, FX wrote: > >>> 2014-09-05 Janne Blomqvist > >>> > >>>PR libfortran/62768 > >>>* io/io.h (gfc_unit): Store C string for the filename. > >>>* io/close.c (st_close): Use gfc_unit.filename. > >>>* io/inquire.c (inquire_via_unit): Likewise. > >>>* io/open.c (new_unit): Likewise. > >>>(already_open): Likewise, unlink file before freeing filename. > >>>* io/unit.c (init_units): Likewise. > >>>(close_unit_1): Likewise. > >>>(filename_from_unit): Likewise. > >>>* io/unix.c (compare_file_filename): Likewise. > >>>(find_file0): Likewise. > >>>(delete_file): Likewise. > > > > OK, if you add a runtime testcase. > > Thanks for the review, committed as r215307. Something went wrong. For cris-elf: libtool: compile: /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -DHAVE_CONFIG_H -I. -I/tmp/hpautotest-gcc1/gcc/libgfortran -iquote/tmp/hpautotest-gcc1/gcc/libgfortran/io -I/tmp/hpautotest-gcc1/gcc/libgfortran/../gcc -I/tmp/hpautotest-gcc1/gcc/libgfortran/../gcc/config -I../.././gcc -I/tmp/hpautotest-gcc1/gcc/libgfortran/../libgcc -I../libgcc -std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -fcx-fortran-rules -ffunction-sections -fdata-sections -g -O2 -MT inquire.lo -MD -MP -MF .deps/inquire.Tpo -c /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c -o inquire.o /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c: In function 'inquire_via_unit': /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:41: error: 'gfc_unit' has no member named 'file' fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); ^ /tmp/hpautotest-gcc1/gcc/libgfortran/io/inquire.c:97:50: error: 'gfc_unit' has no member named 'file_len' fstrcpy (iqp->name, iqp->name_len, u->file, u->file_len); ^ make[3]: *** [inquire.lo] Error 1 brgds, H-P
Re: [gomp4] Offload option handling
On 09/17/2014 01:16 PM, Andrey Turetskiy wrote: How does this look? Do you agree with the approach? I have no objections to supporting a -ftarget-options switch. I had posted a patch a while ago that looked somewhat similar, but also contained an automatic translation step from things like -march=x86_64 to a generic -foffload-abi=lp64. I still think such a mechanism is desirable. Bernd
Re: [gomp4] Offload option handling
Hi, This patch (attached) contains the prototype of mechanism for passing options to offload target compiler. If one need to pass additional options for target compiler, one may add option ‘–ftarget-options==’ to host compiler (target name can be skipped, that will append specified options for every target). Options preparing takes place in lto-wrapper in several stages: 1. Read (host) options from .gnu.target_lto_.opts section for every object file and merge them as during lto 2. Remove CL_TARGET, CL_DRIVER and CL_LTO options 3. For every offload target: search for option ‘–ftarget-options’ with argument corresponding to target triple, parse it and merge with remained host options Here is an example of option merging: ./install/bin/gcc -fopenmp -c a.c -O0 -mno-sse -ftarget-options=="-O2 -msse" ./install/bin/gcc -fopenmp -c b.c -O0 -fpack-struct -ftarget-options=="-O3 -mavx" ./install/bin/gcc -fopenmp a.o b.o After merging at stage 3 we get: -O3 -fpack-struct -ftarget-options==”-O2 –msse” -ftarget-options==”-O3 –mavx” –msse –mavx These options are passed to mkoffload and thus to target compiler. -ftarget-options==”…” itself is ignored by target compiler. However there is an issue: options specified by -ftarget-options are not checked for correctness as usual, so incorrect options inside -ftarget-options==”…” may lead to GCC crashes. Need to investigate possibilities of reusing existing option checks to solve the problem. How does this look? Do you agree with the approach? On Mon, Sep 15, 2014 at 8:22 PM, Ilya Verbin wrote: > On 23 Jul 14:51, Bernd Schmidt wrote: >> Ping. >> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00616.html >> >> On 06/06/2014 05:07 PM, Bernd Schmidt wrote: >> >There's a problem when offloading from a compiler for one target machine >> >to another: the machine specific options don't necessarily match. This >> >patch tries to address this. >> > >> >The idea is that since we have two options sections anyway, with >> >different section name prefixes, we can arrange to pass only >> >target-independent options in the omp_ version of the options section. >> >However, some target-specific options (specifically the ones specifying >> >the ABI) need to be preserved somehow, so there's a new target hook for >> >translating them to common a -foffload-* syntax. >> > >> >How does this look? Comments on the approach, and ok for the >> >gomp-4_0-branch? > > How about passing target-specific options from the command-line? Will it be > possible with this approach? > > We had a patch, that parses options from -foffload-target== > and > stores them to the environment variable. Now it is obsolete, but I like the > idea of having such a user-adjustable options for the accel compilers. > > https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01242.html > > -- Ilya -- Best regards, Andrey Turetskiy offload-option-gomp4.patch Description: Binary data
Re: [arm][patch] fix arm_neon_ok check on !arm_arch7
On 15/09/14 14:29, Richard Earnshaw wrote: Yep, that's fine. Committed, thanks. Andrew
Re: [ping*3] fix build failure of x86_64-mingw32, missing crtbegin/crtend.o
2014-09-17 12:38 GMT+02:00 Olivier Hainque : > Hello Kai, > > On Sep 17, 2014, at 10:52 , Kai Tietz wrote: > >> Sorry for the delay in review. > > No problem at all. Thanks for your feedback :) > >> Patch looks ok. Have you just tested >> -pc- variant, or also -w64- one? > > Just the -pc- variant, by our nightly builders. > > We probably could twist our configuration scripts to experiment the -w64- > variant as well. Might take a bit of time though. Well, would be interesting. As you are using for 64-bit anyway mingw-w64's runtime, it doesn't make here any difference AFAICS. The -pc- vs. the -w64- varaint just controls the enabling of some additional features. So said, -pc- is legacy version. > AFAICS, the w64 variant resorts to the crtstuff objects as well and there's > no special case for -w64 in libgcc/config.host, so it really looks like the > change is needed for this variant also. > > Regards, > > Olivier So, patch is ok. Please apply patch to trunk. Kai
Re: [Patch, AArch64] Enable Address sanitizer and UB sanitizer
On 9 September 2014 13:08, Christophe Lyon wrote:
> On 9 September 2014 12:03, wrote:
>>
>>
>>> On Sep 9, 2014, at 2:50 AM, Marcus Shawcroft
>>> wrote:
>>>
>>> +static unsigned HOST_WIDE_INT
>>> +aarch64_asan_shadow_offset (void)
>>> +{
>>> + return (HOST_WIDE_INT_1 << 36);
>>> +}
>>> +
>>>
>>> Looking around various other ports I see magic numbers including 29,
>>> 41, 44 Help me understand why 36 is the right choice for aarch64?
>>
>> Also why 36? What is the min virtual address space aarch64 Linux kernel
>> supports with 4k pages and 3 level page table? Also does this need to
>> conditionalized on lp64? Since I am about to post glibc patches turning on
>> address sanitizer breaks that.
>>
>
> The address space is 2^39 according to /proc/self/maps:
> [...]
>
> The shadow offset is obtained by dividing this value by 8 -> 2^36.
>
> Note that this value has to match kAArch64_ShadowOffset64 as defined
> in libsanitizer/asan/asan_mapping.h.
>
> I do expect a followup patch to support ilp32, but I wouldn't post a
> patch which I haven't tested.
Presumably for ILP32 the shadow offset should be 1<<29 and we will
need to make both asan_mapping.h and aarch64_asan_shadow_offset
conditional.
This patch for LP64 is OK.
Thanks
/Marcus
Re: [ping*3] fix build failure of x86_64-mingw32, missing crtbegin/crtend.o
Hello Kai, On Sep 17, 2014, at 10:52 , Kai Tietz wrote: > Sorry for the delay in review. No problem at all. Thanks for your feedback :) > Patch looks ok. Have you just tested > -pc- variant, or also -w64- one? Just the -pc- variant, by our nightly builders. We probably could twist our configuration scripts to experiment the -w64- variant as well. Might take a bit of time though. AFAICS, the w64 variant resorts to the crtstuff objects as well and there's no special case for -w64 in libgcc/config.host, so it really looks like the change is needed for this variant also. Regards, Olivier
Re: [PATCH, i386, Pointer Bounds Checker 2/x] Intel Memory Protection Extensions (MPX) instructions support
2014-09-17 13:46 GMT+04:00 Uros Bizjak : > On Wed, Sep 17, 2014 at 10:17 AM, Ilya Enkovich > wrote: > >>> >>>This patch introduces Intel MPX bound registers and instructions. It >>> >>>was approved earlier for 4.9 and had no significant changes since then. >>> >>>I'll assume patch is OK if no objections arise. >>> >>> >>> >>>Patch was bootstrapped and tested for linux-x86_64. >>> OK for the trunk. Please wait until entire set is approved before >>> installing. >>> >>> jeff >>> >> >> Here is an updated version. The only change is in _ldx expand. It >> now has the second operand preparation code moved from ix86_expand_builtin >> as was proposed by Uros. > > How does this patch fit into current patch series? > > Does your patch serie compile at every patch boundary? I am especially > interested in i386 target-dependent patches (30+). This one is the second from the series. Few months ago I checked compiler passes bootstrap and make check after each patch. I didn't run make check for each patch after the latest rebase due to decision to not commit separate patches but to commit whole series instead. So I just checked it compiles at every patch boundary. Thanks, Ilya > > Uros.
Re: [PATCH, i386, Pointer Bounds Checker 2/x] Intel Memory Protection Extensions (MPX) instructions support
On Wed, Sep 17, 2014 at 10:17 AM, Ilya Enkovich wrote: >> >>>This patch introduces Intel MPX bound registers and instructions. It was >> >>>approved earlier for 4.9 and had no significant changes since then. I'll >> >>>assume patch is OK if no objections arise. >> >>> >> >>>Patch was bootstrapped and tested for linux-x86_64. >> OK for the trunk. Please wait until entire set is approved before >> installing. >> >> jeff >> > > Here is an updated version. The only change is in _ldx expand. It now > has the second operand preparation code moved from ix86_expand_builtin as was > proposed by Uros. How does this patch fit into current patch series? Does your patch serie compile at every patch boundary? I am especially interested in i386 target-dependent patches (30+). Uros.
Re: [PATCH, i386, Pointer Bounds Checker 31/x] Pointer Bounds Checker builtins for i386 target
On Wed, Sep 17, 2014 at 10:11 AM, Ilya Enkovich wrote:
> On 16 Sep 12:02, Uros Bizjak wrote:
>>
>> Hm, can this patch be compiled as part of the series? The expanders
>> refer to various gen_bnd patterns that I don't see. Also, I don't see
>> BND mode introduced.
>
> Hi,
>
> Here is a patch from the series that introduces modes and instructions:
> https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00880.html. It needs update in
> bndldx expander as you suggested.
>
>>
>> Anyway, some general observations:
>>
>> > +case IX86_BUILTIN_BNDLDX:
>> > + arg0 = CALL_EXPR_ARG (exp, 0);
>> > + arg1 = CALL_EXPR_ARG (exp, 1);
>> > +
>> > + op0 = expand_normal (arg0);
>> > + op1 = expand_normal (arg1);
>> > +
>> > + op0 = force_reg (Pmode, op0);
>> > + op1 = force_reg (Pmode, op1);
>> > +
>> > + /* Avoid registers which connot be used as index. */
>> > + if (!index_register_operand (op1, Pmode))
>> > + {
>> > + rtx temp = gen_reg_rtx (Pmode);
>> > + emit_move_insn (temp, op1);
>> > + op1 = temp;
>> > + }
>> > +
>> > + /* If op1 was a register originally then it may have
>> > +mode other than Pmode. We need to extend in such
>> > +case because bndldx may work only with Pmode regs. */
>> > + if (GET_MODE (op1) != Pmode)
>> > + op1 = ix86_zero_extend_to_Pmode (op1);
>> > +
>> > + if (REG_P (target))
>> > + emit_insn (TARGET_64BIT
>> > + ? gen_bnd64_ldx (target, op0, op1)
>> > + : gen_bnd32_ldx (target, op0, op1));
>> > + else
>> > + {
>> > + rtx temp = gen_reg_rtx (BNDmode);
>> > + emit_insn (TARGET_64BIT
>> > +? gen_bnd64_ldx (temp, op0, op1)
>> > +: gen_bnd32_ldx (temp, op0, op1));
>> > + emit_move_insn (target, temp);
>> > + }
>> > + return target;
>>
>> I don't like the way arguments are prepared. For the case above,
>> bnd_ldx should have index_register_operand predicate in its pattern,
>> and this predicate (and its mode) should be checked in the expander
>> code. There are many examples of argument expansion in
>> ix86_expand_builtin function, including how Pmode is handled.
>>
>> Also, please see how target is handled there. Target can be null, so
>> REG_P predicate will crash.
>>
>> You should also select insn patterns depending on BNDmode, not TARGET_64BIT.
>>
>> Please use assign_386_stack_local so stack slots can be shared.
>> SLOT_TEMP is intended for short-lived temporaries, you can introduce
>> new slots if you need more live values at once.
>>
>> Uros.
>
> Thanks for comments! Here is a new version in which I addressed all your
> concerns.
Unfortunately, it doesn't. The patch only fixed one instance w.r.t to
target handling, the one I referred as an example. You still have
unchecked target, at least in IX86_BUILTIN_BNDMK.
However, you have a general problems in your builtin expansion code,
so please look at how other builtins are handled. E.g.:
if (optimize || !target
|| GET_MODE (target) != tmode
|| !register_operand(target, tmode))
target = gen_reg_rtx (tmode);
also, here is an example how input operands are prepared:
op0 = expand_normal (arg0);
op1 = expand_normal (arg1);
op2 = expand_normal (arg2);
if (!register_operand (op0, Pmode))
op0 = ix86_zero_extend_to_Pmode (op0);
if (!register_operand (op1, SImode))
op1 = copy_to_mode_reg (SImode, op1);
if (!register_operand (op2, SImode))
op2 = copy_to_mode_reg (SImode, op2);
So, Pmode is handled in a special way, even when x32 is not considered.
BTW: I wonder if word_mode is needed here, Pmode can be SImode with
address prefix (x32).
Inside the expanders, please use expand_simple_binop and expand_unop
on RTX, not tree expressions. Again, please see many examples.
Please use emit_move_insn instead of
emit_insn (gen_move_insn (m1, op1));
As a wish, can you perhaps write a generic cmove expander to be also
used in other places?
> +static void
> +ix86_emit_move_max (rtx dst, rtx src)
Uros.
> Ilya
> --
> 2014-09-17 Ilya Enkovich
>
> * config/i386/i386-builtin-types.def (BND): New.
> (ULONG): New.
> (BND_FTYPE_PCVOID_ULONG): New.
> (VOID_FTYPE_BND_PCVOID): New.
> (VOID_FTYPE_PCVOID_PCVOID_BND): New.
> (BND_FTYPE_PCVOID_PCVOID): New.
> (BND_FTYPE_PCVOID): New.
> (BND_FTYPE_BND_BND): New.
> (PVOID_FTYPE_PVOID_PVOID_ULONG): New.
> (PVOID_FTYPE_PCVOID_BND_ULONG): New.
> (ULONG_FTYPE_VOID): New.
> (PVOID_FTYPE_BND): New.
> * config/i386/i386.c: Include tree-chkp.h, rtl-chkp.h.
> (ix86_builtins): Add
> IX86_BUILTIN_BNDMK, IX86_BUILTIN_BNDSTX,
> IX86_BUILTIN_BNDLDX, IX86_BUILTIN_BNDCL,
> IX86_BUILTIN_BNDCU, IX86_BUILTIN_BNDRET,
> IX86_BUILTIN_BNDNARROW, IX86_BUILTIN_BNDINT,
> IX86_BUILTIN
Re: [PATCH, ira] Ignore some conflict cost
On Wed, Sep 17, 2014 at 05:18:27PM +0800, Zhenqiang Chen wrote: > Hi, > > When assign_hard_reg, we always check_hard_reg_p, which has code > > if (! TEST_HARD_REG_BIT (profitable_regs, hard_regno)) > return false; > > i.e. If a hard_regno is not in profitable_regs, we can not allocate it to > the ira_allocno_t A. > > So the conflict on a hard_regno, which does not belong to the > profitable_regs, can be ignored. Please refer > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63210 for the detail of the > test case. > > Bootstrap and no make check regression on X86-64 and ARM Chromebook. > NO spec2k performance regression on X86-64 and ARM Chromebook. > CSiBE code size is 0.01% better for ARM Cortex-M0. Note that your MUA malformed the patch, so it can't be easily applied. Marek
Re: [patch] allowing configure --target=e500v[12]-etc
Hello Joseph, On Sep 16, 2014, at 23:01 , Joseph S. Myers wrote: > config.sub patches have to go to config-patches first, we only ever import > the latest unmodified config.sub and config.guess from config.git, without > making local changes. Ok, will start there then. Thanks for your prompt feedback. Cheers, Olivier
[PATCH, ira] Ignore some conflict cost
Hi,
When assign_hard_reg, we always check_hard_reg_p, which has code
if (! TEST_HARD_REG_BIT (profitable_regs, hard_regno))
return false;
i.e. If a hard_regno is not in profitable_regs, we can not allocate it to
the ira_allocno_t A.
So the conflict on a hard_regno, which does not belong to the
profitable_regs, can be ignored. Please refer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63210 for the detail of the
test case.
Bootstrap and no make check regression on X86-64 and ARM Chromebook.
NO spec2k performance regression on X86-64 and ARM Chromebook.
CSiBE code size is 0.01% better for ARM Cortex-M0.
OK for trunk?
Thanks!
-Zhenqiang
ChangeLog:
2014-09-17 Zhenqiang Chen
PR rtl-optimization/63210
* ira-color.c (assign_hard_reg): Ignore conflict cost if the
HARD_REGNO is not availabe for CONFLICT_A.
testsuite/ChangeLog:
2014-09-17 Zhenqiang Chen
* gcc.target/arm/pr63210.c: New test.
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index e2ea359..e3fea54 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -1709,6 +1709,7 @@ assign_hard_reg (ira_allocno_t a, bool retry_p)
{
ira_allocno_t conflict_a = OBJECT_ALLOCNO (conflict_obj);
enum reg_class conflict_aclass;
+ allocno_color_data_t data = ALLOCNO_COLOR_DATA (conflict_a);
/* Reload can give another class so we need to check all
allocnos. */
@@ -1780,7 +1781,12 @@ assign_hard_reg (ira_allocno_t a, bool retry_p)
hard_regno = ira_class_hard_regs[aclass][j];
ira_assert (hard_regno >= 0);
k =
ira_class_hard_reg_index[conflict_aclass][hard_regno];
- if (k < 0)
+ if (k < 0
+ /* If HARD_REGNO is not available for CONFLICT_A,
+ the conflict would be ignored, since
HARD_REGNO
+ will never be assigned to CONFLICT_A. */
+ || !TEST_HARD_REG_BIT (data->profitable_hard_regs,
+ hard_regno))
continue;
full_costs[j] -= conflict_costs[k];
}
diff --git a/gcc/testsuite/gcc.target/arm/pr63210.c
b/gcc/testsuite/gcc.target/arm/pr63210.c
new file mode 100644
index 000..c3ae928
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr63210.c
@@ -0,0 +1,12 @@
+/* { dg-do assemble } */
+/* { dg-options "-mthumb -Os " } */
+/* { dg-require-effective-target arm_thumb1_ok } */
+
+int foo1 (int c);
+int foo2 (int c);
+
+int test (int c)
+{
+ return (foo1 (c) || foo2 (c));
+}
+/* { dg-final { object-size text <= 28 } } */
Re: [ping*3] fix build failure of x86_64-mingw32, missing crtbegin/crtend.o
Hello Oliver, Sorry for the delay in review. Patch looks ok. Have you just tested -pc- variant, or also -w64- one? Thanks, Kai
Re: [gomp4] various OpenACC/PTX built-ins and a reduction tweak
On Wed, Sep 17, 2014 at 10:44:12AM +0200, Tobias Burnus wrote:
> Cesar Philippidis wrote:
> > The patch introduces the following OpenACC/PTX-specific built-ins:
> ...
>
> It is not completely clear how they are supposed to get used. Should the
> user call them directly in some cases? Or are they only used internally?
>
> acc_on_device sounds like a function which would be in C/C++ made available
> to the user via #define acc_on_device __builtin_acc_on_device.
And not just providing acc_on_device prototype in some header? Without
looking at the OpenACC standard, it sounds like this function could be
similar to omp_is_initial_device, so can and should be handled supposedly
similarly.
>
> However, the rest looks as if it should rather be an internal function
> instead of a builtin. Or should the user really ever call the builtin
> directly?
GOMP_* functions are builtins and not internal functions too, all those
functions are library functions, while the user typically doesn't call them
directly, they still are implemented in the library. Internal functions are
used for something that doesn't have a library implementation and is not
something user can call directly.
> Regarding Fortran: Builtins aren't directly available to the user. You have to
> wrap them into an intrinsic to make them available. If they have to be made
> available via a module (e.g. via "module acc) - you have to create a virtual
> module, which provides the intrinsic. If you don't want to convert the whole
> module, you could create an auxiliar module (e.g. acc_internal_) which
> provides
> only those bits - and then include it ("use,intrinsic :: ...") it in the
> main module - written in normal Fortran.
For the user callable fortran functions, for OpenMP libgomp just provides
*_ entrypoints to * functions. Perhaps acc_on_device_ could be provided
too.
Jakub
Re: [gomp4] various OpenACC/PTX built-ins and a reduction tweak
Hi,
Cesar Philippidis wrote:
> The patch introduces the following OpenACC/PTX-specific built-ins:
...
It is not completely clear how they are supposed to get used. Should the
user call them directly in some cases? Or are they only used internally?
acc_on_device sounds like a function which would be in C/C++ made available
to the user via #define acc_on_device __builtin_acc_on_device.
However, the rest looks as if it should rather be an internal function
instead of a builtin. Or should the user really ever call the builtin
directly?
Regarding Fortran: Builtins aren't directly available to the user. You have to
wrap them into an intrinsic to make them available. If they have to be made
available via a module (e.g. via "module acc) - you have to create a virtual
module, which provides the intrinsic. If you don't want to convert the whole
module, you could create an auxiliar module (e.g. acc_internal_) which provides
only those bits - and then include it ("use,intrinsic :: ...") it in the
main module - written in normal Fortran.
But in any case, it would be helpful to know how those new built ins are
intended
to be used.
Tobias
Re: [PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
On 16/09/14 23:03, James Greenhalgh wrote: > On Tue, Sep 16, 2014 at 10:36:08PM +0100, Andrew Pinski wrote: >> On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh >> wrote: >>> On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: Hi maintainers, I just added "=r" and retested it. >>> >>> I had a very similar patch to this sitting in my local tree. However, >>> I am surprised you have left operand 3 as an output operand. In my tree >>> I had marked operand 3 as "&r". >>> >>> What do you think? >> >> The clobber needs to be "=&r" as you are writing to the register and >> not just reading from it. I think this is causing some issues >> including linaro bugzilla #667 >> (https://bugs.linaro.org/show_bug.cgi?id=667). > > (+CC Matthias Klose and Steve McIntyre who have also been in contact with me > regarding this bug) > > I've seen this bug locally, and had considered sending the patch you > suggested, which does indeed fix the bug. However, it feels wrong as > the operand is not a formal output of the pattern. It is clobbered - and > indeed earlyclobbered - so yes it is written to, but it isn't an output. > This makes the fix look like a band-aid around the real problem. > > The bug looks similar to pr52573 - regrename fails to spot that it should > not rename to a register used in an earlyclobber operand of any type, rather > than just an output+earlyclobber operand as it does now. > > I've played about with a fix that sits in regrename, and forces it to think > of all earlyclobber operands as starting and ending chains but this didn't > bootstrap clean - we end up with what I believe are false reports of stack > smashing in libstdc++. > > I was planning to look again at my approach tomorrow, I would like to > convince myself that this isn't a deficiency in regrename before I would > support just marking this operand "=&r". > > If you have any other suggestions, or if "=&r" is actually correct and > I am misreading the documentation please let me know. > > Thanks, > James > "=&r" is correct for an early-clobbered scratch. R.
Re: [PATCH, i386, Pointer Bounds Checker 2/x] Intel Memory Protection Extensions (MPX) instructions support
On 19 May 11:23, Jeff Law wrote:
> On 05/19/14 02:19, Ilya Enkovich wrote:
> >On 16 May 13:39, Jeff Law wrote:
> >>On 04/16/14 05:35, Ilya Enkovich wrote:
> >>>Hi,
> >>>
> >>>This patch introduces Intel MPX bound registers and instructions. It was
> >>>approved earlier for 4.9 and had no significant changes since then. I'll
> >>>assume patch is OK if no objections arise.
> >>>
> >>>Patch was bootstrapped and tested for linux-x86_64.
> OK for the trunk. Please wait until entire set is approved before
> installing.
>
> jeff
>
Here is an updated version. The only change is in _ldx expand. It now
has the second operand preparation code moved from ix86_expand_builtin as was
proposed by Uros.
Thanks,
Ilya
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 8e0a583..4e07d70 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -19,7 +19,7 @@
;;; Unused letters:
;;; H
-;;; h jw z
+;;; h j z
;; Integer register constraints.
;; It is not necessary to define 'r' here.
@@ -94,6 +94,9 @@
(define_register_constraint "v" "TARGET_SSE ? ALL_SSE_REGS : NO_REGS"
"Any EVEX encodable SSE register (@code{%xmm0-%xmm31}).")
+(define_register_constraint "w" "TARGET_MPX ? BND_REGS : NO_REGS"
+ "@internal Any bound register.")
+
;; We use the Y prefix to denote any number of conditional register sets:
;; z First SSE register.
;; i SSE2 inter-unit moves to SSE register enabled
@@ -253,6 +256,8 @@
;; T prefix is used for different address constraints
;; v - VSIB address
;; s - address with no segment register
+;; i - address with no index and no rip
+;; b - address with no base and no rip
(define_address_constraint "Tv"
"VSIB address operand"
@@ -261,3 +266,11 @@
(define_address_constraint "Ts"
"Address operand without segment register"
(match_operand 0 "address_no_seg_operand"))
+
+(define_address_constraint "Ti"
+ "MPX address operand without index"
+ (match_operand 0 "address_mpx_no_index_operand"))
+
+(define_address_constraint "Tb"
+ "MPX address operand without base"
+ (match_operand 0 "address_mpx_no_base_operand"))
diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 2c05cec..a1e9289 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -399,6 +399,8 @@ ix86_target_macros_internal (HOST_WIDE_INT isa_flag,
def_or_undef (parse_in, "__XSAVEC__");
if (isa_flag & OPTION_MASK_ISA_XSAVES)
def_or_undef (parse_in, "__XSAVES__");
+ if (isa_flag & OPTION_MASK_ISA_MPX)
+def_or_undef (parse_in, "__MPX__");
}
diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def
index 07e5720..0e302e3 100644
--- a/gcc/config/i386/i386-modes.def
+++ b/gcc/config/i386/i386-modes.def
@@ -87,6 +87,9 @@ VECTOR_MODE (INT, DI, 1); /* V1DI */
VECTOR_MODE (INT, SI, 1); /* V1SI */
VECTOR_MODE (INT, QI, 2); /* V2QI */
+POINTER_BOUNDS_MODE (BND32, 8);
+POINTER_BOUNDS_MODE (BND64, 16);
+
INT_MODE (OI, 32);
INT_MODE (XI, 64);
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 39462bd..c8ef2d2 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -231,6 +231,8 @@ extern void ix86_expand_sse2_mulv4si3 (rtx, rtx, rtx);
extern void ix86_expand_sse2_mulvxdi3 (rtx, rtx, rtx);
extern void ix86_expand_sse2_abs (rtx, rtx);
+extern bool ix86_bnd_prefixed_insn_p (rtx);
+
/* In i386-c.c */
extern void ix86_target_macros (void);
extern void ix86_register_pragmas (void);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 929f1b1..01823ca 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2131,6 +2131,8 @@ enum reg_class const regclass_map[FIRST_PSEUDO_REGISTER] =
/* Mask registers. */
MASK_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
+ /* MPX bound registers */
+ BND_REGS, BND_REGS, BND_REGS, BND_REGS,
};
/* The "default" register map used in 32bit mode. */
@@ -2147,6 +2149,7 @@ int const dbx_register_map[FIRST_PSEUDO_REGISTER] =
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 16-23*/
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 24-31*/
93, 94, 95, 96, 97, 98, 99, 100, /* Mask registers */
+ 101, 102, 103, 104, /* bound registers */
};
/* The "default" register map used in 64bit mode. */
@@ -2163,6 +2166,7 @@ int const dbx64_register_map[FIRST_PSEUDO_REGISTER] =
67, 68, 69, 70, 71, 72, 73, 74, /* AVX-512 registers 16-23 */
75, 76, 77, 78, 79, 80, 81, 82, /* AVX-512 registers 24-31 */
118, 119, 120, 121, 122, 123, 124, 125, /* Mask registers */
+ 126, 127, 128, 129, /* bound registers */
};
/* Define the register numbers to be used in Dwarf debugging information.
@@
Re: [PATCH, i386, Pointer Bounds Checker 31/x] Pointer Bounds Checker builtins for i386 target
On 16 Sep 12:02, Uros Bizjak wrote:
>
> Hm, can this patch be compiled as part of the series? The expanders
> refer to various gen_bnd patterns that I don't see. Also, I don't see
> BND mode introduced.
Hi,
Here is a patch from the series that introduces modes and instructions:
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00880.html. It needs update in
bndldx expander as you suggested.
>
> Anyway, some general observations:
>
> > +case IX86_BUILTIN_BNDLDX:
> > + arg0 = CALL_EXPR_ARG (exp, 0);
> > + arg1 = CALL_EXPR_ARG (exp, 1);
> > +
> > + op0 = expand_normal (arg0);
> > + op1 = expand_normal (arg1);
> > +
> > + op0 = force_reg (Pmode, op0);
> > + op1 = force_reg (Pmode, op1);
> > +
> > + /* Avoid registers which connot be used as index. */
> > + if (!index_register_operand (op1, Pmode))
> > + {
> > + rtx temp = gen_reg_rtx (Pmode);
> > + emit_move_insn (temp, op1);
> > + op1 = temp;
> > + }
> > +
> > + /* If op1 was a register originally then it may have
> > +mode other than Pmode. We need to extend in such
> > +case because bndldx may work only with Pmode regs. */
> > + if (GET_MODE (op1) != Pmode)
> > + op1 = ix86_zero_extend_to_Pmode (op1);
> > +
> > + if (REG_P (target))
> > + emit_insn (TARGET_64BIT
> > + ? gen_bnd64_ldx (target, op0, op1)
> > + : gen_bnd32_ldx (target, op0, op1));
> > + else
> > + {
> > + rtx temp = gen_reg_rtx (BNDmode);
> > + emit_insn (TARGET_64BIT
> > +? gen_bnd64_ldx (temp, op0, op1)
> > +: gen_bnd32_ldx (temp, op0, op1));
> > + emit_move_insn (target, temp);
> > + }
> > + return target;
>
> I don't like the way arguments are prepared. For the case above,
> bnd_ldx should have index_register_operand predicate in its pattern,
> and this predicate (and its mode) should be checked in the expander
> code. There are many examples of argument expansion in
> ix86_expand_builtin function, including how Pmode is handled.
>
> Also, please see how target is handled there. Target can be null, so
> REG_P predicate will crash.
>
> You should also select insn patterns depending on BNDmode, not TARGET_64BIT.
>
> Please use assign_386_stack_local so stack slots can be shared.
> SLOT_TEMP is intended for short-lived temporaries, you can introduce
> new slots if you need more live values at once.
>
> Uros.
Thanks for comments! Here is a new version in which I addressed all your
concerns.
Ilya
--
2014-09-17 Ilya Enkovich
* config/i386/i386-builtin-types.def (BND): New.
(ULONG): New.
(BND_FTYPE_PCVOID_ULONG): New.
(VOID_FTYPE_BND_PCVOID): New.
(VOID_FTYPE_PCVOID_PCVOID_BND): New.
(BND_FTYPE_PCVOID_PCVOID): New.
(BND_FTYPE_PCVOID): New.
(BND_FTYPE_BND_BND): New.
(PVOID_FTYPE_PVOID_PVOID_ULONG): New.
(PVOID_FTYPE_PCVOID_BND_ULONG): New.
(ULONG_FTYPE_VOID): New.
(PVOID_FTYPE_BND): New.
* config/i386/i386.c: Include tree-chkp.h, rtl-chkp.h.
(ix86_builtins): Add
IX86_BUILTIN_BNDMK, IX86_BUILTIN_BNDSTX,
IX86_BUILTIN_BNDLDX, IX86_BUILTIN_BNDCL,
IX86_BUILTIN_BNDCU, IX86_BUILTIN_BNDRET,
IX86_BUILTIN_BNDNARROW, IX86_BUILTIN_BNDINT,
IX86_BUILTIN_SIZEOF, IX86_BUILTIN_BNDLOWER,
IX86_BUILTIN_BNDUPPER.
(builtin_isa): Add leaf_p and nothrow_p fields.
(def_builtin): Initialize leaf_p and nothrow_p.
(ix86_add_new_builtins): Handle leaf_p and nothrow_p
flags.
(bdesc_mpx): New.
(bdesc_mpx_const): New.
(ix86_init_mpx_builtins): New.
(ix86_init_builtins): Call ix86_init_mpx_builtins.
(ix86_emit_move_max): New.
(ix86_expand_builtin): Expand IX86_BUILTIN_BNDMK,
IX86_BUILTIN_BNDSTX, IX86_BUILTIN_BNDLDX,
IX86_BUILTIN_BNDCL, IX86_BUILTIN_BNDCU,
IX86_BUILTIN_BNDRET, IX86_BUILTIN_BNDNARROW,
IX86_BUILTIN_BNDINT, IX86_BUILTIN_SIZEOF,
IX86_BUILTIN_BNDLOWER, IX86_BUILTIN_BNDUPPER.
* config/i386/i386.h (ix86_stack_slot): Added SLOT_BND_STORED.
diff --git a/gcc/config/i386/i386-builtin-types.def
b/gcc/config/i386/i386-builtin-types.def
index 35c0035..989297a 100644
--- a/gcc/config/i386/i386-builtin-types.def
+++ b/gcc/config/i386/i386-builtin-types.def
@@ -47,6 +47,7 @@ DEF_PRIMITIVE_TYPE (UCHAR, unsigned_char_type_node)
DEF_PRIMITIVE_TYPE (QI, char_type_node)
DEF_PRIMITIVE_TYPE (HI, intHI_type_node)
DEF_PRIMITIVE_TYPE (SI, intSI_type_node)
+DEF_PRIMITIVE_TYPE (BND, pointer_bounds_type_node)
# ??? Logically this should be intDI_type_node, but that maps to "long"
# with 64-bit, and that's not how the emmintrin.h is written. Again,
# changing this would change name mangling.
@@ -60,6 +61,7 @@ DEF_PRIMITIVE_TYPE (USHORT, short_unsigned_type_node)
DEF_PRIMITIVE_TYPE (INT, integ
