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

Reply via email to