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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits