[llvm] [RISCV] Hoist immediate addresses from loads/stores (PR #83644)

Visoiu Mistrih Francis via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 5 15:37:36 PST 2024


================
@@ -162,6 +162,14 @@ InstructionCost RISCVTTIImpl::getIntImmCostInst(unsigned Opcode, unsigned Idx,
     // split up large offsets in GEP into better parts than ConstantHoisting
     // can.
     return TTI::TCC_Free;
+  case Instruction::Store:
+    // If the address is a constant, use the materialization cost.
+    if (Idx == 1)
+      return getIntImmCost(Imm, Ty, CostKind);
----------------
francisvm wrote:

> There are more things we should care about after diving into the code of `RISCVMatInt::getIntMatCost`:
> 
> * What if RVC exists?

>From the `getIntImmCost` perspective, we should be able to just rely on `getIntMatCost` for that, no?

> * What about immediates that can be materialized via a single `LUI/SLLI/BSETI/...` like `0x80000000` (w/ or w/o RVC)? These immediates can't be folded into load/store offsets.

`getIntMatCost` does handle that as well.

> * Why we return the size of inst sequence as the cost w/o RVC, but the cost scales by 100 w/ RVC?

Correct me if I'm wrong, but I think `HasRVC` in `getInstSeqCost` is misleading. My understanding is that it's used to compute the cost of compression:
```
// If CompressionCost is true it will use a different cost calculation if RVC is
// enabled. This should be used to compare two different sequences to determine
// which is more compressible.
```
but the rest of the modeling through sequence generation does take into account RVC, just doesn't check for it to minimize differences in generated code:
```
    // NOTE: We don't check for C extension to minimize differences in generated
    // code.
```

`CompressionCost` is passed as `true` only in ISelLowering when the cost of two sequences is analyzed.

https://github.com/llvm/llvm-project/pull/83644


More information about the llvm-commits mailing list