This is an archive of the discontinued LLVM Phabricator instance.

Differential D141017

[MachineCombiner] Use default latency model when no detailed model available
ClosedPublic

Authored by reames on Jan 4 2023, 4:01 PM.

Details

Reviewers
craig.topper
asi-sc
Carrot
shchenz
Commits
rG86eff6be686a: [MachineCombiner] Use default latency model when no detailed model available
Summary

This change adjusts the cost modeling used when the target does not have a schedule model with individual instruction latencies. After this change, we use the default latency information available from TargetSchedule. The default latency information essentially ends up treating most instructions as latency 1, with a few "expensive" ones getting a higher cost.

Previously, we unconditionally applied the first legal pattern - without any consideration of profitability. As a result, this change both prevents some patterns being applied, and changes which patterns are exercised. (i.e. previously the first pattern was applied, afterwards, maybe the second one is because the first wasn't profitable.)

The motivation here is two fold.

First, this brings the default behavior in line with the behavior when -mcpu or -mtune is specified. This improves test coverage, and generally makes it less likely we will have bad surprises when providing more information to the compiler.

Second, this enables some reassociation for ILP by default. Despite being unconditionally enabled, the prior code tended to "reassociate" repeatedly through an entire chain and simply moving the first operand to the end. The result was still a serial chain, just a different one. With this change, one of the intermediate transforms is unprofitable and we end up with a partially flattened tree.

Note that the resulting code diffs show significant room for improvement in the basic algorithm. I am intentionally excluding those from this patch.

For the test diffs, I don't seen any concerning regressions. I took a fairly close look at the RISCV ones, but only skimmed the x86 (particularly vector x86) changes.

Diff Detail

Event Timeline

reames created this revision.Jan 4 2023, 4:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2023, 4:01 PM
Herald added subscribers: armkevincheng, sjarus, eric-k256 and 27 others. · View Herald Transcript
reames requested review of this revision.Jan 4 2023, 4:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2023, 4:01 PM
Herald added subscribers: • pcwang-thead, MaskRay. · View Herald Transcript
lebedev.ri added a subscriber: lebedev.ri.Jan 4 2023, 4:08 PM

the default latency information available from TargetSchedule.

Could you add some more wording here please?
Where does it come from?

Harbormaster completed remote builds in B205792: Diff 486367.Jan 4 2023, 5:29 PM
reames added a comment.Jan 5 2023, 7:45 AM
In D141017#4027369, @lebedev.ri wrote:

the default latency information available from TargetSchedule.

Could you add some more wording here please?
Where does it come from?

TargetSchedModel::computeOperandLatency and computeInstrLatency, end up calling defaultDefLatency. That in turns end up returning 0 for transient instructions, MCSchedModel.LoadLatency for loads, MCSchedModel.HighLatency for "high latency defs", and 1 for everything else.

The TargeSchedModel is initialized from the MCSchedModule and the subtarget info. It mostly just pulls information from the MCSchedModel. The MCSchedModel is initialized (when no mcpu is defined) to a default initialized instance of the class (see GetDefaultSchedModel and MCSchedule.cpp Ln 25). The default values for LoadLatency and HighLatency are 4 and 10 respectively.

isHighLatencyDef defaults to false, and only x86 and AMDGPU override it. AMDGPU isn't interesting as it doesn't use MachineCombiner. X86 lists a set of divides, sqrts, scatter, and gather instructions.

asi-sc added a comment.Jan 9 2023, 1:57 AM

Previously, we unconditionally applied the first legal pattern - without any consideration of profitability

Thanks for addressing this issue for default scheduling models! I also faced it some time ago, but I didn't have time to take a closer look.

LGTM (I don't check all x86 tests)

pengfei added a subscriber: HaohaiWen.Jan 9 2023, 5:43 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 20 2023, 9:28 AM
This revision was landed with ongoing or failed builds.
Closed by commit rG86eff6be686a: [MachineCombiner] Use default latency model when no detailed model available (authored by reames). · Explain Why
This revision was automatically updated to reflect the committed changes.
reames added a commit: rG86eff6be686a: [MachineCombiner] Use default latency model when no detailed model available.
Herald added a subscriber: luke. · View Herald TranscriptJan 20 2023, 9:28 AM

Large Diff

This large diff affects 165 files. Files without inline comments have been collapsed. Expand All Files

Revision Contents

PathSize
llvm/
lib/
CodeGen/
15 lines
test/
CodeGen/
RISCV/
4 lines
8 lines
40 lines
4 lines
36 lines
32 lines
64 lines
30 lines
20 lines
6 lines
38 lines
6 lines
44 lines
32 lines
144 lines
162 lines
24 lines
76 lines
10 lines
6 lines
10 lines
rvv/
8 lines
40 lines
8 lines
12 lines
2 lines
8 lines
8 lines
4 lines
4 lines
42 lines
104 lines
4 lines
10 lines
36 lines
4 lines
4 lines
4 lines
8 lines
74 lines
34 lines
4 lines
4 lines
36 lines
4 lines
8 lines
56 lines
268 lines
1226 lines
380 lines
X86/
3 lines
6 lines
8 lines
12 lines
6 lines
24 lines
80 lines
16 lines
26 lines
71 lines
204 lines
4 lines
12 lines
6 lines
124 lines
68 lines
4 lines
453 lines
377 lines
42 lines
36 lines
6 lines
4 lines
126 lines
4 lines
2 lines
6 lines
32 lines
12 lines
104 lines
104 lines
116 lines
96 lines
26 lines
3 lines
13 lines
16 lines
12 lines
12 lines
100 lines
134 lines
8 lines
32 lines
172 lines
64 lines
180 lines
126 lines
219 lines
56 lines
21 lines
155 lines
4708 lines
335 lines
1108 lines
93 lines
2 lines
66 lines
19 lines
324 lines
8 lines
48 lines
27 lines
38 lines
84 lines
420 lines
6 lines
282 lines
2 lines
97 lines
1339 lines
71 lines
24 lines
75 lines
75 lines
6 lines
214 lines
2 lines
28 lines
65 lines
22 lines
12 lines
232 lines
774 lines
24 lines
128 lines
14 lines
68 lines
8 lines
8 lines
96 lines
384 lines
4 lines
192 lines
154 lines
120 lines
120 lines
176 lines
22 lines
254 lines
120 lines
148 lines
148 lines
158 lines
138 lines
22 lines
120 lines
94 lines
94 lines
28 lines
50 lines
24 lines
122 lines
6 lines
505 lines

Diff 490892

llvm/lib/CodeGen/MachineCombiner.cpp

llvm/test/CodeGen/RISCV/addc-adde-sube-subc.ll

llvm/test/CodeGen/RISCV/addcarry.ll

llvm/test/CodeGen/RISCV/addimm-mulimm.ll

llvm/test/CodeGen/RISCV/alu64.ll

llvm/test/CodeGen/RISCV/bswap-bitreverse.ll

llvm/test/CodeGen/RISCV/calling-conv-ilp32-ilp32f-common.ll

llvm/test/CodeGen/RISCV/calling-conv-ilp32-ilp32f-ilp32d-common.ll

llvm/test/CodeGen/RISCV/calling-conv-lp64-lp64f-lp64d-common.ll

llvm/test/CodeGen/RISCV/copysign-casts.ll

llvm/test/CodeGen/RISCV/div-by-constant.ll

llvm/test/CodeGen/RISCV/div-pow2.ll

llvm/test/CodeGen/RISCV/div.ll

llvm/test/CodeGen/RISCV/fpclamptosat.ll

llvm/test/CodeGen/RISCV/fpclamptosat_vec.ll

llvm/test/CodeGen/RISCV/iabs.ll

llvm/test/CodeGen/RISCV/mul.ll

llvm/test/CodeGen/RISCV/neg-abs.ll

llvm/test/CodeGen/RISCV/rotl-rotr.ll

llvm/test/CodeGen/RISCV/rv32zbb.ll

llvm/test/CodeGen/RISCV/rv64i-w-insts-legalization.ll

llvm/test/CodeGen/RISCV/rv64zbb.ll

llvm/test/CodeGen/RISCV/rvv/expand-no-v.ll

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-elen.ll

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-mask-vp.ll

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-unaligned.ll

llvm/test/CodeGen/RISCV/rvv/vreductions-mask-vp.ll

llvm/test/CodeGen/RISCV/sadd_sat.ll

llvm/test/CodeGen/RISCV/sadd_sat_plus.ll

llvm/test/CodeGen/RISCV/select-binop-identity.ll

llvm/test/CodeGen/RISCV/shadowcallstack.ll

llvm/test/CodeGen/RISCV/shifts.ll

llvm/test/CodeGen/RISCV/split-udiv-by-constant.ll

llvm/test/CodeGen/RISCV/srem-lkk.ll

llvm/test/CodeGen/RISCV/srem-seteq-illegal-types.ll

llvm/test/CodeGen/RISCV/srem-vector-lkk.ll

llvm/test/CodeGen/RISCV/ssub_sat.ll

llvm/test/CodeGen/RISCV/ssub_sat_plus.ll

llvm/test/CodeGen/RISCV/uadd_sat.ll

llvm/test/CodeGen/RISCV/uadd_sat_plus.ll

llvm/test/CodeGen/RISCV/umulo-128-legalisation-lowering.ll

llvm/test/CodeGen/RISCV/unaligned-load-store.ll

llvm/test/CodeGen/RISCV/urem-lkk.ll

llvm/test/CodeGen/RISCV/urem-seteq-illegal-types.ll

llvm/test/CodeGen/RISCV/urem-vector-lkk.ll

llvm/test/CodeGen/RISCV/usub_sat.ll

llvm/test/CodeGen/RISCV/usub_sat_plus.ll

llvm/test/CodeGen/RISCV/vararg.ll

llvm/test/CodeGen/RISCV/wide-scalar-shift-by-byte-multiple-legalization.ll

llvm/test/CodeGen/RISCV/wide-scalar-shift-legalization.ll

llvm/test/CodeGen/RISCV/xaluo.ll

llvm/test/CodeGen/X86/abdu-vector-128.ll

llvm/test/CodeGen/X86/add-sub-bool.ll

llvm/test/CodeGen/X86/alias-static-alloca.ll

llvm/test/CodeGen/X86/avx-vinsertf128.ll

llvm/test/CodeGen/X86/avx512-broadcast-unfold.ll

llvm/test/CodeGen/X86/avx512-intrinsics-x86_64.ll

llvm/test/CodeGen/X86/avx512-intrinsics.ll

llvm/test/CodeGen/X86/avx512-mask-op.ll

llvm/test/CodeGen/X86/avx512-regcall-Mask.ll

llvm/test/CodeGen/X86/avx512-regcall-NoMask.ll

llvm/test/CodeGen/X86/avx512bw-intrinsics-upgrade.ll

llvm/test/CodeGen/X86/avx512fp16-intrinsics.ll

llvm/test/CodeGen/X86/avx512fp16-mov.ll

llvm/test/CodeGen/X86/avx512fp16-mscatter.ll

llvm/test/CodeGen/X86/avx512vl-intrinsics.ll

llvm/test/CodeGen/X86/bmi-out-of-order.ll

llvm/test/CodeGen/X86/combine-add.ll

llvm/test/CodeGen/X86/div-rem-pair-recomposition-signed.ll

llvm/test/CodeGen/X86/div-rem-pair-recomposition-unsigned.ll

llvm/test/CodeGen/X86/divide-by-constant.ll

llvm/test/CodeGen/X86/divmod128.ll

llvm/test/CodeGen/X86/fold-add.ll

llvm/test/CodeGen/X86/fold-masked-merge.ll

llvm/test/CodeGen/X86/fold-tied-op.ll

llvm/test/CodeGen/X86/h-registers-1.ll

llvm/test/CodeGen/X86/hipe-cc.ll

llvm/test/CodeGen/X86/hipe-cc64.ll

llvm/test/CodeGen/X86/horizontal-reduce-add.ll

llvm/test/CodeGen/X86/horizontal-reduce-fadd.ll

llvm/test/CodeGen/X86/horizontal-reduce-smax.ll

llvm/test/CodeGen/X86/horizontal-reduce-smin.ll

llvm/test/CodeGen/X86/horizontal-reduce-umax.ll

llvm/test/CodeGen/X86/horizontal-reduce-umin.ll

llvm/test/CodeGen/X86/horizontal-sum.ll

llvm/test/CodeGen/X86/icmp-shift-opt.ll

llvm/test/CodeGen/X86/imul.ll

llvm/test/CodeGen/X86/lea-opt-cse4.ll

llvm/test/CodeGen/X86/lea-opt2.ll

llvm/test/CodeGen/X86/logic-shift.ll

llvm/test/CodeGen/X86/machine-cp.ll

llvm/test/CodeGen/X86/madd.ll

llvm/test/CodeGen/X86/masked_gather.ll

llvm/test/CodeGen/X86/masked_gather_scatter.ll

llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll

llvm/test/CodeGen/X86/memcmp-more-load-pairs.ll

llvm/test/CodeGen/X86/midpoint-int-vec-128.ll

llvm/test/CodeGen/X86/midpoint-int-vec-256.ll

llvm/test/CodeGen/X86/midpoint-int.ll

llvm/test/CodeGen/X86/movmsk-cmp.ll

llvm/test/CodeGen/X86/mul-constant-i64.ll

llvm/test/CodeGen/X86/mul-constant-result.ll

llvm/test/CodeGen/X86/mul-i1024.ll

llvm/test/CodeGen/X86/mul-i256.ll

llvm/test/CodeGen/X86/mul-i512.ll

llvm/test/CodeGen/X86/mul128.ll

llvm/test/CodeGen/X86/mul64.ll

llvm/test/CodeGen/X86/muloti.ll

llvm/test/CodeGen/X86/optimize-max-0.ll

llvm/test/CodeGen/X86/popcnt.ll

llvm/test/CodeGen/X86/pr34080-2.ll

llvm/test/CodeGen/X86/ptest.ll

llvm/test/CodeGen/X86/rev16.ll

llvm/test/CodeGen/X86/rotate-multi.ll

llvm/test/CodeGen/X86/sad.ll

llvm/test/CodeGen/X86/setcc-wide-types.ll

llvm/test/CodeGen/X86/shift-combine.ll

llvm/test/CodeGen/X86/smul-with-overflow.ll

llvm/test/CodeGen/X86/smul_fix.ll

llvm/test/CodeGen/X86/smul_fix_sat.ll

llvm/test/CodeGen/X86/smulo-128-legalisation-lowering.ll

llvm/test/CodeGen/X86/sse-regcall.ll

llvm/test/CodeGen/X86/stack-clash-large.ll

llvm/test/CodeGen/X86/statepoint-live-in.ll

llvm/test/CodeGen/X86/statepoint-regs.ll

llvm/test/CodeGen/X86/swift-return.ll

llvm/test/CodeGen/X86/umul-with-overflow.ll

llvm/test/CodeGen/X86/umul_fix.ll

llvm/test/CodeGen/X86/umul_fix_sat.ll

llvm/test/CodeGen/X86/umulo-128-legalisation-lowering.ll

llvm/test/CodeGen/X86/umulo-64-legalisation-lowering.ll

llvm/test/CodeGen/X86/urem-seteq-nonzero.ll

llvm/test/CodeGen/X86/v8i1-masks.ll

llvm/test/CodeGen/X86/vec_smulo.ll

llvm/test/CodeGen/X86/vec_umulo.ll

llvm/test/CodeGen/X86/vector-fshr-128.ll

llvm/test/CodeGen/X86/vector-interleaved-store-i16-stride-7.ll

llvm/test/CodeGen/X86/vector-interleaved-store-i8-stride-5.ll

llvm/test/CodeGen/X86/vector-interleaved-store-i8-stride-8.ll

llvm/test/CodeGen/X86/vector-pcmp.ll

llvm/test/CodeGen/X86/vector-reduce-add-mask.ll

llvm/test/CodeGen/X86/vector-reduce-add-sext.ll

llvm/test/CodeGen/X86/vector-reduce-add-zext.ll

llvm/test/CodeGen/X86/vector-reduce-add.ll

llvm/test/CodeGen/X86/vector-reduce-and-bool.ll

llvm/test/CodeGen/X86/vector-reduce-and-cmp.ll

llvm/test/CodeGen/X86/vector-reduce-and.ll

llvm/test/CodeGen/X86/vector-reduce-mul.ll

llvm/test/CodeGen/X86/vector-reduce-or-bool.ll

llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll

llvm/test/CodeGen/X86/vector-reduce-or.ll

llvm/test/CodeGen/X86/vector-reduce-smax.ll

llvm/test/CodeGen/X86/vector-reduce-smin.ll

llvm/test/CodeGen/X86/vector-reduce-umax.ll

llvm/test/CodeGen/X86/vector-reduce-umin.ll

llvm/test/CodeGen/X86/vector-reduce-xor-bool.ll

llvm/test/CodeGen/X86/vector-reduce-xor.ll

llvm/test/CodeGen/X86/vector-trunc-math.ll

llvm/test/CodeGen/X86/vector-trunc-packus.ll

llvm/test/CodeGen/X86/vp2intersect_multiple_pairs.ll

llvm/test/CodeGen/X86/win-smallparams.ll

llvm/test/CodeGen/X86/x86-32-vector-calling-conv.ll

llvm/test/CodeGen/X86/x86-interleaved-access.ll

llvm/test/CodeGen/X86/x86-no_caller_saved_registers-preserve.ll

llvm/test/CodeGen/X86/xmulo.ll