[AMDGPU] Fix bug introduced in 47a5c36b37f0

[dfukalov]

Summary

[AMDGPU] Fix bug introduced in 47a5c36b37f0

[arsenm]

llvm/test/Analysis/CostModel/AMDGPU/shufflevector.ll

55

Should still be checking the returned cost

[dfukalov]

Address comment

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

741

I don't see how this works? Shouldn't this be returning instead of falling through?

[dfukalov]

fix typo

[dfukalov]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

741

You're right, thanks!

[arsenm]

LGTM

[arsenm]

Commit message could uses work though, include revision in the long description, not the first line

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

case Instruction::ShuffleVector: {
     Type *SrcTy = Shuffle->getOperand(0)->getType();
 
     // TODO: Identify and add costs for insert subvector, etc.
     int SubIndex;
     if (Shuffle->isExtractSubvectorMask(SubIndex))
       return getShuffleCost(TTI::SK_ExtractSubvector, SrcTy, SubIndex, Ty);
 
     if (Shuffle->changesLength())
-      return -1;
+      BaseT::getUserCost(U, Operands);
 
     if (Shuffle->isIdentity())
       return 0;
 
     if (Shuffle->isReverse())
       return getShuffleCost(TTI::SK_Reverse, Ty, 0, nullptr);
 
     if (Shuffle->isSelect())

llvm/test/Analysis/CostModel/AMDGPU/shufflevector.ll

-; RUN: opt -cost-model -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=gfx900 %s | FileCheck -check-prefixes=GFX9,GCN %s
-; RUN: opt -cost-model -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=fiji %s | FileCheck -check-prefixes=VI,GCN %s
-; RUN: opt -cost-model -cost-kind=code-size -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=gfx900 %s | FileCheck -check-prefixes=GFX9,GCN %s
-; RUN: opt -cost-model -cost-kind=code-size -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=fiji %s | FileCheck -check-prefixes=VI,GCN %s
+; RUN: opt -cost-model -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=gfx900 %s | FileCheck -check-prefixes=GFX9,GCN,TPT %s
+; RUN: opt -cost-model -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=fiji %s | FileCheck -check-prefixes=VI,GCN,TPT %s
+; RUN: opt -cost-model -cost-kind=code-size -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=gfx900 %s | FileCheck -check-prefixes=GFX9,GCN,CS %s
+; RUN: opt -cost-model -cost-kind=code-size -analyze -mtriple=amdgcn-unknown-amdhsa -mcpu=fiji %s | FileCheck -check-prefixes=VI,GCN,CS %s
 
 ; GCN-LABEL: 'shufflevector_00_v2i16'
 ; GFX9: estimated cost of 0 for {{.*}} shufflevector <2 x i16> %vec, <2 x i16> undef, <2 x i32> zeroinitializer
 ; VI: estimated cost of 1 for {{.*}} shufflevector <2 x i16> %vec, <2 x i16> undef, <2 x i32> zeroinitializer
 define amdgpu_kernel void @shufflevector_00_v2i16(<2 x i16> addrspace(1)* %out, <2 x i16> addrspace(1)* %vaddr) {
   %vec = load <2 x i16>, <2 x i16> addrspace(1)* %vaddr
   %shuf = shufflevector <2 x i16> %vec, <2 x i16> undef, <2 x i32> zeroinitializer
   store <2 x i16> %shuf, <2 x i16> addrspace(1)* %out

 ; GCN: estimated cost of 2 for {{.*}} shufflevector <2 x i16> %vec0, <2 x i16> %vec1, <2 x i32> <i32 0, i32 2>
 define amdgpu_kernel void @shufflevector_02_v2i16(<2 x i16> addrspace(1)* %out, <2 x i16> addrspace(1)* %vaddr0, <2 x i16> addrspace(1)* %vaddr1) {
   %vec0 = load <2 x i16>, <2 x i16> addrspace(1)* %vaddr0
   %vec1 = load <2 x i16>, <2 x i16> addrspace(1)* %vaddr1
   %shuf = shufflevector <2 x i16> %vec0, <2 x i16> %vec1, <2 x i32> <i32 0, i32 2>
   store <2 x i16> %shuf, <2 x i16> addrspace(1)* %out
   ret void
 }
+
+; GCN-LABEL: 'shufflevector_xxx'
+; TPT: Unknown cost for {{.*}} shufflevector <2 x i8> %vec, <2 x i8> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+; CS: estimated cost of 8 for {{.*}} shufflevector <2 x i8> %vec, <2 x i8> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+; Should not assert
+define amdgpu_kernel void @shufflevector_xxx(<4 x i8> addrspace(1)* %out, <2 x i8> addrspace(1)* %vaddr) {
+  %vec = load <2 x i8>, <2 x i8> addrspace(1)* %vaddr
+  %shuf = shufflevector <2 x i8> %vec, <2 x i8> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  store <4 x i8> %shuf, <4 x i8> addrspace(1)* %out
+  ret void
+}