So, based on the discussion so far, is the following patch good to go?
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.guent...@gmail.com> wrote:
>
> On Tue, Nov 9, 2021 at 10:10 AM Jakub Jelinek <ja...@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
