<div dir="ltr"><div>Thanks for showing the proper way to do this and taking the time to show why my analysis of MR388 was wrong!</div><div>We'll update the way Clash interacts with the GHC API.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 9 Mar 2021 at 13:43, Phyx <<a href="mailto:lonetiger@gmail.com">lonetiger@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="auto"><div>Hi, </div><div dir="auto"><br></div><div dir="auto">> But we don't _want_ the shared state, it's simply there.</div><div dir="auto">> This whole issue arises from the fact that we were oblivious to the shared RTS state, resulting in Clash doing GHC API calls where the RTS loads/links an object file twice.</div><div dir="auto"><br></div><div dir="auto">The RTS should under no circumstances be actually loading an object file twice as there's only one linker map and should result in a symbol collision. </div><div dir="auto"><br></div><div>Looking at the error you posted at <a href="https://github.com/clash-lang/clash-compiler/issues/1686" target="_blank">https://github.com/clash-lang/clash-compiler/issues/1686</a> is actually the linker doing the right thing.</div><div><pre style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;margin-top:0px;padding:16px;overflow:auto;line-height:1.45;border-radius:6px;color:rgb(36,41,46);margin-bottom:0px"><code style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;padding:0px;margin:0px;background:initial;border-radius:6px;word-break:normal;border:0px none;display:inline;overflow:visible;line-height:inherit">GHC runtime linker: fatal error: I found a duplicate definition for symbol
   Lib2_plots2_closure
whilst processing object file
   .stack-work/dist/x86_64-linux-tinfo6/Cabal-3.2.1.0/build/exe2/_clashilator/clash-syn/Lib2.o
The symbol was previously defined in
   .stack-work/dist/x86_64-linux-tinfo6/Cabal-3.2.1.0/build/exe1/_clashilator/clash-syn/Lib2.o</code></pre></div><div>You're loading the same object file twice from different build folders and the linker has no guarantee that these two are the same symbol at all.</div><div>This however is indeed a shortcoming of M388 that we can't split the C linker map easily.</div><div dir="auto"><br></div><div dir="auto">> And we're not even explicitly linking/loading object files twice, something to do with the GHC type-checker seems to do that.</div><div dir="auto"><br></div><div>Yes but you have a new object file, in a different path. This can't be resolved by the linker cache.  This looks like it accidentally worked before as the shared Haskell Linker state resolves based on the Close name itself.</div><div>So it never asked the C linker. I say accidental because there's no guarantee that the closure in exe1 and exe2 are the same, despite them having the same name..</div><div dir="auto"><br></div><div dir="auto">> I don't see how I can avoid this issue without being forced to run within a single `runGhc` session. </div><div dir="auto"><br></div><div dir="auto">As I mentioned below, you can override the <code><span>hsc_dynLinker in a wrapper around runGhc.</span></code></div><div dir="auto"><font face="monospace"><br></font></div><div dir="auto"><font face="monospace">i.e.</font></div><div dir="auto"><font face="monospace"><br></font></div><div dir="auto"><font face="monospace">runClashGhc :: <action> -> ..</font></div><div><font face="monospace">  do </font><span style="font-family:monospace">shared_linker <- ...</span></div><div dir="auto"><font face="monospace">     runGhc .. $ do</font></div><div dir="auto"><font face="monospace">       </font><span style="box-sizing:border-box;color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap;border-radius:6px;font-weight:600;display:inline-block">setSession</span><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap"> </span><span style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap">$</span><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap"> hsc_env { </span>

