[llvm-dev] [ThinLTO] Using two different IRMovers for the same composite module? (related to PR28180)

[Taewook Oh via llvm-dev]

Sorry I missed that https://reviews.llvm.org/D23841 is already submitted. Thanks!


On 9/2/16, 10:41 AM, "Taewook Oh" <twoh at fb.com> wrote:

    I just filed a bug (https://llvm.org/bugs/show_bug.cgi?id=30248), which is different from the one discussed in this thread but seems to have a same root. I attached a small repro as well. 
    
    As you said, assertion fails if the mapper tries to map a metadata that already in the destination module, when the same metadata is reached from the source module. More specifically, it is possible that a function parameter V of Mapper::mapValue in lib/Transforms/utils/ValueMapper.cpp is already in the destination module. Still, V is forced to be (re)materialized into the destination module, and during that process the original V can be erase from the module while NewV is created (last if statement of IRLinker::linkGlobalValueProto in lib/Linker/IRMover.cpp). For such case ), “getVM()[V] = NewV” statement in Mapper::mapValue is invalid.
    
    I confirmed that the bug can be fixed by your internal fix that you’ve attached to this thread.
    
    Thanks,
    Taewook  
    
    On 7/23/16, 10:13 AM, "mehdi.amini at apple.com on behalf of Mehdi Amini" <mehdi.amini at apple.com> wrote:
    
        
        > On Jul 22, 2016, at 4:42 PM, Taewook Oh <twoh at fb.com> wrote:
        > 
        > It seems that the patch works for me as well, though the linker crashes with another error after that. Thanks!
        > 
        > Mehdi, I couldn’t quite understand what do you mean by you don’t have a repro so you couldn’t upstream the patch. Aren’t .ll files you attached sufficient to submit along with the patch? If there is anything I can help you to upstream it, please let me know. 
        
        I could submit the patch as-is but it is not in my habit to do that without a total understanding of the situation, i.e. I’m not convinced the test case is totally reduced and I need to “reverse engineer” the debug metadata to get a source-code construct that would trigger this bug.
        
        — 
        Mehdi
        
        
        
        
        
        > 
        > -- Taewook
        > 
        > On 7/22/16, 2:52 PM, "mehdi.amini at apple.com on behalf of Mehdi Amini" <mehdi.amini at apple.com> wrote:
        > 
        > 
        >> On Jul 22, 2016, at 2:47 PM, Davide Italiano <davide at freebsd.org> wrote:
        >> 
        >> On Fri, Jul 22, 2016 at 2:44 PM, Davide Italiano <davide at freebsd.org> wrote:
        >>> On Fri, Jul 22, 2016 at 2:34 PM, Taewook Oh via llvm-dev
        >>> <llvm-dev at lists.llvm.org> wrote:
        >>>> Yes, I have the repro, though I can’t publish it externally. It would be
        >>>> great if you can upstream the patch so I can try it. Thank you for your
        >>>> explanation as well!
        >>> 
        >>> There's a reproducer attached (obtained via lld --reproduce option).
        >>> If that doesn't work, you can checkout mozjs and try to reduce from
        >>> there.
        >>> It happens while doing an LTO build with lld.
        >>> I have a fix for that (Mehdi has one as well, apparently) in my local
        >>> tree, but I don't have time to reduce. If you can take care of that,
        >>> chances are that an upstream fix will be committed shortly after.
        >>> 
        >> 
        >> Just to clarify, I'm talking about the issue in PR28180. I can't
        >> comment about the ThinLTO one, sorry.
        > 
        > If lld is setting enableDebugTypeODRUniquing(); on the context and isn’t using the IRMover to target an empty module, it can be the same bug.
        > I mentioned that it should touch only ThinLTO but I had ld64 in mind.
        > 
        > — 
        > Mehdi
        > 
        >