`-mtune=`/`-mcpu=` support for x86 AMD CPU's #6655

LebedevRI · 2022-03-19T22:05:27Z

As per #6651 (comment),
implements tune features, that only drive -mcpu= for LLVM,
and don't imply any further halide features.

Caveats/missing from this patch:

Autodetection
What about picking optimal multiversion at runtime?
This only handles AMD processors. Intel part can be done, but it is a bigger list. Though, perhaps, that would allow requiring the tune on X86?
Do we want to deduplucate this by moving it to a header with a macro?
Are tests wanted?

zvookin · 2022-03-20T06:19:42Z

These don't affect anything in Halide lowering do they? They're just passthroughs to LLVM. In this case, I'd propose we add a string list to Target that carries the LLVM string. It can be parsed/serialized in the targets string as e.g. "llvm_tune_". The selection between differently compiled kernels using halide_can_use_target_features won't handle thing, but I don't think it will work right with these tuning flags anyway. (Effectively one will need some way of picking a closest tuning for a given processor if there isn't an exact match. And even for an exact match, that requires CPUID sniffing to do something with the feature flags, which I'm not sure makes sense.)

LebedevRI · 2022-03-20T11:58:02Z

Thank you for taking a look. I believe there are three questions in total here, so let me split them up.

These don't affect anything in Halide lowering do they? They're just passthroughs to LLVM.

This is a great question. I believe, when you are asking whether or not that affects anything in Halide lowering,
you aren't asking about this pr (because clearly, it doesn't), but about the larger, greater picture.

I believe the answer is: i disagree, i don't believe that to be the reality.
Each CPU is different, and expecting that no useful information can be extracted
from knowing the exact target CPU may be, well, setting the stage for suboptimal results.

For example, only znver2 finally has single-op AVX256 operations,
before that they were 2op, including loads/stores. I suspect that does affect the optimal schedule:

Halide/apps/conv_layer/conv_layer_generator.cpp

Lines 142 to 146 in be1269b

    
           // On Skylake we have one load per fma and 32 
        
           // registers available, so there's considerable 
        
           // flexibility in the schedule. We'll use 20 accumulator 
        
           // registers in a 4x5 tile. This is also a reasonable 
        
           // choice for ARMv8, which also has 32 registers.

In this case, I'd propose we add a string list to Target that carries the LLVM string. It can be parsed/serialized in the targets string as e.g. "llvm_tune_".

This won't really work as long as we have to be able to serialize them into halide feature string.
The CPU name is not guaranteed to e.g. not have - (the feature separator).

The selection between differently compiled kernels using halide_can_use_target_features won't handle thing, but I don't think it will work right with these tuning flags anyway. (Effectively one will need some way of picking a closest tuning for a given processor if there isn't an exact match. And even for an exact match, that requires CPUID sniffing to do something with the feature flags, which I'm not sure makes sense.)

There are two questions here - picking the exact match, and picking the fallback in case of no exact match.

I don't see why picking the exact match would be anything other than trivial,
in fact i have already implemented that in pull request #6648,
we'll simply need to perform the same CPU detection we'll have to do
when expanding host pseudo-feature. In fact, i will argue that doing so is required
to achieve optimal results, otherwise multi-versioning will end up penalizing performance.

As for picking the fallback, i'm not sure, but picking any compatible variant will be fine in principle.
I guess it depends as to how much custom logic (to do that) can be added to run-time.

zvookin · 2022-03-21T18:50:28Z

Are tuning flags ever used together or is it a "pick one and only one" thing? My general take is the idea is to identify a single microarchitecture, at least for a broad swath of the functionality. (E.g. I could imagine two tuning specifications, one for the scalar part and one for the vector part. Especially in a more decoupled design. But generally everything is connected and one wants to identify the entire microarchitecture model at once.) If so, using bit flags to identify them is not a great fit both in terms of encoding and that it does not implicitly enforce the required "one hot" invariant.

Per the "do tuning flags ever affect lowering" my impression is this needs to be "no, they do not," at least if the point Alex made about ISA enables being separate from tuning is strictly followed. One may indeed choose to used different schedules for different microarchitectures, but that comes from the user or autoscheduler and is not a change in the execution paths in lowering.

This does mean choosing optimized kernels may need to pick between options based solely on the tuning info, not ISA support. I'm not convinced halide_can_use_target_features is the right thing for that. Mainly because it returns a boolean and any prioritization is done by choosing the ordering it is called in. That is not possible when there isn't mostly a superset relationship between the options. (E.g. SSE3 dominates SSE2, and SSE3 code doesn't run on SSE2 only CPUs. However if AMD and Intel introduce processors with exactly the same instruction set, but different performance characteristics, halide_can_use_target_features strictly return the same thing for both for all inputs because the code can run. If I choose to only tune for one of them, that code needs to be used on the other.)

Note my point here isn't that halide_can_use_target_features should constrain our adding support for tuning, but rather I think we're going to need a new runtime mechanism or to tell people to implement their own kernel selection for tunings.

For encoding strings to pass to LLVM where dash ("-") may occur in the flag name, we'll need an escape mechanism or some change to syntax of target strings. An example would be to have a new separator that starts an uninterpreted (string typed) list of key value pairs. (There's a bit of design there. Or we could just use JSON or something.) Some of the keys would just have their values passed through to LLVM. The canonicalization would sort and do a few things to ensure equal targets produce the same string, but generally the new information just be carried in Target.

Adding a map to Target might be a bit costly in in argument passing, but the first order change there would be to make sure Target is passed by reference where possible. Another approach would be to move to pooled storage for certain things and have a compilation context the pool is attached to. That way there'd only be one instance of a given target value. This could be used for other things too, but is a larger code change.

abadams · 2022-03-21T19:18:14Z

We're doing an increasing amount of instruction selection ourselves these days. I can see both this caring about the micro-arch and also the autoscheduler.

We could add a microarch field to Target, which is a separate enum value?

zvookin · 2022-03-21T20:20:41Z

I guess I was thinking we'd add Target flags to control lowering for the cases were there is actual code that pays attention to tuning. Of course one would likely want that flag to also turn on the LLVM tuning flag, which means the features would interact with the string typed stuff, which means Halide has to know something about the strings.

An enum and an added field is a win over feature flags if it is indeed single valued. The shortcoming here is one still cannot access a given backend tuning unless Halide has added support for it. E.g. right now when I want to assess the difference targeting a specific microarchitecture makes in LLVM, I hand edit Halide code. The enum would at least give a place to put that and maybe make it into public Halide, if it isn't just a one off, but there are a lot of these and in some sense they may not be entirely enumerable. Could have an "Other" value in the enum I guess that is backed by a string...

LebedevRI · 2022-03-26T14:59:50Z

Moved per-CPU tunes into their own enum, out of features.
Is this closer to what you had in mind?

src/Target.cpp

abadams · 2022-03-26T19:01:36Z

Works for me. @zvookin ?

src/Target.h

src/Target.cpp

…ification

src/Target.cpp

LebedevRI · 2022-03-30T00:03:46Z

@zvookin @abadams thank you for taking a look!
I believe i have addressed all current review notes, feel free to point out if i missed any!

zvookin · 2022-03-30T02:31:51Z

Looks solid to me. Let me chat with Steven before approval to see whether we want to pull this for a spin first. Either way, should be able to land in the next couple days I think.

python_bindings/src/PyEnums.cpp

src/Target.cpp

src/Target.h

steven-johnson · 2022-03-30T18:18:32Z

No red flags from a test Google integration. LGTM.

zvookin

I think it is down to at most a couple comment changes or such. Approved.

LebedevRI · 2022-03-30T21:32:32Z

Not really sure what is wrong with those two windows CI checks.

steven-johnson · 2022-03-30T21:51:30Z

Not really sure what is wrong with those two windows CI checks.

