[leejet]

Summary

• Benchmark Vulkan direct convolution against im2col for 2D and 3D auto selection, with output validation and timing logs.
• Enable direct convolution only when the probe shows more than 5% improvement; disable it if the probe fails. Preserve other backends' selection logic.
• Document automatic selection and performance tradeoffs in the respective flash attention and direct convolution sections.

Related Issue / Discussion

Fix #2137.

Additional Information

N/A

Checklist

• I have read and confirmed this PR follows the contribution guidelines.

Summary by CodeRabbit

• New Features
  • Vulkan’s automatic direct-convolution setting now benchmarks direct and im2col paths for both 2D and 3D workloads, selecting direct convolution only when measurements show a sufficient improvement.
  • Other backends continue to use support checks when selecting automatically; unsupported backends default to off.
• Documentation
  • Updated performance guidance for flash attention and direct convolution, including hardware and workload variability and recommendations for controlled comparisons.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 82e0d35c-2109-4d79-ab8a-18f1fb0ca10c

📥 Commits

Reviewing files that changed from the base of the PR and between 2891378 and da0a2b2.

📒 Files selected for processing: 4

• docs/backend.md
• docs/backend_probe.md
• docs/performance.md
• src/core/probe/conv_direct.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Vulkan --conv-direct auto now benchmarks direct convolution against the standard im2col path and selects direct convolution based on measured timings. Other backends retain implicit-GEMM support checks. Documentation describes these rules and updates performance guidance for convolution and flash attention.

Changes: Direct convolution selection

Layer / File(s)	Summary
Probe construction and selection src/core/probe/conv_direct.cpp	The probe can build direct or standard convolution graphs and initializes deterministic inputs. Vulkan compares two shape configurations; an incomplete probe selects off. Other backends retain implicit-GEMM checks.
Selection and performance documentation docs/backend.md, docs/backend_probe.md, docs/performance.md	The documentation describes Vulkan timing comparisons and selection criteria, other-backend checks, and performance comparison guidance for direct convolution and flash attention.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to da0a2

Vulkan auto-selection now benchmarks direct convolution before enabling it, and a failed probe falls back to off. No concrete merge-blocking defect was established. Whether the small probe shapes avoid the slowdown on affected AMD GPUs is not verified.

### Architecture Summary

Architecture risk: 🟡 Medium · up to da0a2

The change affects 2 systems.

Changed systems: src, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 1 changed file maps to changed impact.
• observed — docs (service) was modified; 3 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in docs/backend.md: --conv-direct auto now benchmarks Vulkan direct convolution against im2col and selects it per dimension only when the summed median times improve by at least 5%; failed probes select off. This replaces Vulkan’s previous unconditional enablement for both dimensions. Other backends continue to use implicit-GEMM checks without benchmarking, with the documented architecture and implementation requirements; results remain cached per compute backend and dimension.
• observed — Modified behavior in docs/backend_probe.md: The documentation now says Vulkan compares direct convolution with im2col using shared measurements; it removes the prior statement that probe paths do not allocate device tensors, execute kernels, or run measurements.
• observed — Modified behavior in docs/performance.md: Replaces the claims about typical flash-attention speed and supported backends with option behavior, workload-dependent performance caveats, automatic-selection limits, comparison guidance, and a link to backend selection.
• observed — Modified behavior in docs/performance.md: Adds documentation for direct convolution selection between direct and im2col paths. It describes performance variability, Vulkan benchmark and other-backend implicit-GEMM selection, and controlled comparisons that hold flash attention constant and separate probe time from generation time.

Reliability and maintainability

• inferred — Risk-relevant change factors for src: blast_radius_1; direct_dependents_1

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 3 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description includes all required template sections. It clearly explains the Vulkan benchmarking change, selection behavior, documentation updates, related issue, and checklist status.
Title check		The title is concise and accurately identifies the main change: benchmarking Vulkan direct convolution in automatic mode.
Linked Issues check		Issue #2137 identifies incorrect Vulkan automatic direct-convolution selection as the regression. The PR changes Vulkan auto selection to compare direct convolution with im2col for separate 2D and 3D …
Out of Scope Changes check		The changes stay within the scope of issue #2137 and the PR description. The backend documentation describes the new Vulkan probe and preserved non-Vulkan rules. The performance documentation explains…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}