On Mon, Mar 9, 2020 at 1:14 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Thanks for doing this.
>
> "J.W. Jagersma" <jwjager...@gmail.com> writes:
> > On 2020-03-07 20:20, Segher Boessenkool wrote:
> >> Hi!
> >>
> >> On Sat, Mar 07, 2020 at 06:12:45PM +0100, J.W. Jagersma wrote:
> >>> The following patch extends the generation of exception handling
> >>> information to cover volatile asms too. This was already mostly
> >>> implemented, and only minor changes are required in order to make it
> >>> work.
> >>
> >> This should wait for stage 1, IMO. Looks pretty good to me, thanks!
> >
> > Hi, thanks for your response.
> > What does stage 1 refer to? I'm sorry, this is my first gcc patch and
> > I'm still learning how this all works.
> >
> >> Some comments:
> >>
> >>> +When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, a
> >>> +@code{volatile asm} statement is also allowed to throw exceptions. If it
> >>> +does, then the compiler assumes that its output operands have not been
> >>> written
> >>> +yet.
> >>
> >> That reads as if the compiler assumes the outputs retain their original
> >> value, but that isn't true (I hope!) The compiler assumes the output
> >> are clobbered, but it doesn't assume they are assigned any definite
> >> value?
> >
> > Register outputs are assumed to be clobbered, yes. For memory outputs
> > this is not the case, if the asm writes it before throwing then the
> > memory operand retains this value. It should be the user's
> > responsibility to ensure that an asm has no side-effects if it throws.
>
> I guess one problem is that something like "=&m" explicitly says that
> the operand is clobbered "early", and I think it would be fair for
> "early" to include clobbers before the exception. So IMO we should
> allow at least early-clobbered memory outputs to be clobbered by the
> exception.
I think memory operands are fine - my original concern was about
register outputs and SSA form that should reflect the correct def
on the EH vs non-EH edge. From a "miscompile" perspective
doing nothig which means pretending the asm actually set the output
could lead us to false DCE of the old value:
int foo = 0
try
{
asm volatile ("..." : "=r" (foo));
}
catch (...whatever...)
{
foo should be still zero, but SSA doesn't have the correct use here
}
that means the compiler really assumes the asm will populate the outputs
even when it throws.
Test coverage for that would be nice.
> And if we do that, then I'm not sure there's much benefit in trying to
> treat the non-earlyclobber memory case specially.
>
> It would be good to have testcases for the output cases. E.g. for:
>
> int foo;
> int bar = 0;
> try
> {
> foo = 1;
> asm volatile ("..." : "=m" (foo));
> }
> catch (...whatever...)
> {
> bar = foo;
> }
> ...use bar...
>
> What does "bar = foo" read? Is it always undefined behaviour if executed?
> Or does it always read "foo" from memory? Can it be optimised to "bar = 1"?
> Is "foo = 1" dead code?
>
> Thanks,
> Richard
