- By default enable consthoist-gep flag.
- We saw a 6% improvement on SPEC17 povary and improvements on some internal workloads.
- There is a little information on why this is disabled by default. Are there specific targets/workloads for which this leads to regressions?
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I don't recall the details of this, but I think we just didn't spend the time to do the work necessary to enable by default.
For a lot of these regression tests, it isn't obvious why you're passing consthoist-gep=false.
llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll | ||
---|---|---|
38 ↗ | (On Diff #529988) | Regression |
290 ↗ | (On Diff #529988) | Regression. |
This was a code-size opt for an internal workload a couple years ago... we didn't excessively tested to enable by default.
- Avoid hoisting in following cases:
- The cost of the const expr is 'free' for the target.
- The expr can be folded into a legal addressing mode for the target.
- The expr has only single use, and it's not inside a loop.
- Now there are no regressions.
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp | ||
---|---|---|
436 | Hoisting could still be worthwhile even if the offset is "free", but I guess we can likely handle it after isel. It's not clear to me why you're trying to compute whether the offset is "free" in two different ways. | |
674 | The number of operands of the constant gep shouldn't matter. Excluding very exotic cases, a constant GEP is always going to represent the address of a global plus a constant offset. A GEP with more dimensions isn't a fundamentally different computation. | |
889 | I'm not sure I follow how you're proving "not inside a loop" here. |
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp | ||
---|---|---|
436 | Hi Eli, thanks for looking at the patch :))
The other way you are referring to is the cost calculation at line: 451, correct ? But here, this cost calculation asks if the GEP free/should NOT be hoisted or not. | |
674 | This condition represents a case like this: getelementptr i8, ptr @src, 0, 1 getelementptr i8, ptr @src, 0, 2 Gep hoisting will generate replacing instructions like this: %const = bitcast ptr getelementptr i8, ptr @src, i32 0, i32 1) to ptr %mat_gep = getelementptr i8, ptr %const, i32 1 additional bitcast for casting %mat_gep to ptr So hoisting here didn't add a big benefit, instead it caused additional instruction. | |
889 | Because the hoisting occurs in the same block if all uses are in the same block, and if that block is not a loop, |
Hoisting could still be worthwhile even if the offset is "free", but I guess we can likely handle it after isel.
It's not clear to me why you're trying to compute whether the offset is "free" in two different ways.