[PATCH] D59535: [SelectionDAG] Compute known bits of CopyFromReg

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 10 03:36:23 PDT 2019


dmgreen added a comment.

> The patch makes SelectionDAG see more context, so it provides more optimization opportunities.

I think it may be more accurate to say that this breaks a whole class of existing optimisations. They were designed under the expectation that selection dag wouldn't do this. You can argue whether that was a sensible design or not, but it's the state we are in right now.

Do you have examples of this performing useful optimisations? Most of the tests here don't show any changes from what I would call sensible code. I would expect the IR transforms to have handled most things like this already (or being doing the split into different BB's deliberately).

I think the most sensible approach is to revert this for now (and hopefully add some more tests!). It seems that this is new info, that wasn't taken into account in the creation/review of this patch, so unless you can fix this quickly we need to prevent breaking the old code. I'm a little chagrined to do that, because there are some improvements in the results I've seen, there are just outweighed by all the regressions. I'm happy to try and help out with extra testing.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59535





More information about the llvm-commits mailing list