anirudhp marked 2 inline comments as done. anirudhp added inline comments.
================ Comment at: clang/test/CodeGen/target-data.c:256 +// RUN: %clang_cc1 -triple s390x-none-zos -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=ZOS ---------------- MaskRay wrote: > MaskRay wrote: > > anirudhp wrote: > > > MaskRay wrote: > > > > If you add so many RUN lines at once, please use unittests instead. > > > > This would cost some test execution time > > > We're essentially matching what was available for the systemz elf target > > > (above lines) for the z/OS target. Is there a real concern for the > > > execution time here for moving it to a separate unit test. If we did, it > > > would seem to "stand out" for just the z/OS target. > > This is a real concern. Perhaps you can also move the SYstemZ ELF tests to > > a unit test. > It also doesn't appear useful to enumerate every combination when the patch > doesn't touch these combination individually. > > I.e. if you add z/OS condition to `arch8` code, then having a test is fine. > If you don't touch it, I don't think you should enumerate `{elf,goff} * > {z10,arch8,z196,arch9,...}` After a bit of discussions with @uweigand , we've reduced the number of run lines for the SystemZ target(s) (both elf and goff). They should be greatly reduced now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109362/new/ https://reviews.llvm.org/D109362 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits