User Details
- User Since
- Jan 28 2016, 8:30 AM (81 w, 3 d)
Recent Activity
Fri, Aug 11
Jul 21 2017
The implementation of this approach looks good to me. The only question is which way to go to implement v3 vector.
LGTM.
Jul 20 2017
Jun 27 2017
LGTM.
LGTM.
LGTM.
Jun 20 2017
LGTM.
LGTM.
LGTM, with nit.
May 26 2017
LGTM.
May 23 2017
May 22 2017
LGTM.
May 19 2017
fixed as per comments
May 18 2017
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
May 17 2017
May 16 2017
LGTM.
LGTM.
I'm a bit confused with all of these advance... but lets submit and improve this later.
May 15 2017
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.
This looks a bit as using carkeys to open a bottle. Should we have single instruction RP diff returning function without changing any state?
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()
May 12 2017
May 11 2017
LGTM.
Apr 26 2017
LGTM, Thanks!
LGTM.
Apr 25 2017
Looks good.
LGTM.
Apr 21 2017
Looks very good now! Thanks!
Apr 20 2017
Thank you for doing this! I really need it.
Apr 13 2017
Looks good.
Apr 12 2017
LGTM.
For the second assert it may be the case for iterator invalidation, though I haven't checked.
Apr 10 2017
Apr 7 2017
LGTM.
Apr 6 2017
Thanks Axel!
The following tests assert with this (and predecessors) patches with SISched turned on by default:
Apr 5 2017
Apr 4 2017
First part of comments related only to C++ issues
Mar 31 2017
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:
Mar 29 2017
LGTM.
Mostly good.
Mar 28 2017
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?