On Sun, Apr 5, 2020 at 5:25 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> So here's an approach to try and address PR80635.
>
> In this BZ we're getting a false positive uninitialized warning using
> std::optional.
>
> As outlined in the BZ this stems from SRA using a VIEW_CONVERT_EXPR which
> isn't
> handled terribly well by the various optimizers/analysis passes.
>
> We have these key blocks:
>
> ;; basic block 5, loop depth 0
> ;; pred: 3
> ;; 2
> # maybe_a$m_6 = PHI <_5(3), maybe_a$m_4(D)(2)>
> # maybe_a$4_7 = PHI <1(3), 0(2)>
> <L0>:
> _8 = maybe_b.live;
> if (_8 != 0)
> goto <bb 6>; [0.00%]
> else
> goto <bb 7>; [0.00%]
> ;; succ: 6
> ;; 7
>
> ;; basic block 6, loop depth 0
> ;; pred: 5
> B::~B (&maybe_b.D.2512.m_item);
> ;; succ: 7
>
> ;; basic block 7, loop depth 0
> ;; pred: 5
> ;; 6
> maybe_b ={v} {CLOBBER};
> resx 3
> ;; succ: 8
>
> ;; basic block 8, loop depth 0
> ;; pred: 7
> <L1>:
> _9 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_7);
So this is a reg-reg copy. But if you replace it with a NOP_EXPR
it becomes a truncation which is less optimal.
Testcase:
char y;
_Bool x;
void __GIMPLE(ssa) foo()
{
_Bool _1;
char _2;
__BB(2):
_2 = y;
_1 = (_Bool)_2;
x = _1;
return;
}
void __GIMPLE(ssa) bar()
{
_Bool _1;
char _2;
__BB(2):
_2 = y;
_1 = __VIEW_CONVERT <_Bool> (_2);
x = _1;
return;
}
where assembly is
foo:
.LFB0:
.cfi_startproc
movzbl y(%rip), %eax
andl $1, %eax
movb %al, x(%rip)
ret
vs.
bar:
.LFB1:
.cfi_startproc
movzbl y(%rip), %eax
movb %al, x(%rip)
ret
so the reverse transformation is what should be done ...
Which means other analyses have to improve their handling
of VIEW_CONVERT_EXPR instead.
Richard.
> if (_9 != 0)
> goto <bb 9>; [0.00%]
> else
> goto <bb 10>; [0.00%]
> ;; succ: 9
> ;; 10
>
> Where there is a use of maybe_a$m_6 in block #9.
>
> Of course maybe_a$m_6 only takes the value of maybe_a$m_4(D) when we traverse
> the
> edge 2->5 but in that case maybe_a$4_7 will always have the value zero and
> thus
> we can not reach bb #9.. But the V_C_E gets in the way of the analysis and we
> issue the false positive warning. Martin Jambor has indicated that he doesn't
> see a way to avoid the V_C_E from SRA without reintroducing PR52244.
>
> This patch optimizes the V_C_E into a NOP_EXPR by verifying that the V_C_E
> folds
> to a constant value for the min & max values of the range of the input operand
> and the result of folding is equal to the original input. We do some
> additional
> checking beyond just that original value and converted value are equal
> according
> to operand_equal_p.
>
> Eventually the NOP_EXPR also gets removed as well and the conditional in bb8
> tests maybe_a$4_7 against 0 directly.
>
> That in turn allows the uninit analysis to determine the use of maybe_a$_m_6
> in
> block #9 is properly guarded and the false positive is avoided.
>
> The optimization of a V_C_E into a NOP_EXPR via this patch occurs a couple
> hundred times during a bootstrap, so this isn't a horribly narrow change just
> to
> fix a false positive warning.
>
> Bootstrapped and regression tested on x86_64. I've also put it through its
> paces
> in the tester. The tester's current failures (aarch64, mips, h8) are
> unrelated
> to this patch.
>
>
> Thoughts? OK for the trunk? Alternately I wouldn't lose sleep moving this to
> gcc-11.
>
> jeff
>
>
