Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao <qing.zhao@oracle.com>]

Hi, Michael,

> On Jul 1, 2021, at 9:40 AM, Michael Matz <matz@suse.de> wrote:
> 
> Hello,
> 
> I haven't followed this thread too closely, in particular I haven't seen 
> why the current form of the .DEFERRED_INIT call was chosen or suggested, 
> but it triggered my "well, that's obviously wrong" gut feeling; so sorry 
> for stating something which might be obvious to thread participants here.  
> Anyway:
> 
> On Thu, 1 Jul 2021, Richard Sandiford via Gcc-patches wrote:
> 
>>>> It's not a bug in SSA, it's at most a missed optimization there.
>>> 
>>> I still think that SSA cannot handle block-scoped variable correctly 
>>> in this case, it should not move the variable out side of the block 
>>> scope. -:)
>>> 
>>>> But with -ftrivial-auto-var-init it becomes a correctness issue.
>> 
>> I might have lost track of what “it” is here.  Do you mean the 
>> progation, or the fact that we have a PHI in the first place?
>> 
>> For:
>> 
>> unsigned int
>> f (int n)
>> {
>>  unsigned int res = 0;
>>  for (int i = 0; i < n; i += 1)
>>    {
>>      unsigned int foo;
>>      foo += 1;
> 
> Note that here foo is used uninitialized.  That is the thing from which 
> everything else follows.  Garbage in, garbage out.  It makes not too much 
> sense to argue that the generated PHI node on this loop (generated because 
> of the exposed-upward use of foo) is wrong, or right, or should better be 
> something else.  The input program was broken, so anything makes sense.

Yes, the PHI node was generated because of the exposed-upward use of “foo”.
this makes sense.

However, the PHI node should not be put out of the live scope of “foo”, this is wrong.

IMO, even though there is uninitialized variable, it’s not the excuse that compiler should
not maintain correct variable scopes.


> 
> That is the same with Qings shorter testcase:
> 
>  6   for (i = 0; i < a; i++) {
>  7     if (__extension__({int size2;
>  8         size2 = ART_INIT (size2, 2);
> 
> Uninitialized use of size2 right there.  And it's the same for the use of 
> .DEFERRED_INIT as the patch does:
> 
> {
>  int size2;
>  size2 = .DEFERRED_INIT (size2, 2);
>  size2 = 4;
>  _1 = size2 > 5;
>  D.2240 = (int) _1;
> }
> 
> Argument of the pseudo-function-call to .DEFERRED_INIT uninitialized -> 
> boom.
> 
> You can't solve any of this by fiddling with how SSA rewriting behaves at 
> a large scale.  You need to change this and only this specific 
> interpretation of a use.  Or preferrably not generate it to start with.
> 
>> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
>> so there should be no loop-carried dependency here.
>> 
>> So yeah, if “it” meant that the fact that variables live too long,
>> then I agree that becomes a correctness issue with this feature,
>> rather than just an odd quirk.  Could we solve it by inserting
>> a clobber at the end of the variable's scope, or something like that?
> 
> It would possibly make GCC not generate a PHI on this broken input, yes.  
> But why would that be an improvement?

Being adding Clobbers at the end of the variable’s scope, the compiler will keep the correct
Variable scope during this stage, and prevent any more incorrect transformation from happening.
IMO  this should be a nice improvement.  -:)

Thanks.

Qing
> 
>>> Agreed, I have made such change yesterday, and this work around the 
>>> current issue.
>>> 
>>> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>>> 
>>> To:
>>> 
>>> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>> 
>> I think we're going round in circles here though.  The point of having
>> the undefined input was so that we would continue to warn about undefined
>> inputs.  The above issue doesn't seem like a good justification for
>> dropping that.
> 
> If you want to rely on the undefined use for error reporting then you must 
> only generate an undefined use when there was one before, you can't just 
> insert new undefined uses.  I don't see how it could be otherwise, as soon 
> as you introduce new undefined uses you can and will run into GCC making 
> use of the undefinedness, not just this particular issue with lifetime and 
> PHI nodes which might be "solved" by clobbers.
> 
> I think it's a chicken egg problem: you can't add undefined uses, for 
> which you need to know if there was one, but the facility is supposed to 
> help detecting if there is one to start with.
> 
> 
> Ciao,
> Michael.