On Tue, May 14, 2019 at 3:58 PM Vladislav Ivanishin <v...@ispras.ru> wrote: > > Hi! > > The split_critical_edges() function has multiple uses and it seems, a > portion of its code was added to work only when called from tree-ssa-pre > but right now it is executed regardless of the caller. > > The below patch survives bootstrap and regression testing on > x86_64-pc-linux-gnu. Does it make sense?
Yeah. Please rename the in_pre_p parameter to for_edge_insertion_p since that is what it ensures. Note that the use in tree-ssa-sink.c also requires this since it looks for places to sink code to. Similar the use in tree-tail-merge.c (where I'm not sure why it needs split critical edges at all...). So, it seems more safe to have the default of the parameter to true, or rather have no default but adjust all few cases effectively only changing the one before uninit. Better (source) documentation would be using an overload, split_edges_for_insertion? Richard. > If it does, then it uncovers what I think might be a latent bug in the > late uninit pass. > > Without the patch, the crited pass inserts an empty basic block that > almost magically prevents the uninit pass from reporting a false > positive. > > $ gcc -c -fdump-tree-all -fgimple -O -Wuninitialized gimple-fp-hfs.c > > > (no warning) > > --- gimple-fp-hfs.c.170t.veclower21 > +++ gimple-fp-hfs.c.194t.crited1 > @@ -24,10 +24,12 @@ > if (ext_4 <= 7) > goto <bb 3>; [INV] > else > - goto <bb 5>; [INV] > + goto <bb 6>; [INV] > + > + <bb 6> : > > <bb 5> : > - # ext_block_start_3 = PHI <ext_block_start_2(4)> > + # ext_block_start_3 = PHI <ext_block_start_2(6)> > _14 = (int) ext_block_start_3; > return _14; > > And with the patch, the useless bb 6 is not inserted and the following > is produced: > > gimple-fp-hfs.c: In function 'probe_hfsplus': > gimple-fp-hfs.c:5:16: warning: 'ext_block_start' may be used uninitialized in > this function [-Wmaybe-uninitialized] > 5 | unsigned int ext_block_start; > | ^~~~~~~~~~~~~~~ > > If this patch is OK, I'll try to fix the uninit pass next. > > -- > Vlad
