[PATCH] D13963: Create a new interface addSuccessorWithoutWeight(MBB*) in MBB to add successors when optimization is disabled.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:511
@@ -516,1 +510,3 @@
+  // diabled optimization) or has the same size as successor list.
+  if (!Weights.empty() || Successors.empty())
     Weights.push_back(Weight);
----------------
congh wrote:
> davidxl wrote:
> > I find it more readable to say
> > 
> > if (! (Weights.empty() && !Successors.empty())) {
> >    ...
> > }
> > 
> > Since this interface is called only when optimization enabled, so 
> >  (Weights.empty () && Successors.empty()) == false should be asserted -- basically the above if condition is always  true?
> > I find it more readable to say
> > 
> > if (! (Weights.empty() && !Successors.empty())) {
> > 
> > ...
> > }
> 
> Yes.
> 
> > 
> > Since this interface is called only when optimization enabled, so 
> > (Weights.empty () && Successors.empty()) == false should be asserted -- basically the above if condition is always true?
> 
> This is not true. When optimization is disabled, this interface can also be called, even the added weights are never used. That is to say, we cannot not guarantee empty weight list when optimization is disabled.
> 
> 
Basically if the addSuccessor(..) interface is used for the first successor (when opt is disabled), then the weight list will be created. Does that explain the test case result difference?

Also what happens if addSuccessor is first called, and then followed by addSuccessorWithoutWeight -- then the assert (Weights.empty()) will be triggered.


http://reviews.llvm.org/D13963