Add a test to help track wasm features over time

[alexcrichton]

This came up on Zulip where currently we have no notification channel when the generic CPU for WebAssembly changes in LLVM other than users hitting possible breakage. This is going to continue to evolve over time, so the purpose of this test is to provide a location to help indicate what to do when the generic CPU changes. The purpose of this test is to provide a nudge to notify various folks that something is changing and warrants release notes and such. Otherwise it's expected that the test should be pretty easy to update if it starts failing.

[jieyouxu]

Thanks for the test. I think this can cause some friction during LLVM bumps, but we definitely should be tracking wasm proposals being enabled. I'll cc the llvm wg for opinion.

[jieyouxu]

Hi @rust-lang/wg-llvm, would you be okay if we added a wasm proposals test that would fail if LLVM changes the set of enabled wasm proposals for the wasm generic CPU? The intent is to help detect if the set of enabled wasm proposals change, so we can be aware of the changes and tag relnotes for users as suitable.

cc @nikic and @DianQK specifically as I know both of you do quite a bit of the llvm bumps.

[jieyouxu]

If the wg-llvm members don't opposite this change, then you can r=me. EDIT: see below

[DianQK]

When a wholly new feature is added directly, this test case will still pass. Let's query the default enabled features from LLVM. :)

I think we can make this kind of check more generic by extending the output of --print target-features. Then, write a FileCheck.

Maybe like this:

extern "C" size_t LLVMRustGetTargetEnabledFeaturesCount(LLVMTargetMachineRef TM) {
  const TargetMachine *Target = unwrap(TM);
  const MCSubtargetInfo *MCInfo = Target->getMCSubtargetInfo();
  const auto FeatTable =
      MCInfo->getEnabledProcessorFeatures();
  return FeatTable.size();
}

extern "C" void LLVMRustGetTargetEnabledFeature(LLVMTargetMachineRef TM, size_t Index,
                                         const char **Feature,
                                         const char **Desc) {
  const TargetMachine *Target = unwrap(TM);
  const MCSubtargetInfo *MCInfo = Target->getMCSubtargetInfo();
  const auto FeatTable =
      MCInfo->getEnabledProcessorFeatures();
  const SubtargetFeatureKV Feat = FeatTable[Index];
  *Feature = Feat.Key;
  *Desc = Feat.Desc;
}

This code modified from

rust/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

Lines 366 to 384 in 249df9e

	extern "C" size_t LLVMRustGetTargetFeaturesCount(LLVMTargetMachineRef TM) {
	const TargetMachine *Target = unwrap(TM);
	const MCSubtargetInfo *MCInfo = Target->getMCSubtargetInfo();
	const ArrayRef<SubtargetFeatureKV> FeatTable =
	MCInfo->getAllProcessorFeatures();
	return FeatTable.size();
	}
	extern "C" void LLVMRustGetTargetFeature(LLVMTargetMachineRef TM, size_t Index,
	const char **Feature,
	const char **Desc) {
	const TargetMachine *Target = unwrap(TM);
	const MCSubtargetInfo *MCInfo = Target->getMCSubtargetInfo();
	const ArrayRef<SubtargetFeatureKV> FeatTable =
	MCInfo->getAllProcessorFeatures();
	const SubtargetFeatureKV Feat = FeatTable[Index];
	*Feature = Feat.Key;
	*Desc = Feat.Desc;
	}

.

[nikic]

Hi @rust-lang/wg-llvm, would you be okay if we added a wasm proposals test that would fail if LLVM changes the set of enabled wasm proposals for the wasm generic CPU? The intent is to help detect if the set of enabled wasm proposals change, so we can be aware of the changes and tag relnotes for users as suitable.

In principle this is fine, but this should be a no_core test and it will have to be conditional on the LLVM version. E.g. the current test would fail on LLVM 18 if we actually ran it there (which we don't because it's not no_core).

Or alternatively we'd have to explicitly add features on the rust side to make it independent of the LLVM version. This kind of makes sense to have consistent behavior, but on the other hand we sometimes just can't do this, e.g. because enabling +multivalue on LLVM 18 would do something entirely different that we do not actually want.

[jieyouxu]

This kind of makes sense to have consistent behavior, but on the other hand we sometimes just can't do this, e.g. because enabling +multivalue on LLVM 18 would do something entirely different that we do not actually want.

Wait, am I reading this correctly that +multivalue on LLVM 18 and LLVM 19 mean different things?

[nikic]

This kind of makes sense to have consistent behavior, but on the other hand we sometimes just can't do this, e.g. because enabling +multivalue on LLVM 18 would do something entirely different that we do not actually want.

Wait, am I reading this correctly that +multivalue on LLVM 18 and LLVM 19 mean different things?

Yes, +multivalue on LLVM 18 changes the ABI, on LLVM 19 it is essentially a no-op if you don't explicitly request a different ABI with a different flag.

[jieyouxu]

I... Thanks for this crucial info, this makes it much more complicated...

[alexcrichton]

To me this is a relatively minor point centered around making it more reliable that we know when to add release notes for changes, so in that sense I wouldn't consider it worth more C++ or a custom test harness or something like that. That's a good point about LLVM versions, and I can mark this LLVM 19+, but that's also a good point about mixing and matching LLVM versions means that this is likely to just be a noisy test in the end and cause undue churn/pain.

Given that I think I'll actually go ahead and close this. It's probably best tracked externally to not add more burden on LLVM changes in-repo.