Add Google Highway SIMD acceleration for ImageBufAlgo operations

[ssh4net]

perf(IBA): Add Google Highway SIMD fast paths for core operations

Implement SIMD acceleration using Google Highway for ImageBufAlgo add, sub,
mul, pow, and resample operations. Provides 2-8x performance improvement for
contiguous pixel layouts with portable vectorization across x86 and ARM.

New file imagebufalgo_hwy_pvt.h provides reusable SIMD infrastructure with
automatic type promotion/demotion (uint8/uint16/int16/uint32/half/float/double),
generic operation kernels, and smart fallback to scalar code for strided layouts.

Operations use runtime vector width detection (ScalableTag), FMA instructions
where applicable, and handle partial vectors correctly. Code follows OIIO style
with modern C++ casts and comprehensive documentation.

Requires: Google Highway library (MIT license, header-only)
Modified: imagebufalgo_{addsub,muldiv,pixelmath,xform}.cpp + new hwy_pvt.h

Checklist:

• I have read the guidelines on contributions and code review procedures.
• I have updated the documentation if my PR adds features or changes
  behavior.
• I am sure that this PR's changes are tested somewhere in the
  testsuite.
• I have run and passed the testsuite in CI before submitting the
  PR, by pushing the changes to my fork and seeing that the automated CI
  passed there. (Exceptions: If most tests pass and you can't figure out why
  the remaining ones fail, it's ok to submit the PR and ask for help. Or if
  any failures seem entirely unrelated to your change; sometimes things break
  on the GitHub runners.)
• My code follows the prevailing code style of this project and I
  fixed any problems reported by the clang-format CI test.
• If I added or modified a public C++ API call, I have also amended the
  corresponding Python bindings. If altering ImageBufAlgo functions, I also
  exposed the new functionality as oiiotool options.

[lgritz]

It's gonna take me a few days to do a thorough review -- I'm under the weather this week and mostly sacked out in bed with some strong meds, so brain isn't what it should be.

But I like the direction of this. Stylistically, it's not very different from the small number of places that I've "hard coded" our simd.h approaches to certain math IBA ops, but mostly I only did it for a small number of common cases, for example over_impl_rgbafloat special case of IBA::over() when all images are 4 channel float, in imagebufalgo_composite.cpp. But you're handling many more cases, and in a more principled way using hwy.

This PR seems to have completely removed the entire directory of GHA workflows.

[ssh4net]

Sure, no rush. I also will experiment with some IBA functions, that might be a good for SIMD use

[ssh4net]

I have added

OIIO::attribute("enable_hwy", 0) #- disable highway SIMD code path
OIIO::attribute("enable_hwy", 1) #- enable highway SIMD code path

and added hwy_test as a tool (sorry)

that run 10x times current and 10x times SIMD versions of add/mul/sub/pow range expand/compress and resample.
and show a speedup

take into account that native double support is not exist in oiio. data processed in float and cast back to double.

[ssh4net]

Highway SIMD Benchmark
======================
Image size: 2048x2048
Iterations: 20
Initial enable_hwy: 1

[ Add ]
Type       | Scalar(ms) |   SIMD(ms) | Speedup
----------------------------------------------------
uint8      |       3.11 |       0.40 |   7.67x
uint16     |       3.11 |       2.05 |   1.51x
uint32     |      50.06 |      54.43 |   0.92x
float      |       6.04 |       5.51 |   1.10x
half       |       5.87 |       2.22 |   2.65x
double     |      70.42 |      67.11 |   1.05x

[ Sub ]
uint8      |       2.58 |       0.47 |   5.54x
uint16     |       3.18 |       2.35 |   1.35x
uint32     |      53.53 |      49.03 |   1.09x
float      |       5.86 |       5.82 |   1.01x
half       |       5.86 |       2.51 |   2.34x
double     |      73.58 |      69.75 |   1.05x

[ Mul ]
uint8      |       2.90 |       0.68 |   4.26x
uint16     |       3.32 |       2.80 |   1.18x
uint32     |      52.51 |      50.74 |   1.03x
float      |       6.21 |       6.11 |   1.02x
half       |       6.01 |       2.35 |   2.55x
double     |      68.04 |      67.11 |   1.01x

[ Pow ]
uint8      |       9.05 |       8.14 |   1.11x
uint16     |       9.51 |       6.22 |   1.53x
uint32     |      40.89 |      38.70 |   1.06x
float      |       8.05 |       6.70 |   1.20x
half       |      11.80 |       9.00 |   1.31x
double     |      55.06 |      53.29 |   1.03x

