Skip to content

[LLVMCPU] fix: return valid vscale range only for targets supporting SVE#24522

Open
ziereis wants to merge 1 commit into
iree-org:mainfrom
ziereis:zieries_fix_vscale_range_check
Open

[LLVMCPU] fix: return valid vscale range only for targets supporting SVE#24522
ziereis wants to merge 1 commit into
iree-org:mainfrom
ziereis:zieries_fix_vscale_range_check

Conversation

@ziereis
Copy link
Copy Markdown
Contributor

@ziereis ziereis commented May 21, 2026

fixes the getDefaultVscaleRange to only return a vscale range if the target supports it.

Signed-off-by: default <ziereis@roofline.ai>
@ziereis ziereis force-pushed the zieries_fix_vscale_range_check branch from dff6848 to c33c7a1 Compare May 21, 2026 11:43
@ziereis ziereis requested review from bjacob and egebeysel May 22, 2026 07:46
Copy link
Copy Markdown
Contributor

@egebeysel egebeysel left a comment

Choose a reason for hiding this comment

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

The main functionality LGTM, but we could maybe refactor this a bit and move some things around 🤔

@@ -1748,11 +1753,14 @@ bool hasFusedLeadingOp(linalg::LinalgOp rootOp) {

std::optional<vector::VscaleRange>
getDefaultVscaleRange(IREE::HAL::ExecutableTargetAttr targetAttr) {
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.

Should we maybe move this function to CPUUtils instead? I think vscale is only relevant to CPUs and this could be kept there, as well as the hasAnySVEFeature method.

return triple && triple.value().isRISCV64();
}

static bool hasAnySVEFeature(DictionaryAttr targetConfig) {
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 think there is a method for this already, maybe we could converge to using this one:

bool hasAnySVEFeature(DictionaryAttr targetConfig) {

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.

3 participants