tra added inline comments.
================
Comment at: clang/test/Driver/cuda-unsupported-debug-options.cu:22
+// Make sure we do not see any dwarf version other than 2, regardless of
what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
// CHECK: "-triple" "x86_64
----------------
dblaikie wrote:
> tra wrote:
> > MaskRay wrote:
> > > A NOT pattern may easily become stale and do not actually check anything.
> > > Just turn to a positive pattern?
> > In this case the issue is with the CHECK-NOT line above. I'll have to
> > replicate it around the positive match of `-dwarf-version` which would
> > raise more questions.
> >
> > I wish filecheck would allow to `mark` a region and then run multiple
> > matches on it, instead of re-anchoring on each match.
> Sounds like you're looking for CHECK-DAG, maybe? (
> https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive )
I don't think CHECK-DAG can be mixed with CHECK-NOT. At least, not the way I
need them here. From FileCheck docs:
> CHECK-NOT: directives could be mixed with CHECK-DAG: directives to exclude
> strings between the surrounding CHECK-DAG: directives.
So, in order to use it here I'll need something like this:
```
// CHECK: -triple // GPU-side compilation
// CHECK-NOT: {{all unsupported options}}
// CHECK: -dwarf-version=2 // Could be CHECK-DAG, too, it does not matter in
this case.
// We have to repeat the NOT check here because the positive check above
creates another subset of input to check for -NOT.
// CHECK-NOT: {{all unsupported options}}
// CHECK: - triple // Host-side compilation
```
Ideally I want something like this:
```
// CHECK-WITHOUT-ADVANCE: -dwarf-version=2
// CHECK-NOT: {{unsupported options}}
```
If you prefer positive check with replicated -NOT, I'm fine with that.
================
Comment at: clang/test/Driver/debug-options.c:364-366
+// GEMBED_2: warning: debug information option '-gembed-source' is not
supported for target
// NOGEMBED_5-NOT: "-gembed-source"
+// NOGEMBED_2-NOT: warning: debug information option '-gembed-source' is not
supported for target
----------------
scott.linder wrote:
> dblaikie wrote:
> > This is a bit of a loss in fidelity - might need a new diagnostic message
> > (or go hunting around for a more general purpose one than this one at
> > least) to say '-gembed-source' is ignored when not using DWARFv5. (in the
> > nvptx case it's "not supported on target", but in the existing cases
> > covered by this test it's "not supported because the user requested
> > DWARFv2", for instance)
> >
> > @JDevlieghere & @scott.linder for thoughts on this.
> I agree that I'd prefer we detect whether the target-specific clamped version
> is to blame (and use the proposed warning message) or the original DWARF
> version is to blame (and use the old message).
>
> If I were compiling for x86 and gave e.g. `-gdwarf-4 -gembed-source` and the
> error said "not supported by target" I'd probably get the wrong idea.
>
> It would also be nice to retain the error semantics in the case where the
> user is explicitly requesting incompatible options.
This sounds pretty close to what the older iterations of this patch did:
https://reviews.llvm.org/D92617?id=309404, except that it preserved the current
behavior which makes it an error to use -gembed-source with an explicitly
specified DWARF version below 5. Same options with a lower clamped version
produces a warning. I.e. If user specified a nominally valid combination of
`-gdwarf-5 -gembed-source` but the target like NVPTX clamped it down to DWARF2,
there will only be a warning.
I would appreciate if you (as in debug info stakeholders) could clarify couple
of moments for me:
* should `-gdwarf-4 -gembed-source` be an error or warning? It's currently an
error.
* `-gdwarf-5 -gembed-source` with the dwarf version clamped below 5 should
produce a warning. `not supported for target` appears to be correct, but does
not explain why. Do we want to amend/change it to say `because target only
supports DWARF 2` or `target does not support DWARF 5`? Or is `not supported by
target` sufficient as is?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92617/new/
https://reviews.llvm.org/D92617
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
