[PATCH] D124526: [SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both.

Sat Apr 30 04:01:35 PDT 2022

fhahn added a comment.

In D124526#3483889 <https://reviews.llvm.org/D124526#3483889>, @aqjune wrote:

> There was a similar miscompilation report (which was introduced by my patch) and a bug fix due to the existence of `select cond, true, false` in the past: https://reviews.llvm.org/rG5bb38e84d3d0#986154 and https://reviews.llvm.org/rG6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0
> Also a follow-up patch to enhance an assertion condition: https://reviews.llvm.org/rG431a40e1e28f181e87dd247b91a5e6872dd64ab4
>
>> This only fixes trivial unswitching for now, but a similar problem
>> likely exists with non-trivial unswitching.
>
> If the `select cond, true, false` is a concern, what do you think about simplifying it to `cond` (https://alive2.llvm.org/ce/z/p5Ahrp) in advance before loop unswitch?

Is it possible you did not check the latest version of the patch? It uses helpers to skip `select cons, true, false` when trying to match logical and/or.

I don't think we really should simplify/modify the IR if we are not unswitching, which is what 6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0 <https://reviews.llvm.org/rG6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0> did, but SimpleLoopUnswitch claims no changes have been made.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124526