PerfScore inconcistencies with memory accesses

[pentp]

In getInsExecutionCharacteristics there are three different ways used to calculate memory access latencies, used seemingly at random (except for special instructions like mov)

• result.insLatency += ins_latency; - this seems the most logical option, e.g. INS_addsd
• result.insLatency = max(ins_latency, result.insLatency); - looks incorrect to me, e.g. INS_add
• result.insLatency not set if memAccessKind != PERFSCORE_MEMORY_NONE - incorrect, e.g. INS_aesdec

And a related question for insEvaluateExecutionCost - if memAccessKind == PERFSCORE_MEMORY_WRITE it reduces the apparent latency by 3 for writes, but shouldn't it do the same for PERFSCORE_MEMORY_READ_WRITE? Or is the assumption here that for RW instructions it's likely that another R/RW instruction follows for the same memory location?

Asking because I fixed some latency values (#49620) but then noticed the inconsistent logic.

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[JulieLeeMSFT]

cc @AndyAyersMS

[briansull]

I will respond to @pentp questions

[AndyAyersMS]

Unassigning myself for now. Brian, feel free to reassign to me if you need to.

[briansull]

To accurately model the memory latency we would have to keep track of all register/memory defs and register/memory uses
and also simulate the instruction pipeline. For a Memory Write we assume that the memory location just written won't get read back in the next three cycles, Thus we reduce the latency by three. For Memory reads we use an estimate of a three cycle latency.

Thus for Read_Write we must treat it the same way as a read, because we will have to wait three cycles to load the data from memory before processing the data.

See emitter::insEvaluateExecutionCost(instrDesc* id)

        // We assume that we won't read back from memory for the next WR_GENERAL cycles
        // Thus we normally won't pay latency costs for writes.

and

    else if (latency >= 1.0) // Otherwise, If we aren't performing a memory write
    {
        // We assume that the processor's speculation will typically eliminate one cycle of latency
        //
        latency -= 1.0;
    }

When I used the max(ins_latency, result.insLatency) form to set the latency it is because of the numbers found for these instructions were found in https://www.agner.org/optimize/instruction_tables.pdf or https://uops.info/table.html
The Intel processor does speed up several instructions

These are all just estimates and don't need to be exactly correct. They are only used to help the JIT generate generally faster code.

For example if we remove a memory load from inside of a loop and place itr outside of the loop in the preheader that will increase the performance (lower PerfScore) of the instructions inside the loop.