aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88 +def err_drv_bad_target_id : Error< + "invalid target ID: %0 (a target ID is a processor name followed by an " + "optional list of predefined features post-fixed by a plus or minus sign " ---------------- erichkeane wrote: > invalid target ID %0; format is processor name followed by an optional > colon delimited list of features followed by enable/disable sign, .e.g. > 'gfx908:sramecc+;xnack-' > > ?? I think we should be leaning on the example more to explain the format. I think that's an improvement; the current wording is... hard to interpret. I tweaked it slightly, WDYT? ================ Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:92 +def err_drv_bad_offload_arch_combo : Error< + "invalid offload arch combinations: %0 and %1 (for a specific processor, a " + "feature should either exist in all offload archs, or not exist in any " ---------------- erichkeane wrote: > This is an improvement, but this one is... rough. Not sure enough of what it > is saying unfortunately to suggest a better wording. Yeah, I couldn't think of a better way to phrase it, so I figured the original wording is sufficient with some minor cleanups. I'll probably leave this one alone unless someone has a great idea. ================ Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:276 def err_drv_omp_host_ir_file_not_found : Error< - "The provided host compiler IR file '%0' is required to generate code for OpenMP target regions but cannot be found.">; + "provided host compiler IR file '%0' is required to generate code for OpenMP " + "target regions but cannot be found">; ---------------- erichkeane wrote: > provided host compiler IR file '%0' not found; required to generate code > for OpenMP target regions > > ?? I think the original wording reads somewhat better because it is grammatically correct. I don't think we gain a lot by replacing `is` with a semicolon here. ================ Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:319 def warn_drv_dwarf_version_limited_by_target : Warning< - "debug information option '%0' is not supported. It needs DWARF-%2 but target '%1' only provides DWARF-%3.">, + "debug information option '%0' is not supported; it needs DWARF-%2 but " + "target '%1' only provides DWARF-%3">, ---------------- erichkeane wrote: > it 'requires' perhaps? Good call! ================ Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:524 def warn_drv_moutline_unsupported_opt : Warning< - "The '%0' architecture does not support -moutline; flag ignored">, + "the '%0' architecture does not support '-moutline'; flag ignored">, InGroup<OptionIgnored>; ---------------- erichkeane wrote: > consider just removing 'the' from this and the next one. Good call, but I'm also going to drop `architecture` from it so it's just `'%0' does not support...` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107402/new/ https://reviews.llvm.org/D107402 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
