Values of WIDE_INT_MAX_ELTS in gcc11 and gcc12 are different

Qing Zhao qing.zhao@oracle.com
Wed Nov 10 18:02:12 GMT 2021


Pushed the patch as:

https://gcc.gnu.org/pipermail/gcc-cvs/2021-November/356543.html

Qing


> On Nov 10, 2021, at 2:37 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, Nov 9, 2021 at 6:48 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> So, based on the discussion so far,  is the following patch good to go?
> 
> OK.
> 
> Thanks,
> Richard.
> 
>> Let me know if you have more comments on the following patch:
>> 
>> (At the same time, I am testing this patch on both x86 and aarch64)
>> 
>> thanks.
>> 
>> Qing
>> 
>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>> index 0cba95411a6..e8fd16b9c21 100644
>> --- a/gcc/internal-fn.c
>> +++ b/gcc/internal-fn.c
>> @@ -3059,10 +3059,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>       mark_addressable (lhs);
>>       tree var_addr = build_fold_addr_expr (lhs);
>> 
>> -      tree value = (init_type == AUTO_INIT_PATTERN) ?
>> -                   build_int_cst (integer_type_node,
>> -                                  INIT_PATTERN_VALUE) :
>> -                   integer_zero_node;
>> +      tree value = (init_type == AUTO_INIT_PATTERN)
>> +                   ? build_int_cst (integer_type_node,
>> +                                    INIT_PATTERN_VALUE)
>> +                   : integer_zero_node;
>>       tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
>>                                     3, var_addr, value, var_size);
>>       /* Expand this memset call.  */
>> @@ -3073,15 +3073,17 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>       /* If this variable is in a register use expand_assignment.
>>         For boolean scalars force zero-init.  */
>>       tree init;
>> +      scalar_int_mode var_mode;
>>       if (TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE
>>          && tree_fits_uhwi_p (var_size)
>>          && (init_type == AUTO_INIT_PATTERN
>>              || !is_gimple_reg_type (var_type))
>>          && int_mode_for_size (tree_to_uhwi (var_size) * BITS_PER_UNIT,
>> -                               0).exists ())
>> +                               0).exists (&var_mode)
>> +         && have_insn_for (SET, var_mode))
>>        {
>>          unsigned HOST_WIDE_INT total_bytes = tree_to_uhwi (var_size);
>> -         unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>> +         unsigned char *buf = XALLOCAVEC (unsigned char, total_bytes);
>>          memset (buf, (init_type == AUTO_INIT_PATTERN
>>                        ? INIT_PATTERN_VALUE : 0), total_bytes);
>>          tree itype = build_nonstandard_integer_type
>> diff --git a/gcc/testsuite/gcc.target/i386/auto-init-6.c b/gcc/testsuite/gcc.target/i386/auto-init-6.c
>> index 339f8bc2966..e53385f0eb7 100644
>> --- a/gcc/testsuite/gcc.target/i386/auto-init-6.c
>> +++ b/gcc/testsuite/gcc.target/i386/auto-init-6.c
>> @@ -1,4 +1,6 @@
>> /* Verify pattern initialization for complex type automatic variables.  */
>> +/* Note, _Complex long double is initialized to zeroes due to the current
>> +   implemenation limitation.  */
>> /* { dg-do compile } */
>> /* { dg-options "-ftrivial-auto-var-init=pattern -march=x86-64 -mtune=generic -msse" } */
>> 
>> @@ -15,6 +17,6 @@ _Complex long double foo()
>>   return result;
>> }
>> 
>> -/* { dg-final { scan-assembler-times "long\t-16843010" 10  { target { ! ia32 } } } } */
>> -/* { dg-final { scan-assembler-times "long\t-16843010" 6  { target { ia32 } } } } */
>> +/* { dg-final { scan-assembler-times "long\t0" 8  { target { ! ia32 } } } } */
>> +/* { dg-final { scan-assembler-times "long\t-16843010" 6  } } */
>> 
>> 
>> 
>> 
>>> On Nov 9, 2021, at 4:44 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> On Tue, Nov 9, 2021 at 10:10 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>>> 
>>>> On Tue, Nov 09, 2021 at 08:13:57AM +0100, Richard Biener wrote:
>>>>>> Hi, I tried both the following patches:
>>>>>> 
>>>>>> Patch1:
>>>>>> 
>>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ git diff
>>>>>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>>>>>> index 0cba95411a6..ca49d2b4514 100644
>>>>>> --- a/gcc/internal-fn.c
>>>>>> +++ b/gcc/internal-fn.c
>>>>>> @@ -3073,12 +3073,14 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>>>>>      /* If this variable is in a register use expand_assignment.
>>>>>>        For boolean scalars force zero-init.  */
>>>>>>      tree init;
>>>>>> +      scalar_int_mode var_mode;
>>>>>>      if (TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE
>>>>>>         && tree_fits_uhwi_p (var_size)
>>>>>>         && (init_type == AUTO_INIT_PATTERN
>>>>>>             || !is_gimple_reg_type (var_type))
>>>>>>         && int_mode_for_size (tree_to_uhwi (var_size) * BITS_PER_UNIT,
>>>>>> -                               0).exists ())
>>>>>> +                               0).exists (&var_mode)
>>>>>> +         && targetm.scalar_mode_supported_p (var_mode))
>>>>>>       {
>>>>>>         unsigned HOST_WIDE_INT total_bytes = tree_to_uhwi (var_size);
>>>>>>         unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>>>>>> 
>>>>>> AND
>>>>>> 
>>>>>> Patch2:
>>>>>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>>>>>> index 0cba95411a6..7f129655926 100644
>>>>>> --- a/gcc/internal-fn.c
>>>>>> +++ b/gcc/internal-fn.c
>>>>>> @@ -3073,12 +3073,14 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>>>>>      /* If this variable is in a register use expand_assignment.
>>>>>>        For boolean scalars force zero-init.  */
>>>>>>      tree init;
>>>>>> +      scalar_int_mode var_mode;
>>>>>>      if (TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE
>>>>>>         && tree_fits_uhwi_p (var_size)
>>>>>>         && (init_type == AUTO_INIT_PATTERN
>>>>>>             || !is_gimple_reg_type (var_type))
>>>>>>         && int_mode_for_size (tree_to_uhwi (var_size) * BITS_PER_UNIT,
>>>>>> -                               0).exists ())
>>>>>> +                               0).exists (&var_mode)
>>>>>> +         && have_insn_for (SET, var_mode))
>>>>>>       {
>>>>>>         unsigned HOST_WIDE_INT total_bytes = tree_to_uhwi (var_size);
>>>>>>         unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>>>>>> 
>>>>>> Have the same effect:
>>>>>> 
>>>>>> 1. Resolved the ICE in gcc11;
>>>>>> 2. For _Complex long double variables, both return FALSE, as a result, for PATTERN initialization of _Complex long double variables, now they are initialization with ZEROs instead of FEs.
>>>>>> 
>>>>>> Let me know you opinion on this, If the above 2 is okay, then I might pick the above Patch 1 for the final patch to this issue.
>>>>> 
>>>>> I think zero-initialization is OK, but I'd choose Patch2 for
>>>>> consistency with what we do in the memcpy
>>>>> folding.
>>>> 
>>>> Note, I think the code leaks memory (buf is never freed) and
>>>> should be allocated using
>>>> unsigned char *buf = XNEWVEC (unsigned char *, total_bytes);
>>>> and deallocated with XDELETEVEC (buf);
>>> 
>>> Oops.  I think the intention was to use XALLOCAVEC
>>> 
>>>> If lhs is SSA_NAME, I think it would be easiest to use
>>>> native_interpret_expr directly instead of finding some integral
>>>> mode, after memset just native_interpret_expr.
>>> 
>>> I've removed the native_interpret_expr path, I think it would be best to not
>>> rely on expand_assignment for the case of a register destination.  The
>>> intent is to do an integer move into the destination and avoid issues with
>>> things like XFmode or other modes with padding (and possibly trapping loads).
>>> So it should become some
>>> 
>>> (set (subreg:IntMode ...) (const_wide_int ...))
>>> 
>>> but my RTL fu is too weak to try and so we ended up with expand_assignment
>>> which doesn't like V_C_E on SSA names on the LHS (well, because that's
>>> not valid GIMPLE...).
>>> 
>>>> Should be guarded by
>>>> BITS_PER_UNIT == 8 && CHAR_BIT == 8 && BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN
>>>> but I think the current code assumes at least that BITS_PER_UNIT == CHAR_BIT
>>>> too anyway.
>>>> That way you get directly a constant init instead of hopping through
>>>> some integral mode which might not exist at all.
>>>> If lhs is not SSA_NAME but var_type has COMPLEX_TYPE or VECTOR_TYPE, there
>>>> is always an option to find var_mode only for the element type (i.e.
>>>> TREE_TYPE (var_type), use var_size for the element size too, and after
>>>> you compute init fold_build2 (COMPLEX_EXPR, ) or build_vector_from_val.
>>>> For VECTOR_TYPE one would need to check if the vector mode is supported,
>>>> sure (but if it isn't, it will go the BUILT_IN_MEMSET way, I'm pretty sure).
>>>> 
>>>> Also,
>>>>     tree value = (init_type == AUTO_INIT_PATTERN) ?
>>>>                   build_int_cst (integer_type_node,
>>>>                                  INIT_PATTERN_VALUE) :
>>>>                   integer_zero_node;
>>>> a few lines before has bad formatting, both ? and : should be at the start
>>>> of next lines rather than on end of the previous ones.
>>>> 
>>>>       Jakub
>> 



More information about the Gcc-patches mailing list