On Wed, Aug 19, 2015 at 02:08:16PM -0700, David Wohlferd wrote: > >>My intent here is to break this clearly into two @subsubheadings: > >>'Assembler names for data' and 'Assembler names for functions'. Since > >>data is the first section, I removed the word 'function' here. > >I missed that, sorry. Or, did you forget to add the same text to the > >"function" description? > > > >This patch would be much easier to review if you did one change per > >patch. > > I'm not sure what 'one change' might mean in this context.
You're moving various things around. You are removing some, too. It is hard (harder than necessary) to figure out which is which. > >>>> 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 > >>>>-probably be changed to issue an error, rather than a warning, in the > >>>>-future. > >>>>+Reg Vars}. > >>>And this? > >>Vague statements about possible changes that may or not ever be written > >>are not helpful in docs. In this case the statement is particularly > >>unhelpful since even the warning appears to be gone. > >I don't agree it is a vague statement about possible future changes; it > >is more like a statement of intent. > > Saying this will "probably" change at some (unspecified) time "in the > future" seems the very definition of vague. Especially when you > consider that the docs have been threatening this change for over 15 > years now. Certainly. But just deleting it is losing information. > >It tells the reader "don't write code like this". > > If this is intended just for emphasis, That is how I read it, anyway. > how about replacing the existing > text ("It does not make sense to use this feature with a non-static > local variable since such variables do not have assembler names.") with > "Do not use this feature with a non-static local variable." or maybe "It > is not supported to use this feature with a non-static local variable > since such variables do not have assembler names." "You cannot use this feature ..." etc.? Keep the part about not having assembler names, it is useful. > Saying "It does not make sense" to do something doesn't mean the same > thing (to me) as "Do not do this" or "It is not supported to..." It is not the style of the manual, for sure. > >And the warning is still there ("ignoring asm-specifier for non-static > >local variable"). > > Huh. I was unable to get gcc to produce this warning. Is there a trick? See below... > >>>>-Also, you must not use a > >>>>-register name; that would produce completely invalid assembler > >>>>code. GCC > >>>>-does not as yet have the ability to store static variables in > >>>>registers. > >>>>-Perhaps that will be added. > >>>And why remove these? > >>Again with the vague statements about possible future changes. Also, > >>whether or not static variables can be stored in registers has nothing > >>to do with Asm Labels. If this is still true, it belongs in Explicit > >>Reg Vars. > >The first part ("must not use a register name") is an important warning. > > Clarifying this is a good idea. Although limiting it to only saying > "don't use register names" seems a little, well, limiting. Who knows > what kind of offsets or asm qualifiers they might try to cram in here? Register names is the common case to hurt you... "r0" etc. ;-) > How about: > > "Only label names valid for your assembler are permitted within the asm." The assembler calls it "symbol names" (labels end in a colon). But it won't help: e.g. "r0" is a perfectly fine name for the assembler, too! It should say something like "if the string you put here is seen as something else than a symbol name by the assembler, anywhere the compiler puts it, you're on your own", but that is pretty vague as well. > >The second part (about statics) might well be better moved, but it should > >not be _re_moved just like that! And it is still true (gives an error, > >"multiple storage classes"). > > I have already started work on the changes I think are needed for > Explicit Reg Vars (the last section of gcc docs I'm planning on doing). > But I want to finish the (relatively easy) Asm Labels stuff first. > Should I put this change in there? Or would someone just tell me all > the Asm Labels stuff should go in its own patch? > > Also, I'm not seeing the multiple storage classes message either: > > int main() > { > static int a asm("asdf"); > a = 5; > return a; > } You forgot to make it "register", so it is not a register variable. Try this, for both the warning and error (tested with 4.7, 4.9, 5, 6): --- 8< --- int f(void) { static register int lol asm("r20"); int foo asm("myfoo") = 42; return foo + lol; } --- 8< --- fooasm.c: In function 'f': fooasm.c:4:2: error: multiple storage classes in declaration specifiers static register int lol asm("r20"); ^ fooasm.c:5:6: warning: ignoring asm-specifier for non-static local variable 'foo' int foo asm("myfoo") = 42; ^ Segher