Re: GCC libatomic ABI specification draft
On 17/11/16 20:12, Bin Fan wrote: > > Although this ABI specification specifies that 16-byte properly aligned > atomics are inlineable on platforms > supporting cmpxchg16b, we document the caveats here for further discussion. > If we decide to change the > inlineable attribute for those atomics, then this ABI, the compiler and the > runtime implementation should be > updated together at the same time. > > > The compiler and runtime need to check the availability of cmpxchg16b to > implement this ABI specification. > Here is how it would work: The compiler can get the information either from > the compiler flags or by > inquiring the hardware capabilities. When the information is not available, > the compiler should assume that > cmpxchg16b instruction is not supported. The runtime library implementation > can also query the hardware > compatibility and choose the implementation at runtime. Assuming the user > provides correct compiler options with this abi the runtime implementation *must* query the hardware (because there might be inlined cmpxchg16b in use in another module on a hardware that supports it and the runtime must be able to sync with it). currently gcc libatomic does not guarantee this which is dangerously broken: if gcc is configured with --disable-gnu-indirect-function (or on targets without ifunc support: solaris, bsd, android, musl,..) the compiler may inline cmpxchg16b in one translation unit but use incompatible runtime function in another. there is PR 70191 but this issue has wider scope. > and the inquiry returns the correct information, on a platform that supports > cmpxchg16b, the code generated > by the compiler will both use cmpxchg16b; on a platform that does not support > cmpxchg16b, the code generated > by the compiler, including the code generated for a generic platform, always > call the support function, so > there is no compatibility problem.
-Wformat-length and floats
This took me a bit to get to the bottom of, but I know believe
that we need to work both on the documentation and implementation
of -Wformat-length, in particular when it comes to floats.
#include
typedef struct M {
float a, b, c;
} M;
char *foo(M *m) {
static char buf[64];
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
return buf;
}
First of all, it turns out that floats are not covered in the
documentation at all. I've had a look at the code, and think
I'll be able to propose a doc change latest this coming weekend.
Now to what actually happens in the example above:
# gcc -c -o x.o -Wall x.c
x.c: In function ‘foo’:
x.c:9:24: warning: ‘%.2f’ directive writing between 4 and 317 bytes into a
region of size 0 [-Wformat-length=]
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
x.c:9:5: note: format output between 15 and 954 bytes into a destination of
size 64
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
A couple of issues I find really confusing:
- Where is the number of 317 bytes as the upper limit for a sole
%.2f coming from?
- Where is the number of 954 bytes coming from for the full call?
- What is a region of size 0? Why 0?
- And what is the difference between a region and a destination?
I'll see what I can do about documentation; any input on the above
related to that will be helpful.
And something tells me that there may be in issue with the diagnostics
code?
Gerald
Re: -Wformat-length and floats
On 11/29/2016 08:38 AM, Gerald Pfeifer wrote:
This took me a bit to get to the bottom of, but I know believe
that we need to work both on the documentation and implementation
of -Wformat-length, in particular when it comes to floats.
#include
typedef struct M {
float a, b, c;
} M;
char *foo(M *m) {
static char buf[64];
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
return buf;
}
First of all, it turns out that floats are not covered in the
documentation at all. I've had a look at the code, and think
I'll be able to propose a doc change latest this coming weekend.
Thanks for looking at this and bringing it up for discussion!
Suggestions for improvements are very welcome and appreciated.
Now to what actually happens in the example above:
# gcc -c -o x.o -Wall x.c
x.c: In function ‘foo’:
x.c:9:24: warning: ‘%.2f’ directive writing between 4 and 317 bytes into a
region of size 0 [-Wformat-length=]
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
x.c:9:5: note: format output between 15 and 954 bytes into a destination of
size 64
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
A couple of issues I find really confusing:
- Where is the number of 317 bytes as the upper limit for a sole
%.2f coming from?
317 the number of bytes that printf("%f", -DBL_MAX) results in.
There's a bug in that the computation doesn't take precision into
account for %e or %g with unknown arguments so the maximum for
%.2f should actually be 312. Let me fix that.
- Where is the number of 954 bytes coming from for the full call?
It's the result of 3 * 317 plus the spaces (again, with the precision
bug).
- What is a region of size 0? Why 0?
Each directive writes into a region of the destination. The start
of the region moves toward the end of the destination as bytes are
"added" until the remaining space is too small for the rest of the
format string or a directive, or until the format string has been
exhausted. An example that might make it clearer is
char buf[4];
sprintf(buf, "i=%i", 12345);
We get:
warning: ‘%i’ directive writing 5 bytes into a region of size 2
note: format output 8 bytes into a destination of size 4
- And what is the difference between a region and a destination?
Destination is the whole destination object (or buffer) of the call.
The region is the are of the destination the output of each directive
is being written into (as per the above).
I'll see what I can do about documentation; any input on the above
related to that will be helpful.
And something tells me that there may be in issue with the diagnostics
code?
It could be, though at the moment I'm not aware of any. I am
grateful for testing people do on their own and for this kind
of feedback. There's only so much that compiling software can
reveal.
FWIW, some time ago in bug 77696 David brought up some related
issues and questions. I made an effort to describe the logic
behind the wording choices made in the text of the diagnostics
and the common theme between their various forms. It might be
helpful (to all) for you to review the bug and provide additional
input on the wording of the diagnostics there.
Certainly if you find what looks like a bug or where you are not
sure please open an issue for it in Bugzilla or raise it here.
Thanks
Martin
Re: -Wformat-length and floats
I'll see what I can do about documentation; any input on the above related to that will be helpful. This might be a good time to mention that I've thinking about writing up more detailed documentation than what may be suitable for the GCC manual and posting it either on the Wiki or someplace where users can easily find it. I should be able to get to it once all my patches have been approved and committed and I'm done fixing bugs in them. If you (or anyone else) have suggestions I'd be grateful for them. Martin
[libgomp] No references to env.c -> no libgomp construction
Hello,
after a recent change:
commit 44a69dfd2c96110643d05176803c984a080b696b
Author: amonakov
Date: Wed Nov 23 18:36:41 2016 +
OpenMP offloading to NVPTX: libgomp changes
* Makefile.am (libgomp_la_SOURCES): Add atomic.c, icv.c, icv-device.c.
* Makefile.in. Regenerate.
* configure.ac [nvptx*-*-*] (libgomp_use_pthreads): Set and use it...
(LIBGOMP_USE_PTHREADS): ...here; new define.
* configure: Regenerate.
* config.h.in: Likewise.
* config/posix/affinity.c: Move to...
* affinity.c: ...here (new file). Guard use of Pthreads-specific
interface by LIBGOMP_USE_PTHREADS.
* critical.c: Split out GOMP_atomic_{start,end} into...
* atomic.c: ...here (new file).
* env.c: Split out ICV definitions into...
* icv.c: ...here (new file) and...
* icv-device.c: ...here. New file.
the env.c contains now only local symbols (at least for target *-rtems*-*):
nm --defined-only ./arm-rtems4.12/libgomp/env.o
08d8 t initialize_env
d kinds.9043
t parse_boolean
0880 t parse_int.constprop.4
032c t parse_one_place
01a0 t parse_stacksize
00c4 t parse_unsigned_long
Thus the libgomp constructor is not linked in into executables.
--
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.huber at embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [libgomp] No references to env.c -> no libgomp construction
Hello, On Tue, 29 Nov 2016, Sebastian Huber wrote: > * env.c: Split out ICV definitions into... > * icv.c: ...here (new file) and... > * icv-device.c: ...here. New file. > > the env.c contains now only local symbols (at least for target *-rtems*-*): > [...] > > Thus the libgomp constructor is not linked in into executables. Thanks for the report. This issue affects only static libgomp.a (and not on NVPTX where env.c is deliberately empty). I think the minimal solution here is to #include from icv.c instead of compiling it separately (using <> inclusion rather than "" so in case of NVPTX we pick up the empty config/nvptx/env.c from toplevel icv.c). A slightly more involved but perhaps a preferable approach is to remove config/nvptx/env.c, introduce LIBGOMP_OFFLOADED_ONLY macro, and use it to guard inclusion of env.c from icv.c (which then can use the #include "env.c" form). Thanks. Alexander
gcc-5-20161129 is now available
Snapshot gcc-5-20161129 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/5-20161129/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 5 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-5-branch revision 242979 You'll find: gcc-5-20161129.tar.bz2 Complete GCC MD5=21f78419f3e843417b4592da9ba8c09e SHA1=2f8245971c7774e9580e72373e644ec82f78773a Diffs from 5-20161122 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-5 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: -Wformat-length and floats
On 11/29/2016 09:32 AM, Martin Sebor wrote:
On 11/29/2016 08:38 AM, Gerald Pfeifer wrote:
This took me a bit to get to the bottom of, but I know believe
that we need to work both on the documentation and implementation
of -Wformat-length, in particular when it comes to floats.
#include
typedef struct M {
float a, b, c;
} M;
char *foo(M *m) {
static char buf[64];
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
return buf;
}
First of all, it turns out that floats are not covered in the
documentation at all. I've had a look at the code, and think
I'll be able to propose a doc change latest this coming weekend.
Thanks for looking at this and bringing it up for discussion!
Suggestions for improvements are very welcome and appreciated.
Now to what actually happens in the example above:
# gcc -c -o x.o -Wall x.c
x.c: In function ‘foo’:
x.c:9:24: warning: ‘%.2f’ directive writing between 4 and 317 bytes
into a region of size 0 [-Wformat-length=]
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
x.c:9:5: note: format output between 15 and 954 bytes into a
destination of size 64
sprintf(buf, "%.2f %.2f %.2f", m->a, m->b, m->c);
^~~~
After spending some more time on this I think I see the problem
you are pointing out: this is a false positive at level 1 of the
warning. I opened bug 78605 for it with a slightly modified test
case. Please chime in on it if I missed something.
Thanks
Martin
Breakage on trunk, a fix
I had to do this to get my build to work.
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 5d96e5fd..140ff807 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2196,7 +2196,7 @@ continues.
@itemx --with-target-bdw-gc-lib=@var{list}
Specify search directories for the garbage collector header files and
libraries. @var{list} is a comma separated list of key value pairs of the
-form @samp{@var{multilibdir}@=@var{path}}, where the default multilib key
+form @samp{@var{multilibdir}=@var{path}}, where the default multilib key
is named as @samp{.} (dot), or is omitted (e.g.
@samp{--with-target-bdw-gc=/opt/bdw-gc,32=/opt-bdw-gc32}).
The error was:
../../trunk/gcc/doc/install.texi:2199: use braces to give a command as an
argument to @=
make[2]: *** [doc/gccinstall.info] Error 1
make[1]: *** [all-gcc] Error 2
make: *** [all] Error 2
I do not know who is to fix.
Regards
Jerry
Re: Breakage on trunk, a fix
On 11/29/2016 08:38 PM, Jerry DeLisle wrote:
I had to do this to get my build to work.
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 5d96e5fd..140ff807 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2196,7 +2196,7 @@ continues.
@itemx --with-target-bdw-gc-lib=@var{list}
Specify search directories for the garbage collector header files and
libraries. @var{list} is a comma separated list of key value pairs of the
-form @samp{@var{multilibdir}@=@var{path}}, where the default multilib key
+form @samp{@var{multilibdir}=@var{path}}, where the default multilib key
is named as @samp{.} (dot), or is omitted (e.g.
@samp{--with-target-bdw-gc=/opt/bdw-gc,32=/opt-bdw-gc32}).
The error was:
../../trunk/gcc/doc/install.texi:2199: use braces to give a command as
an argument to @=
make[2]: *** [doc/gccinstall.info] Error 1
make[1]: *** [all-gcc] Error 2
make: *** [all] Error 2
I do not know who is to fix.
David M. already took care of the typo for us.
Jeff
