yaxunl added a comment. In https://reviews.llvm.org/D45900#1093160, @rjmccall wrote:
> In https://reviews.llvm.org/D45900#1093154, @yaxunl wrote: > > > In https://reviews.llvm.org/D45900#1083377, @rjmccall wrote: > > > > > Oh, I see, it's not that the lifetime intrinsics don't handle pointers in > > > the alloca address space, it's that we might have already promoted them > > > into `DefaultAS`. > > > > > > Do the LLVM uses of lifetime intrinsics actually look through these > > > address space casts? I'm wondering if we might need to change how we > > > emit the intrinsics so that they're emitted directly on (bitcasts of) the > > > underlying allocas. > > > > > > Some passes do not look through address space casts. Although there is > > InferAddressSpace pass which can eliminate the redundant address space > > casts, still it is desirable not to emit redundant address space in Clang. > > > > To avoid increasing complexity of alloca emitting API, I think we need a > > way to track the original alloca and the alloca casted to default address > > space. I can think of two ways: > > > > 1. add OriginalPointer member to Address, which is the originally emitted > > LLVM value for the variable. Whenever we pass the address of a variable we > > also pass the original LLVM value. > > 2. add a map to CodeGenFunction to map the casted alloca to the real alloca. > > > > Any suggestion? Thanks. > > > Can we just call CreateLifetimeStart (and push the cleanup to call > CreateLifetimeEnd) immediately after creating the alloca instead of waiting > until later like we do now? > > Modifying Address is not appropriate, and adding a map to CGF would be big > waste. Since CreateTempAlloca returns the casted alloca by default, whereas CreateLifetimeStart expects the original alloca. If we want to call CreateLifetimeStart with original alloca, we have to do it in CreateTempAlloca. This incurs two issues: 1. we need an enum parameter to control how lifetime.start/end is emitted. There are cases that lifetime.start is not emitted, and different ways to push cleanups for lifetime.end, e.g. in CodeGenFunction:: EmitMaterializeTemporaryExpr switch (M->getStorageDuration()) { case SD_Automatic: case SD_FullExpression: if (auto *Size = EmitLifetimeStart( CGM.getDataLayout().getTypeAllocSize(Object.getElementType()), Object.getPointer())) { if (M->getStorageDuration() == SD_Automatic) pushCleanupAfterFullExpr<CallLifetimeEnd>(NormalEHLifetimeMarker, Object, Size); else pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Object, Size); } break; 2. There are situations that the pushed cleanup for lifetime.end is deactivated and emitted early, e.g. in AggExprEmitter::withReturnValueSlot // If there's no dtor to run, the copy was the last use of our temporary. // Since we're not guaranteed to be in an ExprWithCleanups, clean up // eagerly. CGF.DeactivateCleanupBlock(LifetimeEndBlock, LifetimeStartInst); CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAddr.getPointer()); In this case there is no good way to get the LifetimeStartInst and the original alloca inst. Basically the emitting of lifetime.start/end is not uniform enough to be incorporated as part of CreateTempAlloca. How about letting CreateTempAlloca have an optional pointer argument for returning the original alloca? https://reviews.llvm.org/D45900 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
