[PATCH] D30760: Record command lines in objects built by clang, Clang part

george.burgess.iv closed this revision. george.burgess.iv added a comment. Herald added a subscriber: JDevlieghere. (Committed as noted by echristo; just trying to clean my queue a bit. :) ) https://reviews.llvm.org/D30760 ___ cfe-commits mailing

[PATCH] D30760: Record command lines in objects built by clang, Clang part

echristo added a comment. Committed thusly: echristo@athyra ~/s/l/t/clang> git svn dcommit Committing to https://llvm.org/svn/llvm-project/cfe/trunk ... M lib/Driver/ToolChains/Clang.cpp M test/Driver/debug-options.c Committed r299037 https://reviews.llvm.org/D30760

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

Not really. Would you please help me commit it? Also there is another diff for LLVM part of this bug. Thanks. On Wed, Mar 29, 2017 at 1:16 PM, Eric Christopher via Phabricator < revi...@reviews.llvm.org> wrote: > echristo accepted this revision. > echristo added a comment. > This revision is

[PATCH] D30760: Record command lines in objects built by clang, Clang part

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sounds good. Do you have commit access? https://reviews.llvm.org/D30760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30760: Record command lines in objects built by clang, Clang part

zhizhouy updated this revision to Diff 93403. zhizhouy marked 2 inline comments as done. zhizhouy added a comment. Added testcase for recording other useful options. https://reviews.llvm.org/D30760 Files: lib/Driver/ToolChains/Clang.cpp test/Driver/debug-options.c Index:

[PATCH] D30760: Record command lines in objects built by clang, Clang part

echristo added inline comments. Comment at: test/Driver/debug-options.c:201-202 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches +// george.burgess.iv wrote: > echristo wrote: > > This seems a little light on the testing, would

[PATCH] D30760: Record command lines in objects built by clang, Clang part

zhizhouy updated this revision to Diff 93401. zhizhouy marked 2 inline comments as done. zhizhouy edited the summary of this revision. zhizhouy added a comment. Added two more testcases, one is options with both grecord-gcc-switches and gno-record-gcc-switches; the other one is testing if "-o -"

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

Sounds good to me. On Sun, Mar 26, 2017, 9:40 AM David Blaikie wrote: > Yeah, I don't know/mind either way - I think there's a tidy simplicity to > including exactly what the user wrote on the command line, so don't mind if > it's not removed, but can see the argument to

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

Yeah, I don't know/mind either way - I think there's a tidy simplicity to including exactly what the user wrote on the command line, so don't mind if it's not removed, but can see the argument to omit it. I'd probably leave it in for simplicity. On Fri, Mar 24, 2017 at 5:48 PM Eric Christopher

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

Not sure, that's why I asked. Is it useful? Is it one of those things we want to remove since it'll be common among all of the TUs that want the text? On Fri, Mar 24, 2017 at 3:43 PM Zhizhou Yang via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Sure I can add some more tests here. > > For

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

Sure I can add some more tests here. For -grecord-gcc-switches itself, I think maybe we could keep it in recording, since it is also one of the options from command line? On Fri, Mar 24, 2017 at 2:54 PM, Eric Christopher via Phabricator < revi...@reviews.llvm.org> wrote: > echristo added inline

[PATCH] D30760: Record command lines in objects built by clang, Clang part

george.burgess.iv added inline comments. Comment at: test/Driver/debug-options.c:201 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches echristo wrote: > This seems a little light on the testing, would you mind adding some more >

[PATCH] D30760: Record command lines in objects built by clang, Clang part

echristo added inline comments. Comment at: test/Driver/debug-options.c:201-202 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches +// This seems a little light on the testing, would you mind adding some more interesting lines

[PATCH] D30760: Record command lines in objects built by clang, Clang part

zhizhouy updated this revision to Diff 93011. zhizhouy added a comment. Added tests into test/Driver/debug-options.c. Thanks. https://reviews.llvm.org/D30760 Files: lib/Driver/ToolChains/Clang.cpp test/Driver/debug-options.c Index: test/Driver/debug-options.c

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

As Adrian mentioned, this can probably be covered/added to an existing test case in clang/test/Driver On Fri, Mar 24, 2017 at 1:57 PM Zhizhou Yang via Phabricator < revi...@reviews.llvm.org> wrote: > zhizhouy updated this revision to Diff 93003. > zhizhouy marked 3 inline comments as done. >

