Re: [PATCH] Make basic asm implicitly clobber memory
On 5/23/2016 12:46 AM, Richard Biener wrote: > On Sun, 22 May 2016, Andrew Haley wrote: >> On 05/20/2016 07:50 AM, David Wohlferd wrote: >>> I realize deprecation/removal is drastic. Especially since basic >>> asm (mostly) works as is. But fixing memory clobbers while leaving >>> the rest broken feels like half a solution, meaning that some day >>> we're going to have to fiddle with this again. >> >> Yes, we will undoubtedly have to fiddle with basic asm again. We >> should plan for deprecation. > > I think adding memory clobbers is worth having. I also think that > deprecating basic asms would be a good thing, so can we please > add a new warning for that? "warning: basic asms are deprecated" I've still got the -Wbasic-asm patch where I proposed this for v6. I can dust it off again and re-submit it. A couple questions first: 1) In this patch the warning was disabled by default. But it sounds like you want it enabled by default? Easy to change, I'm just confirming your intent. 2) Is 'deprecated' handled differently than other types of warnings? There is a -Wno-deprecated, but it seems to have a very specific meaning that does not apply here. 3) The warning text in the old patch was "asm statement in function does not use extended syntax". The intent was: a) Don't make it sound like basic asm is completely gone, since it can still be used at top level. b) Don't make it sound like all inline asm is gone, since extended asm can still be used in functions. c) Convey all that in as few words as possible. Now that we want to add the word 'deprecated,' perhaps one of these: - Basic asm in functions is deprecated in favor of extended syntax - asm in functions without extended syntax is deprecated - Deprecated: basic asm in function - Deprecated: asm in function without extended syntax I like the last one (people may not know what 'basic' means in this context), but any of these would work for me. Preferences? In order to avoid conflicts, I'll wait for Bernd to commit his patch first. dw
Re: [PATCH] Make basic asm implicitly clobber memory
On 5/18/2016 3:07 PM, Jeff Law wrote: On 05/07/2016 11:38 AM, Andrew Haley wrote: My argument in support of Bernd's proposal is that it makes sense from a *practical* software reliability point of view. It wouldn't hurt, and might fix some significant bugs. It's similar to the targets which always implicitly clobber "cc". It corresponds to what I always assumed basic asm did, and I'm sure that I'm not alone. This change might fix some real bugs and it is extremely unlikely to break anything. And by making basic asms use/clobber memory in particular, it means code using them is less likely to break as the optimizers continue to get smarter about memory loads/stores. I haven't gone through the actual patch yet, but I like it's basic goals. Other than the doc changes I already mentioned, I have no technical issues with the patch (although I don't speak 'internals' well enough to approve it either). That said, I'd like to make one final pitch for the alternate solution proposed by Richard Henderson: "deprecate and later completely remove basic asm within functions" IOW: Basic asm can only be used at "top level." Within functions, extended asm must be used. The reason I asked for clarification on what problem this patch is intended to solve, is that basic asm has a number of them. Given it: - Doesn't clobber memory - Doesn't clobber used registers - Doesn't clobber flags - Doesn't have dependencies This makes gcc's basic asm: - incompatible with user expectations. - incompatible with other compilers. - difficult for optimizers to correctly and consistently position. Bernd's patch adding the memory clobber does help with all of these. And I agree that it is unlikely to cause the generation of incorrect code. However unlike Andrew, I'm not prepared to say it doesn't 'hurt.' At a minimum, suddenly forcing an unexpected/unneeded memory clobber can adversely impact the optimization of surrounding code. This can be particularly annoying if the reason for the asm was to improve performance. And adding a memory clobber does add a dependency of sorts, which might cause the location of the asm to shift in an unfortunate way. And there's always the long-shot possibility that some weird quirk or (very) badly-written code will cause the asm to flat out fail when used with a memory clobber. And if this change does produce any of these problems, I feel pity for whoever has to track it down. But the real problem I have with this patch is that it doesn't actually solve the problem of user expectations. Or compatibility with other compilers. Or the problem with positioning. In fact this change makes the problem with expectations worse for the people who are knowledgeable about the current decades-old behavior. Ultimately, the design of gcc makes these problems inherently unsolvable. That's why I believe deprecation/removal is the real solution here. I realize deprecation/removal is drastic. Especially since basic asm (mostly) works as is. But fixing memory clobbers while leaving the rest broken feels like half a solution, meaning that some day we're going to have to fiddle with this again. If we are going to spend the time and effort (both ours and our users') to address basic-asm-in-a-function's problems, let's make sure we really solve them. dw
Re: [PATCH] Make basic asm implicitly clobber memory
A few questions:
1) I'm not clear precisely what problem this patch fixes. It's true
that some people have incorrectly assumed that basic asm clobbers memory
and this change would fix their code. But some people also incorrectly
assume it clobbers registers. I assume that's why Jeff Law proposed
making basic asm "an opaque blob that read/write/clobber any register or
memory location." Do we have enough problem reports from users to know
which is the real solution here?
Whenever I do something for gcc I do it actually for myself, in my own
best interest. And this is no exception.
Seems fair. You are the one putting the time in to change it.
But do you have actual code that is affected by this? You can't really
be planning to wait until v7 is released to have your projects work
correctly?
The way I see it, is this: in simple cases a basic asm behaves as if
it would clobber memory, because of the way Jeff implemented the
asm handling in sched-deps.c some 20 years ago.
Look for ASM_INPUT where we have this comment:
"Traditional and volatile asm instructions must be considered to use
and clobber all hard registers, all pseudo-registers and all of
memory."
The assumption that it is OK to clobber memory in a basic asm will only
break if the asm statement is inlined in a loop, and that may happen
unexpectedly, when gcc rolls out new optimizations.
That's why I consider this to be security relevant.
I'm not sure I follow. Do you fear that gcc could mistakenly move the
asm into a nearby loop during optimization (resulting in who-knows-what
results)? Or is there some way that any basic asm in a loop could have
some kind of a problem?
But OTOH you see immediately that all general registers are in use
by gcc, unless you declare a variable like
register int eax __asm__("rax");
then it is perfectly OK to use rax in a basic asm of course.
According to the docs, that is only supported for global registers. The
docs for local register variables explicitly say that it can't be used
as input/outputs for basic asm.
And if we want to have implicitly clobbered registers, like the
diab compiler handles the basic asm, then this patch will
make it possible to add a target hook that clobbers additional
registers for basic asm.
I think we should try to avoid changing the semantics in v7 for memory
and then changing them again in v8 for registers.
IOW, if I see some basic asm in a project (or on stack overflow/blog as
a code fragment), should I assume it was intended for v6 semantics? v7?
v8? People often copy this stuff without understanding what it does.
The more often the semantics change, the harder it is to use correctly
and maintain.
2) The -Wbasic-asm warning patch wasn't approved for v6. If we are
going to change this behavior now, is it time?
Yes. We have stage1 for gcc-7 development, I can't think of a better
time for it.
I would even say, the -Wbasic-asm warning patch makes more sense now,
because we could warn, that the basich asm clobbers memory, which it
did not previously.
After your patch has been checked in, I'll re-submit this.
4) There are more basic asm docs that need to change: "It also does not
know about side effects of the assembler code, such as modifications to
memory or registers. Unlike some compilers, GCC assumes that no changes
to either memory or registers occur. This assumption may change in a
future release."
Yes, I should change that sentence too.
Maybe this way:
"Unlike some compilers, GCC assumes that no changes to registers
occur. This assumption may change in a future release."
Is it worth describing the fact that the semantics have changed here?
Something like "Before v7, gcc assumed no changes were made to memory."
I guess users can "figure it out" by reading the v6 docs and comparing
it to v7. But if the semantic change has introduced a problem that
someone is trying to debug, this could help them track it down.
Also, I'm kind of hoping that v7 is the 'future release.' If we are
going to change the clobbers, I'd hope that we'd do it all at one time,
rather than spreading it out across several releases.
If no one is prepared to step up and implement (or at least defend) the
idea of clobbering registers, I'd like to see the "This assumption may
change in a future release" part removed. Since (theoretically)
anything can change at any time, the only reason this text made sense
was because a change was imminent. If that's no longer true, it's time
for it to go.
dw
Re: [PATCH] Make basic asm implicitly clobber memory
On 5/5/2016 10:29 AM, Bernd Edlinger wrote: Hi! this patch is inspired by recent discussion about basic asm: Currently a basic asm is an instruction scheduling barrier, but not a memory barrier, and most surprising, basic asm does _not_ implicitly clobber CC on targets where extended asm always implicitly clobbers CC, even if nothing is in the clobber section. This patch makes basic asm implicitly clobber CC on certain targets, and makes the basic asm implicitly clobber memory, but no general registers, which is what could be expected. This is however only done for basic asm with non-empty assembler string, which is in sync with David's proposed basic asm warnings patch. Due to the change in the tree representation, where ASM_INPUT can now be the first element of a PARALLEL block with the implicit clobber elements, there are some changes necessary. Most of the changes in the middle end, were necessary because extract_asm_operands can not be used to find out if a PARALLEL block is an asm statement, but in most cases asm_noperands can be used instead. There are also changes necessary in two targets: pa, and ia64. I have successfully built cross-compilers for these targets. Boot-strapped and reg-tested on x86_64-pc-linux-gnu OK for trunk? A few questions: 1) I'm not clear precisely what problem this patch fixes. It's true that some people have incorrectly assumed that basic asm clobbers memory and this change would fix their code. But some people also incorrectly assume it clobbers registers. I assume that's why Jeff Law proposed making basic asm "an opaque blob that read/write/clobber any register or memory location." Do we have enough problem reports from users to know which is the real solution here? 2) The -Wbasic-asm warning patch wasn't approved for v6. If we are going to change this behavior now, is it time? 3) I assume there are good reasons why extended asm can't be used at top level. Will adding these clobbers cause those problems in basic asm too? 4) There are more basic asm docs that need to change: "It also does not know about side effects of the assembler code, such as modifications to memory or registers. Unlike some compilers, GCC assumes that no changes to either memory or registers occur. This assumption may change in a future release." dw
Re: [DOC Patch] Add sample for @cc constraint
On 4/25/2016 2:51 AM, Bernd Schmidt wrote:
On 04/16/2016 01:12 AM, David Wohlferd wrote:
There were basically 3 changes I was trying for in that doc patch. Are
any of them worth keeping? Or are we done?
1) "Do not clobber flags if they are being used as outputs."
2) Output flags sample (with #if removed).
3) "On the x86 platform, flags are always treated as clobbered by
extended asm whether @code{"cc"} is specified or not."
I'm prepared to send an updated patch if there's anything here that
might get approved.
I think the updated flags sample would be nice to have.
Attached.
dw
Index: extend.texi
===
--- extend.texi (revision 235054)
+++ extend.texi (working copy)
@@ -8135,6 +8135,26 @@
``not'' @var{flag}, or inverted versions of those above
@end table
+This example uses the @code{bt} instruction (which sets the carry flag) to
+see if bit 0 of an integer is set. To see the improvement in the generated
+output, make sure optimizations are enabled.
+
+@example
+void TestEven (int value)
+@{
+ char CarryIsSet;
+
+ asm ("bt $0, %[value]"
+: "=@@ccc" (CarryIsSet)
+: [value] "rm" (value));
+
+ if (CarryIsSet)
+printf ("odd\n");
+ else
+printf ("even\n");
+@}
+@end example
+
@end table
@anchor{InputOperands}
Re: [DOC Patch] Add sample for @cc constraint
I've had it in a successful test run, and committed it with a minor
tweak (__builtin_abort vs return 1).
It didn't find anything, but it's probably good to have.
As for the docs, I think you are unnecessarily worried about things
that are never going to be a problem in practice.
Perhaps so. On the other hand, the reason x86 always clobbers cc now is
that there was a concern during a change that people may have been using
it incorrectly. Giving people yet another way to muck this up seemed
like a bad plan.
Anyway.
There were basically 3 changes I was trying for in that doc patch. Are
any of them worth keeping? Or are we done?
1) "Do not clobber flags if they are being used as outputs."
2) Output flags sample (with #if removed).
3) "On the x86 platform, flags are always treated as clobbered by
extended asm whether @code{"cc"} is specified or not."
I'm prepared to send an updated patch if there's anything here that
might get approved.
dw
Re: [DOC Patch] Add sample for @cc constraint
Ping? dw On 4/1/2016 4:39 PM, David Wohlferd wrote: > I would like executable code that verifies that this feature is indeed working as intended. First draft is attached. It tests all 28 (14 conditions plus 14 inverted). I wasn't sure what to set for optimization (O2? O3? O0?), so I left the default. It looks like even at O3 there are some missed optimizations here, but that's an issue for another day. > Is there any _actual_ problem here? Like, if you combine the output and the clobber you run into problems? Looks to me like an explicit "cc" clobber is just ignored on x86. We just need to make sure this stays working (testcases). Today? No. You can clobber or not clobber and both will produce the exact same output. But letting people program this two different ways guarantees that people *will* program it both ways. And just because there isn't any definable reason to limit this today doesn't mean that there won't ever be. But by then it will be 'too late' to change it because it "breaks existing code." >> 1) Leave this text in. >> 2) Remove the text and add the compiler check to v6. >> 3) Remove the text and add the compiler check to v7. >> 4) Leave the text in v6, then in v7: remove the text and add the compiler check. >> 5) (Reluctantly) remove the text and hope this never becomes a problem. So, I've made my pitch, but it sounds like you want #5? > My question would be, can this information ever be relevant to users? They may notice that their code still works if they omit the "cc", but that's not really a habit we want to encourage. People do this now without understanding how or why it works. > I think this is an internal implementation detail that doesn't necessarily even have to be documented. One time it would matter is if people want to move from basic asm (which doesn't clobber "cc") to any type of extended asm (which always does). It /probably/ won't matter in that case (and may even make things better). But it shouldn't be a secret. dw
Re: [DOC Patch] Add sample for @cc constraint
> I would like executable code that verifies that this feature is
indeed working as intended.
First draft is attached. It tests all 28 (14 conditions plus 14
inverted). I wasn't sure what to set for optimization (O2? O3? O0?), so
I left the default.
It looks like even at O3 there are some missed optimizations here, but
that's an issue for another day.
> Is there any _actual_ problem here? Like, if you combine the output
and the clobber you run into problems? Looks to me like an explicit "cc"
clobber is just ignored on x86. We just need to make sure this stays
working (testcases).
Today? No. You can clobber or not clobber and both will produce the
exact same output.
But letting people program this two different ways guarantees that
people *will* program it both ways. And just because there isn't any
definable reason to limit this today doesn't mean that there won't ever
be. But by then it will be 'too late' to change it because it "breaks
existing code."
>> 1) Leave this text in.
>> 2) Remove the text and add the compiler check to v6.
>> 3) Remove the text and add the compiler check to v7.
>> 4) Leave the text in v6, then in v7: remove the text and add the
compiler check.
>> 5) (Reluctantly) remove the text and hope this never becomes a problem.
So, I've made my pitch, but it sounds like you want #5?
> My question would be, can this information ever be relevant to users?
They may notice that their code still works if they omit the "cc", but
that's not really a habit we want to encourage.
People do this now without understanding how or why it works.
> I think this is an internal implementation detail that doesn't
necessarily even have to be documented.
One time it would matter is if people want to move from basic asm (which
doesn't clobber "cc") to any type of extended asm (which always does).
It /probably/ won't matter in that case (and may even make things
better). But it shouldn't be a secret.
dw
Index: gcc/testsuite/gcc.target/i386/asm-flag-6.c
===
--- gcc/testsuite/gcc.target/i386/asm-flag-6.c (revision 0)
+++ gcc/testsuite/gcc.target/i386/asm-flag-6.c (working copy)
@@ -0,0 +1,276 @@
+/* Executable testcase for 'output flags.' */
+/* { dg-do run } */
+
+char TestC ()
+{
+ char r;
+
+ __asm__ ("stc" : "=@ccc"(r));
+ if (r)
+ {
+__asm__ ("clc" : "=@ccnc"(r));
+if (r)
+ return 1;
+ }
+ return 0;
+}
+
+char TestE ()
+{
+ char r;
+
+ /* 1 equals 1. */
+ __asm__ ("cmp $1, %1" : "=@cce"(r) : "r" (1));
+ if (r)
+ {
+/* 1 not equals 2. */
+__asm__ ("cmp $2, %1" : "=@ccne"(r) : "r" (1));
+if (r)
+ return 1;
+ }
+ return 0;
+}
+
+char TestZ ()
+{
+ char r;
+
+ /* 1 equals 1. */
+ __asm__ ("cmp $1, %1" : "=@ccz"(r) : "r" (1));
+ if (r)
+ {
+/* 1 not equals 2. */
+__asm__ ("cmp $2, %1" : "=@ccnz"(r) : "r" (1));
+if (r)
+ return 1;
+ }
+ return 0;
+}
+
+char TestA ()
+{
+ char r;
+
+ /* 1 a 0. */
+ __asm__ ("cmp $0, %1" : "=@cca"(r) : "r" (1));
+ if (r)
+ {
+/* 1 na 2. */
+__asm__ ("cmp $2, %1" : "=@ccna"(r) : "r" (1));
+if (r)
+{
+ /* 1 na 1. */
+ __asm__ ("cmp $1, %1" : "=@ccna"(r) : "r" (1));
+ if (r)
+ return 1;
+}
+ }
+ return 0;
+}
+
+char TestAE ()
+{
+ char r;
+
+ /* 1 ae 0. */
+ __asm__ ("cmp $0, %1" : "=@ccae"(r) : "r" (1));
+ if (r)
+ {
+/* 1 nae 2. */
+__asm__ ("cmp $2, %1" : "=@ccnae"(r) : "r" (1));
+if (r)
+{
+ /* 1 ae 1. */
+ __asm__ ("cmp $1, %1" : "=@ccae"(r) : "r" (1));
+ if (r)
+ return 1;
+}
+ }
+ return 0;
+}
+
+char TestB ()
+{
+ char r;
+
+ /* 1 b 2. */
+ __asm__ ("cmp $2, %1" : "=@ccb"(r) : "r" (1));
+ if (r)
+ {
+/* 1 nb 0. */
+__asm__ ("cmp $0, %1" : "=@ccnb"(r) : "r" (1));
+if (r)
+{
+ /* 1 nb 1. */
+ __asm__ ("cmp $1, %1" : "=@ccnb"(r) : "r" (1));
+ if (r)
+ return 1;
+}
+ }
+ return 0;
+}
+
+char TestBE ()
+{
+ char r;
+
+ /* 1 be 2. */
+ __asm__ ("cmp $2, %1" : "=@ccbe"(r) : "r" (1));
+ if (r)
+ {
+/* 1 nbe 0. */
+__asm__ ("cmp $0, %1" : "=@ccnbe"(r) : "r" (1));
+if (r)
+{
+ /* 1 be 1. */
+ __asm__ ("cmp $1, %1" : "=@ccbe"(r) : "r" (1));
+ if (r)
+ return 1;
+}
+ }
+ return 0;
+}
+
+char TestG ()
+{
+ char r;
+
+ /* 1 g 0. */
+ __asm__ ("cmp $0, %1" : "=@ccg"(r) : "r" (1));
+ if (r)
+ {
+/* 1 ng 2. */
+__asm__ ("cmp $2, %1" : "=@ccng"(r) : "r" (1));
+if (r)
+{
+ /* 1 ng 1. */
+ __asm__ ("cmp $1, %1" : "=@ccng"(r) : "r" (1));
+ if (r)
+ return 1;
+}
+ }
+ return 0;
+}
+
+char TestGE ()
+{
+ char r;
+
+ /* 1 ge 0. */
+ __asm__ ("cmp $0, %1" : "=@ccge"(r) : "r" (1));
+ if (r)
+ {
+/* 1 nge 2. */
+__asm__ ("cmp $2, %1" : "=@ccnge"(r) : "r" (1));
+if (r)
+{
+ /* 1 ge 1. */
+ __asm__ ("cmp $1, %1" : "=@ccge"(r) : "r" (1));
+ if (r)
+ return 1;
+}
Re: [DOC Patch] Add sample for @cc constraint
Thanks for the feedback. While I agree with some of this, there are
parts that I want to defend. If after explaining why I did what I did
you still feel it should be changed, I'm prepared to do that.
On 3/24/2016 8:00 AM, Bernd Schmidt wrote:
> More problematic than a lack of documentation is that I haven't been
able to find an executable testcase. If you could adapt your example for
use in gcc.target/i386, that would be even more important.
It looks like Richard included some "scan-assembler" statements in the
suites with the original checkin
(https://gcc.gnu.org/viewcvs/gcc?view=revision=225122). Is that
not sufficient? If not, I'm certainly prepared to create a couple
executable cases for the next rev of this patch.
>> +Do not clobber flags if they are being used as outputs.
>
> I don't think the manual should point out the obvious. I'd be
surprised if this wasn't documented or at least strongly implied
elsewhere for normal operands.
Well, *I* thought it was obvious, because it is both documented and
implied elsewhere.
However, the compiler doesn't see it that way. Normally, attempting to
overlap 'clobbers' and 'outputs' generates compile errors, but not when
outputting and clobbering flags. I filed pr68095 about this (including
a rough draft at a patch), but apparently not everyone sees this the way
I do.
Outputting flags is new to v6, so changing the compiler to reject
overlaps before the feature is ever released would be ideal. If we try
to patch this in v7, will it get rejected because it would break
backward compatibility?
If we aren't going to change the code, then I decided it needed to be
hammered home in the docs. Because someday someone is going to want to
do something more with flags, but they won't be able to because it will
break backward compatibility with all the people who have written this
the "obviously wrong" way. Hopefully this text will serve as
justification for that future someone to do it anyway.
That said, I'm ok with any of:
1) Leave this text in.
2) Remove the text and add the compiler check to v6.
3) Remove the text and add the compiler check to v7.
4) Leave the text in v6, then in v7: remove the text and add the
compiler check.
5) (Reluctantly) remove the text and hope this never becomes a problem.
I'll update the patch with whichever option seems best. If it were my
choice to make, I'd go with #4 (followed by 3, 1, 5). 2 would actually
be the best, but probably isn't realistic at this late date.
>> +For builds that don't support flag output operands,
>
> This feels strange, we should just be documenting the capabilities of
this feature. Other parts of the docs already show what to do without it.
While I liked using the #define to contrast how this used to work (not
sure where you think we show this?) with how the feature makes things
better, I think I prefer the shorter example you are proposing. I'll
change this in the next rev of the patch.
>> +Note: On the x86 platform, flags are normally considered clobbered by
>> +extended asm whether the @code{"cc"} clobber is specified or not.
>
> Is it really necessary or helpful to mention that here? Not only is
it not strictly correct (an output operand is not also considered
clobbered), but to me it breaks the flow because you're left wondering
how that sentence relates to the example (it doesn't).
The problem I am trying to fix here is that on x86, the "cc" is implicit
for all extended asm statements, whether it is specified or not and
whether there is a flags output or not. However, that fact isn't
documented anywhere. So, where does that info go? It could go right by
the docs for "cc", but since this behavior only applies to x86, that
would make the docs there messy.
Since the 'output flags' section already has an x86-specific section,
that seemed like a plausible place to put it. But no matter where I put
it in that section, it always looks weird for exactly the reasons you state.
I'll try moving it up by the "cc" clobber in the next rev. Let me know
what you think.
>> +For platform-specific uses of flags, see also
>> +@ref{FlagOutputOperands,Flag Output Operands}.
>
> Is this likely to be helpful? Someone who's looking at how to use
flag outputs probably isn't looking in the "Clobbers" section?
People reading about "cc" may be interested in knowing that you can do
something with flags other than clobbering them. And of course this
lets us put the note about "x86 always clobbers flags" in that other
section.
dw
Re: [DOC Patch] Add sample for @cc constraint
Ping? (link to original post:
https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00743.html )
This patch adds a sample for a new-to-v6 feature. Is this not the right
time for doc improvements?
I considered adding some assembler output. Something like:
Before this feature, you had to write code like this:
asm("bt $0, %1 ; setc %0" : "=q" (a) : "r" (value) : "cc");
if (a)
This would generate code like this:
bt $0, %ebx
setc %al <- Convert flags to byte
testb %al, %al <-- Convert byte back to flags
jne .L5
Using @cc, this code
asm("bt $0, %1" : "=@ccc" (a) : "r" (value) );
if (a)
produces this output:
bt $0, %ebx
jc .L5 <- Use the flags directly
While this helps show the benefit of the feature, it just seemed like
too much detail. Showing people the c code and reminding them to enable
optimizations (what the current patch does) seems like it should be
sufficient.
dw
On 3/12/2016 8:00 PM, David Wohlferd wrote:
The docs for the new(-ish) @cc constraint need an example. Attached.
ChangeLog:
2016-03-12 David Wohlferd <d...@limegreensocks.com>
* doc/extend.texi: Add sample for @cc constraint
Note that while I have a release on file with FSF, I don't have write
access to SVN.
dw
Re: Wonly-top-basic-asm
On 3/14/2016 8:28 AM, Bernd Schmidt wrote:
The example is not good, as discussed previously, and IMO the best
option is to remove it. Otherwise I have no objections to the latest
variant.
Despite the problems I have with the existing sample, adding the
information/warnings is more important to me, and more valuable to
users. Perhaps we can revisit the sample when pr24414 gets addressed.
I have removed my changes to the sample in the attached patch.
ChangeLog:
2016-03-16 David Wohlferd <d...@limegreensocks.com>
Bernd Schmidt <bschm...@redhat.com>
* doc/extend.texi: Doc basic asm behavior re clobbers.
dw
Index: extend.texi
===
--- extend.texi (revision 234245)
+++ extend.texi (working copy)
@@ -7441,7 +7441,8 @@
@end table
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
efficient code, and in most cases it is a better solution than basic
@code{asm}. However, there are two situations where only basic @code{asm}
can be used:
@@ -7481,10 +7482,25 @@
symbol errors during compilation if your assembly code defines symbols or
labels.
-Since GCC does not parse the @var{AssemblerInstructions}, it has no
-visibility of any symbols it references. This may result in GCC discarding
-those symbols as unreferenced.
+@strong{Warning:} The C standards do not specify semantics for @code{asm},
+making it a potential source of incompatibilities between compilers. These
+incompatibilities may not produce compiler warnings/errors.
+GCC does not parse basic @code{asm}'s @var{AssemblerInstructions}, which
+means there is no way to communicate to the compiler what is happening
+inside them. GCC has no visibility of symbols in the @code{asm} and may
+discard them as unreferenced. It also does not know about side effects of
+the assembler code, such as modifications to memory or registers. Unlike
+some compilers, GCC assumes that no changes to either memory or registers
+occur. This assumption may change in a future release.
+
+To avoid complications from future changes to the semantics and the
+compatibility issues between compilers, consider replacing basic @code{asm}
+with extended @code{asm}. See
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert
+from basic asm to extended asm} for information about how to perform this
+conversion.
+
The compiler copies the assembler instructions in a basic @code{asm}
verbatim to the assembly language output file, without
processing dialects or any of the @samp{%} operators that are available with
[DOC Patch] Add sample for @cc constraint
The docs for the new(-ish) @cc constraint need an example. Attached.
ChangeLog:
2016-03-12 David Wohlferd <d...@limegreensocks.com>
* doc/extend.texi: Add sample for @cc constraint
Note that while I have a release on file with FSF, I don't have write
access to SVN.
dw
Index: extend.texi
===
--- extend.texi (revision 234163)
+++ extend.texi (working copy)
@@ -8047,6 +8047,7 @@
Because of the special nature of the flag output operands, the constraint
may not include alternatives.
+Do not clobber flags if they are being used as outputs.
Most often, the target has only one flags register, and thus is an implied
operand of many instructions. In this case, the operand should not be
@@ -8105,6 +8106,43 @@
``not'' @var{flag}, or inverted versions of those above
@end table
+For builds that don't support flag output operands, this example generates
+code to convert the flags to a byte (@code{setc}), then converts the
+byte back to flags (@code{if (a)} would generate a @code{testb}):
+
+@example
+char a;
+
+#ifndef __GCC_ASM_FLAG_OUTPUTS__
+
+/* If outputting flags is not supported. */
+asm ("bt $0, %1\n\t"
+ "setc %0"
+ : "=q" (a)
+ : "r" (b)
+ : "cc");
+
+#else
+
+/* Avoid the redundant setc/testb and use the carry flag directly. */
+asm ("bt $0, %1"
+ : "=@@ccc" (a)
+ : "r" (b));
+
+#endif
+
+if (a)
+ printf ("odd\n");
+else
+ printf ("even\n");
+@end example
+
+To see the improvement in the generated output, make sure optimizations
+are enabled.
+
+Note: On the x86 platform, flags are normally considered clobbered by
+extended asm whether the @code{"cc"} clobber is specified or not.
+
@end table
@anchor{InputOperands}
@@ -8260,6 +8298,8 @@
On other machines, condition code handling is different,
and specifying @code{"cc"} has no effect. But
it is valid no matter what the target.
+For platform-specific uses of flags, see also
+@ref{FlagOutputOperands,Flag Output Operands}.
@item "memory"
The @code{"memory"} clobber tells the compiler that the assembly code
Re: Wonly-top-basic-asm
So, we have been discussing this issue for 4 months now. Over that time, I have tried to incorporate everyone's feedback. As a result we have gone from a tiny doc patch (just describe the current semantics), to a big doc patch (completely deprecate basic asm when used in a function) to a medium doc patch + code fix (warning when using basic asm in a function) and now back to a slightly-bigger-than-tiny doc patch. I have made no changes since the last patch I posted (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01406.html) for the reasons discussed below. I assert that this patch both contains important information users need and is better than the current text. I expect that Sandra is prepared to check this in as soon as someone signs off on its technical accuracy. dw On 2/28/2016 11:02 PM, David Wohlferd wrote: On 2/26/2016 7:09 AM, Bernd Schmidt wrote: On 02/21/2016 11:27 AM, David Wohlferd wrote: So now what? I have one Bernd who likes the sample, and one who doesn't. Obviously I think what I'm proposing is better than what's there now and I've done my best to say why. But me believing it to be better doesn't get anything checked in. I hadn't thought it through well enough. Jan's objection (order isn't guaranteed) is relevant. I'd drop the example. To be clear: Are you suggesting that we delete the sample that is there and have no example at all for basic asm? I'm not sure I agree. Looking at the linux kernel source, there are times and places where basic asm is appropriate, even necessary. I realize that macros are an uncommon usage. But it makes for a more interesting sample than simply outputting a section name. If ordering is your concern, would adding a reference to -fno-toplevel-reorder make you feel better about this? It seems unnecessary in this particular context, but mentioning this option on the basic asm page is certainly appropriate. dw
Re: Wonly-top-basic-asm
On 2/26/2016 7:09 AM, Bernd Schmidt wrote: On 02/21/2016 11:27 AM, David Wohlferd wrote: So now what? I have one Bernd who likes the sample, and one who doesn't. Obviously I think what I'm proposing is better than what's there now and I've done my best to say why. But me believing it to be better doesn't get anything checked in. I hadn't thought it through well enough. Jan's objection (order isn't guaranteed) is relevant. I'd drop the example. To be clear: Are you suggesting that we delete the sample that is there and have no example at all for basic asm? I'm not sure I agree. Looking at the linux kernel source, there are times and places where basic asm is appropriate, even necessary. I realize that macros are an uncommon usage. But it makes for a more interesting sample than simply outputting a section name. If ordering is your concern, would adding a reference to -fno-toplevel-reorder make you feel better about this? It seems unnecessary in this particular context, but mentioning this option on the basic asm page is certainly appropriate. dw
Re: Wonly-top-basic-asm
On 2/20/2016 4:08 AM, Bernd Edlinger wrote: Sorry, but I don't like this example at all. First the new example is essentially academic and useless, When used within a function, basic asm: - causes difficulties for optimizers - produces incompatibilities with other compilers - has semantics that are the opposite of what users expect - may soon produce incompatibilities with other versions of gcc Given all this, why would we want our sample to show using basic asm within a function (as you are suggesting) instead of working to discourage it? Contrawise, even people who are advocating for the deprecation/removal of basic asm (like me) acknowledge that it is needed for top level. That is why I use it for the sample. I suppose I could grab some actual top level code from the linux source (like ".symver __netf2_compat,__netf2@GCC_3.0"). But while it's real-world instead of "academic," it's also not as clear as I'd like for a sample. Obviously this testme code isn't going to be placed verbatim in someone's project. However, if someone wants to use asm macros (a plausible if uncommon case), this sample shows them how, using a simple, easy-to-understand (if not particularly useful) method. In contrast, the "int $3" you seem to favor doesn't really show anything (even how to do multiple lines). And worse, Jeff's plan is to change basic asm statements so they clobber ALL registers plus memory. Even a trivial example shows how this results in gcc generating completely different code around an "int $3." Which makes it rather problematical for debugging (can you say heisenbug?), which is the primary use for int 3. while the previous example could well be used in real world code, except we could note here that we also have a __builtin_trap () now. After reading the documentation for __builtin_trap, I still have absolutely no idea what it does. Worse, after NOT saying precisely what it does on any given platform, the docs make it clear that whatever it does isn't fixed and can change at any time. I'm a big believer in using builtins instead of inline asm. But based on these docs, I can't ever see using __builtin_trap myself. Second if you don't mention that "cc" is already implicitly clobbered by default, and if it is written here it is ignored on i386 targets, then everybody will hurry to modify their asm code when there is no real reason for that. The meaning of the "cc" clobber is defined under extended asm. The usage in this sample is consistent with that definition. In the (unlikely) event that "everybody" decides to change their code, they will all have (slightly) better documented code which is written in accordance with the docs. And which behaves precisely the same as what they have now. So now what? I have one Bernd who likes the sample, and one who doesn't. Obviously I think what I'm proposing is better than what's there now and I've done my best to say why. But me believing it to be better doesn't get anything checked in. What will? dw
Re: AW: Wonly-top-basic-asm
On 2/13/2016 8:00 PM, David Wohlferd wrote:
Fair enough. Committing what we can right now sounds like a good plan.
Attached is the doc patch, minus the proposed warning.
ChangeLog:
2016-02-19 David Wohlferd <d...@limegreensocks.com>
Bernd Schmidt <bschm...@redhat.com>
* doc/extend.texi: Doc basic asm behavior re clobbers.
dw
Index: extend.texi
===
--- extend.texi (revision 233367)
+++ extend.texi (working copy)
@@ -7458,7 +7458,8 @@
@end table
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
efficient code, and in most cases it is a better solution than basic
@code{asm}. However, there are two situations where only basic @code{asm}
can be used:
@@ -7498,10 +7499,25 @@
symbol errors during compilation if your assembly code defines symbols or
labels.
-Since GCC does not parse the @var{AssemblerInstructions}, it has no
-visibility of any symbols it references. This may result in GCC discarding
-those symbols as unreferenced.
+@strong{Warning:} The C standards do not specify semantics for @code{asm},
+making it a potential source of incompatibilities between compilers. These
+incompatibilities may not produce compiler warnings/errors.
+GCC does not parse basic @code{asm}'s @var{AssemblerInstructions}, which
+means there is no way to communicate to the compiler what is happening
+inside them. GCC has no visibility of symbols in the @code{asm} and may
+discard them as unreferenced. It also does not know about side effects of
+the assembler code, such as modifications to memory or registers. Unlike
+some compilers, GCC assumes that no changes to either memory or registers
+occur. This assumption may change in a future release.
+
+To avoid complications from future changes to the semantics and the
+compatibility issues between compilers, consider replacing basic @code{asm}
+with extended @code{asm}. See
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert
+from basic asm to extended asm} for information about how to perform this
+conversion.
+
The compiler copies the assembler instructions in a basic @code{asm}
verbatim to the assembly language output file, without
processing dialects or any of the @samp{%} operators that are available with
@@ -7516,11 +7532,25 @@
Basic @code{asm} provides no
mechanism to provide different assembler strings for different dialects.
-Here is an example of basic @code{asm} for i386:
+Here is an example of top-level basic @code{asm} for i386 that defines an
+asm macro. That macro is then invoked from within a function using
+extended @code{asm}:
@example
-/* Note that this code will not compile with -masm=intel */
-#define DebugBreak() asm("int $3")
+/* Define macro at file scope with basic asm. */
+/* Add macro parameter p to eax. */
+asm (".macro testme p\n\t"
+"addl $\\p, %eax\n\t"
+".endm");
+
+/* Use macro in function using extended asm. It needs
+ the "cc" clobber since the flags are changed and uses
+ the "a" constraint since it modifies eax. */
+int DoAdd (int value)
+@{
+ asm ("testme 5" : "+a" (value) : : "cc");
+ return value;
+@}
@end example
@node Extended Asm
Re: AW: Wonly-top-basic-asm
On 2/12/2016 4:51 AM, Bernd Schmidt wrote: On 02/12/2016 08:05 AM, David Wohlferd wrote: Actually, it was my intent that this apply to v6. It's not like there is a significant change here. We're documenting long-time behavior, and adding a (disabled) warning. The doc patch (minus mentioning the warning) could go in now, but for gcc-6 we're at a stage where we're only accepting regression fixes with very few exceptions. If you can convince a RM that this is important enough then it could still go in. Understood. While I think the patch is small enough to be safe, whether it's important enough for this stage is a different question. 2) There is a significant change to this behavior being proposed for v7. When this happens, having a way to locate affected statements with features from a stable release seems desirable. I'm actually not convinced that we'll want to change much in asm behaviour. Clobbering memory, maybe, but I can't see much beyond that - there's just no gain and some risk. So I'm a little more relaxed about the whole thing. And that's the question. If you are correct that we won't be changing this, then yeah, update the docs for v6, push the code change to v7. Done. But Jeff sounded quite serious when he said "the only rational behaviour for a traditional asm is that it has to be considered a use/clobber of memory and hard registers." If indeed that is the plan for v7, then having this warning available in v6 to allow people to prepare becomes important. Jeff Law: If you are listening, care to share your plans here? Let's let Sandra have the last word [about the docs]. Good plan. dw
Re: AW: Wonly-top-basic-asm
On 2/12/2016 5:03 PM, Sandra Loosemore wrote: On 02/12/2016 05:51 AM, Bernd Schmidt wrote: On 02/12/2016 08:05 AM, David Wohlferd wrote: Actually, it was my intent that this apply to v6. It's not like there is a significant change here. We're documenting long-time behavior, and adding a (disabled) warning. The doc patch (minus mentioning the warning) could go in now, but for gcc-6 we're at a stage where we're only accepting regression fixes with very few exceptions. If you can convince a RM that this is important enough then it could still go in. I looked at the last version of the patch I saw and this is my conclusion as well. If you would like me to commit just the doc change (minus the references to the new warning) now, please split the patch and I will do that. But, I cannot commit the change to add the new warning during Stage 4 without approval from a RM. Fair enough. Committing what we can right now sounds like a good plan. Bernd and I have both posted alternate text to what was in the last patch (see https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00861.html). He proposed (and I agreed) that having you make the call about which was better might be reasonable way to finalize this. If you want to pick one, I'll remove the Wbasic-asm and turn it into a doc-only patch. Or maybe you'd rather scrap them both and propose your own? I'm flexible here. There are important concepts that need to be conveyed. Doesn't much matter to me who writes them. dw
Re: AW: AW: Wonly-top-basic-asm
why not simply -Wbasic-asm ?
Since both you and Bernd favor this shorter name, I have changed it.
Indentation wrong here. The whole block must be indented by 2 spaces.
Fixed.
Comments should end with dot space space */
Fixed.
the DECL_ATTRIBUTES should be at the same column as the "naked".
Fixed.
Comments should end with dot space space */
Fixed.
the DECL_ATTRIBUTES should be at the same column as the "naked".
Fixed.
C++, isn't it always upper case?
Fixed.
ChangeLog lines begin with TAB.
Hmm. Thunderbird changed them to spaces. I've tried something
different this time. Hopefully fixed.
Please split the ChangeLog
use relative file names.
Fixed (I think).
Please add the function name where you changed in brackets.
Fixed. == ChangeLog:
2016-02-11 David Wohlferd <d...@limegreensocks.com> * doc/extend.texi: Doc
basic asm behavior and new -Wbasic-asm option. * doc/invoke.texi: Doc
new -Wbasic-asm option. ChangeLog: 2016-02-11 David Wohlferd
<d...@limegreensocks.com> * c.opt: Define -Wbasic-asm. ChangeLog:
2016-02-11 David Wohlferd <d...@limegreensocks.com> * c-parser.c
(c_parser_asm_statement): Implement -Wbasic-asm for C. ChangeLog:
2016-02-11 David Wohlferd <d...@limegreensocks.com> * parser.c
(cp_parser_asm_definition): Implement -Wbasic-asm for C++. ChangeLog:
2016-02-11 David Wohlferd <d...@limegreensocks.com> *
c-c++-common/Wbasic-asm.c: New tests for -Wbasic-asm. *
c-c++-common/Wbasic-asm-2.c: Ditto. New patch is attached. Note that
Bernd Schmidt and I are still discussing changes to the docs (see next
message). dw
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt (revision 233367)
+++ gcc/c-family/c.opt (working copy)
@@ -585,6 +585,10 @@
C++ ObjC++ Var(warn_namespaces) Warning
Warn on namespace definition.
+Wbasic-asm
+C ObjC ObjC++ C++ Var(warn_basic_asm) Warning
+Warn on unsafe uses of basic asm.
+
Wsized-deallocation
C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
Warn about missing sized deallocation functions.
Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c (revision 233367)
+++ gcc/c/c-parser.c (working copy)
@@ -5978,8 +5978,19 @@
labels = NULL_TREE;
if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
-goto done_asm;
+{
+ /* Warn on basic asm used inside of functions,
+ except when in naked functions. Also allow asm (""). */
+ if (warn_basic_asm && TREE_STRING_LENGTH (str) != 1)
+ if (lookup_attribute ("naked",
+ DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+ warning_at (asm_loc, OPT_Wbasic_asm,
+ "asm statement in function does not use extended syntax");
+ goto done_asm;
+}
+
/* Parse each colon-delimited section of operands. */
nsections = 3 + is_goto;
for (section = 0; section < nsections; ++section)
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 233367)
+++ gcc/cp/parser.c (working copy)
@@ -18041,6 +18041,8 @@
bool goto_p = false;
required_token missing = RT_NONE;
+ location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
+
/* Look for the `asm' keyword. */
cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
@@ -18199,6 +18201,17 @@
/* If the extended syntax was not used, mark the ASM_EXPR. */
if (!extended_p)
{
+ /* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm (""). */
+ if (warn_basic_asm
+ && TREE_STRING_LENGTH (string) != 1)
+ if (lookup_attribute ("naked",
+ DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+ warning_at (asm_loc, OPT_Wbasic_asm,
+ "asm statement in function does not use extended"
+ " syntax");
+
tree temp = asm_stmt;
if (TREE_CODE (temp) == CLEANUP_POINT_EXPR)
temp = TREE_OPERAND (temp, 0);
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 233367)
+++ gcc/doc/extend.texi (working copy)
@@ -7458,7 +7458,8 @@
@end table
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
efficient code, and in most cases it is a better solution than basic
@code{asm}. However, there are two situations where only basic @code{asm}
can be used:
@@ -7516,11 +7517,51 @@
Basic @code{asm} provides no
mechanism to provide different assembler strings for different dialects.
-Here is an example of basic @code{asm} for i386:
+Bas
Re: AW: Wonly-top-basic-asm
I don't think this is a patch we're considering for gcc-6, at least
not for the initial release - I imagine it could be backported from
gcc-7 at some point.
Actually, it was my intent that this apply to v6. It's not like there
is a significant change here. We're documenting long-time behavior, and
adding a (disabled) warning.
The reasons I think this is needed for v6 are:
1) We have become aware of places where basic asm's existing behavior is
a problem. This patch provides a way for users to locate these issues
with a minimal, non-intrusive change.
2) There is a significant change to this behavior being proposed for
v7. When this happens, having a way to locate affected statements with
features from a stable release seems desirable.
Like the other Bernd I have a preference for just -Wbasic-asm. I'd
make the analogy with -Wparentheses - we don't warn about every
parenthesis, but the name of an option is not the place for detailed
explanations of how it works. Less typing and less to remember is
preferrable IMO.
While I still prefer Wbasic-asm-in-function, I really don't have strong
feelings here. Since both you and Bernd prefer this, I have changed it.
+ /* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm (""). */
No all-caps.
Fixed.
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces
smaller,
+safer, and more
efficient code, and in most cases it is a better solution than basic
Rewrap the paragraph?
I could, but then people get cranky about how hard the patch is to review.
-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements do not perform an implicit "memory" clobber
+(@pxref{Clobbers}). Also, there is no implicit clobbering of
@emph{any}
+registers, so (other than in @code{naked} functions which follow the
ABI
+rules) changed registers must be restored to their original value
before
+exiting the @code{asm}. While this behavior has not always been
+documented, GCC has worked this way since at least v2.95.3.
+@strong{Warning:} This "clobber nothing" behavior may be different
than how
+other compilers treat basic @code{asm}, since the C standards for the
+@code{asm} statement provide no guidance regarding these semantics.
As a
+result, @code{asm} statements that work correctly on other compilers
may not
+work correctly with GCC (and vice versa), even though they both compile
+without error.
+
+Future versions of GCC may change basic @code{asm} to clobber memory
and
+perhaps some (or all) registers. This change may fix subtle
problems with
+existing @code{asm} statements. However it may break or slow down ones
+that were working correctly. To ``future-proof'' your asm against
possible
+changes to basic @code{asm}'s semantics, use extended @code{asm}.
+
+You can use @option{-Wbasic-asm-in-function} to locate basic @code{asm}
+statements that may need changes, and refer to
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to
convert
+from basic asm to extended asm} for information about how to perform
the
+conversion.
I still think this is too verbose and would serve to confuse rather
than enlighten in practice. (If anyone feels otherwise, feel free to
overrule me.)
I assume you mean someone besides me...
I'm also no longer sure about references to the wiki.
This is not the first reference to the gcc wiki in the docs (looks like
the 6th for gcc, plus 10 for other wikis).
Let's look at this existing paragraph from the manual:
Since GCC does not parse the @var{AssemblerInstructions}, it has
no visibility of any symbols it references. This may result in GCC
discarding those symbols as unreferenced.
I think extending this would be better. Something like
"Since the C standards does not specify semantics for @code{asm}, it
is a potential source of incompatibilities between compilers. GCC does
not parse the @var{AssemblerInstructions}, which means there is no way
to communicate to the compiler what is happening inside them. GCC has
no visibility of any symbols referenced in the @code{asm} and may
discard them as unreferenced. It also does not know about side effects
that may occur, such as modifications of memory locations or
registers. GCC assumes that no such side effects occur, which may not
be what the user expected if code was written for other compilers.
Since basic @code{asm} cannot easily be used in a reliable way,
@option{-Wbasic-asm} should be used to warn about the use of basic asm
inside a function. See
@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to
convert from basic asm to extended asm} for information about how to
convert code to use extended @code{asm}."
Hmm. Yes, that's better. But there are some things that got lost here
that I think are important. How
Re: AW: Wonly-top-basic-asm
On 2/11/2016 8:03 AM, Sandra Loosemore wrote: On 02/11/2016 08:40 AM, Bernd Schmidt wrote: But again, if someone feels the docs patch as posted is preferrable, go ahead and approve it (for stage1 I assume). TBH, I haven't looked at the documentation patch at all; I've been ignoring this issue because (a) I thought the technical details were still under discussion and (b) adding a new command-line option is stage 1 material not suitable to commit right now anyway. I'll take a look at the docs once the name and behavior of the new option have been settled upon. So far, no one has suggested any changes to the behavior. And we seem to have settled on a name. dw
Re: AW: Wonly-top-basic-asm
Since no one expressed any objections, I have renamed the option from
-Wonly-top-basic-asm to -Wbasic-asm-in-function. This more clearly
conveys what the option does (give a warning if you find basic asm in a
function).
I believe the attached patch addresses all the other outstanding comments.
ChangeLog:
2016-02-10 David Wohlferd <d...@limegreensocks.com>
* doc/extend.texi: Doc basic asm behavior and new
-Wbasic-asm-in-function option.
* doc/invoke.texi: Doc new -Wbasic-asm-in-function option.
* c-family/c.opt: Define -Wbasic-asm-in-function.
* c/c-parser.c: Implement -Wbasic-asm-in-function for C.
* cp/parser.c: Implement -Wbasic-asm-in-function for c++.
* testsuite/c-c++-common/Wbasic-asm-in-function.c: New tests for
-Wbasic-asm-in-function.
* testsuite/c-c++-common/Wbasic-asm-in-function-2.c: Ditto.
While I have a release on file with FSF, I don't have write access to SVN.
dw
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt (revision 233308)
+++ gcc/c-family/c.opt (working copy)
@@ -585,6 +585,10 @@
C++ ObjC++ Var(warn_namespaces) Warning
Warn on namespace definition.
+Wbasic-asm-in-function
+C ObjC ObjC++ C++ Var(warn_basic_asm_in_function) Warning
+Warn on unsafe uses of basic asm.
+
Wsized-deallocation
C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
Warn about missing sized deallocation functions.
Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c (revision 233308)
+++ gcc/c/c-parser.c (working copy)
@@ -5972,7 +5972,18 @@
labels = NULL_TREE;
if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
+ {
+/* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm (""). */
+if (warn_basic_asm_in_function && TREE_STRING_LENGTH (str) != 1)
+ if (lookup_attribute ("naked",
+ DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+ warning_at (asm_loc, OPT_Wbasic_asm_in_function,
+ "asm statement in function does not use extended syntax");
+
goto done_asm;
+ }
/* Parse each colon-delimited section of operands. */
nsections = 3 + is_goto;
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 233308)
+++ gcc/cp/parser.c (working copy)
@@ -18041,6 +18041,8 @@
bool goto_p = false;
required_token missing = RT_NONE;
+ location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
+
/* Look for the `asm' keyword. */
cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
@@ -18199,6 +18201,17 @@
/* If the extended syntax was not used, mark the ASM_EXPR. */
if (!extended_p)
{
+ /* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm (""). */
+ if (warn_basic_asm_in_function
+ && TREE_STRING_LENGTH (string) != 1)
+ if (lookup_attribute ("naked",
+ DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+ warning_at (asm_loc, OPT_Wbasic_asm_in_function,
+ "asm statement in function does not use extended"
+ " syntax");
+
tree temp = asm_stmt;
if (TREE_CODE (temp) == CLEANUP_POINT_EXPR)
temp = TREE_OPERAND (temp, 0);
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 233308)
+++ gcc/doc/extend.texi (working copy)
@@ -7458,7 +7458,8 @@
@end table
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
efficient code, and in most cases it is a better solution than basic
@code{asm}. However, there are two situations where only basic @code{asm}
can be used:
@@ -7516,11 +7517,51 @@
Basic @code{asm} provides no
mechanism to provide different assembler strings for different dialects.
-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements do not perform an implicit "memory" clobber
+(@pxref{Clobbers}). Also, there is no implicit clobbering of @emph{any}
+registers, so (other than in @code{naked} functions which follow the ABI
+rules) changed registers must be restored to their original value before
+exiting the @code{asm}. While this behavior has not always been
+documented, GCC has worked this way since at least v2.95.3.
+@strong{Warning:} This "clobber nothing" behavior may be different than how
+other compilers treat basic @code{asm}, since the C standards for the
+@code{asm} statement provide no guidance regarding these semantics. As a
+result, @code{asm} statements that work correctly on other compilers may not
+work correctly with GCC (a
Re: AW: Wonly-top-basic-asm
On 2/7/2016 10:45 PM, Bernd Edlinger wrote: On 8. 2. 2016 04:45, David Wohlferd wrote: I replied with a patch that includes most of the changes you asked for (see inline below). Were you waiting on me for something more? ChangeLog entries are still missing. I'll add them back in the next post. David, there is a tool that you can use to check the patch for some style-nits before submission. I was not aware of this tool. I'll fix these before the next post. At one point you proposed renaming the option -Wbasic-asm. This seemed a little terse (and possibly misleading) so I counter-proposed -Wbasic-asm-in-function (a little verbose, but clearer). I have no strong preferences here, and you haven't said one way or the other. Are we just going to stick with -Wonly-top-basic-asm? Hopefully one more try and this will be done. Thanks, dw
Re: Wonly-top-basic-asm
Hey Bernd.
I replied with a patch that includes most of the changes you asked for
(see inline below). Were you waiting on me for something more?
I have cleaned up the testcases so they aren't so i386-specific, but
otherwise this patch (attached) is the same. Let me know if there is
something more I need to do here.
Thanks,
dw
On 1/27/2016 11:20 PM, David Wohlferd wrote:
Rumors that I earn a commission for every person I switch to the
"extended asm" plan are completely unfounded... :)
That said, I truly believe there are very few cases where using basic
asm within a function makes sense. What's more, either they currently
work incorrectly and need to be found and fixed, or they are going to
take a performance hit when the new "clobber everything" semantics are
implemented. So yeah, I pushed pretty hard to have the docs say
"DON'T USE IT!"
But reading it all again, you are right: It's too much.
On 1/25/2016 4:25 AM, Bernd Schmidt wrote:
On 01/24/2016 11:23 PM, David Wohlferd wrote:
+Wonly-top-basic-asm
+C ObjC ObjC++ C++ Var(warn_only_top_basic_asm) Warning
+Warn on unsafe uses of basic asm.
Maybe just -Wbasic-asm?
Hmm. Maybe. I've never been completely satisfied with that name. But
wouldn't this sound like it would warn on all uses of basic asm? The
intent is to only flag the ones in functions. They are the ones with
all the problems.
What would you say to -Wbasic-asm-in-function? A little verbose...
+/* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm(""). */
Two spaces after a sentence.
Fixed.
+if (warn_only_top_basic_asm && (TREE_STRING_LENGTH (str) != 1) )
Unnecessary parens, and extra space before closing paren.
Fixed.
+ if (warn_only_top_basic_asm &&
+ (TREE_STRING_LENGTH (string) != 1))
Extra parens, and && goes first on the next line.
Fixed.
+ warning_at(asm_loc, OPT_Wonly_top_basic_asm,
Space before "(".
Fixed.
+"asm statement in function does not use extended
syntax");
Could break that into ".." "..." over two lines so as to keep
indentation.
Fixed.
-asm ("");
+asm ("":::);
Is that necessary? As far as I can tell we're treating these equally.
While you are correct that today they are treated (nearly) equally, if
Jeff does what he plans for v7, asm("") is going to translate (on x64)
to:
asm("":::"rax", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11", "r12",
"r13", "r14", "r15", "rdi", "rsi", "rbp", "cc", "memory");
Given that, it seemed appropriate for the docs to start steering
people away from basic. This particular instance was just something
that was mentioned during the discussion.
However, that's for someday. Maybe "someday" will turn into some
other solution. And since I'm not prepared to go thru all the docs
and change all the other basic asms at this time, I removed this change.
@@ -7487,6 +7490,8 @@
consecutive in the output, put them in a single multi-instruction
@code{asm}
statement. Note that GCC's optimizers can move @code{asm} statements
relative to other code, including across jumps.
+Using inputs and outputs with extended @code{asm} can help
correctly position
+your asm.
Not sure this is needed either. Sounds a bit like advertising :) In
general the doc changes seem much too verbose to me.
I didn't think about this until it was brought up during the
discussion. But once it was pointed out to me, it made sense.
However, at your suggestion I have removed this.
+Extended @code{asm}'s @samp{%=} may help resolve this.
Same here. I think the block that recommends extended asm is good
enough. I think the next part could be shrunk significantly too.
Removed.
-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements within functions do not perform an
implicit
+"memory" clobber (@pxref{Clobbers}). Also, there is no implicit
clobbering
+of @emph{any} registers, so (other than "naked" functions which
follow the
"other than in"? Also @code{naked} maybe.
It works for me either way. Since it bothers you, I changed it.
I'd place a note about clobbering after the existing "To access C
data, it is better to use extended asm".
+ABI rules) changed registers must be restored to their original
value before
+exiting the @code{asm}. While this behavior has not always been
documented,
+GCC has worked this way since at least v2.95.3. Also, lacking
inputs and
+outputs means that GCC's optimizers may have difficulties consistently
+positioning the basic @code{asm} in the ge
Re: Wonly-top-basic-asm
Rumors that I earn a commission for every person I switch to the
"extended asm" plan are completely unfounded... :)
That said, I truly believe there are very few cases where using basic
asm within a function makes sense. What's more, either they currently
work incorrectly and need to be found and fixed, or they are going to
take a performance hit when the new "clobber everything" semantics are
implemented. So yeah, I pushed pretty hard to have the docs say "DON'T
USE IT!"
But reading it all again, you are right: It's too much.
On 1/25/2016 4:25 AM, Bernd Schmidt wrote:
On 01/24/2016 11:23 PM, David Wohlferd wrote:
+Wonly-top-basic-asm
+C ObjC ObjC++ C++ Var(warn_only_top_basic_asm) Warning
+Warn on unsafe uses of basic asm.
Maybe just -Wbasic-asm?
Hmm. Maybe. I've never been completely satisfied with that name. But
wouldn't this sound like it would warn on all uses of basic asm? The
intent is to only flag the ones in functions. They are the ones with
all the problems.
What would you say to -Wbasic-asm-in-function? A little verbose...
+/* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm(""). */
Two spaces after a sentence.
Fixed.
+if (warn_only_top_basic_asm && (TREE_STRING_LENGTH (str) != 1) )
Unnecessary parens, and extra space before closing paren.
Fixed.
+ if (warn_only_top_basic_asm &&
+ (TREE_STRING_LENGTH (string) != 1))
Extra parens, and && goes first on the next line.
Fixed.
+ warning_at(asm_loc, OPT_Wonly_top_basic_asm,
Space before "(".
Fixed.
+"asm statement in function does not use extended
syntax");
Could break that into ".." "..." over two lines so as to keep
indentation.
Fixed.
-asm ("");
+asm ("":::);
Is that necessary? As far as I can tell we're treating these equally.
While you are correct that today they are treated (nearly) equally, if
Jeff does what he plans for v7, asm("") is going to translate (on x64) to:
asm("":::"rax", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11", "r12",
"r13", "r14", "r15", "rdi", "rsi", "rbp", "cc", "memory");
Given that, it seemed appropriate for the docs to start steering people
away from basic. This particular instance was just something that was
mentioned during the discussion.
However, that's for someday. Maybe "someday" will turn into some other
solution. And since I'm not prepared to go thru all the docs and
change all the other basic asms at this time, I removed this change.
@@ -7487,6 +7490,8 @@
consecutive in the output, put them in a single multi-instruction
@code{asm}
statement. Note that GCC's optimizers can move @code{asm} statements
relative to other code, including across jumps.
+Using inputs and outputs with extended @code{asm} can help correctly
position
+your asm.
Not sure this is needed either. Sounds a bit like advertising :) In
general the doc changes seem much too verbose to me.
I didn't think about this until it was brought up during the
discussion. But once it was pointed out to me, it made sense.
However, at your suggestion I have removed this.
+Extended @code{asm}'s @samp{%=} may help resolve this.
Same here. I think the block that recommends extended asm is good
enough. I think the next part could be shrunk significantly too.
Removed.
-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements within functions do not perform an implicit
+"memory" clobber (@pxref{Clobbers}). Also, there is no implicit
clobbering
+of @emph{any} registers, so (other than "naked" functions which
follow the
"other than in"? Also @code{naked} maybe.
It works for me either way. Since it bothers you, I changed it.
I'd place a note about clobbering after the existing "To access C
data, it is better to use extended asm".
+ABI rules) changed registers must be restored to their original
value before
+exiting the @code{asm}. While this behavior has not always been
documented,
+GCC has worked this way since at least v2.95.3. Also, lacking
inputs and
+outputs means that GCC's optimizers may have difficulties consistently
+positioning the basic @code{asm} in the generated code.
The existing text already mentions ordering issues. Lose this block.
I've removed the rest of the paragraph after "Also"
Wait, didn't you tell me to remove the other mention of 'ordering'? I
think I've removed all of them now. Not a huge loss, but was that what
you intended?
+The concept of ``clobbering'' does not apply to basic @code{asm}
statemen
Re: Wonly-top-basic-asm
On 1/26/2016 8:11 AM, Segher Boessenkool wrote:
On Tue, Jan 26, 2016 at 01:11:36PM +0100, Bernd Schmidt wrote:
On 01/26/2016 01:29 AM, Segher Boessenkool wrote:
In my opinion we should not warn for any asm that means the same both
as basic and as extended asm. The problem then becomes, what *is* the
meaning of a basic asm, what does it clobber.
I think this may be too hard to figure out in general without parsing
the asm string, which we don't really want to do.
That depends on the semantics of basic asm. With the currently implemented
semantics, it is trivial.
Oh?
asm("cmp al, '#' # if (c == '#') {");
There's a '{', so it might look like it needs to be escaped, but it
doesn't. The '{' is just part of a comment. And how do you know it's a
comment? Because of the comment marker (#). And how do you know that
it's a comment marker and not a literal? Only by doing more assembler
parsing that I'm prepared to write.
But more importantly, consider this MIPS statement:
asm ("sync");
That is the same for both basic and extended. But I absolutely want the
warning to flag this statement. This is exactly the kind of "broken"
statement that people need to be able to find/fix right now.
And when Jeff makes the changes to basic asm for v7, people may want to
be able to find the statements that are affected by that in order to
*stop* clobbering so many registers.
I'm not clear what people would use this warning for if we made the
change you are suggesting.
dw
Wonly-top-basic-asm
I'm not sure which 'subsystem maintainer' to include on this as it
affects parsers for both C and c++. I've also cc'ed people from the
discussion thread.
While that ~100 message thread on the gcc list about pr24414 didn't come
to any final conclusions about clobbering registers for basic asm, there
were a few things people agreed we could do to help users right now:
- Update the basic asm docs to describe basic asm's current (and
historical) semantics (ie clobber nothing).
- Emphasize how that might be different from users' expectations or the
behavior of other compilers.
- Warn that this could change in future versions of gcc. To avoid
impacts from this change, use extended asm.
- Implement and document -Wonly-top-basic-asm (disabled by default) as a
way to locate affected statements.
This patch does these things. You can review the new doc text at
http://www.LimeGreenSocks.com/gcc/Basic-Asm.html
ChangeLog:
2016-01-24 David Wohlferd <d...@limegreensocks.com>
* doc/extend.texi: Doc basic asm behavior and new
-Wonly-top-basic-asm option.
* doc/invoke.texi: Doc new -Wonly-top-basic-asm option.
* c-family/c.opt: Define -Wonly-top-basic-asm.
* c/c-parser.c: Implement -Wonly-top-basic-asm for C.
* cp/parser.c: Implement -Wonly-top-basic-asm for c++.
* testsuite/c-c++-common/Wonly-top-basic-asm.c: New tests for
-Wonly-top-basic-asm.
* testsuite/c-c++-common/Wonly-top-basic-asm-2.c: Ditto.
Note that while I have a release on file with FSF, I don't have write
access to SVN.
dw
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt (revision 232773)
+++ gcc/c-family/c.opt (working copy)
@@ -585,6 +585,10 @@
C++ ObjC++ Var(warn_namespaces) Warning
Warn on namespace definition.
+Wonly-top-basic-asm
+C ObjC ObjC++ C++ Var(warn_only_top_basic_asm) Warning
+Warn on unsafe uses of basic asm.
+
Wsized-deallocation
C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
Warn about missing sized deallocation functions.
Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c (revision 232773)
+++ gcc/c/c-parser.c (working copy)
@@ -5973,7 +5973,18 @@
labels = NULL_TREE;
if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
+ {
+/* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm(""). */
+if (warn_only_top_basic_asm && (TREE_STRING_LENGTH (str) != 1) )
+ if (lookup_attribute ("naked",
+DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+warning_at(asm_loc, OPT_Wonly_top_basic_asm,
+ "asm statement in function does not use extended syntax");
+
goto done_asm;
+ }
/* Parse each colon-delimited section of operands. */
nsections = 3 + is_goto;
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 232773)
+++ gcc/cp/parser.c (working copy)
@@ -18003,6 +18003,8 @@
bool goto_p = false;
required_token missing = RT_NONE;
+ location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
+
/* Look for the `asm' keyword. */
cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
@@ -18161,6 +18163,16 @@
/* If the extended syntax was not used, mark the ASM_EXPR. */
if (!extended_p)
{
+ /* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions. Also allow asm(""). */
+ if (warn_only_top_basic_asm &&
+ (TREE_STRING_LENGTH (string) != 1))
+ if (lookup_attribute("naked",
+ DECL_ATTRIBUTES (current_function_decl))
+ == NULL_TREE)
+ warning_at(asm_loc, OPT_Wonly_top_basic_asm,
+ "asm statement in function does not use extended syntax");
+
tree temp = asm_stmt;
if (TREE_CODE (temp) == CLEANUP_POINT_EXPR)
temp = TREE_OPERAND (temp, 0);
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 232773)
+++ gcc/doc/extend.texi (working copy)
@@ -2903,12 +2903,14 @@
although the function call is live. To keep such calls from being
optimized away, put
@smallexample
-asm ("");
+asm ("":::);
@end smallexample
@noindent
(@pxref{Extended Asm}) in the called function, to serve as a special
side-effect.
+Older code used @code{asm("")}, but newer code should use the extended
+@code{asm} format.
@item nonnull (@var{arg-index}, @dots{})
@cindex @code{nonnull} function attribute
@@ -7458,7 +7460,8 @@
@end table
@subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Ext
Re: [patch, doc] copy-edit inline asm sections
On 2/5/2015 9:13 AM, Sandra Loosemore wrote:
In addition to fixing markup and capitalization, I've moved things
around a little bit to improve the flow, and rephrased a few things
that I thought were awkward or confusing. I propose to commit this in
a few days unless somebody tells me meanwhile that I've broken
something there's no doubt a lot more that could be done to
improve this section further, but I hope this is at least an
incremental improvement.
Looks like some good changes here. I do have a few suggestions:
Patch line 34: You wrote within a function, but if you want to include
assembly language at. How about within a function, but to include
assembly language at instead?
Patch line 157: You changed AssemblerInstructions to assembler
instructions. How about @var{AssemblerInstructions} instead?
Patch line 653: You wrote Use the @samp{} constraint modifier
(@pxref{Modifiers}) on output how about Use the @samp{} constraint
modifier (@pxref{Modifiers}) on all output instead?
Under Input parameters, the existing text says Specify input operands
by using the format:. Shouldn't this match the change you made to
Output parameters (Patch line 557) Operands are separated by commas.
Each operand has this format:.
Patch line 780: You wrote Specifies a symbolic name for the
operand.o. How about Specifies a symbolic name for the operand. instead?
Patch line 788: You lost something important when you used copy/paste
for the information describing the (zero-based) position from output to
input. The old text for input said if there are two output parameters
and three inputs, @code{%2} refers to the first input, @code{%3} to the
second, and @code{%4} to the third. You changed this to just For
example if there are three output operands, use @samp{%0} in the
template to refer to the first, @samp{%1} for the second, and @samp{%2}
for the third. which makes sense for output, but not when you copy it
down to input.
Patch line 905: You changed /* No outputs. */ to /* No outputs. /*,
but I think the original text was correct.
I understand your (apparent) desire to remove duplicated information.
However, sometimes I think duplicating information serves a purpose.
The asm docs are really long, and having certain information only appear
in one place, no matter how logical that place is, might not be
sufficient to make the information available at the point where the user
needs it. For that reason, I ask you to reconsider removing the
duplication for the max # of parameters, and the fact that having no
output parameters results in the statement being implicitly volatile.
If you absolutely can't stand it, then go ahead and remove them.
Since this last set of changes got checked in, I have been hanging out
at stackoverflow to see how people are using inline asm and what types
of questions they have. I have collected a few additional changes:
1) (Basic asm) Describe what top level asm is and how it works.
Combining my changes with yours gives:
Using extended @code{asm} will typically produce smaller, safer, and more
efficient code, and in most cases it is a better solution than basic
@code{asm}. However, unlike extended @code{asm} which can only be used
in functions, basic @code{asm} can be included at file scope (sometimes
referred to as Top Level asm). This technique is sometimes used to
define assembly routines or assembly language macros that can be invoked
elsewhere. Unlike standard C global initializers (which also appear at
file scope), Top Level asm is not automatically executed at startup or
shutdown. To execute code defined at file scope, it must explicitly be
called from some other location in your code.
Functions declared with the @code{naked} attribute also require basic
@code{asm} (@pxref{Function Attributes}).
2) (Extended asm) Copy the may not perform jumps text from GotoLabels
to AssemblerTemplate.
While you seem to have an aversion to duplicating text, I was seriously
considering copying this text into the AssemblerTemplate section. While
describing it in the context of goto made sense at the time, if you
aren't writing a asm goto statement, you might never see this warning:
@code{asm} statements may not perform jumps into other @code{asm}
statements.
GCC's optimizers do not know about these jumps; therefore they cannot take
account of them when deciding how to optimize.
3) (Extended asm) Describe the (relatively new) support for escaping
dialect chars (r198641).
Sometime after I wrote the text saying Also, there is no ``escape'' for
an open brace (@{), so do not use open braces in an Extended @code{asm}
template other than as a dialect indicator., someone added support for
it. In addition to removing that text, this should get added next to %=
and %%:
@item
%@{ outputs a @{ into the assembler code.
@item
%| outputs a | into the assembler code.
@item
%@} outputs a @} into the
Re: [Patch] PR 61692 - Fix for inline asm ICE
Ping? dw On 11/15/2014 7:59 PM, David Wohlferd wrote: On 9/15/2014 2:51 PM, Jeff Law wrote: Let's go with your original inputs + outputs + labels change and punt the clobbers stuff for now. jeff I have also added the test code you requested. I have a release on file with the FSF, but don't have SVN write access. Problem: extract_insn() in recog.c will ICE if (noperands MAX_RECOG_OPERANDS). Normally this isn't a problem since expand_asm_operands() in cfgexpand.c catches and reports a proper error for this condition. However, expand_asm_operands() only checks (ninputs + noutputs) instead of (ninputs + noutputs + nlabels), so you can get the ICE when using asm goto. ChangeLog: 2014-11-15 David Wohlferd d...@limegreensocks.com PR target/61692 * cfgexpand.c (expand_asm_operands): Count all inline asm params. * testsuite/gcc.dg/pr61692.c: New test. dw
Re: [Patch] PR 61692 - Fix for inline asm ICE
On 9/15/2014 2:51 PM, Jeff Law wrote:
Let's go with your original inputs + outputs + labels change and punt
the clobbers stuff for now.
jeff
I have also added the test code you requested.
I have a release on file with the FSF, but don't have SVN write access.
Problem:
extract_insn() in recog.c will ICE if (noperands MAX_RECOG_OPERANDS).
Normally this isn't a problem since expand_asm_operands() in cfgexpand.c
catches and reports a proper error for this condition. However,
expand_asm_operands() only checks (ninputs + noutputs) instead of
(ninputs + noutputs + nlabels), so you can get the ICE when using asm
goto.
ChangeLog:
2014-11-15 David Wohlferd d...@limegreensocks.com
PR target/61692
* cfgexpand.c (expand_asm_operands): Count all inline asm params.
* testsuite/gcc.dg/pr61692.c: New test.
dw
Index: gcc/cfgexpand.c
===
--- gcc/cfgexpand.c (revision 217623)
+++ gcc/cfgexpand.c (working copy)
@@ -2589,7 +2589,7 @@
}
ninputs += ninout;
- if (ninputs + noutputs MAX_RECOG_OPERANDS)
+ if (ninputs + noutputs + nlabels MAX_RECOG_OPERANDS)
{
error (more than %d operands in %asm%, MAX_RECOG_OPERANDS);
return;
Index: gcc/testsuite/gcc.dg/pr61692.c
===
--- gcc/testsuite/gcc.dg/pr61692.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr61692.c (working copy)
@@ -0,0 +1,173 @@
+/* PR 61692 */
+
+/* Check for ice when exceededing the max #
+ of parameters to inline asm. */
+
+int Labels()
+{
+label01: label02: label03: label04: label05:
+label06: label07: label08: label09: label10:
+label11: label12: label13: label14: label15:
+label16: label17: label18: label19: label20:
+label21: label22: label23: label24: label25:
+label26: label27: label28: label29: label30:
+label31:
+
+__asm__ goto ( /* Works. */
+: /* no outputs */
+: /* no inputs */
+: /* no clobbers */
+: label01, label02, label03, label04, label05,
+ label06, label07, label08, label09, label10,
+ label11, label12, label13, label14, label15,
+ label16, label17, label18, label19, label20,
+ label21, label22, label23, label24, label25,
+ label26, label27, label28, label29, label30);
+
+__asm__ goto ( /* { dg-error more than 30 operands } */
+: /* no outputs */
+: /* no inputs */
+: /* no clobbers */
+: label01, label02, label03, label04, label05,
+ label06, label07, label08, label09, label10,
+ label11, label12, label13, label14, label15,
+ label16, label17, label18, label19, label20,
+ label21, label22, label23, label24, label25,
+ label26, label27, label28, label29, label30,
+ label31);
+
+return 0;
+}
+
+int Labels_and_Inputs()
+{
+int b01, b02, b03, b04, b05, b06, b07, b08, b09, b10;
+int b11, b12, b13, b14, b15, b16, b17, b18, b19, b20;
+int b21, b22, b23, b24, b25, b26, b27, b28, b29, b30;
+int b31;
+
+label01: label02: label03: label04: label05:
+label06: label07: label08: label09: label10:
+label11: label12: label13: label14: label15:
+label16: label17: label18: label19: label20:
+label21: label22: label23: label24: label25:
+label26: label27: label28: label29: label30:
+label31:
+
+b01 = b02 = b03 = b04 = b05 = b06 = b07 = b08 = b09 = b10 = 0;
+b11 = b12 = b13 = b14 = b15 = b16 = b17 = b18 = b19 = b20 = 0;
+b21 = b22 = b23 = b24 = b25 = b26 = b27 = b28 = b29 = b30 = 0;
+b31 = 0;
+
+__asm__ goto ( /* Works. */
+ : /* no outputs */
+ : m (b01), m (b02), m (b03), m (b04), m (b05),
+m (b06), m (b07), m (b08), m (b09), m (b10),
+m (b11), m (b12), m (b13), m (b14), m (b15),
+m (b16), m (b17), m (b18), m (b19), m (b20),
+m (b21), m (b22), m (b23), m (b24), m (b25),
+m (b26), m (b27), m (b28), m (b29)
+ : /* no clobbers */
+ : label01);
+
+__asm__ goto ( /* { dg-error more than 30 operands } */
+ : /* no outputs */
+ : m (b01), m (b02), m (b03), m (b04), m (b05),
+m (b06), m (b07), m (b08), m (b09), m (b10),
+m (b11), m (b12), m (b13), m (b14), m (b15),
+m (b16), m (b17), m (b18), m (b19), m (b20),
+m (b21), m (b22), m (b23), m (b24), m (b25),
+m (b26), m (b27), m (b28), m (b29), m (b30)
+ : /* no clobbers */
+ : label01);
+
+ return 0;
+}
+
+int Outputs()
+{
+int b01, b02, b03, b04, b05, b06, b07, b08, b09, b10;
+int b11, b12, b13, b14, b15, b16, b17, b18, b19, b20;
+int b21, b22, b23, b24, b25, b26, b27, b28, b29, b30;
+int b31;
+
+/* Outputs. */
+__asm__ volatile ( /* Works. */
+ : =m (b01), =m (b02), =m (b03), =m (b04), =m (b05),
+ =m (b06), =m (b07), =m (b08), =m (b09), =m (b10),
+ =m (b11
Re: [Patch] PR 61692 - Fix for inline asm ICE
I sent you the file you requested (off list), but never heard back from you about the valgrind results. In an effort to move this along, I installed ubuntu under virtualbox and did a build of gcc. When running the output of this build with valgrind, I saw a number of memory *leaks* reported, but no overruns, despite having maxed out the operands + clobbers in a variety of ways. I have only tested this on x86, and only with inline asm, but I have had no luck (using code inspection, sprinkling printfs, and now valgrind) locating the error you are expecting to see. Without knowing what is making you quite confident there is a problem, I don't know what else to try. Suggestions? Theoretically I could add the nclobbers in just in case. But unlike adding nlabels, adding nclobbers here will almost certainly break someone's code. I'm not prepared to do that unless there is a clear problem to be fixed, and I'm just not seeing it. If you are also out of ideas, I can re-send the patch for the original ninputs + noutputs + nlabels problem (along with the testcase you requested), and we can at least fix the known ICE. dw On 8/1/2014 11:29 AM, Jeff Law wrote: On 08/01/14 02:07, David Wohlferd wrote: I'd love to. Unfortunately, my platform doesn't support valgrind. Ah. Also, please include the testcase you had nlabels part. I have created the testcase for the 31 labels problem. However, not so much for the nclobbers part. And if I'm going to patch both, I should have testcases for both. Tell you what, pass along what you've got and I'll run it under valgrind here. I'm quite confident both need to be changed -- though it is possible nothing will trigger with the nclobbers stuff if it is indeed handled separately throughout the guts of GCC. Jeff
Re: [Patch] Remove arm-specific formats from asm_fprintf
Sorry for the late response. I spent some time trying to understand your concerns here, but I'm not sure I got what you are saying: 1) Did you assume this patch would somehow disable these formats during cross builds, preventing the i386-arm xgcc from correctly using them? I don't believe that's true. At worst this patch will cause additional compile warnings. 2) Or is it the additional compile warnings this patch introduces that are the problem? It seems to me that warnings SHOULD be generated when encountering platform-specific features from another platform. Do we really want to try to teach every platform about the implementation details of all other platforms? While we could try, I'm not convinced it's even possible. At best, a build could verify the formats that were active (for the all the platforms supported by gnu) at the time it got built. But given new formats, new platforms, etc there's no way to know what's in the code it's trying to compile now. It seems like a lot of complexity, and an ugly co-mingling of platforms, all to prevent a few completely understandable warnings that only occur when building the cross compiler (ie not when actually running the cross compiler, doing bootstrap builds, etc). That said, you have a better understanding of the design and vision of gcc than I do. But if the warnings are your concern, perhaps I can propose other alternatives for dealing with them? Or have I completely missed your point? dw On 7/28/2014 3:21 PM, Joseph S. Myers wrote: On Wed, 23 Jul 2014, David Wohlferd wrote: 2014-07-23 David Wohlferd d...@limegreensocks.com * doc/c-family/c-format.c: Add support for target macro ASM_FPRINTF_TABLE, remove arm-specific formats. * gcc/config/arm/arm.h: Use ASM_FPRINTF_TABLE for %@ and %r. * gcc/doc/tm.texi: Document new target macro. * gcc/doc/tm.texi.in: Ditto. * gcc/testsuite/gcc.dg/format/asm_fprintf-1.c: Make tests for %@ and %r arm-specific. This patch would have the effect that only compilers for ARM target support the formats in asm_fprintf format checking. That is, when building GCC, they would only be accepted when building GCC for ARM *host*. But of course what you want is for them to be accepted when building GCC for ARM *target* but any host. I.e., the support always needs to be compiled into c-format.c, and the decision whether to enable those formats needs to be taken by GCC when it is run, based on whether it is compiling a copy of GCC for ARM target (as opposed to GCC for some other target, or some other program altogether). Existing configuration based on the GCC being compiled is in init_dynamic_asm_fprintf_info and other such functions. I suppose you could have a __gcc_asm_fprintf_arm_formats__ identifier declared in some GCC header when building a compiler for ARM target (I'm supposing these formats are unlikely to be used in files from which tm.h includes could be removed any time soon).
Re: [Patch] PR 61692 - Fix for inline asm ICE
On 7/30/2014 9:58 PM, Jeff Law wrote: On 07/28/14 16:39, David Wohlferd wrote: On 7/28/2014 12:42 PM, Jeff Law wrote: On 07/27/14 01:26, David Wohlferd wrote: I'm not sure which maintainer to cc for inline asm stuff? I have a release on file with the FSF, but don't have SVN write access. Problem: extract_insn() in recog.c will ICE if (noperands MAX_RECOG_OPERANDS). Normally this isn't a problem since expand_asm_operands() in cfgexpand.c catches and reports a proper error for this condition. However, expand_asm_operands() only checks (ninputs + noutputs) instead of (ninputs + noutputs + nlabels), so you can get the ICE when using asm goto. See the bugzilla entry for sample code. ChangeLog: 2014-07-27 David Wohlferd d...@limegreensocks.com PR target/61692 * cfgexpand.c (expand_asm_operands): Count all inline asm parameters. You should also include 'nclobbers'. Reading thru asm_noperands (which is what extract_insn uses to count operands), I would have thought you were right. But while making this fail with nLabels was easy, I wasn't able to get this to ICE at all using clobbers (30 labels + 11 clobbers still didn't ICE). And I'm reluctant to propose that change unless I can see it fail. I understand, but I'm still quite confident it's the right thing to do. Running that 30 label + 11 clobber testcase under valgrind might show the problem, if you can stand waiting that long... I'd love to. Unfortunately, my platform doesn't support valgrind. Also, please include the testcase you had nlabels part. I have created the testcase for the 31 labels problem. However, not so much for the nclobbers part. And if I'm going to patch both, I should have testcases for both. I also worry about potentially breaking existing code. While the 31 labels thing I proposed won't break existing code (it would have been ICE-ing), adding nclobbers to the count of parameters could do so. When Jeff Law says that something is so about gcc, I tend to believe that it is so. But, unless I can see an example of the actual problem here, I have no idea how to create a test case for it. So I spent the last several hours hunting for it. If valgrind was going to show the problem, presumably we were expecting some type of array overrun. And there are a LOT of places that do foo[MAX_RECOG_OPERANDS]. Absent valgrind, I resorted to using printfs sprinkled throughout the code to check indices. However, despite my best efforts, I was unable to see any out of range accesses, gcc_asserts, segment faults or any other indications of a problem. All the places I tried seem to treat clobbers separately. That doesn't prove anything: I could easily have missed something. But I did make a sincere effort. Given how confident you are that there's a problem, could you point out a place+condition I should focus on? That way I can create the additional test case, satisfy my curiosity, and sleep soundly at night knowing I haven't (unnecessarily) broken people's code. Thanks, dw
Re: [Patch] PR 61692 - Fix for inline asm ICE
On 7/28/2014 12:42 PM, Jeff Law wrote: On 07/27/14 01:26, David Wohlferd wrote: I'm not sure which maintainer to cc for inline asm stuff? I have a release on file with the FSF, but don't have SVN write access. Problem: extract_insn() in recog.c will ICE if (noperands MAX_RECOG_OPERANDS). Normally this isn't a problem since expand_asm_operands() in cfgexpand.c catches and reports a proper error for this condition. However, expand_asm_operands() only checks (ninputs + noutputs) instead of (ninputs + noutputs + nlabels), so you can get the ICE when using asm goto. See the bugzilla entry for sample code. ChangeLog: 2014-07-27 David Wohlferd d...@limegreensocks.com PR target/61692 * cfgexpand.c (expand_asm_operands): Count all inline asm parameters. You should also include 'nclobbers'. Reading thru asm_noperands (which is what extract_insn uses to count operands), I would have thought you were right. But while making this fail with nLabels was easy, I wasn't able to get this to ICE at all using clobbers (30 labels + 11 clobbers still didn't ICE). And I'm reluctant to propose that change unless I can see it fail. dw
[Patch] PR 61692 - Fix for inline asm ICE
I'm not sure which maintainer to cc for inline asm stuff?
I have a release on file with the FSF, but don't have SVN write access.
Problem:
extract_insn() in recog.c will ICE if (noperands MAX_RECOG_OPERANDS).
Normally this isn't a problem since expand_asm_operands() in cfgexpand.c
catches and reports a proper error for this condition. However,
expand_asm_operands() only checks (ninputs + noutputs) instead of
(ninputs + noutputs + nlabels), so you can get the ICE when using asm
goto. See the bugzilla entry for sample code.
ChangeLog:
2014-07-27 David Wohlferd d...@limegreensocks.com
PR target/61692
* cfgexpand.c (expand_asm_operands): Count all inline asm
parameters.
dw
Index: cfgexpand.c
===
--- cfgexpand.c (revision 212900)
+++ cfgexpand.c (working copy)
@@ -2554,7 +2554,7 @@
}
ninputs += ninout;
- if (ninputs + noutputs MAX_RECOG_OPERANDS)
+ if (ninputs + noutputs + nlabels MAX_RECOG_OPERANDS)
{
error (more than %d operands in %asm%, MAX_RECOG_OPERANDS);
return;
[Patch] Remove arm-specific formats from asm_fprintf
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
asm_fprintf allows platforms to add support for new format specifiers by
using the ASM_FPRINTF_EXTENSIONS macro. ARM uses this to add support
for %@ and %r specifiers.
However, it isn't enough to add these two items to the case statement in
asm_fprintf (which is what ASM_FPRINTF_EXTENSIONS does). Over in
c-format.c, there is compile-time checking that is done against calls to
asm_fprintf to validate the format string. %@ and %r have been added to
this checking (see asm_fprintf_char_table), but NOT in a
platform-specific way.
This means that using %r or %@ will successfully pass the format
checking on all platforms, but will ICE if used on non-ARM platforms
since without ASM_FPRINTF_EXTENSIONS, there are no case statements in
asm_fprintf to support them.
ChangeLog:
2014-07-23 David Wohlferd d...@limegreensocks.com
* doc/c-family/c-format.c: Add support for target macro
ASM_FPRINTF_TABLE, remove arm-specific formats.
* gcc/config/arm/arm.h: Use ASM_FPRINTF_TABLE for %@ and %r.
* gcc/doc/tm.texi: Document new target macro.
* gcc/doc/tm.texi.in: Ditto.
* gcc/testsuite/gcc.dg/format/asm_fprintf-1.c: Make tests
for %@ and %r arm-specific.
dw
Index: gcc/c-family/c-format.c
===
--- gcc/c-family/c-format.c (revision 212900)
+++ gcc/c-family/c-format.c (working copy)
@@ -637,8 +637,9 @@
{ I, 0, STD_C89, NOARGUMENTS, , , NULL },
{ L, 0, STD_C89, NOARGUMENTS, , , NULL },
{ U, 0, STD_C89, NOARGUMENTS, , , NULL },
- { r, 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, , , NULL },
- { @, 0, STD_C89, NOARGUMENTS, , , NULL },
+#ifdef ASM_FPRINTF_TABLE
+ ASM_FPRINTF_TABLE
+#endif
{ NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
};
Index: gcc/config/arm/arm.h
===
--- gcc/config/arm/arm.h (revision 212900)
+++ gcc/config/arm/arm.h (working copy)
@@ -888,6 +888,12 @@
fputs (reg_names [va_arg (ARGS, int)], FILE); \
break;
+/* Used in c-format.c to add entries to the table used to validate calls
+ to asm_fprintf. */
+#define ASM_FPRINTF_TABLE \
+ { r, 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, , , NULL }, \
+ { @, 0, STD_C89, NOARGUMENTS, , , NULL },
+
/* Round X up to the nearest word. */
#define ROUND_UP_WORD(X) (((X) + 3) ~3)
Index: gcc/doc/tm.texi
===
--- gcc/doc/tm.texi (revision 212900)
+++ gcc/doc/tm.texi (working copy)
@@ -8611,8 +8611,39 @@
The varargs input pointer is @var{argptr} and the rest of the format
string, starting the character after the one that is being switched
upon, is pointed to by @var{format}.
+See also ASM_FPRINTF_TABLE.
+
+Example:
+@smallexample
+#define ASM_FPRINTF_EXTENSIONS(FILE, ARGS, P) \
+ case '@': \
+fputs (ASM_COMMENT_START, FILE); \
+break; \
+ \
+ case 'r': \
+fputs (REGISTER_PREFIX, FILE); \
+fputs (reg_names [va_arg (ARGS, int)], FILE); \
+break;
+@end smallexample
@end defmac
+@defmac ASM_FPRINTF_TABLE
+When using ASM_FPRINTF_EXTENSIONS, you must also use this macro to define
+table entries for the printf format checking performed in c-format.c.
+This macro must contain format_char_info entries for each printf format
+being added.
+
+Example:
+@smallexample
+#define ASM_FPRINTF_TABLE \
+ @{ r, 0, STD_C89, \
+ @{ T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, \
+ BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN @}, \
+ , , NULL @}, \
+ @{ @, 0, STD_C89, NOARGUMENTS, , , NULL @},
+@end smallexample
+@end defmac
+
@defmac ASSEMBLER_DIALECT
If your target supports multiple dialects of assembler language (such as
different opcodes), define this macro as a C expression that gives the
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in (revision 212900)
+++ gcc/doc/tm.texi.in (working copy)
@@ -6370,8 +6370,39 @@
The varargs input pointer is @var{argptr} and the rest of the format
string, starting the character after the one that is being switched
upon, is pointed to by @var{format}.
+See also ASM_FPRINTF_TABLE.
+
+Example:
+@smallexample
+#define ASM_FPRINTF_EXTENSIONS(FILE, ARGS, P) \
+ case '@': \
+fputs (ASM_COMMENT_START, FILE); \
+break; \
+ \
+ case 'r': \
+fputs (REGISTER_PREFIX, FILE); \
+fputs (reg_names [va_arg (ARGS, int)], FILE); \
+break;
+@end smallexample
@end defmac
+@defmac ASM_FPRINTF_TABLE
+When using ASM_FPRINTF_EXTENSIONS, you must also use this macro to define
+table
Re: [Patch] PR 61662
On 7/15/2014 11:10 AM, Uros Bizjak wrote: Hello! The detailed description and examples can be found in pr61662, but in short: using #ifdef __x86_64__ to determine the size of a 'long' does not reliably yield the correct result. This causes _lrotl and _lrotr to return incorrect results on LLP64 systems (like Windows). ChangeLog: 2014-07-09 David Wohlferd d...@limegreensocks.com PR target/61662 * config/i386/ia32intrin.h: Use __LP64__ to determine size of long This is OK for mainline and 4.9 backport. Thank you for reviewing and approving this. However while I have a release on file with FSF, I don't have SVN write permissions, so I cannot check this in. dw
[DOC Patch] gnat doc fix
Eric, here are the ada doc modifications we discussed. I believe I have
addressed the rest of the items that concerned you. While I have a
release on file with the FSF, I do not have SVN write permissions.
Problem description:
The modifications to the c inline asm docs (extend.texi) necessitate a
change to the ada docs. A few other minor clarifications are included.
ChangeLog:
2014-07-14 David Wohlferd d...@limegreensocks.com
* ada/gnat_rm.texi (Machine Code Insertions): Inline asm cleanup.
dw
Index: gnat_rm.texi
===
--- gnat_rm.texi (revision 212420)
+++ gnat_rm.texi (working copy)
@@ -20129,8 +20129,8 @@
The two features are similar, and both are closely related to the mechanism
provided by the asm instruction in the GNU C compiler. Full understanding
and use of the facilities in this package requires understanding the asm
-instruction, see @ref{Extended Asm,, Assembler Instructions with C Expression
-Operands, gcc, Using the GNU Compiler Collection (GCC)}.
+instruction, see @ref{Extended Asm,,, gcc, Using the GNU Compiler
+Collection (GCC)}. GNU C's Basic @code{asm} is not supported.
Calls to the function @code{Asm} and the procedure @code{Asm} have identical
semantic restrictions and effects as described below. Both are provided so
@@ -20137,8 +20137,7 @@
that the procedure call can be used as a statement, and the function call
can be used to form a code_statement.
-The first example given in the GCC documentation is the C @code{asm}
-instruction:
+Consider this C @code{asm} instruction:
@smallexample
asm (fsinx %1 %0 : =f (result) : f (angle));
@end smallexample
@@ -20164,12 +20163,15 @@
parameters. The first is a string, the second is the name of a variable
of the type designated by the attribute prefix. The first (string)
argument is required to be a static expression and designates the
-constraint for the parameter (e.g.@: what kind of register is
-required). The second argument is the variable to be updated with the
+constraint (@pxref{Constraints,,, gcc, Using the GNU Compiler
+Collection (GCC)})
+for the parameter (e.g.@: what kind of register is required). The second
+argument is the variable to be written or updated with the
result. The possible values for constraint are the same as those used in
the RTL, and are dependent on the configuration file used to build the
GCC back end. If there are no output operands, then this argument may
either be omitted, or explicitly given as @code{No_Output_Operands}.
+No support is provided for GNU C's symbolic names for output parameters.
The second argument of @code{@var{my_float}'Asm_Output} functions as
though it were an @code{out} parameter, which is a little curious, but
@@ -20186,8 +20188,9 @@
to be a static expression, and is the constraint for the parameter,
(e.g.@: what kind of register is required). The second argument is the
value to be used as the input argument. The possible values for the
-constant are the same as those used in the RTL, and are dependent on
+constraint are the same as those used in the RTL, and are dependent on
the configuration file used to built the GCC back end.
+No support is provided for GNU C's symbolic names for input parameters.
If there are no input operands, this argument may either be omitted, or
explicitly given as @code{No_Input_Operands}. The fourth argument, not
@@ -20197,20 +20200,23 @@
that must be considered destroyed as a result of the @code{Asm} call. If
this argument is the null string (the default value), then the code
generator assumes that no additional registers are destroyed.
+In addition to registers, the special clobbers @code{memory} and
+@code{cc} as described in the GNU C docs are both supported.
The fifth argument, not present in the above example, called the
@dfn{volatile} argument, is by default @code{False}. It can be set to
the literal value @code{True} to indicate to the code generator that all
optimizations with respect to the instruction specified should be
-suppressed, and that in particular, for an instruction that has outputs,
-the instruction will still be generated, even if none of the outputs are
-used. @xref{Extended Asm,, Assembler Instructions with C Expression Operands,
+suppressed, and in particular an instruction that has outputs
+will still be generated, even if none of the outputs are
+used. @xref{Volatile,,,
gcc, Using the GNU Compiler Collection (GCC)}, for the full description.
Generally it is strongly advisable to use Volatile for any ASM statement
-that is missing either input or output operands, or when two or more ASM
-statements appear in sequence, to avoid unwanted optimizations. A warning
-is generated if this advice is not followed.
+that is missing either input or output operands or to avoid unwanted
+optimizations. A warning is generated if this advice is not followed.
+No support is provided for GNU C's @code
[DOC Patch] gnat updates for makeinfo 5.2
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
gnat docs have warning messages when compiled with makeinfo 5.2. Most
of them revolve around the fact that nodes must be defined in the same
order as they appear in menus.
ChangeLog:
2014-07-14 David Wohlferd d...@limegreensocks.com
* ada/gnat_rm.texi: Various cleanups for makeinfo 5.2.
dw
Index: gnat_rm.texi
===
--- gnat_rm.texi (revision 212420)
+++ gnat_rm.texi (working copy)
@@ -84,8 +84,10 @@
* Implementation of Ada 2012 Features::
* Obsolescent Features::
* GNU Free Documentation License::
-* Index::
+* Index:Concept Index.
+@detailmenu
+
--- The Detailed Node Listing ---
About This Guide
@@ -110,8 +112,8 @@
* Pragma Assertion_Policy::
* Pragma Assume::
* Pragma Assume_No_Invalid_Values::
+* Pragma Ast_Entry::
* Pragma Attribute_Definition::
-* Pragma Ast_Entry::
* Pragma C_Pass_By_Copy::
* Pragma Check::
* Pragma Check_Float_Overflow::
@@ -670,13 +672,16 @@
* The Size of Discriminated Records with Default Discriminants::
* Strict Conformance to the Ada Reference Manual::
-Implementation of Ada 2012 Features
+* Implementation of Ada 2012 Features::
-Obsolescent Features
+* Obsolescent Features::
-GNU Free Documentation License
+* GNU Free Documentation License::
-Index
+* Index:Concept Index.
+
+@end detailmenu
+
@end menu
@end ifnottex
@@ -939,8 +944,8 @@
* Pragma Assertion_Policy::
* Pragma Assume::
* Pragma Assume_No_Invalid_Values::
+* Pragma Ast_Entry::
* Pragma Attribute_Definition::
-* Pragma Ast_Entry::
* Pragma C_Pass_By_Copy::
* Pragma Check::
* Pragma Check_Float_Overflow::
@@ -18826,19 +18831,6 @@
@cindex Time
@cindex @code{GNAT.Calendar.Time_IO} (@file{g-catiio.ads})
-@node GNAT.CRC32 (g-crc32.ads)
-@section @code{GNAT.CRC32} (@file{g-crc32.ads})
-@cindex @code{GNAT.CRC32} (@file{g-crc32.ads})
-@cindex CRC32
-@cindex Cyclic Redundancy Check
-
-@noindent
-This package implements the CRC-32 algorithm. For a full description
-of this algorithm see
-``Computation of Cyclic Redundancy Checks via Table Look-Up'',
-@cite{Communications of the ACM}, Vol.@: 31 No.@: 8, pp.@: 1008-1013,
-Aug.@: 1988. Sarwate, D.V@.
-
@node GNAT.Case_Util (g-casuti.ads)
@section @code{GNAT.Case_Util} (@file{g-casuti.ads})
@cindex @code{GNAT.Case_Util} (@file{g-casuti.ads})
@@ -18913,6 +18905,19 @@
@noindent
Provides a simple interface to handle Ctrl-C keyboard events.
+@node GNAT.CRC32 (g-crc32.ads)
+@section @code{GNAT.CRC32} (@file{g-crc32.ads})
+@cindex @code{GNAT.CRC32} (@file{g-crc32.ads})
+@cindex CRC32
+@cindex Cyclic Redundancy Check
+
+@noindent
+This package implements the CRC-32 algorithm. For a full description
+of this algorithm see
+``Computation of Cyclic Redundancy Checks via Table Look-Up'',
+@cite{Communications of the ACM}, Vol.@: 31 No.@: 8, pp.@: 1008-1013,
+Aug.@: 1988. Sarwate, D.V@.
+
@node GNAT.Current_Exception (g-curexc.ads)
@section @code{GNAT.Current_Exception} (@file{g-curexc.ads})
@cindex @code{GNAT.Current_Exception} (@file{g-curexc.ads})
@@ -19558,17 +19563,6 @@
single global task lock. Appropriate for use in situations where contention
between tasks is very rarely expected.
-@node GNAT.Time_Stamp (g-timsta.ads)
-@section @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
-@cindex @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
-@cindex Time stamp
-@cindex Current time
-
-@noindent
-Provides a simple function that returns a string -MM-DD HH:MM:SS.SS that
-represents the current date and time in ISO 8601 format. This is a very simple
-routine with minimal code and there are no dependencies on any other unit.
-
@node GNAT.Threads (g-thread.ads)
@section @code{GNAT.Threads} (@file{g-thread.ads})
@cindex @code{GNAT.Threads} (@file{g-thread.ads})
@@ -19581,6 +19575,17 @@
further details if your program has threads that are created by a non-Ada
environment which then accesses Ada code.
+@node GNAT.Time_Stamp (g-timsta.ads)
+@section @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
+@cindex @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
+@cindex Time stamp
+@cindex Current time
+
+@noindent
+Provides a simple function that returns a string -MM-DD HH:MM:SS.SS that
+represents the current date and time in ISO 8601 format. This is a very simple
+routine with minimal code and there are no dependencies on any other unit.
+
@node GNAT.Traceback (g-traceb.ads)
@section @code{GNAT.Traceback} (@file{g-traceb.ads})
@cindex @code{GNAT.Traceback} (@file{g-traceb.ads})
@@ -22469,7 +22474,7 @@
@include fdl.texi
@c GNU Free Documentation License
-@node Index,,GNU Free Documentation License, Top
+@node Concept Index
@unnumbered Index
@printindex cp
@@ -22477,5 +22482,3 @@
@contents
@bye
-tablishes the following set of restrictions:
-Pragma Shared
Re: [DOC Patch] gnat updates for makeinfo 5.2 - updated
I changed more than I should have. A slightly smaller patch.
dw
On 7/14/2014 12:31 AM, David Wohlferd wrote:
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
gnat docs have warning messages when compiled with makeinfo 5.2. Most
of them revolve around the fact that nodes must be defined in the same
order as they appear in menus.
ChangeLog:
2014-07-14 David Wohlferd d...@limegreensocks.com
* ada/gnat_rm.texi: Various cleanups for makeinfo 5.2.
dw
Index: gnat_rm.texi
===
--- gnat_rm.texi (revision 212420)
+++ gnat_rm.texi (working copy)
@@ -84,8 +84,9 @@
* Implementation of Ada 2012 Features::
* Obsolescent Features::
* GNU Free Documentation License::
-* Index::
+* Index:Concept Index.
+@detailmenu
--- The Detailed Node Listing ---
About This Guide
@@ -110,8 +111,8 @@
* Pragma Assertion_Policy::
* Pragma Assume::
* Pragma Assume_No_Invalid_Values::
+* Pragma Ast_Entry::
* Pragma Attribute_Definition::
-* Pragma Ast_Entry::
* Pragma C_Pass_By_Copy::
* Pragma Check::
* Pragma Check_Float_Overflow::
@@ -677,6 +678,7 @@
GNU Free Documentation License
Index
+@end detailmenu
@end menu
@end ifnottex
@@ -939,8 +941,8 @@
* Pragma Assertion_Policy::
* Pragma Assume::
* Pragma Assume_No_Invalid_Values::
+* Pragma Ast_Entry::
* Pragma Attribute_Definition::
-* Pragma Ast_Entry::
* Pragma C_Pass_By_Copy::
* Pragma Check::
* Pragma Check_Float_Overflow::
@@ -18826,19 +18828,6 @@
@cindex Time
@cindex @code{GNAT.Calendar.Time_IO} (@file{g-catiio.ads})
-@node GNAT.CRC32 (g-crc32.ads)
-@section @code{GNAT.CRC32} (@file{g-crc32.ads})
-@cindex @code{GNAT.CRC32} (@file{g-crc32.ads})
-@cindex CRC32
-@cindex Cyclic Redundancy Check
-
-@noindent
-This package implements the CRC-32 algorithm. For a full description
-of this algorithm see
-``Computation of Cyclic Redundancy Checks via Table Look-Up'',
-@cite{Communications of the ACM}, Vol.@: 31 No.@: 8, pp.@: 1008-1013,
-Aug.@: 1988. Sarwate, D.V@.
-
@node GNAT.Case_Util (g-casuti.ads)
@section @code{GNAT.Case_Util} (@file{g-casuti.ads})
@cindex @code{GNAT.Case_Util} (@file{g-casuti.ads})
@@ -18913,6 +18902,19 @@
@noindent
Provides a simple interface to handle Ctrl-C keyboard events.
+@node GNAT.CRC32 (g-crc32.ads)
+@section @code{GNAT.CRC32} (@file{g-crc32.ads})
+@cindex @code{GNAT.CRC32} (@file{g-crc32.ads})
+@cindex CRC32
+@cindex Cyclic Redundancy Check
+
+@noindent
+This package implements the CRC-32 algorithm. For a full description
+of this algorithm see
+``Computation of Cyclic Redundancy Checks via Table Look-Up'',
+@cite{Communications of the ACM}, Vol.@: 31 No.@: 8, pp.@: 1008-1013,
+Aug.@: 1988. Sarwate, D.V@.
+
@node GNAT.Current_Exception (g-curexc.ads)
@section @code{GNAT.Current_Exception} (@file{g-curexc.ads})
@cindex @code{GNAT.Current_Exception} (@file{g-curexc.ads})
@@ -19558,17 +19560,6 @@
single global task lock. Appropriate for use in situations where contention
between tasks is very rarely expected.
-@node GNAT.Time_Stamp (g-timsta.ads)
-@section @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
-@cindex @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
-@cindex Time stamp
-@cindex Current time
-
-@noindent
-Provides a simple function that returns a string -MM-DD HH:MM:SS.SS that
-represents the current date and time in ISO 8601 format. This is a very simple
-routine with minimal code and there are no dependencies on any other unit.
-
@node GNAT.Threads (g-thread.ads)
@section @code{GNAT.Threads} (@file{g-thread.ads})
@cindex @code{GNAT.Threads} (@file{g-thread.ads})
@@ -19581,6 +19572,17 @@
further details if your program has threads that are created by a non-Ada
environment which then accesses Ada code.
+@node GNAT.Time_Stamp (g-timsta.ads)
+@section @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
+@cindex @code{GNAT.Time_Stamp} (@file{g-timsta.ads})
+@cindex Time stamp
+@cindex Current time
+
+@noindent
+Provides a simple function that returns a string -MM-DD HH:MM:SS.SS that
+represents the current date and time in ISO 8601 format. This is a very simple
+routine with minimal code and there are no dependencies on any other unit.
+
@node GNAT.Traceback (g-traceb.ads)
@section @code{GNAT.Traceback} (@file{g-traceb.ads})
@cindex @code{GNAT.Traceback} (@file{g-traceb.ads})
@@ -22469,7 +22471,7 @@
@include fdl.texi
@c GNU Free Documentation License
-@node Index,,GNU Free Documentation License, Top
+@node Concept Index
@unnumbered Index
@printindex cp
@@ -22477,5 +22479,3 @@
@contents
@bye
-tablishes the following set of restrictions:
-Pragma Shared
Re: [Patch] PR 61662
Doh! Since the component here is 'Target', I probably should have included the x86-64 Port Maintainer in the TO line. Jan, while I have a release on file with the FSF, I don't have SVN write access. dw On 7/9/2014 12:51 AM, David Wohlferd wrote: As requested, I am posting this patch to gcc-patches. Problem description: The detailed description and examples can be found in pr61662, but in short: using #ifdef __x86_64__ to determine the size of a 'long' does not reliably yield the correct result. This causes _lrotl and _lrotr to return incorrect results on LLP64 systems (like Windows). ChangeLog: 2014-07-09 David Wohlferd d...@limegreensocks.com PR target/61662 * config/i386/ia32intrin.h: Use __LP64__ to determine size of long dw
[Patch] PR 61662
As requested, I am posting this patch to gcc-patches. Problem description: The detailed description and examples can be found in pr61662, but in short: using #ifdef __x86_64__ to determine the size of a 'long' does not reliably yield the correct result. This causes _lrotl and _lrotr to return incorrect results on LLP64 systems (like Windows). ChangeLog: 2014-07-09 David Wohlferd d...@limegreensocks.com PR target/61662 * config/i386/ia32intrin.h: Use __LP64__ to determine size of long dw Index: ia32intrin.h === --- ia32intrin.h (revision 212190) +++ ia32intrin.h (working copy) @@ -256,11 +256,7 @@ #define _bswap64(a) __bswapq(a) #define _popcnt64(a) __popcntq(a) -#define _lrotl(a,b) __rolq((a), (b)) -#define _lrotr(a,b) __rorq((a), (b)) #else -#define _lrotl(a,b) __rold((a), (b)) -#define _lrotr(a,b) __rord((a), (b)) /* Read flags register */ extern __inline unsigned int @@ -280,6 +276,15 @@ #endif +/* on LP64 systems, longs are 64bits. Use the appropriate rotate function */ +#ifdef __LP64__ +#define _lrotl(a,b) __rolq((a), (b)) +#define _lrotr(a,b) __rorq((a), (b)) +#else +#define _lrotl(a,b) __rold((a), (b)) +#define _lrotr(a,b) __rord((a), (b)) +#endif + #define _bit_scan_forward(a) __bsfd(a) #define _bit_scan_reverse(a) __bsrd(a) #define _bswap(a) __bswapd(a)
[DOC Patch] Explicit Register Variables
I don't have permissions to commit this patch, but I do have a release
on file with the FSF.
Problem description:
The text for using Explicit Register Variables is confusing, redundant,
and fails to make certain essential information clear.
Some specific problems:
- There is no reason to call this topic Explicit Reg Vars instead of
Explicit Register Variables.
- Putting text on the Explicit Register Variables menu page (instead
of the Global or Local subpages) is redundant, since any text that
describes the usage of Global or Local register variables will have to
be repeated on the individual subpages.
- Vague promises of features that may some day be available are not
useful in documentation (Eventually there may be a way of asking the
compiler to).
- Vague descriptions of things that are reported to work on certain
platforms are not useful (On the SPARC, there are reports that).
- Describing Local Register Variables as sometimes convenient for use
with the extended asm feature misses the point that this is in fact the
ONLY supported use for Local Register Variables.
- Unambiguous statements such as The following uses are explicitly
/not/ supported when describing things such as calling Basic asm
discourage people from attempting to misuse the feature.
ChangeLog:
2014-06-30 David Wohlferd d...@limegreensocks.com
* doc/extend.texi (Explicit Reg Vars): Rewrite and clarify.
* doc/implement-c.texi (Hints implementation): Use new name.
dw
Index: extend.texi
===
--- extend.texi (revision 210997)
+++ extend.texi (working copy)
@@ -6234,7 +6234,8 @@
* Extended Asm:: Inline assembler with operands.
* Constraints::Constraints for @code{asm} operands
* Asm Labels:: Specifying the assembler name to use for a C symbol.
-* Explicit Reg Vars:: Defining variables residing in specified registers.
+* Explicit Register Variables:: Defining variables residing in specified
+ registers.
* Size of an asm:: How GCC calculates the size of an @code{asm} block.
@end menu
@@ -6723,8 +6724,8 @@
list as many alternates as the @code{asm} statement allows, you will permit
the optimizers to produce the best possible code. If you must use a specific
register, but your Machine Constraints do not provide sufficient
-control to select the specific register you want, Local Reg Vars may provide
-a solution (@pxref{Local Reg Vars}).
+control to select the specific register you want, Local Register Variables
+may provide a solution (@pxref{Local Register Variables}).
@emph{cvariablename}
@*
@@ -6865,8 +6866,8 @@
method based on the current context. Input constraints may not begin with
either = or +. If you must use a specific register, but your Machine
Constraints do not provide sufficient control to select the specific
-register you want, Local Reg Vars may provide a solution
-(@pxref{Local Reg Vars}).
+register you want, Local Register Variables may provide a solution
+(@pxref{Local Register Variables}).
Input constraints can also be digits (for example, @code{0}). This indicates
that the specified input will be in the same place as the output constraint
@@ -6952,7 +6953,8 @@
Clobber descriptions may not in any way overlap with an input or output
operand. For example, you may not have an operand describing a register class
with one member when listing that register in the clobber list. Variables
-declared to live in specific registers (@pxref{Explicit Reg Vars}), and used
+declared to live in specific registers (@pxref{Explicit Register Variables}),
+and used
as @code{asm} input or output operands, must have no part mentioned in the
clobber description. In particular, there is no way to specify that input
operands get modified without also specifying them as output operands.
@@ -7290,7 +7292,7 @@
It does not make sense to use this feature with a non-static local
variable since such variables do not have assembler names. If you are
trying to put the variable in a particular register, see @ref{Explicit
-Reg Vars}. GCC presently accepts such code with a warning, but will
+Register Variables}. GCC presently accepts such code with a warning, but will
probably be changed to issue an error, rather than a warning, in the
future.
@@ -7312,197 +7314,128 @@
does not as yet have the ability to store static variables in registers.
Perhaps that will be added.
-@node Explicit Reg Vars
+@node Explicit Register Variables
@subsection Variables in Specified Registers
@cindex explicit register variables
@cindex variables in specified registers
@cindex specified registers
-@cindex registers, global allocation
-GNU C allows you to put a few global variables into specified hardware
-registers. You can also specify the register in which an ordinary
-register variable should be allocated.
+GNU C allows you to associate specific hardware registers
Re: [DOC Patch] Explicit Register Variables
On 6/30/2014 2:01 PM, Jeff Law wrote: On 06/30/14 02:18, David Wohlferd wrote: I don't have permissions to commit this patch, but I do have a release on file with the FSF. Problem description: The text for using Explicit Register Variables is confusing, redundant, and fails to make certain essential information clear. Some specific problems: - There is no reason to call this topic Explicit Reg Vars instead of Explicit Register Variables. Agreed. - Putting text on the Explicit Register Variables menu page (instead of the Global or Local subpages) is redundant, since any text that describes the usage of Global or Local register variables will have to be repeated on the individual subpages. OK. - Vague promises of features that may some day be available are not useful in documentation (Eventually there may be a way of asking the compiler to). Can't disagree here. I believe this lame comment has been around for 20 years or longer at this point. - Vague descriptions of things that are reported to work on certain platforms are not useful (On the SPARC, there are reports that). I'd disagree. But what's more important here is the registers that are available are a function of the ABI and for someone to attempt to use this feature, they're going to have to be intimately aware of the ABI of their target. If we could say On the SPARC, these registers can be used for I'd be tempted to leave it as an example. But saying Well, someone once said they thought it might work this way on this one specific platform is not helpful for either SPARC or non-SPARC users. - Describing Local Register Variables as sometimes convenient for use with the extended asm feature misses the point that this is in fact the ONLY supported use for Local Register Variables. What makes you believe that this feature is only useful for extended asms? My recollection is that for the last 20+ years this feature has served effectively as a pre-allocation of certain function scoped objects into predetermined registers. That's great feedback. The existing docs don't even mention that use. If you have a link (or if you can put together a few sentences) describing how you could use it for this, I'll add it and re-work the text. And if you can think of any other common uses, I'd be glad to add them too. - Unambiguous statements such as The following uses are explicitly /not/ supported when describing things such as calling Basic asm discourage people from attempting to misuse the feature. Not sure I really agree with this either. There's nothing inherently wrong with someone trying to use a local register variable to try and optimize code for example. Umm. Well. Hmm. You haven't said this was a good idea or even that it would work, so I guess the statement that there's nothing inherently wrong with it is true. However, by explicitly stating it is not supported, we would be making it clear that when the next release uses registers in a slightly different way that wipes out all their improvements, we can say we told you that wasn't a supported use of this feature. However, I'm prepared to remove or rework this statement if you feel strongly. ChangeLog: 2014-06-30 David Wohlferd d...@limegreensocks.com * doc/extend.texi (Explicit Reg Vars): Rewrite and clarify. * doc/implement-c.texi (Hints implementation): Use new name. So in summary, I agree with most of what you've done. The question is how to proceed at this point. We could extract the non-controversial stuff and install it now and iterate on the stuff still in the air until it's done. Or we could iterate on the stuff still in the air and install the final update as an single commit. I don't care much either way, do you have a preference? My preference would be to only do one checkin when we've got it right. I *almost* sent this as an RFD to the gcc list. But it being such an obscure area, I wasn't really expecting much response (see https://gcc.gnu.org/ml/gcc-help/2014-04/msg00124.html for example). The items listed in my problem description were only highlights. As you have no doubt noticed, nearly all the text on these 2 pages got modified. If you have feedback on any of the rest of the text, I'd love to hear it. dw
Re: [DOC Patch] Explicit Register Variables
On 6/30/2014 6:30 PM, Mike Stump wrote: On Jun 30, 2014, at 4:10 PM, David Wohlferd d...@limegreensocks.com wrote: - Vague descriptions of things that are reported to work on certain platforms are not useful (On the SPARC, there are reports that). I'd disagree. But what's more important here is the registers that are available are a function of the ABI and for someone to attempt to use this feature, they're going to have to be intimately aware of the ABI of their target. If we could say On the SPARC, these registers can be used for I'd be tempted to leave it as an example. But saying Well, someone once said they thought it might work this way on this one specific platform is not helpful for either SPARC or non-SPARC users. So, we can do s/there are reports that//; s/should be suitable, as should/are suitable, as are/ and remove the wishy washy language. If someone later wants to correct the definitive statement in some way, they are welcome to it. We could. While an example can be illustrative, the potential value here is offset by the fact that it may not actually be true. Given that, I think I'd just as soon do without an example. My proposed patch says When selecting a register, choose one that is normally saved and restored by function calls on your machine. This ensures library routines which are unaware of this reservation will restore it before returning. This platform-neutral statement seems to cover what needs to be said here. dw
[DOC Patch] Attribute 'naked'
I don't have permissions to commit this patch, but I do have a release
on file with the FSF.
Problem description:
The docs for the function attribute 'naked' are confusing and
self-contradictory. Also, discussion on this thread
https://gcc.gnu.org/ml/gcc/2014-05/msg00100.html has lead to changing
the text from the vague avoid using to the very clear not supported
regarding the usage of Extended asm with 'naked.' Lastly, this
attribute should be mentioned when describing the differences between
Basic and Extended asm.
ChangeLog:
2014-06-17 David Wohlferd d...@limegreensocks.com
* doc/extend.texi (Function Attributes): Update 'naked'
attribute doc.
dw
Index: extend.texi
===
--- extend.texi (revision 210624)
+++ extend.texi (working copy)
@@ -3332,16 +3332,15 @@
@item naked
@cindex function without a prologue/epilogue code
-Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX and SPU
-ports to indicate that the specified function does not need prologue/epilogue
-sequences generated by the compiler.
-It is up to the programmer to provide these sequences. The
-only statements that can be safely included in naked functions are
-@code{asm} statements that do not have operands. All other statements,
-including declarations of local variables, @code{if} statements, and so
-forth, should be avoided. Naked functions should be used to implement the
-body of an assembly function, while allowing the compiler to construct
-the requisite function declaration for the assembler.
+This attribute is available on the ARM, AVR, MCORE, MSP430, NDS32,
+RL78, RX and SPU ports. It allows the compiler to construct the
+requisite function declaration, while allowing the body of the
+function to be assembly code. The specified function will not have
+prologue/epilogue sequences generated by the compiler. Only Basic
+@code{asm} statements can safely be included in naked functions
+(@pxref{Basic Asm}). While using Extended @code{asm} or a mixture of
+Basic @code{asm} and ``C'' code may appear to work, they cannot be
+depended upon to work reliably and are not supported.
@item near
@cindex functions that do not handle memory bank switching on 68HC11/68HC12
@@ -6269,6 +6268,8 @@
efficient code, and in most cases it is a better solution. When writing
inline assembly language outside of C functions, however, you must use Basic
@code{asm}. Extended @code{asm} statements have to be inside a C function.
+Functions declared with the @code{naked} attribute also require Basic
+@code{asm} (@pxref{Function Attributes}).
Under certain circumstances, GCC may duplicate (or remove duplicates of) your
assembly code when optimizing. This can lead to unexpected duplicate
@@ -6388,6 +6389,8 @@
Note that Extended @code{asm} statements must be inside a function. Only
Basic @code{asm} may be outside functions (@pxref{Basic Asm}).
+Functions declared with the @code{naked} attribute also require Basic
+@code{asm} (@pxref{Function Attributes}).
While the uses of @code{asm} are many and varied, it may help to think of an
@code{asm} statement as a series of low-level instructions that convert input
Re: [patch i386]: Combine memory and indirect jump
On 6/12/2014 9:21 AM, Kai Tietz wrote: with addition of adding a second peephole2 pass after sched2 pass, I was able to get some improvement for PR target/39284. I think by this addition we can close bug as fixed. Additionally additional peephole2 pass shows better results for PR target/51840 testcase with disabled ASM_GOTO, too. Any chance this also fixes PR 58670 (see comment #5)? dw
[DOC Patch] Label attributes
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
The docs in (Attribute Syntax) say The only attribute it makes sense to
use after a label is 'unused'. However, there are two others: hot and
cold. The reason it looks like there is only one is that the docs for
the label attribute versions of hot and cold are misfiled under the
(Function Attributes) section. When there was only one label attribute,
it (sort of) made sense not to have a separate section for label
attributes. Now that there are three, not so much.
ChangeLog:
2014-05-18 David Wohlferd d...@limegreensocks.com
* doc/extend.texi: Create Label Attributes section,
move all label attributes into it and reference it.
dw
Index: extend.texi
===
--- extend.texi (revision 210577)
+++ extend.texi (working copy)
@@ -55,6 +55,7 @@
* Mixed Declarations:: Mixing declarations and code.
* Function Attributes:: Declaring that functions have no side effects,
or that they can never return.
+* Label Attributes::Specifying attributes on labels.
* Attribute Syntax::Formal syntax for attributes.
* Function Prototypes:: Prototype declarations and old-style definitions.
* C++ Comments::C++ comments are recognized.
@@ -2181,7 +2182,8 @@
@code{error} and @code{warning}.
Several other attributes are defined for functions on particular
target systems. Other attributes, including @code{section} are
-supported for variables declarations (@pxref{Variable Attributes})
+supported for variables declarations (@pxref{Variable Attributes}),
+labels (@pxref{Label Attributes})
and for types (@pxref{Type Attributes}).
GCC plugins may provide their own attributes.
@@ -3617,8 +3619,8 @@
@cindex @code{hot} function attribute
The @code{hot} attribute on a function is used to inform the compiler that
the function is a hot spot of the compiled program. The function is
-optimized more aggressively and on many target it is placed into special
-subsection of the text section so all hot functions appears close together
+optimized more aggressively and on many targets it is placed into a special
+subsection of the text section so all hot functions appear close together,
improving locality.
When profile feedback is available, via @option{-fprofile-use}, hot functions
@@ -3627,23 +3629,14 @@
The @code{hot} attribute on functions is not implemented in GCC versions
earlier than 4.3.
-@cindex @code{hot} label attribute
-The @code{hot} attribute on a label is used to inform the compiler that
-path following the label are more likely than paths that are not so
-annotated. This attribute is used in cases where @code{__builtin_expect}
-cannot be used, for instance with computed goto or @code{asm goto}.
-
-The @code{hot} attribute on labels is not implemented in GCC versions
-earlier than 4.8.
-
@item cold
@cindex @code{cold} function attribute
The @code{cold} attribute on functions is used to inform the compiler that
the function is unlikely to be executed. The function is optimized for
-size rather than speed and on many targets it is placed into special
-subsection of the text section so all cold functions appears close together
+size rather than speed and on many targets it is placed into a special
+subsection of the text section so all cold functions appear close together,
improving code locality of non-cold parts of program. The paths leading
-to call of cold functions within code are marked as unlikely by the branch
+to calls of cold functions within code are marked as unlikely by the branch
prediction mechanism. It is thus useful to mark functions used to handle
unlikely conditions, such as @code{perror}, as cold to improve optimization
of hot functions that do call marked functions in rare occasions.
@@ -3654,15 +3647,6 @@
The @code{cold} attribute on functions is not implemented in GCC versions
earlier than 4.3.
-@cindex @code{cold} label attribute
-The @code{cold} attribute on labels is used to inform the compiler that
-the path following the label is unlikely to be executed. This attribute
-is used in cases where @code{__builtin_expect} cannot be used, for instance
-with computed goto or @code{asm goto}.
-
-The @code{cold} attribute on labels is not implemented in GCC versions
-earlier than 4.8.
-
@item no_sanitize_address
@itemx no_address_safety_analysis
@cindex @code{no_sanitize_address} function attribute
@@ -4527,6 +4511,65 @@
@code{#pragma GCC} is of use for constructs that do not naturally form
part of the grammar. @xref{Pragmas,,Pragmas Accepted by GCC}.
+@node Label Attributes
+@section Label Attributes
+@cindex Label Attributes
+
+GCC allows attributes to be set on C labels. @xref{Attribute Syntax}, for
+details of the exact syntax for using attributes. Other attributes are
+available for functions (@pxref{Function Attributes}), variables
+(@pxref
Re: [PATCH, doc]: Fix a bunch of warnings in *.texi files
My bad. My version of makeinfo wasn't reporting these errors.
However, this isn't right either. There are two subsections that are
now under Size of an asm that should be under Variables in Specified
Registers. How about this (attached)?
dw
On 5/17/2014 6:06 AM, Uros Bizjak wrote:
Hello!
Attached patch fixes:
md.texi:1057: warning: node next `Constraints' in menu `Asm Labels'
and in sectioning `Size of an asm' differ
extend.texi:7175: warning: node `Asm Labels' is next for `Size of an
asm' in sectioning but not in menu
extend.texi:7175: warning: node prev `Size of an asm' in menu
`Explicit Reg Vars' and in sectioning `Constraints' differ
extend.texi:7197: warning: node prev `Asm Labels' in menu
`Constraints' and in sectioning `Size of an asm' differ
extend.texi:7245: warning: node `Size of an asm' is next for `Explicit
Reg Vars' in menu but not in sectioning
as seen when compiling on Fedora 20.
2014-05-17 Uros Bizjak ubiz...@gmail.com
* doc/extend.texi (Size of an asm): Move node text according
to its @menu entry position.
Tested on x86_64-pc-linux-gnu, committed to mainline SVN.
Uros.
Index: extend.texi
===
--- extend.texi (revision 210575)
+++ extend.texi (working copy)
@@ -7267,28 +7267,6 @@
specified for that operand in the @code{asm}.)
@end itemize
-@node Size of an asm
-@subsection Size of an @code{asm}
-
-Some targets require that GCC track the size of each instruction used
-in order to generate correct code. Because the final length of the
-code produced by an @code{asm} statement is only known by the
-assembler, GCC must make an estimate as to how big it will be. It
-does this by counting the number of instructions in the pattern of the
-@code{asm} and multiplying that by the length of the longest
-instruction supported by that processor. (When working out the number
-of instructions, it assumes that any occurrence of a newline or of
-whatever statement separator character is supported by the assembler --
-typically @samp{;} --- indicates the end of an instruction.)
-
-Normally, GCC's estimate is adequate to ensure that correct
-code is generated, but it is possible to confuse the compiler if you use
-pseudo instructions or assembler macros that expand into multiple real
-instructions, or if you use assembler directives that expand to more
-space in the object file than is needed for a single instruction.
-If this happens then the assembler may produce a diagnostic saying that
-a label is unreachable.
-
@menu
* Global Reg Vars::
* Local Reg Vars::
@@ -7467,6 +7445,28 @@
asm (sysint : =r (result) : 0 (p1), r (p2));
@end smallexample
+@node Size of an asm
+@subsection Size of an @code{asm}
+
+Some targets require that GCC track the size of each instruction used
+in order to generate correct code. Because the final length of the
+code produced by an @code{asm} statement is only known by the
+assembler, GCC must make an estimate as to how big it will be. It
+does this by counting the number of instructions in the pattern of the
+@code{asm} and multiplying that by the length of the longest
+instruction supported by that processor. (When working out the number
+of instructions, it assumes that any occurrence of a newline or of
+whatever statement separator character is supported by the assembler --
+typically @samp{;} --- indicates the end of an instruction.)
+
+Normally, GCC's estimate is adequate to ensure that correct
+code is generated, but it is possible to confuse the compiler if you use
+pseudo instructions or assembler macros that expand into multiple real
+instructions, or if you use assembler directives that expand to more
+space in the object file than is needed for a single instruction.
+If this happens then the assembler may produce a diagnostic saying that
+a label is unreachable.
+
@node Alternate Keywords
@section Alternate Keywords
@cindex alternate keywords
Re: [DOC Patch] symbol rename pragmas
This patch is in response to Rainer Orth's comment (below).
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
The (already committed) patch at the top of this thread removed a
reference to Solaris. It has been requested that this be restored to
show the origins of the directive and clarify when to use it (or not).
ChangeLog:
2014-05-16 David Wohlferd d...@limegreensocks.com
* doc/extend.texi (Symbol-Renaming Pragmas): Restore (slightly
modified) reference to Solaris.
dw
keep documenting the heritage so it's clear why it
exists and when to use it (or not).
Just restoring the first sentence you deleted describing the heritage
should be enough to do so.
Speaking for myself, I didn't see the old text as favoring one course
of action over another. How would you feel about:
GCC supports a @code{#pragma} directive that changes the name used in
assembly for a given declaration. While this pragma is supported on all
platforms, it is intended primarily to provide compatibility with the
Solaris system headers. This effect can also be achieved using the asm
labels extension (@pxref{Asm Labels}).
To me, this subtly discourages using the pragma other than for
Solaris, maintains the heritage, and immediately provides a viable
alternative for other platforms.
Index: extend.texi
===
--- extend.texi (revision 210542)
+++ extend.texi (working copy)
@@ -16895,8 +16895,10 @@
@subsection Symbol-Renaming Pragmas
GCC supports a @code{#pragma} directive that changes the name used in
-assembly for a given declaration. This effect can also be achieved
-using the asm labels extension (@pxref{Asm Labels}).
+assembly for a given declaration. While this pragma is supported on all
+platforms, it is intended primarily to provide compatibility with the
+Solaris system headers. This effect can also be achieved using the asm
+labels extension (@pxref{Asm Labels}).
@table @code
@item redefine_extname @var{oldname} @var{newname}
[DOC Patch] Incorrect @xref in #pragma visibility
I have a release on file with the FSF, but don't have SVN write access.
Problem description:
The existing docs have an @xref in the middle of a sentence, resulting
in weird capitalization and sentence structure:
This pragma allows the user to set the visibility for multiple
declarations without having to give each a visibility attribute See
Function Attributes, for more information about visibility and the
attribute syntax.
ChangeLog:
2014-05-14 David Wohlferd d...@limegreensocks.com
* doc/extend.texi: (Visibility Pragmas) Fix misplaced @xref
dw
Index: extend.texi
===
--- extend.texi (revision 210349)
+++ extend.texi (working copy)
@@ -17075,8 +17075,7 @@
This pragma allows the user to set the visibility for multiple
declarations without having to give each a visibility attribute
-@xref{Function Attributes}, for more information about visibility and
-the attribute syntax.
+(@pxref{Function Attributes}).
In C++, @samp{#pragma GCC visibility} affects only namespace-scope
declarations. Class members and template specializations are not
Re: [DOC Patch] symbol rename pragmas
Thank you for taking the time to review this.
the current text makes reference to compatibility with the Solaris system
headers, the remaining pragma is (according to the existing text)
currently on all platforms. This makes referring to Solaris both
superfluous and potentially confusing.
I disagree: #pragma redefine_extname does exist for Solaris
compatibility only, even if it now works on all platforms. So please
continue to state so.
Hmm. Looking at the original text:
For compatibility with the Solaris system headers, GCC supports two
#pragma directives that change the name used in assembly for a given
declaration.
As I read that, I only saw the motivation for why this pragma was
added. Even now I don't read this as *restricting* these pragmas to
Solaris. Especially when you combine that with:
- The text below which states that redefine_extname is currently on all
platforms.
- This pragma isn't under the Solaris Pragmas headings, but under its
own (apparently platform-neutral) heading.
- It does in fact work on other platforms. Even the
__PRAGMA_REDEFINE_EXTNAME macro is there.
Considering all these points, someone could easily conclude this was not
Solaris-specific. While that may not have been the intent when this
text was written, that's how I read it, and others may have too. Are
you sure you want me to make this change?
I'm not trying to pick a fight here. I'm not using the pragma, so this
doesn't affect me. It's just that this has been out there for a long
time, affects platforms other than Solaris, and I'm not immediately
clear on what problems supporting other platforms creates.||
In the end I guess I'd just like to be sure some other OS Port
Maintainer (or some distraught user) isn't going to be emailing me
demanding to know why I dropped support for this from their platform.
That said.
If this does need to change, I'd recommend something like the attached.
Note that since the previous patch was checked in, this patch updates
from that point. It deletes the Symbol-Renaming Pragmas node, and
moves (most of) the redefine_extname text into the Solaris Pragmas node.
If reading the patch doesn't make it clear what I've done, you can see
the end result here: http://www.LimeGreenSocks.com/gcc/Solaris-Pragmas.html
I believe this makes it crystal clear that the only platform where gcc
supports this pragma is Solaris. If you approve (and I don't hear from
some other Port Maintainer or distraught user), I'll submit this as a
new patch. Be aware, I plan to put your name in the changelog.
dw
Index: extend.texi
===
--- extend.texi (revision 210355)
+++ extend.texi (working copy)
@@ -16657,7 +16657,6 @@
* RS/6000 and PowerPC Pragmas::
* Darwin Pragmas::
* Solaris Pragmas::
-* Symbol-Renaming Pragmas::
* Structure-Packing Pragmas::
* Weak Pragmas::
* Diagnostic Pragmas::
@@ -16851,8 +16850,7 @@
@node Solaris Pragmas
@subsection Solaris Pragmas
-The Solaris target supports @code{#pragma redefine_extname}
-(@pxref{Symbol-Renaming Pragmas}). It also supports additional
+The Solaris target supports additional
@code{#pragma} directives for compatibility with the system compiler.
@table @code
@@ -16880,23 +16878,13 @@
initialization (before @code{main}) or during shared module loading, by
adding a call to the @code{.init} section.
-@end table
-
-@node Symbol-Renaming Pragmas
-@subsection Symbol-Renaming Pragmas
-
-GCC supports a @code{#pragma} directive that changes the name used in
-assembly for a given declaration. This effect can also be achieved
-using the asm labels extension (@pxref{Asm Labels}).
-
-@table @code
@item redefine_extname @var{oldname} @var{newname}
@cindex pragma, redefine_extname
This pragma gives the C function @var{oldname} the assembly symbol
@var{newname}. The preprocessor macro @code{__PRAGMA_REDEFINE_EXTNAME}
-is defined if this pragma is available (currently on all platforms).
-@end table
+is defined if this pragma is available. This effect can also be achieved
+using the asm labels extension (@pxref{Asm Labels}).
This pragma and the asm labels extension interact in a complicated
manner. Here are some corner cases you may want to be aware of:
@@ -16919,6 +16907,8 @@
always the C-language name.
@end enumerate
+@end table
+
@node Structure-Packing Pragmas
@subsection Structure-Packing Pragmas
Re: [DOC Patch] symbol rename pragmas
You misunderstood completely: I'm not at all demanding to remove support
for that pragma in the cross-platform part of the docs or even in the
code,
Whew! Ok, that's good. Sorry I went all crazy on you. When I thought
that *was* what you were asking for, I wanted to try to talk you out of it.
just ask to keep documenting the heritage so it's clear why it
exists and when to use it (or not).
Ok, I get this. I've written text that discouraged using things in
the past, even though technically they work just fine.
Just restoring the first sentence you deleted describing the heritage
should be enough to do so.
Speaking for myself, I didn't see the old text as favoring one course of
action over another. How would you feel about:
GCC supports a @code{#pragma} directive that changes the name used in
assembly for a given declaration. While this pragma is supported on all
platforms, it is intended primarily to provide compatibility with the
Solaris system headers. This effect can also be achieved using the asm
labels extension (@pxref{Asm Labels}).
To me, this subtly discourages using the pragma other than for Solaris,
maintains the heritage, and immediately provides a viable alternative
for other platforms.
dw
[DOC Patch] symbol rename pragmas
I don't have permissions to commit this patch, but I do have a release
on file with the FSF.
Problem description:
The existing docs make reference to the two #pragma directives that
change the name used in assembly. However, the subsequent text only
describes one. It turns out, #pragma extern_prefix was removed in
revision 185240 (March 2012), as part of Remove obsolete Tru64 UNIX
V5.1B support, but the surrounding text wasn't updated to reflect the
new count. Also, while the current text makes reference to
compatibility with the Solaris system headers, the remaining pragma is
(according to the existing text) currently on all platforms. This
makes referring to Solaris both superfluous and potentially confusing.
ChangeLog:
2014-05-12 David Wohlferd d...@limegreensocks.com
* doc/extend.texi: Reflect current pragma count and remove
Solaris.
dw
Index: extend.texi
===
--- extend.texi (revision 210298)
+++ extend.texi (working copy)
@@ -16885,11 +16885,9 @@
@node Symbol-Renaming Pragmas
@subsection Symbol-Renaming Pragmas
-For compatibility with the Solaris system headers, GCC
-supports two @code{#pragma} directives that change the name used in
-assembly for a given declaration. To get this effect
-on all platforms supported by GCC, use the asm labels extension (@pxref{Asm
-Labels}).
+GCC supports a @code{#pragma} directive that changes the name used in
+assembly for a given declaration. This effect can also be achieved
+using the asm labels extension (@pxref{Asm Labels}).
@table @code
@item redefine_extname @var{oldname} @var{newname}
@@ -16901,17 +16899,17 @@
@end table
This pragma and the asm labels extension interact in a complicated
-manner. Here are some corner cases you may want to be aware of.
+manner. Here are some corner cases you may want to be aware of:
@enumerate
-@item Both pragmas silently apply only to declarations with external
+@item This pragma silently applies only to declarations with external
linkage. Asm labels do not have this restriction.
-@item In C++, both pragmas silently apply only to declarations with
+@item In C++, this pragma silently applies only to declarations with
``C'' linkage. Again, asm labels do not have this restriction.
-@item If any of the three ways of changing the assembly name of a
-declaration is applied to a declaration whose assembly name has
+@item If either of the ways of changing the assembly name of a
+declaration are applied to a declaration whose assembly name has
already been determined (either by a previous use of one of these
features, or because the compiler needed the assembly name in order to
generate code), and the new name is different, a warning issues and
[DOC Patch] Remove reference to deleted macro
I don't have permissions to commit this patch, but I do have a release on file with the FSF. Problem description: The existing docs make reference to a macro which is described below. However, this is the final sentence in the section; there is no below. Turns out the macro was deleted a long time ago (June 2012) in revision 188983 (pr33190), leaving this perplexing sentence behind. ChangeLog: 2014-05-11 David Wohlferd d...@limegreensocks.com * doc/tm.texi: Remove reference to deleted macro. * doc/tm.texi.in: Likewise. dw Index: tm.texi === --- tm.texi (revision 210298) +++ tm.texi (working copy) @@ -10067,9 +10067,6 @@ the FPU@. One accesses COP1 registers through standard mips floating-point support; they are not included in this mechanism. -There is one macro used in defining the MIPS coprocessor interface which -you may want to override in subtargets; it is described below. - @node PCH Target @section Parameters for Precompiled Header Validity Checking @cindex parameters, precompiled headers Index: tm.texi.in === --- tm.texi.in (revision 210298) +++ tm.texi.in (working copy) @@ -7565,9 +7565,6 @@ the FPU@. One accesses COP1 registers through standard mips floating-point support; they are not included in this mechanism. -There is one macro used in defining the MIPS coprocessor interface which -you may want to override in subtargets; it is described below. - @node PCH Target @section Parameters for Precompiled Header Validity Checking @cindex parameters, precompiled headers
