<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 3/12/2013 10:28 PM, Gao, Yunzhong
wrote:<br>
</div>
<blockquote
cite="mid:FCC941992AC7344D92244111EE1D32296A98F05C@USCULXMSG03.am.sony.com"
type="cite">
<pre wrap="">Hi Matthew,
Nice to meet you. I have a question.
I do not see a lot of use of __declspec(dllexport) in the polly codes (lib/JSON seems to be the only directory); neither
do I find MSVC-style .def files. But you claimed that "data definitions were dll-exported." I guess I must be missing
something here. Could you help me understand your patch better?
- Gao.
</pre>
</blockquote>
Hello Gao, Nice to meet you as well.<br>
<br>
We created a .def file for symbols to export from clang.exe. Polly
is then linked against the resulting clang.lib. The .def file and
build-related changes have not been upstreamed pending acceptance of
this patch.<br>
<br>
In case there's still confusion about the situation, here's a more
detailed description:<br>
<br>
The symbols in question are defined in the host executable loading
the polly plug-in. For example in polly's lib/IndVarSimplify.cpp the
class PollyIndVarSimplify contains:<br>
<br>
<blockquote><tt> 1: virtual void getAnalysisUsage(AnalysisUsage
&AU) const {</tt><br>
<tt> 2: AU.addRequired<DominatorTree>();</tt><br>
<tt> 3: AU.addRequired<LoopInfo>();</tt><br>
<tt> 4: AU.addRequired<ScalarEvolution>();</tt><br>
<tt> 5: AU.addRequiredID(LoopSimplifyID);</tt><br>
<tt> 6: AU.addRequiredID(LCSSAID);</tt><br>
<tt> 7: if (EnableIVRewrite)</tt><br>
<tt> 8: AU.addRequired<IVUsers>();</tt><br>
<tt> 9: AU.addPreserved<ScalarEvolution>();</tt><br>
<tt>10: AU.addPreservedID(LoopSimplifyID);</tt><br>
<tt>11: AU.addPreservedID(LCSSAID);</tt><br>
<tt>12: if (EnableIVRewrite)</tt><br>
<tt>13: AU.addPreserved<IVUsers>();</tt><br>
<tt>14: AU.setPreservesCFG();</tt><br>
<tt>15: }</tt><br>
</blockquote>
<br>
Line 5 for examples refers to LoopSimplifyID which is declared in
llvm/Transforms/Scalar.h<br>
<br>
<blockquote><tt> 1: extern char &LoopSimplifyID;</tt><tt><br>
</tt></blockquote>
<br>
and defined in lib/Transforms/Utils/LoopSimplify.cpp:<br>
<br>
<blockquote><tt> 1: char LoopSimplify::ID = 0;</tt><tt><br>
</tt><tt> 2: INITIALIZE_PASS_BEGIN(LoopSimplify, "loop-simplify",</tt><tt><br>
</tt><tt> 3: "Canonicalize natural loops", true,
false)</tt><tt><br>
</tt><tt> 4: INITIALIZE_PASS_DEPENDENCY(DominatorTree)</tt><tt><br>
</tt><tt> 5: INITIALIZE_PASS_DEPENDENCY(LoopInfo)</tt><tt><br>
</tt><tt> 6: INITIALIZE_PASS_END(LoopSimplify, "loop-simplify",</tt><tt><br>
</tt><tt> 7: "Canonicalize natural loops", true,
false)</tt><tt><br>
</tt><tt> 8: </tt><tt><br>
</tt><tt> 9: // Publicly exposed interface to pass...</tt><tt><br>
</tt><tt>10: char &llvm::LoopSimplifyID = LoopSimplify::ID;</tt><tt><br>
</tt></blockquote>
<br>
This definition is part of the host executable (typically clang).
Hence the shared data between the polly plug-in and the host
executable. This seems ok except that some bug in the Visual Studio
toolchain results in LoopSimplifyID having one value inside polly
and a different value inside clang.<br>
<br>
This patch adds an alternate mechanism for getting the ID which the
MSVC toolchain compiles and links correctly. In
llvm/Transforms/Scalar.h we add<br>
<br>
<blockquote><tt> 2: const void* GetLoopSimplifyID();</tt><br>
</blockquote>
<br>
and in lib/Transforms/Utils/LoopSimplify.cpp:<br>
<br>
<blockquote><tt>11: const void* llvm::GetLoopSimplifyID() { return
&LoopSimplify::ID; }</tt><br>
</blockquote>
<br>
In PollyIndVarSimplify<tt>::</tt><tt>getAnalysisUsage, line 5
becomes</tt><br>
<tt><br>
</tt>
<blockquote><tt> 5: AU.addRequiredID(</tt><tt>GetLoopSimplifyID());</tt><br>
</blockquote>
<br>
The clang .def file exports GetLoopSimplifyID but not
LoopSimplifyID.<br>
<br>
Aside from being correct, exporting a function rather than data
allows us to use delay loading to resolve the symbols that the
plug-in needs from the host. This makes it possible to load polly
into any host executable, not just clang.exe, as would normally be
required since polly was linked against clang.lib.<br>
<br>
Hope this helps,<br>
Matthew Curtis.<br>
<br>
<blockquote
cite="mid:FCC941992AC7344D92244111EE1D32296A98F05C@USCULXMSG03.am.sony.com"
type="cite">
<pre wrap="">
-----Original Message-----
From: <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [<a class="moz-txt-link-freetext" href="mailto:llvm-commits-bounces@cs.uiuc.edu">mailto:llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Matthew Curtis
Sent: Wednesday, March 06, 2013 6:14 AM
To: Robinson, Paul
Cc: Sebastian Pop; <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; <a class="moz-txt-link-abbreviated" href="mailto:tobias@grosser.es">tobias@grosser.es</a>
Subject: Re: [PATCH] Add getters for Pass IDs used by Polly plug-in.
On 3/5/2013 4:50 PM, Robinson, Paul wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">-----Original Message-----
From: <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [<a class="moz-txt-link-freetext" href="mailto:llvm-commits">mailto:llvm-commits</a>-
<a class="moz-txt-link-abbreviated" href="mailto:bounces@cs.uiuc.edu">bounces@cs.uiuc.edu</a>] On Behalf Of Andrew Trick
Sent: Tuesday, March 05, 2013 1:47 PM
To: Matthew Curtis
Cc: <a class="moz-txt-link-abbreviated" href="mailto:tobias@grosser.es">tobias@grosser.es</a>; <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; Sebastian Pop
Subject: Re: [PATCH] Add getters for Pass IDs used by Polly plug-in.
On Mar 5, 2013, at 11:30 AM, Matthew Curtis <a class="moz-txt-link-rfc2396E" href="mailto:mcurtis@codeaurora.org"><mcurtis@codeaurora.org></a>
wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 3/5/2013 1:03 PM, Andrew Trick wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Mar 4, 2013, at 2:40 PM, Matthew Curtis <a class="moz-txt-link-rfc2396E" href="mailto:mcurtis@codeaurora.org"><mcurtis@codeaurora.org></a>
</pre>
</blockquote>
</blockquote>
<pre wrap="">wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">[re-submitting this patch in the hopes that it will find new life]
This patch helps resolve a couple of issues we encountered while
porting the Polly plug-in to Windows.
The Polly plugin depends on some LLVM Passes which it identifies
via Pass IDs. Pass IDs are implemented as the address of a static
member variable (ID) of the Pass. On Windows however, there seems
to be a
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">bug
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">in the Visual Studio compiler or linker that results in multiple
dll-local copies of this data when referenced directly. The result
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">is
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">that Polly is unable to locate Passes that are part of the
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">executable
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">it is being loaded into.
As a work around, this patch adds getters (GetClassPassID) for the
Pass IDs used by Polly. The symbols for the globals are not
exported. The getters are exported and Polly uses them instead.
Removing use of global data by the Polly plug-in also makes it
possible to delay load LLVM symbols from the containing executable
which in turn removes the restriction that the executable have a
specific name (determined at link-time).
We also considered a couple alternatives to this minimal patch:
1) Update *all* Passes to provide GetClassPassID in lieu of ID.
This leaves the code in the cleanest state and would be a good
choice if starting from scratch. However I started implementing it
and it turns out to have a very large ripple effect. Not only does
it require changing a large number of files in LLVM, but it breaks
source compatibility for passes that are not part of the LLVM
source.
That being said, I'd be willing to do the work if everyone thinks
that's the right thing to do and we're willing break source
compatibility.
Perhaps we could ease the pain by deprecating ID for some time
before removing it.
2) Add GetClassPassID to Passes accessible by plug-ins (i.e. those
under include/llvm), but leave ID as it is.
As with the proposed patch, this may cause confusion because there
are multiple ways of getting a Pass's ID. But at least it's not
specific to the Polly plug-in and other plug-ins on Windows may
benefit.
Thanks,
Matthew Curtis
BTW, there's a brief related discussion on llvm-dev here:
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-January/058838.htm">http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-January/058838.htm</a>
l
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/thread.ht">http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/thread.ht</a>
ml#58982
</pre>
</blockquote>
<pre wrap="">Hi Matthew,
Isn't it more convenient to refer to pass IDs without including all
</pre>
</blockquote>
</blockquote>
<pre wrap="">the pass headers and being forced to define the pass in the header?
</pre>
<blockquote type="cite">
<pre wrap="">Yes, but pass IDs are not always provided separately. For example
the
</pre>
</blockquote>
<pre wrap="">DominatorTree, LoopInfo, and ScalarEvolution passes do not.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap=""> Does the linker resolve the ID address uniquely when you use an
</pre>
</blockquote>
</blockquote>
<pre wrap="">extern global reference instead?
</pre>
<blockquote type="cite">
<pre wrap="">No. It does not work correctly when data is shared across library
</pre>
</blockquote>
<pre wrap="">boundaries.
Sorry, I don't really understand the nature of the linker bug. I
don't understand how it's possible for Polly to import llvm's C++ ABI
if it can't "import data". Are you trying to make a DLL-safe subset
of llvm's
C++ ABI, free of all global data and static fields? That doesn't seem
generally possible without formalizing that interface. It's something
you should discuss on llvm-dev with people who have experience with
that sort of thing!
-Andy
</pre>
</blockquote>
<pre wrap="">Based on the problem description and the fix, it would have to be that:
- function definitions in different DLLs are resolved to one instance,
but
- data-item definitions in different DLLs are NOT resolved to one instance.
</pre>
</blockquote>
<pre wrap="">That is correct.
</pre>
<blockquote type="cite">
<pre wrap="">I could see that behavior cropping up if the function definitions were
dll-exported but the data definitions were not.
Otherwise it seems like a particularly weird Windows bug.
</pre>
</blockquote>
<pre wrap="">Data definitions were dll-exported, so yes it is a particularly weird Windows bug.
Keep in mind that we also wanted to remove exported data definitions so that we could delay load symbols from the executable loading the plug-in. This allows the plug-in dll to be loaded into any executable that provides the symbols, not just the one it was linked against at build time.
</pre>
<blockquote type="cite">
<pre wrap="">I've been away from Windows too long to comment any further.
--paulr
</pre>
</blockquote>
<pre wrap="">
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation</pre>
</body>
</html>