[PATCH] D82709: [MachineLICM] [PowerPC] hoisting rematerializable cheap instructions based on register pressure.

ChenZheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 30 00:30:12 PDT 2020


shchenz marked an inline comment as done.
shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.h:318
+  /// Hoist cheap instructions based on register pressure in Machine LICM.
+  bool shouldHoistCheapInstructions() const override { return true; }
+
----------------
efriedma wrote:
> I'm not really happy about adding target-specific heuristics to MachineLICM.  Each target has its own cost model to some extent; we might want to use different rules for specific instructions, or maybe specific register files if they're particularly tiny.  But I'd like to avoid knobs that universally change the way the algorithm works.  If the core algorithm changes depending on the target, that makes it much harder to understand the the way the code is supposed to work, or make any changes in the future, or implement the hook appropriately.
yeah, agree with you for the hook adding policy. There is a conflict place in MachineLICM on PowerPC:
1: `hasLowDefLatency` overriding on PowerPC in commit https://reviews.llvm.org/rL258142, it indicates that all instructions including cheap instructions should be hoisted outside of loop.
2: In function `MachineLICMBase::CanCauseHighRegPressure`, there are statements:
```
    // Don't hoist cheap instructions if they would increase register pressure,
    // even if we're under the limit.
    if (CheapInstr && !HoistCheapInsts)
      return true;
```

Here I just want to make it consistent on PowerPC target. But it seems strange with two different hooks...Maybe we need another patch to improve this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82709





More information about the llvm-commits mailing list