NEC gather inefficiency

[raver119]

Here's sample code.
https://gist.github.com/raver119/3988237c9bb2376b0cd745120c5bc38e

Both gather and load functions do exactly the same, but since they use slightly different inner loop, compiler either uses gather or load instructions. This results in performance degradation.

Output on Aurora:

/opt/nec/ve/bin/nc++ -O3 -fopenmp bug_gather.cpp
Time gather: [210 us]; Time load: [60 us]

Output on x86:

 g++ -O3 -mmmx -msse -msse2 -msse3 -msse4.1 -msse4.2 -mavx -mavx2 -mfma -mf16c -mprefetchwt1 -fopenmp bug_gather.cpp 

Time gather: [209 us]; Time load: [215 us]

[raver119]

cc @mikishin ^^^

[mikishin]

This is the feedback from our development team.

VE has 2 types of instructions.

• VLD instrucion
  Vector instruction which loads the array of continuous or evenly spaced of elements
• VGT instruction
  Vector instruction which loads the array of other types of elements

In load(), VLD instructions are used.
However in gather(), VGT instructions are used because they are not
continuous or evenly spaced by the subscript expression [xStrd3? i3: 0].

In HW point of view, there is significient difference between VLD and VGT.
In case of store instruction, VE also has same 2 types of instructions.

If you have further question please let me know.

[raver119]

No, they are continuous in this particular case. In the test i've provided, this gather loop is equivalent of this load loop:

z2[i3] = OpType::op(x2[i3], y2[i3]);

so, all 3 subarrays are continuous. and zAxis3 == 128.

[raver119]

And besides of gather instruction performance, look:

We have this code with 2 ternary operators

z2[i3] = OpType::op(x2[xStrd3 ? i3 : 0], y2[yStrd3 ? i3 : 0]);

logically it can be turned into exactly 4 possible cases:

• xStrd > 0 and yStrd > 0
• xStrd > 0 and yStrd == 0
• xStrd ==0 and yStrd == 0 (however, this case is NOT possible here for real)
• xStrd == 0 and yStrd > 0

in all 4 cases compiler could turn into into load instruction with continuous memory access, either for both X and Y arrays or at least X/Y and 1 scalar operand.

And it would look exactly like the code I've provided:

               if(zStrd3 == 1 && xStrd3 == 1 && yStrd3 == 0)
                   for (uint32_t i3 = 0; i3 < zAxis3; ++i3)
                        z2[i3] = OpType::op(x2[i3], *y2);
                else if(zStrd3 == 1 && xStrd3 == 0 && yStrd3 == 1)
                    for (uint32_t i3 = 0; i3 < zAxis3; ++i3)
                        z2[i3] = OpType::op(*x2, y2[i3]);
                else if(zStrd3 == 1 && xStrd3 == 1 && yStrd3 == 1)
                    for (uint32_t i3 = 0; i3 < zAxis3; ++i3)
                        z2[i3] = OpType::op(x2[i3], y2[i3]);

[mikishin]

Oh, sorry I made misunderstood your original request. I will revert it if I get additional feedback from our development team.

[mikishin]

This is additional comment from our dev. team.

gather() loop is programmed like as follows.

 for (uint32_t i3 = 0; i3 < zAxis3; ++i3)
      z2[i3] = OpType::op(x2[xStrd3 ? i3 : 0], y2[yStrd3 ? i3 : 0])

This can be described in more detail in the subscript expression part of the array
like as follows.

 for (uint32_t i3 = 0; i3 < zAxis3; ++i3) {
    uint32_t idx;
    if (xStrd3 != 0) {
       idx = i3;
    }
    else {
       idx = 0;
    }
    z2[i3] = x2[idx] + y2[idx];  <-- OpType::op()は+
 }

The subscript expression of x2, y2 may show 0 in some places and may become
continuous or at equal intervals. That is why we programmed based on VGT inctruction
and load to memory.

If -O3 is specified without -fopenmp, move the above if statement out of the loop,
we move the above if statement for the external loop and optimise based on VGT instructions.
In case that -03 option is specified with -fopenmp, even if -03 is specified, we consider
the case that xStrd3 isthe shared data and may happen to be updated from other thread,
the optimization is not performed and the VGT instruction remains.

[raver119]

Not sure why xStrd3 is considerd modifiable, since compiler is able to see the source code of the method, but i've applied the following modifications to the original code:

int64_t gather(float *x, uint32_t *shapeX, uint32_t *stridesX, float *y, uint32_t *shapeY, uint32_t *stridesY, float *z, uint32_t *shapeZ, uint32_t *stridesZ) {
    auto timeStart = std::chrono::system_clock::now();

    const auto xStrd0 = stridesX[0];
    const auto xStrd1 = stridesX[1];
    const auto xStrd2 = stridesX[2];
    const auto xStrd3 = stridesX[3];

    const auto yStrd0 = stridesY[0];
    const auto yStrd1 = 0;
    const auto yStrd2 = stridesY[2];
    const auto yStrd3 = stridesY[3];

    const auto zStrd0 = stridesZ[0];
    const auto zStrd1 = stridesZ[1];
    const auto zStrd2 = stridesZ[2];
    const auto zStrd3 = stridesZ[3];

    const auto zAxis0 = shapeZ[0];
    const auto zAxis1 = shapeZ[1];
    const auto zAxis2 = shapeZ[2];
    const auto zAxis3 = shapeZ[3];

So, now compiler should be able to understand that xStrd3 and other vars there are not modifiable, and pretty much constant.

[raver119@aurora03 ~]$ /opt/nec/ve/bin/nc++ -O3 -fopenmp bug_gather.cpp 
...
Time gather: [207 us]; Time load: [60 us]