[SandboxVec] Early return checks

[vporpo]

This patch implements a couple of early return checks.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

This patch implements a couple of early return checks.

---

Full diff: https://github.com/llvm/llvm-project/pull/107465.diff

3 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp (+11)
• (added) llvm/test/Transforms/SandboxVectorizer/X86/no_vector_regs.ll (+9)
• (added) llvm/test/Transforms/SandboxVectorizer/no_implicit_float.ll (+9)

diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
index 072a6606694a0f..9c84ca2880ca39 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
@@ -28,7 +28,18 @@ PreservedAnalyses SandboxVectorizerPass::run(Function &F,
 }
 
 bool SandboxVectorizerPass::runImpl(Function &F) {
+  // If the target claims to have no vector registers early return.
+  if (!TTI->getNumberOfRegisters(TTI->getRegisterClassForType(true))) {
+    LLVM_DEBUG(dbgs() << "SBVec: Target has no vector registers, return.\n");
+    return false;
+  }
   LLVM_DEBUG(dbgs() << "SBVec: Analyzing " << F.getName() << ".\n");
+  // Don't vectorize when the attribute NoImplicitFloat is used.
+  if (F.hasFnAttribute(Attribute::NoImplicitFloat)) {
+    LLVM_DEBUG(dbgs() << "SBVec: NoImplicitFloat attribute, return.\n");
+    return false;
+  }
+
   sandboxir::Context Ctx(F.getContext());
   // Create SandboxIR for `F`.
   sandboxir::Function &SBF = *Ctx.createFunction(&F);
diff --git a/llvm/test/Transforms/SandboxVectorizer/X86/no_vector_regs.ll b/llvm/test/Transforms/SandboxVectorizer/X86/no_vector_regs.ll
new file mode 100644
index 00000000000000..3c3a3bd2fe6ea0
--- /dev/null
+++ b/llvm/test/Transforms/SandboxVectorizer/X86/no_vector_regs.ll
@@ -0,0 +1,9 @@
+; RUN: opt -passes=sandbox-vectorizer -debug -mtriple=x86_64-- -mattr=-sse %s 2>&1 | FileCheck %s
+; REQUIRES: asserts
+; Please note that this won't update automatically with update_test_checks.py !
+
+; Check that we early return if the target has no vector registers.
+define void @no_vector_regs() {
+; CHECK: SBVec: Target has no vector registers, return.
+  ret void
+}
diff --git a/llvm/test/Transforms/SandboxVectorizer/no_implicit_float.ll b/llvm/test/Transforms/SandboxVectorizer/no_implicit_float.ll
new file mode 100644
index 00000000000000..8081323c88137f
--- /dev/null
+++ b/llvm/test/Transforms/SandboxVectorizer/no_implicit_float.ll
@@ -0,0 +1,9 @@
+; RUN: opt -passes=sandbox-vectorizer -debug %s 2>&1 | FileCheck %s
+; REQUIRES: asserts
+; Please note that this won't update automatically with update_test_checks.py !
+
+; Check that we early return if the function has the NoImplicitFloat attribute.
+define void @no_implicit_float() noimplicitfloat {
+; CHECK: SBVec: NoImplicitFloat attribute, return.
+  ret void
+}

[aeubanks]

these should probably use update_test_checks.py, checking debug statements is not recommended since it doesn't necessarily check the actual behavior, plus it only works with assert builds

that would require changing this to IR that would certainly be otherwise vectorized. but given that we don't actually have any vectorization in tree yet, it's kinda a pointless check (i.e. this change is not really meaningfully testable right now), but I'm fine with keeping adding this test with some otherwise vectorizable IR so that future changes that might break this would result in a change in this test which hopefully someone would catch

[vporpo]

no_implicit_float can be caught with some IR check and I agree it is better to check the IR.
But the early return due to no vector registers can't, so using a debug statement seems to be the only way to check it.

[aeubanks]

why can't the vector register check be tested the same way?

[vporpo]

because if the target contains no vector registers the code won't get vectorized because the TTI won't let you get a register wide enough to fit 2 or more elements. This check will just save some compile time.

[aeubanks]

oh I see. then we don't need a test for it if it'll be unobservable anyway

[vporpo]

OK let me drop this test and replace the other one with an IR snippet that is trivially vectorizable.

[aeubanks]

should this be stored to a different pointer? otherwise this is dead code

[vporpo]

We could use a different pointer, but it shouldn't matter too much. The vectorizer won't clean it up.