[PATCH v2 2/2] Aarch64: Add new memset for Qualcomm's 0ryon-1 core

Andrew Pinski (QUIC) quic_apinski@quicinc.com
Wed May 22 23:01:53 GMT 2024


> -----Original Message-----
> From: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> Sent: Tuesday, May 14, 2024 12:41 AM
> To: Florian Weimer <fweimer@redhat.com>; Andrew Pinski
> (QUIC) <quic_apinski@quicinc.com>
> Cc: libc-alpha@sourceware.org
> Subject: RE: [PATCH v2 2/2] Aarch64: Add new memset for
> Qualcomm's 0ryon-1 core
> 
> > -----Original Message-----
> > From: Florian Weimer <fweimer@redhat.com>
> > Sent: Tuesday, May 14, 2024 12:32 AM
> > To: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> > Cc: libc-alpha@sourceware.org
> > Subject: Re: [PATCH v2 2/2] Aarch64: Add new memset for
> Qualcomm's
> > 0ryon-1 core
> >
> > * Andrew Pinski:
> >
> > > +L(try_zva):
> > > +	/* Write the first and last 64 byte aligned block using
> stp rather
> > > +	   than using DC ZVA.  This is faster on some cores.
> > > +	 */
> >
> > The “some cores” part seems outdated if it's just a memset
> for the
> > Oryon-1 core (singulare).  This comment and some others,
> for example
> 
> Will fix.
> 
> >
> > > +	/*
> > > +	 * Adjust count and bias for loop. By subtracting extra 1
> from count,
> > > +	 * it is easy to use tbz instruction to check whether
> loop tailing
> > > +	 * count is less than 33 bytes, so as to bypass 2
> unnecessary stps.
> > > +	 */
> >
> > do not use GNU style.  This one is GNU style:
> 
> This was copied exactly from memset_emag.S which has the
> style issue in it too. It was in 9627ab99b50 commit where this
> comment was introduced which copied from
> memset_base64.S.
> Should we fix the other files or just the new files? Because it
> seems like having one version being based on the other once
> and then changing the style in one case but not the other
> seems wrong.
> I suspect there many more GNU comment style issues in the
> aarch64 mem* functions even.

Ping on this question? I don't want to update my patch until I get a further clarification on if the other files should be fixed in a similar way or if it is ok having the two files have different coding styles or should we just keep them the same.
I don't care one way or the other, I will implement it either way. Though it seems like it should be up to the maintainer to fix up coding style issues of what was already there; it should not be the burden of person submitting code that is doing copying.

Thanks,
Andrew Pinski

> 
> Thanks,
> Andrew Pinski
> 
> >
> > > +	/* Set 64..96 bytes.  Write 64 bytes from the start and
> > > +	   32 bytes from the end.  */
> >
> > No separate start end end lines, no * indentation, space after
> the final .
> >
> > Thanks,
> > Florian
> 



More information about the Libc-alpha mailing list