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
