[PATCH] D55263: [CodeGen][ExpandMemcmp] Add an option for allowing overlapping loads.

David Zarzycki via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 11 03:59:38 PST 2018


davezarzycki added a comment.

In D55263#1325390 <https://reviews.llvm.org/D55263#1325390>, @spatel wrote:

> In D55263#1325283 <https://reviews.llvm.org/D55263#1325283>, @davezarzycki wrote:
>
> > In D55263#1320043 <https://reviews.llvm.org/D55263#1320043>, @spatel wrote:
> >
> > > I just looked over the codegen changes so far, but I want to add some more knowledgeable x86 hackers to have a look too. There are 2 concerns:
> > >
> > > 1. Are there any known uarch problems with overlapping loads?
> > > 2. Are there any known uarch problems with unaligned accesses (either scalar or SSE)?
> > >
> > >   If there's any perf data (either nano-benchmarks or full apps) to support the changes, that would be nice to see. This reminds me of PR33329: https://bugs.llvm.org/show_bug.cgi?id=33329 (can we close that now?)
> >
> >
> > Let's not lose sight of the big picture here. If uarch problems exist, are they *worse* than the cost of calling memcmp()? In other words, is the likely register spills, function call overhead, and dynamic algorithm selection (given the constantness of the size parameter is lost) worth it?
>
>
> No real argument from me - I just wanted to be sure that nobody knew of some pre-existing uarch disaster potential. The change should be a nice win for the general x86 case. I've just added some nits in the inline comments.


Great. For whatever it may be worth, I'd wager that the micro-benchmark doesn't simulate branch prediction failures and therefore the performance win is higher in practice.

One of the challenges that is often overlooked with `memcmp` and `memcpy` (or any design with dynamic algorithm selection based on inputs) is that in the real world, inputs are often random from one call to the next, and therefore the algorithm selection branches are unpredictable.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55263





More information about the llvm-commits mailing list