Re: Question on GIMPLE semantics and EH

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    The only part of the compiler that should pay attention to
    DECL_ARTIFICIAL is probably the debug generators.

And things that print error messages and warnings.  You probably don't want to
see unused variable warnings for DECL_ARTIFICIAL stuff, for example.  I
believe we presently suppress it for them.

Re: Question on GIMPLE semantics and EH

[Richard Henderson]

On Sun, Aug 15, 2004 at 08:46:06PM -0400, Richard Kenner wrote:
> And things that print error messages and warnings.  You probably don't want to
> see unused variable warnings for DECL_ARTIFICIAL stuff, for example.  I
> believe we presently suppress it for them.

No, we do not.


r~

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    > And things that print error messages and warnings.  You probably don't
    >want to see unused variable warnings for DECL_ARTIFICIAL stuff, for
    >example.  I believe we presently suppress it for them.

    No, we do not.

I stand corrected, though we still use it for parameters to suppress
unused warnings.

Re: Question on GIMPLE semantics and EH

[Mark Mitchell]

Paul Brook wrote:

>On Saturday 14 August 2004 00:26, Richard Kenner wrote:
>  
>
>>    I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
>>    that would explain the mistake -- the gimplifier thinks it
>>    already *is* a temporary.
>>
>>I can confirm that not setting it fixes this ACATS test and a few others.
>>    
>>
>
>Is this intended as a long-term solution or a temporary kludge?
>
>The fortran frontend also sets DECL_ARTIFICIAL on variables not mentioned in 
>the source.
>  
>
Independent of intent, it should not be a long-term solution.  The use 
of DECL_ARTIFICIAL you describe is clearly correct, according to both 
the documentation and long-standing practice.  The only part of the 
compiler that should pay attention to DECL_ARTIFICIAL is probably the 
debug generators.

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    However, apparently in the Ada case this is not true of all temporaries.

In fact, it's not true for *most* variables it introduces.  Many really
aren't *temporaries* at all, actually, but instead internal variables that
are used for various purposes (in this case for a list of objects that need
finalization).  Often what happens is that a single user variable or type
produces a number of corresponding variables, which have different usage
patterns from either each other or the original variable.

    So is the issue that the gimplify conversion is assuming that
    DECL_ARTIFICIAL variables are SSA?

No, not SSA, but it is making an assumption that's sort of hard to pin down.

However, I'm wondering if for this particular case (a CALL), gimplification
can leave the LHS a variable if it's both DECL_ARTIFICIAL and TREE_READONLY?

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    Is this intended as a long-term solution or a temporary kludge?

Very definitely a temporary kludge, more accurately a "proof of concept".

Re: Question on GIMPLE semantics and EH

[Paul Brook]

On Saturday 14 August 2004 00:46, Joe Buck wrote:
> On Sat, Aug 14, 2004 at 12:34:29AM +0100, Paul Brook wrote:
> > On Saturday 14 August 2004 00:26, Richard Kenner wrote:
> > >     I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
> > >     that would explain the mistake -- the gimplifier thinks it
> > >     already *is* a temporary.
> > >
> > > I can confirm that not setting it fixes this ACATS test and a few
> > > others.
> >
> > Is this intended as a long-term solution or a temporary kludge?
> >
> > The fortran frontend also sets DECL_ARTIFICIAL on variables not mentioned
> > in the source.
>
> It seems to me that many temporaries introduced by language front ends are
> SSA safe, in the sense that they are always assigned before first use and
> always hold the same value.  However, apparently in the Ada case this is
> not true of all temporaries.  So is the issue that the gimplify conversion
> is assuming that DECL_ARTIFICIAL variables are SSA?

Many of the temporaries introduced by the Fortran frontend are not SSA safe. 
We generate everything from simple scalars to loop index variables, 
addressable variables and aggregate/array typed variables.

Between them these break pretty much any assumptions the gimplifier might 
make.

Paul

Re: Question on GIMPLE semantics and EH

[Joe Buck]

On Sat, Aug 14, 2004 at 12:34:29AM +0100, Paul Brook wrote:
> On Saturday 14 August 2004 00:26, Richard Kenner wrote:
> >     I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
> >     that would explain the mistake -- the gimplifier thinks it
> >     already *is* a temporary.
> >
> > I can confirm that not setting it fixes this ACATS test and a few others.
> 
> Is this intended as a long-term solution or a temporary kludge?
> 
> The fortran frontend also sets DECL_ARTIFICIAL on variables not mentioned in 
> the source.

It seems to me that many temporaries introduced by language front ends are
SSA safe, in the sense that they are always assigned before first use and
always hold the same value.  However, apparently in the Ada case this is
not true of all temporaries.  So is the issue that the gimplify conversion
is assuming that DECL_ARTIFICIAL variables are SSA?

Re: Question on GIMPLE semantics and EH

[Paul Brook]

On Saturday 14 August 2004 00:26, Richard Kenner wrote:
>     I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
>     that would explain the mistake -- the gimplifier thinks it
>     already *is* a temporary.
>
> I can confirm that not setting it fixes this ACATS test and a few others.

Is this intended as a long-term solution or a temporary kludge?

The fortran frontend also sets DECL_ARTIFICIAL on variables not mentioned in 
the source.

Paul

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
    that would explain the mistake -- the gimplifier thinks it 
    already *is* a temporary.

I can confirm that not setting it fixes this ACATS test and a few others.

The only remaining failures are three tests involving tasking.

Of course, this doesn't mean we're "done" since I have a number of
kludges in my tree to work around bugs that I'm awaiting resolution on,
but we're getting much closer ..

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    I don't suppose F36b has DECL_ARTIFICIAL set?  