[ RangeCompress ]
uint8      |       7.26 |       0.93 |   7.78x
uint16     |       6.71 |       0.90 |   7.44x
uint32     |      37.41 |      38.62 |   0.97x
float      |       5.33 |       3.61 |   1.48x
half       |       8.62 |       0.66 |  13.11x
double     |      51.29 |      53.41 |   0.96x

[ RangeExpand ]
uint8      |       6.07 |       1.19 |   5.12x
uint16     |       5.92 |       1.63 |   3.64x
uint32     |      35.39 |      40.68 |   0.87x
float      |       4.81 |       3.57 |   1.35x
half       |       8.15 |       0.99 |   8.26x
double     |      49.28 |      48.33 |   1.02x

[ Premult ]
uint8      |       3.35 |       0.90 |   3.73x
uint16     |       3.70 |       1.18 |   3.13x
uint32     |      48.19 |      49.18 |   0.98x
float      |       5.60 |       5.81 |   0.96x
half       |       7.07 |       1.63 |   4.34x
double     |      64.55 |      70.50 |   0.92x

[ Unpremult ]
uint8      |       3.32 |       0.95 |   3.49x
uint16     |       3.34 |       1.86 |   1.80x
uint32     |      47.02 |      51.97 |   0.90x
float      |       5.80 |       5.41 |   1.07x
half       |       6.62 |       2.11 |   3.13x
double     |      65.72 |      65.54 |   1.00x

[ Resample 75% ]
uint8      |    1176.71 |       1.51 | 780.56x
uint16     |    1294.92 |       1.73 | 750.27x
uint32     |    1240.54 |      24.10 |  51.47x
float      |    1221.65 |       1.52 | 804.94x
half       |    1258.42 |       4.67 | 269.21x
double     |    1233.69 |      35.07 |  35.18x

[ Resample 50% ]
uint8      |     535.80 |       0.77 | 698.20x
uint16     |     542.99 |       0.75 | 721.94x
uint32     |     549.54 |      19.68 |  27.92x
float      |     542.92 |       0.67 | 811.66x
half       |     564.81 |       2.27 | 248.49x
double     |     559.15 |      24.08 |  23.22x

[ Resample 25% ]
uint8      |     145.09 |       0.31 | 464.61x
uint16     |     133.62 |       0.31 | 429.51x
uint32     |     146.55 |      15.63 |   9.37x
float      |     128.73 |       0.30 | 422.86x
half       |     149.94 |       0.57 | 261.31x
double     |     151.33 |      16.23 |   9.32x
Type       | Scalar(ms) |   SIMD(ms) | Speedup
----------------------------------------------------

Benchmark complete!

[lgritz]

I would recommend that you stop adding new functionality for now, and let's iterate on the review and whether we like the exact method and form of what you have. If we decide we want to make changes, the more IBA functions you have rewritten, the harder it will be to make the changes in all places.

[lgritz]

Can you please restore the workflows directory that you deleted? As it stands, this PR is not running any CI because of that change.

[lgritz]

I would prefer this be an optional dependency,

Suggested change

	checked_find_package (hwy REQUIRED)
	checked_find_package (hwy
	DEFINITIONS USE_HWY)

And then #ifdef USE_HWY in the cpp files to conditionally compile the parts that need hwy.

If we add a hard dependency, it precludes backporting any part of it to 3.1, since we don't change minimum dependencies in a released branch.

[lgritz]

CI is failing because (perhaps among other reasons) we never have hwy installed. I think it would help to have a build_hwy.cmake auto-builder and maybe a build_hwy.bash to use for CI installation.

[lgritz]

I think the overall scheme here is good, but I'm concerned about the massive amount of extra code here that has to be maintained in parallel with the reference scalar implementation. (And also, I would expect it to greatly increase the compilation time.)

As an example, contrast with add_impl_scalar is about 10 lines of code (for each of the image+image and image+const flavors) and really easy to understand, whereas the hwy implementations are another 200 lines (including image+image having to split into the float and integer cases). This pattern appears to be needed for all IBA functions we wish to implement hwy versions of.

I don't think this is insurmountable at all. I just think we're going to need to work to make the situation easier to manage, and some suggestions come to mind. Not sure if any of what I'm about to say is practical, but maybe we can find a way.

First, I note that each of these functions has a LOT of boilerplate to set up. But I feel like maybe most of that is repeated over and over? For example, it seems to me that the hwy implementations of add() and sub() are almost completely identical, except that at some point one has a hn::Add(a, b) and the other a hn::Sub(a, b). And while I haven't done a line-by-line comparison of the float versus int varieties, do they really differ all that much other than changing hn::Add to hn::SaturatedAdd?

