[PATCH] Add getters for Pass IDs used by Polly plug-in.
Matthew Curtis
mcurtis at codeaurora.org
Wed Mar 13 06:06:45 PDT 2013
On 3/12/2013 10:28 PM, Gao, Yunzhong wrote:
> 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.
Hello Gao, Nice to meet you as well.
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.
In case there's still confusion about the situation, here's a more
detailed description:
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:
1: virtual void getAnalysisUsage(AnalysisUsage &AU) const {
2: AU.addRequired<DominatorTree>();
3: AU.addRequired<LoopInfo>();
4: AU.addRequired<ScalarEvolution>();
5: AU.addRequiredID(LoopSimplifyID);
6: AU.addRequiredID(LCSSAID);
7: if (EnableIVRewrite)
8: AU.addRequired<IVUsers>();
9: AU.addPreserved<ScalarEvolution>();
10: AU.addPreservedID(LoopSimplifyID);
11: AU.addPreservedID(LCSSAID);
12: if (EnableIVRewrite)
13: AU.addPreserved<IVUsers>();
14: AU.setPreservesCFG();
15: }
Line 5 for examples refers to LoopSimplifyID which is declared in
llvm/Transforms/Scalar.h
1: extern char &LoopSimplifyID;
and defined in lib/Transforms/Utils/LoopSimplify.cpp:
1: char LoopSimplify::ID = 0;
2: INITIALIZE_PASS_BEGIN(LoopSimplify, "loop-simplify",
3: "Canonicalize natural loops", true, false)
4: INITIALIZE_PASS_DEPENDENCY(DominatorTree)
5: INITIALIZE_PASS_DEPENDENCY(LoopInfo)
6: INITIALIZE_PASS_END(LoopSimplify, "loop-simplify",
7: "Canonicalize natural loops", true, false)
8:
9: // Publicly exposed interface to pass...
10: char &llvm::LoopSimplifyID = LoopSimplify::ID;
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.
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
2: const void* GetLoopSimplifyID();
and in lib/Transforms/Utils/LoopSimplify.cpp:
11: const void* llvm::GetLoopSimplifyID() { return &LoopSimplify::ID; }
In PollyIndVarSimplify::getAnalysisUsage, line 5 becomes
5: AU.addRequiredID(GetLoopSimplifyID());
The clang .def file exports GetLoopSimplifyID but not LoopSimplifyID.
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.
Hope this helps,
Matthew Curtis.
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Matthew Curtis
> Sent: Wednesday, March 06, 2013 6:14 AM
> To: Robinson, Paul
> Cc: Sebastian Pop; llvm-commits at cs.uiuc.edu; tobias at grosser.es
> Subject: Re: [PATCH] Add getters for Pass IDs used by Polly plug-in.
>
> On 3/5/2013 4:50 PM, Robinson, Paul wrote:
>>> -----Original Message-----
>>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>>> bounces at cs.uiuc.edu] On Behalf Of Andrew Trick
>>> Sent: Tuesday, March 05, 2013 1:47 PM
>>> To: Matthew Curtis
>>> Cc: tobias at grosser.es; llvm-commits at cs.uiuc.edu; 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 <mcurtis at codeaurora.org>
>>> wrote:
>>>
>>>> On 3/5/2013 1:03 PM, Andrew Trick wrote:
>>>>> On Mar 4, 2013, at 2:40 PM, Matthew Curtis <mcurtis at codeaurora.org>
>>> wrote:
>>>>>> [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
>>> bug
>>>>>> in the Visual Studio compiler or linker that results in multiple
>>>>>> dll-local copies of this data when referenced directly. The result
>>> is
>>>>>> that Polly is unable to locate Passes that are part of the
>>> executable
>>>>>> 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:
>>>>>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-January/058838.htm
>>>>>> l
>>>>>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/thread.ht
>>>>>> ml#58982
>>>>> Hi Matthew,
>>>>>
>>>>> Isn't it more convenient to refer to pass IDs without including all
>>> the pass headers and being forced to define the pass in the header?
>>>> Yes, but pass IDs are not always provided separately. For example
>>>> the
>>> DominatorTree, LoopInfo, and ScalarEvolution passes do not.
>>>>> Does the linker resolve the ID address uniquely when you use an
>>> extern global reference instead?
>>>> No. It does not work correctly when data is shared across library
>>> 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
>> 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.
> That is correct.
>> 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.
> 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.
>> I've been away from Windows too long to comment any further.
>> --paulr
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130313/33759733/attachment.html>
More information about the llvm-commits
mailing list