[PATCH] D139295: [Coroutines] Don't mark the parameter attribute of resume function as noalias

Chuanqi Xu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 5 23:25:07 PST 2022


ChuanqiXu added a comment.

In D139295#3973246 <https://reviews.llvm.org/D139295#3973246>, @nikic wrote:

> @ChuanqiXu It's fine to access both the "plain" noalias argument and a GEP of the noalias argument. What is forbidden is to access the object the noalias argument points to through some other pointer that is unrelated to the argument. It's not really clear to me whether this is the case here, would it be possible to provide the IR directly after the CoroSplit pass?

Oh, so I got things wrong. Here is the IR after the CoroSplit pass for test() (and some other optimization):

  define internal fastcc void @_ZL4testv.resume(ptr noalias nonnull align 8 dereferenceable(80) %0) #3 personality ptr @__gxx_personality_v0 {
  resume.entry:
    %index.addr = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 4
    %index = load i2, ptr %index.addr, align 8
    %switch = icmp eq i2 %index, 0
    br i1 %switch, label %AfterCoroSave, label %AfterCoroSave65
  
  AfterCoroSave:                                    ; preds = %resume.entry
    %.reload.addr = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3
    %destroy.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 8
    store ptr @_ZL1av.cleanup, ptr %destroy.addr.i, align 8
    %__promise.reload.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 16
    %index.addr32.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 32
    store i1 false, ptr %index.addr32.i, align 8
    store i2 1, ptr %index.addr, align 8
    %caller.i.i69 = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 24
    store ptr %0, ptr %caller.i.i69, align 8, !tbaa.struct !10
    tail call void @llvm.experimental.noalias.scope.decl(metadata !18)
    store i32 42, ptr %__promise.reload.addr.i, align 8, !tbaa !3, !alias.scope !18
    store ptr null, ptr %.reload.addr, align 8, !alias.scope !18
    %1 = load ptr, ptr %0, align 8
    musttail call fastcc void %1(ptr nonnull %0) #5, !noalias !18
    ret void
  
  AfterCoroSave65:                                  ; preds = %resume.entry
    %__promise.reload.addr = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2
    %__promise.reload.addr.i68 = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 16
    %2 = load i32, ptr %__promise.reload.addr.i68, align 8, !tbaa !3
    store i32 %2, ptr %__promise.reload.addr, align 8, !tbaa !3
    store ptr null, ptr %0, align 8
    %caller.i53 = getelementptr inbounds i8, ptr %0, i64 24
    %retval.sroa.0.0.copyload.i = load ptr, ptr %caller.i53, align 8, !tbaa.struct !10
    %3 = load ptr, ptr %retval.sroa.0.0.copyload.i, align 8
    musttail call fastcc void %3(ptr nonnull %retval.sroa.0.0.copyload.i) #5
    ret void
  }

And this code is actually correct. Problems happens with its caller, the main function:

  ; Function Attrs: norecurse uwtable
  define dso_local noundef i32 @main() local_unnamed_addr #1 personality ptr @__gxx_personality_v0 {
  entry:
    %0 = alloca [80 x i8], align 8
    store ptr @_ZL4testv.resume, ptr %0, align 8
    %destroy.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 1
    store ptr @_ZL4testv.cleanup, ptr %destroy.addr.i, align 8
    %__promise.reload.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2
    store i32 123, ptr %__promise.reload.addr.i, align 8, !tbaa !3
    %caller.i.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2, i32 1
    store ptr @_ZNSt7__n486114__noop_coro_frE, ptr %caller.i.i, align 8, !tbaa.struct !10
    %index.addr70.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 4
    store i2 0, ptr %index.addr70.i, align 8
    call fastcc void @_ZL4testv.resume(ptr nonnull %0)
    %.pre = load i32, ptr %__promise.reload.addr.i, align 8, !tbaa !3
    %call1 = tail call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str, i32 noundef %.pre)
    call fastcc void @_ZL4testv.cleanup(ptr nonnull %0)
    ret i32 0
  }

And after inlining `_ZL4testv.resume` into `main`, the main becomes:

  ; Function Attrs: norecurse uwtable
  define dso_local noundef i32 @main() local_unnamed_addr #1 personality ptr @__gxx_personality_v0 {
  entry:
    %0 = alloca [80 x i8], align 8
    store ptr @_ZL4testv.resume, ptr %0, align 8
    %destroy.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 1
    store ptr @_ZL4testv.cleanup, ptr %destroy.addr.i, align 8
    %__promise.reload.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2
    store i32 123, ptr %__promise.reload.addr.i, align 8, !tbaa !3
    %caller.i.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2, i32 1
    store ptr @_ZNSt7__n486114__noop_coro_frE, ptr %caller.i.i, align 8, !tbaa.struct !10
    %index.addr70.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 4
    store i2 0, ptr %index.addr70.i, align 8
    call void @llvm.experimental.noalias.scope.decl(metadata !12)
    %index.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 4
    %index.i = load i2, ptr %index.addr.i, align 8, !alias.scope !12
    %switch.i = icmp eq i2 %index.i, 0
    br i1 %switch.i, label %AfterCoroSave.i, label %AfterCoroSave65.i
  
  AfterCoroSave.i:                                  ; preds = %entry
    %.reload.addr.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3
    %destroy.addr.i.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 8
    store ptr @_ZL1av.cleanup, ptr %destroy.addr.i.i, align 8, !alias.scope !12
    %__promise.reload.addr.i.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 16
    %index.addr32.i.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 32
    store i1 false, ptr %index.addr32.i.i, align 8, !alias.scope !12
    store i2 1, ptr %index.addr.i, align 8, !alias.scope !12
    %caller.i.i69.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 24
    store ptr %0, ptr %caller.i.i69.i, align 8, !tbaa.struct !10, !alias.scope !12
    call void @llvm.experimental.noalias.scope.decl(metadata !15)
    store i32 42, ptr %__promise.reload.addr.i.i, align 8, !tbaa !3, !alias.scope !18
    store ptr null, ptr %.reload.addr.i, align 8, !alias.scope !18
    %1 = call ptr @llvm.coro.subfn.addr(ptr nonnull %0, i8 0), !alias.scope !12, !noalias !15
    call fastcc void %1(ptr nonnull %0) #6, !noalias !15
    br label %_ZL4testv.resume.exit
  
  AfterCoroSave65.i:                                ; preds = %entry
    %__promise.reload.addr.i6 = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 2
    %__promise.reload.addr.i68.i = getelementptr inbounds %_ZL4testv.Frame, ptr %0, i64 0, i32 3, i64 16
    %2 = load i32, ptr %__promise.reload.addr.i68.i, align 8, !tbaa !3, !alias.scope !12
    store i32 %2, ptr %__promise.reload.addr.i6, align 8, !tbaa !3, !alias.scope !12
    store ptr null, ptr %0, align 8, !alias.scope !12
    %caller.i53.i = getelementptr inbounds i8, ptr %0, i64 24
    %retval.sroa.0.0.copyload.i.i = load ptr, ptr %caller.i53.i, align 8, !tbaa.struct !10, !alias.scope !12
    %3 = call ptr @llvm.coro.subfn.addr(ptr %retval.sroa.0.0.copyload.i.i, i8 0), !noalias !12
    call fastcc void %3(ptr %retval.sroa.0.0.copyload.i.i) #6, !noalias !12
    br label %_ZL4testv.resume.exit
  
  _ZL4testv.resume.exit:                            ; preds = %AfterCoroSave65.i, %AfterCoroSave.i
    %.pre = load i32, ptr %__promise.reload.addr.i, align 8, !tbaa !3
    %call1 = tail call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str, i32 noundef %.pre)
    ret i32 0
  }

Note that now the `store i32 42, ptr %__promise.reload.addr.i.i` is in `!alias.scope !18`, and the call `call fastcc void %1(ptr nonnull %0)` lives in `!noalias !15`. So that the DSE think the store is dead. And in https://github.com/llvm/llvm-project/blob/3cfe412e4c90718a3c5abe0aaf2209053a490982/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1140-L1154, I see the noalias scope metadata is added if the parameter is noalias. So I thought I found the solution. And here is the patch. Remind me if you need a module level IR reproducer (since it may take some time to reduce it..)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139295/new/

https://reviews.llvm.org/D139295



More information about the llvm-commits mailing list