Open wolfy1961 opened 4 years ago
I've committed the simple clamp to 255 for all 8 affected intrinsics in 641d2e5232b423a7dd81afac94dd3db4412a4971. Using 255 avoids needing to decode the element size from the intrinsic.
Simon, do you think I should optimize it to produce 0s when possible? Or should we start working on MMX->SSE
Returning 0s for out of range logical shifts should be a relatively small change, making a dent in [Issue #41665] is likely to be a much bigger issue.
I've committed the simple clamp to 255 for all 8 affected intrinsics in 641d2e5232b423a7dd81afac94dd3db4412a4971. Using 255 avoids needing to decode the element size from the intrinsic.
Simon, do you think I should optimize it to produce 0s when possible? Or should we start working on MMX->SSE
Not that tricky. We need to clamp for psrai. And for the other two emit an i32 zero and an x86isd node to move i32 to mmx. That will pattern match to the pxor zero idiom I think.
The PSRAD with immediate instruction only supports 8-bits worth of shift so we masked 268435456 to 8-bits and got 0. The same issue exists for shift left and shift right for the MMX builtin. We do better with SSE shifts.
I'm inclined to fix this by just clamping the shift amount to 31 for all three shift amounts. We could do better for left/right shifts and the result to 0 explicitly, but I'm not sure we care to optimize MMX that well.
How tricky would it be to match what we do for SSE for out of bounds values?
The PSRAD with immediate instruction only supports 8-bits worth of shift so we masked 268435456 to 8-bits and got 0. The same issue exists for shift left and shift right for the MMX builtin. We do better with SSE shifts.
I'm inclined to fix this by just clamping the shift amount to 31 for all three shift amounts. We could do better for left/right shifts and the result to 0 explicitly, but I'm not sure we care to optimize MMX that well.
Extended Description
The attached IR features a call to a builtin that generates a PSRAD instruction. The shift count is large, and in such cases the instruction's definition says that the result should be equal to the operand's sign bit (or the sign bits of its respective elements) extended to the entire width of the result (i.e. 0 or -1).
The instruction, however, is emitted with a shift count of 0, yielding a result identical to the input operand.
I do not think undefined behaviour is in play here, since I would expect that the semantics of a builtin are directly derived from the that of the instruction they represent, though I could be wrong about that.
The IR has been generated from the following C source:
gcc 7.4.0 prints 0 for this code.
In a late stage IR dump the PSRAD instruction shows up with the correct shift count:
renamable $mm0 = MMX_PSRADri killed renamable $mm0(tied-def 0), 268435456