[SVE] Fix invalid Scalable to fixed width vetor type demotion in LLT

[ctetreau]

Summary

getLLTForType calls LLT::vector(Ty->getNumElements(), ScalarTy) for
fixed width and scalable vectors, losing information on the scalability
of the input vector. Rather than miscompile, return an invalid LLT for
scalable vectors.

[efriedma]

This code still won't actually work, either way. (I think there's a patch out for review to disable GlobalISel for functions that contain scalable vectors?)

If you don't want to deal with it now, just do auto NumElements = cast<FixedVectorType>(VTy)->getNumElements();

[ctetreau]

In D82329#2107553, @efriedma wrote:

This code still won't actually work, either way. (I think there's a patch out for review to disable GlobalISel for functions that contain scalable vectors?)

If you don't want to deal with it now, just do auto NumElements = cast<FixedVectorType>(VTy)->getNumElements();

Unforunately, that causes llvm\test\CodeGen\AArch64\sve-intrinsics-ld1.ll to fail. So it's either this or calling getElementCount().Min. (or adding the feature)

I guess getElementCount().Min would eliminate the behavior change, but it's clearly broken and I'm not sure it's worth preserving the original behavior.

[efriedma]

I talked to Christopher offline, and he said this code is in fact getting hit with SVE enabled. Is D81557 working correctly?

[aemerson]

In D82329#2107913, @efriedma wrote:

I talked to Christopher offline, and he said this code is in fact getting hit with SVE enabled. Is D81557 working correctly?

We'd need to see a backtrace and repro.

[david-arm]

Hi @aemerson @efriedma @ctetreau

#7 0x0000ffffa0f26760 llvm::getLLTForType(llvm::Type&, llvm::DataLayout const&) (/lib64/libc.so.6+0x36760)
#8 0x0000000002556f74 llvm::computeValueLLTs(llvm::DataLayout const&, llvm::Type&, llvm::SmallVectorImpl<llvm::LLT>&, llvm::SmallVectorImpl<unsigned long>*, unsigned long) (.localalias) (./bin/llc+0x2556f74)
#9 0x0000000001aeb108 llvm::IRTranslator::getOrCreateVRegs(llvm::Value const&) (.part.0) (./bin/llc+0x1aeb108)
#10 0x00000000019df8a4 llvm::IRTranslator::runOnMachineFunction(llvm::MachineFunction&) (./bin/llc+0x19df8a4)

So clearly we're not falling back on DAG ISel early enough. I'll try to address this in a separate patch.

[david-arm]

This problem occurs in the IRTranslator just before we call lowerFormalArguments (which is currently the earliest point we detect scalable types and fall back to DAG ISel)

[david-arm]

Perhaps this change is good enough? We call computeValueLLTs just before lowerFormalArguments, which will detect the scalable type and fall back anyway even if the LLT type is wrong. The other option is to add yet another fallBackOnDAGISel type callback that allows the backend to reject certain types before we create the vregs.

[aemerson]

In D82329#2108346, @david-arm wrote:

Perhaps this change is good enough? We call computeValueLLTs just before lowerFormalArguments, which will detect the scalable type and fall back anyway even if the LLT type is wrong. The other option is to add yet another fallBackOnDAGISel type callback that allows the backend to reject certain types before we create the vregs.

I think we just need to move the check from lowerFormalArguments to the for-loop before the call to it, and emit a remark to explain why we're falling back.

[david-arm]

Hi @aemerson @ctetreau, I have a fix here that prevents us entering this code for scalable types:

https://reviews.llvm.org/D82524

[aemerson]

This can be abandoned now?

[ctetreau]

This patch (in the form of an unconditional cast to FixedVectorType on line 22) has been rolled into D82210

llvm/lib/CodeGen/LowLevelType.cpp

 
 #include "llvm/CodeGen/LowLevelType.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 LLT llvm::getLLTForType(Type &Ty, const DataLayout &DL) {
-  if (auto VTy = dyn_cast<VectorType>(&Ty)) {
+  if (auto VTy = dyn_cast<FixedVectorType>(&Ty)) {
     auto NumElements = VTy->getNumElements();
     LLT ScalarTy = getLLTForType(*VTy->getElementType(), DL);
     if (NumElements == 1)
       return ScalarTy;
     return LLT::vector(NumElements, ScalarTy);
   }
 
   if (auto PTy = dyn_cast<PointerType>(&Ty)) {