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

[ChenZheng via Phabricator via llvm-commits]

shchenz marked an inline comment as done.
shchenz added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1115
+          // enough to get the real underlying object.
+          if (!isIdentifiedObject(V))
+            MayAliasForUnidentify = true;
----------------
nikic wrote:
> shchenz wrote:
> > nikic wrote:
> > > This looks overly aggressive to me, and probably loses most of the point of the PointerMayBeCapturedBefore check. We should be classifying objects into isIdentifiedObject and isEscapeSource, where we can use the capture-based logic for isEscapeSource. (It's currently static in BasicAliasAnalysis, but can be exported.)
> > Thanks. Updated.
> I think the basic logic here is now right. I still find the way the flags are set somewhat confusing though, especially that CanDeriveViaCapture is always set, even for non-escape-sources.
> 
> I think something like this would make the different cases clearer: https://gist.github.com/nikic/d900c8b2e901fc9c3ab1eb30650af9c3
Haha, thanks very much. I just uploaded your patch shameless. : )


================
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
----------------
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()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127202