Change CPU model/feature parsing to match LLVM's logic
See: https://github.com/ziglang/zig/pull/21501#issuecomment-2375597396
10551f6a1779c7a5e73f12c7d8d4395f9ea6f3ea should be reverted once this is implemented.
For reference/discussion I'll repeat my point from https://github.com/ziglang/zig/pull/21501#issuecomment-2387999763 here, i.e. I think we can provide stricter parsing that errors in situations that might be unintentional. (If we value compatibility by default, we could have an opt-in flag for this stricter check.)
examples / tentative rules:
- When conflicting features
+a...-aare requested, while it makes sense to pick the rightmost, it might be a typo. All but the last mention of the feature are superfluous and could be flagged. 1.1 (For enabling the use case of "all dependencies ofa, but notaitself, we could allow+a-aonly directly in sequence.) - If
-a+bis requested, butais implied by / included in the setb, by rightmost logicashould be re-enabled, even though that goes against the explicit request. (The user might not know thatbincludes/requiresa.) The solution would be to for the user to reorder this to+b-a, if that's their intent. - The same as 2. with swapped signs
+a-b,-b+ahas clearer intent. - If
+b-ais requested, butais required forb, thenbisn't really satisfied. Again this could be an error / the user being unaware of this dependency, which we could flag. (Not sure if this can/should be supported in strict parsing mode: We could instead print out the specific features ofb, and users could specify only the ones they mean to include.)
I'm not opposed to having such rules to prevent likely mistakes, but I think we should consider that to be a follow-up enhancement, i.e. I suggest filing a separate issue with reference to this one.
My main concern in this particular issue is one of correctness; at the moment, there are combinations of CPU feature options that effectively result in miscompilations from the user's perspective, with no obvious way to resolve them. So I see aligning our parsing with LLVM to be a strict improvement even without making the parser flag mistakes.
One effect of fixing this is that this API shape
https://github.com/ziglang/zig/blob/73dcd1914071984c5a2e7c195212404824dbfb9e/lib/std/Target/Query.zig#L11-L15
no longer makes much sense because the order is lost. Yet we have real uses of this in the repo:
https://github.com/ziglang/zig/blob/73dcd1914071984c5a2e7c195212404824dbfb9e/build.zig#L586-L590 https://github.com/ziglang/zig/blob/73dcd1914071984c5a2e7c195212404824dbfb9e/src/Compilation.zig#L4002-L4020 https://github.com/ziglang/zig/blob/73dcd1914071984c5a2e7c195212404824dbfb9e/test/llvm_targets.zig#L338-L344
Need to think on how the API should look so that the order is maintained and it remains nice to use. It's complicated by the fact that, ideally, we don't want std.Target.Query to contain potentially dynamically-allocated memory (e.g. you could imagine having cpu_features: []const struct { enum { add, sub }, std.Target.Cpu.Feature.Set.Index }). I don't see a way around that though.