[WebAssembly] Mark externref as not being valid vector elements

[pmatos]

Fixes #69894 .

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 3aad77ebc37532a6a3836d902d0014391ed0a0eb d93ec3a6ba414483efd6126fc7a953228b5e4150 -- clang/test/CodeGen/WebAssembly/wasm-externref-novec.c llvm/lib/IR/Type.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index b1d0f12498c3..017b877ea982 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -682,7 +682,7 @@ VectorType *VectorType::get(Type *ElementType, ElementCount EC) {
 }
 
 bool VectorType::isValidElementType(Type *ElemTy) {
-  if(PointerType *PTy = dyn_cast<PointerType>(ElemTy))
+  if (PointerType *PTy = dyn_cast<PointerType>(ElemTy))
     return PTy->getAddressSpace() != 10 && PTy->getAddressSpace() != 20;
 
   return ElemTy->isIntegerTy() || ElemTy->isFloatingPointTy() ||

[pmatos]

cc: @nikic @jcranmer-intel @tlively @asb

I am looking for suggestions for a way forward on this. Issue #69894 triggers the SLPVectorizer that tries to create a vector of 2 externref and calculate its cost. Externref is a WebAssembly type currently represented as a pointer to a "magical" address space (address space 10). The other reference type is funcref that is a pointer to address space 20.

As a pointertype, VectorType::isValidElementType returns true. The "obvious" solution is to say that if the target is webassembly and the type is a pointer type to address space 10 or 20, then it's not a valid element type. This is what I am doing (except actually checking the current target) which I am not sure how to obtain from Type.cpp at this point. However, this would introduce a target dependence which I think is not really something we want in Type.cpp.

The other more radical alternative is to refactor the reference types approach in WebAssembly and use the new-ish TargetExtType. It feels like that's what we would have used had they existed when we initially worked on this. Alas, they didn't exist and we used the magic of address spaces to solve the issue. This refactoring would lead to possibly quite a bit of work, including touching clang to correct the lowering of these types. In addition it might also open a Pandora's box.

I wonder if there's a middle ground here somewhere that doesn't involve adding target dependence to Type.cpp but at the same time doesn't require a full refactoring of these types.

...............

Why is the test failing?

In SLPVectorizer.cpp we are generating the type <16 x ptr addrspace(10)> and calling llvm::TargetTransformInfo::getNumberOfParts on it. This will down the line call getTypeLegalizationCost for this type. The MVT for an externref is MVT::externref. The MVT for a vector of these is iPTR. At some point getTypeForEVT in ValueTypes.cpp:198 it asserts Assertion isExtended() && "Type is not extended!"' failed..

I attempted this with a pointer to int instead of externref because it should work the same. And it does without failing (obviously). This is because the type created is <4 x ptr>. The MVT for a i32 is MVT::i32 and the MVT for the vector is MVT::v4i32. This is extended and nothing asserts.

[nikic]

@pmatos If you don't want to go all the way to target extension types, is it possible to gracefully handle this in wasm's TTI cost model? Return an invalid or very high cost in this case?

[pmatos]

@pmatos If you don't want to go all the way to target extension types, is it possible to gracefully handle this in wasm's TTI cost model? Return an invalid or very high cost in this case?

Unfortunately that actually doesn't work. We never get to the point of calculating a cost because before doing that we calculate in SLPVectorizerPass::tryToVectorizeList we call TTI->getNumberOfParts(VecTy). It's deep inside getNumberOfParts that we fail because we cannot calculate how many parts there are. Which is fair enough, since we build a vector of externref that can't be built. Only later in the function we try to calculate the cost but by then we have asserted long before.

[pmatos]

I am now investigating the use of TargetExtTypes to represent Wasm Reference Types.

[pmatos]

Closing this as #71540 will implement the refactoring I mentioned earlier. Thank you for everyone's time commenting on this PR.