On 14.02.2011 19:18, Peter Maydell wrote: > On 11 February 2011 15:10, <christophe.l...@st.com> wrote: >> From: Christophe Lyon <christophe.l...@st.com> >> >> This patch series provides fixes such that ARM Neon instructions >> VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN now >> pass all my tests. >> >> I have reworked all these patches and I hope they are now easier to >> review. > > Thanks; this was indeed a lot easier to review. Mostly > this is OK, there are a few things: > * minor style issues > * handling of very large shift counts is not right > (both in code you wrote and existing routines)
Yes, I mostly copied existing code to fix the parts I identified as wrong, but indeed very large shift amounts are not part of my tests. > * the shift-and-narrow loop doesn't handle the case > where pass 1 reads registers pass 0 writes > > I have patches which (sitting on top of your 6) fix all > these and give a clean pass on the random instruction > set testing for the shift instructions. > > Unless you object, I think the simplest thing will be for > me to just fix the minor nits I identified in your patches > and then post a combined series of your patches and > mine. OK, thanks. It also seems that the neon_shl_s* helpers don't handle correctly large negative shift amounts. Christophe.