Re: [PATCH] D15998: Implement __attribute__((gc_leaf_function)).
mjacob abandoned this revision. mjacob added a comment. I experimented with another approach in the meantime. http://reviews.llvm.org/D15998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15998: Implement __attribute__((gc_leaf_function)).
mjacob added a comment. In http://reviews.llvm.org/D15998#324354, @reames wrote: > Also, before this gets exposed through Clang, we really should > formalize/document the attribute. In practice, it implies the lack of a > safepoint poll site inside the called function. Annoyingly, it's not an > inferable property since we don't represent the possible insertion of a poll > in the IR. That wouldn't work for my language since it doesn't use polls at all. Instead, a practical definition would be: "a statepoint will not be inserted if the call site or callee has this attribute". My language backend currently relies on this, e.g. if calling printf (currently statepoints don't support vararg callees). > Hm. This makes me wonder... We've moved to a model of inserting safepoints > (specifically for deopt info) early, and then rewriting late in our tree. > We're not even using the PlaceSafepoints pass any more. It's been left > mostly for other users. Would it maybe make sense to fully retire > PlaceSafepoints and migrate over to a scheme where safepoints sites are > explicit from the beginning? This would allow us to infer "gc-leaf" from > FunctionAttrs... With explicit safepoints and the above-mentioned definition the gc-leaf-funtion attribute wouldn't be necessary at all, because then a function is a GC leaf if and only if no safepoint should be inserted when calling this function. > (This high level discussion should probably move to llvm-dev. I can start it > if you'd like, otherwise post something and I'll reply.) I think it makes more sense if you start the discussion because I'm not familiar with your use cases (I only use a subset of safepoint functionality, e.g. no safepoint polls). http://reviews.llvm.org/D15998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15998: Implement __attribute__((gc_leaf_function)).
mjacob added a comment.
In http://reviews.llvm.org/D15998#324757, @aaron.ballman wrote:
> Can you point me to some documentation on what the semantics of this
> attribute are? For instance, how does it play with other attributes (like
> naked or dllexport), is there a reason it shouldn't apply to Objective-C
> methods, etc?
As it was noted earlier in this review, this attribute (as its underlying LLVM
attribute) is underspecified. We should discuss the semantics (and whether we
want to keep it in the first place) of the LLVM attribute on the mailing list.
I'm not sure how we should proceed with this patch in the meantime, probably
one of:
1. Mark this (Clang) attribute as tentative, and remove it in case we remove
the LLVM attribute.
2. Close this revision and create a new patch depending on the outcome of the
discussion.
3. Postpone (but not close) this revision.
Comment at: include/clang/Basic/Attr.td:2175
@@ +2174,3 @@
+def GcLeafFunction : InheritableAttr {
+ let Spellings = [GNU<"gc_leaf_function">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> Any particular reason for this to not have a C++11 spelling under the clang
> namespace, in addition to the GNU-style spelling?
I'll add this in case we decide the attribute is the right approach.
http://reviews.llvm.org/D15998
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15169: [SemaCXX] Fix handling of C-style casts from void pointers to pointers in different address space.
mjacob added inline comments. Comment at: lib/Sema/SemaCast.cpp:1081-1083 @@ -1080,3 +1080,5 @@ } - Kind = CK_BitCast; + unsigned SrcAS = SrcPointee.getAddressSpace(); + unsigned DestAS = DestPointee.getAddressSpace(); + Kind = SrcAS != DestAS ? CK_AddressSpaceConversion : CK_BitCast; return TC_Success; rsmith wrote: > I would expect to get both a `CK_BitCast` *and* a `CK_AddressSpaceCast` > created for this case. Note that by not returning `CK_BitCast`, you lose the > `checkCastAlign` call in the caller. > > Instead, can you push this check down into wherever we're building the > conversion, so that if we try to create a `CK_BitCast` that crosses address > spaces, we additionally create a `CK_AddressSpaceCast`? I'm not sure what you mean by "**additionally** create a `CK_AddressSpaceCast`". Please clarify. Would it also be possible to extend all places which call `checkCastAlign()` to also call it for `CK_AddressSpaceCast`? http://reviews.llvm.org/D15169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15319: [Sema] Don't accept e.g. "(int *)(long)&x" as a constant expression if x is in address space != 0.
mjacob created this revision.
mjacob added a reviewer: rsmith.
mjacob added subscribers: cfe-commits, alex.
The constant expression evaluator for pointers returns a base and an
offset if successful. In this case, CodeGen generates a constant pointer cast,
casting the base (with offset applied if necessary) to the destination type.
However, that is different from casting the base to an integer and then to the
destination type if the destination points to another address space as the
base. Supporting this would require a lot of work because it can't be
expressed as base + offset. Changing the whole constant expression evaluator
is not worthline for supporting this special case.
http://reviews.llvm.org/D15319
Files:
lib/AST/ExprConstant.cpp
test/Sema/const-eval.c
Index: test/Sema/const-eval.c
===
--- test/Sema/const-eval.c
+++ test/Sema/const-eval.c
@@ -137,3 +137,6 @@
void PR24622();
struct PR24622 {} pr24622;
EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{must have a
constant size}}
+
+int __attribute__((address_space(1))) addrspace1;
+int *addrspace0_ptr = (int *)(long)&addrspace1; // expected-error {{not a
compile-time constant}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4995,7 +4995,11 @@
Result.Designator.setInvalid();
return true;
} else {
- // Cast is of an lvalue, no need to change value.
+ // Cast is of an lvalue, no need to change value, unless they have
+ // different address spaces.
+ if (Info.Ctx.getTargetAddressSpace(getType(Value.getLValueBase())) !=
+ Info.Ctx.getTargetAddressSpace(E->getType()->getPointeeType()))
+return false;
Result.setFrom(Info.Ctx, Value);
return true;
}
Index: test/Sema/const-eval.c
===
--- test/Sema/const-eval.c
+++ test/Sema/const-eval.c
@@ -137,3 +137,6 @@
void PR24622();
struct PR24622 {} pr24622;
EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{must have a constant size}}
+
+int __attribute__((address_space(1))) addrspace1;
+int *addrspace0_ptr = (int *)(long)&addrspace1; // expected-error {{not a compile-time constant}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4995,7 +4995,11 @@
Result.Designator.setInvalid();
return true;
} else {
- // Cast is of an lvalue, no need to change value.
+ // Cast is of an lvalue, no need to change value, unless they have
+ // different address spaces.
+ if (Info.Ctx.getTargetAddressSpace(getType(Value.getLValueBase())) !=
+ Info.Ctx.getTargetAddressSpace(E->getType()->getPointeeType()))
+return false;
Result.setFrom(Info.Ctx, Value);
return true;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15319: [Sema] Don't accept e.g. "(int *)(long)&x" as a constant expression if x is in address space != 0.
mjacob added a comment. ping http://reviews.llvm.org/D15319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
