tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM with a nit.



================
Comment at: clang/test/Driver/openmp-offload-gpu-new.c:117
+
+// CHECK-LTO-FEATURES: clang-offload-packager{{.*}}feature={{.*}}ptx
----------------
tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > This should probably be a bit more specific/verbose. E.g. I'd want to 
> > > make sure that `feature=` is part of the `--image` argument and that ptx 
> > > belongs to it and is not part of some other argument (or even a file name 
> > > extension).
> > Sure, I was just hesitant to make it super specific since the specific 
> > feature will change depending on the CUDA installation (or lack thereof).
> I'm all for concise tests as long as they:
> 
> - express what you want to verify in a way that the reader would be able to 
> understand w/o having to look at the source code. 
> - are reasonably robust and do not produce false positives. `.*` wildcards 
> make it very easy to match things unintentionally.  Their use should be 
> carefully restricted. 
> 
> I wish FileCheck would have some sort of nested match check. E.g.
> 
> ```
> ; CHECK : [[CANDIDATE:--option.*?\s+]]
> ; CHECK:  [[SUBMATCH1: match something within [[CANDIDATE]]]]
> ; CHECK:  [[SUBMATCH2: match something within [[SUBMATCH1]]]]
> ```
Nit: ^^^ looks like this one slipped through the cracks and wasn't addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127686

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

Reply via email to