Skip to content

Emit LLVM lifetime intrinsics #1129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ChuanqiXu9 opened this issue Nov 15, 2024 · 9 comments · May be fixed by #1554
Open

Emit LLVM lifetime intrinsics #1129

ChuanqiXu9 opened this issue Nov 15, 2024 · 9 comments · May be fixed by #1554
Assignees
@el-ev @Sir-NoChill
Labels
good first issue Good for newcomers

Comments

@ChuanqiXu9
Copy link
Member

For

struct S {
    int a;
    int b;
};

S getS();

S getS2(const S&);

S foo() {
    return getS2(getS());
}

Compile it with -fclangir -O3 -emit-cir, we got:

cir.func @_Z3foov() -> !ty_S extra(#fn_attr) {
    %0 = cir.alloca !ty_S, !cir.ptr<!ty_S>, ["__retval"] {alignment = 4 : i64} loc(#loc6)
    cir.scope {
      %2 = cir.alloca !ty_S, !cir.ptr<!ty_S>, ["ref.tmp0"] {alignment = 4 : i64} loc(#loc16)
      %3 = cir.call @_Z4getSv() : () -> !ty_S loc(#loc9)
      cir.store %3, %2 : !ty_S, !cir.ptr<!ty_S> loc(#loc9)
      %4 = cir.call @_Z5getS2RK1S(%2) : (!cir.ptr<!ty_S>) -> !ty_S loc(#loc7)
      cir.store %4, %0 : !ty_S, !cir.ptr<!ty_S> loc(#loc7)
    } loc(#loc15)
    %1 = cir.load %0 : !cir.ptr<!ty_S>, !ty_S loc(#loc17)
    cir.return %1 : !ty_S loc(#loc17)
  }

This is good. We explicitly marked the scope for the temporary. However, when we lower it to LLVM, we lost the lifetime informations:

define dso_local %struct.S @_Z3foov() #0 !dbg !7 {
  %1 = alloca %struct.S, i64 1, align 4, !dbg !8
  %2 = alloca %struct.S, i64 1, align 4, !dbg !9
  br label %3, !dbg !8

3:                                                ; preds = %0
  %4 = call %struct.S @_Z4getSv(), !dbg !10
  store %struct.S %4, ptr %1, align 4, !dbg !10
  %5 = call %struct.S @_Z5getS2RK1S(ptr %1), !dbg !11
  store %struct.S %5, ptr %2, align 4, !dbg !11
  br label %6, !dbg !12

6:                                                ; preds = %3
  %7 = load %struct.S, ptr %2, align 4, !dbg !8
  ret %struct.S %7, !dbg !8
}

In the contrary, the emitted LLVM without clangir is:

define dso_local i64 @_Z3foov() #0 {
entry:
  %retval = alloca %struct.S, align 4
  %ref.tmp = alloca %struct.S, align 4
  call void @llvm.lifetime.start.p0(i64 8, ptr %ref.tmp) #3
  %call = call i64 @_Z4getSv()
  store i64 %call, ptr %ref.tmp, align 4
  %call1 = call i64 @_Z5getS2RK1S(ptr noundef nonnull align 4 dereferenceable(8) %ref.tmp)
  store i64 %call1, ptr %retval, align 4
  call void @llvm.lifetime.end.p0(i64 8, ptr %ref.tmp) #3
  %0 = load i64, ptr %retval, align 4
  ret i64 %0
}

where we can see the lifetime markers pretty clearly. The lifetime markers are pretty important for optimizations in the middle end.

We need to do this especially we've already marked the scope clearly.
@bcardosolopes
Copy link
Member

Yes, I'm not sure we have issues tracking this already, but we need to emit lifetime intrinsics, good thing is that we can do that as we unwrap scopes.

@bcardosolopes bcardosolopes changed the title We lack lifetime informations Emit LLVM lifetime intrinsics Nov 22, 2024
@bcardosolopes bcardosolopes added the good first issue Good for newcomers label Nov 22, 2024
@Sir-NoChill
Copy link

I'm interested in taking a crack at this one, I will likely need some help getting started. I'll keep my updates on this issue.

@Lancern Lancern mentioned this issue Dec 4, 2024
Merged
@Sir-NoChill
Copy link

Current plan is to create an op (cir::LifetimeOp is my tentative name) that I will emit when we create a temporary within a scope and then when we unroll the scope we can handle the op as required.

@bcardosolopes
Copy link
Member

@Sir-NoChill sounds good, keep in mind we need one for start and another for end. Those can probably be an attribute!

@Sir-NoChill
Copy link

Should these instructions also be associated in some way with the temporary they refer to? Or is it implicit that they are associated with the next instruction?

@bcardosolopes
Copy link
Member

As you add the tentative cir::LifetimeOp you should also add the LLVM lowering bits, that will inform you what information you should not miss to add to the operation - the LLVM intrinsics require a size and a pointer, you might be able to derive the size from the original alloca type, but you need to pass in the alloca address. Implicit isn't great because other optimizations can move things around.

@el-ev
Copy link
Member

el-ev commented Mar 23, 2025

@Sir-NoChill Are you still working on this?

@Sir-NoChill
Copy link

@el-ev yes, I'm just having difficulty testing... I can push what I have for the op to a branch tomorrow if you want to take a look

@Sir-NoChill
Copy link

@el-ev The lowering is really wreaking havoc on me, so I think if you still wanted this issue you could take it. I think I need to go practice my lowering passes 🤦

@el-ev el-ev linked a pull request Apr 9, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@el-ev el-ev

@Sir-NoChill Sir-NoChill

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[CIR][CIRGen][LowerToLLVM] Emit LLVM lifetime intrinsics el-ev/clangir
4 participants
@bcardosolopes @el-ev @ChuanqiXu9 @Sir-NoChill