[PATCH] D42667: SplitKit: Fix liveness recomputation in some remat cases.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: lib/CodeGen/SplitKit.cpp:1458
+      VNInfo *PredVNI = ParentLI.getVNInfoBefore(PredEnd);
+      assert(PredVNI && "Value available in PhiVNI predecessor");
+      if (Visited.insert(PredVNI).second)
----------------
MatzeB wrote:
> kparzysz wrote:
> > In the text of the assertion, should it be "value not available..."?  Also there is no variable PhiVNI.
> Even though most people write an error message here that should be displayed when the assert fails.
> 
> My personal aesthetics really want me to read an assert as:
> "assert that ...".
> 
> From that perspective I rather state the condition that needs to be true (in this case the value must be available in the predecessor). You are right in that I should write "VNI" instead of "PhiVNI" here. I'll change the text to "value must be available in VNI predecessor".
FWIW, the coding standard says to put error message:
To further assist with debugging, make sure to put some kind of error message in the assertion statement, which is printed if the assertion is tripped.

http://www.llvm.org/docs/CodingStandards.html#assert-liberally

Therefore, I would second Krzysztof here.


Repository:
  rL LLVM

https://reviews.llvm.org/D42667