Re: [PATCH] [RFC] PR target/52813 and target/11807

[Segher Boessenkool <segher@kernel.crashing.org>]

On Thu, Dec 13, 2018 at 11:26:40PM +0200, Dimitar Dimitrov wrote:
> On Thu, Dec 13, 2018 at 8:48:38 EET Segher Boessenkool wrote:
> > On Wed, Dec 12, 2018 at 06:26:10PM +0200, Dimitar Dimitrov wrote:
> > > I expect that if I mark a HW register as "clobber", compiler would save
> > > its
> > > contents before executing the asm statement, and after that it would
> > > restore its contents. This is the GCC behaviour for all but the SP and
> > > PIC registers. That is why I believe that PR52813 is a valid bug.
> > 
> > It won't do it for *any* fixed registers.  But you do not want to error
> > or even warn for some fixed registers, for example the "flags" register
> > on x86 is *always* written to by asm.
> 
> Yes, you are correct.
> 
> > But you never want to warn for non-fixed registers, and e.g.
> > PIC_OFFSET_TABLE_REGNUM isn't always a fixed register (when flag_pic is 0
> > for example).
> I  could not trace how PIC_OFFSET_TABLE_REGNUM on i386 gets marked as fixed 
> register. I'll dig more through the source.
> 
> > > I'm not sure how GCC could recover if SP is clobbered. If SP is clobbered
> > > in such a way that GCC will not notice (e.g. thread switching), then why
> > > should GCC know about it in the first place?
> > 
> > Up until today, GCC has always just ignored it if you claimed to clobber
> > the stack pointer.
> 
> My point is that the silent ignoring is confusing to users, as shown by 
> PR52813. How would you suggest me to proceed:
>  - Leave patch as-is.
>  - Revert patch. Update documentation to point that clobber marker for fixed 
> registers is ignored by GCC. Close PR52813 as invalid.
>  - Revert patch. Discuss more broadly and specify behaviour of asm clobber for 
> fixed registers (and SP in particular).

You need a few tweaks to what you committed.  Or just one perhaps: if
flag_pic is not set, you should not check PIC_OFFSET_TABLE_REGNUM, it is
meaningless in that case.  I'm not sure if you need to check whether the
register is fixed or not.

But there are many more regs than just the PIC reg and the stack pointer
that you would want to similarly warn about, because overwriting those
registers is just as fatal: the frame pointer, the program counter, etc.
_Most_ fixed registers, but not _all_.

So maybe it should be a target hook?  OTOH that is a lot of work for such
a trivial warning, that isn't very useful anyway (a better warning for
any asm is: "Are you sure?" :-) )

So I don't know what is best to do (except that flag_pic part).  Sorry.


Segher