Add no_ features

[alexreinking]

The autoscheduler tuning loop uses get_host_target to remove AVX512 from targets for tuning. This means that we need to distribute this tiny binary along with the autotune_loop.sh script. We also have some features that are negative or enable/disable certain traits.

In the interest of simplifying deployment and making targets more algebraic, I propose changing the target parser to accept no_ as a prefix to any feature. Parsing left to right, a no_ feature ensures that the corresponding feature is disabled up to that point. So you can do things like $TARGET-asserts to ensure asserts are enabled.

There are some targets that already have no_ as a prefix. These are:

1. no_asserts
2. no_bounds_query
3. no_neon
4. no_runtime

These would have their enum values replaced with negatives of their current values. The minimum value would need to be set to 1 and feature 0 is a null feature that means nothing. If we want a string representation for it, we can use nil or none or null (bikeshedding ahoy). In any case no_nil == nil.

There is one other exception, {enable,disable}_llvm_loop_opts. We should put these on a deprecation plan. Add no_llvm_loop_opts and llvm_loop_opts, allow the old strings in the parser, and add enum aliases where appropriate.

The negative values scheme can be made efficient if we ensure that the null feature (0) is never set. Then the test for whether a feature is present becomes:

features[abs(bit)] == (bit > 0)

[steven-johnson]

SGTM, let's do this. Let's explicitly document that if a given feature is repeated multiple times in positive and negative fashion, the final version (parsing left to right) wins, ie host-debug-no_debug means that no_debug wins.

[zvookin]

If we want to go down this path, I would prefer that we parse a set of options, then have a separator that indicates a list of options to turn off follows.

I'd rather we not add this at all. One of my complaints with all compilers I've dealt with, including LLVM, is there are umpteen different ways to specify options and what one wants is never quite what is there. I'd like the abstraction to be like a structure with every single parameter that is available in a typed, straight forward representation. The only way we get that simple to use thing is by being very careful in adding options. Yes, the autoscheduler will need to understand this structure. This is pretty much unavoidable.

[alexreinking]

then have a separator that indicates a list of options to turn off follows

I'm not exactly sure what you mean by this?

One of my complaints with all compilers I've dealt with, including LLVM, is there are umpteen different ways to specify options and what one wants is never quite what is there.

When I think of ways to set options in Halide that I don't want, it's the dozens of environment variables that tweak small settings. Targets have the benefit of being explicit. Making them easier to work with in an algebraically principled way helps us get repetitive logic out of our build tools. You want to turn a feature in a user-supplied target on or off? Just type ${target}-feature or ${target}-no_feature. Simple. No need to build and distribute a companion C++ tool that just manipulates target strings when there's an unsurprising (see -f / -fno flags in gcc compatible compilers) structure for us to use.

I'd like the abstraction to be like a structure with every single parameter that is available in a typed, straight forward representation.

What would the human editable representation of this be? An rc file on disk? A json string? Such structures don't provide easy ways to make small edits, which is what the autoscheduler needs to do here.

[zvookin]

The current design is a concatenation of a set of fields converted to strings with a couple special keywords like "host". This is the serialization I mentioned. We have a few allowances for usability such as not requiring features to be in order, but the outer level does need to be in order and generally we're a bit stricter than most command line designs for this sort of thing. The worst part of stuff in LLVM is even specifying CPU, cpu-bits, ISA features, etc. has different syntaxes in different places and it is difficult to get something passed through to various parts of the toolchain. Our general approach has been to try and get everything into one target spec that controls everything in the compiler. (And yes, there are some env vars that need to go away, though usually they are either for compiler debugging or they affect the runtime, not the compiler.)

What you are initially proposing raises some questions. For example, it seems dubious to ever have "no_feature" appear in a canonical serialization and we've already used no_asserts and a number of others. This was the initial impetus for proposing the separator design. That would be something like:
host-no_runtime-disable_llvm_loop_opt/avx512-avx512_knl-avx512_skylake-avx512_cannonlake
Canonical form would never have a / clause. The specifics of syntax are not solid, it is just for purposes of explanation. Alternatively we could use '/' or '~' in front of a feature instead of "no_" as this eliminated the issue with the current use of "no_". '~' instead '-' between feature names might be a decent bet.

