On 2/24/21 5:13 AM, Jakub Jelinek via Gcc-patches wrote:
On Wed, Feb 24, 2021 at 11:50:10AM +0100, Richard Biener wrote:
In the PR using NOP_EXPR has been discussed as one possibility and has been
rejected because at expansion it will emit a superfluous & 1 operation.
I still think it is a good idea to use NOP_EXPR and so have changed
expansion to not emit that & 1 operation in that case. Both spots are
done with tight conditions (bool only etc.), as I'd like to fix just
that case and not introduce a wider general optimization, but perhaps
later we could lift it and do a general range of arbitrary
type_has_mode_precision_p to non-type_has_mode_precision_p with same
TYPE_MODE case.
But it still is a pessimization. VCE says there's no code to be
generated but NOP_EXPR says there is a conversion involved, even
if you later elide it via ssa_name_has_boolean_range.
I'm not convinced it is a pessimization.
Because, a NOP_EXPR is something the compiler can optimize orders of
magnitude more than VCE.
To back that up by some random numbers,
grep CASE_CONVERT: gimple-match.c | wc -l; grep VIEW_CONVERT_EXPR:
gimple-match.c | wc -l
417
18
So I wonder what other optimizations are prevented here?
Why does uninit warn with VCE but not with NOP_EXPR? Or does the
warning disappear because of those other optimizations you mention?
Can you comment on Jeff's POC patch in the PR? Would it make sense
to apply it (with adjustments if necessary) as well to make the warning
more robust in case the VCE comes back?
Martin
The optimization that it prevents is in this particular case in tree-vrp.c
(vrp_simplify_cond_using_ranges):
if (!is_gimple_assign (def_stmt)
|| !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
return;
so it punts on VIEW_CONVERT_EXPR, with NOP_EXPR it optimizes that:
_9 = (bool) maybe_a$4_7;
if (_9 != 0)
into:
_9 = (bool) maybe_a$4_7;
if (maybe_a$4_7 != 0)
Now, if I apply my patch but manually disable this
vrp_simplify_cond_using_ranges optimization, then the uninit warning is
back, so on the uninit side it is not about VIEW_CONVERT_EXPR vs. NOP_EXPR,
both are bad there, uninit wants the guarding condition to be
that SSA_NAME and not some demotion cast thereof.
We have:
# maybe_a$m_6 = PHI <_5(4), maybe_a$m_4(D)(6)>
# maybe_a$4_7 = PHI <1(4), 0(6)>
...
One of:
_9 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_7);
if (_9 != 0)
or:
_9 = (bool) maybe_a$4_7;
if (_9 != 0)
or:
if (maybe_a$4_7 != 0)
followed by:
goto <bb 11>; [0.00%]
else
goto <bb 14>; [0.00%]
...
<bb 11> [count: 0]:
set (maybe_a$m_6);
and uninit wants to see that maybe_a$m_4(D) is not used if
bb 11 is encountered.
So, if you are strongly opposed to the posted patch, I guess the fix can be
(at least fixes the testcase; completely untested except for
make check-c++-all RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}
dg.exp=pr80635*.C'
) following.
But, I fear there will be dozens of other spots where we'll punt on
optimizing when it is a VCE rather than NOP_EXPR.
2021-02-24 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/80635
* tree-vrp.c (vrp_simplify_cond_using_ranges): Also handle
VIEW_CONVERT_EXPR if modes are the same, innerop is integral and
has mode precision.
* g++.dg/warn/pr80635-1.C: New test.
* g++.dg/warn/pr80635-2.C: New test.
--- gcc/tree-vrp.c.jj 2021-02-24 12:56:58.573939572 +0100
+++ gcc/tree-vrp.c 2021-02-24 13:05:22.675326780 +0100
@@ -4390,11 +4390,24 @@ vrp_simplify_cond_using_ranges (vr_value
gimple *def_stmt = SSA_NAME_DEF_STMT (op0);
tree innerop;
- if (!is_gimple_assign (def_stmt)
- || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
+ if (!is_gimple_assign (def_stmt))
return;
- innerop = gimple_assign_rhs1 (def_stmt);
+ switch (gimple_assign_rhs_code (def_stmt))
+ {
+ CASE_CONVERT:
+ innerop = gimple_assign_rhs1 (def_stmt);
+ break;
+ case VIEW_CONVERT_EXPR:
+ innerop = TREE_OPERAND (gimple_assign_rhs1 (def_stmt), 0);
+ if (TYPE_MODE (TREE_TYPE (op0)) != TYPE_MODE (TREE_TYPE (innerop))
+ || !INTEGRAL_TYPE_P (TREE_TYPE (innerop))
+ || !type_has_mode_precision_p (TREE_TYPE (innerop)))
+ return;
+ break;
+ default:
+ break;
+ }
if (TREE_CODE (innerop) == SSA_NAME
&& !POINTER_TYPE_P (TREE_TYPE (innerop))
--- gcc/testsuite/g++.dg/warn/pr80635-1.C.jj 2021-02-24 12:24:15.176834532
+0100
+++ gcc/testsuite/g++.dg/warn/pr80635-1.C 2021-02-24 12:24:15.176834532
+0100
@@ -0,0 +1,46 @@
+// PR tree-optimization/80635
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -Wmaybe-uninitialized" }
+
+using size_t = decltype (sizeof (1));
+inline void *operator new (size_t, void *p) { return p; }
+template<typename T>
+struct optional
+{
+ optional () : m_dummy (), live (false) {}
+ void emplace () { new (&m_item) T (); live = true; }
+ ~optional () { if (live) m_item.~T (); }
+
+ union
+ {
+ struct {} m_dummy;
+ T m_item;
+ };
+ bool live;
+};
+
+extern int get ();
+extern void set (int);
+
+struct A
+{
+ A () : m (get ()) {}
+ ~A () { set (m); } // { dg-bogus "may be used uninitialized in this
function" }
+
+ int m;
+};
+
+struct B
+{
+ B ();
+ ~B ();
+};
+
+void func ()
+{
+ optional<A> maybe_a;
+ optional<B> maybe_b;
+
+ maybe_a.emplace ();
+ maybe_b.emplace ();
+}
--- gcc/testsuite/g++.dg/warn/pr80635-2.C.jj 2021-02-24 12:24:15.176834532
+0100
+++ gcc/testsuite/g++.dg/warn/pr80635-2.C 2021-02-24 12:24:15.176834532
+0100
@@ -0,0 +1,31 @@
+// PR tree-optimization/80635
+// { dg-do compile { target c++17 } }
+// { dg-options "-O2 -Wmaybe-uninitialized" }
+
+#include <optional>
+
+extern int get ();
+extern void set (int);
+
+struct A
+{
+ A () : m (get ()) {}
+ ~A () { set (m); } // { dg-bogus "may be used uninitialized in this
function" }
+
+ int m;
+};
+
+struct B
+{
+ B ();
+ ~B ();
+};
+
+void func ()
+{
+ std::optional<A> maybe_a;
+ std::optional<B> maybe_b;
+
+ maybe_a.emplace ();
+ maybe_b.emplace ();
+}
Jakub
