- User Since
- Jan 28 2016, 8:30 AM (116 w, 2 d)
Mar 5 2018
I think it looks very good, thank you!
Dec 27 2017
Dec 5 2017
In general adding "custom" code to SelectionDAGBuilder::setValue looks odd. Instead I would add a target-customizable postprocessing loop on pairs of Value <-> SDNode into SelectionDAGISel::SelectBasicBlock right after the DAG is created. The target hook should be able to get whatever it requires LLVM IR analisys and annotate SDNodes.
Nov 20 2017
Nov 17 2017
Added test. It turns out getOccupancyWithLocalMemSize function doesn't account for amdgpu-waves-per-eu attribute so I added call to getWavesPerEU (which does).
Nov 16 2017
Nov 13 2017
fixed per review issues.
Nov 10 2017
Sep 19 2017
Sep 12 2017
SIInstrInfo::doMemOpsHaveSameBasePtr looks generic enough to be default indeed but should we commit this first and then make doMemOpsHaveSameBasePtr default so we could rollback to this one in case of severe regressions? LGTM by the way.
Sep 8 2017
Wow! I wish I had this before! LGTM.
Sep 6 2017
LGTM. However it looks like we should fix TD to follow single naming convention.
Sep 4 2017
Sep 1 2017
Ok, unmangled part looks different indeed. If the issue with pre-link checking is solved this patch is ok with me.
Splitting AMDGPULibFunc in two classes looks a huge overkill. How about modifying AMDGPULibFunc::parse so it could accept unmangled names and just return an enum id for the function (using some fast lookup approach)? Type info for such functions can be left unpopulated and supposed to be handled by the client (as in fold_read_write_pipe).
Aug 28 2017
Aug 11 2017
Jul 21 2017
The implementation of this approach looks good to me. The only question is which way to go to implement v3 vector.
Jul 20 2017
Jun 27 2017
Jun 20 2017
LGTM, with nit.
May 26 2017
May 23 2017
May 22 2017
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
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
Apr 26 2017
Apr 25 2017
Apr 21 2017
Looks very good now! Thanks!
Apr 20 2017
Thank you for doing this! I really need it.
Apr 13 2017
Apr 12 2017
For the second assert it may be the case for iterator invalidation, though I haven't checked.
Apr 10 2017
Apr 7 2017
Apr 6 2017
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?
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: