near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.32k stars 621 forks source link

Figure out what kind of machine we're targetting (and document it) #5291

Open nagisa opened 2 years ago

nagisa commented 2 years ago

Right now wasmer unconditionally emits some AVX1 instructions, which is typically considered to be outside of what x86_64 means. More precisely this would fall into the newly coined x86_64-v3 feature level as described here.

x86_64-v3 is approximately a Haswell or later for Intel and 1st gen Zen or later for AMD, which seems old enough to be a reasonable feature set to target. However, we should explicitly document this choice and ensure we don't exceed this spec (by e.g. using the AVX512 instructions).

We should also have code to ensure that the machine nearcore starts on has the prerequisite features rather than “sometimes” crashing when encountering a contract with an esoteric instruction or instruction sequence.

further reading https://near.zulipchat.com/#narrow/stream/295306-nearinc.2Fcontract-runtime/topic/Target.20machine/near/261621583 cc #5105

bowenwang1996 commented 2 years ago

ensure we don't exceed this spec (by e.g. using the AVX512 instructions).

Is this actually what we want? I believe there may be places where we could benefit from having AVX512 instructions. We may want to evaluate this holistically before committing to a specific version of x86_64

nagisa commented 2 years ago

Conditionally, as detected at runtime (via e.g. is_x86_feature_detected), we could use pretty much any instruction set extensions we want (and we likely do so through primitives such as memcpy).

This issue is more about the baseline requirements (what kind of features can we expect to be available unconditionally). AVX512 is not supported on the E cores of the most recent Intel's Alder Core architecture nor it is available on the most recent AMD's Zen, so it is probably a bad idea to force a requirement on a feature like this.

bowenwang1996 commented 2 years ago

I see. Makes sense

nagisa commented 2 years ago

In practice it seems that wasmer emits instructions requiring following features: popcnt, cmov, sse/sse2 and avx.

cmov and sse/sse2 are present in the base x86_64 target specification and is considered to be universally available. popcnt is only available in the x86_64-v2 feature level and avx1 only becomes available with x86_64-v3.

x86_64-v2 + avx is perhaps the best way to accurately describe the required features today.

x86_64-v3 feature level, compared to x86_64-v2+avx requires support for avx2, bmi1/2, fma and lzcnt. As thus setting our required feature level to x86_64-v3 would “drop” support for at least the following processor families: Sandy Bridge (2011), Ivy Bridge (2012), Jaguar (2013), Puma (2014), Bulldozer (2011), Piledriver (2012), Steamroller (2014) and WuDaoKou.

From my investigation into this I can see a couple action points worth taking:

  1. Build neard with at least -Ctarget-features=+avx,+cmpxchg16b,+popcnt,+sse3,+ssse3,+sse4.1,+sse4.2,+sse4a;
    • This should give significant opportunity for LLVM to improve the codegen for neard itself;
    • However in order to do that we'll need to replace some of the feature checks that use is_x86_feature_supported with cpuid checks – is_x86_feature_detected will unconditionally return true if the feature is enabled via -Ctarget-features;
  2. We could consider requiring x86_64-v3 too – any architecture developed by Intel or AMD in past 6-7 years will run the code just fine too.
bowenwang1996 commented 2 years ago

2. We could consider requiring x86_64-v3 too – any architecture developed by Intel or AMD in past 6-7 years will run the code just fine too.

Yeah this seems reasonable to me

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

nagisa commented 5 months ago

I'm not currently working on this, so unassigning. I don't think much has changed in our hardware requirements since the issue has been filled, but with regards to this specific point:

We should also have code to ensure that the machine nearcore starts on has the prerequisite features rather than “sometimes” crashing when encountering a contract with an esoteric instruction or instruction sequence.

I believe we're good and we do check for the required features currently. The rest of this issue remains relevant.