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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D94447#2505374 <https://reviews.llvm.org/D94447#2505374>, @fhahn wrote:

> 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.

You make a good point here. I think it's worthwhile to distinguish two cases though: The first is where you have a simple chain of and's. I believe this is harmless. Even if you have 32 and's in a row, you would introduce 32 new variables, and that's linear in the input size. This is not a pathological case. The second case is where you have code like this:

  %b = and i1 %a, %a
  %c = and i1 %b, %b
  %d = and i1 %c, %c
  ...

That is, where and operands get reused in a tree. In this case, my previous code introduced an exponential number of variables for `%a`, which is certainly not good. I have updated the implementation to add a `Visited` set, which will cut down the number of renames to linear. I've also added some test cases for deep chains and trees (as well as general tests for nesting).

Does this address this concern sufficiently, or do you want the linear case to be limited as well?


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

https://reviews.llvm.org/D94447