On 11/24/2015 8:58 AM, paul_kon...@dell.com wrote:
On Nov 23, 2015, at 8:39 PM, David Wohlferd <d...@limegreensocks.com> wrote:

On 11/23/2015 1:44 PM, paul_kon...@dell.com wrote:
On Nov 23, 2015, at 4:36 PM, David Wohlferd <d...@limegreensocks.com> wrote:

...
The more I think about it, I'm just not keen on forcing all those old-style 
asms to change.
If you mean you aren't keen to change them to "clobber all," I'm with you.  If 
you are worried about changing them from basic to extended, what kinds of problems do you 
foresee?  I've been reading a lot of basic asm lately, and it seems to me that most of it 
would be fine with a simple colon.  Certainly no worse than the current behavior.
I'm not sure.  I have some asm("sync") which I think assume that this means 
asm("sync"::"memory")
Another excellent reason to nudge people towards using extended asm.  If you saw 
asm("sync":::"memory"), you would *know* what it did, without having to read 
the docs (which don't say anyway).

I'm pretty confident that asm("") doesn't clobber memory on i386, but maybe that behavior 
is platform-specific.  Since i386 doesn't have "sync", I assume you are on something else?
Yes, MIPS.
If you have a chance to experiment, I'd love confirmation from other platforms that 
asm("blah") is the same as asm("blah":).  Feel free to email me off list to 
discuss.
I'm really concerned with loosening the meaning of basic asm.  I wish I could 
find the documentation that says, or implies, that it is a memory clobber.  
And/or that it is implicitly volatile.

The problem is that it's clear from existing code that this assumption was made, and that 
defining it otherwise would break such code.  For example, the code I quoted clearly 
won't work if stores are moved across the asm("sync").

Given the ever improving optimizers, these things are time bombs -- code that 
worked for years might suddenly break when the compiler is upgraded.

If such breakage is to be done, it must at least come with a warning (which 
must default to ON).  But I'd prefer to see the more conservative approach 
(more clobbers) taken.

It looks like Ian has already addressed most of your concerns. Just to emphasize one point:

The current behavior of basic asm is to NOT clobber memory. So your existing code that performs asm("sync") is already at risk.

The 'fix' I am proposing is to give warnings for every use of basic asm inside functions (top-level asm is not a problem). Users should change all such code to use extended so that they can (must) explicitly specify what their asm clobbers (if anything). Armed with this information, the optimizers can do their work safely. And people maintaining the code can finally be clear about what the asm really does.

And the code change should be simple. They can get the same "clobber nothing" behavior that basic asm has always performed by simply adding a colon on the end ( asm("sync":) ). Or they can use any of extended asm's features to get different behavior ( asm("sync":::"memory") ).

Or to put that another way, correctly written basic asm can be converted to extended by just adding a colon. Incorrect code may require a bit more work.

dw

Reply via email to