> On Feb 24, 2022, at 2:46 AM, Richard Biener <rguent...@suse.de> wrote:
>
> On Wed, 23 Feb 2022, Qing Zhao wrote:
>
>>
>>
>>> On Feb 23, 2022, at 11:49 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>>>
>>> On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote:
>>>> From my understanding, __builtin_clear_padding (&object), does not _use_
>>>> any variable,
>>>> therefore, no uninitialized usage warning should be emitted for it.
>>>
>>> __builtin_clear_padding (&object)
>>> sometimes expands to roughly:
>>> *(int *)((char *)&object + 32) = 0;
>>> etc., in that case it shouldn't be suppressed in any way, it doesn't read
>>> anything, only stores.
>>> Or at other times it is:
>>> *(int *)((char *)&object + 32) &= 0xfec7dab1;
>>> etc., in that case it reads bytes from the object which can be
>>> uninitialized, we mask some bits off and store.
>>
>> Okay, I see.
>> So, only the MEM_REF that will be used to read first should be suppressed
>> warning. Then there is only one (out of 4) MEM_REF
>> should be suppressed warning, that’s the following one (line 4371 and then
>> line 4382):
>>
>> 4371 tree dst = build2_loc (buf->loc, MEM_REF, atype,
>> buf->base,
>> 4372 build_int_cst (buf->alias_type,
>> off));
>> 4373 tree src;
>> 4374 gimple *g;
>> 4375 if (all_ones
>> 4376 && nonzero_first == start
>> 4377 && nonzero_last == start + eltsz)
>> 4378 src = build_zero_cst (type);
>> 4379 else
>> 4380 {
>> 4381 src = make_ssa_name (type);
>> 4382 g = gimple_build_assign (src, unshare_expr (dst));
>> 4383 gimple_set_location (g, buf->loc);
>> 4384 gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 4385 tree mask = native_interpret_expr (type,
>> 4386 buf->buf + i +
>> start,
>> 4387 eltsz);
>> 4388 gcc_assert (mask && TREE_CODE (mask) == INTEGER_CST);
>> 4389 mask = fold_build1 (BIT_NOT_EXPR, type, mask);
>> 4390 tree src_masked = make_ssa_name (type);
>> 4391 g = gimple_build_assign (src_masked, BIT_AND_EXPR,
>> 4392 src, mask);
>> 4393 gimple_set_location (g, buf->loc);
>> 4394 gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 4395 src = src_masked;
>> 4396 }
>> 4397 g = gimple_build_assign (dst, src);
>>
>>
>> All the other 3 MEM_REFs are not read. So, we can just exclude them from
>> suppressing warning, right?
>> Another question, for the above MEM_REF, should I suppress warning for line
>> 4371 “dst”? Or shall I
>> Suppress warning for line 4382 (for the “unshared_expr(dst)”)?
>>
>> I think that we should suppress warning for the latter, i.e
>> “unshared_expr(dst)” at line 4382, right?
>
> Yes, the one that's put into the GIMPLE stmt.
Okay.
>
>>>
>>> It is similar to what object.bitfld = 3; expands to,
>>> but usually only after the uninit pass. Though, we have the
>>> optimize_bit_field_compare optimization, that is done very early
>>> and I wonder what uninit does about that. Perhaps it ignores
>>> BIT_FIELD_REFs, I'd need to check that.
>>
>> Yes, I see that uninitialized warning specially handles BIT_INSERT_EXPR as:
>> (tree-ssa-uninit.cc)
>>
>> 573 /* Do not warn if the result of the access is then used for
>> 574 a BIT_INSERT_EXPR. */
>> 575 if (lhs && TREE_CODE (lhs) == SSA_NAME)
>> 576 FOR_EACH_IMM_USE_FAST (luse_p, liter, lhs)
>> 577 {
>> 578 gimple *use_stmt = USE_STMT (luse_p);
>> 579 /* BIT_INSERT_EXPR first operand should not be considered
>> 580 a use for the purpose of uninit warnings. */
>
> That follows the COMPLEX_EXPR handling I think.
>
>>>
>>> Anyway, if we want to disable uninit warnings for __builtin_clear_padding,
>>> we should do that with suppress_warning on the read stmts that load
>>> a byte (or more adjacent ones) before they are masked off and stored again,
>>> so that we don't warn about that.
>>
>> IN addition to this read stmts, shall we suppress warnings for the following:
>>
>> /* Emit a runtime loop:
>> for (; buf.base != end; buf.base += sz)
>> __builtin_clear_padding (buf.base); */
>>
>> static void
>> clear_padding_emit_loop (clear_padding_struct *buf, tree type,
>> tree end, bool for_auto_init)
>> {
>>
>> i.e, should we suppress warnings for the above “buf.base != end”, “buf.base
>> += sz”?
>>
>> No need to suppress warning for them since they just read the address of the
>> object, not the object itself?
>
> No need to supporess those indeed.
agreed.
thanks.
Will send out the new version soon.
Qing
>
> Richard.
