This is an archive of the discontinued LLVM Phabricator instance.

Differential D62707

[AMDGPU] Use InliningThresholdMultiplier for inline hint
ClosedPublic

Authored by rampitec on May 30 2019, 3:27 PM.

Details

Reviewers
scott.linder
arsenm
Commits
rGfbbe5230f434: [AMDGPU] Use InliningThresholdMultiplier for inline hint
rL362239: [AMDGPU] Use InliningThresholdMultiplier for inline hint
Summary

AMDGPU uses multiplier 9 for the inline cost. It is taken into account
everywhere except for inline hint threshold. As a result we are penalizing
functions with the inline hint making them less probable to be inlined
than those without the hint. Defaults are 225 for a normal function and
325 for a function with an inline hint. Currently we have effective
threshold 225 * 9 = 2025 for normal functions and just 325 for those with
the hint. That is fixed by this patch.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.May 30 2019, 3:27 PM
Herald added subscribers: eraman, t-tye, tpr and 7 others. · View Herald TranscriptMay 30 2019, 3:27 PM
arsenm added a comment.May 30 2019, 3:34 PM

Needs test. I'm also not sure I follow how getInliningThresholdMultiplier is used. It looks like it's applied after any of the thresholds are computed in CallAnalyzer::updateThreshold?

Also why do we actually override getInlineThreshold? The alloca object check seems like it shouldn't really be fundamentally different than the SROA checks the default InlineCost does

rampitec added a comment.May 30 2019, 3:43 PM
In D62707#1524219, @arsenm wrote:

Needs test. I'm also not sure I follow how getInliningThresholdMultiplier is used. It looks like it's applied after any of the thresholds are computed in CallAnalyzer::updateThreshold?

Also why do we actually override getInlineThreshold? The alloca object check seems like it shouldn't really be fundamentally different than the SROA checks the default InlineCost does

I can add test, but it essentially will be a quite large function.
Yes, multiplier is applied in the CallAnalyzer::updateThreshold(), which applies to a normal threshold. But here we handle it ourselves, so need to update.
The primary reason to override getInlineCost() is to handle alloca arguments, but it also handles wrapper calls and used to limit inlining based on the number of blocks.

arsenm added a comment.May 30 2019, 3:46 PM
In D62707#1524236, @rampitec wrote:
In D62707#1524219, @arsenm wrote:

Needs test. I'm also not sure I follow how getInliningThresholdMultiplier is used. It looks like it's applied after any of the thresholds are computed in CallAnalyzer::updateThreshold?

Also why do we actually override getInlineThreshold? The alloca object check seems like it shouldn't really be fundamentally different than the SROA checks the default InlineCost does

I can add test, but it essentially will be a quite large function.

You can use the flags to make it a small function

rampitec updated this revision to Diff 202330.May 30 2019, 4:24 PM

Added test.

