Skip to content

[LLVMCPU] Append user-specific cpu-features after the supported features from a cpu name.#24473

Merged
bjacob merged 1 commit into
iree-org:mainfrom
JerryShih:dev/jerrys/cpu_feature
May 18, 2026
Merged

[LLVMCPU] Append user-specific cpu-features after the supported features from a cpu name.#24473
bjacob merged 1 commit into
iree-org:mainfrom
JerryShih:dev/jerrys/cpu_feature

Conversation

@JerryShih
Copy link
Copy Markdown
Contributor

Before this, IREE will only use user-specific features and drop others.

@JerryShih JerryShih force-pushed the dev/jerrys/cpu_feature branch from 5ecbe99 to dd979bf Compare May 13, 2026 08:07
@hanhanW hanhanW requested a review from bjacob May 13, 2026 16:36
Copy link
Copy Markdown
Collaborator

@bjacob bjacob left a comment

Choose a reason for hiding this comment

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

Bonus points if you can find a way to test this with target attributes in a MLIR test under compiler/plugins/target/LLVMCPU/test. Probably a problem that a good AI could solve.

@JerryShih JerryShih force-pushed the dev/jerrys/cpu_feature branch from dd979bf to 2ba12fa Compare May 14, 2026 02:54
@JerryShih JerryShih changed the title [CPU] Append user-specific cpu-features after the supported features from a cpu name. [LLVMCPU] Append user-specific cpu-features after the supported features from a cpu name. May 14, 2026
@JerryShih JerryShih requested a review from bjacob May 14, 2026 02:55
@JerryShih
Copy link
Copy Markdown
Contributor Author

Bonus points if you can find a way to test this with target attributes in a MLIR test under compiler/plugins/target/LLVMCPU/test. Probably a problem that a good AI could solve.

here is the test:
https://github.com/iree-org/iree/pull/24473/changes#diff-9bbcb3dc9be57f3f36ae3e85d8aab2432f426c0ef2164092532715ebc8cd938f

@JerryShih
Copy link
Copy Markdown
Contributor Author

How could I trigger the ci test?

//
// RUN: iree-compile --compile-to=preprocessing --iree-hal-target-device=local --iree-hal-local-target-device-backends=llvm-cpu --iree-llvmcpu-target-triple=x86_64-linux-gnu %s \
// RUN: --iree-llvmcpu-target-cpu=x86-64-v4 \
// RUN: --iree-llvmcpu-target-cpu-features="-avx2,+amx-int8,+amx-bf16" \
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why the -avx2 here? I would love it if negative feature flags worked, however:

  1. The CHECKs below show that they don't in this instance, right? // CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx2.
  2. This particular choice of features is rather unnatural: avx512 and AMX but no avx2. If we wanted to test a negative feature flag, I would rather start from the x86-64-v4 set and remote one of the avx512 features that is not implied by others (e.g. avx512vl, but not avx512f).

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.

The SubtargetFeatures.AddFeature() just append the new feature without check the duplication.
So, all user specific feature will just append after the queried features.
I just try to show that we could disable the queried features with -XXX at the end.
Is it more clear to do the deduplication(remove the match feature with "-"XXX feature str)?
For llvm, it will use the latest feature setting.
E.g. +avx2,......,-avx2 => no avx2

// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx2, <====
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx512bw,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx512cd,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx512dq,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx512f,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +avx512vl,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +fma,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +sse,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +sse2,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +sse3,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +sse4.1,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: +sse4.2,
// CHECK-X86-64-V4-WITH-USER-FEATURES-SAME: -avx2, <===

2. This particular choice of features is rather unnatural: avx512 and AMX but no avx2. If we wanted to test a negative feature flag, I would rather start from the x86-64-v4 set and remote one of the avx512 features that is not implied by others (e.g. avx512vl, but not avx512f).

i will update the case with --iree-llvmcpu-target-cpu-features="-avx512f"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I had completely missed that (I didn't realize about the duplicate entries). Ignore me!

@bjacob
Copy link
Copy Markdown
Collaborator

bjacob commented May 14, 2026

How could I trigger the ci test?

I did it for you. To do it yourself you need write permissions on the repository. https://iree.dev/developers/general/contributing/#obtaining-commit-access

@JerryShih
Copy link
Copy Markdown
Contributor Author

The failed case: futex_test
but i don't know why the compiler cpu feature will affect this.

2026-05-14T14:22:21.2375799Z 1246/2008 Test #1241: iree/base/threading/futex_test  Failed    0.31 sec
2026-05-14T14:22:21.2376660Z [==========] Running 6 tests from 1 test suite.
2026-05-14T14:22:21.2377080Z [----------] Global test environment set-up.
2026-05-14T14:22:21.2377469Z [----------] 6 tests from FutexTest
2026-05-14T14:22:21.2377830Z [ RUN      ] FutexTest.WakeNoWaiters
2026-05-14T14:22:21.2378207Z [       OK ] FutexTest.WakeNoWaiters (0 ms)
2026-05-14T14:22:21.2378623Z [ RUN      ] FutexTest.WaitValueMismatch
2026-05-14T14:22:21.2379044Z [       OK ] FutexTest.WaitValueMismatch (0 ms)
2026-05-14T14:22:21.2379459Z [ RUN      ] FutexTest.WakeWakesWaiter
2026-05-14T14:22:21.2379862Z [       OK ] FutexTest.WakeWakesWaiter (10 ms)
2026-05-14T14:22:21.2380256Z [ RUN      ] FutexTest.WakeCount
2026-05-14T14:22:21.2380607Z [       OK ] FutexTest.WakeCount (70 ms)
2026-05-14T14:22:21.2380979Z [ RUN      ] FutexTest.WaitTimeout
2026-05-14T14:22:21.2381376Z [       OK ] FutexTest.WaitTimeout (50 ms)
2026-05-14T14:22:21.2382298Z [ RUN      ] FutexTest.WaitImmediateDeadline
2026-05-14T14:22:21.2382800Z iree/runtime/src/iree/base/threading/futex_test.cc:167: Failure
2026-05-14T14:22:21.2383432Z Expected: (elapsed) < (10 * 1000000), actual: 23616036 vs 10000000

@JerryShih JerryShih force-pushed the dev/jerrys/cpu_feature branch from 2ba12fa to 068e445 Compare May 15, 2026 04:40
@bjacob
Copy link
Copy Markdown
Collaborator

bjacob commented May 15, 2026

Yes, the futex test failure looks unrelated. You can ignore it.

…res from a cpu name.

Before this, IREE will only use user-specific features and drop others.

Signed-off-by: Jerry Shih <jerry.shih@sifive.com>
@JerryShih JerryShih force-pushed the dev/jerrys/cpu_feature branch from 068e445 to 66db3f4 Compare May 18, 2026 02:33
@JerryShih JerryShih requested a review from bjacob May 18, 2026 02:37
@bjacob bjacob enabled auto-merge (squash) May 18, 2026 10:53
@bjacob bjacob merged commit 0653426 into iree-org:main May 18, 2026
61 of 64 checks passed
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