http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58455
Richard Biener <rguenth at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Last reconfirmed| |2013-09-18 Ever confirmed|0 |1 --- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> --- (In reply to Roland Dreier from comment #2) > Arg, I don't seem to be able to avoid breaking my test cases while > minimizing them. So for my first test case here the issue seems to be that > -Og is the only level that *correctly* warns, which is surprising and I > guess is a bug in it's own right! > > Sorry for the confusion. > > However the test case I was originally working with and then broke is the > following: > > $ cat y.cpp > struct B > { > int offset; > bool is_loaded(unsigned char *p1) > { > if (offset) { > *p1 = offset; > return true; > } > return false; > } > }; > > unsigned char match(struct B ref) > { > do { > unsigned char ref_offset; > if (!ref.is_loaded(&ref_offset) || false) > continue; > return ref_offset; > } while (false); > > return 0; > } > > > $ for o in Og O0 O1 O2 O3; do echo == $o ==; gcc -Wall -$o -Werror -c > ~/y.cpp; done > == Og == > /home/roland/y.cpp: In function ‘unsigned char match(B)’: > /home/roland/y.cpp:17:31: error: ‘ref_offset’ may be used uninitialized in > this function [-Werror=maybe-uninitialized] > unsigned char ref_offset; > ^ > cc1plus: all warnings being treated as errors > == O0 == > == O1 == > == O2 == > == O3 == > > > in that case is_loaded returns true when it sets the pointer; if it returns > false we continue, fall out of the loop and don't touch the pointer value. > So I'm pretty sure this case is correctly classified as spurious. Confirmed. -Og misses a simple jump-threading to un-confuse the CFG here: <bb 2>: _7 = ref.offset; if (_7 != 0) goto <bb 3>; else goto <bb 4>; <bb 3>: ref_offset_8 = (unsigned char) _7; <bb 4>: # _9 = PHI <0(2), 1(3)> # ref_offset_6 = PHI <ref_offset_5(D)(2), ref_offset_8(3)> if (_9 != 0) goto <bb 5>; else goto <bb 6>; <bb 5>: <bb 6>: # iftmp.1_1 = PHI <1(4), 0(5)> if (iftmp.1_1 != 0) goto <bb 8>; else goto <bb 7>; <bb 7>: <bb 8>: # _2 = PHI <ref_offset_6(7), 0(6)> return _2; which is caused by not folding the || false and gimplifying it to D.2218 = B::is_loaded (&ref, &ref_offset); D.2219 = ~D.2218; if (D.2219 != 0) goto <D.2215>; else goto <D.2220>; <D.2220>: if (0 != 0) goto <D.2215>; else goto <D.2216>; <D.2215>: iftmp.1 = 1; goto <D.2217>; <D.2216>: iftmp.1 = 0; <D.2217>: retval.0 = iftmp.1; if (retval.0 != 0) goto <D.2221>; else goto <D.2222>; <D.2221>: goto <D.2211>; <D.2222>: D.2223 = ref_offset; return D.2223; After early CCP we then alreay have the final <bb 7>: # iftmp.1_1 = PHI <1(5), 0(6)> retval.0_10 = iftmp.1_1; if (retval.0_10 != 0) goto <bb 9>; else goto <bb 8>; <bb 8>: ref_offset_11 = ref_offset_13; goto <bb 10>; <bb 9>: _14 = 0; patterns. With -O1+ phiopt merges those blocks. IMHO this asks for moving/adding phiopt in early optimizations. phiopt needs copyproped IL, so the natural place is to add it after a copyprop pass (relying on copyprops simple DCE to clean out empty forwarder blocks and CFG cleanup for removing them).