[PATCH] D99546: [llvm][ValueMap] Share the value map between MappingContext

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

dexonsmith added a comment.

In D99546#2721573 <https://reviews.llvm.org/D99546#2721573>, @tejohnson wrote:

> I'm afraid I'm not going to be much help, at least not without more investigation. The 2 comdat tests mentioned above (I think the second one is supposed to be llvm/test/LTO/Resolution/X86/comdat.ll?) were added after the separate value map for aliases already existed, and weren't related to any value mapper changes afaict. I just did some archeology and found that the separate map exists as far back as when the IRMover code was introduced by @respindola in 2015, being split from older IR linking code, which also seems to have had a separate value map for aliases (didn't look any further back though, the code was structured quite differently).
>
> In what way do they break with this change?

Yeah, I noticed that too — this test seems to incidentally catch a problem. The breakage is that comdat `$c2` does not get linked in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99546