but it is to AMD engineers's job or at least people willing to run benchmarks on AMD cpus, to ensure that "it works just as well on an AMD CPU", not to Intel.
With an open source project like this, every contributor has a responsibility towards all users of that code. That means that if Intel makes a change, they are also responsible for AMD support. Putting a generic optimization behind a vendor detection is not acceptable in the community.
However, as others' have mentioned, this previously might not have benefited AMD, despite technically being supported, so it was likely meant well at the time of implementation. Intel is usually quite good players when it comes to open source contributions.
They didn't harm in any way the AMD users, just didn't invest in their well being. You can't expect intel to benchmark if a feature would be better or not on amd, that's not its role. It submitted a patch that was good for its users. Up to amd to benchmark it for their users. It would still be a lot less expensive to benchmark it and enable it for amd, than develop it from scratch as intel.
> You can't expect intel to benchmark if a feature would be better or not on amd, that's not its role.
Why not? If I was maintainer I wouldn't accept the patch, I'd ask the authors to test on AMD as well. Intel is well funded and glibc is not their project. If they want glibc to include optimizations for their platform they can do the minimal level of effort to see if it can be enabled on AMD as well.
And then, after said minimal amount of effort, it turns out that the patch actually hurts performance on some AMD CPUs, and people bring out pitchforks for Intel intentionally slowing down AMD?
It seems to me that the current approach is the most conservative: limit the impact to what you know best. Leave it to the experts of the other system to do whatever is needed.
How many AMD CPU configurations exist? Does a minimal effort require regression testing all of them? On how many benchmarks? How do you know that there aren’t untested potholes in AMD CPUs that you may not be aware of?
In the end, the result of this feature is a performance improvement for some and status quo (no perf regression) for everybody else. It’s a net benefit with no downside in absolute terms.
In addition, it provides a free roadmap for AMD or its users on how to get the same benefit as well.
The potential backlash of enabling this for AMD as well by somebody of Intel (“active sabotage!!!”) is much larger than this tempest in a teacup where AMD is currently missing out on something.
The same benchmarks you used to justify adding it in the first place, on their current model and ideally a slightly older one, would be sufficient.
> The potential backlash of enabling this for AMD as well by somebody of Intel (“active sabotage!!!”) is much larger than this tempest in a teacup where AMD is currently missing out on something.
Historically Intel has been found to create suboptimal code paths in its own compiler, in addition to not enabling features supported on competitor's platform, in its own compiler.
What happens if the implementation is slower but still works on AMD? Is Intel responsible for also performance testing and determining if/when to disable an implementation on a given chip? You're putting a lot of burden on Intel to do extensive testing and also not protecting them from criticism if a change is suboptimal for a competitor.
I think it's fine to submit a patch that's known to be good for a subset of CPUs and perhaps it should be tagged for another maintainer (e.g. AMD) to review and contribute to as well.
If the original patch contained a benchmark showing that it was slower on the current AMD processors then this would be a reasonable argument.
It did not, because no such testing was done.
I think there's a pretty low baseline of effort they can put in. I also think that if they enabled new code paths for newer instructions that turned out to be slower on AMD, very few people would claim this was Intel being evil. Most would blame AMD for selling a defective product.
That's true. Whoever accepted the patches should have thought of that. But that happens, right? I don't think this is newsworthy at all.
Bad implementations happen, people find better ways, they refactor and improve things. This holy war some people want to have between AMD and Intel is quite boring.
> You can't expect intel to benchmark if a feature would be better or not on amd, that's not its role.
It is, in facts, its role as a major contributor.
And they do, in fact, benchmark and fix AMD as part of its open source efforts, as they should. Case in point: https://github.com/OpenVisualCloud/SVT-VP9/pull/48 (Intel developer contributing AVX2 improvements, specifically mentioned as AMD Epyc improvements).
They can cater only to their own devices when the code only applies to those devices (e.g. the i915 graphics driver). In generic code paths, they must cater to all users. Adding optimization using generic features that have generic flags, but hiding them behind vendor detection, is borderline malicious. The developers here are well aware that there is a feature flag they should check instead.
> With an open source project like this, every contributor has a responsibility towards all users of that code.
What ? no. Do IBM engineers have responsibility towards Qualcomm engineers when they commit IBM patches under IBM-named flags to the linux kernel ? What happens if there's a new CPU company in two years that would also happen to work fine with these flags ?
> Do IBM engineers have responsibility towards Qualcomm engineers
Yes, in every way. However, with Qualcomm not having any PowerPC architectures, there isn't much harm that could be done. And for reference, Intel also tests AMD as they should, and even submits performance improvements for AMD as they should.
But this is quite bad: Instead of: `if (supports(generic_feature)) { do_with_generic_feature(); } else { slow_approach(); }`, they did `if (intel_haswell) { do_with_generic_feature(); } else { slow_approach(); }`.
The only time where that is acceptable from as big a contributor as Intel, is if they tested and concluded that other CPUs were actually slower using this feature.
> What happens if there's a new CPU company in two years that would also happen to work fine with these flags ?
That is exactly why the feature flags exist in this generic code! You can query what instruction sets are available on the CPU. Vendor-specific code should only be added to deal with product-specific defect.
The glibc core maintainers have such responsibility, yes, but it's too much burden for occasional contributors focusing on a single issue or a particular architecture. Setting the bar too high would make glibc lose valuable patches from both Intel and AMD.
A reasonable compromise is requiring architecture-specific contributions to at least do no harm to other vendors, and to not increase maintenance costs for core developers by duplicating code.
In this light, AMD engineers wanting to enable the haswell optimizations for their processors would be asked to share the existing code rather than copy-paste it. Intel engineers would participate in the public review to ensure AMD patches don't cause regressions for Haswell. If they have contributed testcases, they will demand that AMD patches pass them on all supported architectures before being merged.
This is pretty standard in all open source projects with multiple stakeholders.
If intel is intentionally checking based on vendor rather than feature, that is scummy. Glibc should not accept patches that do this check. Instead requiring such checks to be based on feature detection.
why ? AMD just has to do the same. I want to be able to use the CPU I buy to its maximum capability.