[PATCH] D94447: [PredicateInfo] Generalize processing of conditions

Mon Jan 18 12:28:10 PST 2021

fhahn added a comment.

In D94447#2493177 <https://reviews.llvm.org/D94447#2493177>, @nikic wrote:

> The thought that the current limitations are compile-time related crossed my mind, but that doesn't seem to be a concern: https://llvm-compile-time-tracker.com/compare.php?from=00f773cf424699d8eb31591fdc95e0ca18b2682c&to=10ec7c7960ea47e899003fcd7d1ba5c389ba1cae&stat=instructions

I believe compile-time is at least partly a motivation for some limitation here, but not in a way that is exposed by CTMark. I think here we mostly want to guard against the worst case (deeply nested `AND`/`OR`). Those kind of cases are not really covered by the tests in `CTMark` (or other parts in the `test-suite`), but are surprisingly common in practice (e.g. various code-generators that feed into Clang/LLVM). I think it would be desirable to have a limit on the number of items in the worklist to guard against that.

================
Comment at: llvm/test/Transforms/SCCP/conditions-ranges.ll:1012
   %b.lt = icmp ult i32 %b, 50
   %bc.2 = and i1 %bc, %b.lt
   br i1 %bc.2, label %true, label %false
----------------
Perhaps I missed it, but do we have tests with a chain or `Ors`? If not, it would be good to add one, also independent of SCCP. The same also for `assume` And it would probably be also good to cover the case where `and` and `or` are mixed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94447