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 li
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
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 no
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
http://lists.
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: test/Driver/debug
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 you
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 -"
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 omit it. I'd probably le
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 via
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 -
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
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
> in
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 here?
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
=
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.
> zhiz
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
test/Driver/gr
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 fi
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 passe
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 `-gno-
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 co
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
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 prope
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, bu
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 mech
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 u
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 th
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
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, and
28 matches
Mail list logo