So, is it possible that in imagebufalgo_hwy_pvt.h (where you have thankfully already abstracted out the Load/Store/Promote/Demote boilerplate), there is a way to create a single template each for unary, binary, and ternary pixel-by-pixel-independent style operations, where it is passed a functor/lambda with just the bare operation (Add, Sub, etc.)? So that maybe the operation_hwy_impl() for each IBA function is very tiny and mostly just supplying the right kernel operation?

It's a little bit (conceptually) like what we recently did with perpixel_op(), where it's possible to write the simple pixel-by-pixel operations in just a couple lines of new code each.

I'm obviously not experienced with hwy. Do you think this is practical?

It's almost a little confusing that you have done so much already. I feel like it would have been easier if you stopped after a minimal prototype, like only doing IBA::add() and IBA::sub(), and we looked at that and said "what's the absolute minimum text size, least repetition, and simplest way to express each of these that doesn't sacrifice too much performance over the super optimal but maximally verbose version?" and iterated on that to see how simple we can get it, THEN replicate that methodology over as many other IBA functions as needed.

[lgritz]

Also, I see from your benchmarks (which may not reflect the latest code), that the performance improvement is not uniformly good.

The uint32 stands out as often being slower, and never being a big enough improvement over the scalar code to be worth the template expansion of the extra cases, I bet. But do we need it at all? I've never seen an image file where somebody used uint32 pixel values -- except for channels that represent Object IDs or other ID tags, not color values, so those channels would not be meaningful to add or do most other image arithmetic for. So maybe uint32 should be removed from the Hwy templates and just let that case always fall back to the scalar code?

There are other rare cases. Almost all image files are uint8, uint16, half, or float pixel values. The signed integers and 32 bit integers are rarely encountered. And many cases of mixing types are rare as well (for example, if somebody is encountering half at all, it's probably because they have a pipeline where everything is OpenEXR, and will almost never directly do binary image operations on one half image and one uint16 image, say. So maybe we can cut down drastically on compile times and library size bloat if we only expand hwy implementations for the small number of common combinations, let the other combinations fall back to the scalar code that handles all types, and we can always expand the set if we realize that there is a widely used combination that we've missed. Also, may as well fall back to shaders for combinations that we think are common, but we can't seem to make the hwy version be significantly faster. (Again, if we later figure out how to make a missed combination fast, we can add it at any time without any compatibility change.)

[ssh4net]

I also do not happy with uint32 and float performance. Probably it not scaling well because I mostly testing on AVX2 with 8x32 that is only two RGB or RGBA vectors. I need to do a AVX512 benchmarks.
As for lot of a boilerplate code, and much functions, for sure, that's something I want to find how to simplify, maybe with inline functions.
So I still not sure if this branch are ready for upstream yet. There are lot of hidden pitfalls in SIMD intrinsics that might not well unified even with HWY.

[lgritz]

I also do not happy with uint32 and float performance. Probably it not scaling well because I mostly testing on AVX2 with 8x32 that is only two RGB or RGBA vectors.

It's hard to tell. In my experience, sometimes AVX2 is about as fast as you can get. Many of the Intel chips with clock down if you use the 512 bit instructions, and that can slow down subsequent scalar, 128/256-bit simd, or integer code until it raises the clock again. In other words, 512 bit is often only a win if you can do a large number of 512-wide instructions in a row. It can be really hit or miss if you are mixing different width SIMD, or if uou can't keep enough SIMD lanes full.

I'm really not sure why the float results aren't better. It's important to understand. Is it somehow already auto-vectorizing the float case well even for the scalar case? Or is there some reason why the hwy implementation for that is not especially effective? Or is something simple like 'add' limited more by memory bandwidth saturation than by floating point throughput? It's important to keep in mind that for oiiotool (a very important use case), by default it expands most images to float while in memory for maximum precision, so the float case is by are the most common one for anybody using oiiotool.

I need to do a AVX512 benchmarks. As for lot of a boilerplate code, and much functions, for sure, that's something I want to find how to simplify, maybe with inline functions. So I still not sure if this branch are ready for upstream yet. There are lot of hidden pitfalls in SIMD intrinsics that might not well unified even with HWY.

Well, so this is kind of what I was getting at. I feel like this is showing enough promise that I believe we can count on adding some sort of hwy support, as long as it's an optional dependency, but I think some of the internals have some kinks to work out -- which IBA functions to use hwy for, which type combinations, how to express it. But those can be iterated over time.

If it were me, this is the suggestion for how I would proceed, step by step:

1. Submit a NEW PR (against main) that contains just basic core infrastructure:

   • The build system parts: look for optional hwy dependency, set compile flags, supply hwy-building script build_hwy.bash / build_hwy.cmake like the other dependencies have.
   • The part in imageio.cpp that lets you set and query the "enable_hwy" option and related switches.
   • Some parts of imagebufalgo_hwy_pvt.h that we're confident will be needed.
   • Changes to testing.cmake to allow easily running selected tests TWICE -- once with hwy off and once with it on (the latter only if hwy is found and enabled at build time), so we are always in a state of knowing that CI is fully testing the hwy implementations against the corresponding scalar implementations.
   • Benchmarking methodology or test program that allows us to time hwy vs regular. I would prefer that it use our usual Benchmarker approach, which you can see how simple it is in perf: IBA::resample improvements to speed up 20x or more #4993, where I've added resample benchmarks along with my performance fixes.

   This much should be trivial to review and merge right away, and has no risk since it's opt-in both for dependency and use.

2. The next PR after that contains JUST the proposed hwy implementations for

   • IBA::add()
   • IBA::sub()
   • Or, if you prefer, a different choice of at most 2-4 simple candidate functions to experiment.

   And at this point, we can extensively iterate and debate the implementation, focusing on

   • How to most simply and compactly express each op, ideally trying very hard to make the hwy implementation look as simple as the scalar version (but probably relying on some heavy template approaches in imagebufalgo_hwy_pvt.h, which we would anticipate will be widely used by many IBA functions in the future).
   • Benchmarking to understand which type combinations give real benefit, so we aren't bloating the library with hwy code that gives no substantial benefit. And allowing us to understand why any are not as performant as we would expect.

3. We may wish to continue iterating at this point, perhaps while SLOWLY expanding the set of IBA functions with hwy implementations, to continue improving the considerations of how to express it cleanly and improve performance. I'm not worried about having many rounds of refinement or rewrite here, since it's all internals that can't break ABI or behavior (since it's all opt-in and also we are fully testing everything in CI to make sure hwy and scalar results never diverge).

