[PATCH] D120781: [IRLinker] materialize Functions before moving any

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D120781#3381029 <https://reviews.llvm.org/D120781#3381029>, @nickdesaulniers wrote:

> One potential issue, consider the following case:
>
>   declare void @f(i8*)
>   
>   define void @x() {
>     br label %label
>   
>   label:
>     ret void
>   }
>   
>   define linkonce_odr void @y() {
>     br label %label
>   
>   label:
>     call void @f(i8* blockaddress(@x, %label))
>     ret void
>   }
>
> So `@y` contains a `blockaddress` that refers to `@x`, but because of its linkage AND lack of reference from any function w/ external linkage, `@y` is never emitted (let alone materialized).  I think in this case, if we were to record that `@x` had a "backedge" from `@y`, and thus materialized `@y` as a result of materializing `@x`, we'd wind up "over-materializing" again.  This is because `BitcodeReader` doesn't know about `IRMover::Worklist` and the complex linkage type handling, and lazy additions to the worklist.

I think over-materializing here is probably okay, as a penalty for `@x` having exposed an internal pointer via `blockaddress`.

Given that `blockaddress` is a rarely used feature, I think the goals for this patch should be (in rough priority order):

1. Avoid compile-time regressions when not using `blockaddress`. Limit any compile-time regressions to IR intrinsically tied to the feature.
2. Get `blockaddress` working for these (new) cases.
3. Avoid unnecessary compile-time regressions for IR intrinsically tied to the feature.

I think over-materializing `@x` falls into category (3).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120781