[PATCH 2/2] df-scan: remove ad-hoc handling of global regs in asms

[PATCH 2/2] df-scan: remove ad-hoc handling of global regs in asms

[Alexander Monakov]

As discussed in the cover letter, the code removed in this patch is unnecessary,
references to global reg vars from inline asms do not work reliably, and so we
should simply require that inline asms use constraints to make such references
properly visible to the compiler.

Bootstrapped/regtested on powerpc64, will retest on ppc64le and x86 in stage 1.

        PR rtl-optimization/79985
	* df-scan.c (df_insn_refs_collect): Remove special case for
        global registers and asm statements.
---
 gcc/df-scan.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gcc/df-scan.c b/gcc/df-scan.c
index 95e1e0df2d5..cbb08fc36ae 100644
--- a/gcc/df-scan.c
+++ b/gcc/df-scan.c
@@ -3207,17 +3207,6 @@ df_insn_refs_collect (struct df_collection_rec *collection_rec,
   if (CALL_P (insn_info->insn))
     df_get_call_refs (collection_rec, bb, insn_info, flags);
 
-  if (asm_noperands (PATTERN (insn_info->insn)) >= 0)
-    for (unsigned i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-      if (global_regs[i])
-       {
-         /* As with calls, asm statements reference all global regs. */
-         df_ref_record (DF_REF_BASE, collection_rec, regno_reg_rtx[i],
-                        NULL, bb, insn_info, DF_REF_REG_USE, flags);
-         df_ref_record (DF_REF_BASE, collection_rec, regno_reg_rtx[i],
-                        NULL, bb, insn_info, DF_REF_REG_DEF, flags);
-       }
-
   /* Record other defs.  These should be mostly for DF_REF_REGULAR, so
      that a qsort on the defs is unnecessary in most cases.  */
   df_defs_record (collection_rec,
-- 
2.13.3

[PATCH 1/2] extend.texi: update Global Register Variables section

[Alexander Monakov]

This rewrites global register vars doc to reflect that the register is no longer
reserved exclusively, but in fact is available for general allocation, and also
adds the requirement to properly inform the compiler where inline asms are
accessing the variable.

This:
-@item The register is not saved and restored by any functions.
is reworded to verbosely spell out gotchas related to calls/returns.

	* extend.texi (Global Register Variables): Rewrite the bullet list.
	Note that the register is available for allocation. Note that access
	via inline asm must use constraints. Add note about async-signal
	handlers. Remove paragraph about automagic register selection.
---
 gcc/doc/extend.texi | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 5571d05d93b..f663741e36c 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9549,11 +9549,21 @@ After defining a global register variable, for the current compilation
 unit:
 
 @itemize @bullet
-@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.
 @end itemize
 
 Note that these points @emph{only} apply to code that is compiled with the
@@ -9595,7 +9605,10 @@ the comparison function unless the @code{qsort} function itself is rebuilt.
 Similarly, it is not safe to access the global register variables from signal
 handlers or from more than one thread of control. Unless you recompile 
 them specially for the task at hand, the system library routines may 
-temporarily use the register for other things.
+temporarily use the register for other things.  Furthermore, since the register
+is not reserved exclusively for the variable, accessing it from handlers of
+asynchronous signals may observe unrelated temporary values residing in the
+register.
 
 @cindex register variable after @code{longjmp}
 @cindex global register after @code{longjmp}
@@ -9610,10 +9623,6 @@ should make other arrangements to save the values of the global register
 variables, and to restore them in a @code{longjmp}. This way, the same
 thing happens regardless of what @code{longjmp} does.
 
-Eventually there may be a way of asking the compiler to choose a register 
-automatically, but first we need to figure out how it should choose and 
-how to enable you to guide the choice.  No solution is evident.
-
 @node Local Register Variables
 @subsubsection Specifying Registers for Local Variables
 @anchor{Local Reg Vars}
-- 
2.13.3

[PATCH 0/2] Require that constraints are used to reference global regs

[Alexander Monakov]

Hello,

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 and it's not hard to come up with examples
where optimizations break this sort of usage, e.g.:

register int r asm("ebx");

int f(int x, int y)
{
    int t = x/y/r;
    asm("#asm xor %ebx, %ebx");
    return t-x;
}

This produces:

f:
#APP
        #asm xor %ebx, %ebx
#NO_APP
        movl    %edi, %eax
        cltd
        idivl   %esi
        cltd
        idivl   %ebx
        subl    %edi, %eax
        ret

i.e. assembly obviously divides by zero. In this particular example
cprop_hardreg makes the offending propagation.

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).

So this never worked anywhere near reliably and we should simply adjust the
documentation to say that asms must use constraints to say how they are
accessing global reg vars. The ad-hoc dependency injection in DF can then go.

Patch 1 implements doc/extend.texi changes to reflect the status quo. I hope it
can go in now.

Patch 2 removes df-scan handling. It is for stage 1.

Alexander Monakov (2):
  extend.texi: update Global Register Variables section
  df-scan: remove ad-hoc handling of global regs in asms

 gcc/df-scan.c       | 11 -----------
 gcc/doc/extend.texi | 29 +++++++++++++++++++----------
 2 files changed, 19 insertions(+), 21 deletions(-)

-- 
2.13.3

Re: [PATCH 0/2] Require that constraints are used to reference global regs

[Michael Matz]

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.

Re: [PATCH 0/2] Require that constraints are used to reference global regs

[Alexander Monakov]

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

[Michael Matz]

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 world, or none :)

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!).


