middle-end: Skip initialization of opaque type register variables [PR103127]

Message ID ccb5f603-5c1f-0e59-31d9-07251f81e0aa@linux.ibm.com
State New
Headers
Series middle-end: Skip initialization of opaque type register variables [PR103127] |

Commit Message

Peter Bergner Nov. 29, 2021, 9:56 p.m. UTC
  Sorry for dropping the ball on testing the patch from the bugzilla!

The following patch fixes the ICE reported in the bugzilla on the pre-existing
gcc testsuite test case, bootstraps and shows no testsuite regressions
on powerpc64le-linux.  Ok for trunk?

Peter


For -ftrivial-auto-var-init=*, skip initializing the register variable if it
is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes.

gcc/
	PR middle-end/103127
	* internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque.
  

Comments

Qing Zhao Nov. 29, 2021, 10:56 p.m. UTC | #1
Peter,

Thanks a lot for the patch.

Richard, how do you think of the patch?

(The major concern for me is:
	
	With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase,
         However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase.  This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later?

         If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset?
	    then we should update “is_var_need_auto_init” in gimplify.c instead.   However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory?
).

Thanks.

Qing


> On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> Sorry for dropping the ball on testing the patch from the bugzilla!
> 
> The following patch fixes the ICE reported in the bugzilla on the pre-existing
> gcc testsuite test case, bootstraps and shows no testsuite regressions
> on powerpc64le-linux.  Ok for trunk?
> 
> Peter
> 
> 
> For -ftrivial-auto-var-init=*, skip initializing the register variable if it
> is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes.
> 
> gcc/
> 	PR middle-end/103127
> 	* internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque.
> 
> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> index 0cba95411a6..7cc0e9d5293 100644
> --- a/gcc/internal-fn.c
> +++ b/gcc/internal-fn.c
> @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>     }
>   else
>     {
> +      /* Skip variables of opaque types that are in a register.  */
> +      if (OPAQUE_TYPE_P (var_type))
> +	return;
> +
>       /* If this variable is in a register use expand_assignment.
> 	 For boolean scalars force zero-init.  */
>       tree init;
  
Richard Biener Nov. 30, 2021, 8:37 a.m. UTC | #2
On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Peter,
>
> Thanks a lot for the patch.
>
> Richard, how do you think of the patch?
>
> (The major concern for me is:
>
>         With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase,
>          However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase.  This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later?

I think that's inconsistent indeed.  Peter, what are "opaque"
registers?  rs6000-modes.def suggests
that there's __vector_pair and __vector_quad, what's the GIMPLE types
for those?  It seems they
are either SSA names or expanded to pseudo registers but there's no
constants for them.

>
>          If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset?
>             then we should update “is_var_need_auto_init” in gimplify.c instead.   However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory?
> ).

I think we need to bite the bullet at some point to do register initialization
not via expand_assignment but directly based on what the LHS expands to.

Can they be initialized?  I see they can be copied at least.

If such "things" cannot be initialized they should indeed be exempt
from auto-init.  The
documentation suggests that they act as bit-bucked but even bit-buckets should
be initializable, thus why exactly does CONST0_RTX not exist for them?

Richard.


>
> Thanks.
>
> Qing
>
>
> > On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote:
> >
> > Sorry for dropping the ball on testing the patch from the bugzilla!
> >
> > The following patch fixes the ICE reported in the bugzilla on the pre-existing
> > gcc testsuite test case, bootstraps and shows no testsuite regressions
> > on powerpc64le-linux.  Ok for trunk?
> >
> > Peter
> >
> >
> > For -ftrivial-auto-var-init=*, skip initializing the register variable if it
> > is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes.
> >
> > gcc/
> >       PR middle-end/103127
> >       * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque.
> >
> > diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> > index 0cba95411a6..7cc0e9d5293 100644
> > --- a/gcc/internal-fn.c
> > +++ b/gcc/internal-fn.c
> > @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> >     }
> >   else
> >     {
> > +      /* Skip variables of opaque types that are in a register.  */
> > +      if (OPAQUE_TYPE_P (var_type))
> > +     return;
> > +
> >       /* If this variable is in a register use expand_assignment.
> >        For boolean scalars force zero-init.  */
> >       tree init;
>
  
Peter Bergner Nov. 30, 2021, 3:14 p.m. UTC | #3
On 11/30/21 2:37 AM, Richard Biener wrote:
> On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote:
> I think that's inconsistent indeed.  Peter, what are "opaque"
> registers?  rs6000-modes.def suggests
> that there's __vector_pair and __vector_quad, what's the GIMPLE types
> for those?  It seems they
> are either SSA names or expanded to pseudo registers but there's no
> constants for them.

The __vector_pair and __vector_quad types are target specific types
for use with our Matrix-Math-Assist (MMA) unit and they are only
usable with our associated MMA built-in functions.  What they hold
is really dependent on which MMA built-ins you use on them.
You can think of them a generic (and large) vector type where the
subtype is undefined...or defined by which built-in function you
happen to be using.

We do not have any constants defined for them.  How we initialize them
is either by loading values from memory into them or by zeroing them
out using the xxsetaccz instruction (only for __vector_quads).




> Can they be initialized?  I see they can be copied at least.

__vector_quads can be zero initialized using the __builtin_mma_xxsetaccz()
built-in function.  We don't have a method (or use case) for zero initializing
__vector_pairs.



> If such "things" cannot be initialized they should indeed be exempt
> from auto-init.  The
> documentation suggests that they act as bit-bucked but even bit-buckets should
> be initializable, thus why exactly does CONST0_RTX not exist for them?

We used to have CONST0_RTX defined (but nothing else), but we had problems
with the compiler CSEing the initialization for multiple __vector_quads and
then copying the values around.  We'd end up with one xxsetaccz instruction
and copies out of that accumulator register into the other accumulator
registers.  Copies are VERY expensive, while xxsetaccz's are cheap, so we
don't want that.  That said, I think a fix I put in to disable fwprop on
these types may have been the culprit for that problem, so maybe we could
add the CONST0_RTX back?  I'd have to verify that.  If so, then we'd at least
be able to support -ftrivial-auto-var-init=zero.  The =pattern version
would be more problematical...unless the value for pattern was loaded from
memory.

Peter
  
Qing Zhao Nov. 30, 2021, 5:51 p.m. UTC | #4
> On Nov 30, 2021, at 9:14 AM, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> On 11/30/21 2:37 AM, Richard Biener wrote:
>> On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> I think that's inconsistent indeed.  Peter, what are "opaque"
>> registers?  rs6000-modes.def suggests
>> that there's __vector_pair and __vector_quad, what's the GIMPLE types
>> for those?  It seems they
>> are either SSA names or expanded to pseudo registers but there's no
>> constants for them.
> 
> The __vector_pair and __vector_quad types are target specific types
> for use with our Matrix-Math-Assist (MMA) unit and they are only
> usable with our associated MMA built-in functions.  What they hold
> is really dependent on which MMA built-ins you use on them.
> You can think of them a generic (and large) vector type where the
> subtype is undefined...or defined by which built-in function you
> happen to be using.
> 
> We do not have any constants defined for them.  How we initialize them
> is either by loading values from memory into them or by zeroing them
> out using the xxsetaccz instruction (only for __vector_quads).

So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. 

The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as:

void
foo (__vector_quad *dst)
{
  __vector_quad acc;
  __builtin_mma_xxsetaccz(&acc);
  *dst = acc;
}

Is this the correct understanding?

Is there way to initialize such variable to other values than zero at source code level?

Qing

> 
> 
> 
> 
>> Can they be initialized?  I see they can be copied at least.
> 
> __vector_quads can be zero initialized using the __builtin_mma_xxsetaccz()
> built-in function.  We don't have a method (or use case) for zero initializing
> __vector_pairs.


> 
> 
> 
>> If such "things" cannot be initialized they should indeed be exempt
>> from auto-init.  The
>> documentation suggests that they act as bit-bucked but even bit-buckets should
>> be initializable, thus why exactly does CONST0_RTX not exist for them?
> 
> We used to have CONST0_RTX defined (but nothing else), but we had problems
> with the compiler CSEing the initialization for multiple __vector_quads and
> then copying the values around.  We'd end up with one xxsetaccz instruction
> and copies out of that accumulator register into the other accumulator
> registers.  Copies are VERY expensive, while xxsetaccz's are cheap, so we
> don't want that.  That said, I think a fix I put in to disable fwprop on
> these types may have been the culprit for that problem, so maybe we could
> add the CONST0_RTX back?  I'd have to verify that.  If so, then we'd at least
> be able to support -ftrivial-auto-var-init=zero.  The =pattern version
> would be more problematical...unless the value for pattern was loaded from
> memory.
> 
> Peter
> 
>
  
Qing Zhao Nov. 30, 2021, 5:59 p.m. UTC | #5
> On Nov 30, 2021, at 2:37 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> Peter,
>> 
>> Thanks a lot for the patch.
>> 
>> Richard, how do you think of the patch?
>> 
>> (The major concern for me is:
>> 
>>        With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase,
>>         However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase.  This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later?
> 
> I think that's inconsistent indeed.

Can we treat the call to .DEFERRED_INIT to a NOP during expansion phase if we cannot expand it to a valid RTL for the OPAQUE_TYPE?  Will doing this resolve the issues?

>  Peter, what are "opaque"
> registers?  rs6000-modes.def suggests
> that there's __vector_pair and __vector_quad, what's the GIMPLE types
> for those?  It seems they
> are either SSA names or expanded to pseudo registers but there's no
> constants for them.
> 
>> 
>>         If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset?
>>            then we should update “is_var_need_auto_init” in gimplify.c instead.   However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory?
>> ).
> 
> I think we need to bite the bullet at some point to do register initialization
> not via expand_assignment but directly based on what the LHS expands to.

OPAQUE_TYPE is so special, it should not be the reason to rewrite the register initialization from my understanding. 
If later more issue exposed, it might be necessary to rewrite this part.

Qing
> 
> Can they be initialized?  I see they can be copied at least.
> 
> If such "things" cannot be initialized they should indeed be exempt
> from auto-init.  The
> documentation suggests that they act as bit-bucked but even bit-buckets should
> be initializable, thus why exactly does CONST0_RTX not exist for them?
> 
> Richard.
> 
> 
>> 
>> Thanks.
>> 
>> Qing
>> 
>> 
>>> On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote:
>>> 
>>> Sorry for dropping the ball on testing the patch from the bugzilla!
>>> 
>>> The following patch fixes the ICE reported in the bugzilla on the pre-existing
>>> gcc testsuite test case, bootstraps and shows no testsuite regressions
>>> on powerpc64le-linux.  Ok for trunk?
>>> 
>>> Peter
>>> 
>>> 
>>> For -ftrivial-auto-var-init=*, skip initializing the register variable if it
>>> is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes.
>>> 
>>> gcc/
>>>      PR middle-end/103127
>>>      * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque.
>>> 
>>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>>> index 0cba95411a6..7cc0e9d5293 100644
>>> --- a/gcc/internal-fn.c
>>> +++ b/gcc/internal-fn.c
>>> @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>>    }
>>>  else
>>>    {
>>> +      /* Skip variables of opaque types that are in a register.  */
>>> +      if (OPAQUE_TYPE_P (var_type))
>>> +     return;
>>> +
>>>      /* If this variable is in a register use expand_assignment.
>>>       For boolean scalars force zero-init.  */
>>>      tree init;
>>
  
Peter Bergner Nov. 30, 2021, 6:08 p.m. UTC | #6
On 11/30/21 11:51 AM, Qing Zhao wrote:
> So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. 
> 
> The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as:
> 
> void
> foo (__vector_quad *dst)
> {
>   __vector_quad acc;
>   __builtin_mma_xxsetaccz(&acc);
>   *dst = acc;
> }
> 
> Is this the correct understanding?

Correct.  Or via...


> Is there way to initialize such variable to other values than zero at source code level?

Not for any constant values.  You can load it from memory though like below,
which is also allowed for __vector_pair:

void
foo (__vector_quad *dst, __vector_quad *src)
{
  __vector_quad acc;
  acc = *src;
  ...
}
void
bar (__vector_pair *dst, __vector_pair *src)
{
  __vector_pair pair;
  pair = *src;
  ...
}

We do not accept things like:

  acc = 0;
  acc = {0, 0, ... };
  etc.

Peter
  
Qing Zhao Nov. 30, 2021, 7:50 p.m. UTC | #7
> On Nov 30, 2021, at 12:08 PM, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> On 11/30/21 11:51 AM, Qing Zhao wrote:
>> So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. 
>> 
>> The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as:
>> 
>> void
>> foo (__vector_quad *dst)
>> {
>>  __vector_quad acc;
>>  __builtin_mma_xxsetaccz(&acc);
>>  *dst = acc;
>> }
>> 
>> Is this the correct understanding?
> 
> Correct.  Or via...
> 
> 
>> Is there way to initialize such variable to other values than zero at source code level?
> 
> Not for any constant values.  You can load it from memory though like below,
> which is also allowed for __vector_pair:
> 
> void
> foo (__vector_quad *dst, __vector_quad *src)
> {
>  __vector_quad acc;
>  acc = *src;
>  ...
> }
> void
> bar (__vector_pair *dst, __vector_pair *src)
> {
>  __vector_pair pair;
>  pair = *src;
>  ...
> }

However, even with the above, the memory pointed by “src” still need to be initialized somewhere. How to provide the initial value to the variable in the beginning for __vector_pair type?

Qing
> 
> We do not accept things like:
> 
>  acc = 0;
>  acc = {0, 0, ... };
>  etc.
> 
> Peter
  
Peter Bergner Nov. 30, 2021, 8:07 p.m. UTC | #8
On 11/30/21 1:50 PM, Qing Zhao via Gcc-patches wrote:
>> void
>> bar (__vector_pair *dst, __vector_pair *src)
>> {
>>  __vector_pair pair;
>>  pair = *src;
>>  ...
>> }
> 
> However, even with the above, the memory pointed by “src” still need to
> be initialized somewhere. How to provide the initial value to the variable
> in the beginning for __vector_pair type?

Well no initialization is required here in this function.  Isn't that what
matters here?  When generating code for bar(), we assume that src already
points to initialized memory.

As for what src points to, that could be initialized how any other memory/
array could be initialized, so either a static array, read in some data
from a file into an array, compute the array values in a loop, etc. etc.

Peter
  
Qing Zhao Nov. 30, 2021, 8:44 p.m. UTC | #9
Sorry for the confusing…
My major question is:  

for a variable of type __vector_pair,  could it be in a register?
If it could be in a register, can we initialize this register with some constant value? 

Qing

> On Nov 30, 2021, at 2:07 PM, Peter Bergner <bergner@linux.ibm.com> wrote:
> 
> On 11/30/21 1:50 PM, Qing Zhao via Gcc-patches wrote:
>>> void
>>> bar (__vector_pair *dst, __vector_pair *src)
>>> {
>>> __vector_pair pair;
>>> pair = *src;
>>> ...
>>> }
>> 
>> However, even with the above, the memory pointed by “src” still need to
>> be initialized somewhere. How to provide the initial value to the variable
>> in the beginning for __vector_pair type?
> 
> Well no initialization is required here in this function.  Isn't that what
> matters here?  When generating code for bar(), we assume that src already
> points to initialized memory.
> 
> As for what src points to, that could be initialized how any other memory/
> array could be initialized, so either a static array, read in some data
> from a file into an array, compute the array values in a loop, etc. etc.
> 
> Peter
>
  
Peter Bergner Nov. 30, 2021, 10:35 p.m. UTC | #10
On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote:
> Sorry for the confusing…
> My major question is:  
> 
> for a variable of type __vector_pair,  could it be in a register?

Yes.  To be pedantic, it will live in a vector register pair.


> If it could be in a register, can we initialize this register with some constant value? 

For a __vector_pair, no, not as it is setup now.  We also do not have a
use case where we would want to initialize a __vector_pair to a constant.
Our normal (only?) use case with a __vector_pair is to load it up with
some actual data from memory that represents a (partial) row of a matrix. 

For __vector_quad, it too lives in a register (accumulator register) and
represents a small matrix.  We have the __builtin_mma_xxsetaccz (&acc)
builtin to initialize it to a zero constant.

Peter
  
Richard Biener Dec. 1, 2021, 9:01 a.m. UTC | #11
On Tue, Nov 30, 2021 at 11:35 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote:
> > Sorry for the confusing…
> > My major question is:
> >
> > for a variable of type __vector_pair,  could it be in a register?
>
> Yes.  To be pedantic, it will live in a vector register pair.
>
>
> > If it could be in a register, can we initialize this register with some constant value?
>
> For a __vector_pair, no, not as it is setup now.  We also do not have a
> use case where we would want to initialize a __vector_pair to a constant.
> Our normal (only?) use case with a __vector_pair is to load it up with
> some actual data from memory that represents a (partial) row of a matrix.
>
> For __vector_quad, it too lives in a register (accumulator register) and
> represents a small matrix.  We have the __builtin_mma_xxsetaccz (&acc)
> builtin to initialize it to a zero constant.

Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
instead of fixing up things at expansion time.

Richard.

> Peter
>
  
Qing Zhao Dec. 1, 2021, 3:06 p.m. UTC | #12
> On Dec 1, 2021, at 3:01 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, Nov 30, 2021 at 11:35 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>> 
>> On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote:
>>> Sorry for the confusing…
>>> My major question is:
>>> 
>>> for a variable of type __vector_pair,  could it be in a register?
>> 
>> Yes.  To be pedantic, it will live in a vector register pair.
>> 
>> 
>>> If it could be in a register, can we initialize this register with some constant value?
>> 
>> For a __vector_pair, no, not as it is setup now.  We also do not have a
>> use case where we would want to initialize a __vector_pair to a constant.
>> Our normal (only?) use case with a __vector_pair is to load it up with
>> some actual data from memory that represents a (partial) row of a matrix.
>> 
>> For __vector_quad, it too lives in a register (accumulator register) and
>> represents a small matrix.  We have the __builtin_mma_xxsetaccz (&acc)
>> builtin to initialize it to a zero constant.
> 
> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
> instead of fixing up things at expansion time.

Agreed. 

So, Peter, will you update the routine “is_var_need_auto_init” in gimplify.c to exclude OPAQUE_TYPE to fix this issue?

thanks.

Qing
> 
> Richard.
> 
>> Peter
  
Peter Bergner Dec. 1, 2021, 4:08 p.m. UTC | #13
On 12/1/21 9:06 AM, Qing Zhao wrote:
>> On Dec 1, 2021, at 3:01 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
>> instead of fixing up things at expansion time.
> 
> Agreed. 
> 
> So, Peter, will you update the routine “is_var_need_auto_init” in gimplify.c
> to exclude OPAQUE_TYPE to fix this issue?

Sure, I can give that a try and verify it fixes the ICE too.

Peter
  
Peter Bergner Dec. 1, 2021, 5:42 p.m. UTC | #14
On 12/1/21 3:01 AM, Richard Biener wrote:
> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
> instead of fixing up things at expansion time.

The following fixes the ICE.  The bootstrap/regtesting is still running though.

Peter


diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8624f8221fd..326476f0238 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl)
          || !DECL_HARD_REGISTER (decl))
       && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED)
       && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl)))
