[PATCH] D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor

mattias.v.eriksson@ericsson.com via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 1 05:21:05 PDT 2018


materi added a comment.

In https://reviews.llvm.org/D49671#1283621, @jonpa wrote:

> > I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?
>
> @materi: I think your ideas make sense. If you have a patch, could you post it, please?


What I meant was to have subtargets override the default latency calculation if they want to ignore operands that are not in the MCInstrDesc. That is, implement SytemZ::getOperandLatency() and have it return 0 in the cases where DefIdx or UseIdx is too large.

I don't have a patch for this and I really don't know which targets wants the new behavior.

> My first idea for this patch was to loop over the operands / read advances of the instruction in order to propagate read advances to the non-tablegen super-reg operands (see earlier patch proposal under 'History'). This is more arduous than the simply clearing those latencies during DAG construction, per what was committed. I think however this should work for you as well, or?
> 
> I wonder if it would be enough to make a rule to clear latencies only on *implicit* extra operands, and not on explicit ones? In other words if added *explicit* operands were left alone, this would not break anything? But I am not sure if this is possible with variadic instructions, or if it's a good idea...

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.


https://reviews.llvm.org/D49671





More information about the llvm-commits mailing list