nickdesaulniers added a comment.
In D74094#4634966 <https://reviews.llvm.org/D74094#4634966>, @rjmccall wrote:
> Parameter objects are not temporaries and have their own lifetime rules, so
> there's nothing wrong with this idea in principle. This seems to just be a
> bug, probably that we're doing a type check on `E->getType()` without
> considering whether E might be a gl-value. We definitely do not want to be
> emitting lifetime intrinsics for the referent of a reference argument just
> because the underlying type is an aggregate.
I think the issue is more so about returning a reference to one of the
parameters; example:
struct foo { int x; };
// Returns a reference to the minimum foo.
foo &min(foo a, foo b);
void consume (foo&);
void bar () {
foo a, b;
consume(min(a, b));
}
With the current revision, we emit:
define dso_local void @_Z3barv() #0 {
entry:
%a = alloca %struct.foo, align 4
%b = alloca %struct.foo, align 4
%agg.tmp = alloca %struct.foo, align 4
%agg.tmp1 = alloca %struct.foo, align 4
call void @llvm.lifetime.start.p0(i64 4, ptr %a) #4
call void @llvm.lifetime.start.p0(i64 4, ptr %b) #4
call void @llvm.lifetime.start.p0(i64 4, ptr %agg.tmp) #4
call void @llvm.memcpy.p0.p0.i64(ptr align 4 %agg.tmp, ptr align 4 %a, i64
4, i1 false), !tbaa.struct !5
call void @llvm.lifetime.start.p0(i64 4, ptr %agg.tmp1) #4
call void @llvm.memcpy.p0.p0.i64(ptr align 4 %agg.tmp1, ptr align 4 %b, i64
4, i1 false), !tbaa.struct !5
%coerce.dive = getelementptr inbounds %struct.foo, ptr %agg.tmp, i32 0, i32 0
%0 = load i32, ptr %coerce.dive, align 4
%coerce.dive2 = getelementptr inbounds %struct.foo, ptr %agg.tmp1, i32 0,
i32 0
%1 = load i32, ptr %coerce.dive2, align 4
%call = call noundef nonnull align 4 dereferenceable(4) ptr
@_Z3min3fooS_(i32 %0, i32 %1)
call void @llvm.lifetime.end.p0(i64 4, ptr %agg.tmp) #4
call void @llvm.lifetime.end.p0(i64 4, ptr %agg.tmp1) #4
call void @_Z7consumeR3foo(ptr noundef nonnull align 4 dereferenceable(4)
%call)
call void @llvm.lifetime.end.p0(i64 4, ptr %b) #4
call void @llvm.lifetime.end.p0(i64 4, ptr %a) #4
ret void
}
which doesn't look correct (the lifetimes of `%agg.tmp` and `%agg.tmp1` need to
last past the call to `consume`, IIUC).
That makes me think we should probably do this lifetime annotation in the
caller of `CodeGenFunction::EmitCallArg` since it will have information about
the return type of the caller. If a function returns a reference to a type, we
probably don't want to emit the lifetime markers for any parameter of that
type. I wonder if strict aliasing needs to be considered, too?
(or am I chasing the wrong thing here?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74094/new/
https://reviews.llvm.org/D74094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