+      && !OPAQUE_TYPE_P (TREE_TYPE (decl))
       && !is_empty_type (TREE_TYPE (decl)))
     return true;
   return false;
  
Richard Biener Dec. 1, 2021, 7:07 p.m. UTC | #15
On December 1, 2021 6:42:21 PM GMT+01:00, Peter Bergner <bergner@linux.ibm.com> wrote:
>On 12/1/21 3:01 AM, Richard Biener wrote:
>> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
>> instead of fixing up things at expansion time.
>
>The following fixes the ICE.  The bootstrap/regtesting is still running though.

OK. 

Thanks, 
Richard. 

>Peter
>
>
>diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>index 8624f8221fd..326476f0238 100644
>--- a/gcc/gimplify.c
>+++ b/gcc/gimplify.c
>@@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl)
>          || !DECL_HARD_REGISTER (decl))
>       && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED)
>       && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl)))
>+      && !OPAQUE_TYPE_P (TREE_TYPE (decl))
>       && !is_empty_type (TREE_TYPE (decl)))
>     return true;
>   return false;
  
Peter Bergner Dec. 1, 2021, 7:29 p.m. UTC | #16
On 12/1/21 1:07 PM, Richard Biener wrote:
> On December 1, 2021 6:42:21 PM GMT+01:00, Peter Bergner <bergner@linux.ibm.com> wrote:
>> On 12/1/21 3:01 AM, Richard Biener wrote:
>>> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init
>>> instead of fixing up things at expansion time.
>>
>> The following fixes the ICE.  The bootstrap/regtesting is still running though.
> 
> OK. 

