[PATCH] D29015: [SimpleLoopUnswitch] Fix introduction of UB when hoisted condition may be undef or poison

Juneyoung Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 25 21:22:06 PST 2020


aqjune marked an inline comment as done.
aqjune added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2179
     // the branch in the split block.
     buildPartialUnswitchConditionalBranch(*SplitBB, Invariants, Direction,
+                                          *ClonedPH, *LoopPH, insertFreeze);
----------------
reames wrote:
> As a further enhancement (separate change), I think you can improve the logic for this case slightly.  If you can prove that the original condition would have triggered UB if any of the invariant components had been poison, then you can avoid the freeze on the partially unswitched condition.
IIUC, it is addressed by this patch because we could exploit that branching on poison is UB:
```
// want to partial unswitch cond2
for (..)
  if (cond1 && cond2) { A } else { B } // if cond2 is poison, cond1 && cond2 is also poison, so this is UB
=>
if (cond2) // freeze is not needed here
  for (..) { if (cond1) { A } }
else
  for (..) { B }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D29015





More information about the llvm-commits mailing list