You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "cbalint13 (via GitHub)" <gi...@apache.org> on 2023/09/06 14:42:41 UTC
[GitHub] [tvm] cbalint13 opened a new pull request, #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
cbalint13 opened a new pull request, #15685:
URL: https://github.com/apache/tvm/pull/15685
Hi folks,
This PR leverage **LLVM** itself for CPU features lookup, **replacing hard-coded** lists.
In order to keep maintainability with X86 families & features we can rely on LLVM itself.
---
Changes:
* Introduce a single ```target_has_feature()``` replacing all ```target_has_XXX()```.
* Expose ```codegen.target.llvm_x86_get_archlist``` & ```codegen.target.llvm_x86_get_archlist```.
---
There is a test unit for a comprehensive check with the old behaviour.
Thanks,
~Cristian.
Cc: @elvin-n , @vvchernov , @echuraev , @vinx13 , @jcf94, @masahi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1712846790
> I am strongly opting for a backward compatibility for this case.
> Allow a little time (1-2 day) to investigate a way going down `llvm<11`, then I'll be back with the results.
Details of investigation:
**Post ```llvm>=11```:**
* Direct access via [TargetParsers](https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/llvm/include/llvm/TargetParser/X86TargetParser.h#L148-L158) public headers.
* Immediate, no need for target-machine, details are fetched via the public getters.
Implementation here is slim, infos coming from LLVM are precise and maintained.
**Prior ```llvm<=10```:**
* The usefull data from [tablegen descriptor](https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0/llvm/lib/Target/X86/X86.td#L514) in class ```MCSubtargetInfo``` is [private](https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0/llvm/include/llvm/MC/MCSubtargetInfo.h#L77-L78) without useful accessor.
* There is a [unhelpful](https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0/llvm/lib/MC/MCSubtargetInfo.cpp#L96) way of passing ```std::string("help")``` at the creation of ```MCSubtargetInfo()```.
* Usefull queries can be done, only via full llvm taget-machine, this compatibility is implemented here.
There is a burden of ```LLVMint()``` and the target-machine creation, but final check/legalizer is precise w.r.t to the arch.
---
@junrushao , @kparzysz-quic, @echuraev
There are quite some changes now, please help re-reviewing them.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1719991934
I am very excited about this feature and cannot wait to try it out myself! Thank you @cbalint13 for this super well-documented and well-tested PR, and it's going to be super useful for downstream applications!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vvchernov commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "vvchernov (via GitHub)" <gi...@apache.org>.
vvchernov commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1714015693
Hello @cbalint13! Very hard work! One note I should say TVM needs three avx512 intrinsics: vpmaddwd, vpmaddubsw and **vpaddd**. For the latter "+" is used but it is automatically replaced by intrinsics by llvm, and it is in **avx512f** set (I checked it [here](https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#techs=AVX_512&text=vpaddd&ig_expand=113)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1714241667
> Hello @cbalint13! Very hard work! One note I should say TVM needs three avx512 intrinsics: vpmaddwd, vpmaddubsw and **vpaddd**. For the latter "+" is used but it is automatically replaced by intrinsics by llvm, and it is in **avx512f** set (I checked it [here](https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#techs=AVX_512&text=vpaddd&ig_expand=113)).
@vvchernov ,
See now, very good point !
Let's ask for booth, will change with these comments:
```
// avx512f: llvm.x86.avx512.addpd.w.512 (LLVM auto, added)
// avx512bw: llvm.x86.avx512.pmaddubs.w.512" (TVM required)
llvm.x86.avx512.pmaddw.d.512"
if ( target_has_feature("avx512bw")
&& target_has_feature("avx512f")):
```
---
Just as side note (curiosity) on this topic:
Maybe one day someone can add separate _compute(), _update() for topi (more precise one) not letting LLVM itself.
I would be curious of outcome, in the first (i.) LLVM would ommit ```addpd.w.512``` by adding something suboptimal.
* i. "llvm -mcpu=x86-64 -mattr=+avx512bw"
* ii. "llvm -mcpu=x86-64 -mattr=+avx512bw,+avx512f"
As ```i.``` versus ```ii.``` , "x86-64" is the plainest configuration for llvm, allowing only explicit flags.
Anyway, let's go now with ```avx512bw``` && ```avx512f```.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Qianshui-Jiang commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "Qianshui-Jiang (via GitHub)" <gi...@apache.org>.
Qianshui-Jiang commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1717046584
@cbalint13 @vvchernov big thanks for your hard work and dicussion! Here is few comments.
Actually before the CasecadeLake we use avx512 to handle the 8bit and 16bit integer, so pmaddubs used here is for `SkyLake`.
And start from `CasecadeLake` we have `avx512vnni`, so there is other more instrcutions like `vpdpbusd` and `vpdpwssd`, it fused some of `pmadd` instruction we used before,
Now when we move to `SapphireRapids`， we have `amx-vnni`, which use amx instrctions to handle the 8bit integer.
Yes ur right, `avxvnni` in `AlderLake` is due to the lack of its avx512 instruction set in 12th client CPU.
I support to expose interface for fetch all the features in particular arch, but still suggest to let TVM detect the feature throught the arch name some thing like `AlderLake` or `IceLake`. cuz users may not clear of the instuction set equipment of their platform but clear of the name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on a diff in pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #15685:
URL: https://github.com/apache/tvm/pull/15685#discussion_r1317642603
##########
src/target/llvm/llvm_module.cc:
##########
@@ -45,6 +45,7 @@
#include <llvm/Support/raw_ostream.h>
#include <llvm/Target/TargetMachine.h>
#include <llvm/Target/TargetOptions.h>
+#include "llvm/TargetParser/X86TargetParser.h"
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1720011129
> I am very excited about this feature and cannot wait to try it out myself! Thank you @cbalint13 for this super well-documented and well-tested PR, and it's going to be super useful for downstream applications!
Was just a simple idea of utility, crediting the work already done before: @vvchernov , @Qianshui-Jiang and @elvin-n (the original lookup).
Thanks folks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao commented on a diff in pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on code in PR #15685:
URL: https://github.com/apache/tvm/pull/15685#discussion_r1317632043
##########
src/target/llvm/llvm_module.cc:
##########
@@ -45,6 +45,7 @@
#include <llvm/Support/raw_ostream.h>
#include <llvm/Target/TargetMachine.h>
#include <llvm/Target/TargetOptions.h>
+#include "llvm/TargetParser/X86TargetParser.h"
Review Comment:
nit: style
```suggestion
#include <llvm/TargetParser/X86TargetParser.h>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1719963169
To sum it up:
- compliant across all versions, ```llvm<=10``` issue is solved ( Cc @junrushao ), tested versions: llvm={10,14,16,17}
- implementation here exposes single ```target_has_features(XXX)``` replacing the old ```target_has_XXX()``` functions
- it also expose friendly ```llvm_x86_get_archlist()``` and ```llvm_x86_get_features(arch)``` to obtain full arch related lists
There is a comprehensive test unit that also checks the old behaviour (also incorporating the old static lookup table).
This is the final state, passing the CI, until explicit review-requests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vvchernov commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "vvchernov (via GitHub)" <gi...@apache.org>.
vvchernov commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1714358633
Hello @elvin-n! May be second opinion from your to double check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao merged pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao merged PR #15685:
URL: https://github.com/apache/tvm/pull/15685
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on a diff in pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #15685:
URL: https://github.com/apache/tvm/pull/15685#discussion_r1321331750
##########
python/tvm/target/x86.py:
##########
@@ -16,127 +16,19 @@
# under the License.
"""Common x86 related utilities"""
from .._ffi import register_func
-from .target import Target
+from . import _ffi_api
-@register_func("tvm.target.x86.target_has_sse41")
-def target_has_sse41(target):
- return (
- target_has_sse42(target)
- or target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "btver2",
- "penryn",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_sse42")
-def target_has_sse42(target):
- return (
- target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "silvermont",
- "slm",
- "goldmont",
- "goldmont-plus",
- "tremont",
- "nehalem",
- "corei7",
- "westmere",
- "bdver1",
- "bdver2",
- "bdver3",
- "x86-64-v2",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx")
-def target_has_avx(target):
- return (
- target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target in {"sandybridge", "corei7-avx", "ivybridge", "core-avx-i"}
- )
-
-
-@register_func("tvm.target.x86.target_has_avx2")
-def target_has_avx2(target):
- return (
- target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "haswell",
- "core-avx2",
- "broadwell",
- "skylake",
- "bdver4",
- "znver1",
- "znver2",
- "znver3",
- "x86-64-v3",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx512")
-def target_has_avx512(target):
- return target in {
- "skylake-avx512",
- "skx",
- "knl",
- "knm",
- "x86-64-v4",
- "cannonlake",
- # explicit enumeration of VNNI capable due to collision with alderlake
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- }
-
-
-@register_func("tvm.target.x86.target_has_vnni")
-def target_has_vnni(target):
- return target in {
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- "alderlake",
- }
-
-
-@register_func("tvm.target.x86.target_has_amx")
-def target_has_amx(target):
- return target in {
- "sapphirerapids",
- }
+@register_func("tvm.target.x86.target_has_feature")
+def target_has_feature(feature, target=None):
+ return _ffi_api.llvm_x86_has_feature(feature, target)
@register_func("tvm.topi.x86.utils.get_simd_32bit_lanes")
def get_simd_32bit_lanes():
- mcpu = Target.current().mcpu
fp32_vec_len = 4
- if target_has_avx512(mcpu):
+ if target_has_feature("avx512bw"):
Review Comment:
Beside our discuss in the main thread, highlighting here the information:
* clearer view on what ```avx512bw``` provides from: [llvm/clang/Basic/BuiltinsX86.def#L1057-L1058](https://github.com/llvm/llvm-project/blob/release/17.x/clang/include/clang/Basic/BuiltinsX86.def#L1057-L1058)
I'l leave this change-request unresolved until approval in the main thread on this topic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1711439202
> The CI fails because the LLVM version on CI is pretty low (==10). I'm curious if there's any variant of this API on LLVM 10? If not, we should bump LLVM to 15 or 16
Folks,
@junrushao ,
Yes, I am aware of this ```llvm<=10``` issue, so ```llvm==11``` would be the minimum (tested).
Looking at ```llvm<=10``` to see other way (little +extra code costs) of tapping differently into their API.
I am strongly opting for a backward compatibility for this case.
The API fracture, at a first glance:
* ```llvm>=11``` expose/uses a [c_struct](https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0/llvm/lib/TargetParser/X86TargetParser.cpp#L119) + a nice func ```X86::getFeaturesForCPU``` to it.
* ```llvm<=10``` expose a [tablegen](https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0/llvm/lib/Target/X86/X86.td#L514) (same info) so need to see where it ends to use that via some API.
Allow a little time (1-2 day) to investigate a way going down ```llvm<11```, then I'll be back with the results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1717167569
@Qianshui-Jiang ,
Thank you much for your time & clarifications !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1714514112
>"F" would stand for foundation, probably avx512bw implies that CPU also have avx512f (but not vice-versa)
>One example of partial avx512 is the "knl" having the avx512f (foundation) part but not having the avx512bw part.
@vvchernov ,
One more informal expreriment:
* I made this experiment below, and results disprove that ```avx512bw``` implies ```avx512f``` (aka "foundation part").
* The ```x86-64-v4``` popping up is kind of "arch-generic", so let's enforce ```avx512bw``` && ```avx512f``` presence.
* For this ODD case, something like ```llvm -mcpu=x86-64-v4 -mattr=+avx512f``` passes the test below (checked).
```
$ cat ./tvm-check-avx512bw.py
#!/usr/bin/python3
import tvm
from tvm.target import codegen
from tvm.target.x86 import target_has_features
for mcpu in codegen.llvm_x86_get_archlist():
with tvm.target.Target("llvm -mcpu=%s" % mcpu):
if target_has_features("avx512bw"):
has_avx512f = target_has_features("avx512f")
print("ARCH [%s] having `avx512bw` has avx512f=[%i]" % (mcpu, has_avx512f))
$ ./tvm-check-avx512bw.py
ARCH [skylake-avx512] having `avx512bw` has avx512f=[1]
ARCH [skx] having `avx512bw` has avx512f=[1]
ARCH [cascadelake] having `avx512bw` has avx512f=[1]
ARCH [cooperlake] having `avx512bw` has avx512f=[1]
ARCH [cannonlake] having `avx512bw` has avx512f=[1]
ARCH [icelake-client] having `avx512bw` has avx512f=[1]
ARCH [rocketlake] having `avx512bw` has avx512f=[1]
ARCH [icelake-server] having `avx512bw` has avx512f=[1]
ARCH [tigerlake] having `avx512bw` has avx512f=[1]
ARCH [sapphirerapids] having `avx512bw` has avx512f=[1]
ARCH [graniterapids] having `avx512bw` has avx512f=[1]
ARCH [graniterapids-d] having `avx512bw` has avx512f=[1]
ARCH [emeraldrapids] having `avx512bw` has avx512f=[1]
ARCH [znver4] having `avx512bw` has avx512f=[1]
ARCH [x86-64-v4] having `avx512bw` has avx512f=[0] <--- ODD !!! (but pass with -mattr=+avx512f).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "junrushao (via GitHub)" <gi...@apache.org>.
junrushao commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1710896690
The CI fails because the LLVM version on CI is pretty low (==10). I'm curious if there's any variant of this API on LLVM 10? If not, we should bump LLVM to 15 or 16
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vvchernov commented on a diff in pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "vvchernov (via GitHub)" <gi...@apache.org>.
vvchernov commented on code in PR #15685:
URL: https://github.com/apache/tvm/pull/15685#discussion_r1321006825
##########
python/tvm/target/x86.py:
##########
@@ -16,127 +16,19 @@
# under the License.
"""Common x86 related utilities"""
from .._ffi import register_func
-from .target import Target
+from . import _ffi_api
-@register_func("tvm.target.x86.target_has_sse41")
-def target_has_sse41(target):
- return (
- target_has_sse42(target)
- or target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "btver2",
- "penryn",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_sse42")
-def target_has_sse42(target):
- return (
- target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "silvermont",
- "slm",
- "goldmont",
- "goldmont-plus",
- "tremont",
- "nehalem",
- "corei7",
- "westmere",
- "bdver1",
- "bdver2",
- "bdver3",
- "x86-64-v2",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx")
-def target_has_avx(target):
- return (
- target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target in {"sandybridge", "corei7-avx", "ivybridge", "core-avx-i"}
- )
-
-
-@register_func("tvm.target.x86.target_has_avx2")
-def target_has_avx2(target):
- return (
- target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "haswell",
- "core-avx2",
- "broadwell",
- "skylake",
- "bdver4",
- "znver1",
- "znver2",
- "znver3",
- "x86-64-v3",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx512")
-def target_has_avx512(target):
- return target in {
- "skylake-avx512",
- "skx",
- "knl",
- "knm",
- "x86-64-v4",
- "cannonlake",
- # explicit enumeration of VNNI capable due to collision with alderlake
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- }
-
-
-@register_func("tvm.target.x86.target_has_vnni")
-def target_has_vnni(target):
- return target in {
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- "alderlake",
- }
-
-
-@register_func("tvm.target.x86.target_has_amx")
-def target_has_amx(target):
- return target in {
- "sapphirerapids",
- }
+@register_func("tvm.target.x86.target_has_feature")
+def target_has_feature(feature, target=None):
+ return _ffi_api.llvm_x86_has_feature(feature, target)
@register_func("tvm.topi.x86.utils.get_simd_32bit_lanes")
def get_simd_32bit_lanes():
- mcpu = Target.current().mcpu
fp32_vec_len = 4
- if target_has_avx512(mcpu):
+ if target_has_feature("avx512bw"):
Review Comment:
Are you sure that it should be check on avx512bw only? Here and below in the code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #15685:
URL: https://github.com/apache/tvm/pull/15685#issuecomment-1713463565
> Hello @cbalint13! Thank you for big work and good improvement and unification of target check. For me it looks like that "avx512bw" is not the best name to split usual avx512 (e.g. for skylake) and avx512 with VNNI (e.g. for cascadelake), may be return "avx512", I do not see places where it uses in other context
@vvchernov ,
Good question regarding "avx512bw" !
Please help me with a double check on these statements below:
* To my understanding [needed in tvm](https://github.com/search?q=repo%3Aapache%2Ftvm%20llvm.x86.avx512.pmaddubs&type=code) ```.avx512.pmaddubs.w.512``` ,```.avx512.pmaddw.d.512``` are in ```avx512bw``` subset.
* See for reference: https://reviews.llvm.org/D11351 , so the ```avx512bw``` flag is what we seek within LLVM.
There are also other avx512 subsets, but none holding instructions for our topi/tir intrinsics, on their name here:
```
"avx512vl", "avx512dq", "avx512cd", "avx512er", "avx512pf", "avx512vbmi", "avx512ifma",
"avx5124vnniw", "avx5124fmaps", "avx512vpopcntdq", "avx512vbmi2", "gfni", "vpclmulqdq",
"avx512vnni", "avx512bitalg", "avx512bf16"
```
---
Some arches investigated here:
* The "skylake", "cascadelake" have it:
```
print( _ffi_api.llvm_x86_get_features("skylake") )
["cmov", "mmx", "popcnt", "sse", "sse2", "sse3", "ssse3", "sse4.1", "sse4.2",
"avx", "avx2", "fma", "bmi", "bmi2", "aes", "pclmul", "adx", "clflushopt", "cx16",
"cx8", "crc32", "f16c", "fsgsbase", "fxsr", "invpcid", "lzcnt", "movbe", "prfchw",
"rdrnd", "rdseed", "sahf", "sgx", "x87", "xsave", "xsavec", "xsaveopt", "xsaves"]
print( _ffi_api.llvm_x86_get_features("cascadelake") )
["cmov", "mmx", "popcnt", "sse", "sse2", "sse3", "ssse3", "sse4.1", "sse4.2",
"avx", "avx2", "fma", "avx512f", "bmi", "bmi2", "aes", "pclmul",
"avx512vl", "avx512bw", "avx512dq", "avx512cd", "avx512vnni",
"adx", "clflushopt", "clwb", "cx16", "cx8", "crc32", "f16c", "fsgsbase",
"fxsr", "invpcid", "lzcnt", "movbe", "pku", "prfchw", "rdrnd", "rdseed",
"sahf", "x87", "xsave", "xsavec", "xsaveopt", "xsaves"]
```
* The "knl" which have some ```avx512*``` **but not** the desired ```avx512bw```:
```
print( _ffi_api.llvm_x86_get_features("knl") )
["cmov", "mmx", "popcnt", "sse", "sse2", "sse3", "ssse3", "sse4.1", "sse4.2",
"avx", "avx2", "fma", "avx512f", "bmi", "bmi2", "aes", "pclmul",
"avx512cd", "avx512er", "avx512pf", "adx", "cx16", "cx8", "crc32", "f16c",
"fsgsbase", "fxsr", "invpcid", "lzcnt", "movbe", "prefetchwt1", "prfchw",
"rdrnd", "rdseed", "sahf", "x87", "xsave", "xsaveopt"]
```
* The odd "alderlake" have ```avxvnni``` **but not** ```avx512bw``` **either** ```avx512vnni```:
```
print( _ffi_api.llvm_x86_get_features("knl") )
["cmov", "mmx", "popcnt", "sse", "sse2", "sse3", "ssse3", "sse4.1", "sse4.2",
"fma", "bmi", "bmi2", "aes", "pclmul", "gfni", "vpclmulqdq", "adx", "cldemote",
"clflushopt", "clwb", "cx16", "cx8", "crc32", "f16c", "fsgsbase", "fxsr", "invpcid",
"widekl", "lzcnt", "movbe", "movdir64b", "movdiri", "pconfig", "pku", "prfchw",
"ptwrite", "rdpid", "rdrnd", "rdseed", "sahf", "serialize", "sgx", "sha", "shstk",
"vaes", "waitpkg", "x87", "xsave", "xsavec", "xsaveopt", "xsaves", "hreset",
"avxvnni"]
```
BTW, "alderlake" here, is the only "strangeness" having ```avxvnni``` but not ```avx512vnni```.
* The "sapphirerapids" have "avx512bw" "avx512vnni" , and "amx-int8" among with lot of new "interesting goodies".
```
print( _ffi_api.llvm_x86_get_features("sapphirerapids") )
["cmov", "mmx", "popcnt", "sse", "sse2", "sse3", "ssse3", "sse4.1", "sse4.2", "avx", "avx2", "fma",
"avx512f", "bmi", "bmi2", "aes", "pclmul", "avx512vl", "avx512bw", "avx512dq", "avx512cd",
"avx512vbmi", "avx512ifma", "avx512vpopcntdq", "avx512vbmi2", "gfni", "vpclmulqdq",
"avx512vnni", "avx512bitalg", "avx512bf16", "adx", "amx-bf16", "amx-int8", "amx-tile",
"cldemote", "clflushopt", "clwb", "cx16", "cx8", "crc32", "enqcmd", "f16c", "fsgsbase",
"fxsr", "invpcid", "lzcnt", "movbe", "movdir64b", "movdiri", "pconfig", "pku", "prfchw",
"ptwrite", "rdpid", "rdrnd", "rdseed", "sahf", "serialize", "sgx", "sha", "shstk", "tsxldtrk",
"uintr", "vaes", "waitpkg", "wbnoinvd", "x87", "xsave", "xsavec", "xsaveopt", "xsaves",
"avx512fp16", "avxvnni"]
```
---
I put **Cc** @Qianshui-Jiang (author of #13642) he might help with a second opinion in this too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on a diff in pull request #15685: [Target][TOPI] Use LLVM for x86 CPU feature lookup
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #15685:
URL: https://github.com/apache/tvm/pull/15685#discussion_r1322321004
##########
python/tvm/target/x86.py:
##########
@@ -16,127 +16,19 @@
# under the License.
"""Common x86 related utilities"""
from .._ffi import register_func
-from .target import Target
+from . import _ffi_api
-@register_func("tvm.target.x86.target_has_sse41")
-def target_has_sse41(target):
- return (
- target_has_sse42(target)
- or target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "btver2",
- "penryn",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_sse42")
-def target_has_sse42(target):
- return (
- target_has_avx(target)
- or target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "silvermont",
- "slm",
- "goldmont",
- "goldmont-plus",
- "tremont",
- "nehalem",
- "corei7",
- "westmere",
- "bdver1",
- "bdver2",
- "bdver3",
- "x86-64-v2",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx")
-def target_has_avx(target):
- return (
- target_has_avx2(target)
- or target_has_avx512(target)
- or target_has_vnni(target)
- or target in {"sandybridge", "corei7-avx", "ivybridge", "core-avx-i"}
- )
-
-
-@register_func("tvm.target.x86.target_has_avx2")
-def target_has_avx2(target):
- return (
- target_has_avx512(target)
- or target_has_vnni(target)
- or target
- in {
- "haswell",
- "core-avx2",
- "broadwell",
- "skylake",
- "bdver4",
- "znver1",
- "znver2",
- "znver3",
- "x86-64-v3",
- }
- )
-
-
-@register_func("tvm.target.x86.target_has_avx512")
-def target_has_avx512(target):
- return target in {
- "skylake-avx512",
- "skx",
- "knl",
- "knm",
- "x86-64-v4",
- "cannonlake",
- # explicit enumeration of VNNI capable due to collision with alderlake
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- }
-
-
-@register_func("tvm.target.x86.target_has_vnni")
-def target_has_vnni(target):
- return target in {
- "cascadelake",
- "icelake-client",
- "icelake-server",
- "rocketlake",
- "tigerlake",
- "cooperlake",
- "sapphirerapids",
- "alderlake",
- }
-
-
-@register_func("tvm.target.x86.target_has_amx")
-def target_has_amx(target):
- return target in {
- "sapphirerapids",
- }
+@register_func("tvm.target.x86.target_has_feature")
+def target_has_feature(feature, target=None):
+ return _ffi_api.llvm_x86_has_feature(feature, target)
@register_func("tvm.topi.x86.utils.get_simd_32bit_lanes")
def get_simd_32bit_lanes():
- mcpu = Target.current().mcpu
fp32_vec_len = 4
- if target_has_avx512(mcpu):
+ if target_has_feature("avx512bw"):
Review Comment:
Until further change requests, let's solve this with the resolution (avx512bw && avx512f) from the main thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org