[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Nick Desaulniers via Phabricator via cfe-commits]

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