Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cpuinfo,fbgemm,xnnpack] update cpuinfo to latest #23944

Merged
merged 14 commits into from
Apr 12, 2022
Merged

[cpuinfo,fbgemm,xnnpack] update cpuinfo to latest #23944

merged 14 commits into from
Apr 12, 2022

Conversation

luncliff
Copy link
Contributor

@luncliff luncliff commented Apr 2, 2022

What does your PR fix?

With pytorch/cpuinfo#69, cpuinfo now supports CMake find_package.
Remove the unofficial:: prefix from consumer ports.

Affected Ports

Which triplets are supported/not supported? Have you updated the CI baseline?

The current version didn't support the following triplets.

Not supported

  • arm64-windows

Does your PR follow the maintainer guide?

Yes.

@luncliff luncliff marked this pull request as draft April 2, 2022 14:54
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpuinfo/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpuinfo/vcpkg.json
  • ports/fbgemm/vcpkg.json
  • ports/nnpack/vcpkg.json
  • ports/qnnpack/vcpkg.json
  • ports/xnnpack/vcpkg.json

Valid values for the license field can be found in the documentation

@luncliff luncliff changed the title [cpuinfo] update to latest version [cpuinfo, fbgemm, xnnpack] update cpuinfo to latest Apr 2, 2022
@luncliff luncliff changed the title [cpuinfo, fbgemm, xnnpack] update cpuinfo to latest [cpuinfo,fbgemm,xnnpack] update cpuinfo to latest Apr 2, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/fbgemm/vcpkg.json

Valid values for the license field can be found in the documentation

@luncliff luncliff marked this pull request as ready for review April 2, 2022 15:28
@FrankXie05 FrankXie05 added the category:port-update The issue is with a library, which is requesting update new revision label Apr 6, 2022
@FrankXie05
Copy link
Contributor

cc @JackBoosY Could you help review it? Thanks in advance.

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

I'm still worried about the new support expressions, is there any evidence that they didn't support them before?

@luncliff
Copy link
Contributor Author

luncliff commented Apr 8, 2022

I'm still worried about the new support expressions, is there any evidence that they didn't support them before?

Looks like this is the source of the problem. Only x86 implementation for Windows.

https://github.com/pytorch/cpuinfo/blob/b40bae27785787b6dd70788986fd96434cf90ae2/CMakeLists.txt#L144-L146

The ARM folder doesn't contain a Windows platform folder. :(

https://github.com/pytorch/cpuinfo/tree/b40bae27785787b6dd70788986fd96434cf90ae2/src/arm

@JackBoosY
Copy link
Contributor

Looks like this is the source of the problem. Only x86 implementation for Windows.

Does this prove that x86 source code can be used to build under x64?

The ARM folder doesn't contain a Windows platform folder. :(

However, there is a arm api header: https://github.com/pytorch/cpuinfo/blob/5916273f79a21551890fd3d56fc5375a78d1598d/src/arm/api.h

@luncliff
Copy link
Contributor Author

luncliff commented Apr 11, 2022 via email

@JackBoosY
Copy link
Contributor

@luncliff You can remove the supports field and see what happened in pipeline tests.

@luncliff
Copy link
Contributor Author

luncliff commented Apr 11, 2022

It always select x86 implementation in Windows. Current master branch's cpuinfo port will fail with some identifier errors.

"description": "CPU INFOrmation library (x86/x86-64/ARM/ARM64, Linux/Windows/Android/macOS/iOS)",
"homepage": "https://github.com/pytorch/cpuinfo",
"license": "BSD-2-Clause",
"supports": "!(arm & windows) | (arm & uwp)",
Copy link
Contributor

Choose a reason for hiding this comment

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

cpuinfo:arm64-windows also fails to build in master branch.

Copy link
Contributor

@strega-nil-ms strega-nil-ms Apr 11, 2022

Choose a reason for hiding this comment

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

fwiw, I can build it on my machine as both arm64-windows and arm-windows, so I think this supports is wrong.

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Apr 11, 2022
@strega-nil-ms
Copy link
Contributor

Since I can build it on my machine as both arm64-windows and arm-windows, I believe the supports is wrong; let's see what CI says.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for cpuinfo but no changes to version or port version.
-- Version: 2022-04-02
-- Old SHA: db70f0810a8dec170297cd293ea3764cd8d58a0a
-- New SHA: c3aaed3cd5decef8974ea1e636b77b970b74515f
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Apr 11, 2022

It looks like the issue is that cpuinfo is broken on x64 targeting arm64; given that it can still build on arm64 targeting arm64, I do not want to put that in supports; it'd be better to fix it for real.

@luncliff, if you want to fix it, you can; otherwise, I'll do it tomorrow when I get to an x64 computer.

@strega-nil-ms strega-nil-ms added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 11, 2022
@luncliff
Copy link
Contributor Author

It looks like the issue is that cpuinfo is broken on x64 targeting arm64; given that it can still build on arm64 targeting arm64, I do not want to put that in supports; it'd be better to fix it for real.

I will wait for this. Removing the part can be done easily.

@strega-nil-ms
Copy link
Contributor

WFH again today, so I'll take a look tomorrow (sorry!)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for cpuinfo but no changes to version or port version.
-- Version: 2022-04-02
-- Old SHA: db70f0810a8dec170297cd293ea3764cd8d58a0a
-- New SHA: 1dc4937be47f2e534b92c5992892f33ccdbcd2cf
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Apr 12, 2022