hsc_dynLinker <span style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap">=</span><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap"> </span>shared_linker <span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap">}</span></div><div><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap">       <action></span></div><div><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap"><br></span></div><div><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap">Should restore the behavior. You don't need to run inside a single runGhc, you just need to provide a single hsc_dynLinker.</span></div><div><span style="color:rgb(36,41,46);font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:12px;white-space:pre-wrap"><br></span></div><div><font face="SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace" color="#24292e"><span style="font-size:12px;white-space:pre-wrap">That should work.</span></font></div><div><font face="SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace" color="#24292e"><span style="font-size:12px;white-space:pre-wrap"><br></span></font></div><div><font face="SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace" color="#24292e"><span style="font-size:12px;white-space:pre-wrap">Kind Regards,</span></font></div><div><font face="SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace" color="#24292e"><span style="font-size:12px;white-space:pre-wrap">Tamar</span></font></div><div dir="auto"><div dir="auto"><br></div><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Tue, Mar 9, 2021, 11:46 Christiaan Baaij <<a href="mailto:christiaan.baaij@gmail.com" target="_blank">christiaan.baaij@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>But we don't _want_ the shared state, it's simply there.</div><div>This whole issue arises from the fact that we were oblivious to the shared RTS state, resulting in Clash doing GHC API calls where the RTS loads/links an object file twice.</div><div>And we're not even explicitly linking/loading object files twice, something to do with the GHC type-checker seems to do that.<br></div><div>I don't see how I can avoid this issue without being forced to run within a single `runGhc` session.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 9 Mar 2021 at 12:22, Phyx <<a href="mailto:lonetiger@gmail.com" rel="noreferrer" target="_blank">lonetiger@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto">Hi, <div dir="auto"><br></div><div dir="auto">Hmm... I don't agree..</div><div dir="auto"><br></div><div dir="auto">This isn't about grounds of truth or anything like that.. and in fact, an object being in the linker map, doesn't mean its usable at all or meant to be used at all.</div><div dir="auto">It can be temporary state (symbol redirection or supporting of deprecated symbols are two that come to mind).  So this is also a case of.. be careful.</div><div dir="auto"><br></div><div dir="auto">The change introduced in the MR simply decoupled the top level user interface and the C linker.</div><div dir="auto">The reason for this is simply because the majority of projects do not require shared state here, but infact benefit from unshared state.</div><div dir="auto"><br></div><div dir="auto">i.e. interpreters, IDEs etc.  Where you want to be able to process multiple separate files at the same time without needing to create new processes for each.</div><div dir="auto"><br></div><div dir="auto">Now back to your point about runGhc needing to use a shared state.. In my opinion that would be wrong.</div><div dir="auto"><br></div><div dir="auto">Here's the documentation for GHC 8.6.5 <a href="https://hackage.haskell.org/package/ghc-8.6.5/docs/GHC.html" rel="noreferrer" target="_blank">https://hackage.haskell.org/package/ghc-8.6.5/docs/GHC.html</a></div><div dir="auto"><br></div><div dir="auto">specifically:</div><div dir="auto"><br></div><div dir="auto">----</div><div dir="auto"><br></div><div dir="auto">runGhc</div><div dir="auto">  :: Maybe FilePath    - See argument to initGhcMonad.</div><div dir="auto">  -> Ghc a - The action to perform.</div><div dir="auto">  -> IO a - Run function for the Ghc monad.</div><div dir="auto"><br></div><div dir="auto">It initialises the GHC session and warnings via initGhcMonad.</div><div dir="auto">Each call to this function will create a new session which should not be shared among several threads.</div><div dir="auto"><br></div><div dir="auto">Any errors not handled inside the Ghc action are propagated as IO exceptions.</div><div dir="auto"><br></div><div dir="auto">---</div><div dir="auto"><br></div><div dir="auto">And if the session isn't guaranteed there's no guarantee about the underlying state.</div><div dir="auto">This explicit declaration that runGhc will not share state has been in the API for for decades (going as far back as I stopped looking at 7.2).</div><div dir="auto"><br></div><div dir="auto">That Clash is relying on behavior we explicitly stated is not the case is a bug in Clash.</div><div dir="auto"><br></div><div dir="auto">If you require shared state you should not be using the top level runGhc wrapper but instead call unGhc yourself (or call setSession yourself). </div><div dir="auto"><br></div><div dir="auto">There is perhaps a case to be made for a runGhcShared which does this, but runGhc itself never guaranteed one session or one state. <br><br><div dir="auto">Kind regards, </div><div dir="auto">Tamar </div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 9, 2021, 10:27 Christiaan Baaij <<a href="mailto:christiaan.baaij@gmail.com" rel="noreferrer" target="_blank">christiaan.baaij@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Even if MR388 ( <a href="https://gitlab.haskell.org/ghc/ghc/-/merge_requests/388" rel="noreferrer noreferrer" target="_blank">https://gitlab.haskell.org/ghc/ghc/-/merge_requests/388</a> ) is the cause of the issue we're seeing with the API exposed by Clash, I still think MR388 is wrong.<br>My reasoning is the following:<br><br>In 8.8 and earlier we had:<br>- RTS C-code contains the ground truth of what is linked. The API it provides are set-membership, insert, lookup, and delete. Notably it does not allow you to get the set of linked objects.<br>- There is a globally shared MVar (using NOINLINE, sharedCaf, unsafePerformIO newIORef "tricks") to what is basically a log/view of the linked-objects state kept by the RTS C-code.<br><br>With MR388, in 8.10 and later we get:<br>- RTS C-code contains the ground truth of what is linked. The API it provides are set-membership, insert, lookup, and delete. Notably it does not allow you to get the set of linked objects.<br><div>- A _new_ MVar for every call to `runGhc` which is a log/view of the linked-object state kept by the RTS C-code. But that means these MVar get out-of-sync with the ground truth that is the RTS C-code! And since the RTS C-code does not expose an API to get the set of linked objects, there's no way to sync these MVars either!</div><div><br></div><div>I'm building a ghc-8.10.2 with MR388 reverted to see whether it is indeed what is causing the issue we're seeing in Clash.</div><div>Given my analysis above of what I think is wrong with MR388, I'm not saying we should completely revert MR388, but simply ensure that every HscEnv created through `runGhc` gets the globally shared MVar; as opposed to the current call to `newMVar`.<br></div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 7 Mar 2021 at 04:02, ÉRDI Gergő <<a href="mailto:gergo@erdi.hu" rel="noreferrer noreferrer" target="_blank">gergo@erdi.hu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks Matthew and Julian! Unfortunately, trying out GHC before/after this <br>
change didn't turn out to be as easy as I hoped: to do my testing, I <br>
need to build a given GHC commit, and then use that via Stack to install <br>
~140 dependencies so that I can then test the problem I have initially <br>
seen. And it turns out doing that with a random GHC commit is quite <br>
painful because in any given Stackage snapshot there will be packages with <br>
which the GHC-bundled libraries are incompatible... :/<br>
<br>
<br>
<br>
On Thu, 4 Mar 2021, Julian Leviston wrote:<br>
<br>
> Hi,I don’t know enough about what Clash does to comment really, but it sounds like<br>
> it’s to do with my work on enabling multiple linker instances<br>
> in <a href="https://gitlab.haskell.org/ghc/ghc/-/merge_requests/388" rel="noreferrer noreferrer noreferrer" target="_blank">https://gitlab.haskell.org/ghc/ghc/-/merge_requests/388</a> — maybe reading through<br>
> that or the plan I outlined at <a href="https://gitlab.haskell.org/ghc/ghc/-/issues/3372" rel="noreferrer noreferrer noreferrer" target="_blank">https://gitlab.haskell.org/ghc/ghc/-/issues/3372</a> might<br>
> help, though I’m not sure.<br>
> <br>
> Strange, though, as this work was to isolate state in GHC — to change it from using a<br>
> global IORef to use a per-process MVar . But it definitely did change the way state is<br>
> handled, so it might be the related to these issues somehow?<br>
> <br>
> I realise this isn’t much help, but maybe it points you in a direction where you can<br>
> begin to understand some more.<br>
> <br>
> Julian<br>
><br>
>       On 4 Mar 2021, at 10:55 pm, ÉRDI Gergő <<a href="mailto:gergo@erdi.hu" rel="noreferrer noreferrer" target="_blank">gergo@erdi.hu</a>> wrote:<br>
> <br>
> Hi,<br>
> <br>
> I'm trying to figure out a Clash  problem and managed to track it down to a GHC<br>
> upgrade; specifically, a given Clash version, when based on GHC 8.8, has no<br>
> problem synthesizing one module after another from one process; but the same<br>
> Clash version with GHC 8.10 fails with link-time errors on the second<br>
> compilation.<br>
> <br>
> The details are at <a href="https://github.com/clash-lang/clash-compiler/issues/1686" rel="noreferrer noreferrer noreferrer" target="_blank">https://github.com/clash-lang/clash-compiler/issues/1686</a><br>
> but for now I'm just hoping that some lightbulb will go off for someone if some<br>
> handling of internal state has changed in GHC that could mean that the symbol<br>
> tables of loaded modules could persist between GHC invocations from the same<br>
> process.<br>
> <br>
> So, does this ring a bell for anyone?<br>
> <br>
> Thanks,<br>
> Gergo<br>
> _______________________________________________<br>
> ghc-devs mailing list<br>
> <a href="mailto:ghc-devs@haskell.org" rel="noreferrer noreferrer" target="_blank">ghc-devs@haskell.org</a><br>
> <a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs" rel="noreferrer noreferrer noreferrer" target="_blank">http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs</a><br>
> <br>
> <br>
> <br>
><br>
<br>
-- <br>
<br>
   .--= ULLA! =-----------------.<br>
    \     <a href="http://gergo.erdi.hu" rel="noreferrer noreferrer noreferrer" target="_blank">http://gergo.erdi.hu</a>   \<br>
     `---= <a href="mailto:gergo@erdi.hu" rel="noreferrer noreferrer" target="_blank">gergo@erdi.hu</a> =-------'<br>
I tried to commit suicide once by taking over 1,000 aspirin. But after I took 2, I felt better!_______________________________________________<br>
ghc-devs mailing list<br>
<a href="mailto:ghc-devs@haskell.org" rel="noreferrer noreferrer" target="_blank">ghc-devs@haskell.org</a><br>
<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs" rel="noreferrer noreferrer noreferrer" target="_blank">http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs</a><br>
</blockquote></div></div>
_______________________________________________<br>
ghc-devs mailing list<br>
<a href="mailto:ghc-devs@haskell.org" rel="noreferrer noreferrer" target="_blank">ghc-devs@haskell.org</a><br>
<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs" rel="noreferrer noreferrer noreferrer" target="_blank">http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div></div></div>
</div>
</blockquote></div></div>