[PATCH] D87551: [LICM] Make Loop ICM profile aware again

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.
Herald added a subscriber: hoy.

I'm sorry for coming to this very late, but I just noticed this patch due to discussion on another patch (D111806 <https://reviews.llvm.org/D111806>).

This change is inconsistent with the long standing design of LICM.  LICM is a canonicalizing transform, and we specifically *do not* use any profitability metric therein.  MachineLICM exists specifically to be the inverse transformation which is cost based.  I see nothing in the example to make it clear why MachineLICM could not handle this case.

As evidenced by the very existence of the later patch, having this in tree sets a bad precedent.  I believe this patch should be reverted.

Given it's been in tree for nearly a year, this clearly isn't immediately urgent.  Given that, I'm going to give a couple of days for discussion before reverting this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87551