Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Tue, 1 May 2018, Jeff Law wrote: > The very nature of a traditional asm implies that it can read or write > anything visible to compiler. No? Basic asms are not allowed to read/write general purpose registers, the stack pointer, the instruction pointer. I think they are considered to be wild memory reads/writes, but register variables are explicitly separate storage class from memory. I see Michael further clarified the issue is not mainly about basic asms, and I think you indicated agreement, so is patch 2/2 OK to apply? Thanks. Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On 05/02/2018 07:45 AM, Michael Matz wrote: > Hi, > > On Tue, 1 May 2018, Jeff Law wrote: > >> The very nature of a traditional asm implies that it can read or write >> anything visible to compiler. We can't realistically peek inside to see >> what's happening and the user hasn't provided appropriate dataflow >> information. One could make the argument that traditional asms should >> go the way of the dodo bird. I think we looked at that not terribly >> long ago, but didn't really get anywhere. > > Yeah, with traditional asms we have to assume dependencies to everything > (right now, i.e. if we don't want to enter the rathole of deprecating > them). But we were also talking about non-traditional asms, i.e. with > operands, where I think Alexander made the case that those should have > explicit dependencies if they access the same registers as global reg > vars. For a non-traditional ASM ISTM that they should provide the dependency, even for global register variables. So I'd agree with Alexander on that. Jeff
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi, On Tue, 1 May 2018, Jeff Law wrote: > The very nature of a traditional asm implies that it can read or write > anything visible to compiler. We can't realistically peek inside to see > what's happening and the user hasn't provided appropriate dataflow > information. One could make the argument that traditional asms should > go the way of the dodo bird. I think we looked at that not terribly > long ago, but didn't really get anywhere. Yeah, with traditional asms we have to assume dependencies to everything (right now, i.e. if we don't want to enter the rathole of deprecating them). But we were also talking about non-traditional asms, i.e. with operands, where I think Alexander made the case that those should have explicit dependencies if they access the same registers as global reg vars. Ciao, Michael.
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On 04/24/2018 07:21 AM, Michael Matz wrote: > Hi, > > On Tue, 24 Apr 2018, Alexander Monakov wrote: > >>> Sure but even for that we need to decide if we want to go that or the >>> opposite way, and that's not easy when a deadline is lurking behind >>> you. >> >> I am surprised there is any question. Even gcc-3.4 optimizes reg vars >> over asms, on a testcase not unlike the very one you've used I see: > > Thanks for checking. Seems all the code that was supposed to care for > enabling this was always incomplete since a long time, which certainly > speaks for going the explicit dependency way. The very nature of a traditional asm implies that it can read or write anything visible to compiler. We can't realistically peek inside to see what's happening and the user hasn't provided appropriate dataflow information. One could make the argument that traditional asms should go the way of the dodo bird. I think we looked at that not terribly long ago, but didn't really get anywhere. Unfortunately doing the conservative thing and exposing the full set of dependency edges to give the natural semantics of a traditional asm has badly broken stuff in the past. I doubt the change to IRA/LRA has really changed this situation. So what we've done is paper over these issues time and again to try and preserve the expected semantics of a traditional asm. I wonder if we should bite the bullet and try again to deprecate traditional asms. Jeff
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi, On Tue, 24 Apr 2018, Alexander Monakov wrote: > On Tue, 24 Apr 2018, Michael Matz wrote: > > What is lost here (it wasn't explicit before, but is the case and must > > continue to work) is that function calls and returns count as needing the > > observable value in the specified register (function calls also count as > > setters of them). I think that should be made more explicit. > > It's no different from a normal variable in global scope. If the > compiler sees Yeah, that occured to me as well later :) Ignore me on that. Ciao, Michael.
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Tue, 24 Apr 2018, Michael Matz wrote:
> What is lost here (it wasn't explicit before, but is the case and must
> continue to work) is that function calls and returns count as needing the
> observable value in the specified register (function calls also count as
> setters of them). I think that should be made more explicit.
It's no different from a normal variable in global scope. If the compiler sees
gvar = 0;
bar();
gvar = 1;
it can delete the first store iff it can prove that it's dead ('bar' does
not read it), gvar being a register variable making no difference. So I
don't think it needs additional clarification.
> Especially
> because you make it explicit that the register remains available for
> allocation (where it before said it's not) we need to make sure that
> everybody understand that it's only during tiny windows like:
>
> regvar = ...
> ... code not accessing regvar and containing no calls ...
> regvar = ...
No, the first store is not needed. If the regvar is dead, the register is
freely allocatable (if not dead, it's allocatable at a cost of 1 spill).
> The return of functions also must remain making the regvar setter live, to
> give us the important parts of the former "stores aren't deleted even if
> appearing dead" guarantee.
This also follows from the variable being a global variable in the first place.
Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi,
On Tue, 24 Apr 2018, Alexander Monakov wrote:
> On Tue, 24 Apr 2018, Michael Matz wrote:
> > Well, documentation is both: a description and specification. If we
> > change the documentation now we might not be in the position anymore to
> > change behaviour and docu back because people might in the meanwhile rely
> > on the specification. Which is why documentation changes that promise a
> > certain behaviour are exactly the ones that shouldn't be rushed (even less
> > than actual implementation changes).
>
> My patch isn't making any promises; in a nutshell, it says "must use
> constraints", which is the most tight specification and doesn't open
> any new doors (in contrast with the position that you were arguing,
> which would be a new promise).
>
> Did the other changes in patch 1 look fine to you, content-wise?
Let's see again:
-@item The register is reserved entirely for this use, and will not be
-allocated for any other purpose.
-@item The register is not saved and restored by any functions.
-@item Stores into this register are never deleted even if they appear to be
-dead, but references may be deleted, moved or simplified.
+@item If the register is a call-saved register, call ABI is affected:
+the register will not be restored in function epilogue sequences after
+the variable has been assigned. Therefore, functions cannot safely
+return to callers that assume standard ABI.
+@item Conversely, if the register is a call-clobbered register, making
+calls to functions that use standard ABI may lose contents of the variable.
+Such calls may be created by the compiler even if none are evident in
+the original program, for example when libgcc functions are used to
+make up for unavailable instructions.
+@item Accesses to the variable may be optimized as usual and the register
+remains available for allocation and use in any computations, provided that
+observable values of the variable are not affected.
+@item If the variable is referenced in inline assembly, the type of access
+must be provided to the compiler via constraints (@pxref{Constraints}).
+Accesses from basic asms are not supported.
What is lost here (it wasn't explicit before, but is the case and must
continue to work) is that function calls and returns count as needing the
observable value in the specified register (function calls also count as
setters of them). I think that should be made more explicit. Especially
because you make it explicit that the register remains available for
allocation (where it before said it's not) we need to make sure that
everybody understand that it's only during tiny windows like:
regvar = ...
... code not accessing regvar and containing no calls ...
regvar = ...
The return of functions also must remain making the regvar setter live, to
give us the important parts of the former "stores aren't deleted even if
appearing dead" guarantee.
Ciao,
Michael.
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Tue, 24 Apr 2018, Michael Matz wrote: > Well, documentation is both: a description and specification. If we > change the documentation now we might not be in the position anymore to > change behaviour and docu back because people might in the meanwhile rely > on the specification. Which is why documentation changes that promise a > certain behaviour are exactly the ones that shouldn't be rushed (even less > than actual implementation changes). My patch isn't making any promises; in a nutshell, it says "must use constraints", which is the most tight specification and doesn't open any new doors (in contrast with the position that you were arguing, which would be a new promise). Did the other changes in patch 1 look fine to you, content-wise? Thanks. Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi, On Tue, 24 Apr 2018, Alexander Monakov wrote: > > Sure but even for that we need to decide if we want to go that or the > > opposite way, and that's not easy when a deadline is lurking behind > > you. > > I am surprised there is any question. Even gcc-3.4 optimizes reg vars > over asms, on a testcase not unlike the very one you've used I see: Thanks for checking. Seems all the code that was supposed to care for enabling this was always incomplete since a long time, which certainly speaks for going the explicit dependency way. > Besides, I don't really understand the objection about changing the docs > for gcc-8.1. Even if there's a doubt about future development, improving > the documentation to explain the behavior of a released compiler should > be good. Right now the docs are simply wrong. Well, documentation is both: a description and specification. If we change the documentation now we might not be in the position anymore to change behaviour and docu back because people might in the meanwhile rely on the specification. Which is why documentation changes that promise a certain behaviour are exactly the ones that shouldn't be rushed (even less than actual implementation changes). But in the meantime you seem to have established that everyone thinking that asms ever had implicit dependencies on global reg vars was merely wishful. I retract my objection to the documentation change (i.e. the specification of non implicit dependencies). Thanks for persisting :) Ciao, Michael.
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Tue, 24 Apr 2018, Michael Matz wrote:
> Sure but even for that we need to decide if we want to go that or the
> opposite way, and that's not easy when a deadline is lurking behind you.
I am surprised there is any question. Even gcc-3.4 optimizes reg vars over asms,
on a testcase not unlike the very one you've used I see:
register int foo asm("ebx");
void g(void);
void f(void)
{
foo=0;
asm("foo=1");
if (foo) g();
}
.globl f
.type f, @function
f:
.LFB2:
xorl%ebx, %ebx
#APP
foo=1
#NO_APP
ret
(here GCSE makes the propagation)
Besides, I don't really understand the objection about changing the docs for
gcc-8.1. Even if there's a doubt about future development, improving the
documentation to explain the behavior of a released compiler should be good.
Right now the docs are simply wrong.
Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi, On Tue, 24 Apr 2018, Alexander Monakov wrote: > On Mon, 23 Apr 2018, Michael Matz wrote: > > Sooo, hmm, I don't know ;-) We could try doing a roll backwards and > > demand explicit dependencies from asms with unknown effects on the few > > unknown users, though the timing really makes me nervous. But my gut > > feeling says to remain (or become again) very conservative with global reg > > vars (and document that for real then!). > > What's the problem with the timing? The code change is proposed for stage 1, > only the doc change is intended for gcc-8.1. Sure but even for that we need to decide if we want to go that or the opposite way, and that's not easy when a deadline is lurking behind you. Ciao, Michael.
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Mon, 23 Apr 2018, Michael Matz wrote: > Sooo, hmm, I don't know ;-) We could try doing a roll backwards and > demand explicit dependencies from asms with unknown effects on the few > unknown users, though the timing really makes me nervous. But my gut > feeling says to remain (or become again) very conservative with global reg > vars (and document that for real then!). What's the problem with the timing? The code change is proposed for stage 1, only the doc change is intended for gcc-8.1. Thanks. Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi, On Mon, 23 Apr 2018, Alexander Monakov wrote: > I don't see how a user reading the documentation could infer that asms > and global reg vars interact like you say. That was always my interpretation of this clause (old docu, the current bullet list seems to have removed some clarity) and memories of old code in the compiler: -- Defining a global register variable in a certain register reserves that register entirely for this use, at least within the current compilation. The register will not be allocated for any other purpose in the functions in the current compilation. -- It never occured to me that this wouldn't include implicit dependencies from asms to each occurrence of the global reg var, though in re-reading I now see how it could be interpreted as such (i.e. that there are no such implicit dependencies). In my mind accesses to global reg vars are volatile, though I also see that my mind might have made that up years ago as well. Probably the sub-phrase "Stores into this register are never deleted even if they would appear to be dead," caused this (indicating volatile-like), even though it continues to say "but references may be deleted or moved or simplified." (indicating non-volatile-like). > (indications to the contrary can be found on the other hand, e.g. > "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 general purpose registers occur. " in Basic Asm section) > > What am I missing? Maybe nothing, your reading of the docu seems valid. If that really reflects the intent is of course another question. > > Sure, so the other scanners need fixing as well. An alternative is to > > massage the asms while constructing them (e.g. gimplification time) to > > read and write all global reg var registers. Then all scanners will > > automatically work. > > Yes, but that is problematic since on some backends global_regs is > non-empty (e.g. rs6000, tile*) so it would tie all asms (it's also a > problem with the current DF behavior). [side remark: I think these are misuses and should rather set fixed_regs (I see other back-ends also set some global_regs members): there should be a reg-set which precisely captures the user-declared global register variables, and nothing more, which then would avoid the problem (and tie only asms when global reg vars are in scope, but would do so with all of them indeed). But even if we decide that these are mistakes it's probably non-trivial to fix without regressions (short of inventing something dubious like still another reg-set used for globalize_reg() that wouldn't be exported and hence not available to backends).] > > IMHO we can't define our bugs away in this way, after saying for > > decades how something is supposed to work. Especially not rushing it > > in a couple days before release. > > I honestly don't see where GCC was indicating how it's supposed to work. Given all the above I don't see it either anymore. I do remember several threads of discussions with various people over the years that always assumed that asms can validly implicitly refer to registers of global reg vars and expect everything to work, and I do remember code in GCC that tried to make sure to not mess that up. Looking for some minutes I find e.g. old regcprop (at the time still regrename.c:copy_value): /* Do not propagate copies to fixed or global registers, patterns can be relying to see particular fixed register or users can expect the chosen global register in asm. */ if (fixed_regs[dr] || global_regs[dr]) return; (Note the last part of the comment "users can expect the chosen global register in asm"). Also, a snippet from flow.c:mark_used_regs(): /* Traditional and volatile asm instructions must be considered to use and clobber all hard registers, all pseudo-registers and all of memory. So must TRAP_IF and UNSPEC_VOLATILE operations. (it then goes on to say that making all these explicit creates other problems and so doesn't in fact do as the comment says. Those compilers deal with that by not inferring much from volatile asms or propagating things over them.) So, there definitely once was code (or comments) trying to make sure at least volatile asms had access to global reg vars even without explicitely mentioning them. It might already have been incomplete at that time, or it only might have become inconsistent and incomplete since then (certainly we became more aggressive with any implicit things about extended asms), and most probably the changes that did occur in that area weren't all intentional. Hence I can't even begin to guess if a full change to only explicit dependencies would break existing code or not; global reg vars are not used very often, so there's a chance we might break all 10 users in the
Re: [PATCH 0/2] Require that constraints are used to reference global regs
On Mon, 23 Apr 2018, Michael Matz wrote: > In your follow-up patches you actually change the very documentation that > makes the above valid and supported ... I don't see how a user reading the documentation could infer that asms and global reg vars interact like you say. I am particularly unsure if all asms or only volatile ones are supposed to. (indications to the contrary can be found on the other hand, e.g. "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 general purpose registers occur. " in Basic Asm section) What am I missing? > Sure, so the other scanners need fixing as well. An alternative is to > massage the asms while constructing them (e.g. gimplification time) to > read and write all global reg var registers. Then all scanners will > automatically work. Yes, but that is problematic since on some backends global_regs is non-empty (e.g. rs6000, tile*) so it would tie all asms (it's also a problem with the current DF behavior). > > So this never worked anywhere near reliably > > I think you're exaggerating this, it worked fine in old compilers, in > particular before DF, and it still works fine mostly, when you aren't > knowning compiler internals to construct counter examples. By what mechanism was it supposed to work in old compilers, i.e. how did they ensure that asms touch some registers not present in their constraints? Sorry about the exaggeration. > > and we should simply adjust the documentation to say that asms must use > > constraints to say how they are accessing global reg vars. > > IMHO we can't define our bugs away in this way, after saying for > decades how something is supposed to work. Especially not rushing it in a > couple days before release. I honestly don't see where GCC was indicating how it's supposed to work. At the moment I'm quite confused as it seems that * the docs are unclear and the implementation is internally inconsistent * the docs cannot be changed to admit issues that are currently present. The latter doesn't seem quite right. (by the way, patch 1 has other changes beside the requirement for constraints, could they be useful on their own?) Thanks. Alexander
Re: [PATCH 0/2] Require that constraints are used to reference global regs
Hi,
On Mon, 23 Apr 2018, Alexander Monakov wrote:
> In discussion of proposed fix for PR 44281 Michael said:
> [ https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01963.html ]
> > [...] What I mean to say is, the following is currently proper use of
> > global reg vars:
> >
> >
> > register uint64_t ugh __asm__("rbx"); //r11, whatever
> > void write_into_ugh (void)
> > {
> > asm volatile ("mov 1, %%rbx" :::);
> > assert (ugh == 1);
> > }
> >
> >
> > %rbx would have to be implicitly used/clobbered by the asm. [...]
>
> I am skeptical. It is not documented
In your follow-up patches you actually change the very documentation that
makes the above valid and supported ...
> and it's not hard to come up with
> examples where optimizations break this sort of usage, e.g.:
... meaning that all these cases are actually bugs to be fixed.
> In general it is impossible to make GCC "think" that asms may access
> arbitrary global register variables by only patching DF. There's GIMPLE,
> and on RTL there are other scanners beside DF (in fact PR 79985 that I
> intend to close this way is due to sched-deps having other ideas than
> DF).
Sure, so the other scanners need fixing as well. An alternative is to
massage the asms while constructing them (e.g. gimplification time) to
read and write all global reg var registers. Then all scanners will
automatically work.
> So this never worked anywhere near reliably
I think you're exaggerating this, it worked fine in old compilers, in
particular before DF, and it still works fine mostly, when you aren't
knowning compiler internals to construct counter examples.
> and we should simply adjust the documentation to say that asms must use
> constraints to say how they are accessing global reg vars.
IMHO we can't define our bugs away in this way, after saying for
decades how something is supposed to work. Especially not rushing it in a
couple days before release.
So, I disagree with the intent of the patches. If we were designing this
extension of global reg vars from scratch I'd do as you suggest, but
unfortunately we aren't.
Ciao,
Michael.
