[PATCH] D27259: Make processing @llvm.assume more efficient - operand bundles

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Hi Hal,

Thanks for doing this!  I have not yet looked at the code in detail, but I have some high level comments, questions and answers.

One high level question:  did you look at keeping this "affected" set as part of the `AssumptionCache` itself?  That is, keep a map that maps `ValueHandles` to the corresponding assume instruction they affect?

Despite being an implementation detail we should add some basic documentation about this operand bundle to the langref.

I don't have a good solution for using this in SCEV.  One not-great solution is to have a pre-pass in SCEV that does this:

  for (AssumeInst AI : all_assumes_in_func) {
    for (AffectOp : AI) {
      AffectedMap[getSCEV(AffectOp)].push_back(AI);
    }
  }

and then use the `AffectedMap` instead of using the `"affected"` operand bundle directly.

You could also try to scan a SCEV expression to fetch `Value` s out of `SCEVUnknown` nodes and use the use lists of those values.  But they won't catch cases like the loop preheader containing `assume(%a + 1 < %b)` and then a `isKnownPredicate(%a + 1 < %b)` query since `getSCEV("%a + 1")` will look through the addition and create an add expr.

As for your question about guards:  while I initially wanted to use `AssumptionCache` (and I did have an RFC on llvm-dev about this), I did not go ahead with that idea since it wasn't obvious to me that it would be faster (that is, it isn't obviously a good idea; and I'd have to measure both sides to make an informed judgement).  This is because we tend to have lots of guards in every function, and looking at each one *could* get expensive.

However, I think we will be able to use this "affected" infrastructure more readily.


https://reviews.llvm.org/D27259