[PATCH] D130554: [AArch64][GlobalISel] Add heuristics for localizing G_CONSTANT.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:20778
+  // us spending time traversing uses if all we want to know is if it's >= min.
+  auto isUsesAtMost = [&](unsigned Reg, unsigned MaxUses) {
+    unsigned NumUses = 0;
----------------
paquette wrote:
> This seems like it could be a helper function in MachineRegisterInfo
> 
> (or there should be some sort of helper in STLExtras for testing if a range is at most some #elts long)
Good idea.


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/localizer-arm64-tti.ll:117
 
+define i32 @imm_cost_too_large() {
+  ; CHECK-LABEL: name: imm_cost_too_large
----------------
paquette wrote:
> this looks like it tests the
> 
> ```
>     if (RematCost == 2)
>       return 2U;
> ```
> 
> path right?
> 
> what about the other two possible values for `RematCost`?
A cost of 1 is already tested by other localizer tests. That corresponds to a constant being materializable in a single instruction, which we always localize.  I'll add a test for the case of cost > 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130554