Great, pushed.  For posterity, below is what I committed.

Peter


middle-end: Skip initialization of opaque type variables [PR103127]

For -ftrivial-auto-var-init=*, skip initializing the variable if it is an
opaque type, because CONST0_RTX(mode) is not defined for opaque modes.

2021-12-01  Peter Bergner  <bergner@linux.ibm.com>

gcc/
	PR middle-end/103127
	* gimplify.c (is_var_need_auto_init): Handle opaque types.

gcc/testsuite/
	PR middle-end/103127
	* gcc.target/powerpc/pr103127.c: New test.

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8624f8221fd..326476f0238 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl)
 	  || !DECL_HARD_REGISTER (decl))
       && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED)
       && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl)))
+      && !OPAQUE_TYPE_P (TREE_TYPE (decl))
       && !is_empty_type (TREE_TYPE (decl)))
     return true;
   return false;
diff --git a/gcc/testsuite/gcc.target/powerpc/pr103127.c b/gcc/testsuite/gcc.target/powerpc/pr103127.c
new file mode 100644
index 00000000000..801fc0a4620
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr103127.c
@@ -0,0 +1,19 @@
+/* PR target/103127 */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10 -ftrivial-auto-var-init=zero" } */
+
+/* Verify we do not ICE on the following tests.  */
+
+void
+foo (__vector_quad *dst)
+{
+  __vector_quad acc;
+  *dst = acc;
+}
+
+void
+bar (__vector_pair *dst)
+{
+  __vector_pair pair;
+  *dst = pair;
+}
  

Patch

diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 0cba95411a6..7cc0e9d5293 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3070,6 +3070,10 @@  expand_DEFERRED_INIT (internal_fn, gcall *stmt)
     }
   else
     {
+      /* Skip variables of opaque types that are in a register.  */
+      if (OPAQUE_TYPE_P (var_type))
+	return;
+
       /* If this variable is in a register use expand_assignment.
 	 For boolean scalars force zero-init.  */
       tree init;