rjmccall added a comment. In https://reviews.llvm.org/D32210#819577, @ahatanak wrote:
> In https://reviews.llvm.org/D32210#810508, @rjmccall wrote: > > > Hmm. Unfortunately, I'm not sure that's valid. The retains and releases > > of block captures don't protect against anything related to escaping the > > block; they protect against the original variables being modified during > > the lifetime of the block. It is true that a block literal passed to a > > noescape parameter has a shorter effective lifetime — we know for certain > > that it will be unused after the call, and ARC only promises that a value > > in an imprecise-lifetime strong variable like a block capture will be valid > > until the last load of that value from the variable. But that duration is > > still long enough for someone to modify the original variable in a way that > > is properly sequenced after the formation of the block. > > > > Now, if you could prove that the variable was not modified for the duration > > of the call, that would be sufficient. And that would be easy to do in the > > common case by just proving that the address of the variable never escapes. > > Unfortunately, we don't have that information readily available because > > Sema doesn't collect it. > > > OK, so I guess the optimization isn't valid in the following case, for > example: > > void foo3(id); > > id __strong *gp; > > void foo2(id a) { > gp = &a; > nonescapingFunc(^{ foo3(a); }); // this function can modify "a" before > the block is executed. > } > Right. >> There are some other ways you could optimize blocks that are known not to >> escape, though. One big caveat is that you have to make sure the block >> behaves reasonably in response to being copied, becase being noescape >> doesn't guarantee that the callee won't try to copy the block. However: > > I didn't understand the following optimizations. I thought clang doesn't emit > copy and destroy helper for global blocks with or without noescape? They're not really about global blocks, they're about whether we can emit a non-escaping block using the isa that we use for global blocks, which causes copies to be no-ops (because real global blocks do not have local captures). John. ================ Comment at: include/clang/AST/Type.h:3156 }; unsigned char Data; ---------------- ahatanak wrote: > rjmccall wrote: > > Oh! I hadn't noticed that you were adding this to ExtParameterInfo. > > You're modifying the function type system; there's a lot of complexity to > > do that properly which you haven't done in this patch at all. That's > > especially true because, unlike all these other ExtParameterInfo cases, > > there's a natural subtyping rule for escaping parameters: a function type > > that takes a non-escaping parameter should be implicitly convertible to be > > a function type that takes an escaping parameter. You will also need to > > update a bunch of things, including the type printer, the mangler, the C++ > > function conversion rules, the C type compatibility rules, the > > redeclaration type matcher, and maybe even template argument deduction. > > It's a big thing to do. > > > > You will want Sema tests in C, ObjC, C++, and ObjC++. > Rather than adding an enum to ExtParameterInfo and modifying the type system, > I made changes in IRGen to find out whether a function parameter is annotated > with noescape. I'm not sure when it is OK or not OK to change the type system > when adding support for an attribute, but this seemed to be the right > direction since most of the other attributes are handled this way. So, I do think it's acceptable to change the type system here; it's just that doing so definitely makes the change more complicated. The guidance here is: - If it changes the basic rules for making a call, it really *must* be preserved as part of the function type, or else basic things like taking the address of the function will never work correctly. That's why everything currently in ExtParameterInfo is there — ns_consumed affects the balancing rules for how ARC makes calls, the Swift parameter-ABI treatments are part of the CC, etc. - Otherwise, it really comes down to how much we care about the feature working with indirection. I feel like blocks are probably the strongest motivator here because they're always invoked indirectly: if you want the feature to work on a block parameter, it really needs to be part of the function type, because we do not want to get into the business of finding non-canonical sources of information for function types, e.g. param decls from the TypeSourceInfo. Overall, I would say that it tends to be the case that we eventually find attribute-based analyses limiting. If we start doing serious optimizations based on noescape, it is probably something that ought to go in the type system. ================ Comment at: lib/CodeGen/CGCall.cpp:2096 + if (FI.getExtParameterInfo(ArgNo).isNoEscape()) + Attrs.addAttribute(llvm::Attribute::NoCapture); + ---------------- ahatanak wrote: > rjmccall wrote: > > You should make sure that building a CGFunctionInfo with an ObjCMethodDecl* > > collects this from the parameters. (And add tests!) > > > > For blocks, at least, the ObjC method case is arguably more important than > > the C function case. > Should we issue a warning when a parameter of an ObjC method is annotated > with noescape and the corresponding parameter of the overriding method in a > derived class isn't? Also, should we warn about inconsistent C++ virtual > functions too? Mmm. Probably yes in both cases, although we could alternatively just silently propagate the attribute InheritableParamAttr-style. ================ Comment at: lib/CodeGen/CGClass.cpp:2086 CallArg ThisArg(RValue::get(This.getPointer()), D->getThisType(getContext()), - /*NeedsCopy=*/false); + /*NeedsCopy=*/false, /*IsNoEscapse*/false); ---------------- Typo. https://reviews.llvm.org/D32210 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
