ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1643
+if (Ctx.getBlockVarCopyInits(VD))
+ return true;
+ return false;
rjmccall wrote:
> Can you just ask Sema to check `canThrow` for the expression and pass it down?
Since this chang
ahatanak updated this revision to Diff 159388.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBlocks.h
lib/CodeGen/CGDecl.cpp
lib/CodeGen/
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1643
+if (Ctx.getBlockVarCopyInits(VD))
+ return true;
+ return false;
ahatanak wrote:
> rjmccall wrote:
> > Can you just ask Sema to check `canThrow` for the expression and pass it
>
ahatanak updated this revision to Diff 159546.
ahatanak marked an inline comment as done.
ahatanak added a reviewer: rsmith.
ahatanak added a comment.
Herald added a subscriber: jfb.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
include/clang/AST/Comp
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1643
+if (Ctx.getBlockVarCopyInits(VD))
+ return true;
+ return false;
rjmccall wrote:
> ahatanak wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > Can you just ask Sema to chec
ahatanak updated this revision to Diff 159553.
ahatanak added a comment.
Remove a stale comment and add an assertion to check the destructor's exception
specification has been resolved.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
include/clang/AST/ComputeExceptionSpec.h
ahatanak added a comment.
I thought it was okay to skip the work done by `ResolveExceptionSpec` in IRGen
as long as the exception specifications that are needed have already been
resolved in Sema. But calling Sema::canThrow in
Sema::CheckCompleteVariableDeclaration and storing the result in
Bl
ahatanak added a comment.
Since BlockVarCopyInits is a map with key `VarDecl *`, I think we want to add a
flag to VarDecl (NonParmVarDeclBits) that indicates whether the copy expression
can throw or not. Or is there a reason to store the bit in `BlockDecl::Capture`
instead?
Repository:
rC C
ahatanak updated this revision to Diff 159762.
ahatanak added a comment.
Change the value type of BlockVarCopyInits to PointerIntPair.
The boolean flag indicates whether the copy expression can throw.
Serialize/deserialize the copy expression and the boolean flag and add a
regression test to te
ahatanak updated this revision to Diff 159800.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Modify getBlockVarCopyInits and setBlockVarCopyInits to get and set both the
copy expression and the boolean flag that indicates whether the expression can
throw or not.
Reposito
ahatanak added inline comments.
Comment at: include/clang/AST/ASTContext.h:158
+Expr *CopyExpr;
+bool CanThrow;
+ };
rjmccall wrote:
> Using a PointerIntPair in the implementation of this is still a reasonable
> choice. Just make it a proper abstractio
ahatanak updated this revision to Diff 159850.
ahatanak marked 5 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/CodeGen/CGBlocks.cpp
lib/
ahatanak updated this revision to Diff 159926.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/CodeGen/CGBlocks.cpp
lib/
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1682
+ if (IsCopyHelper && Ctx.getBlockVarCopyInits(Var).CanThrow)
+Name += "c";
+}
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > I don't think you nee
ahatanak added a comment.
ping
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak updated this revision to Diff 160040.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Define functions inline in the header file.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/CodeGen/
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1725
BlockFieldFlags Flags, bool EHOnly,
+ bool DisposeCannotThrow, VarDecl *Var,
CodeGenFunction &CGF) {
---
ahatanak updated this revision to Diff 160046.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Call MarkFunctionReferenced to mark the function `__builtin_operator_new` or
`__builtin_operator_delete` will call as referenced.
Repository:
rC Clang
https://reviews.llvm.org/
ahatanak updated this revision to Diff 160053.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Replace the two flags, EHOnly and DisposeCannotThrow, passed to
pushCaptureCleanup with a single flag ForCopyHelper.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339438: [CodeGen] Merge equivalent block copy/helper
functions. (authored by ahatanak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50152?vs=160053&id=160110#toc
Repository:
rC
ahatanak created this revision.
ahatanak added reviewers: rjmccall, arphaman.
Herald added a subscriber: dexonsmith.
Currently, clang generates a new block descriptor global variable for each
block literal. This patch merges block descriptors that are identical inside
and across translation unit
ahatanak added a comment.
A few points I forgot to mention:
- This optimization kicks in only in NonGC mode. I don't think we need to care
much about GC anymore, so I think that's OK.
- There is a lot of redundancy among the copy/dispose helper function strings
and the block layout string in t
ahatanak updated this revision to Diff 161137.
ahatanak added a comment.
Try harder to shorten the names of block descriptor global variables.
The strings extracted from the names of the copy and dispose helpers are merged
whenever it is possible to do so. The block layout string doesn't include
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:271-276
llvm::GlobalVariable *global =
-elements.finishAndCreateGlobal("__block_descriptor_tmp",
- CGM.getPointerAlign(),
- /*constant*/
ahatanak updated this revision to Diff 161155.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Mark the block descriptor global variable as `unnamed_addr`.
Repository:
rC Clang
https://reviews.llvm.org/D50783
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBlocks.h
l
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340041: [CodeGen] Merge identical block descriptor global
variables. (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340041: [CodeGen] Merge identical block descriptor global
variables. (authored by ahatanak, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50783
Files:
lib/CodeGen/CGBlocks.cpp
lib
ahatanak added a comment.
@tra and @rsmith: Can we move forward and fix the incorrect cuda diagnostics in
a separate patch?
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
ahatanak added a comment.
In https://reviews.llvm.org/D47757#1204561, @tra wrote:
> It's a regression. There's a decent chance it breaks someone and this patch,
> if committed by itself, will end up being rolled back.
Is the regression you are referring to about the static function case? I don
ahatanak added a comment.
The code you showed does compile with or without `-fcuda-is-device` after
applying my patch.
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
Herald added a subscriber: dexonsmith.
Currently IRGen doesn't handle variables captured by a block correctly when the
variable is captured by reference by a lambda that encloses the block.
For example, in the following code, t
ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.
Thanks. Yes, it's a typo.
Repository:
rCXX libc++
https://reviews.llvm.org/D51043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340408: [CodeGen] Look at the type of a block capture field
rather than the type (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.
ahatanak added a comment.
Richard and Doug, do you have any thoughts on John's suggestion?
Comment at: test/CodeGenObjCXX/arc-list-init-destruct.mm:1
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -std=c++1z -fobjc-arc
-fobjc-exceptions -fcxx-exceptions -fexceptions -e
ahatanak added a comment.
In https://reviews.llvm.org/D45898#1104025, @rsmith wrote:
> As it happens, the C++ committee fixed the language wording hole here very
> recently. The new rule can be found here: http://wg21.link/p0968r0#2227
> In summary: we should to consider the destructor for all
ahatanak added a comment.
In https://reviews.llvm.org/D45015#1105314, @rsmith wrote:
> Hmm, perhaps our strategy for handling aligned allocation on Darwin should be
> revisited. We shouldn't be defining `__cpp_aligned_allocation` if we believe
> it doesn't work -- that will break code that uses
ahatanak added a comment.
Herald added a subscriber: llvm-commits.
Could you rebase this patch against ToT?
Also, it's not clear to me why only four Exprs (DeclRefExpr, MemberExpr,
ObjCIvarRefExpr, and ObjCPropertyRefExpr) should have the IsTypoCorrected flag.
Can you elaborate on how you chose
ahatanak updated this revision to Diff 148334.
ahatanak added a comment.
Add a test case for the Chromium failure. Also, simplify a bit by capturing the
calling context in Sema::DeduceTemplateArguments.
Repository:
rC Clang
https://reviews.llvm.org/D36918
Files:
lib/Sema/SemaTemplateDeduc
ahatanak added inline comments.
Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18
+ NSUInteger j = 0;
+ NSLog(@"max NSInteger = %zi", i); // CHECK: values of type 'NSInteger'
should not be used as format arguments; add an explicit cast to 'long' instead
+ NSLog(@"max
ahatanak created this revision.
ahatanak added reviewers: rjmccall, t.p.northover.
There is a bug in IRGen where the calling convention of initialization
functions for thread-local static members of c++ template classes isn't set.
This caused InstCombine to remove a call to an initialization fun
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333447: [CodeGen][Darwin] Set the calling-convention of
thread-local variable (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llv
ahatanak added a comment.
ping.
Any comments on this patch or alternate approaches?
https://reviews.llvm.org/D36915
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak added a comment.
ping
https://reviews.llvm.org/D36918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak created this revision.
We found that the IR IRGen emits when expanding __builtin_os_log_format is
quite big and has a lot of redundancy.
For example, when clang compiles the following code:
void foo1(void *buf) {
__builtin_os_log_format(buf, "%d %d", 11, 22);
}
The IR looks li
ahatanak updated this revision to Diff 117955.
ahatanak added a comment.
Always mark the helper function as linkonce_odr and hidden.
https://reviews.llvm.org/D38606
Files:
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGen/builtins.c
test/CodeGenObjC/os_log.m
Index: t
ahatanak added a comment.
I think you are right. The linkage should be linkonce_odr even when not
compiling with -Oz. I've fixed it in the latest patch.
https://reviews.llvm.org/D38606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315045: [CodeGen] Emit a helper function for
__builtin_os_log_format to reduce (authored by ahatanak).
Changed prior to commit:
https://reviews.llvm.org/D38606?vs=117955&id=117957#toc
Repository:
rL
ahatanak added a comment.
ping
https://reviews.llvm.org/D36918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak updated this revision to Diff 118119.
ahatanak added a comment.
Address review comments.
https://reviews.llvm.org/D36918
Files:
lib/Sema/SemaAccess.cpp
test/SemaCXX/access.cpp
Index: test/SemaCXX/access.cpp
===
--- t
ahatanak added inline comments.
Comment at: lib/Sema/SemaAccess.cpp:1798
+ EffectiveContext EC(CurScope->getEntity());
+ if (IsAccessible(*this, EC, Entity) == ::AR_accessible)
+return AR_accessible;
Rakete wrote:
> You don't need that scope resolution
ahatanak created this revision.
This is a follow-up patch to r314370.
Rather than throwing away the enclosing parentheses in maybeUndoReclaimObject,
this patch walks down the expression until an ARCReclaimReturnedObject cast is
found and removes just the cast, preserving the syntactic sugar exp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315261: [Sema][ObjC] Preserve syntactic sugar when removing
(authored by ahatanak).
Changed prior to commit:
https://reviews.llvm.org/D38659?vs=118121&id=118302#toc
Repository:
rL LLVM
https://revie
ahatanak added a comment.
In https://reviews.llvm.org/D47757#1211276, @tra wrote:
> I've confirmed that the patch does not break anything in our CUDA code, so
> it's good to go as far as CUDA is concerned.
Thanks. @rsmith, do you have any other comments about the patch?
Repository:
rC Clan
ahatanak added a comment.
ping
Repository:
rC Clang
https://reviews.llvm.org/D45898
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak added a comment.
The GNUstep documentation I found replaces '@' with '\1'. Would that fix the
problem?
https://github.com/gnustep/libobjc2/blob/master/ABIDoc/abi.tex
Repository:
rC Clang
https://reviews.llvm.org/D50783
___
cfe-commits
ahatanak updated this revision to Diff 163612.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.
Move the call that checks the element's destructor into
InitListChecker::CheckStructUnionTypes.
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
Herald added a subscriber: dexonsmith.
This patch changes the way `__block` variables that aren't captured by escaping
blocks are handled:
- Since non-escaping blocks on the stack never get copied to the heap (see
https://revi
ahatanak added inline comments.
Comment at: lib/Sema/SemaInit.cpp:1827-1829
+ if (SemaRef.DiagnoseUseOfDecl(Destructor, Loc))
+return false;
+ return true;
rsmith wrote:
> Usual Clang convention is to return `true` on error.
I also renamed the function to h
ahatanak updated this revision to Diff 163871.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Point the diagnostic at either the relevant init list element or at the close
brace.
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit.cpp
test
ahatanak added inline comments.
Comment at: include/clang/AST/Decl.h:977
+
+unsigned EscapingByref : 1;
};
rjmccall wrote:
> This doesn't actually seem to be initialized anywhere.
VarDecl's constructor in Decl.cpp initializes it to false.
```
AllBits =
ahatanak updated this revision to Diff 164069.
ahatanak marked 11 inline comments as done.
ahatanak added a comment.
Address review comments. Fix bugs in the handling of `__block` variables that
have reference types.
Repository:
rC Clang
https://reviews.llvm.org/D51564
Files:
include/clan
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341629: [Sema] Check that the destructor for each element of
class type is (authored by ahatanak, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341754: Distinguish `__block` variables that are captured by
escaping blocks (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm
ahatanak updated this revision to Diff 156332.
ahatanak added a comment.
Fix a bug where the capture cleanups weren't pushed when
BlockDecl::doesNotEscape() returns true.
Test that, when ARC is enabled, captures are retained and copied into the stack
block object and destroyed when the stack bl
ahatanak updated this revision to Diff 156404.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Add a comment that explains the meaning of BLOCK_IS_NOESCAPE to the docs.
Rename function needsCopyDispose to needsCopyDisposeHelpers.
Repository:
rC Clang
https://reviews.llvm
This revision was automatically updated to reflect the committed changes.
ahatanak marked an inline comment as done.
Closed by commit rL337580: [CodeGen][ObjC] Make copying and disposing of a
non-escaping block (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Change
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
Herald added a subscriber: dexonsmith.
Block copy/dispose helper functions currently aren’t exception-safe. When an
exception is thrown in a copy function, the function exits without destroying
or releasing C++ objects, ObjC ob
ahatanak added a comment.
Note that in order to destruct C++ objects, I'm using pushDestroy which pushes
a NormalCleanup when exception handling isn't enabled. This is different from
PushDestructorCleanup which always pushes a NormalAndEHCleanup, but I think
using pushDestroy is more correct si
ahatanak updated this revision to Diff 157095.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
I think I should be able to merge pushBlockObjectRelease and enterByrefCleanup,
but the BlockFieldFlags that are passed are different. We set
BLOCK_FIELD_
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1905
+case BlockCaptureEntityKind::None:
+ llvm_unreachable("unexpected BlockCaptureEntityKind");
}
rjmccall wrote:
> These two switches differ only in (1) what kind of cleanup the
ahatanak updated this revision to Diff 157149.
ahatanak added a comment.
Merge pushBlockObjectRelease and enterByrefCleanup.
In https://reviews.llvm.org/D49718#1174113, @rjmccall wrote:
> Heh, okay. It looks like the runtime doesn't do anything different, but I
> think it's probably more corre
ahatanak updated this revision to Diff 157373.
ahatanak added a comment.
Check if destructors are accessible from InitListChecker's constructor. Add
test cases for designated initializer and brace elision.
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit.cpp
ahatanak updated this revision to Diff 157380.
ahatanak added a comment.
Set the BLOCK_FIELD_IS_WEAK bit of the flag passed to the call to
enterByrefCleanup in EmitAutoVarCleanups and fix test case
test/CodeGenObjC/blocks.m.
Repository:
rC Clang
https://reviews.llvm.org/D49718
Files:
lib
ahatanak added a comment.
ping
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ahatanak updated this revision to Diff 157395.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Document the meaning of enterByrefCleanup's parameters.
Repository:
rC Clang
https://reviews.llvm.org/D49718
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGDecl.cpp
lib/Co
ahatanak updated this revision to Diff 157398.
ahatanak added a comment.
Add a test case for an interface conforming to a non-escaping protocol.
In https://reviews.llvm.org/D49119#1164285, @vsapsai wrote:
> Also I had a few ideas for tests when the warning isn't required and it is
> absent. But
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:2582
/// yet; if a cleanup is required for the variable itself, that needs
/// to be done externally.
+void CodeGenFunction::enterByrefCleanup(CleanupKind Kind, Address Addr,
rjmccall wrote:
>
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338041: [CodeGen][ObjC] Make block copy/dispose helper
functions exception-safe. (authored by ahatanak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49718?vs=157395&id=157514#toc
ahatanak added a comment.
Thanks, I'll update the patch and commit it today.
In https://reviews.llvm.org/D49119#1176139, @vsapsai wrote:
> In https://reviews.llvm.org/D49119#1176047, @ahatanak wrote:
>
> > In https://reviews.llvm.org/D49119#1164285, @vsapsai wrote:
> >
> > > Also I had a few ide
ahatanak updated this revision to Diff 157815.
ahatanak added a comment.
- Produce a note that tells users where the class extension conforming to the
protocol containing the non-escaping method is declared.
- Add a class method that has the same name as the instance method and check
that no spu
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338189: [Sema][ObjC] Warn when a method declared in a
protocol takes a (authored by ahatanak, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D49119
Files:
include/clang/Basic/Diagnost
ahatanak created this revision.
ahatanak added reviewers: rjmccall, arphaman.
Herald added subscribers: dexonsmith, mgrang.
Currently, clang generates different copy or dispose helper functions for each
block literal on the stack if the block has the possibility of being copied to
the heap. This
ahatanak added inline comments.
Comment at: test/CodeGenCXX/block-byref-cxx-objc.cpp:36
+// CHECK: call void @_Block_object_dispose(
+
+int testB() {
Should the second call to @_Block_object_dispose be an invoke if the destructor
can throw? The FIXME in CodeGenF
ahatanak added inline comments.
Comment at: test/CodeGenCXX/block-byref-cxx-objc.cpp:36
+// CHECK: call void @_Block_object_dispose(
+
+int testB() {
ahatanak wrote:
> Should the second call to @_Block_object_dispose be an invoke if the
> destructor can throw? T
ahatanak updated this revision to Diff 158640.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Use llvm::sort.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGNonTrivialStruct.cpp
lib/CodeGen/CodeGenFunction.cpp
ahatanak updated this revision to Diff 158774.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBlocks.h
lib/CodeGen/CGNonTrivialStruct.cpp
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1638
+switch (E.Kind) {
+case BlockCaptureEntityKind::CXXRecord: {
+ Name += "c";
rjmccall wrote:
> I forget whether this case has already filtered out trivial
> copy-constructors,
ahatanak added a comment.
I just wanted to make sure that this doesn't have the same problem as
https://reviews.llvm.org/D34556. Is that correct?
The patch was reverted in r306859. https://reviews.llvm.org/D34574#791158
explains why the approach taken in the patch was wrong.
Repository:
rCX
ahatanak updated this revision to Diff 158892.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBlocks.h
lib/CodeGen/CGNonTrivialStruct.cpp
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:492
+if (!RD->isExternallyVisible())
+ info.CapturesNonExternalType = true;
+
rjmccall wrote:
> You only need to do this if you're going to mangle the type name, i.e. if
> it's
ahatanak updated this revision to Diff 159250.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBlocks.h
lib/CodeGen/CGDecl.cpp
lib/CodeGen/
ahatanak added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1691
+
+Name += "_" + llvm::to_string(E.Capture->getOffset().getQuantity());
+ }
rjmccall wrote:
> I feel like this reads a little better if you write the quantity first. Also
> I think y
This revision was automatically updated to reflect the committed changes.
ahatanak marked 2 inline comments as done.
Closed by commit rC324269: Add support for attribute 'trivial_abi'.
(authored by ahatanak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41039?vs=129397&id=1
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324269: Add support for attribute 'trivial_abi'.
(authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41039?vs=129397&id=132
ahatanak added inline comments.
Comment at: lib/AST/ExprConstant.cpp:1165-1173
+ auto LB = Temporaries.lower_bound(Key);
+
+ // If an element with key Key is found, reset the value and return it. This
+ // can happen if Key is part of a default argument expression.
+ if (LB !
ahatanak updated this revision to Diff 133947.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address Erik's and Roman's review comments.
https://reviews.llvm.org/D42776
Files:
include/clang/AST/APValue.h
lib/AST/APValue.cpp
lib/AST/ExprConstant.cpp
test/SemaCXX/co
ahatanak added inline comments.
Comment at: lib/AST/ExprConstant.cpp:597
+}
+void clearDefaultArgNum() { CurDefaultArgNum = 0; }
+unsigned CurDefaultArgNum = 0, NextDefaultArgNum = 0;
rsmith wrote:
> This is wrong: these scopes can nest, so you can't
ahatanak added a comment.
OK, I see. It's pretty easy to come up with an example.
constexpr int foo1(int a = 12) {
return a * a;
}
constexpr int foo2(int a = foo1()) {
return a - 12;
}
https://reviews.llvm.org/D42776
___
cfe-comm
ahatanak updated this revision to Diff 133994.
ahatanak marked 8 inline comments as done.
ahatanak added a comment.
Address review comments and feedback I got from John offline.
The main changes are in CGNonTrivialStruct.cpp. I cleaned up the class
hierarchy and used variadic template functions
ahatanak added a comment.
AMDGPUAnnotateKernelFeatures::addFeatureAttributes looks for an llvm.trap
instruction and adds attribute "amdgpu-queue-ptr" to the function if it finds
one.
Other than that, it looks like your idea would work too.
Repository:
rL LLVM
https://reviews.llvm.org/D1083
1 - 100 of 1234 matches
Mail list logo
