[GlobalOpt] Bail out on SROA of a Global if a scalable vector type is seen

[cameron.mcinally]

Summary

This is more a bug report than an earnest patch. The GlobalOpt pass is ICE'ing on scalable vector types. I've included a little patch to bail out if a scalable type is seen, but I'm not sure if that's still the current best practice for handling passes that haven't been updated for scalable vector types.

If this patch is acceptable, I'll be happy to commit the change...

[efriedma]

Bailing out seems reasonable: we can't actually tell how many bytes a scalable load/store is touching, so the normal SRA algorithm won't work. Maybe more intuitive to put the bailout in collectSRATypes().

Not sure why you're sticking the test in llvm/test/CodeGen/AArch64 instead of llvm/test/Transforms/GlobalOpt/. (If you need TargetTransformInfo, you can make a subfolder llvm/test/Transforms/GlobalOpt/AArch64, but I suspect you don't.)

[cameron.mcinally]

Not sure why you're sticking the test in llvm/test/CodeGen/AArch64 instead of llvm/test/Transforms/GlobalOpt/.

Good call, @efriedma. I was in SVE mode and not thinking. Patch updated.

[efriedma]

LGTM

[cameron.mcinally]

Apologies, @efriedma. I've updated the patch again to move the check into collectSRATypes(). Would you mind doing one more review?

[efriedma]

LGTM

llvm/lib/Transforms/IPO/GlobalOpt.cpp

if (Value *Ptr = getLoadStorePointerOperand(V)) {
         return false;
 
       // TODO: We currently require that all accesses at a given offset must
       // use the same type. This could be relaxed.
       Type *Ty = getLoadStoreType(V);
       auto It = Types.try_emplace(Offset.getZExtValue(), Ty).first;
       if (Ty != It->second)
         return false;
+
+      // Scalable types not currently supported.
+      if (isa<ScalableVectorType>(Ty))
+        return false;
+
       continue;
     }
 
     // Ignore dead constant users.
     if (auto *C = dyn_cast<Constant>(V)) {
       if (!isSafeToDestroyConstant(C))
         return false;
       continue;

llvm/test/Transforms/GlobalOpt/2022-08-23-ScalableVectorCrash.ll

+; RUN: opt -globalopt < %s
+
+; Ensure we don't ICE by trying to optimize a scalable vector load of a global
+; variable.
+
+%struct.xxx = type <{ [96 x i8] }>
+
+@.bss = internal unnamed_addr global %struct.xxx zeroinitializer, align 32
+
+define dso_local void @foo() local_unnamed_addr align 16 #0 {
+L.entry:
+  store <vscale x 2 x double> zeroinitializer, ptr @.bss, align 1
+  %0 = load <vscale x 2 x double>, ptr @.bss, align 8
+  unreachable
+}