Allow vector additions to be optimized #5636
Conversation
This change allows optimizeAddedConstants to work on vector additions and subtractions. There were no other changes. I don't know how what I did worked, or why it worked without additional changes, but it worked. Fixes WebAssembly#5633.
| @@ -2769,7 +2773,7 @@ struct OptimizeInstructions | |||
| // and combine them note that we ignore division/shift-right, as rounding | |||
| // makes this nonlinear, so not a valid opt | |||
| Expression* optimizeAddedConstants(Binary* binary) { | |||
| assert(binary->type.isInteger()); | |||
| assert(binary->type.isInteger() || binary->type.isVector()); | |||
|
|
|||
| uint64_t constant = 0; | |||
There was a problem hiding this comment.
I think this needs to be generalized. This is the constant value that is added up, and 64 bits is enough for i32 and i64 but not v128. Instead, this can use a Literal which abstracts over all constants.
And for testing, see for example test/lit/passes/optimize-instructions-strings.wast - please create a parallel one for simd instead of strings.
There was a problem hiding this comment.
How do I generalize that? I can make constant and mul Literals and update everything accordingly, but that doesn't seem to work properly. It seems that v128 Literals can't be added or multiplied with other Literals, which makes sense.
There was a problem hiding this comment.
I think we just need to add vector Literals with other vector Literals? That is, we'd be gathering Literals all of the same type in all cases (all i32, or all i64, or all v128), and we need to make Literals of the right type for shifts etc.
There was a problem hiding this comment.
How do I add two v128 Literals together if the type of add (i32x4, f64x2, etc.) can vary? I'm unfamiliar with Binaryen's source in general, so I don't know any tricks that I could use.
There was a problem hiding this comment.
Ah, yes, you'd need to make the adds use the right operation. So right now where it does Abstract::getBinary(binary->type, Abstract::Add), which abstracts over i32 and i64, I guess we'd need to look for SIMD operations, and handle each of them there (using the right method). But maybe there's an even better way somehow, using the interpreter.
@tlively I'm guessing there is some clever abstract interpretation solution to this..?
There was a problem hiding this comment.
Yes ;) It would be similar to equipping precompute-propagate with the ability to do (some) algebra, though, so not a tiny project, even with an abstract interpretation framework in place.
This change allows optimizeAddedConstants to work on vector additions and subtractions. There were no other changes. I don't know how what I did worked, or why it worked without additional changes, but it worked.
Fixes #5633.