[PATCH] D127686: [Offloading] Embed the target features in the OffloadBinary

[Artem Belevich via Phabricator via cfe-commits]

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