[PATCH] D110752: [SimpleLoopUnswitch] Don't unswitch constant conditions

Daniil Suchkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 30 12:04:53 PDT 2021


DaniilSuchkov added a comment.

In D110752#3034441 <https://reviews.llvm.org/D110752#3034441>, @lebedev.ri wrote:

> Is this falling into the traditional trap of the fact that constants and constant expressions are both represented by the `Constant` class?
> Presumably while we obviously shouldn't unswitch immediate constants, constant expressions are a fair game still?

I don't quite understand your question. Let me explain what's going on.

This bug is introduced by this quick fix: https://reviews.llvm.org/rG6b4b1dc6ec6f
Before that we would never add a constant (i.e. Value with `isa<Constant> == true`) to the list of unswitch candidates. After that patch it became possible for `true` or `false` to become a condition that we're trying to unswitch.

As for the assertion which I added, in `replaceLoopInvariantUses` there's a piece of code that does very similar thing and it has `assert(!isa<Constant>(Invariant) && "Why are we unswitching on a constant?");`, so lack of such an assertion in this code seems just an oversight.


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

https://reviews.llvm.org/D110752



More information about the llvm-commits mailing list