The example I use above is intentional to indicate that "just turn off avx512" is more complicated than one might think and for something like the autoscheduler, we probably want an allowed set of features to be enabled if the host supports them, not a set of features to remove. (E.g. the current approach also breaks when "avx1024" is added.)

Over in the arm_sve branch I've added a feature vector_bits_#### which sets a numeric field in the Target. It only appears in the serialization if the field is non-zero. Dillon and I have discussed having min and max vector bits specs as well. This is also changing things up a bit in that it is more a named parameter than a feature and also it isn't clear what to do if multiple occurrences appear. (I think I made it an error, but I'm not sure.) The idea was that potentially we could have an architecture independent way to control what level of ISA is targeted by naming them with bit size. There would still be arch specific features for variants within that. I'm not sure how this is going to pan out, but it is somewhat related.

My main take here is we should strive to make sure the Python bindings are handy everywhere we need something like this and use the Target API to do manipulations like this, but this is more on the general principle of avoiding building little languages unless they carry their weight.

[alexreinking]

For example, it seems dubious to ever have "no_feature" appear in a canonical serialization and we've already used no_asserts and a number of others. [...] Over in the arm_sve branch I've added a feature vector_bits_#### which sets a numeric field in the Target. It only appears in the serialization if the field is non-zero.

I'm not sure why this seems dubious. The canonical serialization should include only the flags that change defaults. So since asserts is on by default, we would serialize to no_asserts. Same with your vector_bits_#### that only shows up when it's non-zero.

Alternatively we could use '/' or ~ in front of a feature instead of "no_" as this eliminated the issue with the current use of "no_". ~ instead '-' between feature names might be a decent bet.

Yes, this is exactly the same feature. On the other hand, it's a little subtle at a glance. But I think we're arguing aesthetics / bike-shedding. The point is to have a feature to manipulate opaque targets. Partitioning negative features with / doesn't fit the bill because if a user specifies host/some-feature, $user_target/avx512... would be invalid (/ appears twice).

This is also changing things up a bit in that it is more a named parameter than a feature and also it isn't clear what to do if multiple occurrences appear. (I think I made it an error, but I'm not sure.)

There's nothing wrong with left-to-right overriding. Users will not manually type it out twice. We could issue a diagnostic at debug(1) if we anticipate issues with tooling. GCC/Clang don't bother with that, but MSVC does (by default, ugh).

[zvookin]

I'll simply go on record saying I don't think this is a good idea. As the AVX512 example indicates, removing features is error prone. Details of how target strings serialize do matter to our build system in subtle ways and this has potential to break that. More over, the reduction of this to "aesthetics" and "bikeshedding" without acknowledging underlying issues indicates a lack of appreciation that this is introducing a small language into a text representation that is already used in a lot of places and will come to be used in many more. Things like "the serialization prints all non default features" are important to surface in the design.

(Noting that the original description above starting with "These would have their enum values replaced with negatives of their current values..." seems to imply a different approach. Perhaps this is proposing renaming the existing "no_" features as mentioned below. It is unclear to me.)

The reason I say "dubious" is best illustrated by assuming we started with this design. In that case, "no_asserts" would not have been called that. It would be e.g. "disable_asserts." We could in fact consider removing all "no_" feature names, though it would be a breaking change.

The purpose of specialized syntax such as the tilde is to make the visuals indicate that there is actually a language feature in play. Anyone sees this and understands that the operator can be applied to any feature. Note that part of the reason I have issues around "no_" is because systems are inconsistent in whether the _ is there. The one I run into most often (e.g. https://abseil.io/docs/python/guides/flags) does not have it so it would be e.g. "nodisable_llvm_loop_opt". I often mistype that and find it visually unpleasant, but I believe it is the predominant model we would want to be consistent with.

A better approach would be to promote the get_host_target tool to a standard "halide_target" command line tool in the distribution that produces target strings for a variety of uses and see how it evolves. This is potentially a bigger usability win as that tool can print both the host and JIT targets and can surface flags/options to produce target strings for e.g. "get me a .stmt file with no cruft in it." Eventually this might get moved into the default target string parsing.,

[zvookin]

Also, we should definitely do the halide_target command line tool even if we do this. The tool can do equality and subset testing of targets. It can put targets in canonical ordering. If we do end up deprecating or renaming features, it can rewrite target strings to the correct current form, etc. Provide validation with clean error messages for scripts, etc.

It would likely be a library call, so surfaced from C++ and Python as well.