Yeah, it sure does!  Ada sets it for any variable that doesn't directly
correspond to a user-level variable.

    If it does, then that would explain the mistake -- the gimplifier
    thinks it already *is* a temporary.

    If so, we'll have to some up with some other way to identify
    gimple temporaries.

Ugh ...

Re: Question on GIMPLE semantics and EH

[Richard Henderson]

On Fri, Aug 13, 2004 at 02:59:51PM -0400, Richard Kenner wrote:
> Maybe, as you say.   But .t04.generic has
> 
>     F36b = c761006_2__adj_checkDA (F36b, &another_target, 1);
> 
> which already might be wrong depending on the definition of "wrong".

I don't suppose F36b has DECL_ARTIFICIAL set?  If it does, then
that would explain the mistake -- the gimplifier thinks it 
already *is* a temporary.

If so, we'll have to some up with some other way to identify
gimple temporaries.


r~

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    Ah, so the problem is not out of the gimplifier, but out of the
    nested function lowering?  Maybe.

Maybe, as you say.   But .t04.generic has

    F36b = c761006_2__adj_checkDA (F36b, &another_target, 1);

which already might be wrong depending on the definition of "wrong".

    I'll try to build a smaller test case to see if I can reproduce this.

Thanks.

Re: Question on GIMPLE semantics and EH

[Richard Henderson]

On Fri, Aug 13, 2004 at 02:29:35PM -0400, Richard Kenner wrote:
> OK, that explains it.  The type was a scalar (a pointer), but it was a
> component of an aggregrate, with the aggregate being the FRAME variable.
> So no temporary was used.  Then SRA made it into a scalar variable.
...
> So we have in .t05.nested:
> 
>     FRAME.178.F36b = 0B;
>     ...
>     try { 
> 	...
> 	FRAME.178.F36b = c761006_2__adj_checkDA (T.200, &another_target, 1);

Ah, so the problem is not out of the gimplifier, but out of the
nested function lowering?  Maybe.

I'll try to build a smaller test case to see if I can reproduce this.


r~

Re: Question on GIMPLE semantics and EH

[Richard Kenner]

    Not exactly.  It depends on the type of foo.  From the assignment
    from zero, I assume it's scalar.  Which means that the value always
    dies at the function call.  Which is why you should *not* have seen
    an assignment to foo directly out of the .10.gimple dump -- it should
    have been a temporary.

OK, that explains it.  The type was a scalar (a pointer), but it was a
component of an aggregrate, with the aggregate being the FRAME variable.
So no temporary was used.  Then SRA made it into a scalar variable.

    Who knows.  Give us a proper bug report and we might be able to help.

The test case is the ACATS test c761006, but it's a large mess of
expanded code, so it's hard to read.  The function in question is
Adjust_Test and the variable is F36b.

So we have in .t05.nested:

    FRAME.178.F36b = 0B;
    ...
    try { 
	...
	FRAME.178.F36b = c761006_2__adj_checkDA (T.200, &another_target, 1);

That's the one that throws.

The "finally" has a call with FRAME.178 as the static chain.

The .t13.lower inlined the second call and has:

              T.182 = CHAIN.181->F36b;
              system__finalization_implementation__finalize_list (T.182);

The .30.ch has

  FRAME.178.F36b = 0B;
	...
  FRAME.178.F36b = c761006_2__adj_checkDA (0B, &another_target, 1);

	...
<bb 40>:
  T.182_464 = FRAME.178.F36b;
  system__finalization_implementation__finalize_list (T.182_464);

.t32.sra then has:

  SR.269_1504 = 0B;
	...
  SR.269_1505 = c761006_2__adj_checkDA (0B, &another_target, 1);
	...

<bb 40>:
  T.182_464 = SR.269_1505;
  system__finalization_implementation__finalize_list (T.182_464);

which is the case I mentioned and copyrename2 replaces the target of the
call with T.182_1505.

Re: Question on GIMPLE semantics and EH

[Richard Henderson]

On Fri, Aug 13, 2004 at 08:34:51AM -0400, Richard Kenner wrote:
> Suppose I have something like:
> 
> 	foo = 0;
> 	...
> 	foo = some_function_call_that_may_throw ( ...);
> 
> in one basic block and then in the exception handler, we have:
> 
> 	bar = foo;
> 
> As I understand it, the semantics of GIMPLE is that the second assignment to
> FOO may not happen, but if it doesn't, then the value must remain 0.

Not exactly.  It depends on the type of foo.  From the assignment
from zero, I assume it's scalar.  Which means that the value always
dies at the function call.  Which is why you should *not* have seen
an assignment to foo directly out of the .10.gimple dump -- it should
have been a temporary.

I presume the temporary name got folded away by the first copyrename
pass, but that in the dump you're looking at, the two occurrences of
foo have different ssa names.

> copyrename2 is replacing the second assignment's LHS with BAR.

No.  It's replacing the base of the SSA_NAME, which is absolutely
not the same thing.

Do not drop ssa names from problems you report.  It only confuses
the issue.

> What's going wrong here?

Who knows.  Give us a proper bug report and we might be able to help.


r~

Question on GIMPLE semantics and EH

[Richard Kenner]

Suppose I have something like:

	foo = 0;
	...
	foo = some_function_call_that_may_throw ( ...);

in one basic block and then in the exception handler, we have:

	bar = foo;

As I understand it, the semantics of GIMPLE is that the second assignment to
FOO may not happen, but if it doesn't, then the value must remain 0.

copyrename2 is replacing the second assignment's LHS with BAR.  That seems
wrong since BAR might be assigned the value of zero.  With that substitution,
BAR will be undefined if the function does throw.

What's going wrong here?

We're down to four remaining ACATS failures and I think this is what
is causing those four, so we may be clean when this is fixed.