[PATCH] D125398: [ControlHeightReduction] Freeze condition when converting select to branch

Thu May 12 02:12:05 PDT 2022

nikic added inline comments.

================
Comment at: llvm/test/Transforms/PGOProfile/chr.ll:1182
+; CHECK-NEXT:    [[DOTFR1:%.*]] = freeze i32 [[TMP0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[DOTFR1]], 255
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i32 [[TMP1]], 0
----------------
aqjune wrote:
> It seems this freeze is introduced even if the source code (`entry`) had a conditional branch.
> Is it because CHR first converts the conditional branch into `select` and then convert it back into a conditional branch?
`%0` is used in select conditions further down, that's where the freeze comes from. It then gets hoisted up by InstCombine so that everything uses a common variable.

We can exclude that `%0` is poison because we're branching on a condition involving it first, but we cannot exclude that it has undef bits based on that.

================
Comment at: llvm/test/Transforms/PGOProfile/chr.ll:1293
+; CHECK-NEXT:    [[TMP0:%.*]] = or i1 [[V3]], [[V1_NOT]]
+; CHECK-NEXT:    br i1 [[TMP0]], label [[ENTRY_SPLIT_NONCHR:%.*]], label [[BB1:%.*]], !prof [[PROF19:![0-9]+]]
 ; CHECK:       entry.split.nonchr:
----------------
fhahn wrote:
> nikic wrote:
> > This is the only non-trivial change. Apparently it restores the result to what it was before https://github.com/llvm/llvm-project/commit/1bf8f9e228546bd54ef9739aa808b71b97ea6051, so this is probably fine.
> It would be interesting to know what simplification we are missing and why. Looks like @spatel landed 1bf8f9e228546bd54ef9739aa808b71b97ea6051,, perhaps he has an idea?
Hm, now that I look closer, my initial impression here was incorrect. We're missing this fold: https://alive2.llvm.org/ce/z/WrXbGk

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

https://reviews.llvm.org/D125398