velox icon indicating copy to clipboard operation
velox copied to clipboard

Remove CPU_TARGET and automate arch inference

Open majetideepak opened this issue 1 year ago • 1 comments

majetideepak avatar Jun 28 '24 14:06 majetideepak

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 004468db930f3e8dc6bcd186bea06f836e062729
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/668e8527b0928c0008eccb3f

netlify[bot] avatar Jun 28 '24 14:06 netlify[bot]

My one concern with this change is that we cannot force builds to use a particular architecture (for e.g you want an SSE only build

@kgpai as mentioned in the other comment, avx has been available since a long time now. I doubt there is a use case to build only for sse.

majetideepak avatar Jul 09 '24 14:07 majetideepak

We can always add the CPU_TARGET back if there is a use for it.

majetideepak avatar Jul 09 '24 15:07 majetideepak

@majetideepak Should we also change the documentation as part of this change (The README still refers to this )

kgpai avatar Jul 09 '24 16:07 kgpai

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Jul 10 '24 17:07 facebook-github-bot

@pedroerp merged this pull request in facebookincubator/velox@7f2d7adaa1544c72129fd1c8d0766755ff354455.

facebook-github-bot avatar Jul 10 '24 18:07 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit 7f2d7ada.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Jul 10 '24 18:07 conbench-facebook[bot]