LGTM.

LGTM.

fixed as per comments

In D33289#758647, @rampitec wrote:

In D33289#758351, @vpykhtin wrote:

I inserted MachineInstrRegs between reset and recede, all functions are in old places. I should move MachineInstrRegs higher

I see it now, thanks. Can you move it higher please?

I agree if this is a bugfix.

I inserted MachineInstrRegs between reset and recede, all functions are in old places. I should move MachineInstrRegs higher

LGTM.

LGTM.

I'm a bit confused with all of these advance... but lets submit and improve this later.

You can do the reset with move - this would not require copy.

I really don't understand it all. Why not just have reset(MachineInstr *, LiveRegSet ) and do a reset on the next BB with liveset from previous BB? It looks like your code does it. Another question: your reset does skip debugs, but the caller doesn't know about it and can supply unskipped iterator to the next advance.

In D33105#755186, @rampitec wrote:

In D33105#755182, @vpykhtin wrote:

This looks a bit as using carkeys to open a bottle. Should we have single instruction RP diff returning function without changing any state?

We probably should, but it probably does not belong to this change.

This looks a bit as using carkeys to open a bottle. Should we have single instruction RP diff returning function without changing any state?

In D33105#755142, @rampitec wrote:

In D33105#754976, @vpykhtin wrote:

If I don't mistake downward tracker cannot work on arbitrary instruction order, so let's change it's interface so that it tracks current instruction and move it, likewise like llvm standard tracker, that is reset(MI), advance()

In general it cannot work on an arbitrary order, but it can be used as a probe to schedule next arbitrary instruction and then reset to the previous state.
Then in D33117 I'm using advanceBefore to cross basic block boundary and I do not really want to slow down advance method by checking for the iterator end condition which is needed relatively seldom.

If I don't mistake downward tracker cannot work on arbitrary instruction order, so let's change it's interface so that it tracks current instruction and move it, likewise like llvm standard tracker, that is reset(MI), advance()

LGTM.

LGTM, Thanks!

LGTM.

Looks good.

LGTM.

LGTM.

Looks very good now! Thanks!

Thank you for doing this! I really need it.

Looks good.

LGTM.

For the second assert it may be the case for iterator invalidation, though I haven't checked.

LGTM.

LGTM.

LGTM.

LGTM.

LGTM.

LGTM.

LGTM.

Thanks Axel!

The following tests assert with this (and predecessors) patches with SISched turned on by default:

LGTM.

First part of comments related only to C++ issues

If SU(i) uses register produced by SI(j):

I may miss something, but it looks that you can build data edges when building a superdag consisting of blocks. Incoming data edges would be liveins, outcoming - liveouts.

In general, I think moving instructions just to use standard RP tracker to discover liveins/liveouts isn't a good idea. It isn't only slow but doesn't look reliable too. Why not discover these sets using DAG directly?

Stack trace:

I didn't debugged it and I don't know why you decided so.

I ran lit tests with sished with ShouldTrackLaneMasks=true enabled by default with this patch, the following tests asserted:

LGTM.

LGTM.

LGTM.

Mostly good.

Axel, thanks for the update.

This looks elegant, mine was longer :), thanks!

Ok, I understood SIScheduleBlockCreator::scheduleInsideBlocks() moves instructions to actually get LiveIn and LiveOut set for a block, but this is rather heavy. Have you thought about getting those using DAG directly, not regpressure tracker? By the common sence the dependencies between blocks correspond to that liveness info. There is a problem however: LiveIn and LiveOut dependencies aren't modelled for boundary SUs. I have local patch that build such dependencies - scheduling region LiveIns edges comes from EntrySU, LiveOut - to ExitSU. Another problem - dependency edges doesn't have lanemask, need to think how to deal with this.

Why SIScheduleBlockCreator::scheduleInsideBlocks() actually move instructions? Why it isn't done on the final scheduling?

Sorry but I give up reviewing this. Your code looks like after "inline all" pass. Can you refactor common parts into functions with meaningfull names?

In D30148#686726, @axeldavy wrote:

In D30148#682960, @arsenm wrote:

It's still missing a few. VINTRP and EXP. I think it would be better to check instruction latency data. In absence of that, I think it would be simpler to just use isHighLatencyInstruction, and !isHighLatencyInstruction

The true usage of these helper functions is to deduce instructions which need a VMCNT wait or a LGKMCNT wait (former ones being incorrectly called 'high latency' and the latter ones 'lon latency').

What about renaming to isVMCNTInstruction and isLGKMCNTInstruction ?

LGTM.

LGTM.

LGTM, Thanks!

LGTM.

LGTM.