[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jakub Jelinek changed: What|Removed |Added Summary|[8/9/10/11 regression] |[8/9/10 regression] |std::optional and bogus |std::optional and bogus |-Wmaybe-uninitialized |-Wmaybe-uninitialized |warning |warning --- Comment #62 from Jakub Jelinek --- Fixed on the trunk.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
--- Comment #53 from Liu Hao ---
For people who are not willing to turn off this warning:
This warning may be suppressed by introducing a volatile member in the union
that is used as the storage.
Using Martin Sebor's testcase, this look likes this:
```
union {
T t;
char x[sizeof (T)];
volatile char dont_use; // this silences such warnings.
};
```
(the `sizeof (T)` thing is unnecessary, as the union will always have correct
size and alignment w/o it.)
Hope it helps.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jeffrey A. Law changed: What|Removed |Added Assignee|jamborm at gcc dot gnu.org |law at redhat dot com --- Comment #52 from Jeffrey A. Law --- ACK. I guess I'll try to polish up the vr-values hack I was playing with.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 --- Comment #51 from Martin Jambor --- (In reply to Andrew Pinski from comment #48) > This should also work too: > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index ea8594db193..83b1d981439 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -2499,6 +2499,7 @@ analyze_access_subtree (struct access *root, struct > access *parent, > For integral types this means the precision has to match. > Avoid assumptions based on the integral type kind, too. */ >if (INTEGRAL_TYPE_P (root->type) > + && TREE_CODE (root->type) != BOOLEAN_TYPE > && (TREE_CODE (root->type) != INTEGER_TYPE > || TYPE_PRECISION (root->type) != root->size) > /* But leave bitfield accesses alone. */ > > CUT Well, this re-introduces bug PR 52244 and makes the associated testcase fail. PR 52244 fix specifically aimed to disallow boolean replacements. (In reply to Jeffrey A. Law from comment #50) > Reassigning to Martin Jambor since the real fix is to avoid creating the > V_C_E in the first place. I hoped that changing SRA to emit a NOP_EXPR instead of V_C_E would help, but unfortunately it doesn't. I've been looking at this for the whole evening yesterday and ATM I do not see how I could avoid conversion without reintroducing PR 52244 (in the general case - this is another consequence of the fact that SRA is not flow sensitive).
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jeffrey A. Law changed: What|Removed |Added Assignee|law at redhat dot com |jamborm at gcc dot gnu.org --- Comment #50 from Jeffrey A. Law --- Reassigning to Martin Jambor since the real fix is to avoid creating the V_C_E in the first place.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jakub Jelinek changed: What|Removed |Added Target Milestone|8.4 |8.5 --- Comment #49 from Jakub Jelinek --- GCC 8.4.0 has been released, adjusting target milestone.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 --- Comment #48 from Andrew Pinski --- (In reply to Jeffrey A. Law from comment #47) > Martin, yea, your patch does prevent creation of the V_C_E. That in turn > allows maybe_a$live_7 to be directly used in the conditional which in turn > allows tree-ssa-uninit.c to realize the problematic path isn't runtime > feasible and suppresses the warning. This should also work too: diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index ea8594db193..83b1d981439 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2499,6 +2499,7 @@ analyze_access_subtree (struct access *root, struct access *parent, For integral types this means the precision has to match. Avoid assumptions based on the integral type kind, too. */ if (INTEGRAL_TYPE_P (root->type) + && TREE_CODE (root->type) != BOOLEAN_TYPE && (TREE_CODE (root->type) != INTEGER_TYPE || TYPE_PRECISION (root->type) != root->size) /* But leave bitfield accesses alone. */ CUT
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 --- Comment #47 from Jeffrey A. Law --- Martin, yea, your patch does prevent creation of the V_C_E. That in turn allows maybe_a$live_7 to be directly used in the conditional which in turn allows tree-ssa-uninit.c to realize the problematic path isn't runtime feasible and suppresses the warning.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 --- Comment #46 from Martin Jambor --- (In reply to Jason Merrill from comment #45) > > We SRA a bool field into a QItype variable and then think we need a VCE to > get back to bool. Could the SRA variable have type bool? A semi-wild guess, would the following make SRA not invent the V_C_E (and do what you want)? diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 5561ea6f655..c3551b469f1 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2515,7 +2515,7 @@ analyze_access_subtree (struct access *root, struct access *parent, /* Always create access replacements that cover the whole access. For integral types this means the precision has to match. Avoid assumptions based on the integral type kind, too. */ - if (INTEGRAL_TYPE_P (root->type) + if (false && INTEGRAL_TYPE_P (root->type) && (TREE_CODE (root->type) != INTEGER_TYPE || TYPE_PRECISION (root->type) != root->size) /* But leave bitfield accesses alone. */
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 --- Comment #45 from Jason Merrill --- (In reply to Jeffrey A. Law from comment #44) > I suspect in the context where SRA creates the V_C_E we don't have enough > information to know that the range of the input object is constrained > enough. We base the decision solely on the types. We SRA a bool field into a QItype variable and then think we need a VCE to get back to bool. Could the SRA variable have type bool?
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jeffrey A. Law changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |law at redhat dot com --- Comment #44 from Jeffrey A. Law --- I suspect in the context where SRA creates the V_C_E we don't have enough information to know that the range of the input object is constrained enough. We base the decision solely on the types. By the time we get into the middle end we can follow the use-def chain to the PHI and realize the source object only has two possible values at runtime and we can optimize the V_C_E into a NOP_EXPR. I've contacted Eric B. offline on the semantics of V_C_E. I think the agreement is in this kind of scenario we ought to be able to simplify it into a NOP_EXPR which should be sufficient to eliminate the false positive.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
--- Comment #43 from Jason Merrill ---
(In reply to Jeffrey A. Law from comment #42)
> I suspect the V_C_E inhibits tree-ssa-uninit.c code to avoid false
> positives. If we know the range of the V_C_E operand is narrow enough that
> it fits into the type of the result of the V_C_E, could we optimize the
> V_C_E into a NOP_EXPR? We'd then forward propagate away the NOP_EXPR and
> the result is something the suppression code in tree-ssa-uninit.c can
> analyze better.
Looks like the V_C_E comes from sra_modify_assign:
if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
{
=>rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
rhs);
Here rhs (the scalar replacement variable) has unsigned QI type and lhs has
type bool.
Changing the type of 'live' to unsigned char avoids the V_C_E and thus indeed
the warning. This also works with the libstdc++ optional.
This seems like an SRA problem with fields of type bool.
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
Jeffrey A. Law changed:
What|Removed |Added
CC||law at redhat dot com
--- Comment #42 from Jeffrey A. Law ---
Jason, thanks for the testcase in c#41. Here's the blocks of interest:
;; 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)>
:
_8 = maybe_b.live;
if (_8 != 0)
goto ; [0.00%]
else
goto ; [0.00%]
;;succ: 6
;;7
;; basic block 6, loop depth 0
;;pred: 5
B::~B (_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
:
_9 = VIEW_CONVERT_EXPR(maybe_a$4_7);
if (_9 != 0)
goto ; [0.00%]
else
goto ; [0.00%]
;;succ: 9
;;10
I believe the complaint arises from the PHI in BB5 which is then used BB9.
Intuitively we can see that anytime BB5 is reached directly from BB2 we know
that BB8 will transfer control to BB10, avoiding the problematical use in BB9.
However, the form of the CFG is particularly difficult for forward threader to
handle. BB5 is a join point. We can only have one other block in the
threading path with side effects -- and unfortunately we have BB6 and BB7. We
can fudge a bit on BB8 due to some work Alex did a couple years back, but
ultimately the forward threader isn't going to handle this.
The backwards threader supports multiple blocks with side effects, but there
can only be one path from the start to the destination. 2->5->6->7->8 and
2->5->7->8 makes two paths, so it gets rejected. This is a limitation we
really should lift in the backwards threader.
I suspect the V_C_E inhibits tree-ssa-uninit.c code to avoid false positives.
If we know the range of the V_C_E operand is narrow enough that it fits into
the type of the result of the V_C_E, could we optimize the V_C_E into a
NOP_EXPR? We'd then forward propagate away the NOP_EXPR and the result is
something the suppression code in tree-ssa-uninit.c can analyze better.
We can see that the path 2->5->6->7->8->9 is infeasible as is the path
2->5->7->8->9. ANytime BB5 is reached directly from BB2
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
--- Comment #41 from Jason Merrill ---
(In reply to Jason Merrill from comment #38)
> This is better:
Complete revised testcase:
#ifdef USE_STD
#include
using std::optional;
#else
using size_t = decltype(sizeof(1));
inline void *operator new (size_t, void *p) { return p; }
template
struct optional
{
optional () : m_dummy (), live (false) {}
void emplace () { new (_item) T (); live = true; }
~optional () { if (live) m_item.~T (); }
union
{
struct {} m_dummy;
T m_item;
};
bool live;
};
#endif
extern int get ();
extern void set (int);
struct A
{
A () : m (get ()) {}
~A () { set (m); }
int m;
};
struct B
{
B ();
~B ();
};
void func ()
{
optional maybe_a;
optional maybe_b;
maybe_a.emplace ();
maybe_b.emplace ();
}
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jason Merrill changed: What|Removed |Added CC||malcolm.parsons at gmail dot com --- Comment #40 from Jason Merrill --- *** Bug 92194 has been marked as a duplicate of this bug. ***
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Martin Liška changed: What|Removed |Added CC||f.heckenb...@fh-soft.de --- Comment #39 from Martin Liška --- *** Bug 92700 has been marked as a duplicate of this bug. ***
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Richard Biener changed: What|Removed |Added Priority|P3 |P2 Version|unknown |8.2.0
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Richard Biener changed: What|Removed |Added Target Milestone|--- |8.4
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
--- Comment #38 from Jason Merrill ---
(In reply to Jason Merrill from comment #37)
>(because the warning is correct for the over-reduced optional):
This is better:
template
struct optional
{
optional () : m_dummy (), live (false) {}
void emplace () { new (_item) T (); live = true; }
~optional () { if (live) m_item.~T (); }
union
{
int m_dummy;
T m_item;
};
bool live;
};
[Bug tree-optimization/80635] [8/9/10 regression] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Jason Merrill changed: What|Removed |Added Component|c++ |tree-optimization Known to work||5.5.0 Summary|std::optional and bogus |[8/9/10 regression] |-Wmaybe-uninitialized |std::optional and bogus |warning |-Wmaybe-uninitialized ||warning Known to fail||10.0, 6.5.0, 7.3.1, 8.3.1, ||9.2.1 --- Comment #37 from Jason Merrill --- Regression from GCC 5, because with the GCC 6 -flifetime-dse changes we properly recognize the initial state of the object as uninitialized; with -fno-lifetime-dse we think it starts out zero-initialized. The problem is that we have two parallel variables that aren't being treated as parallel. From the .optimized dump of the comment 0 test using std::optional (because the warning is correct for the over-reduced optional): [count: 0]: # maybe_a$m_51 = PHI // _M_value # maybe_a$4_54 = PHI <0(2), 1(5)> // _M_engaged ... _12 = VIEW_CONVERT_EXPR(maybe_a$4_54); if (_12 != 0) goto ; [0.00%] else goto ; [0.00%] [count: 0]: set (maybe_a$m_51); Here maybe_a$m_51 is uninitialized iff maybe_a$4_54 is 0, and in that case we don't use the uninitialized value. The PHIs are completely parallel. I'm a bit surprised the optimizer doesn't already handle this sort of situation, where a conditional jump determines which PHI argument we have, and therefore should be able to select the corresponding argument from parallel PHIs in the target block. So bb 10 should be set (_29); and we shouldn't get the warning.
