Re: [RFA/RFC] [PR tree-optimization/80635] Optimize some V_C_Es with limited ranges into NOP_EXPRs

[Richard Biener <richard.guenther@gmail.com>]

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
>
>