[PATCH] D115602: [X86] Memset is lowered to rep stos if MinSize is present

Thu Dec 16 07:23:50 PST 2021

vdsered added a comment.

In D115602#3195463 <https://reviews.llvm.org/D115602#3195463>, @spatel wrote:

> I don't remember seeing this part of x86 codegen before, so adding some more reviewers. I'm also not sure what is recommended as the best perf x86 asm on recent CPUs. Should we be using this for "optsize" too?

Not sure too, but rep stos seems to be a good choice if we are directly asked for MinSize. I'd not add OptSize here as we will probably sacrifice performance in some cases. At least, I'd not do that in that patch.

1. There is a pull request for dotnet runtime with some similar problem Use xmm for stack prolog zeroing rather than rep stos <https://github.com/dotnet/runtime/pull/32538>. Seems like in general rep stos is not the best choice for performance.

2. Intel in their manual "F.8.3.7 Copy and String Copy" say to use specifically only rep stosb for performance or as it is written  "Software wishing to have a simple default string copy or store routine that will work well on a range of implementations (including future implementations) should consider using REP MOVSB or REP STOSB on implementations that support Enhanced REP MOVSB and STOSB".

3. On the other hand, I found that AMD says rep stos is in general less efficient, but this is mentioned in only two guides Software Optimization Guide for AMD Family 15h Processors, (see 9.3 Repeated String Instructions) <https://www.amd.com/system/files/TechDocs/47414_15h_sw_opt_guide.pdf> and Software Optimization Guide for AMD64 Processors (this one it too old and probably outdated) <http://support.amd.com/TechDocs/25112.PDF>. Plus, on the contrary to Intel, AMD recommends to use rep stos(b/w/d/q) to improve performance.

To sum up, I'd emit only rep stosb for Intel, if possible, and rep stos(b/w/d) for AMD when MinSize is present for now

Does it seem to be a good solution if we add the comment from @pengfei to this? Waiting for other opinions...

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115602/new/

https://reviews.llvm.org/D115602