• Address comments

• Update argument comments

In D63215#1541086, @gchatelet wrote:

In D63215#1540160, @efriedma wrote:

What happens if findOptimalMemOpLowering fails? We have other ways of lowering memcpy/memset that could also violate the "one store per byte" rule.

Yes indeed, I need to take care of this as well.

In D63215#1540160, @efriedma wrote:

What happens if findOptimalMemOpLowering fails? We have other ways of lowering memcpy/memset that could also violate the "one store per byte" rule.

• Add documentation.
• Fix permissive HasNoRuntimeAttribute
• Mark interleave as disabled in the documentation.
• Use no-builtin instead of no-runtime-for.
• Adding an llvm.memcpy.inline intrinsic.
• Adding __builtin_memcpy_inline clang builtin.

• Use no-builtin instead of no-runtime-for.
• Use one attribute per runtime function to make merging easier.

AFAIU here is a coarse plan of what needs to happen

I'll submit this one shortly. @reames please let me know if you have any concerns.

In D61634#1502201, @tejohnson wrote:

Using function level attributes instead of module flags does provide finer grained control and avoids the conservativeness when merging IR for LTO. The downsides I see, mostly just in terms of the engineering effort to get this to work, are:

• need to prevent inlining with different attributes

In D61634#1500453, @efriedma wrote:

My main blocker is that I want to make sure we're moving in the right direction: towards LLVM IR with clear semantics, towards straightforward rules for writing freestanding C code, and towards solutions which behave appropriately for all targets. There's clearly a problem here that's worth solving, but I want to make sure we're adding small features that interact with existing features in an obvious way. The patch as written is adding a new attribute that changes the semantics of C and LLVM IR in a subtle way that interacts with existing optimizations and lowering in ways that are complex and hard to understand.

In D61634#1498376, @efriedma wrote:

I still think there are really two things you're trying to accomplish here, which should be handled separately.

1. Add a function attribute that works like -fno-builtin-memcpy currently does, to prevent the compiler from synthesizing calls to memcpy.
2. Provide a convenient way to write a small, fixed-length "memcpy", in C and in LLVM IR, that is always expanded to optimal straight-line code (without any calls or loops).
   
   These correspond to proposals (1) and (2) from your RFC; I think we need both to arrive at a reasonable final state.
   
   (The "rep; movs" is a third thing, which I think should also be handled separately, but it sounds like you don't think that's very important.)

In D61634#1494518, @alexandre.isoard wrote:

I'm not convinced by the approach.

Can it still recognize the loop idiom into memcpy implementation but use memmove (as only memcpy has been blacklisted)?

In D61634#1493927, @efriedma wrote:

I would be careful about trying to over-generalize here. There are a few different related bits of functionality which seem to be interesting, given the discussion in the llvm-dev thread, here, and in related patches:

In D61634#1493350, @theraven wrote:

I wonder if a list of comma-separated names is the right approach. Would it make more sense to add a new attribute for each of the helpers, such as #no-runtime-for-memcpy? That should make querying simpler (one lookup in the table, rather than a lookup and a string scan) and also make it easier to add and remove attributes (merging is now just a matter of trying to add all of them, with the existing logic to deduplicate repeated attributes working).

• Add documentation.
• Fix permissive HasNoRuntimeAttribute

• Addressing comments

• Addressing comments

• Turn this patch into an NFC.

• Fix typo.

• clang-format

Thx for the review @lebedev.ri !

• Remove cfi noise by adding nounwind

• Update documentation

• Fix typo

In D60719#1470632, @t.p.northover wrote:

IIUC freestanding environment should not rely on memcpy being present so my take on it was that by "fixing" freestanding I could have my cake and eat it too.

The formal situation is that freestanding implementations are only required to provide language support stuff like va_list. They're allowed to give more of the standard library if they want though, as implementation defined behaviour.

In practice, everyone I know provides the basic string.h functions and the compiler is pretty explicit about relying on them being present for correctness. They're part of the surrounding environment a user is expected to supply (much like the various exception handling callbacks if you want C++ exceptions, but always required).

For the people actually using freestanding I think they're mostly considered important enough that they're implemented in assembly anyway so this isn't really a burden, but...

In D60719#1469958, @t.p.northover wrote:

I think it'd be pretty unpopular with the people I know who use freestanding. They're mostly working on microcontrollers and compiling -Oz so the extra code size would be untenable; they also have memcpy implementations anyway because they use it in their own code.

If I was trying to solve this (and I'm also not 100% sure it needs solving), I think I'd instead generate a linkonce definition of memcpy whenever it's needed and leave CodeGen unmodified.

Add test.

In D60401#1462567, @lebedev.ri wrote:

In D60401#1462522, @gchatelet wrote:

...

So i agree in general, but i don't think i agree with fine print.

What if i don't want to validate the whole entirety of the instructions,
but only one of these instructions that is Measurable through another instruction?
I simply can't?

I understand the motivation behind this change but I think we need a more principled approach. I'll try to sum up my reasoning here.
To me there are two useful modes for llvm-exegesis:

• We want to look at a particular instruction latency - useful when we want to quickly check an assumption,
• We want to get the latency for all the instructions - useful to fully characterize or check a processor.

• Move comment to the right line.

Intermediate issue to solve: creating all these 2-instr chained configs must then also
create configs to measure the params of that second instr (without going into endless loop).

To me, a better approach would be to read all the experiments, create the dependency graph between the 2-instructions snippets and solve a system of equations to recover the per instruction latency, then use the analyzer on the result.

Can you explain that in a bit more detail? Something like

lat(i_0) = m_0
sum(lat(i_t)+lat(i_0)) = m_1
lat(i_1) = m_2
sum(lat(i_t)+lat(i_1)) = m_3
...
lat(i_n) => ?
sum(lat(i_t)+lat(i_n)) = m_n
lat(i_t) => ?

Do you suggest to take known lat(i_0)..lat(i_n) from measurements too?

It doesn't feel like the right approach to me.
llvm-exegesis is about relying on measurement to deduce informations. Here you're using a priori knowledge (SchedClass) which may be wrong.
To me, a better approach would be to read all the experiments, create the dependency graph between the 2-instructions snippets and solve a system of equations to recover the per instruction latency, then use the analyzer on the result.

In D59821#1444043, @craig.topper wrote:

What protects R8-R15 in 32-bit mode or SIL/DIL/BPL/SPL?

Thx!

In D56593#1393325, @jyknight wrote:

It'd be great to see this somewhat more widely publicized, outside of just the clang community. If libc implementors are aware of the gains and are willing to provide an actually-faster bcmp implementation, it'd be a lot better, than having this optimization that doesn't really optimize anything without users providing their own bcmp implementation.

In D57699#1386639, @lebedev.ri wrote:

I think i'm gonna land this.
It's semantically correct, has better perf characteristics, is a very small change, and is symmetrical with writeAsHex().

Thx for working on this Roman. I'm not the one who designed the YAML IO library so perhaps the author is a better reviewer.
LGTM AFAIC

In D53488#1307834, @aaron.ballman wrote:

LGTM!

• Removed redundant options in regression tests

• Fixing documentation.

Thank you for bearing with me @aaron.ballman.

• Addressing comments