Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Sorry for my late reply (I just came back from vacation last night).
> On Aug 23, 2021, at 8:55 AM, Richard Biener wrote:
>
Looks like for the following code:
3026 if (!reg_lhs)
3027 {
3028 /* If this is a VLA or the variable is not in register,
3029expand to a memset to initialize it. */
3030 mark_addressable (lhs);
3031 tree var_addr = build_fold_addr_expr (lhs);
3032
3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
3034 build_int_cst (integer_type_node,
3035INIT_PATTERN_VALUE) :
3036 integer_zero_node;
3037 tree m_call = build_call_expr (builtin_decl_implicit
(BUILT_IN_MEMSET),
3038 3, var_addr, value, var_size);
3039 /* Expand this memset call. */
3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
3041 }
At line 3030, “lhs” could be a SSA_NAME.
My questions are:
1. Could the routine “mark_addressable” and “build_fold_addr_expr” be
applied on SSA_NAME?
>>>
>>> No.
>>>
2. Could the routine “expand_builtin_memset” be applied on the memset call
whose “DEST” is
an address expression on SSA_NAME?
>>>
>>> No.
>>>
3. Within “expand_DEFERRED_INIT”, can I call “expand_builtin_memset” to
expand .DEFERRED_INIT?
>>>
>>> Well, not with "invalid" GENERIC I fear (address of a SSA name).
>>>
I suspect that one of the above 3 might be the issue, but not sure which
one?
>>>
>>> All of the above ;) So while reg_lhs is now precise as to how the
>>> variable will end up (the SSA name will end up as a stack variable in this
>>> case, for whatever reason), expansion via memcpy only works when
>>> working on the RTL representation. The usual "workaround" (ugh)
>>> is to use make_tree (), so in the !reg_lhs path you'd do
>>>
>>> /* Get a new GENERIC representation for the RTL. That's necesary
>>>in case LHS is an SSA name. */
>>> lhs = make_tree (TREE_TYPE (lhs), tem);
>>
>> This resolved the issue.
>>
>> Another question,
>>
>> Previously, I used
>>
>>if (TREE_CODE (lhs) == SSA_NAME)
>> lhs = SSA_NAME_VAR (lhs);
>>
>> To resolve this issue. The purpose looks like the same as “make_tree”, just
>> get an generic tree for the LHS.
>>
>> Why you said using SSA_NAME_VAR is broken? Is it because SSA_NAME_VAR will
>> not always return a valid TREE?
>
> Because it's simply the wrong entity - I have no idea why that even
> worked. Ah, cfgexpand associates it with some DECL_RTL for the
> benefit of debug info. But it's still wrong.
>
>> I should use as following
>>
>>
>> If (TREE_CODE (lhs) == SSA_NAME) && SSA_NAME_VAR (lhs))
>> Lhs = SSA_NAME_VAR (lhs)
>>
>> ?
>
> No. A SSA_NAME_VAR can have multiple SSA_NAMEs (obviously) and
> they do not necessarily have to be allocated to the same variable
> partition - that is, there's no 1:1 relationship between SSA_NAME
> and stack slot or (pseudo) register. You want to initialize the
> storage associated with the SSA_NAME in the .DEFERRED_INIT call,
> not some other storage.
Okay, thanks for the explanation. Now I understand.
>
>>>
>>> alternatively you could maybe do
>>>
>>> if (DECL_P (lhs))
>>> {
>>> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>>> + reg_lhs = !MEM_P (tem);
>>> }
>>> else if (TREE_CODE (lhs) == SSA_NAME)
>>> reg_lhs = true;
>>> else
>>> reg_lhs = false;
>>>
>>> thus treat SSA names as register storage always (even if it will end
>>> up on the stack).
>>
>> My question here, for a complicate structure SSA_NAME, will expanding
>> through memset better than expand_asssignment?
>
> It depends. In the end I'd consider it a missed-optimization bug on
> the side that generates worse code - but I do expect cases will exist
> for both.
Okay. I agree.
> Clearly memset will be worse when dealing with register
> initialization (thus the !MEM_P check) and I expect memset to be OK
> for stack where member-wise init esp. with non-zero might turn up
> worse code.
So, for SSA_NAME, since they are all treated as reg_lhs, they will be expanded
through “expand_assignment”
as member-wise init, therefore might generate worse code for a SSA_NAME that is
in stack actually.
Qing
>
> Richard.
>
>> Qing
>>>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Thu, 19 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 19, 2021, at 4:00 AM, Richard Biener wrote:
> >
> > On Wed, 18 Aug 2021, Qing Zhao wrote:
> >
> >>
> >>
> >>> On Aug 18, 2021, at 2:15 AM, Richard Biener wrote:
> >>>
> >>> On Tue, 17 Aug 2021, Qing Zhao wrote:
> >>>
>
>
> > On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
> > wrote:
> >
> >
> >
> >> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
> >>
> >> On Mon, 16 Aug 2021, Qing Zhao wrote:
> >>
> >>> My current code for expand_DEFERRED_INIT is like the following, could
> >>> you check and see whether there is any issue for it:
> >>>
> >>> #define INIT_PATTERN_VALUE 0xFE
> >>> static void
> >>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> >>> {
> >>> tree lhs = gimple_call_lhs (stmt);
> >>> tree var_size = gimple_call_arg (stmt, 0);
> >>> enum auto_init_type init_type
> >>> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> >>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> >>>
> >>> tree var_type = TREE_TYPE (lhs);
> >>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> >>>
> >>> if (is_vla || (!use_register_for_decl (lhs)))
> >>> {
> >>> if (TREE_CODE (lhs) == SSA_NAME)
> >>> lhs = SSA_NAME_VAR (lhs);
> >>
> >> this should not be necessary (in fact you shouldn't see a SSA_NAME
> >> here, if you do then using SSA_NAME_VAR is wrong)
> > You mean during RTL expansion phase, all SSA_NAMEs are gone already?
>
> Actually, the lhs could be SSA_NAME here,
>
> Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
> ../../latest-gcc/gcc/internal-fn.c:3021
> 3021 mark_addressable (lhs);
> (gdb) call debug_tree(lhs)
> type size
> unit-size
> align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
> 0x7fffe959b2a0 precision:32
> pointer_to_this >
> visited var
> def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
> version:5>
>
> when I deleted:
>
> if (TREE_CODE (lhs) == SSA_NAME
> lhs = SSA_NAME_VAR (lhs);
> >>>
> >>> but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
> >>> isn't the correct thing to look at. I think we need to look at what
> >>> the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
> >>> but also plain DECLs but not what we get from VLAs where we'd see
> >>> *ptr). So sth like
> >>>
> >>> bool reg_lhs;
> >>> if (SSA_VAR_P (lhs))
> >>> {
> >>> rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> >>> reg_lhs = !MEM_P (tem);
> >>> /* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
> >>>also CONCAT or lowpart...?) */
> >>> }
> >>> else
> >>> {
> >>> gcc_assert (is_vla);
> >>> reg_lhs = false;
> >>> }
> >>>
> >>> if (!reg_lhs)
> >>> memset path
> >>> else
> >>> expand_assignment path
> >>
> >> After making the following change:
> >>
> >> + bool reg_lhs = true;
> >>
> >> tree var_type = TREE_TYPE (lhs);
> >> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> >>
> >> - if (is_vla || (!use_register_for_decl (lhs)))
> >> + if (SSA_VAR_P (lhs))
> >> +{
> >> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> >> + reg_lhs = !MEM_P (tem);
> >> +}
> >> + else
> >> +{
> >> + gcc_assert (is_vla);
> >> + reg_lhs = false;
> >> +}
> >> +
> >> + if (!reg_lhs)
> >> {
> >>
> >> I got exactly the same internal error that failed at expr.c:
> >>
> >> 8436 /* We must have made progress. */
> >> 8437 gcc_assert (inner != exp);
> >>
> >>
> >> Looks like for the following code:
> >>
> >> 3026 if (!reg_lhs)
> >> 3027 {
> >> 3028 /* If this is a VLA or the variable is not in register,
> >> 3029expand to a memset to initialize it. */
> >> 3030 mark_addressable (lhs);
> >> 3031 tree var_addr = build_fold_addr_expr (lhs);
> >> 3032
> >> 3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
> >> 3034 build_int_cst (integer_type_node,
> >> 3035INIT_PATTERN_VALUE) :
> >> 3036 integer_zero_node;
> >> 3037 tree m_call = build_call_expr (builtin_decl_implicit
> >> (BUILT_IN_MEMSET),
> >> 3038 3, var_addr, value, var_size);
> >> 3039 /* Expand this memset call. */
> >> 3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> >> 3041 }
> >>
> >> At line 3030, “lhs” could be a SSA_NAME.
> >>
> >> My questions are:
> >>
> >> 1. Could the routine “mark_addressable” and “build_fold_addr_expr” be
> >> applied on SSA_NAME?
> >
> > No.
> >
> >> 2. Could the routine “expand_builtin_memset” be applied on the
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 19, 2021, at 8:54 AM, Qing Zhao via Gcc-patches
> wrote:
>
> Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
> ../../latest-gcc/gcc/internal-fn.c:3021
> 3021mark_addressable (lhs);
> (gdb) call debug_tree(lhs)
> type size
> unit-size
> align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
> 0x7fffe959b2a0 precision:32
> pointer_to_this >
> visited var
> def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
> version:5>
>
> when I deleted:
>
> if (TREE_CODE (lhs) == SSA_NAME
> lhs = SSA_NAME_VAR (lhs);
but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
isn't the correct thing to look at. I think we need to look at what
the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
but also plain DECLs but not what we get from VLAs where we'd see
*ptr). So sth like
bool reg_lhs;
if (SSA_VAR_P (lhs))
{
rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
reg_lhs = !MEM_P (tem);
/* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
also CONCAT or lowpart...?) */
}
else
{
gcc_assert (is_vla);
reg_lhs = false;
}
if (!reg_lhs)
memset path
else
expand_assignment path
>>>
>>> After making the following change:
>>>
>>> + bool reg_lhs = true;
>>>
>>> tree var_type = TREE_TYPE (lhs);
>>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>>
>>> - if (is_vla || (!use_register_for_decl (lhs)))
>>> + if (SSA_VAR_P (lhs))
>>> +{
>>> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>>> + reg_lhs = !MEM_P (tem);
>>> +}
>>> + else
>>> +{
>>> + gcc_assert (is_vla);
>>> + reg_lhs = false;
>>> +}
>>> +
>>> + if (!reg_lhs)
>>>{
>>>
>>> I got exactly the same internal error that failed at expr.c:
>>>
>>> 8436 /* We must have made progress. */
>>> 8437 gcc_assert (inner != exp);
>>>
>>>
>>> Looks like for the following code:
>>>
>>> 3026 if (!reg_lhs)
>>> 3027 {
>>> 3028 /* If this is a VLA or the variable is not in register,
>>> 3029expand to a memset to initialize it. */
>>> 3030 mark_addressable (lhs);
>>> 3031 tree var_addr = build_fold_addr_expr (lhs);
>>> 3032
>>> 3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
>>> 3034 build_int_cst (integer_type_node,
>>> 3035INIT_PATTERN_VALUE) :
>>> 3036 integer_zero_node;
>>> 3037 tree m_call = build_call_expr (builtin_decl_implicit
>>> (BUILT_IN_MEMSET),
>>> 3038 3, var_addr, value, var_size);
>>> 3039 /* Expand this memset call. */
>>> 3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>> 3041 }
>>>
>>> At line 3030, “lhs” could be a SSA_NAME.
>>>
>>> My questions are:
>>>
>>> 1. Could the routine “mark_addressable” and “build_fold_addr_expr” be
>>> applied on SSA_NAME?
>>
>> No.
>>
>>> 2. Could the routine “expand_builtin_memset” be applied on the memset call
>>> whose “DEST” is
>>> an address expression on SSA_NAME?
>>
>> No.
>>
>>> 3. Within “expand_DEFERRED_INIT”, can I call “expand_builtin_memset” to
>>> expand .DEFERRED_INIT?
>>
>> Well, not with "invalid" GENERIC I fear (address of a SSA name).
>>
>>> I suspect that one of the above 3 might be the issue, but not sure which
>>> one?
>>
>> All of the above ;) So while reg_lhs is now precise as to how the
>> variable will end up (the SSA name will end up as a stack variable in this
>> case, for whatever reason), expansion via memcpy only works when
>> working on the RTL representation. The usual "workaround" (ugh)
>> is to use make_tree (), so in the !reg_lhs path you'd do
>>
>> /* Get a new GENERIC representation for the RTL. That's necesary
>>in case LHS is an SSA name. */
>> lhs = make_tree (TREE_TYPE (lhs), tem);
>
> This resolved the issue.
However, there was two runtime errors with CPU2017 with this change.
I guess that there might be some issue with “expand_expr” + “make_tree” here.
So, I changed the code to use the alternative solution that you suggested:
>
>>
>> alternatively you could maybe do
>>
>> if (DECL_P (lhs))
>> {
>> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>> + reg_lhs = !MEM_P (tem);
>> }
>> else if (TREE_CODE (lhs) == SSA_NAME)
>> reg_lhs = true;
>> else
>> reg_lhs = false;
>>
>> thus treat SSA names as register storage always (even if it will end
>> up on the stack).
This did resolve all the issues including CPU2017 runtime errors.
With this solution, all SSA_NAMEs will be expanded through “expand_assignment”
even though they are in stack.
the generated code will be correct. But the performance might be
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 19, 2021, at 4:00 AM, Richard Biener wrote:
>
> On Wed, 18 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 18, 2021, at 2:15 AM, Richard Biener wrote:
>>>
>>> On Tue, 17 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
> wrote:
>
>
>
>> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
>>
>> On Mon, 16 Aug 2021, Qing Zhao wrote:
>>
>>> My current code for expand_DEFERRED_INIT is like the following, could
>>> you check and see whether there is any issue for it:
>>>
>>> #define INIT_PATTERN_VALUE 0xFE
>>> static void
>>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>> {
>>> tree lhs = gimple_call_lhs (stmt);
>>> tree var_size = gimple_call_arg (stmt, 0);
>>> enum auto_init_type init_type
>>> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>>
>>> tree var_type = TREE_TYPE (lhs);
>>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>>
>>> if (is_vla || (!use_register_for_decl (lhs)))
>>> {
>>> if (TREE_CODE (lhs) == SSA_NAME)
>>> lhs = SSA_NAME_VAR (lhs);
>>
>> this should not be necessary (in fact you shouldn't see a SSA_NAME
>> here, if you do then using SSA_NAME_VAR is wrong)
> You mean during RTL expansion phase, all SSA_NAMEs are gone already?
Actually, the lhs could be SSA_NAME here,
Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
../../latest-gcc/gcc/internal-fn.c:3021
3021 mark_addressable (lhs);
(gdb) call debug_tree(lhs)
>>> type >>> size
unit-size
align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
0x7fffe959b2a0 precision:32
pointer_to_this >
visited var
def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
version:5>
when I deleted:
if (TREE_CODE (lhs) == SSA_NAME
lhs = SSA_NAME_VAR (lhs);
>>>
>>> but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
>>> isn't the correct thing to look at. I think we need to look at what
>>> the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
>>> but also plain DECLs but not what we get from VLAs where we'd see
>>> *ptr). So sth like
>>>
>>> bool reg_lhs;
>>> if (SSA_VAR_P (lhs))
>>> {
>>> rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>>> reg_lhs = !MEM_P (tem);
>>> /* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
>>>also CONCAT or lowpart...?) */
>>> }
>>> else
>>> {
>>> gcc_assert (is_vla);
>>> reg_lhs = false;
>>> }
>>>
>>> if (!reg_lhs)
>>> memset path
>>> else
>>> expand_assignment path
>>
>> After making the following change:
>>
>> + bool reg_lhs = true;
>>
>> tree var_type = TREE_TYPE (lhs);
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>
>> - if (is_vla || (!use_register_for_decl (lhs)))
>> + if (SSA_VAR_P (lhs))
>> +{
>> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
>> + reg_lhs = !MEM_P (tem);
>> +}
>> + else
>> +{
>> + gcc_assert (is_vla);
>> + reg_lhs = false;
>> +}
>> +
>> + if (!reg_lhs)
>> {
>>
>> I got exactly the same internal error that failed at expr.c:
>>
>> 8436 /* We must have made progress. */
>> 8437 gcc_assert (inner != exp);
>>
>>
>> Looks like for the following code:
>>
>> 3026 if (!reg_lhs)
>> 3027 {
>> 3028 /* If this is a VLA or the variable is not in register,
>> 3029expand to a memset to initialize it. */
>> 3030 mark_addressable (lhs);
>> 3031 tree var_addr = build_fold_addr_expr (lhs);
>> 3032
>> 3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
>> 3034 build_int_cst (integer_type_node,
>> 3035INIT_PATTERN_VALUE) :
>> 3036 integer_zero_node;
>> 3037 tree m_call = build_call_expr (builtin_decl_implicit
>> (BUILT_IN_MEMSET),
>> 3038 3, var_addr, value, var_size);
>> 3039 /* Expand this memset call. */
>> 3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>> 3041 }
>>
>> At line 3030, “lhs” could be a SSA_NAME.
>>
>> My questions are:
>>
>> 1. Could the routine “mark_addressable” and “build_fold_addr_expr” be
>> applied on SSA_NAME?
>
> No.
>
>> 2. Could the routine “expand_builtin_memset” be applied on the memset call
>> whose “DEST” is
>>an address expression on SSA_NAME?
>
> No.
>
>> 3. Within “expand_DEFERRED_INIT”, can I call “expand_builtin_memset” to
>> expand .DEFERRED_INIT?
>
> Well, not with "invalid" GENERIC I fear (address of a SSA name).
>
>> I suspect that one of the above 3 might be the issue, but not sure which one?
>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Wed, 18 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 18, 2021, at 2:15 AM, Richard Biener wrote:
> >
> > On Tue, 17 Aug 2021, Qing Zhao wrote:
> >
> >>
> >>
> >>> On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
> >>> wrote:
> >>>
> >>>
> >>>
> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
>
> On Mon, 16 Aug 2021, Qing Zhao wrote:
>
> > My current code for expand_DEFERRED_INIT is like the following, could
> > you check and see whether there is any issue for it:
> >
> > #define INIT_PATTERN_VALUE 0xFE
> > static void
> > expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> > {
> > tree lhs = gimple_call_lhs (stmt);
> > tree var_size = gimple_call_arg (stmt, 0);
> > enum auto_init_type init_type
> > = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> > bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> >
> > tree var_type = TREE_TYPE (lhs);
> > gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> >
> > if (is_vla || (!use_register_for_decl (lhs)))
> > {
> >if (TREE_CODE (lhs) == SSA_NAME)
> > lhs = SSA_NAME_VAR (lhs);
>
> this should not be necessary (in fact you shouldn't see a SSA_NAME
> here, if you do then using SSA_NAME_VAR is wrong)
> >>> You mean during RTL expansion phase, all SSA_NAMEs are gone already?
> >>
> >> Actually, the lhs could be SSA_NAME here,
> >>
> >> Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
> >> ../../latest-gcc/gcc/internal-fn.c:3021
> >> 3021 mark_addressable (lhs);
> >> (gdb) call debug_tree(lhs)
> >> >>type >>size
> >>unit-size
> >>align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
> >> 0x7fffe959b2a0 precision:32
> >>pointer_to_this >
> >>visited var
> >>def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
> >>version:5>
> >>
> >> when I deleted:
> >>
> >> if (TREE_CODE (lhs) == SSA_NAME
> >> lhs = SSA_NAME_VAR (lhs);
> >
> > but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
> > isn't the correct thing to look at. I think we need to look at what
> > the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
> > but also plain DECLs but not what we get from VLAs where we'd see
> > *ptr). So sth like
> >
> > bool reg_lhs;
> > if (SSA_VAR_P (lhs))
> >{
> > rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> > reg_lhs = !MEM_P (tem);
> > /* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
> > also CONCAT or lowpart...?) */
> >}
> > else
> >{
> > gcc_assert (is_vla);
> > reg_lhs = false;
> >}
> >
> > if (!reg_lhs)
> >memset path
> > else
> >expand_assignment path
>
> After making the following change:
>
> + bool reg_lhs = true;
>
>tree var_type = TREE_TYPE (lhs);
>gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> - if (is_vla || (!use_register_for_decl (lhs)))
> + if (SSA_VAR_P (lhs))
> +{
> + rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> + reg_lhs = !MEM_P (tem);
> +}
> + else
> +{
> + gcc_assert (is_vla);
> + reg_lhs = false;
> +}
> +
> + if (!reg_lhs)
> {
>
> I got exactly the same internal error that failed at expr.c:
>
> 8436 /* We must have made progress. */
> 8437 gcc_assert (inner != exp);
>
>
> Looks like for the following code:
>
> 3026 if (!reg_lhs)
> 3027 {
> 3028 /* If this is a VLA or the variable is not in register,
> 3029expand to a memset to initialize it. */
> 3030 mark_addressable (lhs);
> 3031 tree var_addr = build_fold_addr_expr (lhs);
> 3032
> 3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
> 3034 build_int_cst (integer_type_node,
> 3035INIT_PATTERN_VALUE) :
> 3036 integer_zero_node;
> 3037 tree m_call = build_call_expr (builtin_decl_implicit
> (BUILT_IN_MEMSET),
> 3038 3, var_addr, value, var_size);
> 3039 /* Expand this memset call. */
> 3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> 3041 }
>
> At line 3030, “lhs” could be a SSA_NAME.
>
> My questions are:
>
> 1. Could the routine “mark_addressable” and “build_fold_addr_expr” be applied
> on SSA_NAME?
No.
> 2. Could the routine “expand_builtin_memset” be applied on the memset call
> whose “DEST” is
> an address expression on SSA_NAME?
No.
> 3. Within “expand_DEFERRED_INIT”, can I call “expand_builtin_memset” to
> expand .DEFERRED_INIT?
Well, not with "invalid" GENERIC I fear (address of a SSA name).
> I suspect that one of the above 3 might be the issue, but not sure which one?
All of the above ;) So while reg_lhs is now precise as to how the
variable will end up (the SSA name will
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 18, 2021, at 2:15 AM, Richard Biener wrote:
>
> On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>>
>>>
On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
On Mon, 16 Aug 2021, Qing Zhao wrote:
> My current code for expand_DEFERRED_INIT is like the following, could you
> check and see whether there is any issue for it:
>
> #define INIT_PATTERN_VALUE 0xFE
> static void
> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> {
> tree lhs = gimple_call_lhs (stmt);
> tree var_size = gimple_call_arg (stmt, 0);
> enum auto_init_type init_type
> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>
> tree var_type = TREE_TYPE (lhs);
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> if (is_vla || (!use_register_for_decl (lhs)))
> {
>if (TREE_CODE (lhs) == SSA_NAME)
> lhs = SSA_NAME_VAR (lhs);
this should not be necessary (in fact you shouldn't see a SSA_NAME
here, if you do then using SSA_NAME_VAR is wrong)
>>> You mean during RTL expansion phase, all SSA_NAMEs are gone already?
>>
>> Actually, the lhs could be SSA_NAME here,
>>
>> Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
>> ../../latest-gcc/gcc/internal-fn.c:3021
>> 3021 mark_addressable (lhs);
>> (gdb) call debug_tree(lhs)
>> >type >size
>>unit-size
>>align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
>> 0x7fffe959b2a0 precision:32
>>pointer_to_this >
>>visited var
>>def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
>>version:5>
>>
>> when I deleted:
>>
>> if (TREE_CODE (lhs) == SSA_NAME
>> lhs = SSA_NAME_VAR (lhs);
>
> but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
> isn't the correct thing to look at. I think we need to look at what
> the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
> but also plain DECLs but not what we get from VLAs where we'd see
> *ptr). So sth like
>
> bool reg_lhs;
> if (SSA_VAR_P (lhs))
>{
> rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> reg_lhs = !MEM_P (tem);
> /* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
> also CONCAT or lowpart...?) */
>}
> else
>{
> gcc_assert (is_vla);
> reg_lhs = false;
>}
>
> if (!reg_lhs)
>memset path
> else
>expand_assignment path
After making the following change:
+ bool reg_lhs = true;
tree var_type = TREE_TYPE (lhs);
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
- if (is_vla || (!use_register_for_decl (lhs)))
+ if (SSA_VAR_P (lhs))
+{
+ rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
+ reg_lhs = !MEM_P (tem);
+}
+ else
+{
+ gcc_assert (is_vla);
+ reg_lhs = false;
+}
+
+ if (!reg_lhs)
{
I got exactly the same internal error that failed at expr.c:
8436 /* We must have made progress. */
8437 gcc_assert (inner != exp);
Looks like for the following code:
3026 if (!reg_lhs)
3027 {
3028 /* If this is a VLA or the variable is not in register,
3029expand to a memset to initialize it. */
3030 mark_addressable (lhs);
3031 tree var_addr = build_fold_addr_expr (lhs);
3032
3033 tree value = (init_type == AUTO_INIT_PATTERN) ?
3034 build_int_cst (integer_type_node,
3035INIT_PATTERN_VALUE) :
3036 integer_zero_node;
3037 tree m_call = build_call_expr (builtin_decl_implicit
(BUILT_IN_MEMSET),
3038 3, var_addr, value, var_size);
3039 /* Expand this memset call. */
3040 expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
3041 }
At line 3030, “lhs” could be a SSA_NAME.
My questions are:
1. Could the routine “mark_addressable” and “build_fold_addr_expr” be applied
on SSA_NAME?
2. Could the routine “expand_builtin_memset” be applied on the memset call
whose “DEST” is
an address expression on SSA_NAME?
3. Within “expand_DEFERRED_INIT”, can I call “expand_builtin_memset” to expand
.DEFERRED_INIT?
I suspect that one of the above 3 might be the issue, but not sure which one?
Thanks a lot.
Qing
> bool reg_lhs;
> if (SSA_VAR_P (lhs))
>{
> rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> reg_lhs = !MEM_P (tem);
> /* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
> also CONCAT or lowpart...?) */
>}
> else
>{
> gcc_assert (is_vla);
> reg_lhs = false;
>}
>
>> Many testing cases failed with internal compiler error:
>>
>> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/c-c++-common/auto-init-3.c:9:9:
>> internal
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 18, 2021, at 2:19 AM, Richard Biener wrote:
>
> On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 17, 2021, at 10:04 AM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>>
>>>
On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches
wrote:
>> From the above IR file after “FRE”, we can see that the major issue with
>> this IR is:
>>
>> The address taken auto variable “alt_reloc” has been completely replaced
>> by the temporary variable “_1” in all
>> the uses of the original “alt_reloc”.
>
> Well, this can happen with regular code as well, there's no need for
> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
> uses late.
>
> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
> -Wuninitialized isn't really achievable.
Okay. Sounds reasonable to me too.
>
>> The major problem with such IR is, during uninitialized analysis phase,
>> the original use of “alt_reloc” disappeared completely.
>> So, the warning cannot be reported.
>>
>>
>> My questions:
>>
>> 1. Is it possible to get the original “alt_reloc” through the temporary
>> variable “_1” with some available information recorded in the IR?
>> 2. If not, then we have to record the relationship between “alt_reloc”
>> and “_1” when the original “alt_reloc” is replaced by “_1” and get such
>> relationship during
>> Uninitialized analysis phase. Is this doable?
>
> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
> diagnostics. The difficulty is to avoid tracking it as actual use so
> you could for example pass a string with the declarations name though
> this wouldn't give the association with the actual decl.
Good suggestion, I can try this a little bit.
>>>
>>> I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
>>>
>>> 1st argument: SIZE of the DECL;
>>> 2nd argument: INIT_TYPE;
>>> 3rd argument: IS_VLA, 0 NO, 1 YES;
>>> + 4th argument: The NAME for the DECL;
>>>
>>> - as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
>>> + as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
>>>
>>> + tree name_node
>>> += build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
>>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>>>
>>> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
>>> IFN_DEFERRED_INIT,
>>> - TREE_TYPE (decl), 3,
>>> + TREE_TYPE (decl), 4,
>>> decl_size, init_type_node,
>>> - is_vla_node);
>>> + is_vla_node, name_node);
>>>
>>>
>>> And got the following IR in .uninit1 dump:
>>>
>>>
>>> ….
>>>
>>> _1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
>>> if (_1 != 0)
>>> ….
>>>
>>>
>>> My questions:
>>>
>>> 1. Is “build_string_literal” the correct utility routine to use for this
>>> new argument?
>>> 2. Will Such string literal nodes have potential other impact?
>>
>> I tried to get the 4th argument from the call to .DEFERED_INIT during
>> uninitialized variable analysis in tree-ssa-uninit.c:
>>
>> @@ -197,18 +197,25 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree
>> var,
>> the COMPLEX_EXPRs real part in that case. See PR71581. */
>> if (expr == NULL_TREE
>> && var == NULL_TREE
>> - && SSA_NAME_VAR (t) == NULL_TREE
>> - && is_gimple_assign (SSA_NAME_DEF_STMT (t))
>> - && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR)
>> -{
>> - tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
>> - if (TREE_CODE (v) == SSA_NAME
>> - && has_undefined_value_p (v)
>> - && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
>> + && SSA_NAME_VAR (t) == NULL_TREE)
>> +{
>> + if (is_gimple_assign (SSA_NAME_DEF_STMT (t))
>> + && (gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) ==
>> COMPLEX_EXPR))
>>{
>> - expr = SSA_NAME_VAR (v);
>> - var = expr;
>> + tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
>> + if (TREE_CODE (v) == SSA_NAME
>> + && has_undefined_value_p (v)
>> + && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
>> + {
>> + expr = SSA_NAME_VAR (v);
>> + var = expr;
>> + }
>>}
>> + else if (gimple_call_internal_p (SSA_NAME_DEF_STMT (t),
>> IFN_DEFERRED_INIT))
>> + {
>> + expr = gimple_call_arg (SSA_NAME_DEF_STMT (t), 3);
>> + var = expr;
>> + }
>> }
>>
>> However, this 4th argument is not a regular variable, it’s just an ADDR_EXPR
>> that includes the constant string for the name of
>> the deleted variable.
>> If we’d
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 17, 2021, at 10:04 AM, Qing Zhao via Gcc-patches
> > wrote:
> >
> >
> >
> >> On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches
> >> wrote:
> >>
> From the above IR file after “FRE”, we can see that the major issue with
> this IR is:
>
> The address taken auto variable “alt_reloc” has been completely replaced
> by the temporary variable “_1” in all
> the uses of the original “alt_reloc”.
> >>>
> >>> Well, this can happen with regular code as well, there's no need for
> >>> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
> >>> uses late.
> >>>
> >>> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
> >>> -Wuninitialized isn't really achievable.
> >>
> >> Okay. Sounds reasonable to me too.
> >>
> >>>
> The major problem with such IR is, during uninitialized analysis phase,
> the original use of “alt_reloc” disappeared completely.
> So, the warning cannot be reported.
>
>
> My questions:
>
> 1. Is it possible to get the original “alt_reloc” through the temporary
> variable “_1” with some available information recorded in the IR?
> 2. If not, then we have to record the relationship between “alt_reloc”
> and “_1” when the original “alt_reloc” is replaced by “_1” and get such
> relationship during
> Uninitialized analysis phase. Is this doable?
> >>>
> >>> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
> >>> diagnostics. The difficulty is to avoid tracking it as actual use so
> >>> you could for example pass a string with the declarations name though
> >>> this wouldn't give the association with the actual decl.
> >> Good suggestion, I can try this a little bit.
> >
> > I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
> >
> >1st argument: SIZE of the DECL;
> >2nd argument: INIT_TYPE;
> >3rd argument: IS_VLA, 0 NO, 1 YES;
> > + 4th argument: The NAME for the DECL;
> >
> > - as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
> > + as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
> >
> > + tree name_node
> > += build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
> > + IDENTIFIER_POINTER (DECL_NAME (decl)));
> >
> > tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
> > IFN_DEFERRED_INIT,
> > - TREE_TYPE (decl), 3,
> > + TREE_TYPE (decl), 4,
> >decl_size, init_type_node,
> > - is_vla_node);
> > + is_vla_node, name_node);
> >
> >
> > And got the following IR in .uninit1 dump:
> >
> >
> > ….
> >
> > _1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
> > if (_1 != 0)
> > ….
> >
> >
> > My questions:
> >
> > 1. Is “build_string_literal” the correct utility routine to use for this
> > new argument?
> > 2. Will Such string literal nodes have potential other impact?
>
> I tried to get the 4th argument from the call to .DEFERED_INIT during
> uninitialized variable analysis in tree-ssa-uninit.c:
>
> @@ -197,18 +197,25 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree
> var,
> the COMPLEX_EXPRs real part in that case. See PR71581. */
>if (expr == NULL_TREE
>&& var == NULL_TREE
> - && SSA_NAME_VAR (t) == NULL_TREE
> - && is_gimple_assign (SSA_NAME_DEF_STMT (t))
> - && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR)
> -{
> - tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
> - if (TREE_CODE (v) == SSA_NAME
> - && has_undefined_value_p (v)
> - && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
> + && SSA_NAME_VAR (t) == NULL_TREE)
> +{
> + if (is_gimple_assign (SSA_NAME_DEF_STMT (t))
> + && (gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR))
> {
> - expr = SSA_NAME_VAR (v);
> - var = expr;
> + tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
> + if (TREE_CODE (v) == SSA_NAME
> + && has_undefined_value_p (v)
> + && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
> + {
> + expr = SSA_NAME_VAR (v);
> + var = expr;
> + }
> }
> + else if (gimple_call_internal_p (SSA_NAME_DEF_STMT (t),
> IFN_DEFERRED_INIT))
> + {
> + expr = gimple_call_arg (SSA_NAME_DEF_STMT (t), 3);
> + var = expr;
> + }
> }
>
> However, this 4th argument is not a regular variable, it’s just an ADDR_EXPR
> that includes the constant string for the name of
> the deleted variable.
> If we’d like to report the warning based on this ADDR_EXPR, a complete new
> code to report the warnings
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
> > wrote:
> >
> >
> >
> >> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
> >>
> >> On Mon, 16 Aug 2021, Qing Zhao wrote:
> >>
> >>> My current code for expand_DEFERRED_INIT is like the following, could you
> >>> check and see whether there is any issue for it:
> >>>
> >>> #define INIT_PATTERN_VALUE 0xFE
> >>> static void
> >>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> >>> {
> >>> tree lhs = gimple_call_lhs (stmt);
> >>> tree var_size = gimple_call_arg (stmt, 0);
> >>> enum auto_init_type init_type
> >>> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> >>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> >>>
> >>> tree var_type = TREE_TYPE (lhs);
> >>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> >>>
> >>> if (is_vla || (!use_register_for_decl (lhs)))
> >>> {
> >>> if (TREE_CODE (lhs) == SSA_NAME)
> >>> lhs = SSA_NAME_VAR (lhs);
> >>
> >> this should not be necessary (in fact you shouldn't see a SSA_NAME
> >> here, if you do then using SSA_NAME_VAR is wrong)
> > You mean during RTL expansion phase, all SSA_NAMEs are gone already?
>
> Actually, the lhs could be SSA_NAME here,
>
> Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
> ../../latest-gcc/gcc/internal-fn.c:3021
> 3021mark_addressable (lhs);
> (gdb) call debug_tree(lhs)
> type size
> unit-size
> align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
> 0x7fffe959b2a0 precision:32
> pointer_to_this >
> visited var
> def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
> version:5>
>
> when I deleted:
>
> if (TREE_CODE (lhs) == SSA_NAME
>lhs = SSA_NAME_VAR (lhs);
but then using SSA_NAME_VAR is broken. I suspect use_register_for_decl
isn't the correct thing to look at. I think we need to look at what
the LHS expanded to if it is a SSA_VAR_P (that includes SSA names
but also plain DECLs but not what we get from VLAs where we'd see
*ptr). So sth like
bool reg_lhs;
if (SSA_VAR_P (lhs))
{
rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
reg_lhs = !MEM_P (tem);
/* If not MEM_P reg_lhs should be REG_P or SUBREG_P (but maybe
also CONCAT or lowpart...?) */
}
else
{
gcc_assert (is_vla);
reg_lhs = false;
}
if (!reg_lhs)
memset path
else
expand_assignment path
> Many testing cases failed with internal compiler error:
>
> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/c-c++-common/auto-init-3.c:9:9:
> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8437
> 0xe237aa expand_expr_addr_expr_1
> ../../latest-gcc/gcc/expr.c:8437
> 0xe24059 expand_expr_addr_expr
> ../../latest-gcc/gcc/expr.c:8525
> 0xe32b56 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:11741
> 0xe2da52 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:10777
> 0xe24706 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:8713
> 0xc13f15 expand_expr
> ../../latest-gcc/gcc/expr.h:301
> 0xc17acb get_memory_rtx
> ../../latest-gcc/gcc/builtins.c:1370
> 0xc2223d expand_builtin_memset_args
> ../../latest-gcc/gcc/builtins.c:4102
> 0xc21a20 expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
> ../../latest-gcc/gcc/builtins.c:3886
> 0xfb5c85 expand_DEFERRED_INIT
> ../../latest-gcc/gcc/internal-fn.c:3031
>
>
> So, did I do anything wrong?
>
> Qing
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 17, 2021, at 10:04 AM, Qing Zhao via Gcc-patches
> wrote:
>
>
>
>> On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches
>> wrote:
>>
From the above IR file after “FRE”, we can see that the major issue with
this IR is:
The address taken auto variable “alt_reloc” has been completely replaced
by the temporary variable “_1” in all
the uses of the original “alt_reloc”.
>>>
>>> Well, this can happen with regular code as well, there's no need for
>>> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
>>> uses late.
>>>
>>> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
>>> -Wuninitialized isn't really achievable.
>>
>> Okay. Sounds reasonable to me too.
>>
>>>
The major problem with such IR is, during uninitialized analysis phase,
the original use of “alt_reloc” disappeared completely.
So, the warning cannot be reported.
My questions:
1. Is it possible to get the original “alt_reloc” through the temporary
variable “_1” with some available information recorded in the IR?
2. If not, then we have to record the relationship between “alt_reloc” and
“_1” when the original “alt_reloc” is replaced by “_1” and get such
relationship during
Uninitialized analysis phase. Is this doable?
>>>
>>> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
>>> diagnostics. The difficulty is to avoid tracking it as actual use so
>>> you could for example pass a string with the declarations name though
>>> this wouldn't give the association with the actual decl.
>> Good suggestion, I can try this a little bit.
>
> I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
>
>1st argument: SIZE of the DECL;
>2nd argument: INIT_TYPE;
>3rd argument: IS_VLA, 0 NO, 1 YES;
> + 4th argument: The NAME for the DECL;
>
> - as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
> + as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
>
> + tree name_node
> += build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>
> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
> IFN_DEFERRED_INIT,
> - TREE_TYPE (decl), 3,
> + TREE_TYPE (decl), 4,
>decl_size, init_type_node,
> - is_vla_node);
> + is_vla_node, name_node);
>
>
> And got the following IR in .uninit1 dump:
>
>
> ….
>
> _1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
> if (_1 != 0)
> ….
>
>
> My questions:
>
> 1. Is “build_string_literal” the correct utility routine to use for this new
> argument?
> 2. Will Such string literal nodes have potential other impact?
I tried to get the 4th argument from the call to .DEFERED_INIT during
uninitialized variable analysis in tree-ssa-uninit.c:
@@ -197,18 +197,25 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree
var,
the COMPLEX_EXPRs real part in that case. See PR71581. */
if (expr == NULL_TREE
&& var == NULL_TREE
- && SSA_NAME_VAR (t) == NULL_TREE
- && is_gimple_assign (SSA_NAME_DEF_STMT (t))
- && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR)
-{
- tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
- if (TREE_CODE (v) == SSA_NAME
- && has_undefined_value_p (v)
- && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
+ && SSA_NAME_VAR (t) == NULL_TREE)
+{
+ if (is_gimple_assign (SSA_NAME_DEF_STMT (t))
+ && (gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR))
{
- expr = SSA_NAME_VAR (v);
- var = expr;
+ tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
+ if (TREE_CODE (v) == SSA_NAME
+ && has_undefined_value_p (v)
+ && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t
+ {
+ expr = SSA_NAME_VAR (v);
+ var = expr;
+ }
}
+ else if (gimple_call_internal_p (SSA_NAME_DEF_STMT (t),
IFN_DEFERRED_INIT))
+ {
+ expr = gimple_call_arg (SSA_NAME_DEF_STMT (t), 3);
+ var = expr;
+ }
}
However, this 4th argument is not a regular variable, it’s just an ADDR_EXPR
that includes the constant string for the name of
the deleted variable.
If we’d like to report the warning based on this ADDR_EXPR, a complete new code
to report the warnings other than the current one that based on
“Variables” need to be added, this might make the code very ugly.
My questions:
1. Is there better way to do this?
1. As you mentioned before, it’s very unrealistic to meet the goal of “zero
regression” for -Wuninitialized, can we leave this part of work
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 17, 2021, at 9:50 AM, Qing Zhao via Gcc-patches
> wrote:
>
>
>
>> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
>>
>> On Mon, 16 Aug 2021, Qing Zhao wrote:
>>
>>> My current code for expand_DEFERRED_INIT is like the following, could you
>>> check and see whether there is any issue for it:
>>>
>>> #define INIT_PATTERN_VALUE 0xFE
>>> static void
>>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>> {
>>> tree lhs = gimple_call_lhs (stmt);
>>> tree var_size = gimple_call_arg (stmt, 0);
>>> enum auto_init_type init_type
>>> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>>
>>> tree var_type = TREE_TYPE (lhs);
>>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>>
>>> if (is_vla || (!use_register_for_decl (lhs)))
>>> {
>>> if (TREE_CODE (lhs) == SSA_NAME)
>>> lhs = SSA_NAME_VAR (lhs);
>>
>> this should not be necessary (in fact you shouldn't see a SSA_NAME
>> here, if you do then using SSA_NAME_VAR is wrong)
> You mean during RTL expansion phase, all SSA_NAMEs are gone already?
Actually, the lhs could be SSA_NAME here,
Breakpoint 1, expand_DEFERRED_INIT (stmt=0x7fffe96ae348) at
../../latest-gcc/gcc/internal-fn.c:3021
3021 mark_addressable (lhs);
(gdb) call debug_tree(lhs)
unit-size
align:32 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type
0x7fffe959b2a0 precision:32
pointer_to_this >
visited var
def_stmt temp1_5 = .DEFERRED_INIT (4, 2, 0, &"temp1"[0]);
version:5>
when I deleted:
if (TREE_CODE (lhs) == SSA_NAME
lhs = SSA_NAME_VAR (lhs);
Many testing cases failed with internal compiler error:
/home/opc/Work/GCC/latest-gcc/gcc/testsuite/c-c++-common/auto-init-3.c:9:9:
internal compiler error: in expand_expr_addr_expr_1, at expr.c:8437
0xe237aa expand_expr_addr_expr_1
../../latest-gcc/gcc/expr.c:8437
0xe24059 expand_expr_addr_expr
../../latest-gcc/gcc/expr.c:8525
0xe32b56 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../latest-gcc/gcc/expr.c:11741
0xe2da52 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../latest-gcc/gcc/expr.c:10777
0xe24706 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier,
rtx_def**, bool)
../../latest-gcc/gcc/expr.c:8713
0xc13f15 expand_expr
../../latest-gcc/gcc/expr.h:301
0xc17acb get_memory_rtx
../../latest-gcc/gcc/builtins.c:1370
0xc2223d expand_builtin_memset_args
../../latest-gcc/gcc/builtins.c:4102
0xc21a20 expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
../../latest-gcc/gcc/builtins.c:3886
0xfb5c85 expand_DEFERRED_INIT
../../latest-gcc/gcc/internal-fn.c:3031
So, did I do anything wrong?
Qing
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches
> wrote:
>
>>> From the above IR file after “FRE”, we can see that the major issue with
>>> this IR is:
>>>
>>> The address taken auto variable “alt_reloc” has been completely replaced by
>>> the temporary variable “_1” in all
>>> the uses of the original “alt_reloc”.
>>
>> Well, this can happen with regular code as well, there's no need for
>> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
>> uses late.
>>
>> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
>> -Wuninitialized isn't really achievable.
>
> Okay. Sounds reasonable to me too.
>
>>
>>> The major problem with such IR is, during uninitialized analysis phase,
>>> the original use of “alt_reloc” disappeared completely.
>>> So, the warning cannot be reported.
>>>
>>>
>>> My questions:
>>>
>>> 1. Is it possible to get the original “alt_reloc” through the temporary
>>> variable “_1” with some available information recorded in the IR?
>>> 2. If not, then we have to record the relationship between “alt_reloc” and
>>> “_1” when the original “alt_reloc” is replaced by “_1” and get such
>>> relationship during
>>> Uninitialized analysis phase. Is this doable?
>>
>> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
>> diagnostics. The difficulty is to avoid tracking it as actual use so
>> you could for example pass a string with the declarations name though
>> this wouldn't give the association with the actual decl.
> Good suggestion, I can try this a little bit.
I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
1st argument: SIZE of the DECL;
2nd argument: INIT_TYPE;
3rd argument: IS_VLA, 0 NO, 1 YES;
+ 4th argument: The NAME for the DECL;
- as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
+ as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
+ tree name_node
+= build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
IFN_DEFERRED_INIT,
- TREE_TYPE (decl), 3,
+ TREE_TYPE (decl), 4,
decl_size, init_type_node,
- is_vla_node);
+ is_vla_node, name_node);
And got the following IR in .uninit1 dump:
….
_1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
if (_1 != 0)
….
My questions:
1. Is “build_string_literal” the correct utility routine to use for this new
argument?
2. Will Such string literal nodes have potential other impact?
Qing
>
>>
>>> 3. Looks like that for “address taken” auto variable, if we have to
>>> introduce a new temporary variable and split the call to .DEFERRED_INIT
>>> into two:
>>>
>>> temp = .DEFERRED_INIT (4, 2, 0);
>>> alt_reloc = temp;
>>>
>>> More issues might possible.
>>>
>>> Any comments and suggestions on this issue?
>>
>> I don't see any good possibilities that would not make optimizing code
>> as good as w/o .DEFERRED_INIT more difficult. My stake here is always
>> that GCC is an optimizing compiler, not a static analysis engine and
>> thus I side with "broken" diagnostics and better optimization.
> That’s true and reasonable, too.
>
> thanks.
>
> Qing
>>
>> Richard.
>>
>>> Qing
>>>
>>> j
On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
wrote:
>
>
>> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>>
>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
>> wrote:
>>> I modified the routine “gimple_add_init_for_auto_var” as the following:
>>>
>>> /* Generate initialization to automatic variable DECL based on
>>> INIT_TYPE.
>>> Build a call to internal const function DEFERRED_INIT:
>>> 1st argument: SIZE of the DECL;
>>> 2nd argument: INIT_TYPE;
>>> 3rd argument: IS_VLA, 0 NO, 1 YES;
>>>
>>> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
>>> static void
>>> gimple_add_init_for_auto_var (tree decl,
>>> enum auto_init_type init_type,
>>> bool is_vla,
>>> gimple_seq *seq_p)
>>> {
>>> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC
>>> (decl));
>>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>>>
>>> tree init_type_node
>>> = build_int_cst (integer_type_node, (int) init_type);
>>> tree is_vla_node
>>> = build_int_cst (integer_type_node, (int) is_vla);
>>>
>>> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
>>> IFN_DEFERRED_INIT,
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 17, 2021, at 9:45 AM, Richard Biener wrote:
>
> On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 17, 2021, at 3:43 AM, Richard Biener wrote:
>>>
>>> On Mon, 16 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
>
> On Thu, 12 Aug 2021, Qing Zhao wrote:
>
>> Hi, Richard,
>>
>> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
>> suggestions like following:
>>
>> ==
>> #define INIT_PATTERN_VALUE 0xFE
>> static void
>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>> {
>> tree lhs = gimple_call_lhs (stmt);
>> tree var_size = gimple_call_arg (stmt, 0);
>> enum auto_init_type init_type
>> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>
>> tree var_type = TREE_TYPE (lhs);
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>
>> if (is_vla || (!can_native_interpret_type_p (var_type)))
>> {
>> /* If this is a VLA or the type of the variable cannot be natively
>> interpreted, expand to a memset to initialize it. */
>>if (TREE_CODE (lhs) == SSA_NAME)
>> lhs = SSA_NAME_VAR (lhs);
>>tree var_addr = NULL_TREE;
>>if (is_vla)
>> var_addr = TREE_OPERAND (lhs, 0);
>>else
>> {
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>> }
>>tree value = (init_type == AUTO_INIT_PATTERN) ?
>> build_int_cst (unsigned_char_type_node,
>> INIT_PATTERN_VALUE) :
>> build_zero_cst (unsigned_char_type_node);
>>tree m_call = build_call_expr (builtin_decl_implicit
>> (BUILT_IN_MEMSET),
>> 3, var_addr, value, var_size);
>>/* Expand this memset call. */
>>expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>> }
>> else
>> {
>> /* If this is not a VLA and the type of the variable can be natively
>> interpreted, expand to assignment to generate better code. */
>>tree pattern = NULL_TREE;
>>unsigned HOST_WIDE_INT total_bytes
>> = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>>
>>if (init_type == AUTO_INIT_PATTERN)
>> {
>>unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>>memset (buf, INIT_PATTERN_VALUE, total_bytes);
>>pattern = native_interpret_expr (var_type, buf, total_bytes);
>>gcc_assert (pattern);
>> }
>>
>>tree init = (init_type == AUTO_INIT_PATTERN) ?
>> pattern :
>> build_zero_cst (var_type);
>>expand_assignment (lhs, init, false);
>> }
>> }
>> ===
>>
>> Now, I used “can_native_interpret_type_p (var_type)” instead of
>> “use_register_for_decl (lhs)” to decide
>> whether to use “memset” or use “assign” to expand this function.
>>
>> However, this exposed an bug that is very hard to be addressed:
>>
>> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>>
>> /* { dg-do compile } */
>> /* { dg-options "-O2 -Wuninitialized" } */
>>
>> int sys_msgctl (void)
>> {
>> struct { int mode; } setbuf;
>> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
>> ==
>>
>> **the above auto var “setbuf” has “struct” type, which
>> “can_native_interpret_type_p(var_type)” is false, therefore,
>> Expanding this .DEFERRED_INIT call went down the “memset” expansion
>> route.
>>
>> However, this structure type can be fitted into a register, therefore
>> cannot be taken address anymore at this stage, even though I tried:
>>
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>
>> To create an address variable for it, the expansion still failed at
>> expr.c: line 8412:
>> during RTL pass: expand
>> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
>> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
>> 0xd04104 expand_expr_addr_expr_1
>> ../../latest-gcc/gcc/expr.c:8412
>> 0xd04a95 expand_expr_addr_expr
>> ../../latest-gcc/gcc/expr.c:8525
>> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:11741
>> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:8713
>> 0xaed1d3 expand_expr
>> ../../latest-gcc/gcc/expr.h:301
>> 0xaf0d89 get_memory_rtx
>>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 17, 2021, at 3:29 AM, Richard Biener wrote:
>
> On Mon, 16 Aug 2021, Qing Zhao wrote:
>
>> My current code for expand_DEFERRED_INIT is like the following, could you
>> check and see whether there is any issue for it:
>>
>> #define INIT_PATTERN_VALUE 0xFE
>> static void
>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>> {
>> tree lhs = gimple_call_lhs (stmt);
>> tree var_size = gimple_call_arg (stmt, 0);
>> enum auto_init_type init_type
>>= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>
>> tree var_type = TREE_TYPE (lhs);
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>
>> if (is_vla || (!use_register_for_decl (lhs)))
>>{
>> if (TREE_CODE (lhs) == SSA_NAME)
>>lhs = SSA_NAME_VAR (lhs);
>
> this should not be necessary (in fact you shouldn't see a SSA_NAME
> here, if you do then using SSA_NAME_VAR is wrong)
You mean during RTL expansion phase, all SSA_NAMEs are gone already?
>
>>/* If this is a VLA or the variable is not in register,
>> expand to a memset to initialize it. */
>> tree var_addr = NULL_TREE;
>> if (is_vla)
>>var_addr = TREE_OPERAND (lhs, 0);
>> else
>>{
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>}
>
> use, independent of is_vla
>
> mark_addressable (lhs);
> var_addr = build_fold_addr_expr (lhs);
Okay.
>
>>
>> tree value = (init_type == AUTO_INIT_PATTERN) ?
>>build_int_cst (unsigned_char_type_node,
>> INIT_PATTERN_VALUE) :
>>build_zero_cst (unsigned_char_type_node);
>
> since memset has an integer argument for the value use
> integer_zero_node for the zero case and build_int_cst (integer_type_node,
> ...) for the pattern case
Okay.
>
>> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
>> 3, var_addr, value, var_size);
>> /* Expand this memset call. */
>> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>}
>> else
>>{
>>/* If this variable is in a register, use expand_assignment might
>> generate better code. */
>> tree pattern = NULL_TREE;
>> unsigned HOST_WIDE_INT total_bytes
>>= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>>
>> if (init_type == AUTO_INIT_PATTERN)
>>{
>> if (can_native_interpret_type_p (var_type))
>>{
>> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>> memset (buf, INIT_PATTERN_VALUE, total_bytes);
>> pattern = native_interpret_expr (var_type, buf, total_bytes);
>> gcc_assert (pattern);
>>}
>> else
>>{
>> tree index_type = build_index_type (size_int (total_bytes - 1));
>> tree array_type = build_array_type (unsigned_char_type_node,
>> index_type);
>> tree element = build_int_cst (unsigned_char_type_node,
>>INIT_PATTERN_VALUE);
>> vec *elts = NULL;
>> for (unsigned int i = 0; i < total_bytes; i++)
>>CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE, element);
>> pattern = build_constructor (array_type, elts);
>> pattern = build1 (VIEW_CONVERT_EXPR, var_type, pattern);
>>}
>>}
>>
>> tree init = (init_type == AUTO_INIT_PATTERN) ?
>> pattern :
>> build_zero_cst (var_type);
>
> maybe conditionally initialize init instead of pattern and init?
> Thus replace pattern by init and do
>
>else
> init = build_zero_cst (var_type);
You mean the following:
tree init = pattern;
If (init_type != AUTO_INIT_PATTERN)
Init = build_zero_cst (var_type);
Or something else?
>
>
> the above should work, as said the RTL expansion part can possibly
> be improved but we can do this as followup as well.
Okay.
Qing
>
>> expand_assignment (lhs, init, false);
>>}
>> }
>>
>> Thanks.
>>
>> Qing
>>
>>
>>
>>
>
> --
> Richard Biener
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 17 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 17, 2021, at 3:43 AM, Richard Biener wrote:
> >
> > On Mon, 16 Aug 2021, Qing Zhao wrote:
> >
> >>
> >>
> >>> On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
> >>>
> >>> On Thu, 12 Aug 2021, Qing Zhao wrote:
> >>>
> Hi, Richard,
>
> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
> suggestions like following:
>
> ==
> #define INIT_PATTERN_VALUE 0xFE
> static void
> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> {
> tree lhs = gimple_call_lhs (stmt);
> tree var_size = gimple_call_arg (stmt, 0);
> enum auto_init_type init_type
> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>
> tree var_type = TREE_TYPE (lhs);
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> if (is_vla || (!can_native_interpret_type_p (var_type)))
> {
> /* If this is a VLA or the type of the variable cannot be natively
> interpreted, expand to a memset to initialize it. */
> if (TREE_CODE (lhs) == SSA_NAME)
> lhs = SSA_NAME_VAR (lhs);
> tree var_addr = NULL_TREE;
> if (is_vla)
> var_addr = TREE_OPERAND (lhs, 0);
> else
> {
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
> }
> tree value = (init_type == AUTO_INIT_PATTERN) ?
> build_int_cst (unsigned_char_type_node,
> INIT_PATTERN_VALUE) :
> build_zero_cst (unsigned_char_type_node);
> tree m_call = build_call_expr (builtin_decl_implicit
> (BUILT_IN_MEMSET),
> 3, var_addr, value, var_size);
> /* Expand this memset call. */
> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> }
> else
> {
> /* If this is not a VLA and the type of the variable can be natively
> interpreted, expand to assignment to generate better code. */
> tree pattern = NULL_TREE;
> unsigned HOST_WIDE_INT total_bytes
> = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>
> if (init_type == AUTO_INIT_PATTERN)
> {
> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> memset (buf, INIT_PATTERN_VALUE, total_bytes);
> pattern = native_interpret_expr (var_type, buf, total_bytes);
> gcc_assert (pattern);
> }
>
> tree init = (init_type == AUTO_INIT_PATTERN) ?
> pattern :
> build_zero_cst (var_type);
> expand_assignment (lhs, init, false);
> }
> }
> ===
>
> Now, I used “can_native_interpret_type_p (var_type)” instead of
> “use_register_for_decl (lhs)” to decide
> whether to use “memset” or use “assign” to expand this function.
>
> However, this exposed an bug that is very hard to be addressed:
>
> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -Wuninitialized" } */
>
> int sys_msgctl (void)
> {
> struct { int mode; } setbuf;
> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
> ==
>
> **the above auto var “setbuf” has “struct” type, which
> “can_native_interpret_type_p(var_type)” is false, therefore,
> Expanding this .DEFERRED_INIT call went down the “memset” expansion
> route.
>
> However, this structure type can be fitted into a register, therefore
> cannot be taken address anymore at this stage, even though I tried:
>
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
>
> To create an address variable for it, the expansion still failed at
> expr.c: line 8412:
> during RTL pass: expand
> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
> 0xd04104 expand_expr_addr_expr_1
> ../../latest-gcc/gcc/expr.c:8412
> 0xd04a95 expand_expr_addr_expr
> ../../latest-gcc/gcc/expr.c:8525
> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:11741
> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:8713
> 0xaed1d3 expand_expr
> ../../latest-gcc/gcc/expr.h:301
> 0xaf0d89 get_memory_rtx
>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 17, 2021, at 3:43 AM, Richard Biener wrote:
>
> On Mon, 16 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
>>>
>>> On Thu, 12 Aug 2021, Qing Zhao wrote:
>>>
Hi, Richard,
For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
suggestions like following:
==
#define INIT_PATTERN_VALUE 0xFE
static void
expand_DEFERRED_INIT (internal_fn, gcall *stmt)
{
tree lhs = gimple_call_lhs (stmt);
tree var_size = gimple_call_arg (stmt, 0);
enum auto_init_type init_type
= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
tree var_type = TREE_TYPE (lhs);
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
if (is_vla || (!can_native_interpret_type_p (var_type)))
{
/* If this is a VLA or the type of the variable cannot be natively
interpreted, expand to a memset to initialize it. */
if (TREE_CODE (lhs) == SSA_NAME)
lhs = SSA_NAME_VAR (lhs);
tree var_addr = NULL_TREE;
if (is_vla)
var_addr = TREE_OPERAND (lhs, 0);
else
{
TREE_ADDRESSABLE (lhs) = 1;
var_addr = build_fold_addr_expr (lhs);
}
tree value = (init_type == AUTO_INIT_PATTERN) ?
build_int_cst (unsigned_char_type_node,
INIT_PATTERN_VALUE) :
build_zero_cst (unsigned_char_type_node);
tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
3, var_addr, value, var_size);
/* Expand this memset call. */
expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
}
else
{
/* If this is not a VLA and the type of the variable can be natively
interpreted, expand to assignment to generate better code. */
tree pattern = NULL_TREE;
unsigned HOST_WIDE_INT total_bytes
= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
if (init_type == AUTO_INIT_PATTERN)
{
unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
memset (buf, INIT_PATTERN_VALUE, total_bytes);
pattern = native_interpret_expr (var_type, buf, total_bytes);
gcc_assert (pattern);
}
tree init = (init_type == AUTO_INIT_PATTERN) ?
pattern :
build_zero_cst (var_type);
expand_assignment (lhs, init, false);
}
}
===
Now, I used “can_native_interpret_type_p (var_type)” instead of
“use_register_for_decl (lhs)” to decide
whether to use “memset” or use “assign” to expand this function.
However, this exposed an bug that is very hard to be addressed:
***For the testing case: test suite/gcc.dg/uninit-I.c:
/* { dg-do compile } */
/* { dg-options "-O2 -Wuninitialized" } */
int sys_msgctl (void)
{
struct { int mode; } setbuf;
return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
==
**the above auto var “setbuf” has “struct” type, which
“can_native_interpret_type_p(var_type)” is false, therefore,
Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
However, this structure type can be fitted into a register, therefore
cannot be taken address anymore at this stage, even though I tried:
TREE_ADDRESSABLE (lhs) = 1;
var_addr = build_fold_addr_expr (lhs);
To create an address variable for it, the expansion still failed at
expr.c: line 8412:
during RTL pass: expand
/home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
0xd04104 expand_expr_addr_expr_1
../../latest-gcc/gcc/expr.c:8412
0xd04a95 expand_expr_addr_expr
../../latest-gcc/gcc/expr.c:8525
0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../latest-gcc/gcc/expr.c:11741
0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../latest-gcc/gcc/expr.c:8713
0xaed1d3 expand_expr
../../latest-gcc/gcc/expr.h:301
0xaf0d89 get_memory_rtx
../../latest-gcc/gcc/builtins.c:1370
0xafb4fb expand_builtin_memset_args
../../latest-gcc/gcc/builtins.c:4102
0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
../../latest-gcc/gcc/builtins.c:3886
0xe97fb3 expand_DEFERRED_INIT
**That’s the major reason why I
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Mon, 16 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
> >
> > On Thu, 12 Aug 2021, Qing Zhao wrote:
> >
> >> Hi, Richard,
> >>
> >> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
> >> suggestions like following:
> >>
> >> ==
> >> #define INIT_PATTERN_VALUE 0xFE
> >> static void
> >> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> >> {
> >> tree lhs = gimple_call_lhs (stmt);
> >> tree var_size = gimple_call_arg (stmt, 0);
> >> enum auto_init_type init_type
> >>= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> >> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> >>
> >> tree var_type = TREE_TYPE (lhs);
> >> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> >>
> >> if (is_vla || (!can_native_interpret_type_p (var_type)))
> >>{
> >>/* If this is a VLA or the type of the variable cannot be natively
> >> interpreted, expand to a memset to initialize it. */
> >> if (TREE_CODE (lhs) == SSA_NAME)
> >>lhs = SSA_NAME_VAR (lhs);
> >> tree var_addr = NULL_TREE;
> >> if (is_vla)
> >>var_addr = TREE_OPERAND (lhs, 0);
> >> else
> >>{
> >> TREE_ADDRESSABLE (lhs) = 1;
> >> var_addr = build_fold_addr_expr (lhs);
> >>}
> >> tree value = (init_type == AUTO_INIT_PATTERN) ?
> >>build_int_cst (unsigned_char_type_node,
> >> INIT_PATTERN_VALUE) :
> >>build_zero_cst (unsigned_char_type_node);
> >> tree m_call = build_call_expr (builtin_decl_implicit
> >> (BUILT_IN_MEMSET),
> >> 3, var_addr, value, var_size);
> >> /* Expand this memset call. */
> >> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> >>}
> >> else
> >>{
> >>/* If this is not a VLA and the type of the variable can be natively
> >> interpreted, expand to assignment to generate better code. */
> >> tree pattern = NULL_TREE;
> >> unsigned HOST_WIDE_INT total_bytes
> >>= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
> >>
> >> if (init_type == AUTO_INIT_PATTERN)
> >>{
> >> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> >> memset (buf, INIT_PATTERN_VALUE, total_bytes);
> >> pattern = native_interpret_expr (var_type, buf, total_bytes);
> >> gcc_assert (pattern);
> >>}
> >>
> >> tree init = (init_type == AUTO_INIT_PATTERN) ?
> >> pattern :
> >> build_zero_cst (var_type);
> >> expand_assignment (lhs, init, false);
> >>}
> >> }
> >> ===
> >>
> >> Now, I used “can_native_interpret_type_p (var_type)” instead of
> >> “use_register_for_decl (lhs)” to decide
> >> whether to use “memset” or use “assign” to expand this function.
> >>
> >> However, this exposed an bug that is very hard to be addressed:
> >>
> >> ***For the testing case: test suite/gcc.dg/uninit-I.c:
> >>
> >> /* { dg-do compile } */
> >> /* { dg-options "-O2 -Wuninitialized" } */
> >>
> >> int sys_msgctl (void)
> >> {
> >> struct { int mode; } setbuf;
> >> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
> >> ==
> >>
> >> **the above auto var “setbuf” has “struct” type, which
> >> “can_native_interpret_type_p(var_type)” is false, therefore,
> >> Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
> >>
> >> However, this structure type can be fitted into a register, therefore
> >> cannot be taken address anymore at this stage, even though I tried:
> >>
> >> TREE_ADDRESSABLE (lhs) = 1;
> >> var_addr = build_fold_addr_expr (lhs);
> >>
> >> To create an address variable for it, the expansion still failed at
> >> expr.c: line 8412:
> >> during RTL pass: expand
> >> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
> >> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
> >> 0xd04104 expand_expr_addr_expr_1
> >>../../latest-gcc/gcc/expr.c:8412
> >> 0xd04a95 expand_expr_addr_expr
> >>../../latest-gcc/gcc/expr.c:8525
> >> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> >> expand_modifier, rtx_def**, bool)
> >>../../latest-gcc/gcc/expr.c:11741
> >> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> >> expand_modifier, rtx_def**, bool)
> >>../../latest-gcc/gcc/expr.c:8713
> >> 0xaed1d3 expand_expr
> >>../../latest-gcc/gcc/expr.h:301
> >> 0xaf0d89 get_memory_rtx
> >>../../latest-gcc/gcc/builtins.c:1370
> >> 0xafb4fb expand_builtin_memset_args
> >>../../latest-gcc/gcc/builtins.c:4102
> >> 0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
> >>../../latest-gcc/gcc/builtins.c:3886
> >> 0xe97fb3 expand_DEFERRED_INIT
> >>
> >> **That’s the major reason why I chose
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Mon, 16 Aug 2021, Qing Zhao wrote:
> My current code for expand_DEFERRED_INIT is like the following, could you
> check and see whether there is any issue for it:
>
> #define INIT_PATTERN_VALUE 0xFE
> static void
> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> {
> tree lhs = gimple_call_lhs (stmt);
> tree var_size = gimple_call_arg (stmt, 0);
> enum auto_init_type init_type
> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>
> tree var_type = TREE_TYPE (lhs);
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> if (is_vla || (!use_register_for_decl (lhs)))
> {
> if (TREE_CODE (lhs) == SSA_NAME)
> lhs = SSA_NAME_VAR (lhs);
this should not be necessary (in fact you shouldn't see a SSA_NAME
here, if you do then using SSA_NAME_VAR is wrong)
> /* If this is a VLA or the variable is not in register,
>expand to a memset to initialize it. */
> tree var_addr = NULL_TREE;
> if (is_vla)
> var_addr = TREE_OPERAND (lhs, 0);
> else
> {
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
> }
use, independent of is_vla
mark_addressable (lhs);
var_addr = build_fold_addr_expr (lhs);
>
> tree value = (init_type == AUTO_INIT_PATTERN) ?
> build_int_cst (unsigned_char_type_node,
>INIT_PATTERN_VALUE) :
> build_zero_cst (unsigned_char_type_node);
since memset has an integer argument for the value use
integer_zero_node for the zero case and build_int_cst (integer_type_node,
...) for the pattern case
> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
> 3, var_addr, value, var_size);
> /* Expand this memset call. */
> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> }
> else
> {
> /* If this variable is in a register, use expand_assignment might
>generate better code. */
> tree pattern = NULL_TREE;
> unsigned HOST_WIDE_INT total_bytes
> = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>
> if (init_type == AUTO_INIT_PATTERN)
> {
> if (can_native_interpret_type_p (var_type))
> {
> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> memset (buf, INIT_PATTERN_VALUE, total_bytes);
> pattern = native_interpret_expr (var_type, buf, total_bytes);
> gcc_assert (pattern);
> }
> else
> {
> tree index_type = build_index_type (size_int (total_bytes - 1));
> tree array_type = build_array_type (unsigned_char_type_node,
> index_type);
> tree element = build_int_cst (unsigned_char_type_node,
> INIT_PATTERN_VALUE);
> vec *elts = NULL;
> for (unsigned int i = 0; i < total_bytes; i++)
> CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE, element);
> pattern = build_constructor (array_type, elts);
> pattern = build1 (VIEW_CONVERT_EXPR, var_type, pattern);
> }
> }
>
> tree init = (init_type == AUTO_INIT_PATTERN) ?
>pattern :
>build_zero_cst (var_type);
maybe conditionally initialize init instead of pattern and init?
Thus replace pattern by init and do
else
init = build_zero_cst (var_type);
the above should work, as said the RTL expansion part can possibly
be improved but we can do this as followup as well.
> expand_assignment (lhs, init, false);
> }
> }
>
> Thanks.
>
> Qing
>
>
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
>
> On Thu, 12 Aug 2021, Qing Zhao wrote:
>
>> Hi, Richard,
>>
>> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
>> suggestions like following:
>>
>> ==
>> #define INIT_PATTERN_VALUE 0xFE
>> static void
>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>> {
>> tree lhs = gimple_call_lhs (stmt);
>> tree var_size = gimple_call_arg (stmt, 0);
>> enum auto_init_type init_type
>>= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>
>> tree var_type = TREE_TYPE (lhs);
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>
>> if (is_vla || (!can_native_interpret_type_p (var_type)))
>>{
>>/* If this is a VLA or the type of the variable cannot be natively
>> interpreted, expand to a memset to initialize it. */
>> if (TREE_CODE (lhs) == SSA_NAME)
>>lhs = SSA_NAME_VAR (lhs);
>> tree var_addr = NULL_TREE;
>> if (is_vla)
>>var_addr = TREE_OPERAND (lhs, 0);
>> else
>>{
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>}
>> tree value = (init_type == AUTO_INIT_PATTERN) ?
>>build_int_cst (unsigned_char_type_node,
>> INIT_PATTERN_VALUE) :
>>build_zero_cst (unsigned_char_type_node);
>> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
>> 3, var_addr, value, var_size);
>> /* Expand this memset call. */
>> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>}
>> else
>>{
>>/* If this is not a VLA and the type of the variable can be natively
>> interpreted, expand to assignment to generate better code. */
>> tree pattern = NULL_TREE;
>> unsigned HOST_WIDE_INT total_bytes
>>= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>>
>> if (init_type == AUTO_INIT_PATTERN)
>>{
>> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>> memset (buf, INIT_PATTERN_VALUE, total_bytes);
>> pattern = native_interpret_expr (var_type, buf, total_bytes);
>> gcc_assert (pattern);
>>}
>>
>> tree init = (init_type == AUTO_INIT_PATTERN) ?
>> pattern :
>> build_zero_cst (var_type);
>> expand_assignment (lhs, init, false);
>>}
>> }
>> ===
>>
>> Now, I used “can_native_interpret_type_p (var_type)” instead of
>> “use_register_for_decl (lhs)” to decide
>> whether to use “memset” or use “assign” to expand this function.
>>
>> However, this exposed an bug that is very hard to be addressed:
>>
>> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>>
>> /* { dg-do compile } */
>> /* { dg-options "-O2 -Wuninitialized" } */
>>
>> int sys_msgctl (void)
>> {
>> struct { int mode; } setbuf;
>> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
>> ==
>>
>> **the above auto var “setbuf” has “struct” type, which
>> “can_native_interpret_type_p(var_type)” is false, therefore,
>> Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
>>
>> However, this structure type can be fitted into a register, therefore cannot
>> be taken address anymore at this stage, even though I tried:
>>
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>
>> To create an address variable for it, the expansion still failed at expr.c:
>> line 8412:
>> during RTL pass: expand
>> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
>> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
>> 0xd04104 expand_expr_addr_expr_1
>> ../../latest-gcc/gcc/expr.c:8412
>> 0xd04a95 expand_expr_addr_expr
>> ../../latest-gcc/gcc/expr.c:8525
>> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:11741
>> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:8713
>> 0xaed1d3 expand_expr
>> ../../latest-gcc/gcc/expr.h:301
>> 0xaf0d89 get_memory_rtx
>> ../../latest-gcc/gcc/builtins.c:1370
>> 0xafb4fb expand_builtin_memset_args
>> ../../latest-gcc/gcc/builtins.c:4102
>> 0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
>> ../../latest-gcc/gcc/builtins.c:3886
>> 0xe97fb3 expand_DEFERRED_INIT
>>
>> **That’s the major reason why I chose “use_register_for_decl(lhs)” to
>> decide “memset” expansion or “assign” expansion, “memset” expansion
>> needs to take address of the variable, if the variable has been decided to
>> fit into a register, then its address cannot taken anymore at this stage.
>>
>> **using
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 2:11 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>> Hi,
>>
>> I met another issue for “address taken” auto variable, see below for details:
>>
>> the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>>
>> int foo, bar;
>>
>> static
>> void decode_reloc(int reloc, int *is_alt)
>> {
>> if (reloc >= 20)
>> *is_alt = 1;
>> else if (reloc >= 10)
>> *is_alt = 0;
>> }
>>
>> void testfunc()
>> {
>> int alt_reloc;
>>
>> decode_reloc(foo, _reloc);
>>
>> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>>bar = 42;
>> }
>>
>> When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
>> -fdump-tree-all:
>>
>> .*gimple dump:
>>
>> void testfunc ()
>> {
>> int alt_reloc;
>>
>> try
>>{
>> _1 = .DEFERRED_INIT (4, 2, 0);
>> alt_reloc = _1;
>> foo.0_2 = foo;
>> decode_reloc (foo.0_2, _reloc);
>> alt_reloc.1_3 = alt_reloc;
>> if (alt_reloc.1_3 != 0) goto ; else goto ;
>> :
>> bar = 42;
>> :
>>}
>> finally
>>{
>> alt_reloc = {CLOBBER};
>>}
>> }
>>
>> **fre1 dump:
>>
>> void testfunc ()
>> {
>> int alt_reloc;
>> int _1;
>> int foo.0_2;
>>
>> :
>> _1 = .DEFERRED_INIT (4, 2, 0);
>> foo.0_2 = foo;
>> if (foo.0_2 > 19)
>>goto ; [50.00%]
>> else
>>goto ; [50.00%]
>>
>> :
>> goto ; [100.00%]
>>
>> :
>> if (foo.0_2 > 9)
>>goto ; [50.00%]
>> else
>>goto ; [50.00%]
>>
>> :
>> goto ; [100.00%]
>>
>> :
>> if (_1 != 0)
>>goto ; [INV]
>> else
>>goto ; [INV]
>>
>> :
>> bar = 42;
>>
>> :
>> return;
>>
>> }
>>
>> From the above IR file after “FRE”, we can see that the major issue with
>> this IR is:
>>
>> The address taken auto variable “alt_reloc” has been completely replaced by
>> the temporary variable “_1” in all
>> the uses of the original “alt_reloc”.
>
> Well, this can happen with regular code as well, there's no need for
> .DEFERRED_INIT. This is the usual problem with reporting uninitialized
> uses late.
>
> IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
> -Wuninitialized isn't really achievable.
Okay. Sounds reasonable to me too.
>
>> The major problem with such IR is, during uninitialized analysis phase, the
>> original use of “alt_reloc” disappeared completely.
>> So, the warning cannot be reported.
>>
>>
>> My questions:
>>
>> 1. Is it possible to get the original “alt_reloc” through the temporary
>> variable “_1” with some available information recorded in the IR?
>> 2. If not, then we have to record the relationship between “alt_reloc” and
>> “_1” when the original “alt_reloc” is replaced by “_1” and get such
>> relationship during
>>Uninitialized analysis phase. Is this doable?
>
> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
> diagnostics. The difficulty is to avoid tracking it as actual use so
> you could for example pass a string with the declarations name though
> this wouldn't give the association with the actual decl.
Good suggestion, I can try this a little bit.
>
>> 3. Looks like that for “address taken” auto variable, if we have to
>> introduce a new temporary variable and split the call to .DEFERRED_INIT into
>> two:
>>
>> temp = .DEFERRED_INIT (4, 2, 0);
>> alt_reloc = temp;
>>
>> More issues might possible.
>>
>> Any comments and suggestions on this issue?
>
> I don't see any good possibilities that would not make optimizing code
> as good as w/o .DEFERRED_INIT more difficult. My stake here is always
> that GCC is an optimizing compiler, not a static analysis engine and
> thus I side with "broken" diagnostics and better optimization.
That’s true and reasonable, too.
thanks.
Qing
>
> Richard.
>
>> Qing
>>
>> j
>>> On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
>>>
>>> On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
>>> wrote:
> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>
> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
> wrote:
>> I modified the routine “gimple_add_init_for_auto_var” as the following:
>>
>> /* Generate initialization to automatic variable DECL based on INIT_TYPE.
>> Build a call to internal const function DEFERRED_INIT:
>> 1st argument: SIZE of the DECL;
>> 2nd argument: INIT_TYPE;
>> 3rd argument: IS_VLA, 0 NO, 1 YES;
>>
>> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
>> static void
>> gimple_add_init_for_auto_var (tree decl,
>> enum auto_init_type init_type,
>> bool is_vla,
>> gimple_seq *seq_p)
>> {
>> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC
>> (decl));
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 2:40 AM, Richard Biener wrote:
>
> On Thu, 12 Aug 2021, Qing Zhao wrote:
>
>> Hi, Richard,
>>
>> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
>> suggestions like following:
>>
>> ==
>> #define INIT_PATTERN_VALUE 0xFE
>> static void
>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>> {
>> tree lhs = gimple_call_lhs (stmt);
>> tree var_size = gimple_call_arg (stmt, 0);
>> enum auto_init_type init_type
>>= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>
>> tree var_type = TREE_TYPE (lhs);
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>
>> if (is_vla || (!can_native_interpret_type_p (var_type)))
>>{
>>/* If this is a VLA or the type of the variable cannot be natively
>> interpreted, expand to a memset to initialize it. */
>> if (TREE_CODE (lhs) == SSA_NAME)
>>lhs = SSA_NAME_VAR (lhs);
>> tree var_addr = NULL_TREE;
>> if (is_vla)
>>var_addr = TREE_OPERAND (lhs, 0);
>> else
>>{
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>}
>> tree value = (init_type == AUTO_INIT_PATTERN) ?
>>build_int_cst (unsigned_char_type_node,
>> INIT_PATTERN_VALUE) :
>>build_zero_cst (unsigned_char_type_node);
>> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
>> 3, var_addr, value, var_size);
>> /* Expand this memset call. */
>> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>}
>> else
>>{
>>/* If this is not a VLA and the type of the variable can be natively
>> interpreted, expand to assignment to generate better code. */
>> tree pattern = NULL_TREE;
>> unsigned HOST_WIDE_INT total_bytes
>>= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>>
>> if (init_type == AUTO_INIT_PATTERN)
>>{
>> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>> memset (buf, INIT_PATTERN_VALUE, total_bytes);
>> pattern = native_interpret_expr (var_type, buf, total_bytes);
>> gcc_assert (pattern);
>>}
>>
>> tree init = (init_type == AUTO_INIT_PATTERN) ?
>> pattern :
>> build_zero_cst (var_type);
>> expand_assignment (lhs, init, false);
>>}
>> }
>> ===
>>
>> Now, I used “can_native_interpret_type_p (var_type)” instead of
>> “use_register_for_decl (lhs)” to decide
>> whether to use “memset” or use “assign” to expand this function.
>>
>> However, this exposed an bug that is very hard to be addressed:
>>
>> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>>
>> /* { dg-do compile } */
>> /* { dg-options "-O2 -Wuninitialized" } */
>>
>> int sys_msgctl (void)
>> {
>> struct { int mode; } setbuf;
>> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
>> ==
>>
>> **the above auto var “setbuf” has “struct” type, which
>> “can_native_interpret_type_p(var_type)” is false, therefore,
>> Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
>>
>> However, this structure type can be fitted into a register, therefore cannot
>> be taken address anymore at this stage, even though I tried:
>>
>> TREE_ADDRESSABLE (lhs) = 1;
>> var_addr = build_fold_addr_expr (lhs);
>>
>> To create an address variable for it, the expansion still failed at expr.c:
>> line 8412:
>> during RTL pass: expand
>> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
>> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
>> 0xd04104 expand_expr_addr_expr_1
>> ../../latest-gcc/gcc/expr.c:8412
>> 0xd04a95 expand_expr_addr_expr
>> ../../latest-gcc/gcc/expr.c:8525
>> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:11741
>> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
>> expand_modifier, rtx_def**, bool)
>> ../../latest-gcc/gcc/expr.c:8713
>> 0xaed1d3 expand_expr
>> ../../latest-gcc/gcc/expr.h:301
>> 0xaf0d89 get_memory_rtx
>> ../../latest-gcc/gcc/builtins.c:1370
>> 0xafb4fb expand_builtin_memset_args
>> ../../latest-gcc/gcc/builtins.c:4102
>> 0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
>> ../../latest-gcc/gcc/builtins.c:3886
>> 0xe97fb3 expand_DEFERRED_INIT
>>
>> **That’s the major reason why I chose “use_register_for_decl(lhs)” to
>> decide “memset” expansion or “assign” expansion, “memset” expansion
>> needs to take address of the variable, if the variable has been decided to
>> fit into a register, then its address cannot taken anymore at this stage.
>>
>> **using
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 10:08 AM, Richard Biener wrote:
>
> On August 16, 2021 4:48:16 PM GMT+02:00, Qing Zhao
> wrote:
>>
>>
>>> On Aug 16, 2021, at 2:12 AM, Richard Biener wrote:
>>>
>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>
Hi,
I finally decided to take another approach to resolve this issue, it
resolved all the potential issues with the “address taken” auto variable.
The basic idea is to avoid generating the temporary variable in the
beginning.
As you mentioned, "The reason is that alt_reloc is memory (because it is
address taken) and that GIMPLE says that register typed stores
need to use a is_gimple_val RHS which the call is not.”
In order to avoid generating the temporary variable for “address taken”
auto variable, I updated the utility routine “is_gimple_val” as following:
diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
index a2563a45c37d..d5ef1aef8cea 100644
--- a/gcc/gimple-expr.c
+++ b/gcc/gimple-expr.c
@@ -787,8 +787,20 @@ is_gimple_reg (tree t)
return !DECL_NOT_GIMPLE_REG_P (t);
}
+/* Return true if T is a call to .DEFERRED_INIT internal function. */
+static bool
+is_deferred_init_call (tree t)
+{
+ if (TREE_CODE (t) == CALL_EXPR
+ && CALL_EXPR_IFN (t) == IFN_DEFERRED_INIT)
+return true;
+ return false;
+}
+
-/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant.
*/
+/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant,
+ or a call to .DEFERRED_INIT internal function because the call to
+ .DEFERRED_INIT will eventually be expanded as a constant. */
bool
is_gimple_val (tree t)
@@ -799,7 +811,8 @@ is_gimple_val (tree t)
&& !is_gimple_reg (t))
return false;
- return (is_gimple_variable (t) || is_gimple_min_invariant (t));
+ return (is_gimple_variable (t) || is_gimple_min_invariant (t)
+ || is_deferred_init_call (t));
}
With this change, the temporary variable will not be created for “address
taken” auto variable, and uninitialized analysis does not need any change.
Everything works well.
And I believe that treating “call to .DEFERRED_INIT” as “is_gimple_val” is
reasonable since this call actually is a constant.
Let me know if you have any objection on this solution.
>>>
>>> Yeah, I object to this solution.
>>
>> Can you explain what’s the major issue for this solution?
>
> It punches a hole into the GIMPLE IL which is very likely incomplete and will
> cause issues elsewhere. In particular the corresponding type check is missing
> and not computable since the RHS of this call isn't separately available.
>
> If you go down this route then you should have added a new constant class
> tree code instead of an internal function call.
Okay. I see.
>
>> To me, treating “call to .DEFERRED_INIT” as “is_gimple_val” is reasonable
>> since this call actually is a constant.
>
> Sure, but it's not represented as such.
Thank you!.
Qing
>
> Richard.
>
>> Thanks.
>>
>> Qing
>>> Richard.
>>>
thanks.
Qing
> On Aug 11, 2021, at 3:30 PM, Qing Zhao via Gcc-patches
> wrote:
>
> Hi,
>
> I met another issue for “address taken” auto variable, see below for
> details:
>
> the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>
> int foo, bar;
>
> static
> void decode_reloc(int reloc, int *is_alt)
> {
> if (reloc >= 20)
>*is_alt = 1;
> else if (reloc >= 10)
>*is_alt = 0;
> }
>
> void testfunc()
> {
> int alt_reloc;
>
> decode_reloc(foo, _reloc);
>
> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
> bar = 42;
> }
>
> When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
> -fdump-tree-all:
>
> .*gimple dump:
>
> void testfunc ()
> {
> int alt_reloc;
>
> try
> {
>_1 = .DEFERRED_INIT (4, 2, 0);
>alt_reloc = _1;
>foo.0_2 = foo;
>decode_reloc (foo.0_2, _reloc);
>alt_reloc.1_3 = alt_reloc;
>if (alt_reloc.1_3 != 0) goto ; else goto ;
>:
>bar = 42;
>:
> }
> finally
> {
>alt_reloc = {CLOBBER};
> }
> }
>
> **fre1 dump:
>
> void testfunc ()
> {
> int alt_reloc;
> int _1;
> int foo.0_2;
>
> :
> _1 = .DEFERRED_INIT (4, 2, 0);
> foo.0_2 = foo;
> if (foo.0_2 > 19)
> goto ; [50.00%]
> else
> goto ; [50.00%]
>
> :
> goto ; [100.00%]
>
> :
> if (foo.0_2 > 9)
> goto ; [50.00%]
> else
> goto ; [50.00%]
>
> :
> goto ; [100.00%]
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On August 16, 2021 4:48:16 PM GMT+02:00, Qing Zhao wrote:
>
>
>> On Aug 16, 2021, at 2:12 AM, Richard Biener wrote:
>>
>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>
>>> Hi,
>>>
>>> I finally decided to take another approach to resolve this issue, it
>>> resolved all the potential issues with the “address taken” auto variable.
>>>
>>> The basic idea is to avoid generating the temporary variable in the
>>> beginning.
>>> As you mentioned, "The reason is that alt_reloc is memory (because it is
>>> address taken) and that GIMPLE says that register typed stores
>>> need to use a is_gimple_val RHS which the call is not.”
>>> In order to avoid generating the temporary variable for “address taken”
>>> auto variable, I updated the utility routine “is_gimple_val” as following:
>>>
>>> diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
>>> index a2563a45c37d..d5ef1aef8cea 100644
>>> --- a/gcc/gimple-expr.c
>>> +++ b/gcc/gimple-expr.c
>>> @@ -787,8 +787,20 @@ is_gimple_reg (tree t)
>>> return !DECL_NOT_GIMPLE_REG_P (t);
>>> }
>>>
>>> +/* Return true if T is a call to .DEFERRED_INIT internal function. */
>>> +static bool
>>> +is_deferred_init_call (tree t)
>>> +{
>>> + if (TREE_CODE (t) == CALL_EXPR
>>> + && CALL_EXPR_IFN (t) == IFN_DEFERRED_INIT)
>>> +return true;
>>> + return false;
>>> +}
>>> +
>>>
>>> -/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant.
>>> */
>>> +/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant,
>>> + or a call to .DEFERRED_INIT internal function because the call to
>>> + .DEFERRED_INIT will eventually be expanded as a constant. */
>>>
>>> bool
>>> is_gimple_val (tree t)
>>> @@ -799,7 +811,8 @@ is_gimple_val (tree t)
>>> && !is_gimple_reg (t))
>>> return false;
>>>
>>> - return (is_gimple_variable (t) || is_gimple_min_invariant (t));
>>> + return (is_gimple_variable (t) || is_gimple_min_invariant (t)
>>> + || is_deferred_init_call (t));
>>> }
>>>
>>> With this change, the temporary variable will not be created for “address
>>> taken” auto variable, and uninitialized analysis does not need any change.
>>> Everything works well.
>>>
>>> And I believe that treating “call to .DEFERRED_INIT” as “is_gimple_val” is
>>> reasonable since this call actually is a constant.
>>>
>>> Let me know if you have any objection on this solution.
>>
>> Yeah, I object to this solution.
>
>Can you explain what’s the major issue for this solution?
It punches a hole into the GIMPLE IL which is very likely incomplete and will
cause issues elsewhere. In particular the corresponding type check is missing
and not computable since the RHS of this call isn't separately available.
If you go down this route then you should have added a new constant class tree
code instead of an internal function call.
>To me, treating “call to .DEFERRED_INIT” as “is_gimple_val” is reasonable
>since this call actually is a constant.
Sure, but it's not represented as such.
Richard.
>Thanks.
>
>Qing
>> Richard.
>>
>>> thanks.
>>>
>>> Qing
>>>
On Aug 11, 2021, at 3:30 PM, Qing Zhao via Gcc-patches
wrote:
Hi,
I met another issue for “address taken” auto variable, see below for
details:
the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
int foo, bar;
static
void decode_reloc(int reloc, int *is_alt)
{
if (reloc >= 20)
*is_alt = 1;
else if (reloc >= 10)
*is_alt = 0;
}
void testfunc()
{
int alt_reloc;
decode_reloc(foo, _reloc);
if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
bar = 42;
}
When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
-fdump-tree-all:
.*gimple dump:
void testfunc ()
{
int alt_reloc;
try
{
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
foo.0_2 = foo;
decode_reloc (foo.0_2, _reloc);
alt_reloc.1_3 = alt_reloc;
if (alt_reloc.1_3 != 0) goto ; else goto ;
:
bar = 42;
:
}
finally
{
alt_reloc = {CLOBBER};
}
}
**fre1 dump:
void testfunc ()
{
int alt_reloc;
int _1;
int foo.0_2;
:
_1 = .DEFERRED_INIT (4, 2, 0);
foo.0_2 = foo;
if (foo.0_2 > 19)
goto ; [50.00%]
else
goto ; [50.00%]
:
goto ; [100.00%]
:
if (foo.0_2 > 9)
goto ; [50.00%]
else
goto ; [50.00%]
:
goto ; [100.00%]
:
if (_1 != 0)
goto ; [INV]
else
goto ; [INV]
:
bar = 42;
:
return;
}
From the above IR file after “FRE”, we can see that the major issue with
this IR is:
The address
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 16, 2021, at 2:12 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>> Hi,
>>
>> I finally decided to take another approach to resolve this issue, it
>> resolved all the potential issues with the “address taken” auto variable.
>>
>> The basic idea is to avoid generating the temporary variable in the
>> beginning.
>> As you mentioned, "The reason is that alt_reloc is memory (because it is
>> address taken) and that GIMPLE says that register typed stores
>> need to use a is_gimple_val RHS which the call is not.”
>> In order to avoid generating the temporary variable for “address taken” auto
>> variable, I updated the utility routine “is_gimple_val” as following:
>>
>> diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
>> index a2563a45c37d..d5ef1aef8cea 100644
>> --- a/gcc/gimple-expr.c
>> +++ b/gcc/gimple-expr.c
>> @@ -787,8 +787,20 @@ is_gimple_reg (tree t)
>> return !DECL_NOT_GIMPLE_REG_P (t);
>> }
>>
>> +/* Return true if T is a call to .DEFERRED_INIT internal function. */
>> +static bool
>> +is_deferred_init_call (tree t)
>> +{
>> + if (TREE_CODE (t) == CALL_EXPR
>> + && CALL_EXPR_IFN (t) == IFN_DEFERRED_INIT)
>> +return true;
>> + return false;
>> +}
>> +
>>
>> -/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant.
>> */
>> +/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant,
>> + or a call to .DEFERRED_INIT internal function because the call to
>> + .DEFERRED_INIT will eventually be expanded as a constant. */
>>
>> bool
>> is_gimple_val (tree t)
>> @@ -799,7 +811,8 @@ is_gimple_val (tree t)
>> && !is_gimple_reg (t))
>> return false;
>>
>> - return (is_gimple_variable (t) || is_gimple_min_invariant (t));
>> + return (is_gimple_variable (t) || is_gimple_min_invariant (t)
>> + || is_deferred_init_call (t));
>> }
>>
>> With this change, the temporary variable will not be created for “address
>> taken” auto variable, and uninitialized analysis does not need any change.
>> Everything works well.
>>
>> And I believe that treating “call to .DEFERRED_INIT” as “is_gimple_val” is
>> reasonable since this call actually is a constant.
>>
>> Let me know if you have any objection on this solution.
>
> Yeah, I object to this solution.
Can you explain what’s the major issue for this solution?
To me, treating “call to .DEFERRED_INIT” as “is_gimple_val” is reasonable
since this call actually is a constant.
Thanks.
Qing
> Richard.
>
>> thanks.
>>
>> Qing
>>
>>> On Aug 11, 2021, at 3:30 PM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>> Hi,
>>>
>>> I met another issue for “address taken” auto variable, see below for
>>> details:
>>>
>>> the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>>>
>>> int foo, bar;
>>>
>>> static
>>> void decode_reloc(int reloc, int *is_alt)
>>> {
>>> if (reloc >= 20)
>>> *is_alt = 1;
>>> else if (reloc >= 10)
>>> *is_alt = 0;
>>> }
>>>
>>> void testfunc()
>>> {
>>> int alt_reloc;
>>>
>>> decode_reloc(foo, _reloc);
>>>
>>> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>>> bar = 42;
>>> }
>>>
>>> When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
>>> -fdump-tree-all:
>>>
>>> .*gimple dump:
>>>
>>> void testfunc ()
>>> {
>>> int alt_reloc;
>>>
>>> try
>>> {
>>> _1 = .DEFERRED_INIT (4, 2, 0);
>>> alt_reloc = _1;
>>> foo.0_2 = foo;
>>> decode_reloc (foo.0_2, _reloc);
>>> alt_reloc.1_3 = alt_reloc;
>>> if (alt_reloc.1_3 != 0) goto ; else goto ;
>>> :
>>> bar = 42;
>>> :
>>> }
>>> finally
>>> {
>>> alt_reloc = {CLOBBER};
>>> }
>>> }
>>>
>>> **fre1 dump:
>>>
>>> void testfunc ()
>>> {
>>> int alt_reloc;
>>> int _1;
>>> int foo.0_2;
>>>
>>> :
>>> _1 = .DEFERRED_INIT (4, 2, 0);
>>> foo.0_2 = foo;
>>> if (foo.0_2 > 19)
>>> goto ; [50.00%]
>>> else
>>> goto ; [50.00%]
>>>
>>> :
>>> goto ; [100.00%]
>>>
>>> :
>>> if (foo.0_2 > 9)
>>> goto ; [50.00%]
>>> else
>>> goto ; [50.00%]
>>>
>>> :
>>> goto ; [100.00%]
>>>
>>> :
>>> if (_1 != 0)
>>> goto ; [INV]
>>> else
>>> goto ; [INV]
>>>
>>> :
>>> bar = 42;
>>>
>>> :
>>> return;
>>>
>>> }
>>>
>>> From the above IR file after “FRE”, we can see that the major issue with
>>> this IR is:
>>>
>>> The address taken auto variable “alt_reloc” has been completely replaced by
>>> the temporary variable “_1” in all
>>> the uses of the original “alt_reloc”.
>>>
>>> The major problem with such IR is, during uninitialized analysis phase,
>>> the original use of “alt_reloc” disappeared completely.
>>> So, the warning cannot be reported.
>>>
>>>
>>> My questions:
>>>
>>> 1. Is it possible to get the original “alt_reloc” through the temporary
>>> variable “_1” with some available information recorded in the IR?
>>> 2. If not, then we have to record the relationship between “alt_reloc” and
>>> “_1” when the original “alt_reloc” is replaced by “_1” and
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Thu, 12 Aug 2021, Qing Zhao wrote:
> Hi, Richard,
>
> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
> suggestions like following:
>
> ==
> #define INIT_PATTERN_VALUE 0xFE
> static void
> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> {
> tree lhs = gimple_call_lhs (stmt);
> tree var_size = gimple_call_arg (stmt, 0);
> enum auto_init_type init_type
> = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>
> tree var_type = TREE_TYPE (lhs);
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> if (is_vla || (!can_native_interpret_type_p (var_type)))
> {
> /* If this is a VLA or the type of the variable cannot be natively
>interpreted, expand to a memset to initialize it. */
> if (TREE_CODE (lhs) == SSA_NAME)
> lhs = SSA_NAME_VAR (lhs);
> tree var_addr = NULL_TREE;
> if (is_vla)
> var_addr = TREE_OPERAND (lhs, 0);
> else
> {
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
> }
> tree value = (init_type == AUTO_INIT_PATTERN) ?
> build_int_cst (unsigned_char_type_node,
>INIT_PATTERN_VALUE) :
> build_zero_cst (unsigned_char_type_node);
> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
> 3, var_addr, value, var_size);
> /* Expand this memset call. */
> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> }
> else
> {
> /* If this is not a VLA and the type of the variable can be natively
>interpreted, expand to assignment to generate better code. */
> tree pattern = NULL_TREE;
> unsigned HOST_WIDE_INT total_bytes
> = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>
> if (init_type == AUTO_INIT_PATTERN)
> {
> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> memset (buf, INIT_PATTERN_VALUE, total_bytes);
> pattern = native_interpret_expr (var_type, buf, total_bytes);
> gcc_assert (pattern);
> }
>
> tree init = (init_type == AUTO_INIT_PATTERN) ?
>pattern :
>build_zero_cst (var_type);
> expand_assignment (lhs, init, false);
> }
> }
> ===
>
> Now, I used “can_native_interpret_type_p (var_type)” instead of
> “use_register_for_decl (lhs)” to decide
> whether to use “memset” or use “assign” to expand this function.
>
> However, this exposed an bug that is very hard to be addressed:
>
> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -Wuninitialized" } */
>
> int sys_msgctl (void)
> {
> struct { int mode; } setbuf;
> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
> ==
>
> **the above auto var “setbuf” has “struct” type, which
> “can_native_interpret_type_p(var_type)” is false, therefore,
> Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
>
> However, this structure type can be fitted into a register, therefore cannot
> be taken address anymore at this stage, even though I tried:
>
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
>
> To create an address variable for it, the expansion still failed at expr.c:
> line 8412:
> during RTL pass: expand
> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
> internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
> 0xd04104 expand_expr_addr_expr_1
> ../../latest-gcc/gcc/expr.c:8412
> 0xd04a95 expand_expr_addr_expr
> ../../latest-gcc/gcc/expr.c:8525
> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:11741
> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> ../../latest-gcc/gcc/expr.c:8713
> 0xaed1d3 expand_expr
> ../../latest-gcc/gcc/expr.h:301
> 0xaf0d89 get_memory_rtx
> ../../latest-gcc/gcc/builtins.c:1370
> 0xafb4fb expand_builtin_memset_args
> ../../latest-gcc/gcc/builtins.c:4102
> 0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
> ../../latest-gcc/gcc/builtins.c:3886
> 0xe97fb3 expand_DEFERRED_INIT
>
> **That’s the major reason why I chose “use_register_for_decl(lhs)” to
> decide “memset” expansion or “assign” expansion, “memset” expansion
> needs to take address of the variable, if the variable has been decided to
> fit into a register, then its address cannot taken anymore at this stage.
>
> **using “can_native_interpret_type_p” did make the “pattern” generation
> part much cleaner and simpler, however, looks like it
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Wed, 11 Aug 2021, Qing Zhao wrote:
> Hi,
>
> I finally decided to take another approach to resolve this issue, it resolved
> all the potential issues with the “address taken” auto variable.
>
> The basic idea is to avoid generating the temporary variable in the
> beginning.
> As you mentioned, "The reason is that alt_reloc is memory (because it is
> address taken) and that GIMPLE says that register typed stores
> need to use a is_gimple_val RHS which the call is not.”
> In order to avoid generating the temporary variable for “address taken” auto
> variable, I updated the utility routine “is_gimple_val” as following:
>
> diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
> index a2563a45c37d..d5ef1aef8cea 100644
> --- a/gcc/gimple-expr.c
> +++ b/gcc/gimple-expr.c
> @@ -787,8 +787,20 @@ is_gimple_reg (tree t)
>return !DECL_NOT_GIMPLE_REG_P (t);
> }
>
> +/* Return true if T is a call to .DEFERRED_INIT internal function. */
> +static bool
> +is_deferred_init_call (tree t)
> +{
> + if (TREE_CODE (t) == CALL_EXPR
> + && CALL_EXPR_IFN (t) == IFN_DEFERRED_INIT)
> +return true;
> + return false;
> +}
> +
>
> -/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant. */
> +/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant,
> + or a call to .DEFERRED_INIT internal function because the call to
> + .DEFERRED_INIT will eventually be expanded as a constant. */
>
> bool
> is_gimple_val (tree t)
> @@ -799,7 +811,8 @@ is_gimple_val (tree t)
>&& !is_gimple_reg (t))
> return false;
>
> - return (is_gimple_variable (t) || is_gimple_min_invariant (t));
> + return (is_gimple_variable (t) || is_gimple_min_invariant (t)
> + || is_deferred_init_call (t));
> }
>
> With this change, the temporary variable will not be created for “address
> taken” auto variable, and uninitialized analysis does not need any change.
> Everything works well.
>
> And I believe that treating “call to .DEFERRED_INIT” as “is_gimple_val” is
> reasonable since this call actually is a constant.
>
> Let me know if you have any objection on this solution.
Yeah, I object to this solution.
Richard.
> thanks.
>
> Qing
>
> > On Aug 11, 2021, at 3:30 PM, Qing Zhao via Gcc-patches
> > wrote:
> >
> > Hi,
> >
> > I met another issue for “address taken” auto variable, see below for
> > details:
> >
> > the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
> >
> > int foo, bar;
> >
> > static
> > void decode_reloc(int reloc, int *is_alt)
> > {
> > if (reloc >= 20)
> > *is_alt = 1;
> > else if (reloc >= 10)
> > *is_alt = 0;
> > }
> >
> > void testfunc()
> > {
> > int alt_reloc;
> >
> > decode_reloc(foo, _reloc);
> >
> > if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
> >bar = 42;
> > }
> >
> > When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
> > -fdump-tree-all:
> >
> > .*gimple dump:
> >
> > void testfunc ()
> > {
> > int alt_reloc;
> >
> > try
> >{
> > _1 = .DEFERRED_INIT (4, 2, 0);
> > alt_reloc = _1;
> > foo.0_2 = foo;
> > decode_reloc (foo.0_2, _reloc);
> > alt_reloc.1_3 = alt_reloc;
> > if (alt_reloc.1_3 != 0) goto ; else goto ;
> > :
> > bar = 42;
> > :
> >}
> > finally
> >{
> > alt_reloc = {CLOBBER};
> >}
> > }
> >
> > **fre1 dump:
> >
> > void testfunc ()
> > {
> > int alt_reloc;
> > int _1;
> > int foo.0_2;
> >
> > :
> > _1 = .DEFERRED_INIT (4, 2, 0);
> > foo.0_2 = foo;
> > if (foo.0_2 > 19)
> >goto ; [50.00%]
> > else
> >goto ; [50.00%]
> >
> > :
> > goto ; [100.00%]
> >
> > :
> > if (foo.0_2 > 9)
> >goto ; [50.00%]
> > else
> >goto ; [50.00%]
> >
> > :
> > goto ; [100.00%]
> >
> > :
> > if (_1 != 0)
> >goto ; [INV]
> > else
> >goto ; [INV]
> >
> > :
> > bar = 42;
> >
> > :
> > return;
> >
> > }
> >
> > From the above IR file after “FRE”, we can see that the major issue with
> > this IR is:
> >
> > The address taken auto variable “alt_reloc” has been completely replaced by
> > the temporary variable “_1” in all
> > the uses of the original “alt_reloc”.
> >
> > The major problem with such IR is, during uninitialized analysis phase,
> > the original use of “alt_reloc” disappeared completely.
> > So, the warning cannot be reported.
> >
> >
> > My questions:
> >
> > 1. Is it possible to get the original “alt_reloc” through the temporary
> > variable “_1” with some available information recorded in the IR?
> > 2. If not, then we have to record the relationship between “alt_reloc” and
> > “_1” when the original “alt_reloc” is replaced by “_1” and get such
> > relationship during
> >Uninitialized analysis phase. Is this doable?
> > 3. Looks like that for “address taken” auto variable, if we have to
> > introduce a new temporary variable and split the call to .DEFERRED_INIT
> > into two:
> >
> >
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Wed, 11 Aug 2021, Qing Zhao wrote:
> Hi,
>
> I met another issue for “address taken” auto variable, see below for details:
>
> the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>
> int foo, bar;
>
> static
> void decode_reloc(int reloc, int *is_alt)
> {
> if (reloc >= 20)
> *is_alt = 1;
> else if (reloc >= 10)
> *is_alt = 0;
> }
>
> void testfunc()
> {
> int alt_reloc;
>
> decode_reloc(foo, _reloc);
>
> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
> bar = 42;
> }
>
> When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
> -fdump-tree-all:
>
> .*gimple dump:
>
> void testfunc ()
> {
> int alt_reloc;
>
> try
> {
> _1 = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = _1;
> foo.0_2 = foo;
> decode_reloc (foo.0_2, _reloc);
> alt_reloc.1_3 = alt_reloc;
> if (alt_reloc.1_3 != 0) goto ; else goto ;
> :
> bar = 42;
> :
> }
> finally
> {
> alt_reloc = {CLOBBER};
> }
> }
>
> **fre1 dump:
>
> void testfunc ()
> {
> int alt_reloc;
> int _1;
> int foo.0_2;
>
>:
> _1 = .DEFERRED_INIT (4, 2, 0);
> foo.0_2 = foo;
> if (foo.0_2 > 19)
> goto ; [50.00%]
> else
> goto ; [50.00%]
>
>:
> goto ; [100.00%]
>
>:
> if (foo.0_2 > 9)
> goto ; [50.00%]
> else
> goto ; [50.00%]
>
>:
> goto ; [100.00%]
>
>:
> if (_1 != 0)
> goto ; [INV]
> else
> goto ; [INV]
>
>:
> bar = 42;
>
>:
> return;
>
> }
>
> From the above IR file after “FRE”, we can see that the major issue with this
> IR is:
>
> The address taken auto variable “alt_reloc” has been completely replaced by
> the temporary variable “_1” in all
> the uses of the original “alt_reloc”.
Well, this can happen with regular code as well, there's no need for
.DEFERRED_INIT. This is the usual problem with reporting uninitialized
uses late.
IMHO this shouldn't be a blocker. The goal of zero "regressions" wrt
-Wuninitialized isn't really achievable.
> The major problem with such IR is, during uninitialized analysis phase, the
> original use of “alt_reloc” disappeared completely.
> So, the warning cannot be reported.
>
>
> My questions:
>
> 1. Is it possible to get the original “alt_reloc” through the temporary
> variable “_1” with some available information recorded in the IR?
> 2. If not, then we have to record the relationship between “alt_reloc” and
> “_1” when the original “alt_reloc” is replaced by “_1” and get such
> relationship during
> Uninitialized analysis phase. Is this doable?
Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
diagnostics. The difficulty is to avoid tracking it as actual use so
you could for example pass a string with the declarations name though
this wouldn't give the association with the actual decl.
> 3. Looks like that for “address taken” auto variable, if we have to introduce
> a new temporary variable and split the call to .DEFERRED_INIT into two:
>
> temp = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = temp;
>
>More issues might possible.
>
> Any comments and suggestions on this issue?
I don't see any good possibilities that would not make optimizing code
as good as w/o .DEFERRED_INIT more difficult. My stake here is always
that GCC is an optimizing compiler, not a static analysis engine and
thus I side with "broken" diagnostics and better optimization.
Richard.
> Qing
>
> j
> > On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
> >
> > On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
> > wrote:
> >>
> >>
> >>> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
> >>>
> >>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
> >>> wrote:
> I modified the routine “gimple_add_init_for_auto_var” as the following:
>
> /* Generate initialization to automatic variable DECL based on INIT_TYPE.
> Build a call to internal const function DEFERRED_INIT:
> 1st argument: SIZE of the DECL;
> 2nd argument: INIT_TYPE;
> 3rd argument: IS_VLA, 0 NO, 1 YES;
>
> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
> static void
> gimple_add_init_for_auto_var (tree decl,
> enum auto_init_type init_type,
> bool is_vla,
> gimple_seq *seq_p)
> {
> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC
> (decl));
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>
> tree init_type_node
> = build_int_cst (integer_type_node, (int) init_type);
> tree is_vla_node
> = build_int_cst (integer_type_node, (int) is_vla);
>
> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
> IFN_DEFERRED_INIT,
>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi,
Although I kept my previous "use_register_for_decl(lhs)” to decide “memset”
expansion or “assign” expansion when expanding .DEFERRED_INIT
When generating “pattern” for “assign” expansion, I found that
“can_native_interpret_type_p(var_type)” combined with “native_interpret_expr”
make
the implementation cleaner and simpler as following:
if (init_type == AUTO_INIT_PATTERN)
{
if (can_native_interpret_type_p (var_type))
{
unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
memset (buf, INIT_PATTERN_VALUE, total_bytes);
pattern = native_interpret_expr (var_type, buf, total_bytes);
gcc_assert (pattern);
}
else
{
tree index_type = build_index_type (size_int (total_bytes - 1));
tree array_type = build_array_type (unsigned_char_type_node,
index_type);
tree element = build_int_cst (unsigned_char_type_node,
INIT_PATTERN_VALUE);
vec *elts = NULL;
for (unsigned int i = 0; i < total_bytes; i++)
CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE, element);
pattern = build_constructor (array_type, elts);
pattern = build1 (VIEW_CONVERT_EXPR, var_type, pattern);
}
}
Thanks.
Qing
On Aug 12, 2021, at 2:24 PM, Qing Zhao via Gcc-patches
wrote:
>
>
> Hi, Richard,
>
> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
> suggestions like following:
>
> ==
> #define INIT_PATTERN_VALUE 0xFE
> static void
> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> {
> tree lhs = gimple_call_lhs (stmt);
> tree var_size = gimple_call_arg (stmt, 0);
> enum auto_init_type init_type
>= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>
> tree var_type = TREE_TYPE (lhs);
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>
> if (is_vla || (!can_native_interpret_type_p (var_type)))
>{
>/* If this is a VLA or the type of the variable cannot be natively
> interpreted, expand to a memset to initialize it. */
> if (TREE_CODE (lhs) == SSA_NAME)
>lhs = SSA_NAME_VAR (lhs);
> tree var_addr = NULL_TREE;
> if (is_vla)
>var_addr = TREE_OPERAND (lhs, 0);
> else
>{
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
>}
> tree value = (init_type == AUTO_INIT_PATTERN) ?
>build_int_cst (unsigned_char_type_node,
> INIT_PATTERN_VALUE) :
>build_zero_cst (unsigned_char_type_node);
> tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
> 3, var_addr, value, var_size);
> /* Expand this memset call. */
> expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>}
> else
>{
>/* If this is not a VLA and the type of the variable can be natively
> interpreted, expand to assignment to generate better code. */
> tree pattern = NULL_TREE;
> unsigned HOST_WIDE_INT total_bytes
>= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>
> if (init_type == AUTO_INIT_PATTERN)
>{
> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> memset (buf, INIT_PATTERN_VALUE, total_bytes);
> pattern = native_interpret_expr (var_type, buf, total_bytes);
> gcc_assert (pattern);
>}
>
> tree init = (init_type == AUTO_INIT_PATTERN) ?
> pattern :
> build_zero_cst (var_type);
> expand_assignment (lhs, init, false);
>}
> }
> ===
>
> Now, I used “can_native_interpret_type_p (var_type)” instead of
> “use_register_for_decl (lhs)” to decide
> whether to use “memset” or use “assign” to expand this function.
>
> However, this exposed an bug that is very hard to be addressed:
>
> ***For the testing case: test suite/gcc.dg/uninit-I.c:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -Wuninitialized" } */
>
> int sys_msgctl (void)
> {
> struct { int mode; } setbuf;
> return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
> ==
>
> **the above auto var “setbuf” has “struct” type, which
> “can_native_interpret_type_p(var_type)” is false, therefore,
> Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
>
> However, this structure type can be fitted into a register, therefore cannot
> be taken address anymore at this stage, even though I tried:
>
> TREE_ADDRESSABLE (lhs) = 1;
> var_addr = build_fold_addr_expr (lhs);
>
> To create an address variable for it, the expansion still failed at expr.c:
> line 8412:
>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi, Richard,
For RTL expansion of call to .DEFERRED_INIT, I changed my code per your
suggestions like following:
==
#define INIT_PATTERN_VALUE 0xFE
static void
expand_DEFERRED_INIT (internal_fn, gcall *stmt)
{
tree lhs = gimple_call_lhs (stmt);
tree var_size = gimple_call_arg (stmt, 0);
enum auto_init_type init_type
= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
tree var_type = TREE_TYPE (lhs);
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
if (is_vla || (!can_native_interpret_type_p (var_type)))
{
/* If this is a VLA or the type of the variable cannot be natively
interpreted, expand to a memset to initialize it. */
if (TREE_CODE (lhs) == SSA_NAME)
lhs = SSA_NAME_VAR (lhs);
tree var_addr = NULL_TREE;
if (is_vla)
var_addr = TREE_OPERAND (lhs, 0);
else
{
TREE_ADDRESSABLE (lhs) = 1;
var_addr = build_fold_addr_expr (lhs);
}
tree value = (init_type == AUTO_INIT_PATTERN) ?
build_int_cst (unsigned_char_type_node,
INIT_PATTERN_VALUE) :
build_zero_cst (unsigned_char_type_node);
tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
3, var_addr, value, var_size);
/* Expand this memset call. */
expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
}
else
{
/* If this is not a VLA and the type of the variable can be natively
interpreted, expand to assignment to generate better code. */
tree pattern = NULL_TREE;
unsigned HOST_WIDE_INT total_bytes
= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
if (init_type == AUTO_INIT_PATTERN)
{
unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
memset (buf, INIT_PATTERN_VALUE, total_bytes);
pattern = native_interpret_expr (var_type, buf, total_bytes);
gcc_assert (pattern);
}
tree init = (init_type == AUTO_INIT_PATTERN) ?
pattern :
build_zero_cst (var_type);
expand_assignment (lhs, init, false);
}
}
===
Now, I used “can_native_interpret_type_p (var_type)” instead of
“use_register_for_decl (lhs)” to decide
whether to use “memset” or use “assign” to expand this function.
However, this exposed an bug that is very hard to be addressed:
***For the testing case: test suite/gcc.dg/uninit-I.c:
/* { dg-do compile } */
/* { dg-options "-O2 -Wuninitialized" } */
int sys_msgctl (void)
{
struct { int mode; } setbuf;
return setbuf.mode; /* { dg-warning "'setbuf\.mode' is used" } */
==
**the above auto var “setbuf” has “struct” type, which
“can_native_interpret_type_p(var_type)” is false, therefore,
Expanding this .DEFERRED_INIT call went down the “memset” expansion route.
However, this structure type can be fitted into a register, therefore cannot be
taken address anymore at this stage, even though I tried:
TREE_ADDRESSABLE (lhs) = 1;
var_addr = build_fold_addr_expr (lhs);
To create an address variable for it, the expansion still failed at expr.c:
line 8412:
during RTL pass: expand
/home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24:
internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
0xd04104 expand_expr_addr_expr_1
../../latest-gcc/gcc/expr.c:8412
0xd04a95 expand_expr_addr_expr
../../latest-gcc/gcc/expr.c:8525
0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../latest-gcc/gcc/expr.c:11741
0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier,
rtx_def**, bool)
../../latest-gcc/gcc/expr.c:8713
0xaed1d3 expand_expr
../../latest-gcc/gcc/expr.h:301
0xaf0d89 get_memory_rtx
../../latest-gcc/gcc/builtins.c:1370
0xafb4fb expand_builtin_memset_args
../../latest-gcc/gcc/builtins.c:4102
0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
../../latest-gcc/gcc/builtins.c:3886
0xe97fb3 expand_DEFERRED_INIT
**That’s the major reason why I chose “use_register_for_decl(lhs)” to
decide “memset” expansion or “assign” expansion, “memset” expansion
needs to take address of the variable, if the variable has been decided to fit
into a register, then its address cannot taken anymore at this stage.
**using “can_native_interpret_type_p” did make the “pattern” generation
part much cleaner and simpler, however, looks like it didn’t work correctly.
Based on this, I’d like to keep my previous implementation by using
“use_register_for_decl” to decide whether to take “memset” expansion or
“assign” expansion.
Therefore, I might still need to keep the “UGLY” implementation of generatting
“pattern”
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi,
I finally decided to take another approach to resolve this issue, it resolved
all the potential issues with the “address taken” auto variable.
The basic idea is to avoid generating the temporary variable in the beginning.
As you mentioned, "The reason is that alt_reloc is memory (because it is
address taken) and that GIMPLE says that register typed stores
need to use a is_gimple_val RHS which the call is not.”
In order to avoid generating the temporary variable for “address taken” auto
variable, I updated the utility routine “is_gimple_val” as following:
diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
index a2563a45c37d..d5ef1aef8cea 100644
--- a/gcc/gimple-expr.c
+++ b/gcc/gimple-expr.c
@@ -787,8 +787,20 @@ is_gimple_reg (tree t)
return !DECL_NOT_GIMPLE_REG_P (t);
}
+/* Return true if T is a call to .DEFERRED_INIT internal function. */
+static bool
+is_deferred_init_call (tree t)
+{
+ if (TREE_CODE (t) == CALL_EXPR
+ && CALL_EXPR_IFN (t) == IFN_DEFERRED_INIT)
+return true;
+ return false;
+}
+
-/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant. */
+/* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant,
+ or a call to .DEFERRED_INIT internal function because the call to
+ .DEFERRED_INIT will eventually be expanded as a constant. */
bool
is_gimple_val (tree t)
@@ -799,7 +811,8 @@ is_gimple_val (tree t)
&& !is_gimple_reg (t))
return false;
- return (is_gimple_variable (t) || is_gimple_min_invariant (t));
+ return (is_gimple_variable (t) || is_gimple_min_invariant (t)
+ || is_deferred_init_call (t));
}
With this change, the temporary variable will not be created for “address
taken” auto variable, and uninitialized analysis does not need any change.
Everything works well.
And I believe that treating “call to .DEFERRED_INIT” as “is_gimple_val” is
reasonable since this call actually is a constant.
Let me know if you have any objection on this solution.
thanks.
Qing
> On Aug 11, 2021, at 3:30 PM, Qing Zhao via Gcc-patches
> wrote:
>
> Hi,
>
> I met another issue for “address taken” auto variable, see below for details:
>
> the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>
> int foo, bar;
>
> static
> void decode_reloc(int reloc, int *is_alt)
> {
> if (reloc >= 20)
> *is_alt = 1;
> else if (reloc >= 10)
> *is_alt = 0;
> }
>
> void testfunc()
> {
> int alt_reloc;
>
> decode_reloc(foo, _reloc);
>
> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>bar = 42;
> }
>
> When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
> -fdump-tree-all:
>
> .*gimple dump:
>
> void testfunc ()
> {
> int alt_reloc;
>
> try
>{
> _1 = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = _1;
> foo.0_2 = foo;
> decode_reloc (foo.0_2, _reloc);
> alt_reloc.1_3 = alt_reloc;
> if (alt_reloc.1_3 != 0) goto ; else goto ;
> :
> bar = 42;
> :
>}
> finally
>{
> alt_reloc = {CLOBBER};
>}
> }
>
> **fre1 dump:
>
> void testfunc ()
> {
> int alt_reloc;
> int _1;
> int foo.0_2;
>
> :
> _1 = .DEFERRED_INIT (4, 2, 0);
> foo.0_2 = foo;
> if (foo.0_2 > 19)
>goto ; [50.00%]
> else
>goto ; [50.00%]
>
> :
> goto ; [100.00%]
>
> :
> if (foo.0_2 > 9)
>goto ; [50.00%]
> else
>goto ; [50.00%]
>
> :
> goto ; [100.00%]
>
> :
> if (_1 != 0)
>goto ; [INV]
> else
>goto ; [INV]
>
> :
> bar = 42;
>
> :
> return;
>
> }
>
> From the above IR file after “FRE”, we can see that the major issue with this
> IR is:
>
> The address taken auto variable “alt_reloc” has been completely replaced by
> the temporary variable “_1” in all
> the uses of the original “alt_reloc”.
>
> The major problem with such IR is, during uninitialized analysis phase, the
> original use of “alt_reloc” disappeared completely.
> So, the warning cannot be reported.
>
>
> My questions:
>
> 1. Is it possible to get the original “alt_reloc” through the temporary
> variable “_1” with some available information recorded in the IR?
> 2. If not, then we have to record the relationship between “alt_reloc” and
> “_1” when the original “alt_reloc” is replaced by “_1” and get such
> relationship during
>Uninitialized analysis phase. Is this doable?
> 3. Looks like that for “address taken” auto variable, if we have to introduce
> a new temporary variable and split the call to .DEFERRED_INIT into two:
>
> temp = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = temp;
>
> More issues might possible.
>
> Any comments and suggestions on this issue?
>
> Qing
>
> j
>> On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
>>
>> On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
>> wrote:
>>>
>>>
On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
wrote:
> I modified the
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi,
I met another issue for “address taken” auto variable, see below for details:
the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
int foo, bar;
static
void decode_reloc(int reloc, int *is_alt)
{
if (reloc >= 20)
*is_alt = 1;
else if (reloc >= 10)
*is_alt = 0;
}
void testfunc()
{
int alt_reloc;
decode_reloc(foo, _reloc);
if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
bar = 42;
}
When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized
-fdump-tree-all:
.*gimple dump:
void testfunc ()
{
int alt_reloc;
try
{
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
foo.0_2 = foo;
decode_reloc (foo.0_2, _reloc);
alt_reloc.1_3 = alt_reloc;
if (alt_reloc.1_3 != 0) goto ; else goto ;
:
bar = 42;
:
}
finally
{
alt_reloc = {CLOBBER};
}
}
**fre1 dump:
void testfunc ()
{
int alt_reloc;
int _1;
int foo.0_2;
:
_1 = .DEFERRED_INIT (4, 2, 0);
foo.0_2 = foo;
if (foo.0_2 > 19)
goto ; [50.00%]
else
goto ; [50.00%]
:
goto ; [100.00%]
:
if (foo.0_2 > 9)
goto ; [50.00%]
else
goto ; [50.00%]
:
goto ; [100.00%]
:
if (_1 != 0)
goto ; [INV]
else
goto ; [INV]
:
bar = 42;
:
return;
}
From the above IR file after “FRE”, we can see that the major issue with this
IR is:
The address taken auto variable “alt_reloc” has been completely replaced by the
temporary variable “_1” in all
the uses of the original “alt_reloc”.
The major problem with such IR is, during uninitialized analysis phase, the
original use of “alt_reloc” disappeared completely.
So, the warning cannot be reported.
My questions:
1. Is it possible to get the original “alt_reloc” through the temporary
variable “_1” with some available information recorded in the IR?
2. If not, then we have to record the relationship between “alt_reloc” and “_1”
when the original “alt_reloc” is replaced by “_1” and get such relationship
during
Uninitialized analysis phase. Is this doable?
3. Looks like that for “address taken” auto variable, if we have to introduce a
new temporary variable and split the call to .DEFERRED_INIT into two:
temp = .DEFERRED_INIT (4, 2, 0);
alt_reloc = temp;
More issues might possible.
Any comments and suggestions on this issue?
Qing
j
> On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
>
> On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
> wrote:
>>
>>
>>> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>>>
>>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
>>> wrote:
I modified the routine “gimple_add_init_for_auto_var” as the following:
/* Generate initialization to automatic variable DECL based on INIT_TYPE.
Build a call to internal const function DEFERRED_INIT:
1st argument: SIZE of the DECL;
2nd argument: INIT_TYPE;
3rd argument: IS_VLA, 0 NO, 1 YES;
as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
static void
gimple_add_init_for_auto_var (tree decl,
enum auto_init_type init_type,
bool is_vla,
gimple_seq *seq_p)
{
gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
tree init_type_node
= build_int_cst (integer_type_node, (int) init_type);
tree is_vla_node
= build_int_cst (integer_type_node, (int) is_vla);
tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
IFN_DEFERRED_INIT,
TREE_TYPE (decl), 3,
decl_size, init_type_node,
is_vla_node);
/* If this DECL is a VLA, a temporary address variable for it has been
created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
we should use it as the LHS of the call. */
tree lhs_call
= is_vla ? DECL_VALUE_EXPR (decl) : decl;
gimplify_assign (lhs_call, call, seq_p);
}
With this change, the current issue is resolved, the gimple dump now is:
(*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
However, there is another new issue:
For the following testing case:
==
[opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
int bar;
extern void decode_reloc(int *);
void testfunc()
{
int alt_reloc;
decode_reloc(_reloc);
if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
bar = 42;
}
=
In the above, the auto var “alt_reloc” is address taken, then the gimple
dump for it when compiled with -ftrivial-auto-var-init=zero
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 11:55 AM, Richard Biener wrote:
>
> On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao
> wrote:
>>
>>
>>> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>>>
>>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
>>> wrote:
I modified the routine “gimple_add_init_for_auto_var” as the following:
/* Generate initialization to automatic variable DECL based on INIT_TYPE.
Build a call to internal const function DEFERRED_INIT:
1st argument: SIZE of the DECL;
2nd argument: INIT_TYPE;
3rd argument: IS_VLA, 0 NO, 1 YES;
as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
static void
gimple_add_init_for_auto_var (tree decl,
enum auto_init_type init_type,
bool is_vla,
gimple_seq *seq_p)
{
gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
tree init_type_node
= build_int_cst (integer_type_node, (int) init_type);
tree is_vla_node
= build_int_cst (integer_type_node, (int) is_vla);
tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
IFN_DEFERRED_INIT,
TREE_TYPE (decl), 3,
decl_size, init_type_node,
is_vla_node);
/* If this DECL is a VLA, a temporary address variable for it has been
created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
we should use it as the LHS of the call. */
tree lhs_call
= is_vla ? DECL_VALUE_EXPR (decl) : decl;
gimplify_assign (lhs_call, call, seq_p);
}
With this change, the current issue is resolved, the gimple dump now is:
(*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
However, there is another new issue:
For the following testing case:
==
[opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
int bar;
extern void decode_reloc(int *);
void testfunc()
{
int alt_reloc;
decode_reloc(_reloc);
if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
bar = 42;
}
=
In the above, the auto var “alt_reloc” is address taken, then the gimple
dump for it when compiled with -ftrivial-auto-var-init=zero is:
void testfunc ()
{
int alt_reloc;
try
{
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
decode_reloc (_reloc);
alt_reloc.0_2 = alt_reloc;
if (alt_reloc.0_2 != 0) goto ; else goto ;
:
bar = 42;
:
}
finally
{
alt_reloc = {CLOBBER};
}
}
I.e, instead of the expected IR:
alt_reloc = .DEFERRED_INIT (4, 2, 0);
We got the following:
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
I guess the temp “_1” is created because “alt_reloc” is address taken.
>>>
>>> Yes and no. The reason is that alt_reloc is memory (because it is address
>>> taken) and that GIMPLE says that register typed stores need to use a
>>> is_gimple_val RHS which the call is not.
>>
>> Okay.
>>>
My questions:
Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is
address taken?
>>>
>>> I think so. Note it doesn't necessarily need address taking but any other
>>> reason that prevents SSA rewriting the variable suffices.
>>
>> You mean, in addition to “address taken”, there are other situations that
>> will introduce such IR:
>>
>> temp = .DEFERRED_INIT();
>> auto_var = temp;
>>
>> So, such IR is unavoidable and we have to handle it?
>
> Yes.
>
>> If we have to handle it, what’ the best way to do it?
>>
>> The solution in my mind is:
>> 1. During uninitialized analysis phase, following the data flow to connect
>> .DEFERRED_INIT to “auto_var”, and then decide that “auto_var” is
>> uninitialized.
>
> Yes. Basically if there's an artificial variable auto initialized you have to
> look at its uses.
Okay.
>
>> 2. During RTL expansion, following the data flow to connect .DEFERRED_INIT
>> to “auto_var”, and then delete “temp”, and then expand .DEFERRED_INIT to
>> auto_var.
>
> That shouldn't be necessary. You'd initialize a temporary register which is
> then copied to the real variable. That's good enough and should be optimized
> by the RTL pipeline.
Okay, I see.
I will try to update the code to see whether all the issues can be resolved.
Thanks a lot for your help.
Qing
>
>> Let me know your comments and suggestions on this.
>>
>>
>>>
>>> The only other option is to force. DEFERED_INIT making the LHS address
>>> taken which I think could
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao wrote:
>
>
>> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>>
>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
>> wrote:
>>> I modified the routine “gimple_add_init_for_auto_var” as the following:
>>>
>>> /* Generate initialization to automatic variable DECL based on INIT_TYPE.
>>> Build a call to internal const function DEFERRED_INIT:
>>> 1st argument: SIZE of the DECL;
>>> 2nd argument: INIT_TYPE;
>>> 3rd argument: IS_VLA, 0 NO, 1 YES;
>>>
>>> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
>>> static void
>>> gimple_add_init_for_auto_var (tree decl,
>>> enum auto_init_type init_type,
>>> bool is_vla,
>>> gimple_seq *seq_p)
>>> {
>>> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
>>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>>>
>>> tree init_type_node
>>> = build_int_cst (integer_type_node, (int) init_type);
>>> tree is_vla_node
>>> = build_int_cst (integer_type_node, (int) is_vla);
>>>
>>> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
>>> IFN_DEFERRED_INIT,
>>> TREE_TYPE (decl), 3,
>>> decl_size, init_type_node,
>>> is_vla_node);
>>>
>>> /* If this DECL is a VLA, a temporary address variable for it has been
>>>created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
>>>we should use it as the LHS of the call. */
>>>
>>> tree lhs_call
>>> = is_vla ? DECL_VALUE_EXPR (decl) : decl;
>>> gimplify_assign (lhs_call, call, seq_p);
>>> }
>>>
>>> With this change, the current issue is resolved, the gimple dump now is:
>>>
>>> (*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> However, there is another new issue:
>>>
>>> For the following testing case:
>>>
>>> ==
>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
>>> int bar;
>>>
>>> extern void decode_reloc(int *);
>>>
>>> void testfunc()
>>> {
>>> int alt_reloc;
>>>
>>> decode_reloc(_reloc);
>>>
>>> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>>> bar = 42;
>>> }
>>> =
>>>
>>> In the above, the auto var “alt_reloc” is address taken, then the gimple
>>> dump for it when compiled with -ftrivial-auto-var-init=zero is:
>>>
>>> void testfunc ()
>>> {
>>> int alt_reloc;
>>>
>>> try
>>> {
>>> _1 = .DEFERRED_INIT (4, 2, 0);
>>> alt_reloc = _1;
>>> decode_reloc (_reloc);
>>> alt_reloc.0_2 = alt_reloc;
>>> if (alt_reloc.0_2 != 0) goto ; else goto ;
>>> :
>>> bar = 42;
>>> :
>>> }
>>> finally
>>> {
>>> alt_reloc = {CLOBBER};
>>> }
>>> }
>>>
>>> I.e, instead of the expected IR:
>>>
>>> alt_reloc = .DEFERRED_INIT (4, 2, 0);
>>>
>>> We got the following:
>>>
>>> _1 = .DEFERRED_INIT (4, 2, 0);
>>> alt_reloc = _1;
>>>
>>> I guess the temp “_1” is created because “alt_reloc” is address taken.
>>
>> Yes and no. The reason is that alt_reloc is memory (because it is address
>> taken) and that GIMPLE says that register typed stores need to use a
>> is_gimple_val RHS which the call is not.
>
>Okay.
>>
>>> My questions:
>>>
>>> Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is
>>> address taken?
>>
>> I think so. Note it doesn't necessarily need address taking but any other
>> reason that prevents SSA rewriting the variable suffices.
>
>You mean, in addition to “address taken”, there are other situations that will
>introduce such IR:
>
>temp = .DEFERRED_INIT();
>auto_var = temp;
>
>So, such IR is unavoidable and we have to handle it?
Yes.
>If we have to handle it, what’ the best way to do it?
>
>The solution in my mind is:
>1. During uninitialized analysis phase, following the data flow to connect
>.DEFERRED_INIT to “auto_var”, and then decide that “auto_var” is uninitialized.
Yes. Basically if there's an artificial variable auto initialized you have to
look at its uses.
>2. During RTL expansion, following the data flow to connect .DEFERRED_INIT to
>“auto_var”, and then delete “temp”, and then expand .DEFERRED_INIT to auto_var.
That shouldn't be necessary. You'd initialize a temporary register which is
then copied to the real variable. That's good enough and should be optimized by
the RTL pipeline.
>Let me know your comments and suggestions on this.
>
>
>>
>> The only other option is to force. DEFERED_INIT making the LHS address taken
>> which I think could be achieved by passing it the address as argument
>> instead of having a LHS. But let's not go down this route - it will have
>> quite bad behavior on alias analysis and optimization.
>
>Okay.
>
>Qing
>>
>>> If so, “uninitialized analysis” phase need to be further adjusted to
>>> specially handle such IR.
>>>
>>> If not, what should we do when the auto
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 11:15 AM, Richard Sandiford > wrote: > > Qing Zhao writes: >>> On Aug 11, 2021, at 4:02 AM, Richard Sandiford >>> wrote: I came up with the following solution: Define the IFN_DEFERRED_INIT function as: LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA); if IS_VLA is false, the LHS is the DECL itself, if IS_VLA is true, the LHS is the pointer to this DECL that created by gimplify_vla_decl. The benefit of this solution are: 1. Resolved the invalid IR issue; 2. The call stmt carries the address of the VLA natually; The issue with this solution is: For VLA and non-VLA, the LHS will be different, Do you see any other potential issues with this solution? >>> >>> The idea behind the DECL version of the .DEFERRED_INIT semantics was >>> that .DEFERRED_INIT just returns a SIZE-byte value that the caller >>> then assigns to a SIZE-byte lhs (with the caller choosing the lhs). >>> .DEFEREED_INIT itself doesn't read or write memory and so can be const, >>> which in turn allows alias analysis to be more precise. >> Yes. That’s right. >> >>> >>> If we want to handle the VLA case using pointers instead then I think >>> that needs to be a different IFN. >>> >>> If we did handle the VLA case using pointers (not expressing an opinion >>> on that), then it would be the caller's job to allocate the VLA and work >>> out the address of the VLA; >> >> the current routine “gimplify_vla_decl” has done this already: >> >> It created a temporary variable for the address of the VLA, and created a >> call to “alloca” to allocate the VLA. > > Right, that's what I mean. It's this alloca that allocates the VLA > and determines its address. This address is therefore logically an > input rather than an output to the following zero/pattern initialisation. > > In C you wouldn't write: > > addr = alloca(size); > addr = initialise(size); > > to allocate and initialise a size-byte buffer, because initialise() > would need to know the address of the memory it's supposed to initialise. > The same is true for this gimple code. This really make good sense to me. :-) > >> My -ftrivial-auto-var-init work just try to use the “address variable of the >> VLA” in the new .DEFERRED_INIT call to carry it to RTL expansion phase. >> >> >>> this isn't something that .DEFERRED_INIT >>> would work out on the caller's behalf. The address of the VLA should >>> therefore be an argument to the new IFN, rather than something that >>> the IFN returns. >> >> Then what’s the LHS of this call? Currently the major issue is the LHS is >> invalid gimple. > > For this (different, address-taking, VLA-only) IFN, there would be no lhs. > The IFN would be similar to a memset. I see. > > Like I say, this is all hypothetical, based on “if we did handle the VLA > case using pointers”. As discussed, it would make alias analysis less > precise. I was just answering the question about whether there were > potential issues. Okay, understood. I will not handle the VLA case using pointers at this time. Per discussion with Richard Biener in the other emails, I might go the other route to special handle the _1 = .DEFERRED_INIT (4, 2, 0); alt_reloc = _1; To see whether that can resolve the issues. Let me know your opinion. Thanks a lot. Qing > > Thanks, > Richard
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 10:53 AM, Richard Biener wrote:
>
> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao
> wrote:
>> I modified the routine “gimple_add_init_for_auto_var” as the following:
>>
>> /* Generate initialization to automatic variable DECL based on INIT_TYPE.
>> Build a call to internal const function DEFERRED_INIT:
>> 1st argument: SIZE of the DECL;
>> 2nd argument: INIT_TYPE;
>> 3rd argument: IS_VLA, 0 NO, 1 YES;
>>
>> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
>> static void
>> gimple_add_init_for_auto_var (tree decl,
>> enum auto_init_type init_type,
>> bool is_vla,
>> gimple_seq *seq_p)
>> {
>> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>>
>> tree init_type_node
>> = build_int_cst (integer_type_node, (int) init_type);
>> tree is_vla_node
>> = build_int_cst (integer_type_node, (int) is_vla);
>>
>> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
>> IFN_DEFERRED_INIT,
>> TREE_TYPE (decl), 3,
>> decl_size, init_type_node,
>> is_vla_node);
>>
>> /* If this DECL is a VLA, a temporary address variable for it has been
>>created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
>>we should use it as the LHS of the call. */
>>
>> tree lhs_call
>> = is_vla ? DECL_VALUE_EXPR (decl) : decl;
>> gimplify_assign (lhs_call, call, seq_p);
>> }
>>
>> With this change, the current issue is resolved, the gimple dump now is:
>>
>> (*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
>>
>> However, there is another new issue:
>>
>> For the following testing case:
>>
>> ==
>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
>> int bar;
>>
>> extern void decode_reloc(int *);
>>
>> void testfunc()
>> {
>> int alt_reloc;
>>
>> decode_reloc(_reloc);
>>
>> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>> bar = 42;
>> }
>> =
>>
>> In the above, the auto var “alt_reloc” is address taken, then the gimple
>> dump for it when compiled with -ftrivial-auto-var-init=zero is:
>>
>> void testfunc ()
>> {
>> int alt_reloc;
>>
>> try
>> {
>> _1 = .DEFERRED_INIT (4, 2, 0);
>> alt_reloc = _1;
>> decode_reloc (_reloc);
>> alt_reloc.0_2 = alt_reloc;
>> if (alt_reloc.0_2 != 0) goto ; else goto ;
>> :
>> bar = 42;
>> :
>> }
>> finally
>> {
>> alt_reloc = {CLOBBER};
>> }
>> }
>>
>> I.e, instead of the expected IR:
>>
>> alt_reloc = .DEFERRED_INIT (4, 2, 0);
>>
>> We got the following:
>>
>> _1 = .DEFERRED_INIT (4, 2, 0);
>> alt_reloc = _1;
>>
>> I guess the temp “_1” is created because “alt_reloc” is address taken.
>
> Yes and no. The reason is that alt_reloc is memory (because it is address
> taken) and that GIMPLE says that register typed stores need to use a
> is_gimple_val RHS which the call is not.
Okay.
>
>> My questions:
>>
>> Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is
>> address taken?
>
> I think so. Note it doesn't necessarily need address taking but any other
> reason that prevents SSA rewriting the variable suffices.
You mean, in addition to “address taken”, there are other situations that will
introduce such IR:
temp = .DEFERRED_INIT();
auto_var = temp;
So, such IR is unavoidable and we have to handle it?
If we have to handle it, what’ the best way to do it?
The solution in my mind is:
1. During uninitialized analysis phase, following the data flow to connect
.DEFERRED_INIT to “auto_var”, and then decide that “auto_var” is uninitialized.
2. During RTL expansion, following the data flow to connect .DEFERRED_INIT to
“auto_var”, and then delete “temp”, and then expand .DEFERRED_INIT to auto_var.
Let me know your comments and suggestions on this.
>
> The only other option is to force. DEFERED_INIT making the LHS address taken
> which I think could be achieved by passing it the address as argument instead
> of having a LHS. But let's not go down this route - it will have quite bad
> behavior on alias analysis and optimization.
Okay.
Qing
>
>> If so, “uninitialized analysis” phase need to be further adjusted to
>> specially handle such IR.
>>
>> If not, what should we do when the auto var is address taken?
>>
>> Thanks a lot.
>>
>> Qing
>>
>>
>>> On Aug 11, 2021, at 8:58 AM, Richard Biener wrote:
>>>
>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>>>
>>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 10, 2021, at 3:16
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Qing Zhao writes: >> On Aug 11, 2021, at 4:02 AM, Richard Sandiford >> wrote: >>> I came up with the following solution: >>> >>> Define the IFN_DEFERRED_INIT function as: >>> >>> LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA); >>> >>> if IS_VLA is false, the LHS is the DECL itself, >>> if IS_VLA is true, the LHS is the pointer to this DECL that created by >>> gimplify_vla_decl. >>> >>> >>> The benefit of this solution are: >>> >>> 1. Resolved the invalid IR issue; >>> 2. The call stmt carries the address of the VLA natually; >>> >>> The issue with this solution is: >>> >>> For VLA and non-VLA, the LHS will be different, >>> >>> Do you see any other potential issues with this solution? >> >> The idea behind the DECL version of the .DEFERRED_INIT semantics was >> that .DEFERRED_INIT just returns a SIZE-byte value that the caller >> then assigns to a SIZE-byte lhs (with the caller choosing the lhs). >> .DEFEREED_INIT itself doesn't read or write memory and so can be const, >> which in turn allows alias analysis to be more precise. > Yes. That’s right. > >> >> If we want to handle the VLA case using pointers instead then I think >> that needs to be a different IFN. >> >> If we did handle the VLA case using pointers (not expressing an opinion >> on that), then it would be the caller's job to allocate the VLA and work >> out the address of the VLA; > > the current routine “gimplify_vla_decl” has done this already: > > It created a temporary variable for the address of the VLA, and created a > call to “alloca” to allocate the VLA. Right, that's what I mean. It's this alloca that allocates the VLA and determines its address. This address is therefore logically an input rather than an output to the following zero/pattern initialisation. In C you wouldn't write: addr = alloca(size); addr = initialise(size); to allocate and initialise a size-byte buffer, because initialise() would need to know the address of the memory it's supposed to initialise. The same is true for this gimple code. > My -ftrivial-auto-var-init work just try to use the “address variable of the > VLA” in the new .DEFERRED_INIT call to carry it to RTL expansion phase. > > >> this isn't something that .DEFERRED_INIT >> would work out on the caller's behalf. The address of the VLA should >> therefore be an argument to the new IFN, rather than something that >> the IFN returns. > > Then what’s the LHS of this call? Currently the major issue is the LHS is > invalid gimple. For this (different, address-taking, VLA-only) IFN, there would be no lhs. The IFN would be similar to a memset. Like I say, this is all hypothetical, based on “if we did handle the VLA case using pointers”. As discussed, it would make alias analysis less precise. I was just answering the question about whether there were potential issues. Thanks, Richard
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao wrote:
>I modified the routine “gimple_add_init_for_auto_var” as the following:
>
>/* Generate initialization to automatic variable DECL based on INIT_TYPE.
> Build a call to internal const function DEFERRED_INIT:
> 1st argument: SIZE of the DECL;
> 2nd argument: INIT_TYPE;
> 3rd argument: IS_VLA, 0 NO, 1 YES;
>
> as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
>static void
>gimple_add_init_for_auto_var (tree decl,
> enum auto_init_type init_type,
> bool is_vla,
> gimple_seq *seq_p)
>{
> gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>
> tree init_type_node
>= build_int_cst (integer_type_node, (int) init_type);
> tree is_vla_node
>= build_int_cst (integer_type_node, (int) is_vla);
>
> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION,
> IFN_DEFERRED_INIT,
>TREE_TYPE (decl), 3,
>decl_size, init_type_node,
>is_vla_node);
>
> /* If this DECL is a VLA, a temporary address variable for it has been
> created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
> we should use it as the LHS of the call. */
>
> tree lhs_call
>= is_vla ? DECL_VALUE_EXPR (decl) : decl;
> gimplify_assign (lhs_call, call, seq_p);
>}
>
>With this change, the current issue is resolved, the gimple dump now is:
>
> (*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
>
>However, there is another new issue:
>
>For the following testing case:
>
>==
>[opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
>int bar;
>
>extern void decode_reloc(int *);
>
>void testfunc()
>{
> int alt_reloc;
>
> decode_reloc(_reloc);
>
> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>bar = 42;
>}
>=
>
>In the above, the auto var “alt_reloc” is address taken, then the gimple dump
>for it when compiled with -ftrivial-auto-var-init=zero is:
>
>void testfunc ()
>{
> int alt_reloc;
>
> try
>{
> _1 = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = _1;
> decode_reloc (_reloc);
> alt_reloc.0_2 = alt_reloc;
> if (alt_reloc.0_2 != 0) goto ; else goto ;
> :
> bar = 42;
> :
>}
> finally
>{
> alt_reloc = {CLOBBER};
>}
>}
>
>I.e, instead of the expected IR:
>
>alt_reloc = .DEFERRED_INIT (4, 2, 0);
>
>We got the following:
>
> _1 = .DEFERRED_INIT (4, 2, 0);
> alt_reloc = _1;
>
>I guess the temp “_1” is created because “alt_reloc” is address taken.
Yes and no. The reason is that alt_reloc is memory (because it is address
taken) and that GIMPLE says that register typed stores need to use a
is_gimple_val RHS which the call is not.
>My questions:
>
>Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is
>address taken?
I think so. Note it doesn't necessarily need address taking but any other
reason that prevents SSA rewriting the variable suffices.
The only other option is to force. DEFERED_INIT making the LHS address taken
which I think could be achieved by passing it the address as argument instead
of having a LHS. But let's not go down this route - it will have quite bad
behavior on alias analysis and optimization.
>If so, “uninitialized analysis” phase need to be further adjusted to specially
>handle such IR.
>
>If not, what should we do when the auto var is address taken?
>
>Thanks a lot.
>
>Qing
>
>
>> On Aug 11, 2021, at 8:58 AM, Richard Biener wrote:
>>
>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>
>>>
>>>
On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>
>> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>>
>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>
>>>
>>>
On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
wrote:
Hi, Richard,
> On Aug 10, 2021, at 10:22 AM, Richard Biener
> wrote:
>>>
>>> Especially in the VLA case but likely also in general (though
>>> unlikely
>>> since usually the receiver of initializations are simple enough).
>>> I'd
>>> expect the VLA case end up as
>>>
>>> *ptr_to_decl = .DEFERRED_INIT (...);
>>>
>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>
>> So, for the following small testing case:
>>
>>
>> extern void bar (int);
>>
>> void foo(int n)
>> {
>> int arr[n];
>> bar (arr[2]);
>> return;
>> }
>> =
>>
>> If I compile it with -ftrivial-auto-var-init=zero
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
I modified the routine “gimple_add_init_for_auto_var” as the following:
/* Generate initialization to automatic variable DECL based on INIT_TYPE.
Build a call to internal const function DEFERRED_INIT:
1st argument: SIZE of the DECL;
2nd argument: INIT_TYPE;
3rd argument: IS_VLA, 0 NO, 1 YES;
as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA). */
static void
gimple_add_init_for_auto_var (tree decl,
enum auto_init_type init_type,
bool is_vla,
gimple_seq *seq_p)
{
gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));
tree init_type_node
= build_int_cst (integer_type_node, (int) init_type);
tree is_vla_node
= build_int_cst (integer_type_node, (int) is_vla);
tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFERRED_INIT,
TREE_TYPE (decl), 3,
decl_size, init_type_node,
is_vla_node);
/* If this DECL is a VLA, a temporary address variable for it has been
created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
we should use it as the LHS of the call. */
tree lhs_call
= is_vla ? DECL_VALUE_EXPR (decl) : decl;
gimplify_assign (lhs_call, call, seq_p);
}
With this change, the current issue is resolved, the gimple dump now is:
(*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);
However, there is another new issue:
For the following testing case:
==
[opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
int bar;
extern void decode_reloc(int *);
void testfunc()
{
int alt_reloc;
decode_reloc(_reloc);
if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
bar = 42;
}
=
In the above, the auto var “alt_reloc” is address taken, then the gimple dump
for it when compiled with -ftrivial-auto-var-init=zero is:
void testfunc ()
{
int alt_reloc;
try
{
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
decode_reloc (_reloc);
alt_reloc.0_2 = alt_reloc;
if (alt_reloc.0_2 != 0) goto ; else goto ;
:
bar = 42;
:
}
finally
{
alt_reloc = {CLOBBER};
}
}
I.e, instead of the expected IR:
alt_reloc = .DEFERRED_INIT (4, 2, 0);
We got the following:
_1 = .DEFERRED_INIT (4, 2, 0);
alt_reloc = _1;
I guess the temp “_1” is created because “alt_reloc” is address taken.
My questions:
Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is address
taken?
If so, “uninitialized analysis” phase need to be further adjusted to specially
handle such IR.
If not, what should we do when the auto var is address taken?
Thanks a lot.
Qing
> On Aug 11, 2021, at 8:58 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
>>>
>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>
> On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>> Hi, Richard,
>>>
On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>>
>> Especially in the VLA case but likely also in general (though
>> unlikely
>> since usually the receiver of initializations are simple enough).
>> I'd
>> expect the VLA case end up as
>>
>> *ptr_to_decl = .DEFERRED_INIT (...);
>>
>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>
> So, for the following small testing case:
>
>
> extern void bar (int);
>
> void foo(int n)
> {
> int arr[n];
> bar (arr[2]);
> return;
> }
> =
>
> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple
> -S -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>
> =
> void foo (int n)
> {
> int n.0;
> sizetype D.1950;
> bitsizetype D.1951;
> sizetype D.1952;
> bitsizetype D.1953;
> sizetype D.1954;
> int[0:D.1950] * arr.1;
> void * saved_stack.2;
> int arr[0:D.1950] [value-expr: *arr.1];
>
> saved_stack.2 = __builtin_stack_save ();
> try
> {
> n.0 = n;
> _1 = (long int) n.0;
> _2 = _1 + -1;
> _3 = (sizetype) _2;
> D.1950 = _3;
> _4 = (sizetype) n.0;
> _5 = (bitsizetype) _4;
> _6 = _5 * 32;
> D.1951 = _6;
> _7 = (sizetype) n.0;
> _8 = _7 * 4;
> D.1952 = _8;
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 8:58 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
>>>
>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>
> On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>> Hi, Richard,
>>>
On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>>
>> Especially in the VLA case but likely also in general (though
>> unlikely
>> since usually the receiver of initializations are simple enough).
>> I'd
>> expect the VLA case end up as
>>
>> *ptr_to_decl = .DEFERRED_INIT (...);
>>
>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>
> So, for the following small testing case:
>
>
> extern void bar (int);
>
> void foo(int n)
> {
> int arr[n];
> bar (arr[2]);
> return;
> }
> =
>
> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple
> -S -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>
> =
> void foo (int n)
> {
> int n.0;
> sizetype D.1950;
> bitsizetype D.1951;
> sizetype D.1952;
> bitsizetype D.1953;
> sizetype D.1954;
> int[0:D.1950] * arr.1;
> void * saved_stack.2;
> int arr[0:D.1950] [value-expr: *arr.1];
>
> saved_stack.2 = __builtin_stack_save ();
> try
> {
> n.0 = n;
> _1 = (long int) n.0;
> _2 = _1 + -1;
> _3 = (sizetype) _2;
> D.1950 = _3;
> _4 = (sizetype) n.0;
> _5 = (bitsizetype) _4;
> _6 = _5 * 32;
> D.1951 = _6;
> _7 = (sizetype) n.0;
> _8 = _7 * 4;
> D.1952 = _8;
> _9 = (sizetype) n.0;
> _10 = (bitsizetype) _9;
> _11 = _10 * 32;
> D.1953 = _11;
> _12 = (sizetype) n.0;
> _13 = _12 * 4;
> D.1954 = _13;
> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> arr = .DEFERRED_INIT (D.1952, 2, 1);
> _14 = (*arr.1)[2];
> bar (_14);
> return;
> }
> finally
> {
> __builtin_stack_restore (saved_stack.2);
> }
> }
>
>
>
> You think that the above .DEFEERED_INIT is not correct?
> It should be:
>
> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>
> ?
Yes.
>>>
>>> I updated gimplify.c for VLA and now it emits the call to
>>> .DEFERRED_INIT as:
>>>
>>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> However, this call triggered the assertion failure in
>>> verify_gimple_call of tree-cfg.c because the LHS is not a valid LHS.
>>> Then I modify tree-cfg.c as:
>>>
>>> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>>> index 330eb7dd89bf..180d4f1f9e32 100644
>>> --- a/gcc/tree-cfg.c
>>> +++ b/gcc/tree-cfg.c
>>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>>> }
>>>
>>> tree lhs = gimple_call_lhs (stmt);
>>> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
>>> + a pointer for the VLA variable, which is not a valid LHS of
>>> + a gimple call, we ignore the asssertion on this. */
>>> if (lhs
>>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>>>&& (!is_gimple_reg (lhs)
>>> && (!is_gimple_lvalue (lhs)
>>> || verify_types_in_gimple_reference
>>>
>>> The assertion failure in tree-cfg.c got resolved, but I got another
>>> assertion failure in operands_scanner::get_expr_operands (tree *expr_p,
>>> int flags), line 945:
>>>
>>> 939 /* If we get here, something has gone wrong. */
>>> 940 if (flag_checking)
>>> 941 {
>>> 942 fprintf (stderr, "unhandled expression in
>>> get_expr_operands():\n");
>>> 943 debug_tree (expr);
>>> 944 fputs ("\n", stderr);
>>> 945 gcc_unreachable ();
>>> 946 }
>>>
>>> Looks like that the gimple statement:
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>>>
>>> How to resolve this issue?
>
> It sounds like the LHS is an INDIRECT_REF maybe? That means it's
> still not properly gimplified because it should end up as a MEM_REF
> instead.
>
> But I'm just guessing here ... if you are
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
> >
> > On Wed, 11 Aug 2021, Qing Zhao wrote:
> >
> >>
> >>
> >>> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
> >>>
> >>> On Tue, 10 Aug 2021, Qing Zhao wrote:
> >>>
>
>
> > On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
> > wrote:
> >
> > Hi, Richard,
> >
> >> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>
> Especially in the VLA case but likely also in general (though
> unlikely
> since usually the receiver of initializations are simple enough).
> I'd
> expect the VLA case end up as
>
> *ptr_to_decl = .DEFERRED_INIT (...);
>
> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
> >>>
> >>> So, for the following small testing case:
> >>>
> >>>
> >>> extern void bar (int);
> >>>
> >>> void foo(int n)
> >>> {
> >>> int arr[n];
> >>> bar (arr[2]);
> >>> return;
> >>> }
> >>> =
> >>>
> >>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple
> >>> -S -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
> >>>
> >>> =
> >>> void foo (int n)
> >>> {
> >>> int n.0;
> >>> sizetype D.1950;
> >>> bitsizetype D.1951;
> >>> sizetype D.1952;
> >>> bitsizetype D.1953;
> >>> sizetype D.1954;
> >>> int[0:D.1950] * arr.1;
> >>> void * saved_stack.2;
> >>> int arr[0:D.1950] [value-expr: *arr.1];
> >>>
> >>> saved_stack.2 = __builtin_stack_save ();
> >>> try
> >>> {
> >>> n.0 = n;
> >>> _1 = (long int) n.0;
> >>> _2 = _1 + -1;
> >>> _3 = (sizetype) _2;
> >>> D.1950 = _3;
> >>> _4 = (sizetype) n.0;
> >>> _5 = (bitsizetype) _4;
> >>> _6 = _5 * 32;
> >>> D.1951 = _6;
> >>> _7 = (sizetype) n.0;
> >>> _8 = _7 * 4;
> >>> D.1952 = _8;
> >>> _9 = (sizetype) n.0;
> >>> _10 = (bitsizetype) _9;
> >>> _11 = _10 * 32;
> >>> D.1953 = _11;
> >>> _12 = (sizetype) n.0;
> >>> _13 = _12 * 4;
> >>> D.1954 = _13;
> >>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> >>> arr = .DEFERRED_INIT (D.1952, 2, 1);
> >>> _14 = (*arr.1)[2];
> >>> bar (_14);
> >>> return;
> >>> }
> >>> finally
> >>> {
> >>> __builtin_stack_restore (saved_stack.2);
> >>> }
> >>> }
> >>>
> >>>
> >>>
> >>> You think that the above .DEFEERED_INIT is not correct?
> >>> It should be:
> >>>
> >>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
> >>>
> >>> ?
> >>
> >> Yes.
> >>
> >
> > I updated gimplify.c for VLA and now it emits the call to
> > .DEFERRED_INIT as:
> >
> >arr.1 = __builtin_alloca_with_align (D.1954, 32);
> >*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >
> > However, this call triggered the assertion failure in
> > verify_gimple_call of tree-cfg.c because the LHS is not a valid LHS.
> > Then I modify tree-cfg.c as:
> >
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 330eb7dd89bf..180d4f1f9e32 100644
> > --- a/gcc/tree-cfg.c
> > +++ b/gcc/tree-cfg.c
> > @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
> >}
> >
> > tree lhs = gimple_call_lhs (stmt);
> > + /* For .DEFERRED_INIT call, the LHS might be an indirection of
> > + a pointer for the VLA variable, which is not a valid LHS of
> > + a gimple call, we ignore the asssertion on this. */
> > if (lhs
> > + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> > && (!is_gimple_reg (lhs)
> >&& (!is_gimple_lvalue (lhs)
> >|| verify_types_in_gimple_reference
> >
> > The assertion failure in tree-cfg.c got resolved, but I got another
> > assertion failure in operands_scanner::get_expr_operands (tree *expr_p,
> > int flags), line 945:
> >
> > 939 /* If we get here, something has gone wrong. */
> > 940 if (flag_checking)
> > 941 {
> > 942 fprintf (stderr, "unhandled expression in
> > get_expr_operands():\n");
> > 943 debug_tree (expr);
> > 944 fputs ("\n", stderr);
> > 945 gcc_unreachable ();
> > 946 }
> >
> > Looks like that the gimple statement:
> > *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >
> > Is not valid. i.e, the LHS should not be an indirection to a pointer.
> >
> > How to resolve this issue?
> >>>
> >>> It sounds like the LHS is an INDIRECT_REF maybe? That means it's
> >>> still not properly gimplified because it should end up as a MEM_REF
> >>> instead.
> >>>
> >>> But I'm just guessing here ... if you are in a debugger then you can
> >>>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 8:37 AM, Richard Biener wrote:
>
> On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>>>
>>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>>
> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
> wrote:
>
> Hi, Richard,
>
>> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
Especially in the VLA case but likely also in general (though unlikely
since usually the receiver of initializations are simple enough). I'd
expect the VLA case end up as
*ptr_to_decl = .DEFERRED_INIT (...);
where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>>
>>> So, for the following small testing case:
>>>
>>>
>>> extern void bar (int);
>>>
>>> void foo(int n)
>>> {
>>> int arr[n];
>>> bar (arr[2]);
>>> return;
>>> }
>>> =
>>>
>>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S
>>> -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>>>
>>> =
>>> void foo (int n)
>>> {
>>> int n.0;
>>> sizetype D.1950;
>>> bitsizetype D.1951;
>>> sizetype D.1952;
>>> bitsizetype D.1953;
>>> sizetype D.1954;
>>> int[0:D.1950] * arr.1;
>>> void * saved_stack.2;
>>> int arr[0:D.1950] [value-expr: *arr.1];
>>>
>>> saved_stack.2 = __builtin_stack_save ();
>>> try
>>> {
>>> n.0 = n;
>>> _1 = (long int) n.0;
>>> _2 = _1 + -1;
>>> _3 = (sizetype) _2;
>>> D.1950 = _3;
>>> _4 = (sizetype) n.0;
>>> _5 = (bitsizetype) _4;
>>> _6 = _5 * 32;
>>> D.1951 = _6;
>>> _7 = (sizetype) n.0;
>>> _8 = _7 * 4;
>>> D.1952 = _8;
>>> _9 = (sizetype) n.0;
>>> _10 = (bitsizetype) _9;
>>> _11 = _10 * 32;
>>> D.1953 = _11;
>>> _12 = (sizetype) n.0;
>>> _13 = _12 * 4;
>>> D.1954 = _13;
>>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>> arr = .DEFERRED_INIT (D.1952, 2, 1);
>>> _14 = (*arr.1)[2];
>>> bar (_14);
>>> return;
>>> }
>>> finally
>>> {
>>> __builtin_stack_restore (saved_stack.2);
>>> }
>>> }
>>>
>>>
>>>
>>> You think that the above .DEFEERED_INIT is not correct?
>>> It should be:
>>>
>>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>>>
>>> ?
>>
>> Yes.
>>
>
> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT
> as:
>
>arr.1 = __builtin_alloca_with_align (D.1954, 32);
>*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>
> However, this call triggered the assertion failure in verify_gimple_call
> of tree-cfg.c because the LHS is not a valid LHS.
> Then I modify tree-cfg.c as:
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 330eb7dd89bf..180d4f1f9e32 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>}
>
> tree lhs = gimple_call_lhs (stmt);
> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
> + a pointer for the VLA variable, which is not a valid LHS of
> + a gimple call, we ignore the asssertion on this. */
> if (lhs
> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> && (!is_gimple_reg (lhs)
>&& (!is_gimple_lvalue (lhs)
>|| verify_types_in_gimple_reference
>
> The assertion failure in tree-cfg.c got resolved, but I got another
> assertion failure in operands_scanner::get_expr_operands (tree *expr_p,
> int flags), line 945:
>
> 939 /* If we get here, something has gone wrong. */
> 940 if (flag_checking)
> 941 {
> 942 fprintf (stderr, "unhandled expression in
> get_expr_operands():\n");
> 943 debug_tree (expr);
> 944 fputs ("\n", stderr);
> 945 gcc_unreachable ();
> 946 }
>
> Looks like that the gimple statement:
> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>
> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>
> How to resolve this issue?
>>>
>>> It sounds like the LHS is an INDIRECT_REF maybe? That means it's
>>> still not properly gimplified because it should end up as a MEM_REF
>>> instead.
>>>
>>> But I'm just guessing here ... if you are in a debugger then you can
>>> invoke debug_tree (lhs) in the inferior to see what it exactly is
>>> at the point of the failure.
>>
>> Yes, it’s an INDIRECT_REF at the point of the failure even though I added a
>>
>> gimplify_var_or_parm_decl (lhs)
>
> I think the easiest is to build the .DEFERRED_INIT as GENERIC
> and use gimplify_assign () to gimplify and add the result
> to the sequence. Thus,
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 4:02 AM, Richard Sandiford
> wrote:
>
> Qing Zhao via Gcc-patches writes:
>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>> Hi, Richard,
>>>
On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>>
>> Especially in the VLA case but likely also in general (though unlikely
>> since usually the receiver of initializations are simple enough). I'd
>> expect the VLA case end up as
>>
>> *ptr_to_decl = .DEFERRED_INIT (...);
>>
>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>
> So, for the following small testing case:
>
>
> extern void bar (int);
>
> void foo(int n)
> {
> int arr[n];
> bar (arr[2]);
> return;
> }
> =
>
> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S
> -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>
> =
> void foo (int n)
> {
> int n.0;
> sizetype D.1950;
> bitsizetype D.1951;
> sizetype D.1952;
> bitsizetype D.1953;
> sizetype D.1954;
> int[0:D.1950] * arr.1;
> void * saved_stack.2;
> int arr[0:D.1950] [value-expr: *arr.1];
>
> saved_stack.2 = __builtin_stack_save ();
> try
> {
>n.0 = n;
>_1 = (long int) n.0;
>_2 = _1 + -1;
>_3 = (sizetype) _2;
>D.1950 = _3;
>_4 = (sizetype) n.0;
>_5 = (bitsizetype) _4;
>_6 = _5 * 32;
>D.1951 = _6;
>_7 = (sizetype) n.0;
>_8 = _7 * 4;
>D.1952 = _8;
>_9 = (sizetype) n.0;
>_10 = (bitsizetype) _9;
>_11 = _10 * 32;
>D.1953 = _11;
>_12 = (sizetype) n.0;
>_13 = _12 * 4;
>D.1954 = _13;
>arr.1 = __builtin_alloca_with_align (D.1954, 32);
>arr = .DEFERRED_INIT (D.1952, 2, 1);
>_14 = (*arr.1)[2];
>bar (_14);
>return;
> }
> finally
> {
>__builtin_stack_restore (saved_stack.2);
> }
> }
>
>
>
> You think that the above .DEFEERED_INIT is not correct?
> It should be:
>
> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>
> ?
Yes.
>>>
>>> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
>>>
>>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> However, this call triggered the assertion failure in verify_gimple_call of
>>> tree-cfg.c because the LHS is not a valid LHS.
>>> Then I modify tree-cfg.c as:
>>>
>>> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>>> index 330eb7dd89bf..180d4f1f9e32 100644
>>> --- a/gcc/tree-cfg.c
>>> +++ b/gcc/tree-cfg.c
>>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>>> }
>>>
>>> tree lhs = gimple_call_lhs (stmt);
>>> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
>>> + a pointer for the VLA variable, which is not a valid LHS of
>>> + a gimple call, we ignore the asssertion on this. */
>>> if (lhs
>>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>>> && (!is_gimple_reg (lhs)
>>> && (!is_gimple_lvalue (lhs)
>>> || verify_types_in_gimple_reference
>>>
>>> The assertion failure in tree-cfg.c got resolved, but I got another
>>> assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int
>>> flags), line 945:
>>>
>>> 939 /* If we get here, something has gone wrong. */
>>> 940 if (flag_checking)
>>> 941 {
>>> 942 fprintf (stderr, "unhandled expression in
>>> get_expr_operands():\n");
>>> 943 debug_tree (expr);
>>> 944 fputs ("\n", stderr);
>>> 945 gcc_unreachable ();
>>> 946 }
>>>
>>> Looks like that the gimple statement:
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>>>
>>> How to resolve this issue?
>>
>> I came up with the following solution:
>>
>> Define the IFN_DEFERRED_INIT function as:
>>
>> LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
>>
>> if IS_VLA is false, the LHS is the DECL itself,
>> if IS_VLA is true, the LHS is the pointer to this DECL that created by
>> gimplify_vla_decl.
>>
>>
>> The benefit of this solution are:
>>
>> 1. Resolved the invalid IR issue;
>> 2. The call stmt carries the address of the VLA natually;
>>
>> The issue with this solution is:
>>
>> For VLA and non-VLA, the LHS will be different,
>>
>> Do you see any other potential issues with this solution?
>
> The idea behind the DECL version of the .DEFERRED_INIT semantics was
> that .DEFERRED_INIT just returns a SIZE-byte value that the caller
> then assigns to a SIZE-byte lhs (with the caller choosing the lhs).
> .DEFEREED_INIT itself doesn't read or write memory and so can be const,
> which in turn allows alias analysis to be more precise.
Yes. That’s right.
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Wed, 11 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
> >
> > On Tue, 10 Aug 2021, Qing Zhao wrote:
> >
> >>
> >>
> >>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
> >>> wrote:
> >>>
> >>> Hi, Richard,
> >>>
> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
> >>
> >> Especially in the VLA case but likely also in general (though unlikely
> >> since usually the receiver of initializations are simple enough). I'd
> >> expect the VLA case end up as
> >>
> >> *ptr_to_decl = .DEFERRED_INIT (...);
> >>
> >> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
> >
> > So, for the following small testing case:
> >
> >
> > extern void bar (int);
> >
> > void foo(int n)
> > {
> > int arr[n];
> > bar (arr[2]);
> > return;
> > }
> > =
> >
> > If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S
> > -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
> >
> > =
> > void foo (int n)
> > {
> > int n.0;
> > sizetype D.1950;
> > bitsizetype D.1951;
> > sizetype D.1952;
> > bitsizetype D.1953;
> > sizetype D.1954;
> > int[0:D.1950] * arr.1;
> > void * saved_stack.2;
> > int arr[0:D.1950] [value-expr: *arr.1];
> >
> > saved_stack.2 = __builtin_stack_save ();
> > try
> > {
> >n.0 = n;
> >_1 = (long int) n.0;
> >_2 = _1 + -1;
> >_3 = (sizetype) _2;
> >D.1950 = _3;
> >_4 = (sizetype) n.0;
> >_5 = (bitsizetype) _4;
> >_6 = _5 * 32;
> >D.1951 = _6;
> >_7 = (sizetype) n.0;
> >_8 = _7 * 4;
> >D.1952 = _8;
> >_9 = (sizetype) n.0;
> >_10 = (bitsizetype) _9;
> >_11 = _10 * 32;
> >D.1953 = _11;
> >_12 = (sizetype) n.0;
> >_13 = _12 * 4;
> >D.1954 = _13;
> >arr.1 = __builtin_alloca_with_align (D.1954, 32);
> >arr = .DEFERRED_INIT (D.1952, 2, 1);
> >_14 = (*arr.1)[2];
> >bar (_14);
> >return;
> > }
> > finally
> > {
> >__builtin_stack_restore (saved_stack.2);
> > }
> > }
> >
> >
> >
> > You think that the above .DEFEERED_INIT is not correct?
> > It should be:
> >
> > *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
> >
> > ?
>
> Yes.
>
> >>>
> >>> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT
> >>> as:
> >>>
> >>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> >>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >>>
> >>> However, this call triggered the assertion failure in verify_gimple_call
> >>> of tree-cfg.c because the LHS is not a valid LHS.
> >>> Then I modify tree-cfg.c as:
> >>>
> >>> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> >>> index 330eb7dd89bf..180d4f1f9e32 100644
> >>> --- a/gcc/tree-cfg.c
> >>> +++ b/gcc/tree-cfg.c
> >>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
> >>> }
> >>>
> >>> tree lhs = gimple_call_lhs (stmt);
> >>> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
> >>> + a pointer for the VLA variable, which is not a valid LHS of
> >>> + a gimple call, we ignore the asssertion on this. */
> >>> if (lhs
> >>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> >>> && (!is_gimple_reg (lhs)
> >>> && (!is_gimple_lvalue (lhs)
> >>> || verify_types_in_gimple_reference
> >>>
> >>> The assertion failure in tree-cfg.c got resolved, but I got another
> >>> assertion failure in operands_scanner::get_expr_operands (tree *expr_p,
> >>> int flags), line 945:
> >>>
> >>> 939 /* If we get here, something has gone wrong. */
> >>> 940 if (flag_checking)
> >>> 941 {
> >>> 942 fprintf (stderr, "unhandled expression in
> >>> get_expr_operands():\n");
> >>> 943 debug_tree (expr);
> >>> 944 fputs ("\n", stderr);
> >>> 945 gcc_unreachable ();
> >>> 946 }
> >>>
> >>> Looks like that the gimple statement:
> >>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >>>
> >>> Is not valid. i.e, the LHS should not be an indirection to a pointer.
> >>>
> >>> How to resolve this issue?
> >
> > It sounds like the LHS is an INDIRECT_REF maybe? That means it's
> > still not properly gimplified because it should end up as a MEM_REF
> > instead.
> >
> > But I'm just guessing here ... if you are in a debugger then you can
> > invoke debug_tree (lhs) in the inferior to see what it exactly is
> > at the point of the failure.
>
> Yes, it’s an INDIRECT_REF at the point of the failure even though I added a
>
> gimplify_var_or_parm_decl (lhs)
I think the easiest is to build the .DEFERRED_INIT as GENERIC
and use gimplify_assign () to gimplify and add the result
to the sequence. Thus, build a GENERIC CALL_EXPR and then
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 11, 2021, at 2:02 AM, Richard Biener wrote:
>
> On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>>
>>
>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
>>> wrote:
>>>
>>> Hi, Richard,
>>>
On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>>
>> Especially in the VLA case but likely also in general (though unlikely
>> since usually the receiver of initializations are simple enough). I'd
>> expect the VLA case end up as
>>
>> *ptr_to_decl = .DEFERRED_INIT (...);
>>
>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>
> So, for the following small testing case:
>
>
> extern void bar (int);
>
> void foo(int n)
> {
> int arr[n];
> bar (arr[2]);
> return;
> }
> =
>
> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S
> -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>
> =
> void foo (int n)
> {
> int n.0;
> sizetype D.1950;
> bitsizetype D.1951;
> sizetype D.1952;
> bitsizetype D.1953;
> sizetype D.1954;
> int[0:D.1950] * arr.1;
> void * saved_stack.2;
> int arr[0:D.1950] [value-expr: *arr.1];
>
> saved_stack.2 = __builtin_stack_save ();
> try
> {
>n.0 = n;
>_1 = (long int) n.0;
>_2 = _1 + -1;
>_3 = (sizetype) _2;
>D.1950 = _3;
>_4 = (sizetype) n.0;
>_5 = (bitsizetype) _4;
>_6 = _5 * 32;
>D.1951 = _6;
>_7 = (sizetype) n.0;
>_8 = _7 * 4;
>D.1952 = _8;
>_9 = (sizetype) n.0;
>_10 = (bitsizetype) _9;
>_11 = _10 * 32;
>D.1953 = _11;
>_12 = (sizetype) n.0;
>_13 = _12 * 4;
>D.1954 = _13;
>arr.1 = __builtin_alloca_with_align (D.1954, 32);
>arr = .DEFERRED_INIT (D.1952, 2, 1);
>_14 = (*arr.1)[2];
>bar (_14);
>return;
> }
> finally
> {
>__builtin_stack_restore (saved_stack.2);
> }
> }
>
>
>
> You think that the above .DEFEERED_INIT is not correct?
> It should be:
>
> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>
> ?
Yes.
>>>
>>> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
>>>
>>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> However, this call triggered the assertion failure in verify_gimple_call of
>>> tree-cfg.c because the LHS is not a valid LHS.
>>> Then I modify tree-cfg.c as:
>>>
>>> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>>> index 330eb7dd89bf..180d4f1f9e32 100644
>>> --- a/gcc/tree-cfg.c
>>> +++ b/gcc/tree-cfg.c
>>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>>> }
>>>
>>> tree lhs = gimple_call_lhs (stmt);
>>> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
>>> + a pointer for the VLA variable, which is not a valid LHS of
>>> + a gimple call, we ignore the asssertion on this. */
>>> if (lhs
>>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>>> && (!is_gimple_reg (lhs)
>>> && (!is_gimple_lvalue (lhs)
>>> || verify_types_in_gimple_reference
>>>
>>> The assertion failure in tree-cfg.c got resolved, but I got another
>>> assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int
>>> flags), line 945:
>>>
>>> 939 /* If we get here, something has gone wrong. */
>>> 940 if (flag_checking)
>>> 941 {
>>> 942 fprintf (stderr, "unhandled expression in
>>> get_expr_operands():\n");
>>> 943 debug_tree (expr);
>>> 944 fputs ("\n", stderr);
>>> 945 gcc_unreachable ();
>>> 946 }
>>>
>>> Looks like that the gimple statement:
>>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>>
>>> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>>>
>>> How to resolve this issue?
>
> It sounds like the LHS is an INDIRECT_REF maybe? That means it's
> still not properly gimplified because it should end up as a MEM_REF
> instead.
>
> But I'm just guessing here ... if you are in a debugger then you can
> invoke debug_tree (lhs) in the inferior to see what it exactly is
> at the point of the failure.
Yes, it’s an INDIRECT_REF at the point of the failure even though I added a
gimplify_var_or_parm_decl (lhs)
Qing
>
>> I came up with the following solution:
>>
>> Define the IFN_DEFERRED_INIT function as:
>>
>> LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
>>
>> if IS_VLA is false, the LHS is the DECL itself,
>> if IS_VLA is true, the LHS is the pointer to this DECL that created by
>> gimplify_vla_decl.
>>
>>
>> The benefit of this solution are:
>>
>> 1. Resolved the invalid IR issue;
>> 2. The call stmt carries the address of the VLA natually;
>>
>> The issue with this solution is:
>>
>> For VLA and non-VLA, the
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Qing Zhao via Gcc-patches writes:
>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
>> wrote:
>>
>> Hi, Richard,
>>
>>> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>
> Especially in the VLA case but likely also in general (though unlikely
> since usually the receiver of initializations are simple enough). I'd
> expect the VLA case end up as
>
> *ptr_to_decl = .DEFERRED_INIT (...);
>
> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
So, for the following small testing case:
extern void bar (int);
void foo(int n)
{
int arr[n];
bar (arr[2]);
return;
}
=
If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
=
void foo (int n)
{
int n.0;
sizetype D.1950;
bitsizetype D.1951;
sizetype D.1952;
bitsizetype D.1953;
sizetype D.1954;
int[0:D.1950] * arr.1;
void * saved_stack.2;
int arr[0:D.1950] [value-expr: *arr.1];
saved_stack.2 = __builtin_stack_save ();
try
{
n.0 = n;
_1 = (long int) n.0;
_2 = _1 + -1;
_3 = (sizetype) _2;
D.1950 = _3;
_4 = (sizetype) n.0;
_5 = (bitsizetype) _4;
_6 = _5 * 32;
D.1951 = _6;
_7 = (sizetype) n.0;
_8 = _7 * 4;
D.1952 = _8;
_9 = (sizetype) n.0;
_10 = (bitsizetype) _9;
_11 = _10 * 32;
D.1953 = _11;
_12 = (sizetype) n.0;
_13 = _12 * 4;
D.1954 = _13;
arr.1 = __builtin_alloca_with_align (D.1954, 32);
arr = .DEFERRED_INIT (D.1952, 2, 1);
_14 = (*arr.1)[2];
bar (_14);
return;
}
finally
{
__builtin_stack_restore (saved_stack.2);
}
}
You think that the above .DEFEERED_INIT is not correct?
It should be:
*arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
?
>>>
>>> Yes.
>>>
>>
>> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
>>
>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>
>> However, this call triggered the assertion failure in verify_gimple_call of
>> tree-cfg.c because the LHS is not a valid LHS.
>> Then I modify tree-cfg.c as:
>>
>> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>> index 330eb7dd89bf..180d4f1f9e32 100644
>> --- a/gcc/tree-cfg.c
>> +++ b/gcc/tree-cfg.c
>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>> }
>>
>> tree lhs = gimple_call_lhs (stmt);
>> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
>> + a pointer for the VLA variable, which is not a valid LHS of
>> + a gimple call, we ignore the asssertion on this. */
>> if (lhs
>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>> && (!is_gimple_reg (lhs)
>> && (!is_gimple_lvalue (lhs)
>> || verify_types_in_gimple_reference
>>
>> The assertion failure in tree-cfg.c got resolved, but I got another
>> assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int
>> flags), line 945:
>>
>> 939 /* If we get here, something has gone wrong. */
>> 940 if (flag_checking)
>> 941 {
>> 942 fprintf (stderr, "unhandled expression in get_expr_operands():\n");
>> 943 debug_tree (expr);
>> 944 fputs ("\n", stderr);
>> 945 gcc_unreachable ();
>> 946 }
>>
>> Looks like that the gimple statement:
>>*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>>
>> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>>
>> How to resolve this issue?
>
> I came up with the following solution:
>
> Define the IFN_DEFERRED_INIT function as:
>
>LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
>
>if IS_VLA is false, the LHS is the DECL itself,
>if IS_VLA is true, the LHS is the pointer to this DECL that created by
>gimplify_vla_decl.
>
>
> The benefit of this solution are:
>
> 1. Resolved the invalid IR issue;
> 2. The call stmt carries the address of the VLA natually;
>
> The issue with this solution is:
>
> For VLA and non-VLA, the LHS will be different,
>
> Do you see any other potential issues with this solution?
The idea behind the DECL version of the .DEFERRED_INIT semantics was
that .DEFERRED_INIT just returns a SIZE-byte value that the caller
then assigns to a SIZE-byte lhs (with the caller choosing the lhs).
.DEFEREED_INIT itself doesn't read or write memory and so can be const,
which in turn allows alias analysis to be more precise.
If we want to handle the VLA case using pointers instead then I think
that needs to be a different IFN.
If we did handle the VLA case using pointers (not expressing an opinion
on that), then it would be the caller's job to
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
> > wrote:
> >
> > Hi, Richard,
> >
> >> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>
> Especially in the VLA case but likely also in general (though unlikely
> since usually the receiver of initializations are simple enough). I'd
> expect the VLA case end up as
>
> *ptr_to_decl = .DEFERRED_INIT (...);
>
> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
> >>>
> >>> So, for the following small testing case:
> >>>
> >>>
> >>> extern void bar (int);
> >>>
> >>> void foo(int n)
> >>> {
> >>> int arr[n];
> >>> bar (arr[2]);
> >>> return;
> >>> }
> >>> =
> >>>
> >>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S
> >>> -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
> >>>
> >>> =
> >>> void foo (int n)
> >>> {
> >>> int n.0;
> >>> sizetype D.1950;
> >>> bitsizetype D.1951;
> >>> sizetype D.1952;
> >>> bitsizetype D.1953;
> >>> sizetype D.1954;
> >>> int[0:D.1950] * arr.1;
> >>> void * saved_stack.2;
> >>> int arr[0:D.1950] [value-expr: *arr.1];
> >>>
> >>> saved_stack.2 = __builtin_stack_save ();
> >>> try
> >>> {
> >>> n.0 = n;
> >>> _1 = (long int) n.0;
> >>> _2 = _1 + -1;
> >>> _3 = (sizetype) _2;
> >>> D.1950 = _3;
> >>> _4 = (sizetype) n.0;
> >>> _5 = (bitsizetype) _4;
> >>> _6 = _5 * 32;
> >>> D.1951 = _6;
> >>> _7 = (sizetype) n.0;
> >>> _8 = _7 * 4;
> >>> D.1952 = _8;
> >>> _9 = (sizetype) n.0;
> >>> _10 = (bitsizetype) _9;
> >>> _11 = _10 * 32;
> >>> D.1953 = _11;
> >>> _12 = (sizetype) n.0;
> >>> _13 = _12 * 4;
> >>> D.1954 = _13;
> >>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> >>> arr = .DEFERRED_INIT (D.1952, 2, 1);
> >>> _14 = (*arr.1)[2];
> >>> bar (_14);
> >>> return;
> >>> }
> >>> finally
> >>> {
> >>> __builtin_stack_restore (saved_stack.2);
> >>> }
> >>> }
> >>>
> >>>
> >>>
> >>> You think that the above .DEFEERED_INIT is not correct?
> >>> It should be:
> >>>
> >>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
> >>>
> >>> ?
> >>
> >> Yes.
> >>
> >
> > I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
> >
> > arr.1 = __builtin_alloca_with_align (D.1954, 32);
> > *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >
> > However, this call triggered the assertion failure in verify_gimple_call of
> > tree-cfg.c because the LHS is not a valid LHS.
> > Then I modify tree-cfg.c as:
> >
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 330eb7dd89bf..180d4f1f9e32 100644
> > --- a/gcc/tree-cfg.c
> > +++ b/gcc/tree-cfg.c
> > @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
> > }
> >
> > tree lhs = gimple_call_lhs (stmt);
> > + /* For .DEFERRED_INIT call, the LHS might be an indirection of
> > + a pointer for the VLA variable, which is not a valid LHS of
> > + a gimple call, we ignore the asssertion on this. */
> > if (lhs
> > + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> > && (!is_gimple_reg (lhs)
> > && (!is_gimple_lvalue (lhs)
> > || verify_types_in_gimple_reference
> >
> > The assertion failure in tree-cfg.c got resolved, but I got another
> > assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int
> > flags), line 945:
> >
> > 939 /* If we get here, something has gone wrong. */
> > 940 if (flag_checking)
> > 941 {
> > 942 fprintf (stderr, "unhandled expression in
> > get_expr_operands():\n");
> > 943 debug_tree (expr);
> > 944 fputs ("\n", stderr);
> > 945 gcc_unreachable ();
> > 946 }
> >
> > Looks like that the gimple statement:
> >*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> >
> > Is not valid. i.e, the LHS should not be an indirection to a pointer.
> >
> > How to resolve this issue?
It sounds like the LHS is an INDIRECT_REF maybe? That means it's
still not properly gimplified because it should end up as a MEM_REF
instead.
But I'm just guessing here ... if you are in a debugger then you can
invoke debug_tree (lhs) in the inferior to see what it exactly is
at the point of the failure.
> I came up with the following solution:
>
> Define the IFN_DEFERRED_INIT function as:
>
>LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
>
>if IS_VLA is false, the LHS is the DECL itself,
>if IS_VLA is true, the LHS is the pointer to this DECL that created by
>gimplify_vla_decl.
>
>
> The benefit of this solution are:
>
> 1. Resolved the invalid IR issue;
> 2. The call stmt carries the address of the VLA natually;
>
> The issue with this solution is:
>
> For VLA and non-VLA, the LHS will be different,
>
> Do you see any other potential issues with this solution?
>
> thanks.
>
> Qing
>
>
>
>
>
--
Richard Biener
SUSE Software Solutions Germany
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches
> wrote:
>
> Hi, Richard,
>
>> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
Especially in the VLA case but likely also in general (though unlikely
since usually the receiver of initializations are simple enough). I'd
expect the VLA case end up as
*ptr_to_decl = .DEFERRED_INIT (...);
where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>>
>>> So, for the following small testing case:
>>>
>>>
>>> extern void bar (int);
>>>
>>> void foo(int n)
>>> {
>>> int arr[n];
>>> bar (arr[2]);
>>> return;
>>> }
>>> =
>>>
>>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
>>> auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>>>
>>> =
>>> void foo (int n)
>>> {
>>> int n.0;
>>> sizetype D.1950;
>>> bitsizetype D.1951;
>>> sizetype D.1952;
>>> bitsizetype D.1953;
>>> sizetype D.1954;
>>> int[0:D.1950] * arr.1;
>>> void * saved_stack.2;
>>> int arr[0:D.1950] [value-expr: *arr.1];
>>>
>>> saved_stack.2 = __builtin_stack_save ();
>>> try
>>> {
>>> n.0 = n;
>>> _1 = (long int) n.0;
>>> _2 = _1 + -1;
>>> _3 = (sizetype) _2;
>>> D.1950 = _3;
>>> _4 = (sizetype) n.0;
>>> _5 = (bitsizetype) _4;
>>> _6 = _5 * 32;
>>> D.1951 = _6;
>>> _7 = (sizetype) n.0;
>>> _8 = _7 * 4;
>>> D.1952 = _8;
>>> _9 = (sizetype) n.0;
>>> _10 = (bitsizetype) _9;
>>> _11 = _10 * 32;
>>> D.1953 = _11;
>>> _12 = (sizetype) n.0;
>>> _13 = _12 * 4;
>>> D.1954 = _13;
>>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>> arr = .DEFERRED_INIT (D.1952, 2, 1);
>>> _14 = (*arr.1)[2];
>>> bar (_14);
>>> return;
>>> }
>>> finally
>>> {
>>> __builtin_stack_restore (saved_stack.2);
>>> }
>>> }
>>>
>>>
>>>
>>> You think that the above .DEFEERED_INIT is not correct?
>>> It should be:
>>>
>>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>>>
>>> ?
>>
>> Yes.
>>
>
> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
>
> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>
> However, this call triggered the assertion failure in verify_gimple_call of
> tree-cfg.c because the LHS is not a valid LHS.
> Then I modify tree-cfg.c as:
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 330eb7dd89bf..180d4f1f9e32 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
> }
>
> tree lhs = gimple_call_lhs (stmt);
> + /* For .DEFERRED_INIT call, the LHS might be an indirection of
> + a pointer for the VLA variable, which is not a valid LHS of
> + a gimple call, we ignore the asssertion on this. */
> if (lhs
> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> && (!is_gimple_reg (lhs)
> && (!is_gimple_lvalue (lhs)
> || verify_types_in_gimple_reference
>
> The assertion failure in tree-cfg.c got resolved, but I got another assertion
> failure in operands_scanner::get_expr_operands (tree *expr_p, int flags),
> line 945:
>
> 939 /* If we get here, something has gone wrong. */
> 940 if (flag_checking)
> 941 {
> 942 fprintf (stderr, "unhandled expression in get_expr_operands():\n");
> 943 debug_tree (expr);
> 944 fputs ("\n", stderr);
> 945 gcc_unreachable ();
> 946 }
>
> Looks like that the gimple statement:
>*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
>
> Is not valid. i.e, the LHS should not be an indirection to a pointer.
>
> How to resolve this issue?
I came up with the following solution:
Define the IFN_DEFERRED_INIT function as:
LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
if IS_VLA is false, the LHS is the DECL itself,
if IS_VLA is true, the LHS is the pointer to this DECL that created by
gimplify_vla_decl.
The benefit of this solution are:
1. Resolved the invalid IR issue;
2. The call stmt carries the address of the VLA natually;
The issue with this solution is:
For VLA and non-VLA, the LHS will be different,
Do you see any other potential issues with this solution?
thanks.
Qing
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi, Richard,
> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>>>
>>> Especially in the VLA case but likely also in general (though unlikely
>>> since usually the receiver of initializations are simple enough). I'd
>>> expect the VLA case end up as
>>>
>>> *ptr_to_decl = .DEFERRED_INIT (...);
>>>
>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>
>> So, for the following small testing case:
>>
>>
>> extern void bar (int);
>>
>> void foo(int n)
>> {
>> int arr[n];
>> bar (arr[2]);
>> return;
>> }
>> =
>>
>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
>> auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>>
>> =
>> void foo (int n)
>> {
>> int n.0;
>> sizetype D.1950;
>> bitsizetype D.1951;
>> sizetype D.1952;
>> bitsizetype D.1953;
>> sizetype D.1954;
>> int[0:D.1950] * arr.1;
>> void * saved_stack.2;
>> int arr[0:D.1950] [value-expr: *arr.1];
>>
>> saved_stack.2 = __builtin_stack_save ();
>> try
>>{
>> n.0 = n;
>> _1 = (long int) n.0;
>> _2 = _1 + -1;
>> _3 = (sizetype) _2;
>> D.1950 = _3;
>> _4 = (sizetype) n.0;
>> _5 = (bitsizetype) _4;
>> _6 = _5 * 32;
>> D.1951 = _6;
>> _7 = (sizetype) n.0;
>> _8 = _7 * 4;
>> D.1952 = _8;
>> _9 = (sizetype) n.0;
>> _10 = (bitsizetype) _9;
>> _11 = _10 * 32;
>> D.1953 = _11;
>> _12 = (sizetype) n.0;
>> _13 = _12 * 4;
>> D.1954 = _13;
>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>> arr = .DEFERRED_INIT (D.1952, 2, 1);
>> _14 = (*arr.1)[2];
>> bar (_14);
>> return;
>>}
>> finally
>>{
>> __builtin_stack_restore (saved_stack.2);
>>}
>> }
>>
>>
>>
>> You think that the above .DEFEERED_INIT is not correct?
>> It should be:
>>
>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>>
>> ?
>
> Yes.
>
I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
arr.1 = __builtin_alloca_with_align (D.1954, 32);
*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
However, this call triggered the assertion failure in verify_gimple_call of
tree-cfg.c because the LHS is not a valid LHS.
Then I modify tree-cfg.c as:
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 330eb7dd89bf..180d4f1f9e32 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
}
tree lhs = gimple_call_lhs (stmt);
+ /* For .DEFERRED_INIT call, the LHS might be an indirection of
+ a pointer for the VLA variable, which is not a valid LHS of
+ a gimple call, we ignore the asssertion on this. */
if (lhs
+ && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
&& (!is_gimple_reg (lhs)
&& (!is_gimple_lvalue (lhs)
|| verify_types_in_gimple_reference
The assertion failure in tree-cfg.c got resolved, but I got another assertion
failure in operands_scanner::get_expr_operands (tree *expr_p, int flags), line
945:
939 /* If we get here, something has gone wrong. */
940 if (flag_checking)
941 {
942 fprintf (stderr, "unhandled expression in get_expr_operands():\n");
943 debug_tree (expr);
944 fputs ("\n", stderr);
945 gcc_unreachable ();
946 }
Looks like that the gimple statement:
*arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
Is not valid. i.e, the LHS should not be an indirection to a pointer.
How to resolve this issue?
Thanks a lot for your help.
Qing
>>>
What do you mean by “such” decl? A decl whole “DECL_VALUE_EXPR(DECL)” is
valid?
>>>
>>> A 'decl' that has a DECL_VALUE_EXPR should not appear in the IL, it should
>>> always be refered to as its DECL_VALUE_EXPR.
>>
>> Okay.
>
> I'm going to test
>
> diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
> index ebf7eea3b04..15c73b6d6f4 100644
> --- a/gcc/tree-ssa-operands.c
> +++ b/gcc/tree-ssa-operands.c
> @@ -799,10 +799,11 @@ operands_scanner::get_expr_operands (tree *expr_p,
> int flags)
> flags | opf_not_non_addressable |
> opf_address_taken);
> return;
>
> -case SSA_NAME:
> case VAR_DECL:
> case PARM_DECL:
> case RESULT_DECL:
> + gcc_checking_assert (!DECL_HAS_VALUE_EXPR_P (expr));
> +case SSA_NAME:
> case STRING_CST:
> case CONST_DECL:
> if (!(flags & opf_address_taken))
>
> which should pass on unmodified trunk (fingers crossing ;)), but
> it would likely trip on the current -ftrivial-auto-init patch.
>
> The issue with the current IL is that nothing keeps arr.1 live
> and thus the allocation could be DCEd but the .DEFERRED_INIT
> call would remain, eventually being expanded to zero storage
> that isn't there.
>
> Richard.
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote:
>
> On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>> Hi,
>>
>>> On Aug 10, 2021, at 9:16 AM, Richard Biener wrote:
>>>
>>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>>
>>>
>>> +static void
>>> +expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>> +{
>>> + tree var = gimple_call_lhs (stmt);
>>> + tree size_of_var = gimple_call_arg (stmt, 0);
>>> + tree vlaaddr = NULL_TREE;
>>> + tree var_type = TREE_TYPE (var);
>>> + bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>>> + enum auto_init_type init_type
>>> += (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt,
>>> 1));
>>> +
>>> + gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>>> +
>>> + /* if this variable is a VLA, get its SIZE and ADDR first. */
>>> + if (is_vla)
>>> +{
>>> + /* The temporary address variable for this vla should have been
>>> +created during gimplification phase. Refer to
>>> gimplify_vla_decl
>>> +for details. */
>>> + tree var_decl = (TREE_CODE (var) == SSA_NAME) ?
>>> + SSA_NAME_VAR (var) : var;
>>> + gcc_assert (DECL_HAS_VALUE_EXPR_P (var_decl));
>>> + gcc_assert (TREE_CODE (DECL_VALUE_EXPR (var_decl)) ==
>>> INDIRECT_REF);
>>> + /* Get the address of this vla variable. */
>>> + vlaaddr = TREE_OPERAND (DECL_VALUE_EXPR (var_decl), 0);
>>>
>>> err - isn't the address of the decl represented by the LHS
>>> regardless whether this is a VLA or not?
>>
>> The LHS of the call to .DEFERRED_INIT is the DECL itself whatever it’s a
>> VLA or not.
>>
>> In order to create a memset call, we need the Address of this DECL as
>> the first argument.
>> If the DECL is not a VLA, we just simply apply “build_fold_addr_expr” on
>> this DECL to get its address,
>> However, for VLA, during gimplification phase “gimplify_vla_decl”, we
>> have already created a temporary
>> address variable for this DECL, and recorded this address variable with
>> “DECL_VALUE_EXPR(DECL),
>> We should use this already created address variable for VLAs.
>
> So the issue is that the LHS of the .DEFERRED_INIT call is not properly
> gimplified. We should not have such decl there but I see we do not
> have IL verification that covers this.
Don’t quite understand here: do you mean all the LHS of .DEFERRED_INIT
call are not properly gimplified, or
Only the LHS of .DEFERRED_INIT call for VLA are not properly gimplified?
>>>
>>> Especially in the VLA case but likely also in general (though unlikely
>>> since usually the receiver of initializations are simple enough). I'd
>>> expect the VLA case end up as
>>>
>>> *ptr_to_decl = .DEFERRED_INIT (...);
>>>
>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>
>> So, for the following small testing case:
>>
>>
>> extern void bar (int);
>>
>> void foo(int n)
>> {
>> int arr[n];
>> bar (arr[2]);
>> return;
>> }
>> =
>>
>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
>> auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>>
>> =
>> void foo (int n)
>> {
>> int n.0;
>> sizetype D.1950;
>> bitsizetype D.1951;
>> sizetype D.1952;
>> bitsizetype D.1953;
>> sizetype D.1954;
>> int[0:D.1950] * arr.1;
>> void * saved_stack.2;
>> int arr[0:D.1950] [value-expr: *arr.1];
>>
>> saved_stack.2 = __builtin_stack_save ();
>> try
>>{
>> n.0 = n;
>> _1 = (long int) n.0;
>> _2 = _1 + -1;
>> _3 = (sizetype) _2;
>> D.1950 = _3;
>> _4 = (sizetype) n.0;
>> _5 = (bitsizetype) _4;
>> _6 = _5 * 32;
>> D.1951 = _6;
>> _7 = (sizetype) n.0;
>> _8 = _7 * 4;
>> D.1952 = _8;
>> _9 = (sizetype) n.0;
>> _10 = (bitsizetype) _9;
>> _11 = _10 * 32;
>> D.1953 = _11;
>> _12 = (sizetype) n.0;
>> _13 = _12 * 4;
>> D.1954 = _13;
>> arr.1 = __builtin_alloca_with_align (D.1954, 32);
>> arr = .DEFERRED_INIT (D.1952, 2, 1);
>> _14 = (*arr.1)[2];
>> bar (_14);
>> return;
>>}
>> finally
>>{
>> __builtin_stack_restore (saved_stack.2);
>>}
>> }
>>
>>
>>
>> You think that the above .DEFEERED_INIT is not correct?
>> It should be:
>>
>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>>
>> ?
>
> Yes.
>
>>>
What do you mean by “such” decl? A decl whole “DECL_VALUE_EXPR(DECL)” is
valid?
>>>
>>> A 'decl' that has a DECL_VALUE_EXPR should not appear in the IL, it should
>>> always be refered to as its DECL_VALUE_EXPR.
>>
>> Okay.
>
> I'm going to test
>
> diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
> index ebf7eea3b04..15c73b6d6f4 100644
> --- a/gcc/tree-ssa-operands.c
> +++ b/gcc/tree-ssa-operands.c
> @@ -799,10 +799,11 @@
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 10 Aug 2021, Qing Zhao wrote:
> Hi,
>
> > On Aug 10, 2021, at 9:16 AM, Richard Biener wrote:
> >
> > On Tue, 10 Aug 2021, Qing Zhao wrote:
> >
> >
> > +static void
> > +expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> > +{
> > + tree var = gimple_call_lhs (stmt);
> > + tree size_of_var = gimple_call_arg (stmt, 0);
> > + tree vlaaddr = NULL_TREE;
> > + tree var_type = TREE_TYPE (var);
> > + bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> > + enum auto_init_type init_type
> > += (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt,
> > 1));
> > +
> > + gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> > +
> > + /* if this variable is a VLA, get its SIZE and ADDR first. */
> > + if (is_vla)
> > +{
> > + /* The temporary address variable for this vla should have been
> > +created during gimplification phase. Refer to
> > gimplify_vla_decl
> > +for details. */
> > + tree var_decl = (TREE_CODE (var) == SSA_NAME) ?
> > + SSA_NAME_VAR (var) : var;
> > + gcc_assert (DECL_HAS_VALUE_EXPR_P (var_decl));
> > + gcc_assert (TREE_CODE (DECL_VALUE_EXPR (var_decl)) ==
> > INDIRECT_REF);
> > + /* Get the address of this vla variable. */
> > + vlaaddr = TREE_OPERAND (DECL_VALUE_EXPR (var_decl), 0);
> >
> > err - isn't the address of the decl represented by the LHS
> > regardless whether this is a VLA or not?
>
> The LHS of the call to .DEFERRED_INIT is the DECL itself whatever it’s a
> VLA or not.
>
> In order to create a memset call, we need the Address of this DECL as
> the first argument.
> If the DECL is not a VLA, we just simply apply “build_fold_addr_expr” on
> this DECL to get its address,
> However, for VLA, during gimplification phase “gimplify_vla_decl”, we
> have already created a temporary
> address variable for this DECL, and recorded this address variable with
> “DECL_VALUE_EXPR(DECL),
> We should use this already created address variable for VLAs.
> >>>
> >>> So the issue is that the LHS of the .DEFERRED_INIT call is not properly
> >>> gimplified. We should not have such decl there but I see we do not
> >>> have IL verification that covers this.
> >>
> >> Don’t quite understand here: do you mean all the LHS of .DEFERRED_INIT
> >> call are not properly gimplified, or
> >> Only the LHS of .DEFERRED_INIT call for VLA are not properly gimplified?
> >
> > Especially in the VLA case but likely also in general (though unlikely
> > since usually the receiver of initializations are simple enough). I'd
> > expect the VLA case end up as
> >
> > *ptr_to_decl = .DEFERRED_INIT (...);
> >
> > where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>
> So, for the following small testing case:
>
>
> extern void bar (int);
>
> void foo(int n)
> {
> int arr[n];
> bar (arr[2]);
> return;
> }
> =
>
> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
> auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>
> =
> void foo (int n)
> {
> int n.0;
> sizetype D.1950;
> bitsizetype D.1951;
> sizetype D.1952;
> bitsizetype D.1953;
> sizetype D.1954;
> int[0:D.1950] * arr.1;
> void * saved_stack.2;
> int arr[0:D.1950] [value-expr: *arr.1];
>
> saved_stack.2 = __builtin_stack_save ();
> try
> {
> n.0 = n;
> _1 = (long int) n.0;
> _2 = _1 + -1;
> _3 = (sizetype) _2;
> D.1950 = _3;
> _4 = (sizetype) n.0;
> _5 = (bitsizetype) _4;
> _6 = _5 * 32;
> D.1951 = _6;
> _7 = (sizetype) n.0;
> _8 = _7 * 4;
> D.1952 = _8;
> _9 = (sizetype) n.0;
> _10 = (bitsizetype) _9;
> _11 = _10 * 32;
> D.1953 = _11;
> _12 = (sizetype) n.0;
> _13 = _12 * 4;
> D.1954 = _13;
> arr.1 = __builtin_alloca_with_align (D.1954, 32);
> arr = .DEFERRED_INIT (D.1952, 2, 1);
> _14 = (*arr.1)[2];
> bar (_14);
> return;
> }
> finally
> {
> __builtin_stack_restore (saved_stack.2);
> }
> }
>
>
>
> You think that the above .DEFEERED_INIT is not correct?
> It should be:
>
> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>
> ?
Yes.
> >
> >> What do you mean by “such” decl? A decl whole “DECL_VALUE_EXPR(DECL)” is
> >> valid?
> >
> > A 'decl' that has a DECL_VALUE_EXPR should not appear in the IL, it should
> > always be refered to as its DECL_VALUE_EXPR.
>
> Okay.
I'm going to test
diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index ebf7eea3b04..15c73b6d6f4 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -799,10 +799,11 @@ operands_scanner::get_expr_operands (tree *expr_p,
int flags)
flags | opf_not_non_addressable |
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi,
> On Aug 10, 2021, at 9:16 AM, Richard Biener wrote:
>
> On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>
> +static void
> +expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> +{
> + tree var = gimple_call_lhs (stmt);
> + tree size_of_var = gimple_call_arg (stmt, 0);
> + tree vlaaddr = NULL_TREE;
> + tree var_type = TREE_TYPE (var);
> + bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
> + enum auto_init_type init_type
> += (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
> +
> + gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
> +
> + /* if this variable is a VLA, get its SIZE and ADDR first. */
> + if (is_vla)
> +{
> + /* The temporary address variable for this vla should have been
> +created during gimplification phase. Refer to gimplify_vla_decl
> +for details. */
> + tree var_decl = (TREE_CODE (var) == SSA_NAME) ?
> + SSA_NAME_VAR (var) : var;
> + gcc_assert (DECL_HAS_VALUE_EXPR_P (var_decl));
> + gcc_assert (TREE_CODE (DECL_VALUE_EXPR (var_decl)) ==
> INDIRECT_REF);
> + /* Get the address of this vla variable. */
> + vlaaddr = TREE_OPERAND (DECL_VALUE_EXPR (var_decl), 0);
>
> err - isn't the address of the decl represented by the LHS
> regardless whether this is a VLA or not?
The LHS of the call to .DEFERRED_INIT is the DECL itself whatever it’s a
VLA or not.
In order to create a memset call, we need the Address of this DECL as the
first argument.
If the DECL is not a VLA, we just simply apply “build_fold_addr_expr” on
this DECL to get its address,
However, for VLA, during gimplification phase “gimplify_vla_decl”, we have
already created a temporary
address variable for this DECL, and recorded this address variable with
“DECL_VALUE_EXPR(DECL),
We should use this already created address variable for VLAs.
>>>
>>> So the issue is that the LHS of the .DEFERRED_INIT call is not properly
>>> gimplified. We should not have such decl there but I see we do not
>>> have IL verification that covers this.
>>
>> Don’t quite understand here: do you mean all the LHS of .DEFERRED_INIT call
>> are not properly gimplified, or
>> Only the LHS of .DEFERRED_INIT call for VLA are not properly gimplified?
>
> Especially in the VLA case but likely also in general (though unlikely
> since usually the receiver of initializations are simple enough). I'd
> expect the VLA case end up as
>
> *ptr_to_decl = .DEFERRED_INIT (...);
>
> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
So, for the following small testing case:
extern void bar (int);
void foo(int n)
{
int arr[n];
bar (arr[2]);
return;
}
=
If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o
auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
=
void foo (int n)
{
int n.0;
sizetype D.1950;
bitsizetype D.1951;
sizetype D.1952;
bitsizetype D.1953;
sizetype D.1954;
int[0:D.1950] * arr.1;
void * saved_stack.2;
int arr[0:D.1950] [value-expr: *arr.1];
saved_stack.2 = __builtin_stack_save ();
try
{
n.0 = n;
_1 = (long int) n.0;
_2 = _1 + -1;
_3 = (sizetype) _2;
D.1950 = _3;
_4 = (sizetype) n.0;
_5 = (bitsizetype) _4;
_6 = _5 * 32;
D.1951 = _6;
_7 = (sizetype) n.0;
_8 = _7 * 4;
D.1952 = _8;
_9 = (sizetype) n.0;
_10 = (bitsizetype) _9;
_11 = _10 * 32;
D.1953 = _11;
_12 = (sizetype) n.0;
_13 = _12 * 4;
D.1954 = _13;
arr.1 = __builtin_alloca_with_align (D.1954, 32);
arr = .DEFERRED_INIT (D.1952, 2, 1);
_14 = (*arr.1)[2];
bar (_14);
return;
}
finally
{
__builtin_stack_restore (saved_stack.2);
}
}
You think that the above .DEFEERED_INIT is not correct?
It should be:
*arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
?
>
>> What do you mean by “such” decl? A decl whole “DECL_VALUE_EXPR(DECL)” is
>> valid?
>
> A 'decl' that has a DECL_VALUE_EXPR should not appear in the IL, it should
> always be refered to as its DECL_VALUE_EXPR.
Okay.
Qing
>
> Richard.
>
>> Qing
>>>
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 10 Aug 2021, Qing Zhao wrote:
>
>
> > On Aug 10, 2021, at 2:36 AM, Richard Biener wrote:
> >
> > On Mon, 9 Aug 2021, Qing Zhao wrote:
> >
> >> Hi, Richard,
> >>
> >> Thanks a lot for you review.
> >>
> >> Although these comments are not made on the latest patch (7th version)
> >> :-), all the comments are valid since the parts you commented
> >> are not changed in the 7th version.
> >>
> >>
> >>> On Aug 9, 2021, at 9:09 AM, Richard Biener wrote:
> >>>
> >>> On Tue, 27 Jul 2021, Qing Zhao wrote:
> >>>
> Hi,
>
> This is the 6th version of the patch for the new security feature for
> GCC.
>
> I have tested it with bootstrap on both x86 and aarch64, regression
> testing on both x86 and aarch64.
> Also compile CPU2017 (running is ongoing), without any issue. (With the
> fix to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>
> Please take a look and let me know any issue.
> >>>
> >>> +/* Handle an "uninitialized" attribute; arguments as in
> >>> + struct attribute_spec.handler. */
> >>> +
> >>> +static tree
> >>> +handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
> >>> (args),
> >>> + int ARG_UNUSED (flags), bool
> >>> *no_add_attrs)
> >>> +{
> >>> + if (!VAR_P (*node))
> >>> +{
> >>> + warning (OPT_Wattributes, "%qE attribute ignored", name);
> >>> + *no_add_attrs = true;
> >>> +}
> >>>
> >>> you are documenting this attribute for automatic variables but
> >>> here you allow placement on globals as well (not sure if at this
> >>> point TREE_STATIC / DECL_EXTERNAL are set correctly).
> >>
> >> Right, I should warn when the attribute is placed for globals or static
> >> variables.
> >> I will try TREE_STATIC/DECL_EXTERNAL to see whether it’s work or not.
> >>
> >>>
> >>> + /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
> >>> + function node for padding initialization. */
> >>> + if (!fn)
> >>> +{
> >>> + tree ftype = build_function_type_list (void_type_node,
> >>> +ptr_type_node,
> >>>
> >>> the "appropriate" place to do this would be
> >>> tree.c:build_common_builtin_nodes
> >>
> >> Sure, will move the creation of function node of BUILT_IN_CLEAR_PADDING
> >> for Fortran etc. to tree.c:build_common_builtin_nodes.
> >>
> >>>
> >>> You seem to marshall the is_vla argument as for_auto_init when
> >>> expanding/folding the builtin and there it's used to suppress
> >>> diagnostics (and make covered pieces not initialized?).
> >>
> >> Yes, I added an extra argument “for_auto_init” for
> >> “BUILT_IN_CLEAR_PADDING”, this argument is added to suppress errors
> >> emitted during folding
> >> BUILT_IN_CLEAR_PADDING for flexible array member . Such errors should Not
> >> be emitted when “BUILT_IN_CLEAR_PADDING” is called with compiler automatic
> >> initialization.
> >> Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586, comment #6
> >> from Jakub Jelinek.
> >>
> >>> I suggest
> >>> to re-name is_vla/for_auto_init to something more descriptive.
> >>
> >> Okay, I will.
> >>>
> >>> + gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
> >>> + not emit some of the error messages since doing that
> >>> + might confuse the end user. */
> >>>
> >>> doesn't explain to me whether errors still might be raised or
> >>> what the actual behavior is.
> >>
> >> Okay, will make this more clear in the comments.
> >>
> >>>
> >>> +static gimple *
> >>> +build_deferred_init (tree decl,
> >>> +enum auto_init_type init_type,
> >>> +bool is_vla)
> >>> +{
> >>> + gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
> >>> + || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
> >>>
> >>> so the is_vla parameter looks redundant (and the assert dangerous?).
> >>> Either the caller knows it deals with a VLA, then that should be
> >>> passed through - constant sizes can also later appear during
> >>> optimization after all - or is_vla should be determined here
> >>> based on whether the size at gimplification time is constant.
> >>
> >> The routine “build_deferred_init” is ONLY called during gimplification
> >> phase by the routine “gimple_add_init_for_auto_var", at this place,
> >> Is_vla should be determined by the caller to check the size of the DECL.
> >> If it’s a vla, the “maybe_with_size_expr” will be applied for
> >> DECL to make it to a WITH_SIZE_EXPR. So, the assertion is purely to make
> >> sure this at gimplification phase.
> >>
> >> Yes, the size of the VLA decl might become a constant later due to
> >> constant propagation, etc. but during the gimplification phase, the
> >> assertion should be true.
> >>>
> >>> + /* If the user requests to initialize automatic variables, we
> >>> +should initialize paddings inside the variable. Add a call to
> >>> +
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
> On Aug 10, 2021, at 2:36 AM, Richard Biener wrote:
>
> On Mon, 9 Aug 2021, Qing Zhao wrote:
>
>> Hi, Richard,
>>
>> Thanks a lot for you review.
>>
>> Although these comments are not made on the latest patch (7th version) :-),
>> all the comments are valid since the parts you commented
>> are not changed in the 7th version.
>>
>>
>>> On Aug 9, 2021, at 9:09 AM, Richard Biener wrote:
>>>
>>> On Tue, 27 Jul 2021, Qing Zhao wrote:
>>>
Hi,
This is the 6th version of the patch for the new security feature for GCC.
I have tested it with bootstrap on both x86 and aarch64, regression
testing on both x86 and aarch64.
Also compile CPU2017 (running is ongoing), without any issue. (With the
fix to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
Please take a look and let me know any issue.
>>>
>>> +/* Handle an "uninitialized" attribute; arguments as in
>>> + struct attribute_spec.handler. */
>>> +
>>> +static tree
>>> +handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
>>> (args),
>>> + int ARG_UNUSED (flags), bool
>>> *no_add_attrs)
>>> +{
>>> + if (!VAR_P (*node))
>>> +{
>>> + warning (OPT_Wattributes, "%qE attribute ignored", name);
>>> + *no_add_attrs = true;
>>> +}
>>>
>>> you are documenting this attribute for automatic variables but
>>> here you allow placement on globals as well (not sure if at this
>>> point TREE_STATIC / DECL_EXTERNAL are set correctly).
>>
>> Right, I should warn when the attribute is placed for globals or static
>> variables.
>> I will try TREE_STATIC/DECL_EXTERNAL to see whether it’s work or not.
>>
>>>
>>> + /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
>>> + function node for padding initialization. */
>>> + if (!fn)
>>> +{
>>> + tree ftype = build_function_type_list (void_type_node,
>>> +ptr_type_node,
>>>
>>> the "appropriate" place to do this would be
>>> tree.c:build_common_builtin_nodes
>>
>> Sure, will move the creation of function node of BUILT_IN_CLEAR_PADDING for
>> Fortran etc. to tree.c:build_common_builtin_nodes.
>>
>>>
>>> You seem to marshall the is_vla argument as for_auto_init when
>>> expanding/folding the builtin and there it's used to suppress
>>> diagnostics (and make covered pieces not initialized?).
>>
>> Yes, I added an extra argument “for_auto_init” for “BUILT_IN_CLEAR_PADDING”,
>> this argument is added to suppress errors emitted during folding
>> BUILT_IN_CLEAR_PADDING for flexible array member . Such errors should Not be
>> emitted when “BUILT_IN_CLEAR_PADDING” is called with compiler automatic
>> initialization.
>> Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586, comment #6
>> from Jakub Jelinek.
>>
>>> I suggest
>>> to re-name is_vla/for_auto_init to something more descriptive.
>>
>> Okay, I will.
>>>
>>> + gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
>>> + not emit some of the error messages since doing that
>>> + might confuse the end user. */
>>>
>>> doesn't explain to me whether errors still might be raised or
>>> what the actual behavior is.
>>
>> Okay, will make this more clear in the comments.
>>
>>>
>>> +static gimple *
>>> +build_deferred_init (tree decl,
>>> +enum auto_init_type init_type,
>>> +bool is_vla)
>>> +{
>>> + gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
>>> + || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
>>>
>>> so the is_vla parameter looks redundant (and the assert dangerous?).
>>> Either the caller knows it deals with a VLA, then that should be
>>> passed through - constant sizes can also later appear during
>>> optimization after all - or is_vla should be determined here
>>> based on whether the size at gimplification time is constant.
>>
>> The routine “build_deferred_init” is ONLY called during gimplification phase
>> by the routine “gimple_add_init_for_auto_var", at this place,
>> Is_vla should be determined by the caller to check the size of the DECL. If
>> it’s a vla, the “maybe_with_size_expr” will be applied for
>> DECL to make it to a WITH_SIZE_EXPR. So, the assertion is purely to make
>> sure this at gimplification phase.
>>
>> Yes, the size of the VLA decl might become a constant later due to constant
>> propagation, etc. but during the gimplification phase, the assertion should
>> be true.
>>>
>>> + /* If the user requests to initialize automatic variables, we
>>> +should initialize paddings inside the variable. Add a call to
>>> +__BUILTIN_CLEAR_PADDING (, 0, for_auto_init = true) to
>>> +initialize paddings of object always to zero regardless of
>>> +INIT_TYPE. */
>>> + if (opt_for_fn (current_function_decl, flag_auto_var_init)
>>> + > AUTO_INIT_UNINITIALIZED
>>> +
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Mon, 9 Aug 2021, Qing Zhao wrote:
> Hi, Richard,
>
> Thanks a lot for you review.
>
> Although these comments are not made on the latest patch (7th version) :-),
> all the comments are valid since the parts you commented
> are not changed in the 7th version.
>
>
> > On Aug 9, 2021, at 9:09 AM, Richard Biener wrote:
> >
> > On Tue, 27 Jul 2021, Qing Zhao wrote:
> >
> >> Hi,
> >>
> >> This is the 6th version of the patch for the new security feature for GCC.
> >>
> >> I have tested it with bootstrap on both x86 and aarch64, regression
> >> testing on both x86 and aarch64.
> >> Also compile CPU2017 (running is ongoing), without any issue. (With the
> >> fix to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
> >>
> >> Please take a look and let me know any issue.
> >
> > +/* Handle an "uninitialized" attribute; arguments as in
> > + struct attribute_spec.handler. */
> > +
> > +static tree
> > +handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
> > (args),
> > + int ARG_UNUSED (flags), bool
> > *no_add_attrs)
> > +{
> > + if (!VAR_P (*node))
> > +{
> > + warning (OPT_Wattributes, "%qE attribute ignored", name);
> > + *no_add_attrs = true;
> > +}
> >
> > you are documenting this attribute for automatic variables but
> > here you allow placement on globals as well (not sure if at this
> > point TREE_STATIC / DECL_EXTERNAL are set correctly).
>
> Right, I should warn when the attribute is placed for globals or static
> variables.
> I will try TREE_STATIC/DECL_EXTERNAL to see whether it’s work or not.
>
> >
> > + /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
> > + function node for padding initialization. */
> > + if (!fn)
> > +{
> > + tree ftype = build_function_type_list (void_type_node,
> > +ptr_type_node,
> >
> > the "appropriate" place to do this would be
> > tree.c:build_common_builtin_nodes
>
> Sure, will move the creation of function node of BUILT_IN_CLEAR_PADDING for
> Fortran etc. to tree.c:build_common_builtin_nodes.
>
> >
> > You seem to marshall the is_vla argument as for_auto_init when
> > expanding/folding the builtin and there it's used to suppress
> > diagnostics (and make covered pieces not initialized?).
>
> Yes, I added an extra argument “for_auto_init” for “BUILT_IN_CLEAR_PADDING”,
> this argument is added to suppress errors emitted during folding
> BUILT_IN_CLEAR_PADDING for flexible array member . Such errors should Not be
> emitted when “BUILT_IN_CLEAR_PADDING” is called with compiler automatic
> initialization.
> Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586, comment #6
> from Jakub Jelinek.
>
> > I suggest
> > to re-name is_vla/for_auto_init to something more descriptive.
>
> Okay, I will.
> >
> > + gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
> > + not emit some of the error messages since doing that
> > + might confuse the end user. */
> >
> > doesn't explain to me whether errors still might be raised or
> > what the actual behavior is.
>
> Okay, will make this more clear in the comments.
>
> >
> > +static gimple *
> > +build_deferred_init (tree decl,
> > +enum auto_init_type init_type,
> > +bool is_vla)
> > +{
> > + gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
> > + || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
> >
> > so the is_vla parameter looks redundant (and the assert dangerous?).
> > Either the caller knows it deals with a VLA, then that should be
> > passed through - constant sizes can also later appear during
> > optimization after all - or is_vla should be determined here
> > based on whether the size at gimplification time is constant.
>
> The routine “build_deferred_init” is ONLY called during gimplification phase
> by the routine “gimple_add_init_for_auto_var", at this place,
> Is_vla should be determined by the caller to check the size of the DECL. If
> it’s a vla, the “maybe_with_size_expr” will be applied for
> DECL to make it to a WITH_SIZE_EXPR. So, the assertion is purely to make
> sure this at gimplification phase.
>
> Yes, the size of the VLA decl might become a constant later due to constant
> propagation, etc. but during the gimplification phase, the assertion should
> be true.
> >
> > + /* If the user requests to initialize automatic variables, we
> > +should initialize paddings inside the variable. Add a call to
> > +__BUILTIN_CLEAR_PADDING (, 0, for_auto_init = true) to
> > +initialize paddings of object always to zero regardless of
> > +INIT_TYPE. */
> > + if (opt_for_fn (current_function_decl, flag_auto_var_init)
> > + > AUTO_INIT_UNINITIALIZED
> > + && VAR_P (object)
> > + && !DECL_EXTERNAL (object)
> > + &&
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On August 9, 2021 6:38:21 PM GMT+02:00, Qing Zhao wrote:
>Hi, Richard,
>
>Thanks a lot for you review.
>
>Although these comments are not made on the latest patch (7th version) :-),
>all the comments are valid since the parts you commented
>are not changed in the 7th version.
I actually reviewed the 7th patch, just appearantly picked the wrong mail to
reply to...
>
>> On Aug 9, 2021, at 9:09 AM, Richard Biener wrote:
>>
>> On Tue, 27 Jul 2021, Qing Zhao wrote:
>>
>>> Hi,
>>>
>>> This is the 6th version of the patch for the new security feature for GCC.
>>>
>>> I have tested it with bootstrap on both x86 and aarch64, regression testing
>>> on both x86 and aarch64.
>>> Also compile CPU2017 (running is ongoing), without any issue. (With the fix
>>> to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>>>
>>> Please take a look and let me know any issue.
>>
>> +/* Handle an "uninitialized" attribute; arguments as in
>> + struct attribute_spec.handler. */
>> +
>> +static tree
>> +handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
>> (args),
>> + int ARG_UNUSED (flags), bool
>> *no_add_attrs)
>> +{
>> + if (!VAR_P (*node))
>> +{
>> + warning (OPT_Wattributes, "%qE attribute ignored", name);
>> + *no_add_attrs = true;
>> +}
>>
>> you are documenting this attribute for automatic variables but
>> here you allow placement on globals as well (not sure if at this
>> point TREE_STATIC / DECL_EXTERNAL are set correctly).
>
>Right, I should warn when the attribute is placed for globals or static
>variables.
>I will try TREE_STATIC/DECL_EXTERNAL to see whether it’s work or not.
>
>>
>> + /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
>> + function node for padding initialization. */
>> + if (!fn)
>> +{
>> + tree ftype = build_function_type_list (void_type_node,
>> +ptr_type_node,
>>
>> the "appropriate" place to do this would be
>> tree.c:build_common_builtin_nodes
>
>Sure, will move the creation of function node of BUILT_IN_CLEAR_PADDING for
>Fortran etc. to tree.c:build_common_builtin_nodes.
>
>>
>> You seem to marshall the is_vla argument as for_auto_init when
>> expanding/folding the builtin and there it's used to suppress
>> diagnostics (and make covered pieces not initialized?).
>
>Yes, I added an extra argument “for_auto_init” for “BUILT_IN_CLEAR_PADDING”,
>this argument is added to suppress errors emitted during folding
>BUILT_IN_CLEAR_PADDING for flexible array member . Such errors should Not be
>emitted when “BUILT_IN_CLEAR_PADDING” is called with compiler automatic
>initialization.
>Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586, comment #6
>from Jakub Jelinek.
>
>> I suggest
>> to re-name is_vla/for_auto_init to something more descriptive.
>
>Okay, I will.
>>
>> + gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
>> + not emit some of the error messages since doing that
>> + might confuse the end user. */
>>
>> doesn't explain to me whether errors still might be raised or
>> what the actual behavior is.
>
>Okay, will make this more clear in the comments.
>
>>
>> +static gimple *
>> +build_deferred_init (tree decl,
>> +enum auto_init_type init_type,
>> +bool is_vla)
>> +{
>> + gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
>> + || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
>>
>> so the is_vla parameter looks redundant (and the assert dangerous?).
>> Either the caller knows it deals with a VLA, then that should be
>> passed through - constant sizes can also later appear during
>> optimization after all - or is_vla should be determined here
>> based on whether the size at gimplification time is constant.
>
>The routine “build_deferred_init” is ONLY called during gimplification phase
>by the routine “gimple_add_init_for_auto_var", at this place,
>Is_vla should be determined by the caller to check the size of the DECL. If
>it’s a vla, the “maybe_with_size_expr” will be applied for
>DECL to make it to a WITH_SIZE_EXPR. So, the assertion is purely to make sure
>this at gimplification phase.
>
>Yes, the size of the VLA decl might become a constant later due to constant
>propagation, etc. but during the gimplification phase, the assertion should
>be true.
>>
>> + /* If the user requests to initialize automatic variables, we
>> +should initialize paddings inside the variable. Add a call to
>> +__BUILTIN_CLEAR_PADDING (, 0, for_auto_init = true) to
>> +initialize paddings of object always to zero regardless of
>> +INIT_TYPE. */
>> + if (opt_for_fn (current_function_decl, flag_auto_var_init)
>> + > AUTO_INIT_UNINITIALIZED
>> + && VAR_P (object)
>> + && !DECL_EXTERNAL (object)
>> + && !TREE_STATIC (object))
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi, Richard,
Thanks a lot for you review.
Although these comments are not made on the latest patch (7th version) :-), all
the comments are valid since the parts you commented
are not changed in the 7th version.
> On Aug 9, 2021, at 9:09 AM, Richard Biener wrote:
>
> On Tue, 27 Jul 2021, Qing Zhao wrote:
>
>> Hi,
>>
>> This is the 6th version of the patch for the new security feature for GCC.
>>
>> I have tested it with bootstrap on both x86 and aarch64, regression testing
>> on both x86 and aarch64.
>> Also compile CPU2017 (running is ongoing), without any issue. (With the fix
>> to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>>
>> Please take a look and let me know any issue.
>
> +/* Handle an "uninitialized" attribute; arguments as in
> + struct attribute_spec.handler. */
> +
> +static tree
> +handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
> (args),
> + int ARG_UNUSED (flags), bool
> *no_add_attrs)
> +{
> + if (!VAR_P (*node))
> +{
> + warning (OPT_Wattributes, "%qE attribute ignored", name);
> + *no_add_attrs = true;
> +}
>
> you are documenting this attribute for automatic variables but
> here you allow placement on globals as well (not sure if at this
> point TREE_STATIC / DECL_EXTERNAL are set correctly).
Right, I should warn when the attribute is placed for globals or static
variables.
I will try TREE_STATIC/DECL_EXTERNAL to see whether it’s work or not.
>
> + /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
> + function node for padding initialization. */
> + if (!fn)
> +{
> + tree ftype = build_function_type_list (void_type_node,
> +ptr_type_node,
>
> the "appropriate" place to do this would be
> tree.c:build_common_builtin_nodes
Sure, will move the creation of function node of BUILT_IN_CLEAR_PADDING for
Fortran etc. to tree.c:build_common_builtin_nodes.
>
> You seem to marshall the is_vla argument as for_auto_init when
> expanding/folding the builtin and there it's used to suppress
> diagnostics (and make covered pieces not initialized?).
Yes, I added an extra argument “for_auto_init” for “BUILT_IN_CLEAR_PADDING”,
this argument is added to suppress errors emitted during folding
BUILT_IN_CLEAR_PADDING for flexible array member . Such errors should Not be
emitted when “BUILT_IN_CLEAR_PADDING” is called with compiler automatic
initialization.
Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586, comment #6 from
Jakub Jelinek.
> I suggest
> to re-name is_vla/for_auto_init to something more descriptive.
Okay, I will.
>
> + gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
> + not emit some of the error messages since doing that
> + might confuse the end user. */
>
> doesn't explain to me whether errors still might be raised or
> what the actual behavior is.
Okay, will make this more clear in the comments.
>
> +static gimple *
> +build_deferred_init (tree decl,
> +enum auto_init_type init_type,
> +bool is_vla)
> +{
> + gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
> + || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
>
> so the is_vla parameter looks redundant (and the assert dangerous?).
> Either the caller knows it deals with a VLA, then that should be
> passed through - constant sizes can also later appear during
> optimization after all - or is_vla should be determined here
> based on whether the size at gimplification time is constant.
The routine “build_deferred_init” is ONLY called during gimplification phase by
the routine “gimple_add_init_for_auto_var", at this place,
Is_vla should be determined by the caller to check the size of the DECL. If
it’s a vla, the “maybe_with_size_expr” will be applied for
DECL to make it to a WITH_SIZE_EXPR. So, the assertion is purely to make sure
this at gimplification phase.
Yes, the size of the VLA decl might become a constant later due to constant
propagation, etc. but during the gimplification phase, the assertion should be
true.
>
> + /* If the user requests to initialize automatic variables, we
> +should initialize paddings inside the variable. Add a call to
> +__BUILTIN_CLEAR_PADDING (, 0, for_auto_init = true) to
> +initialize paddings of object always to zero regardless of
> +INIT_TYPE. */
> + if (opt_for_fn (current_function_decl, flag_auto_var_init)
> + > AUTO_INIT_UNINITIALIZED
> + && VAR_P (object)
> + && !DECL_EXTERNAL (object)
> + && !TREE_STATIC (object))
> + gimple_add_padding_init_for_auto_var (object, false, pre_p);
> + return ret;
>
> I think you want to use either auto_var_p (object) or
> auto_var_in_fn_p (object, current_function_decl). Don't you also
> want to check for the 'uninitialized'
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, 27 Jul 2021, Qing Zhao wrote:
> Hi,
>
> This is the 6th version of the patch for the new security feature for GCC.
>
> I have tested it with bootstrap on both x86 and aarch64, regression testing
> on both x86 and aarch64.
> Also compile CPU2017 (running is ongoing), without any issue. (With the fix
> to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>
> Please take a look and let me know any issue.
+/* Handle an "uninitialized" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_uninitialized_attribute (tree *node, tree name, tree ARG_UNUSED
(args),
+ int ARG_UNUSED (flags), bool
*no_add_attrs)
+{
+ if (!VAR_P (*node))
+{
+ warning (OPT_Wattributes, "%qE attribute ignored", name);
+ *no_add_attrs = true;
+}
you are documenting this attribute for automatic variables but
here you allow placement on globals as well (not sure if at this
point TREE_STATIC / DECL_EXTERNAL are set correctly).
+ /* for languages that do not support BUILT_IN_CLEAR_PADDING, create the
+ function node for padding initialization. */
+ if (!fn)
+{
+ tree ftype = build_function_type_list (void_type_node,
+ptr_type_node,
the "appropriate" place to do this would be
tree.c:build_common_builtin_nodes
You seem to marshall the is_vla argument as for_auto_init when
expanding/folding the builtin and there it's used to suppress
diagnostics (and make covered pieces not initialized?). I suggest
to re-name is_vla/for_auto_init to something more descriptive.
+ gimple_fold_builtin_clear_padding. If FOR_AUTO_INIT,
+ not emit some of the error messages since doing that
+ might confuse the end user. */
doesn't explain to me whether errors still might be raised or
what the actual behavior is.
+static gimple *
+build_deferred_init (tree decl,
+enum auto_init_type init_type,
+bool is_vla)
+{
+ gcc_assert ((is_vla && TREE_CODE (decl) == WITH_SIZE_EXPR)
+ || (!is_vla && TREE_CODE (decl) != WITH_SIZE_EXPR));
so the is_vla parameter looks redundant (and the assert dangerous?).
Either the caller knows it deals with a VLA, then that should be
passed through - constant sizes can also later appear during
optimization after all - or is_vla should be determined here
based on whether the size at gimplification time is constant.
+ /* If the user requests to initialize automatic variables, we
+should initialize paddings inside the variable. Add a call to
+__BUILTIN_CLEAR_PADDING (, 0, for_auto_init = true) to
+initialize paddings of object always to zero regardless of
+INIT_TYPE. */
+ if (opt_for_fn (current_function_decl, flag_auto_var_init)
+ > AUTO_INIT_UNINITIALIZED
+ && VAR_P (object)
+ && !DECL_EXTERNAL (object)
+ && !TREE_STATIC (object))
+ gimple_add_padding_init_for_auto_var (object, false, pre_p);
+ return ret;
I think you want to use either auto_var_p (object) or
auto_var_in_fn_p (object, current_function_decl). Don't you also
want to check for the 'uninitialized' attribute here? I suggest
to abstract the check on whether 'object' should be subject
to autoinit to a helper function.
There's another path above this calling gimplify_init_constructor
for the case of
const struct S x = { ... };
struct S y = x;
where it will try to init 'y' from the CTOR directly, it seems you
do not cover this case. I also think that the above place applies
to all aggregate assignment statements, not only to INIT_EXPRs?
So don't you want to restrict clear-padding emit here?
+static void
+expand_DEFERRED_INIT (internal_fn, gcall *stmt)
+{
+ tree var = gimple_call_lhs (stmt);
+ tree size_of_var = gimple_call_arg (stmt, 0);
+ tree vlaaddr = NULL_TREE;
+ tree var_type = TREE_TYPE (var);
+ bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
+ enum auto_init_type init_type
+= (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
+
+ gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
+
+ /* if this variable is a VLA, get its SIZE and ADDR first. */
+ if (is_vla)
+{
+ /* The temporary address variable for this vla should have been
+created during gimplification phase. Refer to gimplify_vla_decl
+for details. */
+ tree var_decl = (TREE_CODE (var) == SSA_NAME) ?
+ SSA_NAME_VAR (var) : var;
+ gcc_assert (DECL_HAS_VALUE_EXPR_P (var_decl));
+ gcc_assert (TREE_CODE (DECL_VALUE_EXPR (var_decl)) ==
INDIRECT_REF);
+ /* Get the address of this vla variable. */
+ vlaaddr = TREE_OPERAND (DECL_VALUE_EXPR (var_decl), 0);
err - isn't the address of the decl represented by the LHS
regardless whether this is a VLA or not? Looking at DECL_VALUE_EXPR
looks quite fragile since that's not sth data dependence
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Hi, Kees,
Thanks a lot for your testing and the small testing case.
I just studied the root cause of this bug, and found that it’s because the call
to “__builtin_clear_padding()” should NOT be inserted BEFORE
the variable initialization. It should be inserted AFTER the variable
initialization.
Currently since the call to “__builtin_clear_padding()” is inserted Before the
variable initialization like the following:
__builtin_clear_padding (, 0B, 1);
obj = {};
obj.val = val;
Then as a result, the reference to “obj” in the call to
“__builtin_clear_padding” is considered as an uninitialized usage.
I will move the call to __builtin_clear_padding after the variable
initialization.
Thanks.
Qing
> On Jul 28, 2021, at 3:21 PM, Kees Cook wrote:
>
> On Tue, Jul 27, 2021 at 03:26:00AM +, Qing Zhao wrote:
>> This is the 6th version of the patch for the new security feature for GCC.
>>
>> I have tested it with bootstrap on both x86 and aarch64, regression testing
>> on both x86 and aarch64.
>> Also compile CPU2017 (running is ongoing), without any issue. (With the fix
>> to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>>
>> Please take a look and let me know any issue.
>
> Good news, this passes all my initialization tests in the kernel. Yay! :)
>
> However, I see an unexpected side-effect from some static initializations:
>
> net/core/sock.c: In function 'sock_no_sendpage':
> net/core/sock.c:2849:23: warning: 'msg' is used uninitialized
> [-Wuninitialized]
> 2849 | struct msghdr msg = {.msg_flags = flags};
> | ^~~
>
> It seems like -Wuninitialized has suddenly stopped noticing explicit
> static initializers when there are bit fields in the struct. Here's a
> minimized case:
>
> $ cat init.c
> struct weird {
>int bit : 1;
>int val;
> };
>
> int func(int val)
> {
>struct weird obj = { .val = val };
>return obj.val;
> }
>
> $ gcc -c -o init.o -Wall -O2 -ftrivial-auto-var-init=zero init.c
> init.c: In function ‘func’:
> init.c:8:22: warning: ‘obj’ is used uninitialized [-Wuninitialized]
>8 | struct weird obj = { .val = val };
> | ^~~
> init.c:8:22: note: ‘obj’ declared here
>8 | struct weird obj = { .val = val };
> | ^~~
>
>
>
> --
> Kees Cook
Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
On Tue, Jul 27, 2021 at 03:26:00AM +, Qing Zhao wrote:
> This is the 6th version of the patch for the new security feature for GCC.
>
> I have tested it with bootstrap on both x86 and aarch64, regression testing
> on both x86 and aarch64.
> Also compile CPU2017 (running is ongoing), without any issue. (With the fix
> to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586).
>
> Please take a look and let me know any issue.
Good news, this passes all my initialization tests in the kernel. Yay! :)
However, I see an unexpected side-effect from some static initializations:
net/core/sock.c: In function 'sock_no_sendpage':
net/core/sock.c:2849:23: warning: 'msg' is used uninitialized [-Wuninitialized]
2849 | struct msghdr msg = {.msg_flags = flags};
| ^~~
It seems like -Wuninitialized has suddenly stopped noticing explicit
static initializers when there are bit fields in the struct. Here's a
minimized case:
$ cat init.c
struct weird {
int bit : 1;
int val;
};
int func(int val)
{
struct weird obj = { .val = val };
return obj.val;
}
$ gcc -c -o init.o -Wall -O2 -ftrivial-auto-var-init=zero init.c
init.c: In function ‘func’:
init.c:8:22: warning: ‘obj’ is used uninitialized [-Wuninitialized]
8 | struct weird obj = { .val = val };
| ^~~
init.c:8:22: note: ‘obj’ declared here
8 | struct weird obj = { .val = val };
| ^~~
--
Kees Cook
