[PATCH] D86836: Support a list of CostPerUse values

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/RegisterClassInfo.h:125
   /// the registers in getOrder(RC).
-  unsigned getMinCost(const TargetRegisterClass *RC) {
-    return get(RC).MinCost;
-  }
+  uint8_t getMinCost(const TargetRegisterClass *RC) { return get(RC).MinCost; }
 
----------------
qcolombet wrote:
> Could we stick to unsigned here to avoid leaking the implementation details of MinCost?
Will do.


================
Comment at: llvm/include/llvm/CodeGen/TargetRegisterInfo.h:215
+  const bool
+      *InAllocatableClass; // Register belongs to an allocatable regclass.
 };
----------------
qcolombet wrote:
> The allocatable part cannot change, can it?
> I.e., why is this an array?
I have changed the way tablegen integrates these values to TargetRegisterInfo. It was an array of struct earlier and now a pointer to a table for the benefit of supporting the list of register costs. I thought it could be ok to change InAllocatableClass because it is part of TargetRegisterInfoDesc class. Otherwise, one of these members should be moved out of the class.



================
Comment at: llvm/include/llvm/CodeGen/TargetRegisterInfo.h:343
+  /// returned by RegisterCostTableIndex.
+  ArrayRef<uint8_t> getRegisterCosts(const MachineFunction &MF) const {
+    unsigned Idx = getRegisterCostTableIndex(MF);
----------------
qcolombet wrote:
> Could we not expose this table and have the users stick to getCostPerUse instead?
It's an overhead to get the index on every cost query with `getCostPerUse`. This will instead, help to extract out a slice of the regcost table (as returned by `getRegisterCosts`) dynamically once from the tablegened register-costs-table with the help of the target handler `getRegisterCostTableIndex`.


================
Comment at: llvm/include/llvm/Target/Target.td:176
+  // considerably reduce the table size.
+  list<int> CostPerUse = [0];
 
----------------
qcolombet wrote:
> Is there a way to use a type that enforce that instead of just writing it in a comment?
I don't think Tablegen have different sized int types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86836