Ciao,
Michael.

Re: [PATCH 0/2] Require that constraints are used to reference global regs

[Alexander Monakov]

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

[Michael Matz]

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

[Alexander Monakov]

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

[Michael Matz]

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

[Alexander Monakov]

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

[Michael Matz]

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

[Alexander Monakov]

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

[Michael Matz]

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

[Jeff Law]

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

[Michael Matz]

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

[Jeff Law]

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

[Alexander Monakov]

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 1/2] extend.texi: update Global Register Variables section

[Alexander Monakov]

On Mon, 23 Apr 2018, Alexander Monakov wrote:

> This rewrites global register vars doc to reflect that the register is no longer
> reserved exclusively, but in fact is available for general allocation, and also
> adds the requirement to properly inform the compiler where inline asms are
> accessing the variable.
> 
> This:
> -@item The register is not saved and restored by any functions.
> is reworded to verbosely spell out gotchas related to calls/returns.
> 
> 	* extend.texi (Global Register Variables): Rewrite the bullet list.
> 	Note that the register is available for allocation. Note that access
> 	via inline asm must use constraints. Add note about async-signal
> 	handlers. Remove paragraph about automagic register selection.
> ---
>  gcc/doc/extend.texi | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)

Ping? I believe the substance of new text has been hashed out with Michael.
I'd appreciate a review for language and style issues.

Thanks.
Alexander

> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 5571d05d93b..f663741e36c 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -9549,11 +9549,21 @@ After defining a global register variable, for the current compilation
>  unit:
>  
>  @itemize @bullet
> -@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.
>  @end itemize
>  
>  Note that these points @emph{only} apply to code that is compiled with the
> @@ -9595,7 +9605,10 @@ the comparison function unless the @code{qsort} function itself is rebuilt.
>  Similarly, it is not safe to access the global register variables from signal
>  handlers or from more than one thread of control. Unless you recompile 
>  them specially for the task at hand, the system library routines may 
> -temporarily use the register for other things.
> +temporarily use the register for other things.  Furthermore, since the register
> +is not reserved exclusively for the variable, accessing it from handlers of
> +asynchronous signals may observe unrelated temporary values residing in the
> +register.
>  
>  @cindex register variable after @code{longjmp}
>  @cindex global register after @code{longjmp}
> @@ -9610,10 +9623,6 @@ should make other arrangements to save the values of the global register
>  variables, and to restore them in a @code{longjmp}. This way, the same
>  thing happens regardless of what @code{longjmp} does.
>  
> -Eventually there may be a way of asking the compiler to choose a register 
> -automatically, but first we need to figure out how it should choose and 
> -how to enable you to guide the choice.  No solution is evident.
> -
>  @node Local Register Variables
>  @subsubsection Specifying Registers for Local Variables
>  @anchor{Local Reg Vars}
> 

Re: [PATCH 2/2] df-scan: remove ad-hoc handling of global regs in asms

[Alexander Monakov]

On Mon, 23 Apr 2018, Alexander Monakov wrote:

> As discussed in the cover letter, the code removed in this patch is unnecessary,
> references to global reg vars from inline asms do not work reliably, and so we
> should simply require that inline asms use constraints to make such references
> properly visible to the compiler.
> 
> Bootstrapped/regtested on powerpc64, will retest on ppc64le and x86 in stage 1.
> 
>         PR rtl-optimization/79985
> 	* df-scan.c (df_insn_refs_collect): Remove special case for
>         global registers and asm statements.

Ping. I've retested once on ppc64le since posting.

