Skip to content

[LLVMCPU] Implement conv heuristics in getVectorPreProcStrategy#24004

Draft
josephbak wants to merge 1 commit intoiree-org:mainfrom
josephbak:fix-conv-vector-preproc-strategy
Draft

[LLVMCPU] Implement conv heuristics in getVectorPreProcStrategy#24004
josephbak wants to merge 1 commit intoiree-org:mainfrom
josephbak:fix-conv-vector-preproc-strategy

Conversation

@josephbak
Copy link
Copy Markdown
Contributor

@josephbak josephbak commented Apr 3, 2026

Summary

Named conv ops previously returned VectorPreProcStrategy::Noneunconditionally in getVectorPreProcStrategy, bypassing all target-aware heuristics. This caused two problems:

  1. The peeling pipeline was never attached to conv ops.
  2. allowIncompleteTile stayed false, forcing the tiler to find exact divisors and producing suboptimal tile sizes for odd kernel shapes (e.g. KH=3, KW=3).

Remove the early exit so named convs fall through to the existing target-aware heuristics. In setConvRootConfig, clamp Masking to None (not yet wired for convs) and connect vecPreProcStrategy to distConfig.allowIncompleteTile, mirroring the matmul path in setContractionRootConfig.

Testing

Tested locally on AArch64 NEON. x86 (no AVX-512) and RISC-V are expected to follow the same Peeling path but rely on CI for validation. SVE (Masking) is explicitly clamped to None pending future wiring.

Notes

  • Pre-existing test failure in select_aarch64_lowering_strategy.mlir due to pipeline format mismatch (CPUDoubleTilingExpert vs #iree_cpu.pipeline<DoubleTilingExpert>) unrelated to this fix.
  • Masking support for convs is deferred, tracked by the TODO added in setConvRootConfig.

Assisted-by: Claude (Anthropic)

Signed-off-by: Joseph Bak <joseph.bak31415@gmail.com>
Copy link
Copy Markdown
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch, I wonder if you did any benchmarking for the change? What is the outcome of this PR?

assert(!getLoweringConfig(convOp) && "expected lowering_config is not set");

// Masking is not yet wired for convs (no pipelineConfig branch exists).
// TODO: wire Masking support for convs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont follow this. Masking support should be available. It is not flipped because we don't look at conv perf for a while on CPU. What do you mean by this comment?

On the other hand, we should not silently fallback here. It is better to have the logic in getVectorPreProcStrategy, so we wont spread logic everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

Benchmarks: Ran iree-benchmark-module on M-series AArch64 NEON (local-task) — inconclusive so far on tested shapes. Hunting odd-dim convs (KH=3) next; will add numbers soon.

Masking: Got it — moving conv strategy logic to getVectorPreProcStrategy, removing setConvRootConfig fallback. Update coming shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants