On Fri, 2 Aug 2024 at 18:14, Andre Vieira (lists)
<andre.simoesdiasvie...@arm.com> wrote:
>
> Hi,
>
> This looks great to me, only one small suggestion, but take it or leave
> it I think it's a matter of preference.
>
> On 11/07/2024 22:42, Christophe Lyon wrote:
>
> > + /* No predicate, no suffix. */
> > if (e.type_suffix (0).integer_p)
> > if (e.type_suffix (0).unsigned_p)
> > - code = code_for_mve_q_m (m_unspec_for_m_uint,
> > m_unspec_for_m_uint, e.vector_mode (0));
> > + code = code_for_mve_q (m_unspec_for_uint, m_unspec_for_uint,
> > mode);
> > else
> > - code = code_for_mve_q_m (m_unspec_for_m_sint,
> > m_unspec_for_m_sint, e.vector_mode (0));
> > + code = code_for_mve_q (m_unspec_for_sint, m_unspec_for_sint,
> > mode);
> > else
> > - code = code_for_mve_q_m_f (m_unspec_for_m_fp, e.vector_mode
> > (0));
> > + code = code_for_mve_q_f (m_unspec_for_fp, mode);
> > break;
>
> I'd write this as:
> if (e.type_suffix (0).integer_p)
> {
> int unspec = (e.type_suffix (0).unsigned_p
> ? m_unspec_for_m_uint
> : m_unspec_for_m_sint);
> code = code_for_mve_q (unspec, unspec, mode);
> }
> else
> code = code_for_mve_q_f (m_unspec_for_fp, mode);
> break;
>
> And same for similar below ofc. I have a slight preference to this as
> it makes it clear that both parameters are the same without needing to
> make a more difficult visual comparison and it also makes clear what the
> difference is between the unsigned_p true or false.
>
> Not a strong opinion here though.
I think you are right, this goes in the same direction of what this
cleanup intended: remove duplication and make it easier to
read/maintain.
I'll update this patch accordingly.
Thanks,
Christophe
