[PATCH] D127202: [InlineFunction] Handle early exit during getUnderlyingObjects due to MaxLookup

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/Coroutines/coro-retcon-opaque-ptr.ll:38
-; CHECK-NEXT:    call void @print(i32 5), !noalias !0
-; CHECK-NEXT:    call void @print(i32 6), !noalias !3
 ; CHECK-NEXT:    ret i32 0
----------------
shchenz wrote:
> nikic wrote:
> > shchenz wrote:
> > > nikic wrote:
> > > > Do you know why these changes happen? (Just wondering, it looks harmless.)
> > > The `!noalias` metadata is added when do inlining:
> > > 
> > > ```
> > > define i32 @main() local_unnamed_addr {
> > >   %cont1 = call i8* @f.resume.0(i8* nonnull %.sub, i1 zeroext false)
> > > }
> > > ```
> > > 
> > > ```
> > > define internal i8* @f.resume.0(i8* noalias nocapture nonnull align 4 dereferenceable(8) %0, i1 zeroext %1) {
> > > 
> > >   %n.val.reload.addr = bitcast i8* %0 to i32*
> > >   %n.val.reload = load i32, i32* %n.val.reload.addr, align 4
> > >   %inc = add i32 %n.val.reload, 1
> > >   store i32 %inc, i32* %n.val.reload.addr, align 4
> > > 
> > >   tail call void @print(i32 %inc)
> > > }
> > > ```
> > > 
> > > Before the change, underlying object `%inc = add i32 %n.val.reload, 1` for `tail call void @print(i32 %inc)` parameter is `RequiresNoCaptureBefore` and `PointerMayBeCapturedBefore()` returns false, so there will be `noalias` metadata added for the function call.
> > > 
> > > But after the change, underlying object `%inc = add i32 %n.val.reload, 1` is treated as a `UsesUnknownObject`.
> > > 
> > > Can the `add` (one operand is a load an the other is a constantInt) be handled in the `isEscapeSource()`?
> > An `add` should never be an underlying object, because it's not a pointer.
> > 
> > I believe it gets added due to this code: https://github.com/llvm/llvm-project/blob/9081d3d8097af5e89905faf0ed568ba7e848e8df/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1046-L1052 I don't think the comment there is correct, and we should drop the `IsArgMemOnlyCall &&` check. If a pointer is passed as an integer, then the ptrtoint will already capture it. And https://github.com/llvm/llvm-project/blob/9081d3d8097af5e89905faf0ed568ba7e848e8df/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1111-L1114 will always check for captures for non-argmemonly function calls.
> Yes, should I posted another patch to fix this issue? I think this fix should not reply on this patch, right? We can just ignore the non-pointer arguments in the PtrArgs collection phase and `PointerMayBeCapturedBefore()` will always check captures for the parameter even it is not a pointer for a non-argmemonly function calls.
Yeah, sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127202