fhahn added a comment.

Thanks!



================
Comment at: clang/test/Driver/aarch64-cpus.c:2
+// Check target CPUs are correctly passed.
+// TODO: The files should be split up by categories, e.g. by architecture 
versions, to avoid excessive test
 // times for large single test files.
----------------
simon_tatham wrote:
> tmatheson wrote:
> > tyb0807 wrote:
> > > simon_tatham wrote:
> > > > Tiniest nit ever: what files is this comment talking about?
> > > > 
> > > > (In the previous version, it followed on from a previous comment that 
> > > > gave the name of the file containing other half of the tests in 
> > > > question. Now it doesn't, so the wording is no longer clear.)
> > > This is for just in case we want to split up this file (aarch64-cpus.c) 
> > > further, into smaller files such as aarch64-cortex-a53.c, 
> > > aarch64-cortex-a57.c, etc. Do you think we still need to do this, or this 
> > > file looks ok to you in its current state?
> > 
> I do quite like the idea of a whole set of aarch64-cortex-<foo>.c files, for 
> the same reason I mentioned in D120875: it means that when someone is 
> implementing support for the next CPU, they'll look at the existing test 
> collection and naturally add a new file alongside all the others, instead of 
> naturally appending another stanza to this file which will eventually grow 
> too big again.
> 
> But like the previous patch, I won't block you making this much improvement 
> just because you haven't made even more improvement :-)
> I do quite like the idea of a whole set of aarch64-cortex-<foo>.c files

Yeah that sounds good. Maybe ` aarch64-cortex-cpus.c` might be a good start, to 
avoid having too many small files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121093/new/

https://reviews.llvm.org/D121093

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to