NVM, I've figured it out: until pytorch/cpuinfo#82 is merged, arm[64]-windows is unsupported, so marking unsupported. When that's merged, we can do an update and only fail on arm32-windows :)

Merging once green.

@strega-nil-ms strega-nil-ms removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 12, 2022
@strega-nil-ms strega-nil-ms merged commit e809a42 into microsoft:master Apr 12, 2022
@luncliff luncliff deleted the port/cpuinfo branch April 12, 2022 21:08
BillyONeal added a commit that referenced this pull request Jan 18, 2023
* [onnx] feature: foxi

* https://github.com/houseroad/foxi
    * install the project's copyright (MIT)
* pytorch requires `foxi_loader`

The CMake target will be renamed to `onnxifi_*` for convenience.

* [onnx] force onnx/onnx_proto static in Windows

Checked the protject's CI logs.  It turned out onnx/onnx_proto are ALWAYS static.
Specify it in CMakeLists.txt because vcpkg configures `BUILD_SHARED_LIBS=ON`
If the triplet requires it.

There are no `ONNXIFI_ENABLE_EXT=ON` case.
Removed the misused build options in portfile.

Add port feature `protobuf-lite` which is in build option.

* [onnx] support windows static triplets

* remove SHARED for `onnxifi_wrapper` and `onnxifi_dummy`

* [onnx] fix wrong LICENSE install

* [onnx] remove feature 'foxi'

* also remove redundant part in patch files

* [libtorch] rework patch files

* [libtorch] config fixup ATen, Torch

* use `link_libraries` to vcpkg installed folder
  * future work may use library names without `find_library`
* update versions JSON to use `version-semver`

* [libtorch] make shared only

* Caffe2 is exported when BUILD_SHARED_LIBS

* [libtorch] remove headers after install

* [libtorch] rewrite patches and feature options

* checked osx / linux build

* [libtorch] use eigen3 always

* Caffe2 eigen_utils.h requires it

* [libtorch] error if BLAS feature collision

* [libtorch] remove !static

* [libtorch] replace vcpkg_find_acquire_program

* let's see python3 from find_program supports

* Dependency python3

* [libtorch] migrate works from luncliff/vcpkg-registry

* Update target version and dependencies
* Removed unsupported features

* [libtorch] misc fix, update version, baseline

* fix merge confict for 'onnx'

* [libtorch] install pip packages

* typing-extensions, pyyaml

* [libtorch] turn off Metal options

* [onnx] revert 'onnx' changes

* [libtorch] refine patches

* [libtorch] link with foxi_loader

* removed wrong onnx related source changes

* [libtorch] update git-tree

* [libtorch] reduce patch size

* [libtorch] find numa and activate USE_NUMA

* Update ports/libtorch/portfile.cmake

Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>

* Update ports/libtorch/portfile.cmake

Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>

* Update ports/libtorch/portfile.cmake

Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>

* [libtorch] fix mistype and update version JSON

* Add double quotes

* version

* Fix support expression

* version

* [libtorch] update cpuinfo usage

* #23944
* update version

* [tensorpipe] fix linux install

* find_package(Tensorpipe) may fail because of case mismatch

* [tensorpipe] update versions JSON

* [libtorch] fix feature failures

* [libtorch] remove CUDA feature

* [libtorch] giveup 'fbgemm' feature

* [libtorch] use mpi, openmpi in Linux

* [libtorch] fix glog link error

* [tensorpipe] bump port version

* see #23569

* [libtorch] fix patch list

* [libtorch] use official libuv config

* see #24745

* Update ports/libtorch/portfile.cmake

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>

* Update ports/libtorch/portfile.cmake

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>

* update versions JSON

* revert unnecessary 'nnpack' changes

* Update ports/libtorch/portfile.cmake

Co-authored-by: Adam Johnson <AdamJohnso@gmail.com>

* [libtorch] use vcpkg-get-python-packages

* [libtorch] provide path of python3

* Update ports/libtorch/portfile.cmake

Co-authored-by: Billy O'Neal <bion@microsoft.com>

* Fix version database.

* [libtorch] use openmpi in linux/osx

* [libtorch] update to v1.12.1

* [libtorch] find_program(python3, python)

* [libtorch] provide PYTHON_EXECUTABLE directly

* [xnnpack] update to 2022-02-17

* [xnnpack] use C11, C++11

* [libtorch] more patches, DISABLE_PARALLEL_CONFIGURE

* [libtorch] allow static torch_cpu build

* Revert "[libtorch] allow static torch_cpu build"

This reverts commit 5fd4ef0.

* [libtorch] find_package(BLAS)

* [libtorch] simplify Python3, NumPy option use

* [libtorch] fix install in Windows

* [libtorch] exclude torch_global_deps in Windows

* [libtorch] platform of nnpack feature

* [libtorch] fix MPI option in Windows

* [libtorch] fixing LNK1161

* [libtorch] fix some mistypes

* [libtorch] define NOMINMAX for c10

* [libtorch] disable vulkan feature in Windows

* ci.baseline.txt: allow libtorch failure

* fails with Visual Studio 2022 17.4.2
* requires 17.4.3+

* Enable testing port on Windows

* [caffe2] redirect to libtorch

* update baseline

Co-authored-by: JackBoosY <yuzaiyang@beyondsoft.com>
Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
Co-authored-by: Billy Robert O'Neal <bion@microsoft.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Co-authored-by: Adam Johnson <AdamJohnso@gmail.com>
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants