[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

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