[PATCH] D116058: [InstCombine] Convert binop(phi, v) to phi(binop) for constant phi operands

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

Just want to point out that this optimization does not introduce redundancy with diamond shape CFG and care needs to be taken for triangular shaped CFG. it might also increase code size though depending on the target -- it might need to be guarded against size optimization.

Some test case change analysis:

1. llvm/test/Transforms/InstCombine/zext-or-icmp.ll -- the patch produces much *cleaner* code

2. llvm/test/Transforms/InstCombine/rem.ll --- need to examine why the patch blocks the intended hoisting transformation. Note that the intended the transformation may introduce redundancy depending on branch probablities.

3. llvm/test/Transforms/InstCombine/phi.ll  --- the patch produces *cleaner* and *neater* code

4. llvm/test/Transforms/InstCombine/not-add.ll  -- the patch produces cleaner code with less redundancy

5. llvm/test/Transforms/InstCombine/narrow.ll   -- this is the case Sanjay pointed out.  There is potential redundancy introduced with the patch.

If issue in 5) can be detected, all in all this patch can improve IR quality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116058