Test simplified

clang-format error corrected

Minor changes: comment corrected, class member name capitalized

In D149281#4329306, @fhahn wrote:

In D149281#4322079, @nikic wrote:

The question is why the vectorizer failed to unroll the loop in your workload.

The review title was changed to better reflect the changes being submitted.
The variable name was changed accordingly.
The test was added to check that

In D149281#4299934, @fhahn wrote:

In D149281#4299890, @rampitec wrote:

Add a test?

That would be helpful. It would be good to understand why runtime unrolling is needed here. Does the interleave heuristic not kick in?

@nikic, thanks a lot, I have cherry-picked that commits, which helps.
I suspected we were "lucky" enough to hit the middle of an actively developed feature.

This change caused a regression in AMDGPU backend.
In case optimization is done before inline, it covers opportunities for other inst-combines that may have higher precedence.
As a result, we get suboptimal code.

My idea is just to postpone the zero check folding before the compare argument, which is a call, is inlined.
If it is, the next inst-combine invocation will have a chance to do a better job, if not - it will do the same as before.

minor changes according to the reviewer request

schedule printing function moved out from the GCNSchedStage class.
objects passed by reference are marked as const

refactorerd getScheduleMetrics to avoid copying

amdgpu-schedule-metric-bias description updated

amdgpu-schedule-metric-bias=100

makes the scheduler always prefer the occupancy over the latency.
but the schedule metrics are still computed.
Did you mean the option to completely switch the getScheduleMetrics OFF?

In D139710#3998487, @rampitec wrote:

I still want to have an option to disable this heuristic.
Also are there any performance measurements done?

register class and move size from the virtual source register
operand type check is loosen to !isReg()

1. metric calculation calls localized to the UnclusteredHighRPStage::shouldRevertSchedule. Now the getScheduleMetric is only called if it is really necessary.
2. -amdgpu-schedule-metric-bias=<unsigned value> compiler option was added to ease the further testing and tuning. It defaults to 10 which means the schedule w/o bubbles gets 10 points reward.
3. Several other changes according to the reviewer request.

reviewers comments addressed

add operand instead of adding immediate

assign getImm() result to unsigned for implicit conversion

changed as requested

test renamed

test added

floating points changed to scaled integers

Fixed in https://reviews.llvm.org/D139874

In D139852#3989299, @JonChesterfield wrote:

In D139852#3989251, @alex-t wrote:

Since we ensure all the VGPR to SGPR copies are uniform, we just need to V_READFIRSTLANE_B32 here.

What ensures this copy is uniform?

Since we ensure all the VGPR to SGPR copies are uniform, we just need to V_READFIRSTLANE_B32 here.

Model map renamed to ReadyCycles

changes as requested

yet one more minor code cleanup

odd changes removed

removed changes not relevant directly to the current revision
added assert to check if the flat scratch offset is aligned

In D136169#3943669, @foad wrote:

There are still setIsDead changes and whitespace changes. Please try to strip them all out so we get a minimal patch to review.

removed unrelated changes

added test for using and restoring frame register for offset calculation.
S_SUBB_U32 changed to S_ADDC_U32 with "-Offset"

Since the https://reviews.llvm.org/D137574 has been landed, this review is updated to use backward PEI.

rebased

TargetRegisterInfo::eliminateFrameIndex signature changed to return "true" if the MachineInstr passed in by iterator was removed

In D137574#3925621, @arsenm wrote:

LGTM. Can you create follow up patches to start migrating all the other targets?

TargetRegisterInfo::supportsBackwardScavenger() description changed

Added test case for the frame index in the last instruction in the BB. "::llvm" removed.

In D134423#3914502, @foad wrote:

In D134423#3912577, @alex-t wrote:

BTW, if %5 is divergent we have a bug in ISel. We now should not have any V2S copy with the divergent source.

Look at the MIR that @skc7 quoted. %5 is divergent - it's copied from a vgpr function argument.

define <4 x i32> @extract0_bitcast_raw_buffer_load_v4i32(<4 x i32> inreg %rsrc, i32 %ofs, i32 %sofs) local_unnamed_addr #0 {
%var = tail call <4 x i32> @llvm.amdgcn.raw.buffer.load.v4i32(<4 x i32> %rsrc, i32 %ofs, i32 %sofs, i32 0)
ret <4 x i32> %var
}

IR dump after amdgpu-isel:

bb.0 (%ir-block.0):
liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4, $vgpr5

Common code for debug instructions frame index operand replacement is factored
out to separate function.

no else after return

Formatting

The first really working version of the frame index elimination with backward walk.

TargetRegisterInfo::eliminateFrameIndex invalidate iterator passed to it. It can remove the instruction or change the number of its operands.
I am thinking of changing its interface to make it return an iterator that points to the new/changed instruction to let caller handle this.

In D134423#3911947, @foad wrote:

In D134423#3911726, @skc7 wrote:

%8:sreg_32 = COPY %5:vgpr_32
%7:vgpr_32 = BUFFER_LOAD_DWORD_OFFEN %4:vgpr_32, killed %6:sgpr_128, %8:sreg_32, 0, 0, 0, 0, implicit $exec ::

I need more context. Is %5 uniform?

Temporary workaround to avoid SCC liveness scanning loop and using manually set
"dead" flags to decrease the amount of unnecessary SCC preserving code.

Sorry for my tediousness but
I would like to see any inspirational reason for this change.

Corrected wrong Frame Register restore code.
Several minor changes as requested.

In D136169#3872475, @rampitec wrote:

Am I correct to assume that frame offset is always DWORD aligned? If so we have couple spare bits:

S_ADDC_U32 $reg, offset
S_BITCMP1_B32 $reg, 0
S_BITSET0_B32 $reg, 0

Applied approach suggested by @rampitec

In fact, to avoid this unwanted loop I can use another hack - just forward scavenger one position to look up the SCC liveness at the insertion slot and then immediately backward scavenger to restore its position and avoid breaking the PEI logic.
This looks weird but avoids the loop.

SCC copy restore has been made wave size independent.