[PATCH] D106426: [FuncSpec] Support specialising recursive functions

Chuanqi Xu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 3 03:04:28 PDT 2021


ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:1259-1261
+      // Jut bail in case predicate info for ssa.copy is missing.
+      if (!PI)
+        return;
----------------
SjoerdMeijer wrote:
> ChuanqiXu wrote:
> > When would PI be nullptr? It looks like we didn't update the solver in time.
> Yeah, that is a good point. 
> I have looked into this, but did not yet get the bottom. Either way, I thought that simply returning is more graceful than an assert (which does not even happen in a release build), especially if it is a matter of not updating the solver in time. I thought the solver should be robust against these things, and thus thought returning is better.
It looks like that this diff triggers the assertion and removes the assertion simply. If it is in this case, we need more explanation to remove it. Although there are redundant assertions, we can't remove them arbitrarily.
If this diff wouldn't trigger the assertion, I think we could remove this change and leave it to follow up patches (Like to make SCCPSolver more robust to not update everything in the beginning).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106426/new/

https://reviews.llvm.org/D106426



More information about the llvm-commits mailing list