Looks like a flake, I restarted them. (Our Windows buildbots aren't as robust as they really should be...)

LebedevRI · 2022-03-30T22:26:04Z

Not really sure what is wrong with those two windows CI checks.

Looks like a flake, I restarted them. (Our Windows buildbots aren't as robust as they really should be...)

Almost there... :)

LebedevRI · 2022-03-31T21:56:47Z

Thanks everyone!

Looks like the last bot finally went green.
Any further comments, @abadams @zvookin @steven-johnson ?
Anything for me left to do here?

LebedevRI · 2022-03-31T22:24:50Z

Thank you!

Sooo. Uh, remember when in halide#6655 we've agreed that we want to add support to precisely specify the CPU for which the code should be *tuned* for, but not *targeted* for. Aka, similar to clang's `-mtune=` option, that does not affect the ISA set selection? So guess what, that's not what we did, apparently. `CodeGen_LLVM::mcpu()` / `halide_mcpu` actually do specify the *target* CPU. It was obvious in retrospect, because e.g. `CodeGen_X86::mattrs()` does not, in fact, ever specify `+avx2`, yet we get AVX2 :) So we've unintentionally added `-march=` support. Oops. While i'd like to add `-march=` support, that was not the goal here. Fixing this is complicated by the fact that `llvm::Target::createTargetMachine()` only takes `CPU Target` string, you can't specify `CPU Tune`. But this is actually a blessing in disguise, because it allows us to fix another bug at the same time: There is a problem with halide "compile to llvm ir assembly", a lot of information from Halide Target is not //really// lowered into LLVM Module, but is embedded as a metadata, that is then extracted by halide `make_target_machine()`. While that is not a problem in itself, it makes it *impossible* to dump the LLVM IR, and manually play with it, because e.g. the CPU [Target] and Attributes (ISA set) are not actually lowered into the form LLVM understands, but are in some halide-specific metadata. So, to fix the first bug, we must lower the CPU Tune into per-function `"tune-cpu"` metadata, and while there we might as well lower `"target-cpu"` and `"target-features"` similarly.

* Fix fundamental confusion about target/tune CPU Sooo. Uh, remember when in #6655 we've agreed that we want to add support to precisely specify the CPU for which the code should be *tuned* for, but not *targeted* for. Aka, similar to clang's `-mtune=` option, that does not affect the ISA set selection? So guess what, that's not what we did, apparently. `CodeGen_LLVM::mcpu()` / `halide_mcpu` actually do specify the *target* CPU. It was obvious in retrospect, because e.g. `CodeGen_X86::mattrs()` does not, in fact, ever specify `+avx2`, yet we get AVX2 :) So we've unintentionally added `-march=` support. Oops. While i'd like to add `-march=` support, that was not the goal here. Fixing this is complicated by the fact that `llvm::Target::createTargetMachine()` only takes `CPU Target` string, you can't specify `CPU Tune`. But this is actually a blessing in disguise, because it allows us to fix another bug at the same time: There is a problem with halide "compile to llvm ir assembly", a lot of information from Halide Target is not //really// lowered into LLVM Module, but is embedded as a metadata, that is then extracted by halide `make_target_machine()`. While that is not a problem in itself, it makes it *impossible* to dump the LLVM IR, and manually play with it, because e.g. the CPU [Target] and Attributes (ISA set) are not actually lowered into the form LLVM understands, but are in some halide-specific metadata. So, to fix the first bug, we must lower the CPU Tune into per-function `"tune-cpu"` metadata, and while there we might as well lower `"target-cpu"` and `"target-features"` similarly. * Address review notes * Hopefully silence bogus issue reported by ancient GCC * Call `set_function_attributes_from_halide_target_options()` when JIT compiling * Fix grammar

* `-mtune=`/`-mcpu=` support for x86 AMD CPU's (#6655) * `-mtune=`/`-mcpu=` support for x86 AMD CPU's * Move processor tune into it's own enum, out of features * clang-format * Target: make Processor more optional * Processor: add explanatory comments which CPU is what * Drop outdated changes * Make comments in Processor more readable / fix BtVer2 comment * Target: don't require passing Processor * Make processor more optional in the features string serialization/verification * Address review notes * Undo introduction of halide_target_processor_t * Fix year for btver2/jaguar * Fix GPU depredication/scalarization (#6669) * Scalarize predicated Loads * Cleanup * Fix gpu_vectorize scalarization for D3D12 * Fix OpenCL scalarization * Minor fixes * Formatting * Address review comments * Move Shuffle impl to CodeGen_GPU_C class * Extra space removal Co-authored-by: Shoaib Kamil <kamil@adobe.com> * Allow PyPipeline and PyFunc to realize() scalar buffers (#6674) * Future-proof 'processor` to `tune processor` (#6673) * Fix ctors for Realization (#6675) For vector-of-Buffers, the ctor took a non-const ref to the argument, which was weird and nonsensical. Replaced with a const-ref version and and an rvalue-ref version; it turns out that literally *all* of the internal calls were able to use the latter, trivially saving some copies. * `-mtune=native` CPU autodetection for AMD Zen 3 CPU (#6648) * `-mtune=native` CPU autodetection for AMD Zen 3 CPU * Address review notes. * Fix MSVC build * Address review notes * Bump development Halide version to 15.0.0 (#6678) * Bump development Halide version to 15.0.0 * trigger buildbots * Clean up Python extensions in python_bindings (#6670) * Remove the nobuild/partialbuildmethod tests from python_bindings/ They no longer serve a purpose and are redundant to other tests. * WIP * Update pystub.py * wip * wip * wip * Update TargetExportScript.cmake * Update PythonExtensionHelpers.cmake * PyExtensionGen didn't handle zero-dimensional buffers * Halide::Tools::save_image() should accept buffers with `const` types (#6679) * Fix "set but not used" warnings/errors (#6683) * Fix "set but not used" warnings/errors Apparently XCode 13.3 has smarter warnings about unused code and emits warnings/errors for these, so let's clean them up. * Also fix missing `ssize_t` usage * Remove deprecated `Halide::Output` type (#6685) It was deprecated (in favor of `OutputFileType` in Halide 14; let's remove it entirely for Halide 15. * Remove deprecated `build()` support from Generators (#6684) This was deprecated in Halide 14; let's remove it entirely for Halide 15. * Drop support for LLVM12 (#6686) * Drop support for LLVM12 Halide 15 only needs to support LLVM13 and LLVM13. Drop all the special-casing for LLVM12. * Update packaging.yml * Update presubmit.yml * 13 * more * Update presubmit.yml * woo * Update presubmit.yml * Update run-clang-tidy.sh * Update run-clang-tidy.sh * Update .clang-tidy * Update .clang-tidy * wer * Update Random.cpp * wer * sdf * sdf * Update packaging.yml * Upgrade to clang-format 13 (#6689) Goal here: eliminate the need for a local version of llvm/clang-12, and don't stay too far behind the toolchain. As always, clang-format doesn't promise backwards compatibility, but the main differences in formatting are: - more regularization of spaces at the start of comments (I like this change) - minor difference of formatting of function-pointer-type declarations (not a fan of this, but I can't find a way to disable it and it's only really used in a handful of place in the Python bindings) * Always mark _ucon as 'unused' in Codegen_C (#6691) * Always mark _ucon as 'unused' in Codegen_C, even if asserts are enabled, since generated closure functions may not use it * halide_unused -> halide_maybe_unused * fix test_internal * More halide_unused -> halide_maybe_unused * Add `break` to avoid 'possible unintentional fallthru' warning (#6694) * Silence "unknown warning" in Clang 13 (#6693) Clang 13 removed the `return-std-move-in-c++11` warning entirely, so specifying it now warns that the warning is unknown. * Fixes for top-of-tree LLVM (#6697) * Faster `widening_mul(int16x, int16x) -> int32x` for x86 (AVX2 and SSE2) (#6677) * add widening_mul using vpmaddwd for AVX2 * add vpmaddwd/pmaddwd test * add widening_mul with pmaddwd for SSE2 * Remove deprecated versions of Func::prefetch() (#6698) * Remove deprecated JIT handler setters (#6699) * Drop support for Matlab extensions (#6696) * Drop support for Matlab extensions Anecdotally, this hasn't been used in ~years, and the original author (@dsharletg) had suggested dropping it a while back. I'm going to propose we go ahead and drop it for Halide 15 and see who complains. * Fixes for top-of-tree LLVM * Update force_include_types.cpp * trigger buildbots * Update CodeGen_LLVM.cpp * llvm no longer wants a type suffix on vst intrinsics (#6701) * llvm no longer wants a type suffix on vst intrinsics * Fix silly mistake * Change 64-bit only Co-authored-by: Andrew Adams <anadams@adobe.com> * Python: make Func implicitly convertible to Stage (#6702) (#6704) This allows for `compute_with` and `rfactor` to work more seamlessly in Python. Also: - Move two compute_with() variant bindings from PyFunc and PyStage to PyScheduleMethods, as they are identical between the two - drive-by removal of redundant `py::implicitly_convertible<ImageParam, Func>();` call * Fix type-mangling for vst on arm32 for LLVM15 (#6705) * Remove the last remaining call to getPointerElementType() (#6715) * Remove the last remaining call to getPointerElementType() LLVM is moving to opaque pointers, we must have missed this one in previous work * ARM vst mangling needs to be conditional on opaque ptrs The fixes from last week regarding mangling of arm vst intrinsics needs to be made conditional on whether the pointer is opaque or not; this will change based on whether `-D CLANG_ENABLE_OPAQUE_POINTERS=ON|OFF` is defined when LLVM is built, but should be sniffed via this API, according to my LLVM contact. * Revert "ARM vst mangling needs to be conditional on opaque ptrs" This reverts commit 9901314ff75dd0bf651b23d09c1d1f5f07d49ffd. * ARM vst mangling needs to be conditional on opaque ptrs (#6716) The fixes from last week regarding mangling of arm vst intrinsics needs to be made conditional on whether the pointer is opaque or not; this will change based on whether `-D CLANG_ENABLE_OPAQUE_POINTERS=ON|OFF` is defined when LLVM is built, but should be sniffed via this API, according to my LLVM contact. * Combine string constants in combine_strings() (#6717) * Combine string constants in combine_strings() This is a pretty trivial optimization, but when printing (or enabling `debug`), it cuts the number of `halide_string_to_string()` calls we generate by ~half. * Update IROperator.cpp * Update CodeGen_PTX_Dev to use new PassManager (#6718) * Update CodeGen_PTX_Dev to use new PassManager This was still using the LegacyPassManager for optimization, which will be going away at some point. (Code changes by @alinas; I'm just opening this PR on her behalf) * Fixes after review * Closure functions for parallel tasks should be internal, not external (#6720) Minor optimization. * Smarten type_of<> for fn ptrs; fix async_parallel for C backend (#6719) * Smarten type_of<> for fn ptrs; fix async_parallel for C backend (Fixes #2093) This basically just adds the right type annotations to make the parallel code produced by the C backend compile properly. This could have been fixed by inserted some brute-force void* casting into the C backend, but this felt a lot cleaner. The one thing here I'm a little unsure about is how I extended the Type code to be able to handle function-pointer types correctly; it works but doesn't feel very elegant. * Update Makefile * Update LowerParallelTasks.cpp * FunctionTypedef * Remove legacy::FunctionPassManager usage in Codegen_PTX_Dev (#6722) LLVM devs indicate that none of the passes in this usage actually do anything and it can be safely removed. * `get_amd_processor()`: implement detection for the rest of supported AMD CPU's (#6711) I have *not* personally tested that these are detected correctly, Cross-reference between * https://github.com/llvm/llvm-project/blob/955cff803e081640e149fed0742f57ae1b84db7d/llvm/lib/Support/Host.cpp#L968-L1041 * https://github.com/llvm/llvm-project/blob/955cff803e081640e149fed0742f57ae1b84db7d/compiler-rt/lib/builtins/cpu_model.c#L520-L586 * https://github.com/gcc-mirror/gcc/blob/000c1b89d259fadb466e1f2e63c79da45fd17372/gcc/common/config/i386/cpuinfo.h#L111-L264 * Add Func::output_type() method (#6724) * Add Func::output_type() method * Add Python * Grab-bag of minor Python fixes (#6725) * Deprecate variadic-template version of Realization ctor (#6695) * Deprecate variadic-template version of Realization ctor The variadic-template approach was useful before C++11 (!) added brace initialization, but preferring an explicit vector-of-Buffer is arguably better, and provides better symmetry with the Python bindings. Also, some drive-by tweaks to other Realization methods. * Update PyPipeline.cpp * trigger buildbots * Remove `rounding_halving_sub` and non-existent arm rhsub instructions (#6723) * remove arm (s | u)rhsub instructions * remove rounding_halving_sub intrinsic entirely * Augment Halide::Func to allow for constraining Type and Dimensionality (#6734) This enhances Func by allowing you to (optionally) constrain the type(s) of Exprs that the Func can contain, and/or the dimensionality of the Func. (Attempting to violate either of these will assert-fail.) There are a few goals here: - Enhanced code readability; in cases where a Func's values may not be obvious from the code flow, this can allow an in-code way of declaring it (rather than via comments) - Enhanced type enforcement; specifying constraints allows us to fail in type-mismatched compilations somewhat sooner, with somewhat better error messages. - Better symmetry for AOT/JIT code generation with ImageParam, in which the inputs (ImageParam) have a way to specify the required concrete type, but the outputs (Funcs) don't. If this is accepted, then subsequent changes will likely add uses where it makes sense (e.g., the Func associated with an ImageParam should always have both type and dimensionality specified since it will always be well-known). Note that this doesn't add any C++ template class for static declarations (e.g. `FuncT<float, 2>` -> `Func(Float(32), 2)`); these could be added later if desired. * More typed-Func work (#6735) - Allow Func output_type(s)(), outputs(), dimensions(), and output_buffer(s)() to be called on undefined Funcs if the Func has required_type and required_dimensions specified. This allows for greater flexibility in defining pipelines in which you may want to set or examine constraints on a Func that hasn't been defined yet; previously this required restructuring code or other awkwardness. - Ensure that the Funcs that are defined for ImageParams and Generator fields define the types-and-dims when known. - Add some tests. * Add missing #include <functional> in ThreadPool.h (#6738) * Add missing #include <function> in ThreadPool.h * Update ThreadPool.h * Fix regression from #6734 (#6739) That change inadvertently required the RHS of an update stage that used `+=` (or similar operators) to match the LHS type, which should be required (implicit casting of the RHS is expected). Restructured to remove this, but still ensure that auto-injection of a pure definition matches the required types (if any), and updated tests. * Add forwarding for the recently-added Func::output_type() method (#6741) * Silence "unscheduled update stage" warnings in msan_generator.cpp (#6740) * Add __pycache__ to toplevel .gitignore file (#6743) * Revise PyStub calling convention for GeneratorParams (#6742) This is a rethink of https://github.com/halide/Halide/pull/6661, trying to make it saner in anticipation of the ongoing Python Generator work. TL;DR: instead of mixing GeneratorParams in with the rest of the keywords, segregate them into an optional `generator_params` keyword argument, which is a plain Python dict. This neatly solves a couple of problems: - synthetic params with funky names aren't a problem anymore. - error reporting is simpler because before an unknown keyword could have been intended to be a GP or an Input. - GP values are now clear and distinct from Inputs, which is IMHO a good thing. This is technically a breaking change, but I doubt anyone will notice; this is mainly here to get a sane convention in place for use with Python Generators as well. Also, a drive-by change to Func::output_types() to fix the assertion error message. * Silence "may be used uninitialized" in Buffer::for_each_element() (#6747) In at least one version of GCC (Debian 11.2.0-16+build1), an optimized build using `Buffer::for_each_element(int *pos)` will give (incorrect) compiler warnings/errors that "pos may be used uninitialized). From inspection of the code I feel pretty sure this is a false positive -- i.e., the optimizer is confused -- and since no other compiler we've encountered issues a similar warning (nor do we see actual misbehavior), I'm inclined not to worry -- but the warning does break some build configurations. Rather than try to fight with selectively disabling this warning, I'm going to propose inserting a memset() here to reassure the compiler that the memory really is initialized; while it's unnnecessary, it's likely to be insignificant compared to the cost of usual calls to for_each_element(). (BTW, this is not a new issue, I've seen it for quite a while as this GCC is the default on one of my Linux machines... it just finally annoyed me enough to want to make it shut up.) * Update WABT to 1.0.29 (#6748) * Update hannk README link to hosted models page (#6749) The current one is being sunsetted * Add a `HalideError` base class to Python bindings (#6750) * Add a `HalideError` base class to Python bindings Per suggestion from @alexreinking, this remaps all exceptions thrown by the Halide Python bindings to be `halide.HalideError` (or a subclass thereof), rather than plain old `RuntimeError`. * Remove scalpel left in patient * Don't use a subclass for PyStub error handling * Deprecate GeneratorContext getters with `get_` prefix (#6753) Minor hygiene: most getters in Halide don't have a `get_` prefix. These are very rarely used (only one instance in our test suite I could find) but, hey, cleanliness. * Add GeneratorFactoryProvider to generate_filter_main() (#6755) * Add GeneratorFactoryProvider to generate_filter_main() This provides hooks to allow overriding the Generator(s) that generate_filter_main() can use; normally it defaults to the global registry of C++ Generators, but this allows for (e.g.) alternate-language-bindings to selectively override this (e.g. to enumerate only Generators that are visible in that language, etc). (No visible change in behavior from this PR; this is just cherry-picked from work-in-progress elsewhere to simplify review & merge) * Update Generator.cpp * Fix error handling * Deprecate disable_llvm_loop_opt (#4113) (#6754) This PR proposes to (finally) deprecate disable_llvm_loop_opt: - make LLVM codegen default to no loop optimization; you must use enable_llvm_loop_opt explicitly to enable it - disable_llvm_loop_opt still exists, but does nothing (except issue a user_warning that the feature is deprecated) - Remove various uses of disable_llvm_loop_opt - Add comments everywhere that the default is different in Halide 15 and that the disable_llvm_loop_opt feature will be removed entirely in Halide 16 Note that all Halide code at Google has defaulted to having disable_llvm_loop_opt set for ~years now, so this is a well-tested codepath, and consensus on the Issue seemed to be that this was a good move. * Minor metadata-related cleanups (#6759) (Harvested from #6757, which probably won't land) - Add clarifying comment/reference in Generator - Add assertion to compile_to_multitarget() function - Fix misleading/wrong code in correctness_compile_to_multitarget * Expand the x86 SIMD variants tested in correctness_vector_reductions (#6762) A recent bug in LLVM codegen was missed because it only affected x86 architectures with earlier-than-AVX2 SIMD enabled; it didn't show up for AVX2 or later. This revamps correctness_vector_reductions to re-run multiple times when multiple SIMD architectures are available on x86 systems. (correctness_vector_reductions was chosen here because it reliably demonstrated the specific failures in this case.) * Fix Param<T>::set_estimate for T=void (#6766) * Fix Param<T>::set_estimate for T=void * Add tests * add_python_aot_extension should use FUNCTION_NAME for the .so output … (#6767) add_python_aot_extension should use FUNCTION_NAME for the .so output (otherwise you can't produce multiple aot extensions from the same Generator) * Update the list of fused_pairs and run validate_fused_group for specalization definitions too (#6770) * Update the list of fused_pairs and run validate_fused_group for specialization definitions too. Fixes https://github.com/halide/Halide/issues/6763. * Address review comments * Add const to auto& * Add Func::type()/types(), deprecate Func::output_type()/output_types() (#6772) * rename GIOBase::type() and friends * Func::output_type() -> Func::type() * Add type() forwarders for inputs * Add Func::dimensions() wrapper * Update Func.h * Fix fundamental confusion about target/tune CPU (#6765) * Fix fundamental confusion about target/tune CPU Sooo. Uh, remember when in https://github.com/halide/Halide/pull/6655 we've agreed that we want to add support to precisely specify the CPU for which the code should be *tuned* for, but not *targeted* for. Aka, similar to clang's `-mtune=` option, that does not affect the ISA set selection? So guess what, that's not what we did, apparently. `CodeGen_LLVM::mcpu()` / `halide_mcpu` actually do specify the *target* CPU. It was obvious in retrospect, because e.g. `CodeGen_X86::mattrs()` does not, in fact, ever specify `+avx2`, yet we get AVX2 :) So we've unintentionally added `-march=` support. Oops. While i'd like to add `-march=` support, that was not the goal here. Fixing this is complicated by the fact that `llvm::Target::createTargetMachine()` only takes `CPU Target` string, you can't specify `CPU Tune`. But this is actually a blessing in disguise, because it allows us to fix another bug at the same time: There is a problem with halide "compile to llvm ir assembly", a lot of information from Halide Target is not //really// lowered into LLVM Module, but is embedded as a metadata, that is then extracted by halide `make_target_machine()`. While that is not a problem in itself, it makes it *impossible* to dump the LLVM IR, and manually play with it, because e.g. the CPU [Target] and Attributes (ISA set) are not actually lowered into the form LLVM understands, but are in some halide-specific metadata. So, to fix the first bug, we must lower the CPU Tune into per-function `"tune-cpu"` metadata, and while there we might as well lower `"target-cpu"` and `"target-features"` similarly. * Address review notes * Hopefully silence bogus issue reported by ancient GCC * Call `set_function_attributes_from_halide_target_options()` when JIT compiling * Fix grammar * Fix annoying typo in Func.h (#6774) Update Func.h * Add execute_generator() API (#6771) This refactors the existing `generate_filter_main()` call in two, moving the interesting implementation of how to drive AOT into the new `execute_generator()` call (reducing `generate_filter_main()` to parsing argc/argv and error reporting). The new `execute_generator()` is intended to be used (eventually) from Python, as a way to drive Generator compilation from a Python script more easily. The PR doesn't provide a Python wrapper for this call yet (that will come in a subsequent PR). Also, a drive-by removal of the "error_output" arg to generate_filter_main() -- AFAICT, no one has ever used it for anything but stderr, and the refactoring now just directs all errors to `user_error` uniformly. * Allow overriding of `Generator::init_from_context()` for debug purposes (#6760) * Allow overriding of `Generator::init_from_context()` for debug purposes * Update Generator.h * Attempt to clarify contract * Convert some assert-only usage of output_types() -> types() (#6779) * [miscompile] Don't de-negate and change direction of shifts-by-unsigned (#6782) I'm afraid the problem is really obvious: https://github.com/halide/Halide/blob/b5f024fa83b6f1cfe5e83a459c9378b7c5bf096d/src/CodeGen_LLVM.cpp#L2628-L2649 ^ the shift direction is treated as flippable by the codegen iff the shift amount is signed :) The newly-added test fails without the fix. I've hit this when writing tests for https://github.com/halide/Halide/pull/6775 * Move some options from execute_generator back to generate_filter_main (#6787) Loading plugins and setting the default autoscheduler name both change global state, which isn't a desirable fit for execute_generator(), since it's not intended to mutate global state. (Mutating the state from a main() function is of course a reasonable thing to do.) * LLVM codegen: register AA pipeline if LLVM is older than 14 (#6785) It's the default after https://reviews.llvm.org/D113210 / https://github.com/llvm/llvm-project/commit/13317286f8298eb3bafa9ddebd1c03bef4918948, but still needs to be done for earlier LLVM's. Refs. https://github.com/halide/Halide/issues/6783 Refs. https://github.com/halide/Halide/pull/6718 Partially reverts https://github.com/halide/Halide/pull/6718 * halide_type_of<>() should always be constexpr (#6790) The ones in HalideRuntime.h have been marked constexpr for a while, but the ones in Float16.h got missed * Define an AbstractGenerator interface (#6637) * AbstractGenerator (rebased, v3) * Update AbstractGenerator.h * clang-format * Update Generator.cpp * IOKind -> ArgInfoKind * Various cleanups of AbstractGenerator * clang-format * fix pystub * Update abstractgeneratortest_generator.cpp * dead code * ArgInfoDirection * cleanup * Delete PyGenerator.cpp * Update PyStubImpl.cpp * Update PyStubImpl.cpp * Fixes from review comments * Remove `get_` prefix from getters in AbstractGenerator * Missed some fixes * Fixes * Add GeneratorFactoryProvider for generate_filter_main() * Add GeneratorFactoryProvider to generate_filter_main() This provides hooks to allow overriding the Generator(s) that generate_filter_main() can use; normally it defaults to the global registry of C++ Generators, but this allows for (e.g.) alternate-language-bindings to selectively override this (e.g. to enumerate only Generators that are visible in that language, etc). (No visible change in behavior from this PR; this is just cherry-picked from work-in-progress elsewhere to simplify review & merge) * Update Generator.cpp * fixes * Update Generator.cpp * Restore build_module() and build_gradient_module() methods * Update Generator.h * fixes * Update Generator.cpp * Update AbstractGenerator.h * hexagon_scatter test should run only if target has HVX (#6793) It will run otherwise, but is slow on some other targets; rather than trying to (e.g.) shard it, just skip it * Add Target support for architectures with implementation specific vector size. (#6786) Move vector_bits_* Target support from fixed_width_vectors branch to make smaller PRs. * slow tests should support sharding (#6780) * slow tests should support sharding The simd_op_check test suite is pretty slow (especially for wasm, where it is interpreted); at one point we tried to use ThreadPool to speed it up, but too many pieces of Halide IR aren't threadsafe and we disabled it long ago. This removes the ThreadPool usage entirely, and instead adds support for the GoogleTest 'sharded test' protocol, which uses certain env vars to allow a test to opt in for splitting its test into smaller pieces. At present our buildbot isn't attempting to make use of this feature, but it will be a big win for downstream usage in Google, where tests that run "too long" are problematic and splitting them into multiple shards makes various day to day activiites much more pleasant. * Add missing include to test_sharding.h (#6795) * Pacify clang-tidy (#6796) * Pacify clang-tidy Newer versions can warn about "parameter 'f' shadows member inherited from type 'StubOutputBufferBase'", etc -- easy enough * Update .clang-tidy * Silence a "possibly uninitialized" warning (#6797) * Silence a "possibly uninitialized" warning At least one compiler thinks we can use this without initialization, which isn't true, but this silences it. * trigger buildbots * Make all tests default to `-fvisibility=hidden` (#6799) * Step 1 * still more * Export the error classes so they can be caught * Minor typedef cleanup (#6800) * cleanup * format * Fix auto_schedule/machine_params parsing (#6804) The recent refactoring that added `execute_generator` accidentally nuked setting these two GeneratorParams. Oops. Fixed. * Rewrite strided loads of 4 in AlignLoads (#6806) * Rewrite strided loads of 4 in AlignLoads * Add a check for strided 4 load * Fix two minor bugs triggered by an or reduction with early-out (#6807) * Fix two minor bugs triggered by a or reduction with early-out * Gotta print success * Appease clang-tidy * [CMake] Mark multi-threaded tests as such (#6810) * Add support for vscale vector code generation. (#6802) Add support for vscale vector code generation. Factored from the fixed_length_vectors branch to make PRs smaller and easier to review. This will be used to support the ARM SVE/SVE2 and RISC V Vector architectures. * Rework .gitignore (#6822) * reorganize .gitignore * Add exclusions for CMake build * .gitignore: comment, drop stale rules * fully and precisely exclude CMake build tree * add debugging directions to .gitignore * ignore CMake install tree * Sort groups * Update presets to format version 3 (#6824) * Fix for top-of-tree LLVM (#6825) * Tweak python apps for better Blaze/Bazel compatibility (#6823) * Tweak python apps/tutorials for better Blaze/Bazel compatibility - Don't write to current directory (rely on an env var to say where to write) - Don't read from arbitrary absolute paths (again, rely on an env var) - Drive-by removal of unnecessary #include in Codegen_LLVM.cpp inside a lambda (!) * Recommended fixes * Revert all changes to tutorial * Revise apps * Remove apps_helpers.py * Change stub module names in Python to be _pystub rather than _stub (#6830) This is a bit finicky, but making this the default nomenclature will make some downstream usages less ambiguous and a bit easier to manage. (Yes, I realize that #6821 removes the Makefile entirely, but until it lands, it needs fixing there too.) * Apply CMAKE_C_COMPILER_LAUNCHER to initmod clang calls (#6831) * Remove Python bindings from Makefiles (#6821) * Remove Python bindings from Makefiles * Restore test_li2018 in Makefile (now C++-only) * Add dummy `test_python` target for buildbots * Add a new, alternate JIT-call convention (#6777) * Prototype of revised JIT-call convention Experiment to try out a way to call JIT code in C++ using the same calling conventions as AOT code. Very much experimental. * Update Pipeline.h * Add Python support for `compile_to_callable` + make empty_ucon static * Update PyCallable.cpp * Update buffer.py * wip * Update callable.py * WIP * Update custom_allocator.cpp * Update Callable.cpp * Add Generator support for Callables * Update Generator.cpp * Update PyPipeline.cpp * Fixes * Update callable.cpp * Update CMakeLists.txt * create_callable_from_generator * More cleanup * Update Generator.cpp * Fix Python bounds inference * Add Python wrapper for create_callable_from_generator() + Add kwarg support for Callable * Add set_generatorparam_values() + usage * Fix auto_schedule/machine_params parsing The recent refactoring that added `execute_generator` accidentally nuked setting these two GeneratorParams. Oops. Fixed. * Move the type-checking code into a constexpr code * Update Callable.h * clang-tidy * CLANG-TIDY * Add `make_std_function`, + more general cleanup * Update example_jittest.cpp * Update Callable.h * Update Callable.h * More tweaking, smaller CallCheckInfo * Still more cleanup * make_std_function now does Buffer type/dim checking where possible * Add tests for calling `AbstractGenreator::compile_to_callable()` directly * enable exports * Various fixes * Improve fill_slot for Halide::Buffer * kill report_if_error * Update callable_bad_arguments.cpp * Update Pipeline.cpp * Revise error handling * Update Callable.cpp * Update callable.py * Update callable_generator.cpp * Update callable.py * HALIDE_MUST_USE_RESULT -> HALIDE_FUNCTION_ATTRS for Callable * Scrub Python from Makefile after buildbot update (#6833) * Remove unused function in callable_generator.cpp (#6834) * Disable testing for apps/linear_algebra on x86-32-linux/Make (#6836) * Disable testing for apps/linear_algebra on x86-32-linux/Make This wasn't biting us before because we were disabling *all* apps/ on x86-32-linux (oops); the recent change to remove python testing under Make also re-enabled this test. TL;DR: this can probably be made to work somehow, but it's not worth debugging, since that case is both pretty nice, and already covered under CMake. It's literally not worth the time to fix. * Update Makefile * Rearrange subdirectories in python_bindings (#6835) This is intended to facilitate a few things: - Move all Generators used in tests, apps, etc to a single directory to simplify the build rules (this is especially useful for the work in https://github.com/halide/Halide/pull/6764) - Put all the test and apps stuff under a single directory to facilitate adding some Python packaging that can make integration into Bazel/Blaze builds a bit less painful @alexreinking, does this look like the layout we discussed before? * Better lowering of halving_sub and rounding_halving_add (#6827) * Better lowering of halving_sub and rounding_halving_add Previously, lower_halving_sub and lower_rounding_halving_add both used 9 ops. This change redirects halving_sub to use rounding_halving_add, and redirects rounding_halving_add to use halving_add. In the case that none of these instructions exist natively, this reduces it to 7/8 ops for signed/unsigned halving sub and 6 ops for rounding halving add. More importantly, this lets halving_sub make use of pavgw/b on x86 to reduce it to 3 ops for u8 and u16 inputs. * Make signed rounding_halving_add on x86 use pavgb/w too * Cast result back to signed * Add explanatory comment * Fix comment * Add explanation of signed case * Check RDom::where predicates for race conditions (#6842) Fixes #6808 * Remove Generator::value_tracker and friends (#6845) This is an internal-to-Generator helper that is used to try to detect certain classes of errors when using GeneratorStubs. To the best of my knowledge, it has ~never found a useful error in all of its existence; combined with the very limited usage of GeneratorStubs, I think this code no longer pays for itself, and should be removed. (Note that this was never externally visible, thus no deprecation warnings should be necessary.) * Deprecate/remove Generator::get_externs_map() and friends (#6844) * Deprecate/remove Generator::get_externs_map() and friends This is a feature of Generator that was added years ago to allow adding external code libraries in LLVM bitcode form (rather than simply as extern "C" or similar). In theory it allow for better codegen for external code modules (since LLVM has access to all the bitcode for optimization); in practice, we only know of one project that ever used it, and that project no longer exists. Additionally, it tended to be fairly flaky in terms of actual use -- e.g., missing symbols tended to crop up unpredictably. The issues with this feature are likely fixable, but since it hasn't (AFAICT) been used in ~years, we're better off deprecating it for Halide 15 and removing for Halide 16. (If anyone out there is still relying on this feature, obviously you should speak up ASAP.) * Also remove ExternalCode.h & friends * Also remove correctness/external_code.cpp * HALIDE_ALLOW_GENERATOR_EXTERNS_MAP -> HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE * Add placeholder code for bfloat16 in Python (#6849) (#6850) * Add placeholder code for bfloat16 in Python (#6849) This is a no-op change; I just want to mark the place(s) in the Python bindings that need attention if/when it becomes possible to support bfloat16 in Python buffers. * Update PyBinaryOperators.h * Fix the PLUGINS argument to properly join multiple arguments (#6851) * Add autoscheduling to the generator_aot_stubuser test (#6855) * Add autoscheduling to the generator_aot_stubuser test * fix test_apps * fix test_apps, again * Silence Adams2019 Autoscheduler (#6854) * Make aslog() a proper ostream * Ensure that all `dump()` calls take and use an ostream * Progress Bar only draws as LogLevel >= 1 * clang-format * Rework all aslog(0) statements * Update ASLog.cpp * syntax * Update ASLog.cpp * Revert fancy aslog stuff * Update ASLog.h * trigger buildbots * Rework autoscheduler API (#6788) (#6838) * Rework autoschduler API (#6788) * Oops * Update test_function_dag.cpp * clang-tidy * trigger buildbots * Update Generator.h * Minor cleanups * Update README_cmake.md * Check for malformed autoscheduler_params dicts * Add alias-with-autoscheduler code, plus tweaks * Update stubtest_jittest.cpp * Update Makefile * trigger buildbots * fixes * Update AbstractGenerator.cpp * Update stubtest_generator.cpp * Update Makefile * Add deprecation warning for HALIDE_ALLOW_LEGACY_AUTOSCHEDULER_API * Make AutoschedulerParams a real struct * clang-tidy * [vulkan phase0] Add adts for containers and memory allocation to runtime (#6829) * Cherry pick runtime internals as standalone commit (preparation work for Vulkan runtime) * Clang format/tidy fixes * Fix runtime test linkage and include paths to not include libHalide * Update test/runtime/CMakeLists.txt Fix typo mismatch for HALIDE_VERSION_PATCH Co-authored-by: Alex Reinking <reinking@google.com> * Add compiler id guard to build options for runtime tests * Avoid building runtime tests on MSVC since Halide runtime headers are not MS compatible Remove CLANG warning flag for runtime test * Change runtime test compile definitions to be PRIVATE. Remove PUBLIC_EXPORTS from runtime test definition. * Add comment about GNU warnings for 'no-builtin-declaration-mismatch' * Change to debug(user_context) for debug messages where context is valid. Wrap verbose debugging with DEBUG_RUNTIME ifdef. Syle pass based on review comments. * Add note explaining why we disable the internal runtime tests on MSVC. * Cleanup cmake logic for disabling runtime internal tests for MSVC and add a status message. * Don't use strncpy for prepend since some implementations may insert a null char regardless of the length used * Workaround varying platform str implementations and handle termination directly. * Clang Tidy/Format pass Co-authored-by: Derek Gerstmann <dgerstmann@adobe.com> Co-authored-by: Alex Reinking <reinking@google.com> * Promote Reinterpret Intrinsic into an Reinterpret IR Node (#6853) * Promote Reinterpret Intrinsic into an Reinterpret IR Node As discussed in https://github.com/halide/Halide/issues/6801#issuecomment-1152731683 I don't think this is complete, there are likely a few more places that need to be taught about it still, altough i think this is mostly it. Note that this only promotes the intrinsic, this does not adjust it's handling, as hinted in: https://github.com/halide/Halide/issues/6801#issuecomment-1155603752 * Silence buildbot warning * Speculative fix for Codegen C failure? * Restore comment * Delete obsolete FIXME * RegionCost: reinterpret is free * LICM: actually adjust the comment * Python source reorg (#6867) * Move python binding sources to src/halide/halide_ * Rename native module to halide_ * Fix tests * Avoid copying Python sources * Fix installation rules * Make diff smaller * trigger buildbots * Add issue todo Co-authored-by: Steven Johnson <srj@google.com> * Fix simd_op_check for top-of-tree LLVM (#6874) * Fix simd_op_check for top-of-tree LLVM * clang-format * Use pmaddubsw 8-bit horizontal widening adds (Fixes #6859) (#6873) * use pmaddubsw 8-bit horizontal widening adds * add SSE3 versions too * add pmaddubsw tests * [Codegen_LLVM] Radically simplify `visit(const Reinterpret *op)` (#6865) 1. LLVM IR `bitcast` happily bitcasts between vectors and scalars: https://godbolt.org/z/9zqx11rna 2. `ptrtoint` already implicitly truncates/zero-extends if the int is larger than the pointer type: https://llvm.org/docs/LangRef.html#ptrtoint-to-instruction 3. `inttoptr` already implicitly truncates/zero-extends if the int is larger than the pointer type: https://llvm.org/docs/LangRef.html#inttoptr-to-instruction So we don't need to do any of that 'special' handling. * [Codegen] Fail to codegen `Call::undef`, just like `Call::signed_integer_overflow` (#6871) See discussion in https://github.com/halide/Halide/pull/6866. It's not obvious if that codepath is ever hit, let's optimistically assume that it is not. If this turns out to be not true, we'll have to deal with a more complicated question of the proper lowering for it, can it be `poison`, or must it be a `freeze poison`. * Fix error in Makefile for Adams2019 on OSX (#6877) We erroneously link in the dylib and also dynamically load it, causing an error. We should skip the linkage and always load dynamically.. * Refactor/cleanup in Autoscheduler code (#6858) * Move ASLog.cpp/.h to common/ * Add trivial Parsing utility & use it * Update ParamParser.h * fixes * fixes * Ensure $CMAKE_{lang}_OUTPUT_EXTENSION is set before using it (#6879) Ensure CMAKE_{lang}_OUTPUT_EXTENSION is set before using it Co-authored-by: Shoaib Kamil <kamil@adobe.com> * #6863 - Fixes to make address sanitizer happy for internal runtime classes (#6880) * Fixes to make address sanitizer happy. Fixed initialization defects in StringStorage that could cause buffer overruns Fixed memory leaks within RegionAllocator and BlockAllocator Added system memory allocation tracking to all internal runtime tests. * Clang Tidy / Format pass * Fix formatting to use braces around if statements Co-authored-by: Derek Gerstmann <dgerstmann@adobe.com> * [Codegen_LLVM] Define all the things (#6866) Long-term plan for LLVM is to get rid of `undef`, and replace it with zero-initialization, err, `poison`, because it has nicer semantics. Everywhere we use `undef` as a placeholder in shuffle (be it either for a second operand, or undef shuffle mask element), or as a base 'empty' vector we are about to fully override via insertelement, we can just switch those to poison nowadays. The scary part is the `Call::undef` semantics/lowering, perhaps it will need to be `freeze poison`. * Add set-host-dirty/copy-to-host to PythonExtensionGen (#6869) * Add set-host-dirty/copy-to-host to PythonExtensionGen See https://github.com/halide/Halide/issues/6868: Python Buffers are host-memory-only, so if the AOT-compiled halide code runs on (say) GPU, it may fail to copy the inputs to device and/or the results back to host. This fixes that. (We still need a solution that allows for lazy copies, but that will require adding another protocol that supports it.) * Update PythonExtensionGen.cpp * Rewrite PythonExtensionGen to be C++ based (#6888) * Rewrite PythonExtensionGen to be C++ based This is intended as an alternative to #6885 -- this is even *more* gratuitous, but: - We have ~always compiled Python extensions using C++ anyway - This code is arguably terser, cleaner, and safer (the cleanups happen via dtors) - The code size difference is negligible (~300 bytes out of 160k for addconstant.cpython-39-darwin.so) * Update PythonExtensionGen.cpp * Fixes to allow compiling with LLVM16 (#6889) * Add support for generating x86 sum-of-absolute-difference reductions (#6872) * Remove (most) of the env var usage from Adams2019 (#6861) * Move ASLog.cpp/.h to common/ * Add trivial Parsing utility & use it * Update ParamParser.h * fixes * wip * fixes * Fixes * clang-format * Update Makefile * Remove may_subtile * Update Cache.cpp * Update Cache.cpp * Update AutoSchedule.cpp * Update AutoSchedule.cpp * [vulkan phase1] Add SPIR-V IR (#6882) * Import SPIRV-IR from personal branch * Refactor SPIR-V IR into separate header / source files. * Refactory SPIR-V factory methods. Fix SPIR-V interface library and header paths. Add SPIR-V internal test. * Hookup internal SPIRV IR test * Fixes and cleanups to address PR #6882 Refactor logic of SPIR-V dependency to make fetch dependecy optional Change SPIR-V fetch dependency to avoid building and just populate contents Change SPIR-V internal test to always link against method ... only enabled if WITH_SPIRV is defined Add missing SPIRV target feature * Update src/CMakeLists.txt Co-authored-by: Alex Reinking <reinking@google.com> * Add missing iostream header when WITH_SPIRV is undefined * Fix declaration ordering for TARGET_SPIRV option so that dependencies get triggered * Turn on FETCH_SPIRV_HEADERS by default to get build to pass for now * Fix path finding logic for SPIR-V header path from populated fetch dependency * Revert back to Halide_SPIRV target name * Don't use imported interface for SPIR-V. Use Halide_SPIRV naming since target is defined before Halide itself. * Add local copy of SPIR-V header file, along with license and readme. Update CMake rules to use local include path by default. * Make SPIR-V include path a system path to avoid clang format/tidy processing * Remove SpirvIR.h header file from being included with Halide.h (since it's only used internally for CodeGen) * Add ./dependencies/spirv to clang format ignore file * Add comment about *not* including internally used headers like SpirvIR.h * Refactor is_defined() asserts into check_defined() for reuse * Add comment to SpirvIR.h header clarifying this file should not be exported. Fix formatting to avoid single line if statements. Use reserve for constructing vector components * Rename hash_* methods to make_*_key methods (since they construct a key and don't actually hash the value) Fix typo on components * Clang format/tidy pass * Fix formatting for more single-line if statements * Disable TARGET_SPIRV by default for now Co-authored-by: Derek Gerstmann <dgerstmann@adobe.com> Co-authored-by: Alex Reinking <reinking@google.com> * Add `auto_schedule` label to Adams2019 and Li2018 tests in CMake (#6898) * Add `auto_schedule` label to Adams2019 and Li2018 tests in CMake These were ~never getting tested on the buildbots (and still aren't, I need to update it to run `auto_schedule` tests) but conceptually these tests should be in the same group as for Mullapudi. Also, drive-by fix to broken test_apps_autoscheduler injected in https://github.com/halide/Halide/pull/6861. * trigger buildbots * [Simplify] Drop no-op single-input identity shuffles (#6901) * [Codegen_LLVM] Annotate LLVM IR functions with `nounwind`/`mustprogress` attributes (#6897) My reasoning is as follows, please correct me if i'm wrong: 1. Halide-generated code never throws exceptions 2. Halide-generated code always `call`s (as opposed to `invoke`s) the functions, there is no exception-safety RAII 3. Halide loops are meant to have finite number of iterations, they aren't meant to be endless and side-effect free 4. Halide (IR) assertions *might* abort. 5. Likewise, external callees *might* abort. (???) Therefore, when not in presence of external calls, it is obvious that (1) no exception will be unwinded out of the halide-generated function, (2) none of the loops will end up being endless with no observable side-effects. ... which is the semantics that is being stated by the LLVM IR function attributes `nounwind`+`mustprogress`. I'm less clear as to what are the prerequisites on the behavior of the external callees, but i do believe that they must also at least not unwind. I guess they are also at least required to either return or abort eventually. * Don't try to fold saturating_sub of VectorReduce (#6896) don't fold saturating_sub of VectorReduce * Upgrade clang-format and clang-tidy to v14 (v2) (#6902) * Allow AMX instructions with K dimension larger than 4 bytes (#6582) * recognize the patterns used for the RHS matrix * make 1d tile matcher more robust * put getting rhs tile's index into a separate func * expand the tests used in correctness check * add exclamation mark * remove unused vars * run format and tidy * check for null before using IR in the next step * check if the broadcast was found * llvm below 13 is no longer supported * replace single pattern with commutative permutations * check if the stride is an `IntImm`, otherwise reject pattern * apply clang-format-13 * rename wild_i32 -> v2 * check if v1 could be the stride value * add more detail to a receiving a bad type * added short explanation of the right-hand matrix layout * added explanation for where the 4 comes from * provide further documentation as to the layout of AMX * add comments for expected patterns to get_3d_rhs_tile_index * Document the matched pattern Co-authored-by: Steven Johnson <srj@google.com> * Fix autoscheduling trivial lut wrappers (#6905) * Fix autoscheduling trivial lut wrappers Fixes #6899 * trigger buildbots Co-authored-by: Steven Johnson <srj@google.com> * Fix broken Makefile rules for autoschedulers on OSX (#6906) * Fix broken Makefile rules for autoschedulers on OSX A few issues here: - Make was building the plugins as .dylib on OSX, but they should have been .so to match Linux (and just on general principles) - On OSX, explicitly linking libHalide.dylib into a plugin means that it will load its own copy of libHalide, which is bad, because it means the plugin doesn't share the same set of globals. We need to omit that explicit dependency and allow it to just find the exported symbols at load time. - Add a test to verify the fix; run it everywhere even though it should only have been failing for Make-build OSX builds. Finally, let me add that we really need to set a sunset date for supporting Make in Halide. The Makefiles aren't really maintained properly anymore, and when something subtle goes wrong, it takes an unreasonable amount of time to debug for something that is no longer our canonical build tool. * Use order-only prerequisites * Remove new load_plugin.cpp test Not worth the complexity for the extra test coverage. * Start developing pip package (#6886) Co-authored-by: Lukas Trümper <lukas.truemper@outlook.de> * LICENSE.txt: Include full text of Apache 2.0 license (not just the 'header' version) (#6912) * LICENSE.txt: add spirv license (#6913) * LICENSE.txt: add BLAS license. (#6914) * Upgrade CMake minimum version to 3.22 (#6916) Fixes #6910. * Remove unused GHA and packaging workflows. (#6917) * Fix two warnings found with clang 16 (#6918) - variable 'count' set but not used - warning: use of bitwise '|' with boolean operands * Fix bug when realize condition depends on tuple call (#6915) If the realization is tuple-valued, and the condition on the realization uses a tuple call (index != 0), then the condition wasn't getting resolved during the split_tuples pass. The cause was a missing mutate call. * Fix wrong install path for *.py files (#6921) * Fix wrong install path for *.py files We were looking in a nonexistent dir, so we never copied `__init__.py` as we should have. * Update CMakeLists.txt * Make use of CMake 3.22 features (#6919) * Remove AddCudaToTarget.cmake * Remove MakeShellPath.cmake * Use CheckLinkerFlag in TargetExportScript * Use DEPFILE for all generators * Use REQUIRED with find_program, where applicable * Use REQUIRED with find_library, where applicable * Use CMake 3.21 cache behavior in HalideTargetHelpers.cmake * Replace uses of get_filename_component with cmake_path * Rework BLAS detection in linear_algebra app * Drive-by: fix autotune_loop.sh install rule. * Fix CBLAS header in linear_algebra test_halide_blas * Make saturating_cast an intrinsic (#6900) * Make saturating_cast an intrinsic * handle saturating_cast in Bounds.cpp + add bounds tests * update saturating_cast CodeGen * with_lanes should work on intrinsics as well * lift to saturating_cast in FindIntrinsics * update intrinsics test for u16_sat * better sat_cast(widen(expr)) handling in find_intrinsics * simplify bounds of saturating_cast + update is_monotonic * Remove guard for MCTS * Halide::Error should not extend std::runtime_error (#6927) * Halide::Error should not extend std::runtime_error Unfortunately, the std error/exception classes aren't marked for DLLEXPORT under MSVC; we need our Error classes to be DLLEXPORT for libHalide (and python bindings). The current situation basically causes MSVC to generator another version of `std::runtime_error` marked for DLLEXPORT, which can lead to ODR violations, which are bad. AFAICT we don't really rely on this inheritance anywhere, so this just eliminates the inheritance entirely. (Note that I can't point to a specific malfunction resulting from this, but casual googling based on the many warnings MSVC emits about the current situation has me convinced that it needs addressing.) * noexcept * Rework internal PYTHONPATH maintenance (#6922) * Rework PYTHONPATH * Move pure-Python file copying logic to build time. * Use TARGET_RUNTIME_DLLS to copy all DLLs instead of just Halide. * Ensure that the last path component for Halide_Python is always `halide` * Simplify __init__.py now that it's copied to build tree * Add helper to de-duplicate PYTHONPATH test logic Fixes #6870 Co-authored-by: Alex Reinking <alex.reinking@gmail.com> Co-authored-by: Alex Reinking <reinking@google.com> * Tutorial 10 needs to be skipped for Python when targeting Wasm (just as non-Python does) (#6932) * Tutorial 10 needs to be skipped for Python when targeting Wasm (just as non-Python does) * fixes * Update CMakeLists.txt * Add build & test presets for release and debug CMake builds (#6934) Also, drive-by rename of 'default' to 'base' to better imply the inheritance * Add ASAN support to CMake via toolchain file (#6920) Add ASAN support Co-authored-by: Alex Reinking <reinking@google.com> * Fix badly-merged CMakePresets.json file (#6936) * Add minimal useful implementation of extracting and concatenating bits (#6928) * Minimal approach to making Deinterleave correct for Reinterpret * Add minimal useful implementation of extracting and concatenating bits * clang-tidy * More clang-tidy fixes * Add missing error message * Add low-bit-depth noise test * Add test to cmake build * Fix power-of-two check * Remove dead object * Add little-endian comment to reinterpret IR node * Simplify concat_bits of single arg * Add missing second arg * Fix concat_bits call Co-authored-by: Andrew Adams <anadams@adobe.com> * Show error when user_error occurs * Be even more liberal about printing errors * fixed merge issue that omitted the PyEvictionKey.cpp from makefile Co-authored-by: Roman Lebedev <lebedev.ri@gmail.com> Co-authored-by: Shoaib Kamil <shoaibkamil@gmail.com> Co-authored-by: Shoaib Kamil <kamil@adobe.com> Co-authored-by: Steven Johnson <srj@google.com> Co-authored-by: Alex Reinking <alex.reinking@gmail.com> Co-authored-by: Alexander Root <32245479+rootjalex@users.noreply.github.com> Co-authored-by: Andrew Adams <andrew.b.adams@gmail.com> Co-authored-by: Andrew Adams <anadams@adobe.com> Co-authored-by: Volodymyr Kysenko <vksnk@google.com> Co-authored-by: Zalman Stern <zalman@google.com> Co-authored-by: Alex Reinking <reinking@google.com> Co-authored-by: Derek Gerstmann <derek.gerstmann@gmail.com> Co-authored-by: Derek Gerstmann <dgerstmann@adobe.com> Co-authored-by: Frederik <frederik.engels@codeplay.com> Co-authored-by: Lukas Trümper <lukas.truemper@outlook.de> Co-authored-by: Petter Larsson <petterinteon@github.com>

* `-mtune=`/`-mcpu=` support for x86 AMD CPU's * Move processor tune into it's own enum, out of features * clang-format * Target: make Processor more optional * Processor: add explanatory comments which CPU is what * Drop outdated changes * Make comments in Processor more readable / fix BtVer2 comment * Target: don't require passing Processor * Make processor more optional in the features string serialization/verification * Address review notes * Undo introduction of halide_target_processor_t * Fix year for btver2/jaguar

* Fix fundamental confusion about target/tune CPU Sooo. Uh, remember when in halide#6655 we've agreed that we want to add support to precisely specify the CPU for which the code should be *tuned* for, but not *targeted* for. Aka, similar to clang's `-mtune=` option, that does not affect the ISA set selection? So guess what, that's not what we did, apparently. `CodeGen_LLVM::mcpu()` / `halide_mcpu` actually do specify the *target* CPU. It was obvious in retrospect, because e.g. `CodeGen_X86::mattrs()` does not, in fact, ever specify `+avx2`, yet we get AVX2 :) So we've unintentionally added `-march=` support. Oops. While i'd like to add `-march=` support, that was not the goal here. Fixing this is complicated by the fact that `llvm::Target::createTargetMachine()` only takes `CPU Target` string, you can't specify `CPU Tune`. But this is actually a blessing in disguise, because it allows us to fix another bug at the same time: There is a problem with halide "compile to llvm ir assembly", a lot of information from Halide Target is not //really// lowered into LLVM Module, but is embedded as a metadata, that is then extracted by halide `make_target_machine()`. While that is not a problem in itself, it makes it *impossible* to dump the LLVM IR, and manually play with it, because e.g. the CPU [Target] and Attributes (ISA set) are not actually lowered into the form LLVM understands, but are in some halide-specific metadata. So, to fix the first bug, we must lower the CPU Tune into per-function `"tune-cpu"` metadata, and while there we might as well lower `"target-cpu"` and `"target-features"` similarly. * Address review notes * Hopefully silence bogus issue reported by ancient GCC * Call `set_function_attributes_from_halide_target_options()` when JIT compiling * Fix grammar

LebedevRI mentioned this pull request Mar 19, 2022

Halide feature names for AVX512 don't match commonly-used abbreviations #6651

Open

LebedevRI added 2 commits March 26, 2022 17:18

-mtune=/-mcpu= support for x86 AMD CPU's

a22f836

Move processor tune into it's own enum, out of features

8c0c07b

LebedevRI force-pushed the x86_mtune_amd branch from 3f0595e to 8c0c07b Compare March 26, 2022 14:57

clang-format

78bfb58

abadams reviewed Mar 26, 2022

View reviewed changes

src/Target.cpp Show resolved Hide resolved

zvookin reviewed Mar 29, 2022

View reviewed changes

src/Target.h Show resolved Hide resolved

LebedevRI commented Mar 29, 2022

View reviewed changes

src/Target.cpp Outdated Show resolved Hide resolved

LebedevRI added 6 commits March 30, 2022 02:04

Target: make Processor more optional

8b9aaef

Processor: add explanatory comments which CPU is what

2c3e5f9

Drop outdated changes

a1128b4

Make comments in Processor more readable / fix BtVer2 comment

1fe950f

Target: don't require passing Processor

bc4d8fb

Make processor more optional in the features string serialization/ver…

e218a4b

…ification

LebedevRI commented Mar 29, 2022

View reviewed changes

src/Target.cpp Outdated Show resolved Hide resolved

steven-johnson reviewed Mar 30, 2022

View reviewed changes

python_bindings/src/PyEnums.cpp Outdated Show resolved Hide resolved

src/Target.cpp Outdated Show resolved Hide resolved

src/Target.cpp Outdated Show resolved Hide resolved

src/Target.h Show resolved Hide resolved

Address review notes

9bda615

steven-johnson requested a review from halidebuildbots March 30, 2022 18:18

LebedevRI added 2 commits March 30, 2022 22:12

Undo introduction of halide_target_processor_t

f2086fd

Fix year for btver2/jaguar

d78fb63

zvookin approved these changes Mar 30, 2022

View reviewed changes

abadams approved these changes Mar 31, 2022

View reviewed changes

steven-johnson merged commit 40f895d into halide:main Mar 31, 2022

LebedevRI deleted the x86_mtune_amd branch March 31, 2022 22:24

This was referenced Mar 31, 2022

-mtune=native CPU autodetection for AMD Zen 3 CPU #6648

Merged

Future-proof 'processor to tune processor` #6673

Merged

[META] Better X86 CPU targeting #6682

Closed

LebedevRI mentioned this pull request May 13, 2022

Fix fundamental confusion about target/tune CPU #6765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`-mtune=`/`-mcpu=` support for x86 AMD CPU's #6655

`-mtune=`/`-mcpu=` support for x86 AMD CPU's #6655

LebedevRI commented Mar 19, 2022

zvookin commented Mar 20, 2022

LebedevRI commented Mar 20, 2022

zvookin commented Mar 21, 2022

abadams commented Mar 21, 2022

zvookin commented Mar 21, 2022

LebedevRI commented Mar 26, 2022

abadams commented Mar 26, 2022

LebedevRI commented Mar 30, 2022

zvookin commented Mar 30, 2022

steven-johnson commented Mar 30, 2022

zvookin left a comment

LebedevRI commented Mar 30, 2022

steven-johnson commented Mar 30, 2022

LebedevRI commented Mar 30, 2022

LebedevRI commented Mar 31, 2022

LebedevRI commented Mar 31, 2022

-mtune=/-mcpu= support for x86 AMD CPU's #6655

-mtune=/-mcpu= support for x86 AMD CPU's #6655

Conversation

LebedevRI commented Mar 19, 2022

zvookin commented Mar 20, 2022

LebedevRI commented Mar 20, 2022

zvookin commented Mar 21, 2022

abadams commented Mar 21, 2022

zvookin commented Mar 21, 2022

LebedevRI commented Mar 26, 2022

abadams commented Mar 26, 2022

LebedevRI commented Mar 30, 2022

zvookin commented Mar 30, 2022

steven-johnson commented Mar 30, 2022

zvookin left a comment

Choose a reason for hiding this comment

LebedevRI commented Mar 30, 2022

steven-johnson commented Mar 30, 2022

LebedevRI commented Mar 30, 2022

LebedevRI commented Mar 31, 2022

LebedevRI commented Mar 31, 2022

`-mtune=`/`-mcpu=` support for x86 AMD CPU's #6655

`-mtune=`/`-mcpu=` support for x86 AMD CPU's #6655