Added intrinsics for the instructions:
Added test cases to existing buffer load/store tests.
Matt, we do actually need these intrinsics as we have an urgent requirement for them Open Vulkan (which is of course my motivation for implementing them).
As Tim commented, the load ubyte and load short instructions extend to 32 bits. While float is a little odd, it does match the behavior of the other buffer_load instructions. Also I think changing it would require a disproportionate amount of effort.
If we define an overloaded intrinsic with a return type of i8, and the IR using it wants the value zero extended to i32, the frontend would then have to emit a separate zext. I guess we could optimize that to the zero-extending instruction in instruction selection, but wouldn't it be better to have the intrinsic match what the ISA instruction does by returning the zero extended i32?
You shouldn't be inspecting the users. You can just unconditionally use one or the other. You're going to have to insert a truncate back to the original type at the end anyway. You can then add a separate optimization to fold in the sext_inreg or mask into the buffer like is done for loads
You can just unconditionally use load_ubyte/load_ushort. Folding the sign extend in is then a separate optimization on a sext (or more likely a sext_inreg)
So outputting byte/short based on sign_extend in the tablgen pattern? This won't allow re-use of the existing multiclass without changes.
I think I remember there being a reason Nicolai and I decided not to do this.
This looks identical to the other part, which is kind of surprising to me but this should be factored into something common
This comment can be removed
Should have a hasOneUse check
This is missing a check on the source type. If you want to be fancier, you can split out the remainder bits into a new sign extend but there probably isn't much reason to
"will be set by" part doesn't make sense here
The base test case shouldn't have a extend of the use and directly use the value. You should also have one with an explicit zext
Could use a testcase with a second non-extended use
Do you mean that there is simliar/same code in the different cases of buffer_load? (ie struct, raw, normal) And that these should be factored into something common (ie function call)?
You mean there should be a hasOneUse check on the SIGN_EXTEND_INREG right?
Src is the BUFFER_LOAD_XXX. The only way this code is executed is if the Src is a BUFFER_LOAD_XXX. I'm not sure we need a redundant check here do we?
No, the buffer operation. If there are multiple uses you will end up creating multiple loads
The number of bits in the sext_inreg may not match the load's from-8/16 bit source. You can test this with something like
There will need more shifts to clear the extra bits in the loaded value
You can either leave it as
I'm not sure there's much practical difference between them
Mostly LGTM except some nits. The major one is avoiding the repeated lowering code for each of these cases
You can just hardcoded this to MVT::Other
Extra space before ==
Extra space before ==