> ---
>  gcc/df-scan.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/gcc/df-scan.c b/gcc/df-scan.c
> index 95e1e0df2d5..cbb08fc36ae 100644
> --- a/gcc/df-scan.c
> +++ b/gcc/df-scan.c
> @@ -3207,17 +3207,6 @@ df_insn_refs_collect (struct df_collection_rec *collection_rec,
>    if (CALL_P (insn_info->insn))
>      df_get_call_refs (collection_rec, bb, insn_info, flags);
>  
> -  if (asm_noperands (PATTERN (insn_info->insn)) >= 0)
> -    for (unsigned i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> -      if (global_regs[i])
> -       {
> -         /* As with calls, asm statements reference all global regs. */
> -         df_ref_record (DF_REF_BASE, collection_rec, regno_reg_rtx[i],
> -                        NULL, bb, insn_info, DF_REF_REG_USE, flags);
> -         df_ref_record (DF_REF_BASE, collection_rec, regno_reg_rtx[i],
> -                        NULL, bb, insn_info, DF_REF_REG_DEF, flags);
> -       }
> -
>    /* Record other defs.  These should be mostly for DF_REF_REGULAR, so
>       that a qsort on the defs is unnecessary in most cases.  */
>    df_defs_record (collection_rec,
> 

Re: [PATCH 1/2] extend.texi: update Global Register Variables section

[Jeff Law]

On 05/10/2018 03:47 AM, Alexander Monakov wrote:
> 
> 
> On Mon, 23 Apr 2018, Alexander Monakov wrote:
> 
>> This rewrites global register vars doc to reflect that the register is no longer
>> reserved exclusively, but in fact is available for general allocation, and also
>> adds the requirement to properly inform the compiler where inline asms are
>> accessing the variable.
>>
>> This:
>> -@item The register is not saved and restored by any functions.
>> is reworded to verbosely spell out gotchas related to calls/returns.
>>
>> 	* extend.texi (Global Register Variables): Rewrite the bullet list.
>> 	Note that the register is available for allocation. Note that access
>> 	via inline asm must use constraints. Add note about async-signal
>> 	handlers. Remove paragraph about automagic register selection.
>> ---
>>  gcc/doc/extend.texi | 29 +++++++++++++++++++----------
>>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> Ping? I believe the substance of new text has been hashed out with Michael.
> I'd appreciate a review for language and style issues.
> 
> Thanks.
> Alexander
> 
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 5571d05d93b..f663741e36c 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -9549,11 +9549,21 @@ After defining a global register variable, for the current compilation
>>  unit:
>>  
>>  @itemize @bullet
>> -@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.
Whether or not a global register is saved in the prologue and restored
in the epilogue is actually a function of the target's prologue/epilogue
implementation.

So ISTM this paragraph needs to be refined a bit.  Essentially it may or
may not change the ABI.  It really depends on the target implementation.


The rest of the doc changes look fine.

jeff

Re: [PATCH 1/2] extend.texi: update Global Register Variables section

[Alexander Monakov]

On Mon, 21 May 2018, Jeff Law wrote:
> Whether or not a global register is saved in the prologue and restored
> in the epilogue is actually a function of the target's prologue/epilogue
> implementation.
> 
> So ISTM this paragraph needs to be refined a bit.  Essentially it may or
> may not change the ABI.  It really depends on the target implementation.

As far as I can tell, that would be a target bug; if global registers are
restored in epilogues, they are not useful.

Alexander

Re: [PATCH 2/2] df-scan: remove ad-hoc handling of global regs in asms

[Jeff Law]

On 05/16/2018 04:30 AM, Alexander Monakov wrote:
> 
> 
> On Mon, 23 Apr 2018, Alexander Monakov wrote:
> 
>> As discussed in the cover letter, the code removed in this patch is unnecessary,
>> references to global reg vars from inline asms do not work reliably, and so we
>> should simply require that inline asms use constraints to make such references
>> properly visible to the compiler.
>>
>> Bootstrapped/regtested on powerpc64, will retest on ppc64le and x86 in stage 1.
>>
>>         PR rtl-optimization/79985
>> 	* df-scan.c (df_insn_refs_collect): Remove special case for
>>         global registers and asm statements.
> 
> Ping. I've retested once on ppc64le since posting.
This has the potential to break existing code that we've tried to keep
working.  Worse yet, it's not code we're likely to see until gcc-9 goes
into wide deployment.  So there's certainly a risk of complaints around
this change.

I would not expect our testsuite to provide any meaningful test coverage
here.  Matz (in the discussion around pr44281 on gcc-patches) indicates
that some jits may utilize global registers.  Unfortunately, he doesn't
indicate which ones -- which would provide a pointer for deeper testing.

But even with those caveats, I think the consensus is to go forward with
the doc change.  This change naturally follows from the doc update.

So OK for the trunk.  If there's fallout in gcc-9, we'll obviously have
to deal with it.

jeff

Re: [PATCH 2/2] df-scan: remove ad-hoc handling of global regs in asms

[Alexander Monakov]

On Tue, 22 May 2018, Jeff Law wrote:
> So OK for the trunk.  If there's fallout in gcc-9, we'll obviously have
> to deal with it.

IMHO what happened here is not healthy. Thank you for the green light.

Alexander