AArch64: uses store from GP reg where vectorized reg would be better

[MatzeB]

Repro: https://godbolt.org/z/9bW58ax69

#include <string.h>
#include <arm_neon.h>
#include <arm_sve.h>

void foobar(uint16_t *p, uint8_t *p2, uint64x2_t vec, int x) {
  vec += (uint64x2_t){3, 4};
  if (x) {
    asm volatile(""); // side-effect so conditional move cannot be used.
    vec |= (uint64x2_t){0x12, 0x34};
    *p2 = vec[1];
    *p = vec[0];
  } else {
    // Commenting out the next two instructions will improve code in the "then" branch!
    vec ^= (uint64x2_t){0x34, 0x12};
    *p = vec[0];
  }
}

// clang++ -target aarch64-redhat-linux-gnu -O3 -S -o - test.cpp -march=armv9-a+sve2+fp16

https://godbolt.org/z/41MoofTzP

The compiler ends up compiling the "then" part of the branch as:

...
       umov    w8, v0.h[0]
       st1     { v0.b }[8], [x1]
       strh    w8, [x0]

while this could be done without going throgh the w8 GP register:

       st1     { v0.b }[8], [x1]
       str     h0, [x0]

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Matthias Braun (MatzeB)

Repro: ``` #include <string.h> #include <arm_neon.h> #include <arm_sve.h>

void foobar(uint16_t *p, uint8_t *p2, uint64x2_t vec, int x) {
vec += (uint64x2_t){3, 4};
if (x) {
asm volatile(""); // side-effect so conditional move cannot be used.
vec |= (uint64x2_t){0x12, 0x34};
*p2 = vec[1];
*p = vec[0];
} else {
// Commenting out the next two instructions will improve code in the "then" branch!
vec ^= (uint64x2_t){0x34, 0x12};
*p = vec[0];
}
}

// clang++ -target aarch64-redhat-linux-gnu -O3 -S -o - test.cpp -march=armv9-a+sve2+fp16

The compiler ends up compiling the "then" part of the branch as:

...
umov w8, v0.h[0]
st1 { v0.b }[8], [x1]
strh w8, [x0]

while this could be done without going throgh the w8 GP register:

   st1     { v0.b }[8], [x1]
   str     h0, [x0]

</details>

[MatzeB]

(this mirrors meta T220660269)

[MatzeB]

At a first glance this seems to be an unfortunate combination of events:

• At ISel time the code is normalized/optimized and has optimized + CSEd into a single store behind the if
• Though selecting between vector and GPR "regbank" is tricky and may even look different depending on what path is looked at.
• Later tailduplication duplicates the common store back to the then/else branch (and aarch64 backend currently has no mechanism to reverse/redo the choice between GPR vector reg).

(FWIW global-isel is having similar trouble, having G_XOR coming from one branch and G_EXTRACT_VECTOR_ELT from the other choosing a GPR regbank as far as I can tell).

So not sure if there is an easy fix here...

[efriedma-quic]

Taildup is happening way too late to interact with anything else. So pretend it's not happening.

Given that context, I see the following possible solutions:

• Some sort of reverse-gvn pass to duplicate the store. But that seems way too niche to really be practical?
• Be less aggressive with turning vector ops into integer ops, on the assumption that people have a good reason to explicitly write vector ops. If we do the xor in a vector register, the rest works out.