[PATCH] D147078: [LICM][BPF] Disable hoistMinMax() optimization for BPF target

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.

I'm rejecting this request for a TTI policy exception for the BPF backend in LICM. I've reviewed the discussion, but have found no grounds to support such an exception.

The discussion seems to be based around an assumption that backends should have a right to opt-out of certain transforms -- however, this assumption is simply incorrect for target-independent canonicalization passes like LICM. An exception would require some special circumstance, e.g. the inability to perform a backend undo transform because the transform is lossy, becomes lossy in conjunction with other transforms, or is not reversible without undue effort for other reasons. However, no evidence that this is the case here has been presented.

In fact, the assumptions underlying this discussion make me especially unwilling to grant an exception, because it seems clear that such an exception would be taken as precedent to make similar changes in other places. While I don't care particularly strongly about maintaining canonicality for this particular (relatively exotic) transform, extending this to certain other areas would be much more problematic. For example, the discussion mentions the InstCombine range check canonicalization as another motivating example, and that one **is** an important part of canonical IR form. As such, I would rather nip this entire direction in the bud.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147078