Stream: git-wasmtime

Topic: wasmtime / issue #5490 Update Intel x86 CPU presets to ma...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 26 2022 at 19:24):

bjorn3 commented on issue #5490:

I think this looks good. Maybe add a comment at the top of the file referencing the exact file+commit you used? The github url you linked would work.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 30 2022 at 02:11):

jameysharp commented on issue #5490:

Thank you, at a quick glance this looks great! A bunch of people are out due to holidays, but if nobody else gets to it first I intend to review this next week. Feel free to comment again if you don't hear anything by the end of next week.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 03 2023 at 22:10):

jameysharp commented on issue #5490:

This seems like the right direction!

My only quibble is that I think we should only include 64-bit CPUs (the ones that have FeatureX86_64 in LLVM). We should be able to generate code for any x86 CPU running in 64-bit mode, and if we can't right now I think we're at least close. But I believe we'd need to do a lot more work to support 32-bit mode, so I think it makes sense to just fail if someone requests a target that can only support 32-bit mode.

I haven't yet checked that, for each target we already supported, the set of features is still correct. I believe you probably got it right, but the LLVM tables are hard enough to read that I think it's worth double-checking, just in case.

Overall, I think this is the right patch. If you could remove those 32-bit CPUs I'd appreciate it, and hopefully I'll get to double-checking the features lists soon, or talk somebody else into doing it. :grin:

view this post on Zulip Wasmtime GitHub notifications bot (Jan 11 2023 at 21:20):

MozarellaMan commented on issue #5490:

Thank you for the comments! Sorry, I was away a bit. Will have a look at addressing what you've brought up. Thanks for taking the time to double check - I didn't realise the difference regarding the feature implication

view this post on Zulip Wasmtime GitHub notifications bot (Jan 11 2023 at 21:27):

MozarellaMan edited a comment on issue #5490:

Thank you for the comments! Sorry, I was away a bit. Will have a look at addressing what you've brought up. Thanks for taking the time to double check - I didn't realise the difference regarding the feature implication. And of course, these are for x86 presets :face_palm:

Last updated: Nov 22 2024 at 16:03 UTC