4. Finally, when we are happy that we are on the right track to maximal performance and minimal repeated boilerplate, that it's all going to be easy to maintain in the future, then we can start greatly expanding the set of IBA functions with hwy implementations. At each stage, though, we should only add hwy implementations (and within each hwy IBA implementation, only the type combinations) that show sufficiently positive gains in the benchmarks. I'd rather have only 10 hwy augmented IBA functions that really make a difference, than be burdened with 100 alternate implementations that mostly only have tiny perf gains.

[ssh4net]

@lgritz, I want to run code restructuring first in this PR, and after that will create a new PR for main with limited resample + add/sub/mul/mad/div
is this good?

[lgritz]

@lgritz, I want to run code restructuring first in this PR, and after that will create a new PR for main with limited resample + add/sub/mul/mad/div is this good?

Yes, definitely.

I'm just trying to lay out a path that, ultimately, will make it easiest to accept your changes. What I'm afraid of is that you'll write so much code before we have refined the methodology, that you will experience some "sunk cost fallacy" where it will seem like too much work to change it all to a better methodology.

To use another analogy, do you know the adage about how if you throw a frog in boiling water, it will jump right out, but if you put the frog into room temperature water that slowly heats to a boil, he will never notice until it's too late?

I'm the frog! I'm saying that a series of small PRs -- each of which is obvious and totally defensible as the right approach, each of which is easy to read/understand/test, each of which is simple to steer to be better if I or other onlookers have suggestions -- will get accepted quickly in series, whereas the same set of changes all batched together in total into one huge PR (in a branch that my have diverged quite a bit from main if they take a long time) may be very difficult to review and decide to merge.

[lgritz]

As an aside, I would love your review of #4993 and then after I merge it, for you to rebase on top of it (or merge into your work) so that you are benchmarking versus my improved IBA::resample() fixes.

In general, I think that ideally, we would like to aim for the hwy implementations to result in a 2-10x improvement versus scalar. That would be awesome, and is about as good as one can reasonably expect from turning scalar code into fairly good SIMD code for AVX2 or AVX512.

• If we are seeing >> 10x improvement (as you did with resample), I think that's an indication of a serious perf bug in the existing scalar version, and we should look into fixing that independent of the hwy work. Doing so will also allow you to more properly evaluate how much gain hwy brings to that particular function.
• If we are seeing << 2x improvement, we should investigate exactly why that is, and consider perhaps not using hwy for that particular IBA function or type combination if the gain is too small to justify the additional code and maintenance over the years.