r294676
On Thu, Feb 9, 2017 at 4:05 PM David L. Jones via Phabricator <
revi...@reviews.llvm.org> wrote:
> dlj added inline comments.
>
>
>
> Comment at: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp:125
> + switch (Status) {
> + default:
> +llvm_unreachable("Do not expect to
dlj added inline comments.
Comment at: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp:125
+ switch (Status) {
+ default:
+llvm_unreachable("Do not expect to enter a file from current scope");
As a heads up... this fails under -Werror:
llvm/tools/clang/lib/Code
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294637: [DebugInfo] Added support to Clang FE for generating
debug info for… (authored by aaboud).
Changed prior to commit:
https://reviews.llvm.org/D16135?vs=87819&id=87881#toc
Repository:
rL LLVM
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Could you also add support for -g3? It looks like we are supporting -g0 and
-g1, so we should probably also mirror -g3.
Thanks a lot for all your work!
https://reviews.llvm.org/D16135
_
aaboud updated this revision to Diff 87819.
aaboud marked 2 inline comments as done.
aaboud added a comment.
Added test cases for driver debug info macro flags in
test/Driver/debug-options.c
Applied some changes to the documentation suggested by Ardian.
https://reviews.llvm.org/D16135
Files:
aprantl added a comment.
Please also add driver testcase (e.g., to test/Driver/debug-options.c).
Technically clang does accept -g3 as an option, so we could wire it up for
compatibility. I would still keep the -fmacro-debug driver option though.
Comment at: docs/UsersManual.rs
aaboud added a comment.
> How are you measuring the build time? Total time for, say "ninja clang" with
> full parallelism? That'd be hard to measure the actual impact (since it could
> be the link time or other things are dominating, etc). If you have a reliable
> way to time (I'm assuming Inte
aaboud updated this revision to Diff 87796.
aaboud added a comment.
Added flag to control macro debug info generation:
1. -fdebug-macro (-fno-debug-macro) for Clang driver
2. -debug-info-macro for Clang compiler (-cc1)
Also, made sure FE will discard this flag when debug info is not required, i.
> On Feb 8, 2017, at 2:31 PM, David Blaikie wrote:
>
>
>
> On Wed, Feb 8, 2017 at 2:25 PM Amjad Aboud via Phabricator
> mailto:revi...@reviews.llvm.org>> wrote:
> aaboud added a comment.
>
> > How much does the build directory grow?
> > Is there any noticeable compile time regression?
>
>
On Wed, Feb 8, 2017 at 2:25 PM Amjad Aboud via Phabricator <
revi...@reviews.llvm.org> wrote:
> aaboud added a comment.
>
> > How much does the build directory grow?
> > Is there any noticeable compile time regression?
>
> I build clang in release mode using GCC, then used that build to build
> c
aaboud added a comment.
> How much does the build directory grow?
> Is there any noticeable compile time regression?
I build clang in release mode using GCC, then used that build to build clang in
debug mode with "-fstandalone-debug" flag, one time with my changes and another
time without my c
On Tue, Feb 7, 2017 at 11:01 AM Amjad Aboud via Phabricator <
revi...@reviews.llvm.org> wrote:
> aaboud added a comment.
>
> In https://reviews.llvm.org/D16135#669416, @aprantl wrote:
>
> > In https://reviews.llvm.org/D16135#669045, @aaboud wrote:
> >
> > > Addressed Adrian last comments.
> > > A
aaboud added a comment.
In https://reviews.llvm.org/D16135#669416, @aprantl wrote:
> In https://reviews.llvm.org/D16135#669045, @aaboud wrote:
>
> > Addressed Adrian last comments.
> > Added a LIT tests that covers all the macro kinds:
> >
> > 1. built-in (define)
> > 2. command-line (define, in
aprantl added a comment.
In https://reviews.llvm.org/D16135#669045, @aaboud wrote:
> Addressed Adrian last comments.
> Added a LIT tests that covers all the macro kinds:
>
> 1. built-in (define)
> 2. command-line (define, include, undef)
> 3. main source (define, include, undef) Checked the abov
aaboud updated this revision to Diff 87408.
aaboud marked 2 inline comments as done.
aaboud added a comment.
Addressed Adrian last comments.
Added a LIT tests that covers all the macro kinds:
1. built-in (define)
2. command-line (define, include, undef)
3. main source (define, include, undef)
Ch
aprantl added a comment.
Few more inline comments.
I think the code itself is looking good now. Why doesn't this patch include any
test cases? I would have expected some that exercise the command line include
handling, etc., ...
Comment at: lib/CodeGen/MacroPPCallbacks.h:35
+
aaboud updated this revision to Diff 87020.
aaboud marked 4 inline comments as done.
aaboud added a comment.
Addressed Adrian's comments.
Please, let me know if I missed any.
Also, I improved the code and added explanation about the file inclusion
structure that is being handled in the macro cal
aaboud added a comment.
Thanks Adrian for the fast response.
See some answers below.
I will update the code and upload a new patch soon.
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:154
+StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+ // Only car
aprantl added a comment.
Thanks! Couple more inline comments.
Comment at: lib/CodeGen/CGDebugInfo.h:415
+ /// Get macro debug info.
+ llvm::DIMacro *CreateMacro(llvm::DIMacroFile *Parent, unsigned MType,
It would be good to be a bit more descriptive here. I
aaboud updated this revision to Diff 86669.
aaboud marked an inline comment as done.
aaboud added a comment.
Address Adrian and Richard comments.
Please, review the new patch and let me know if I need to change anything else.
Thanks,
Amjad
https://reviews.llvm.org/D16135
Files:
include/clan
aaboud marked 7 inline comments as done.
aaboud added a comment.
Thanks Adrian and Richard for the comments, I appreciate that.
I am uploading a new patch that address most of the comments.
Adrian, I answered some of your comments below.
Thanks,
Amjad
Comment at: lib/CodeGen/
rsmith added inline comments.
Comment at: include/clang/CodeGen/ModuleBuilder.h:71
+ ///
+ /// This methods can be called before initializing the CGDebugInfo calss.
+ /// But the returned value should not be used until after initialization.
Typo 'calss'
aprantl added inline comments.
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:62
+: DebugInfo(DI), PP(PP), FirstInclude(false), FirstIncludeDone(false),
+ CommandIncludeFiles(0), SkipFiles(2) {
+ Parents.push_back(nullptr);
aprantl wrote:
> Comment why Sk
mkuper resigned from this revision.
mkuper added a comment.
I admit it would be nice if someone reviewed this, but I'm really the wrong
person for this code area. :-)
https://reviews.llvm.org/D16135
___
cfe-commits mailing list
cfe-commits@lists.ll
aaboud added a comment.
Kindly reminder, I am still waiting for a review on this new patch.
Thanks
https://reviews.llvm.org/D16135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaboud added a comment.
Richard, can you please review the new patch?
https://reviews.llvm.org/D16135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaboud updated this revision to Diff 82696.
aaboud added a comment.
Herald added subscribers: mgorny, nemanjai.
Improved code based on Richard's comments.
Removed the change to ASTConsumer, now it does not need to know anything about
PP consumer.
However, I still needed to change the CodeGenerato
aaboud added a comment.
Thanks Richard for reviewing the patch and for the comments.
Please, see answers below.
Regards,
Amjad
Comment at: include/clang/AST/ASTConsumer.h:163
+ /// The caller takes ownership on the returned pointer.
+ virtual std::unique_ptr
CreatePreproces
rsmith added inline comments.
Comment at: include/clang/AST/ASTConsumer.h:163
+ /// The caller takes ownership on the returned pointer.
+ virtual std::unique_ptr
CreatePreprocessorCallbacks(Preprocessor &PP);
};
aaboud wrote:
> Richard,
> I know that you sugg
aaboud added reviewers: erichkeane, bwyma.
aaboud updated this revision to Diff 65551.
aaboud added a comment.
Updated patch to top of trunk (r276746) - Thanks to Ranjeet Singh.
Please, provide your feedback.
https://reviews.llvm.org/D16135
Files:
include/clang/AST/ASTConsumer.h
lib/AST/AS
; From: Ranjeet Singh [mailto:ranjeet.si...@arm.com]
> Sent: Friday, July 22, 2016 20:16
> To: Aboud, Amjad ; rich...@metafoo.co.uk;
> paul.robin...@sony.com; apra...@apple.com
> Cc: ranjeet.si...@arm.com; cfe-commits@lists.llvm.org
> Subject: Re: [PATCH] D16135: Macro Debug Info sup
rs added a subscriber: rs.
rs added a comment.
Hi Amjad,
are you still planning on getting this patch and
https://reviews.llvm.org/D16077 committed ? It looks like these two patches are
final pieces in the puzzle to get macro information in the DWARF debug output.
I've downloaded the diffs and
aaboud updated this revision to Diff 47997.
aaboud marked an inline comment as done.
aaboud added a comment.
Applied Adrian comment.
http://reviews.llvm.org/D16135
Files:
include/clang/AST/ASTConsumer.h
lib/AST/ASTConsumer.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CGDebugInfo.h
lib/
aprantl added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:2099
@@ -2098,1 +2098,3 @@
+llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent, bool
IsUndef,
+SourceLocation LineLoc, StringRef Name,
aaboud updated this revision to Diff 46034.
aaboud marked 6 inline comments as done.
aaboud added a comment.
Added comments explaining the implementation.
http://reviews.llvm.org/D16135
Files:
include/clang/AST/ASTConsumer.h
lib/AST/ASTConsumer.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGe
aprantl added a comment.
Added a few stylistic comments and request for more documentation.
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:1
@@ +1,2 @@
+//===--- MacroPPCallbacks.h ---*- C++
-*-===//
+//
Please run your patch t
aaboud added inline comments.
Comment at: include/clang/AST/ASTConsumer.h:163
@@ -155,1 +162,3 @@
+ /// The caller takes ownership on the returned pointer.
+ virtual std::unique_ptr
CreatePreprocessorCallbacks(Preprocessor &PP);
};
Richard,
I know that you sug
aaboud created this revision.
aaboud added reviewers: dblaikie, rsmith, aprantl, probinson.
aaboud added a subscriber: cfe-commits.
Added support for FE Clang to create Debug Info entries (DIMacro and
DIMacroFile) into generated module if "debug-info-kind" flag is set to
"standalone".
http://re
38 matches
Mail list logo
