[PATCH] D71209: InstCombine: Don't rewrite phi-of-bitcast when the phi has other users

Mon Dec 9 10:15:44 PST 2019

nikic added a comment.

Change looks fine to me. I'd suggest to add test coverage for a few more of the cases you check. In particular, right now you only cover the case where the outer phi has extra uses, but not the case where an inner phi has them.

================
Comment at: llvm/test/Transforms/InstCombine/pr44242.ll:1
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
----------------
cwabbott wrote:
> lebedev.ri wrote:
> > Please autogenerate and precommit tests.
> Sorry, but what exactly do you mean?
You can use `utils/update_test_checks.py` to generate the expected output. Then you should first commit just the test (with generated output without your patch). The patch will then only show the IR diff in the test, making is clearer what actually changed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71209