[PATCH] D125248: [instcombine] Propagate freeze back to def

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 9 11:45:12 PDT 2022


nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

I don't think this is the right place to do this. You should either:

1. Remove the one-use restriction on this fold, and replace other uses with the frozen variant as well. This is on the premise that pushing freeze higher is more worthwhile than having some un-frozen uses.

2. Adjust the freezeDominatedUses() fold to freeze not only dominated uses. Basically canonicalize %x to freeze %x. This is on the premise that allowing more exploitation of value identity is more worthwhile than having un-frozen uses.

I'm not sure which one of these is the right approach, and I didn't really understand what optimization is supposed to be achieved in your example -- you might want to construct it in a way that something actually folds away as a result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125248



More information about the llvm-commits mailing list