[PATCH] D86836: Support a list of CostPerUse values

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:790
   // Try to evict interference from a cheaper alternative.
-  unsigned Cost = TRI->getCostPerUse(PhysReg);
+  unsigned Cost = RegCosts[PhysReg];
 
----------------
There is an implicit upcast happening here from `uint8_t` to `unsigned`. This may lead to unexpected behavior on different platform, I'd recommend to be consistent unless absolutely necessary to use `unsigned`.


================
Comment at: llvm/utils/TableGen/CodeGenRegisters.h:154
     unsigned EnumValue;
-    unsigned CostPerUse;
+    std::vector<int64_t> CostPerUse;
     bool CoveredBySubRegs;
----------------
Is there any specific reason to use `int64_t` type for `CostPerUse`?


================
Comment at: llvm/utils/TableGen/RegisterInfoEmitter.cpp:1457
+  // i.e. the costs for all registers corresponds to index 0, 1, 2, etc.
+  // Size of the emitted array should be NumRegCosts * (Regs.size() + 1).
+  std::vector<unsigned> RegCosts;
----------------
This block of comment can be split and put above the relevant line for better readability. Having a block of comment doesn't really say which logical block of code is meant for. 


================
Comment at: llvm/utils/TableGen/RegisterInfoEmitter.cpp:1458
+  // Size of the emitted array should be NumRegCosts * (Regs.size() + 1).
+  std::vector<unsigned> RegCosts;
+  std::vector<bool> InAllocClass;
----------------
Probably a better name for `RegCosts`. Something like `AllRegCostPerUse`.


================
Comment at: llvm/utils/TableGen/RegisterInfoEmitter.cpp:1459
+  std::vector<unsigned> RegCosts;
+  std::vector<bool> InAllocClass;
+  RegCosts.insert(RegCosts.end(), NumRegCosts, 0);
----------------
I think you can use `std::bitset` or `llvm::BitVector` here. `vector<bool>` is less space optimal than these. 


================
Comment at: llvm/utils/TableGen/RegisterInfoEmitter.cpp:1460
+  std::vector<bool> InAllocClass;
+  RegCosts.insert(RegCosts.end(), NumRegCosts, 0);
+  InAllocClass.push_back(false);
----------------
What is the purpose of this extra block of 0s at the beginning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86836