[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-26 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. Thanks! Cherry-picked to 10.x as 6f4f4f2c8ce1ad17bdec9fe2071d3fe439eca9eb Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-25 Thread Rong Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG11857d49948b: [remark][diagnostics] [codegen] Fix PR44896 (authored by xur). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D748

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-25 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Nice work, LGTM too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. SGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-25 Thread Jeroen Dobbelaere via Phabricator via cfe-commits
jeroen.dobbelaere accepted this revision. jeroen.dobbelaere added a comment. This revision is now accepted and ready to land. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-24 Thread Rong Xu via Phabricator via cfe-commits
xur updated this revision to Diff 246265. xur added a comment. Uploaded a wrong patch in the last time. This is the correct one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-24 Thread Rong Xu via Phabricator via cfe-commits
xur updated this revision to Diff 246260. xur added a comment. check if the -fdiscard-value-names explicitly in args (suggested by Jeroen and Serge) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 Files: clang/include/clang/Basic/DiagnosticDr

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-24 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. In D74878#1889460 , @jeroen.dobbelaere wrote: > Will this also give a warning when passing a .ll file to a release clang, > without explicitly passing the '-fdiscard-value-names' ? Is this what we > want it to be ?

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-24 Thread Jeroen Dobbelaere via Phabricator via cfe-commits
jeroen.dobbelaere added a comment. Will this also give a warning when passing a .ll file to a release clang, without explicitly passing the '-fdiscard-value-names' ? Is this what we want it to be ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D748

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-24 Thread Rong Xu via Phabricator via cfe-commits
xur added a comment. Gentle ping. Is the newest patch ok? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-21 Thread Rong Xu via Phabricator via cfe-commits
xur updated this revision to Diff 245891. xur added reviewers: mehdi_amini, serge-sans-paille. xur added a comment. Update the patch based on the reviews from Mehdi and Serge. Reuse Sege's warning code in https://reviews.llvm.org/D74871?id=245594. CHANGES SINCE LAST ACTION https://reviews.llvm

Re: [PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-21 Thread Rong Xu via cfe-commits
OK. I will add a warning but not turn it off at the CodeGenOpt (but it will not pass to LLVMContext). I will update the patch shortly. On Fri, Feb 21, 2020 at 1:53 AM serge via Phabricator < revi...@reviews.llvm.org> wrote: > serge-sans-paille added a comment. > > Let's go that way then. @xur I

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-21 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Let's go that way then. @xur I leave it to you to add the appropriate warning at least on the clang side? Feel free to just use the code from https://reviews.llvm.org/D74871?id=245594. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-20 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. > It seems to me that we are not dropping the flag of -fdiscard-value-names. I also reads this as overriding a cc1 flag / ignoring the flag, I don't know if we consistently warn in such cases though. Overall LGTM for the fix, pending resolution for the warning, than

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-20 Thread Rong Xu via Phabricator via cfe-commits
xur added a comment. In D74878#1884769 , @serge-sans-paille wrote: > @lebedev.ri maybe we should at least warn the user if there was a flag at > clang level to force discarding? That's what I've been doing in > https://reviews.llvm.org/D74871?id=245594

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. @lebedev.ri maybe we should at least warn the user if there was a flag at clang level to force discarding? That's what I've been doing in https://reviews.llvm.org/D74871?id=245594 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://revie

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This seems like the reasonable fix to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-20 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. See also https://reviews.llvm.org/D74871 I don't know which is the better fix here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-19 Thread Rong Xu via Phabricator via cfe-commits
xur created this revision. xur added reviewers: tejohnson, jeroen.dobbelaere. This patch fixes PR44896. For IR input files, option fdiscard-value-names should be ignored as we need named values in loadModule(). Commit 60d3947922