arsenm added inline comments.May 30 2019, 4:33 PM
test/CodeGen/AMDGPU/inline-hint.ll
1 ↗(On Diff #202330)

Can you remove the -O1, and also move to test/Transforms/Inline/AMDGPU

rampitec updated this revision to Diff 202332.May 30 2019, 4:42 PM
rampitec marked an inline comment as done.

Removed -O1 and moved test.

rampitec updated this revision to Diff 202337.May 30 2019, 5:21 PM

Restored -O1. opt does not run inliner w/o it.

arsenm added inline comments.May 31 2019, 7:53 AM
test/Transforms/Inline/AMDGPU/inline-hint.ll
1 ↗(On Diff #202337)

You need to replace -O1 with -inline, not just remove it

rampitec updated this revision to Diff 202443.May 31 2019, 8:37 AM
rampitec marked an inline comment as done.

Replaced -O1 with -amdgpu-inline.

arsenm accepted this revision.May 31 2019, 8:54 AM

LGTM

This revision is now accepted and ready to land.May 31 2019, 8:54 AM
Closed by commit rL362239: [AMDGPU] Use InliningThresholdMultiplier for inline hint (authored by rampitec). · Explain WhyMay 31 2019, 9:17 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2019, 9:17 AM
aeubanks mentioned this in D94153: [AMDGPU][Inliner] Remove amdgpu-inline and add a new TTI inline hook.Jan 5 2021, 9:25 PM
aeubanks mentioned this in rGa11bf9a7fbd3: [AMDGPU][Inliner] Remove amdgpu-inline and add a new TTI inline hook.Jan 21 2021, 8:29 PM

Revision Contents

PathSize
llvm/
trunk/
lib/
Target/
AMDGPU/
3 lines
test/
Transforms/
Inline/
AMDGPU/
77 lines

Diff 202450

llvm/trunk/lib/Target/AMDGPU/AMDGPUInline.cpp

Show First 20 Lines • Show All 105 Lines • ▼ Show 20 Linesunsigned AMDGPUInliner::getInlineThreshold(CallSite CS) const {
Function *Caller = CS.getCaller(); Function *Caller = CS.getCaller();
// Listen to the inlinehint attribute when it would increase the threshold // Listen to the inlinehint attribute when it would increase the threshold
// and the caller does not need to minimize its size. // and the caller does not need to minimize its size.
Function *Callee = CS.getCalledFunction(); Function *Callee = CS.getCalledFunction();
bool InlineHint = Callee && !Callee->isDeclaration() && bool InlineHint = Callee && !Callee->isDeclaration() &&
Callee->hasFnAttribute(Attribute::InlineHint); Callee->hasFnAttribute(Attribute::InlineHint);
if (InlineHint && Params.HintThreshold && Params.HintThreshold > Thres if (InlineHint && Params.HintThreshold && Params.HintThreshold > Thres
&& !Caller->hasFnAttribute(Attribute::MinSize)) && !Caller->hasFnAttribute(Attribute::MinSize))
Thres = Params.HintThreshold.getValue(); Thres = Params.HintThreshold.getValue() *
TTIWP->getTTI(*Callee).getInliningThresholdMultiplier();
const DataLayout &DL = Caller->getParent()->getDataLayout(); const DataLayout &DL = Caller->getParent()->getDataLayout();
if (!Callee) if (!Callee)
return (unsigned)Thres; return (unsigned)Thres;
// If we have a pointer to private array passed into a function // If we have a pointer to private array passed into a function
// it will not be optimized out, leaving scratch usage. // it will not be optimized out, leaving scratch usage.
// Increase the inline threshold to allow inliniting in this case. // Increase the inline threshold to allow inliniting in this case.
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines

llvm/trunk/test/Transforms/Inline/AMDGPU/inline-hint.ll

; RUN: opt -S -mtriple=amdgcn-unknown-amdhsa -amdgpu-inline --inline-threshold=1 --inlinehint-threshold=2 < %s | FileCheck %s
define hidden <16 x i32> @div_hint(<16 x i32> %x, <16 x i32> %y) #0 {
entry:
%div.1 = udiv <16 x i32> %x, %y
%div.2 = udiv <16 x i32> %div.1, %y
%div.3 = udiv <16 x i32> %div.2, %y
%div.4 = udiv <16 x i32> %div.3, %y
%div.5 = udiv <16 x i32> %div.4, %y
%div.6 = udiv <16 x i32> %div.5, %y
%div.7 = udiv <16 x i32> %div.6, %y
%div.8 = udiv <16 x i32> %div.7, %y
%div.9 = udiv <16 x i32> %div.8, %y
%div.10 = udiv <16 x i32> %div.9, %y
%div.11 = udiv <16 x i32> %div.10, %y
%div.12 = udiv <16 x i32> %div.11, %y
%div.13 = udiv <16 x i32> %div.12, %y
%div.14 = udiv <16 x i32> %div.13, %y
%div.15 = udiv <16 x i32> %div.14, %y
%div.16 = udiv <16 x i32> %div.15, %y
%div.17 = udiv <16 x i32> %div.16, %y
%div.18 = udiv <16 x i32> %div.17, %y
%div.19 = udiv <16 x i32> %div.18, %y
ret <16 x i32> %div.19
}
; CHECK-LABEL: define amdgpu_kernel void @caller_hint
; CHECK-NOT: call
; CHECK: udiv
; CHECK: ret void
define amdgpu_kernel void @caller_hint(<16 x i32> addrspace(1)* nocapture %x, <16 x i32> addrspace(1)* nocapture readonly %y) {
entry:
%tmp = load <16 x i32>, <16 x i32> addrspace(1)* %x, align 4
%tmp1 = load <16 x i32>, <16 x i32> addrspace(1)* %y, align 4
%div.i = tail call <16 x i32> @div_hint(<16 x i32> %tmp, <16 x i32> %tmp1) #0
store <16 x i32> %div.i, <16 x i32> addrspace(1)* %x, align 4
ret void
}
define hidden <16 x i32> @div_nohint(<16 x i32> %x, <16 x i32> %y) {
entry:
%div.1 = udiv <16 x i32> %x, %y
%div.2 = udiv <16 x i32> %div.1, %y
%div.3 = udiv <16 x i32> %div.2, %y
%div.4 = udiv <16 x i32> %div.3, %y
%div.5 = udiv <16 x i32> %div.4, %y
%div.6 = udiv <16 x i32> %div.5, %y
%div.7 = udiv <16 x i32> %div.6, %y
%div.8 = udiv <16 x i32> %div.7, %y
%div.9 = udiv <16 x i32> %div.8, %y
%div.10 = udiv <16 x i32> %div.9, %y
%div.11 = udiv <16 x i32> %div.10, %y
%div.12 = udiv <16 x i32> %div.11, %y
%div.13 = udiv <16 x i32> %div.12, %y
%div.14 = udiv <16 x i32> %div.13, %y
%div.15 = udiv <16 x i32> %div.14, %y
%div.16 = udiv <16 x i32> %div.15, %y
%div.17 = udiv <16 x i32> %div.16, %y
%div.18 = udiv <16 x i32> %div.17, %y
%div.19 = udiv <16 x i32> %div.18, %y
ret <16 x i32> %div.19
}
; CHECK-LABEL: define amdgpu_kernel void @caller_nohint
; CHECK-NOT: udiv
; CHECK: tail call <16 x i32> @div_nohint
; CHECK: ret void
define amdgpu_kernel void @caller_nohint(<16 x i32> addrspace(1)* nocapture %x, <16 x i32> addrspace(1)* nocapture readonly %y) {
entry:
%tmp = load <16 x i32>, <16 x i32> addrspace(1)* %x
%tmp1 = load <16 x i32>, <16 x i32> addrspace(1)* %y
%div.i = tail call <16 x i32> @div_nohint(<16 x i32> %tmp, <16 x i32> %tmp1)
store <16 x i32> %div.i, <16 x i32> addrspace(1)* %x
ret void
}
attributes #0 = { inlinehint }