v_float32x16 ctor must be used instead of native intrinsics.

Outdated

typename T
v_uint16::nlanes

Code should be consistent.
Don't use assumptions in generic implementation (especially silently).

The point is that this function handles cases when data is of unsigned short type and when data is of signed short type. In both cases nlanes is one and the same. nlanes = ength vector in bits / number bits in types. For this case 128(SSE42)/16 = 8. So for both types U16 and S16 nlanes = 8 for SSE42. So there is no particular need to separate two these cases.

So, we are expecting T=ushort or T=short here; in this case, maybe it would be better to explicitly check that by asserts, smth like:

bool isShort = std::is_same<T, ushort>::value || std::is_same<T, short>::value;
GAPI_Assert(isShort == true);

This also should be applied to absdiffc_simd_c3_impl, I think. There is the same issue

Don't use assumptions in generic implementation (especially silently).

Changed.

Avoid declarations of multiple vars at once:

• debugger is not able to show the right statement if this code goes out of buffer range

Done.

vx_load_expand_q
vx_load_expand_q
vx_load_expand_q
vx_load_expand_q

Reduce pressure on CPU's LOAD units. Fetched memory is equal to vx_load(in + x).
Load v_uint8 first and then repack in registers.

I didn't quite understand your proposal. could you please clarify your idea?

Replace 4 load instructions to one.

But I need initialize 4 vectors for further work with them. How can I load four vectors with one vx_load call?

@terfendail Could you please comment or clarify Alexander's proposal? How will Alexander's approach affect performance?

I think Alexander means something like

v_uint16 ld0, ld1;
v_expand(vx_load(in+x), ld0, ld1);
v_float32 a1 = v_cvt_f32(v_expand_low(ld0));
v_float32 a2 = v_cvt_f32(v_expand_high(ld0));
v_float32 a3 = v_cvt_f32(v_expand_low(ld1));
v_float32 a4 = v_cvt_f32(v_expand_high(ld1));

@terfendail Ok. Thank you so much for clarification!

I think Alexander means something like

v_uint16 ld0, ld1;
v_expand(vx_load(in+x), ld0, ld1);
v_float32 a1 = v_cvt_f32(v_expand_low(ld0));
v_float32 a2 = v_cvt_f32(v_expand_high(ld0));
v_float32 a3 = v_cvt_f32(v_expand_low(ld1));
v_float32 a4 = v_cvt_f32(v_expand_high(ld1));

@alalek I applied your proposal for 8U and gather performance report for AVX512 vectors. I observed average performance degradation equals to 12.6%. For 8UC3 test cases performance degradation is up to 33.3%. So I wouldn't like to apply this proposal to my snippet. Please take a look at the comparative performance report: //cc @dmatveev

vx_load_expand_q_vs_v_expand_high_low.xlsx

You can see applied proposal in the "Performance experiment" commit.

I think it would be better to extend init array to v_float::nlanes +2 and than just load
s1 =vx_load(init+0)
For 2 and 4 channels you could use the same approach or try vx_lut_pairs/vx_lut_quads(scalar, vx_setzero_s32()) whatever show better performance

agreed about simplifying/minimization of initialization code (no real performance impact)

I think it would be better to extend init array to v_float::nlanes +2 and than just load
s1 =vx_load(init+0)

Thanks for advice. I'll try.

For 2 and 4 channels you could use the same approach or try vx_lut_pairs/vx_lut_quads(scalar, vx_setzero_s32()) whatever show better performance

try vx_lut_pairs/vx_lut_quads(scalar, vx_setzero_s32())

It is not so good idea. vx_lut_pairs() calls (pefix)_i32gather_epi64() intrinsic that has latency equals to about 25. For comparison, vx_load() has the latency equals to 7.

And vx_lut_quads() has summary latency about equals to 33 when the latency of the vx_load() equals to 7 .

You could use v_cvt_f32(v_reinterpret_as_s32(vx_load_expand(in + x))) and avoid defining v_cvt_f32 for uint32

Done.

No need to obfuscate code:

-*(scalar + i % chan)
+scalar[i % chan]

Outdated.

+ 2

Why?

As I've already written in the post above, that loading to each next coefficient vector occurs with an offset:

v_float32 s1 = vx_load(init);

#if CV_SIMD_WIDTH == 32
    v_float32 s2 = vx_load(init + 2);
    v_float32 s3 = vx_load(init + 1);
#else
    v_float32 s2 = vx_load(init + 1);
    v_float32 s3 = vx_load(init + 2); 
#endif

Maximal offset is 2.
Also @terfendail has already write about it here

Size of vector equals to nlanes. If loading start at second element of init array, then it'll finish at nlanes+2 element of init.

There is no such code in this function.

Ok. It's a typo.

Outdated.

@dmatveev AFAIK, Fluid backend performs per-row processing.
So it make sense to implement support for initializer code of such constants.

Ok. Scratch buffer was applied.

Is there CV_StaticAssert() support in standalone mode? IE?

Changed to static_assert()

No need to perform hand-made registers spilling. Compilers are smart enough and can do that for you if necessary (moreover AVX512 has up to 32 vector registers)

This data is:

• loaded once
• used once

Move data loading to corresponding places.

@alalek Could you please clarify what you mean under hand-made registers spilling? If you mean v_load_f32(), it isn't hand-made registers spilling. It is just an overloaded function for ease of writing templates.
If you mean for loop, for initialization 12 vectors- are you sure that you want to see 12 load lines instead of one?
I don't quite understand the essence of your request. Please clarify.

1. Data is loaded from the memory.
2. On the same line data is stored back to the memory.
3. Data re-loaded later once again for processing.

Do you see here redundant steps?

P.S. No need to load all 12 SIMD vectors at once. Load data on demand.

P.S. No need to load all 12 SIMD vectors at once. Load data on demand.

It's necessary because of specificities of the algorithm.

No need to perform hand-made registers spilling. Compilers are smart enough and can do that for you if necessary (moreover AVX512 has up to 32 vector registers)

This data is:

• loaded once
• used once

Move data loading to corresponding places.

Reworked.

Great 👍