Repository navigation
Add experimental _hasTargetFeature predicate for CPU/ISA feature query - #92189
hjyamauchi wants to merge 1 commit into
Conversation
|
Please test with following pull request: @swift-ci Please test |
| bool BridgedASTContext_hasTargetFeature(BridgedASTContext cContext, | ||
| BridgedStringRef cName) { | ||
| auto *loader = cContext.unbridged().getClangModuleLoader(); | ||
| // Can only be null in unit tests. |
There was a problem hiding this comment.
Does it make sense to consider a separate testing interface? Doing so allows us to have a reduced branch in the production path. Hopefully we can eliminate the testing interface for release binaries.
There was a problem hiding this comment.
What do you mean by a testing interface?
There was a problem hiding this comment.
The "bridging" here is happening between the Swift and C++ portions of the compiler itself. This is not a runtime test.
There was a problem hiding this comment.
@hjyamauchi the "Can only be null in unit tests" aspects is what I was referring to. Rather than having a bridge for the unit tests, introduce a separate testing entry point for testing.
There was a problem hiding this comment.
ah I see. It turned out that this was false, unit tests aren't calling this. I removed the null check.
|
Please test with following pull request: @swift-ci Please test |
|
Please test with following pull request: @swift-ci Please test |
| return hasAttribute(Ctx.LangOpts, attributeName); | ||
| } else if (KindName == "_hasTargetFeature") { | ||
| auto featureName = cast<StringLiteralExpr>(Arg)->getValue(); | ||
| auto *loader = Ctx.getClangModuleLoader(); | ||
| // Can only be null in unit tests | ||
| return loader && loader->hasTargetFeature(featureName); |
There was a problem hiding this comment.
FWIW, most of this is dead code nowadays, since we require a host Swift everywhere.
| } | ||
|
|
||
| bool ClangImporter::hasTargetFeature(StringRef featureName) const { | ||
| return getTargetInfo().hasFeature(featureName); |
There was a problem hiding this comment.
I'm not sure about this implementation. Doesn't the @target attribute change what target features are available within a particular function?
There was a problem hiding this comment.
Yes, but this is orthogonal. This is meant to support the following idiom:
#if arch(particular processor) && _hasTargetFeature(particular feature)
... code optimized for particular ISA ...
#else if arch(other processor) && _hasTargetFeature(other feature)
... code optimized for some other ISA ...
#else
... general code without any ISA assumptions ...
#endif
There was a problem hiding this comment.
Yes and this allows us to choose among different versions at build time. What I'm aiming for here is the combination of @_target and this so that we can call separate code optimized for different ISA features based on the build-time baseline target CPU, something like:
@_target("sve2")
func foo_sve2() { ... }
@_target("sve")
func foo_sve() { ... }
// default
func foo_default() { ... }
#if _hasTargetFeature("sve2")
foo_sve2()
#elseif _hasTargetFeature("sve")
foo_sve()
#else
foo_default()
#endif
where foo_sve2() is taken when it's built with sve2 and foo_sve() is taken when it's built with sve
There was a problem hiding this comment.
Okay, now put that into a function that uses @target:
@target(particular feature)
func f() {
#if arch(particular processor) && _hasTargetFeature(particular feature)
... code optimized for particular ISA ...
#else if arch(other processor) && _hasTargetFeature(other feature)
... code optimized for some other ISA ...
#else
... general code without any ISA assumptions ...
#endif
}When arch(particular processor) holds, and the target provided on the command line doesn't imply _hasTargetFeature(particular feature), what should I expect this to do?
There was a problem hiding this comment.
_hasTargetFeature() refers to the current processor architecture being targeted by the current compilation context. This may be affected by compiler command-line options and by surrounding @target() annotations.
In your example here, the presence of @target(particular feature) indicates that this function is being compiled for a processor with that particular feature, so I would expect _hasTargetFeature(particular feature) to evaluate true.
To be honest, this seems like a pointless combination in practice. For an MVP, I'd be happy for _hasTargetFeature() to diagnose when put inside a function with @target() on it.
There was a problem hiding this comment.
I still think #if is the right design here. The whole point is being able to choose different implementations at compile time and that's exactly what #if expresses.
As I said, there's not much point to combining these two. Is it feasible to diagnose? I'd have to think about whether I would be willing to just silently ignore @target() for this purpose. Maybe @stephentyrone has some insight?
There was a problem hiding this comment.
Some variation of function multi-versioning could conceivably be an alternative: clang supports a feature where you provide overloads of a function and annotate each one with the particular processor/target feature that it should be used for. This should allow us to defer final function selection until IRGen (or even later).
There was a problem hiding this comment.
If we have multi-versioning, do we need this #if _hasTargetFeature?
There was a problem hiding this comment.
I think #if allows purely compile-time conditional code while multi-versioning involves runtime/link-time checks/indirection. An expected use case is when we are willing to raise baseline target cpu and have multiple builds optimized for each CPU generation. Maybe not be a perfect example but writing code sort of like this would be enabled.
There was a problem hiding this comment.
Yes, we absolutely need (a good version of) this in the presence of multi-versioning (if anything, it becomes more important, not less). Consider something like a two-register lookup table in SIMD. This is going to get inlined into a multi-versioned function. On arm64, it should become TBL. On baseline x86 it has to be an explicit memory gather. But on x86 with SSSE3 (whether via command line flags, target, or multi-versioning), it should become two PSHUFBs and a blend. On x86 with the right AVX512 extensions, it should be a single instruction. But there's no way to write that in a target-independent manner*. We actually need to have different implementations of the two-register lookup table depending on what features are available in the context we get inlined into.
(*) LLVM has been working on this, so it might be soon, but lots of other things still won't be.
Even if one isn't writing explicit SIMD, I frequently want to use a different algorithm depending on what instructions are available. For instance, in something like Augmented.product from Numerics, we would like to use an FMA-based algorithm when targeting x86 with FMA, but Dekker's hi-lo split algorithm when we do not have FMA available, rather than going through a soft float FMA from the host libm.
|
CC: @stephentyrone |
e0a93ae to
172b428
Compare
|
@swift-ci please test |
|
Please test with following pull request: @swift-ci Please test |
|
Please test with following pull request: @swift-ci Please test macOS platform |
This allows conditional compiles (#if...) per the target CPU/ISA features. This is guarded behind an experimental feature flag. This is the main swift repo part. There's a separate swift-syntax part.
172b428 to
c07754b
Compare
|
Rebased |
|
Please test with following pull request: @swift-ci Please test macOS platform |
|
Please test with following pull request: @swift-ci Please test windows platform |
|
Please test with following pull request: @swift-ci Please test linux platform |
This allows conditional compiles (#if...) per the target CPU/ISA features. This is guarded behind an experimental feature flag.
This is the main swift repo part. There's a separate swift-syntax part.