On 18/07/16 12:57, Dave Gordon wrote:
> On 18/07/16 12:35, Chris Wilson wrote:
>> On Mon, Jul 18, 2016 at 12:15:32PM +0100, Tvrtko Ursulin wrote:
>>> I am not sure about this, but looking at the raid6 for example, it
>>> has a lot more annotations in cases like this.
>>>
>>> It seems to be telling the compiler which memory ranges does each
>>> instruction access, and also uses "asm volatile" - whether or not
>>> that is really needed I don't know.
>>>
>>> For example:
>>> asm volatile("movdqa %0,%%xmm4" :: "m" (dptr[z0][d]));
>>>
>>> And:
>>> asm volatile("movdqa %%xmm4,%0" : "=m" (q[d]));
>>>
>>> Each one is telling the compiler the instruction is either reading
>>> or writing respectively from a certain memory address.
>>>
>>> You don't have any of that, and don't even specify nothing as an
>>> output parameter so I am not sure if your code is safe.
>>
>> The asm is correct. We do not modify either of the two pointers which we
>> pass in via register inputs, but the memory behind them - hence the
>> memory
>> clobber.
>
> This is a choice of how much we let the compiler decide about
> addressing, and how much we tell it about what the asm code really does.
> The examples above get the compiler to generate *any* suitable
> addressing mode for each specific location involved in the transfers, so
> the compiler knows a lot about what's happening and can track where each
> datum comes from and goes to.
>
> OTOH Chris' code
>
> + asm("movntdqa (%0), %%xmm0\n"
> + "movntdqa 16(%0), %%xmm1\n"
> + "movntdqa 32(%0), %%xmm2\n"
> + "movntdqa 48(%0), %%xmm3\n"
> + "movaps %%xmm0, (%1)\n"
> + "movaps %%xmm1, 16(%1)\n"
> + "movaps %%xmm2, 32(%1)\n"
> + "movaps %%xmm3, 48(%1)\n"
> + :: "r" (src), "r" (dst) : "memory");
>
> - doesn't need "volatile" because asm statements that have no output
> operands are implicitly volatile.
>
> - makes the compiler give us the source and destination *addresses* in a
> register each; beyond that, it doesn't know what we're doing with them,
> so the third ("clobbers") parameter has to say "memory" i.e. treat *all*
> memory contents as unknown after this.
>
> [[From GCC docs: The "memory" clobber tells the compiler that the
> assembly code performs memory reads or writes to items other than those
> listed in the input and output operands (for example, accessing the
> memory pointed to by one of the input parameters). To ensure memory
> contains correct values, GCC may need to flush specific register values
> to memory before executing the asm. Further, the compiler does not
> assume that any values read from memory before an asm remain unchanged
> after that asm; it reloads them as needed. Using the "memory" clobber
> effectively forms a read/write memory barrier for the compiler.]]
>
> BTW, should we not tell it we've *also* clobbered %xmm[0-3]?
>
> So they're both correct, just taking different approaches. I don't know
> which would give the best performance for this specific case.
Cool, learn something new every day. :)
I've tried writing it as:
struct qw2 {
u64 q[2];
} __attribute__((packed));
static void __memcpy_ntdqa(struct qw2 *dst, const struct qw2 *src, unsigned long len)
{
kernel_fpu_begin();
len >>= 4;
while (len >= 4) {
asm("movntdqa (%0), %%xmm0" :: "r" (src), "m" (src[0]));
asm("movntdqa 16(%0), %%xmm1" :: "r" (src), "m" (src[1]));
asm("movntdqa 32(%0), %%xmm2" :: "r" (src), "m" (src[2]));
asm("movntdqa 48(%0), %%xmm3" :: "r" (src), "m" (src[3]));
asm("movaps %%xmm0, (%1)" : "=m" (dst[0]) : "r" (dst));
asm("movaps %%xmm1, 16(%1)" : "=m" (dst[1]) : "r" (dst));
asm("movaps %%xmm2, 32(%1)" : "=m" (dst[2]) : "r" (dst));
asm("movaps %%xmm3, 48(%1)" : "=m" (dst[3]) : "r" (dst));
src += 4;
dst += 4;
len -= 4;
}
while (len--) {
asm("movntdqa (%0), %%xmm0" :: "r" (src), "m" (src[0]));
asm("movaps %%xmm0, (%1)" : "=m" (dst[0]) : "r" (dst));
src++;
dst++;
}
kernel_fpu_end();
}
That appears to allow GCC to interleave SSE and normal instructions,
presumably that means it is trying to utilize the execution units better?
I wonder if it makes a difference in speed?
Old code main loop assembly looks like:
58: 66 0f 38 2a 00 movntdqa (%rax),%xmm0
5d: 66 0f 38 2a 48 10 movntdqa 0x10(%rax),%xmm1
63: 66 0f 38 2a 50 20 movntdqa 0x20(%rax),%xmm2
69: 66 0f 38 2a 58 30 movntdqa 0x30(%rax),%xmm3
6f: 0f 29 01 movaps %xmm0,(%rcx)
72: 0f 29 49 10 movaps %xmm1,0x10(%rcx)
76: 0f 29 51 20 movaps %xmm2,0x20(%rcx)
7a: 0f 29 59 30 movaps %xmm3,0x30(%rcx)
7e: 49 83 e8 04 sub $0x4,%r8
82: 48 83 c0 40 add $0x40,%rax
86: 48 83 c1 40 add $0x40,%rcx
8a: 49 83 f8 03 cmp $0x3,%r8
8e: 77 c8 ja 58 <i915_memcpy_from_wc+0x58>
While the above version generates:
58: 66 0f 38 2a 00 movntdqa (%rax),%xmm0
5d: 66 0f 38 2a 48 10 movntdqa 0x10(%rax),%xmm1
63: 66 0f 38 2a 50 20 movntdqa 0x20(%rax),%xmm2
69: 66 0f 38 2a 58 30 movntdqa 0x30(%rax),%xmm3
6f: 49 83 e8 04 sub $0x4,%r8
73: 48 83 c0 40 add $0x40,%rax
77: 0f 29 01 movaps %xmm0,(%rcx)
7a: 0f 29 49 10 movaps %xmm1,0x10(%rcx)
7e: 0f 29 51 20 movaps %xmm2,0x20(%rcx)
82: 0f 29 59 30 movaps %xmm3,0x30(%rcx)
86: 48 83 c1 40 add $0x40,%rcx
8a: 49 83 f8 03 cmp $0x3,%r8
8e: 77 c8 ja 58 <i915_memcpy_from_wc+0x58>
Interestingly, in both cases GCC does some in my mind futile
shuffling aroung between the two loops. Instead of just
carrying on with src and dst and len how they are, it goes
to use a different register set for the second loop:
So this reshuffling:
90: 48 8d 42 fc lea -0x4(%rdx),%rax
94: 83 e2 03 and $0x3,%edx
97: 48 c1 e8 02 shr $0x2,%rax
9b: 48 83 c0 01 add $0x1,%rax
9f: 48 c1 e0 06 shl $0x6,%rax
a3: 48 01 c6 add %rax,%rsi
a6: 48 01 c7 add %rax,%rdi
a9: 48 8d 42 ff lea -0x1(%rdx),%rax
ad: 48 85 d2 test %rdx,%rdx
b0: 74 1a je cc <i915_memcpy_from_wc+0xcc>
And then the second loop:
b2: 66 0f 38 2a 06 movntdqa (%rsi),%xmm0
b7: 48 83 e8 01 sub $0x1,%rax
bb: 48 83 c6 10 add $0x10,%rsi
bf: 0f 29 07 movaps %xmm0,(%rdi)
c2: 48 83 c7 10 add $0x10,%rdi
c6: 48 83 f8 ff cmp $0xffffffffffffffff,%rax
ca: 75 e6 jne b2 <i915_memcpy_from_wc+0xb2>
Any thoughts on this?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