[PATCH] D30760: Record command lines in objects built by clang, Clang part

zhizhouy updated this revision to Diff 93003. zhizhouy marked 3 inline comments as done. zhizhouy added a comment. Checked both grecord-gcc-swtiches and gno-record-gcc-switches. Modified testcases for it. https://reviews.llvm.org/D30760 Files: lib/Driver/ToolChains/Clang.cpp

[PATCH] D30760: Record command lines in objects built by clang, Clang part

aprantl added inline comments. Comment at: test/CodeGen/debug-info-grecord-gcc-switches.c:1 +// RUN: %clang -g -grecord-gcc-switches -S -emit-llvm -o - %s | FileCheck %s +int main (void) { Shouldn't this test be in the Driver/ directory? There is already a big

[PATCH] D30760: Record command lines in objects built by clang, Clang part

echristo added a comment. Needs more testing. Might want to make sure that you actually are recording some useful command line options and that you're looking at the cc1 command line. This should also be a Driver test and not CodeGen. You can then use -### to inspect the command lines as

[PATCH] D30760: Record command lines in objects built by clang, Clang part

george.burgess.iv added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:4328 + if (getToolChain().UseDwarfDebugFlags() || + Args.hasArg(options::OPT_grecord_gcc_switches)) { ArgStringList OriginalArgs; looks like we have to consider

[PATCH] D30760: Record command lines in objects built by clang, Clang part

zhizhouy updated this revision to Diff 92885. zhizhouy retitled this revision from "Record command lines in objects built by clang" to "Record command lines in objects built by clang, Clang part". zhizhouy edited the summary of this revision. zhizhouy added a reviewer: aprantl. zhizhouy added a

[PATCH] D30760: Record command lines in objects built by clang

aprantl added a comment. In https://reviews.llvm.org/D30760#707178, @zhizhouy wrote: > Hi aprantl, thanks for replying. I checked the usage of DwarfDebugFlags, it > seems that it really did the same work of recording command line options. > > And I noticed that it is set to false by default. Is

[PATCH] D30760: Record command lines in objects built by clang

zhizhouy added a comment. Hi aprantl, thanks for replying. I checked the usage of DwarfDebugFlags, it seems that it really did the same work of recording command line options. And I noticed that it is set to false by default. Is it because of some concerns like the debug info size? Is it

Re: [PATCH] D30760: Record command lines in objects built by clang

I think that's a good idea. On Tue, Mar 21, 2017 at 5:19 PM Adrian Prantl via Phabricator < revi...@reviews.llvm.org> wrote: > aprantl added a comment. > > Sorry for being late to the party, but have you looked at > CodeGenOptions::DwarfDebugFlags? It looks like it almost does what you > want,

[PATCH] D30760: Record command lines in objects built by clang

aprantl added a comment. Sorry for being late to the party, but have you looked at CodeGenOptions::DwarfDebugFlags? It looks like it almost does what you want, but it is currently only enabled for MachO and when a specific environment variable is set. Would it make sense to generalize that

[PATCH] D30760: Record command lines in objects built by clang

echristo requested changes to this revision. echristo added a comment. This revision now requires changes to proceed. Why aren't we passing the flags down as a string in the IR? Needs a testcase: you should only check for IR in clang tests. To make sure something is propagated to the other end

[PATCH] D30760: Record command lines in objects built by clang

zhizhouy updated this revision to Diff 92546. zhizhouy marked 3 inline comments as done. zhizhouy edited the summary of this revision. zhizhouy added a reviewer: dblaikie. zhizhouy added a comment. Herald added a subscriber: mehdi_amini. Fixed George's comments. Still need to create a test for

[PATCH] D30760: Record command lines in objects built by clang

george.burgess.iv added a comment. This is looking pretty good! Aside from a few nits, I'm happy with this after we add tests. Comment at: lib/Driver/ToolChains/Clang.cpp:2725 + if (Args.hasArg(options::OPT_grecord_gcc_switches)) { +std::string CmdLineOpts = ""; +for

[PATCH] D30760: Record command lines in objects built by clang

zhizhouy updated this revision to Diff 92410. zhizhouy added a comment. Re-implemented the the patch: reusing -grecord-gcc-switches from gcc instead of passing arguments to DiagnosticsEngine. If user specify the -grecord-gcc-switches in command line, Clang will start to record it in Driver,