[PATCH] D145008: [ControlHeightReduction] Don't combine a "poison" branch

[Kazu Hirata via Phabricator via llvm-commits]

kazu added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp:1966
+  // Freeze potentially poisonous conditions.
+  if (!isGuaranteedNotToBeUndefOrPoison(Cond))
     Cond = IRB.CreateFreeze(Cond);
----------------
nikic wrote:
> Is it possible to detect the case where a swap occurred, or is this too complicated?
What do you mean by a swap?  Are you referring to a case where we should generate:

```
select i1 %arg, i1 poison, i1 false
```

instead of

```
select i1 poison, i1 %arg, i1 false
```
(modulo `freeze`)?

If so, yes, it's possible but complicated.  One way goes something like this.  When we call `addToMergedCondition` to add `poison` to the condition chain, we could defer that and then add `poison` at the end of the condition chain.  But then I am not sure what we are really achieving.  Also, special-casing `poison` here goes against your earlier comment that we don't want to go out of our way to handle `poison`.  I am inclined to keeping things simple with `freeze`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145008