[AArch64] Consider all vector types for FeatureSlowMisaligned128Store

[evandro]

Summary

The original code considered only v2i64 as slow for this feature. This patch consider all 128-bit long vector types as slow candidates.

It's not clear why Cyclone, the first processor to use this feature, restricted it to just v2i64 and excluded the other 128-bit vector types. But, in internal tests, extending this feature to all 128-bit vector types resulted in an overall improvement of 1% on Exynos M1.

[mssimpso]

This makes sense to me, but it's probably a good idea to have someone with Cyclone knowledge comment.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

471 ↗

(On Diff #82137)

Would it make sense to check the legalized type here? Something like LT.second.is128BitVector()?

[rengolin]

This makes sense. LGTM. Thanks!

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

471 ↗

(On Diff #82137)

I'm not sure the type would make any difference on the store. All that matters is that this is a 128-bit store and the required alignment is larger than that. The previous conditional was weird, I think.

[evandro]

I'll wait a few more days for a Cyclone maintainer to comment before committing this patch.

[mssimpso]

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

471 ↗

(On Diff #82137)

I guess I was thinking about the case where Src is something like a <4 x i64>. I believe getPrimativeSizeInBits would be 256. But this should get legalized into two 128 bit stores. So isn't the size of the legalized type what matters?

[evandro]

No comments on how this change could affect Cyclone. Does anyone object if I commit this patch in 12h?

[mssimpso]

Evandro,

Unless I'm missing something, I still think the size of the legalized type is what matters. Do you disagree? Also, please add a test case for the <4 x i64> store I mentioned previously, so we know we have the right cost.

[evandro]

In D27998#637583, @mssimpso wrote:

... Also, please add a test case for the <4 x i64> store I mentioned previously, so we know we have the right cost.

Will do.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

471 ↗

(On Diff #82137)

I believe that at this point the type is already legalized, but I'll double check it.

[mssimpso]

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

471 ↗

(On Diff #82137)

LT.second is the legalized version of Src. As I previously commented, you should just use LT.second.is128BitVector(). The cost of <4 x i64> should be 24. LT.first is the legalization cost (the number of stores, 2 in this case). So 2*2*6 = 24.

[ab]

Digging around, here's my summary of the various comments:

• with FeatureSlowMisaligned128Store, 128-bit misaligned stores are split because they're slow
• v2i64 is special because it's generated by memcpy lowering; splitting was disabled for that type because it turned out to cause regressions
• to minimize the amount of other (non-memcpy) misaligned v2i64 stores, the cost model was taught to penalize them heavily; otherwise, these aren't split and cause slowdowns.

From that perspective, penalizing other 128-bit types sounds reasonable to me, but I don't have any concrete cyclone numbers.

[evandro]

Thanks for digging it up, @ab.

[mssimpso]

Hey Evandro,

It looks like you may have forgotten the test case for unlegalized types (e.g., <4 x i64>).

[evandro]

Included 256-bit vector types in the test.

[mssimpso]

LGTM now. Thanks!

llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

if (SelCondTy.isSimple() && SelValTy.isSimple()) {
                                                      SelCondTy.getSimpleVT(),
                                                      SelValTy.getSimpleVT()))
         return Entry->Cost;
     }
   }
   return BaseT::getCmpSelInstrCost(Opcode, ValTy, CondTy);
 }
 
-int AArch64TTIImpl::getMemoryOpCost(unsigned Opcode, Type *Src,
+int AArch64TTIImpl::getMemoryOpCost(unsigned Opcode, Type *Ty,
                                     unsigned Alignment, unsigned AddressSpace) {
-  std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, Src);
+  auto LT = TLI->getTypeLegalizationCost(DL, Ty);
 
   if (ST->isMisaligned128StoreSlow() && Opcode == Instruction::Store &&
-      Src->isVectorTy() && Alignment != 16 &&
-      Src->getVectorElementType()->isIntegerTy(64)) {
-    // Unaligned stores are extremely inefficient. We don't split
-    // unaligned v2i64 stores because the negative impact that has shown in
-    // practice on inlined memcpy code.
-    // We make v2i64 stores expensive so that we will only vectorize if there
+      LT.second.is128BitVector() && Alignment < 16) {
+    // Unaligned stores are extremely inefficient. We don't split all
+    // unaligned 128-bit stores because the negative impact that has shown in
+    // practice on inlined block copy code.
+    // We make such stores expensive so that we will only vectorize if there
     // are 6 other instructions getting vectorized.
-    int AmortizationCost = 6;
+    const int AmortizationCost = 6;
 
     return LT.first * 2 * AmortizationCost;
   }
 
-  if (Src->isVectorTy() && Src->getVectorElementType()->isIntegerTy(8) &&
-      Src->getVectorNumElements() < 8) {
+  if (Ty->isVectorTy() && Ty->getVectorElementType()->isIntegerTy(8) &&
+      Ty->getVectorNumElements() < 8) {
     // We scalarize the loads/stores because there is not v.4b register and we
     // have to promote the elements to v.4h.
-    unsigned NumVecElts = Src->getVectorNumElements();
+    unsigned NumVecElts = Ty->getVectorNumElements();
     unsigned NumVectorizableInstsToAmortize = NumVecElts * 2;
     // We generate 2 instructions per vector element.
     return NumVectorizableInstsToAmortize * NumVecElts * 2;
   }
 
   return LT.first;
 }
 

llvm/trunk/test/Analysis/CostModel/AArch64/store.ll

-; RUN: opt < %s  -cost-model -analyze -mtriple=aarch64-apple-ios | FileCheck %s
-; RUN: opt < %s  -cost-model -analyze -mtriple=aarch64-apple-ios -mattr=slow-misaligned-128store | FileCheck %s --check-prefix=SLOW_MISALIGNED_128_STORE
+; RUN: opt < %s -cost-model -analyze -mtriple=aarch64-unknown | FileCheck %s
+; RUN: opt < %s -cost-model -analyze -mtriple=aarch64-unknown -mattr=slow-misaligned-128store | FileCheck %s --check-prefix=SLOW_MISALIGNED_128_STORE
 
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32-S32"
 ; CHECK-LABEL: getMemoryOpCost
 ; SLOW_MISALIGNED_128_STORE-LABEL: getMemoryOpCost
 define void @getMemoryOpCost() {
-    ; If FeatureSlowMisaligned128Store is set, we penalize <2 x i64> stores. On
-    ; Cyclone, for example, such stores should be expensive because we don't
-    ; split them and misaligned 16b stores have bad performance.
-    ;
-    ; CHECK: cost of 1 {{.*}} store
-    ; SLOW_MISALIGNED_128_STORE: cost of 12 {{.*}} store
+    ; If FeatureSlowMisaligned128Store is set, we penalize 128-bit stores.
+    ; The unlegalized 256-bit stores are further penalized when legalized down
+    ; to 128-bit stores. 
+
+    ; CHECK: cost of 2 for {{.*}} store <4 x i64>
+    ; SLOW_MISALIGNED_128_STORE: cost of 24 for {{.*}} store <4 x i64>
+    store <4 x i64> undef, <4 x i64> * undef
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <8 x i32>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <8 x i32>
+    store <8 x i32> undef, <8 x i32> * undef
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <16 x i16>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <16 x i16>
+    store <16 x i16> undef, <16 x i16> * undef
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <32 x i8>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <32 x i8>
+    store <32 x i8> undef, <32 x i8> * undef
+
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <4 x double>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <4 x double>
+    store <4 x double> undef, <4 x double> * undef
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <8 x float>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <8 x float>
+    store <8 x float> undef, <8 x float> * undef
+    ; CHECK-NEXT: cost of 2 for {{.*}} store <16 x half>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 24 for {{.*}} store <16 x half>
+    store <16 x half> undef, <16 x half> * undef
+
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <2 x i64>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <2 x i64>
     store <2 x i64> undef, <2 x i64> * undef
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <4 x i32>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <4 x i32>
+    store <4 x i32> undef, <4 x i32> * undef
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <8 x i16>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <8 x i16>
+    store <8 x i16> undef, <8 x i16> * undef
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <16 x i8>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <16 x i8>
+    store <16 x i8> undef, <16 x i8> * undef
+
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <2 x double>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <2 x double>
+    store <2 x double> undef, <2 x double> * undef
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <4 x float>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <4 x float>
+    store <4 x float> undef, <4 x float> * undef
+    ; CHECK-NEXT: cost of 1 for {{.*}} store <8 x half>
+    ; SLOW_MISALIGNED_128_STORE-NEXT: cost of 12 for {{.*}} store <8 x half>
+    store <8 x half> undef, <8 x half> * undef
 
     ; We scalarize the loads/stores because there is no vector register name for
     ; these types (they get extended to v.4h/v.2s).
     ; CHECK: cost of 16 {{.*}} store
     store <2 x i8> undef, <2 x i8> * undef
     ; CHECK: cost of 64 {{.*}} store
     store <4 x i8> undef, <4 x i8> * undef
     ; CHECK: cost of 16 {{.*}} load
     load <2 x i8> , <2 x i8> * undef
     ; CHECK: cost of 64 {{.*}} load
     load <4 x i8> , <4 x i8> * undef
 
     ret void
 }