llvm dev - Mar 2016 - Problem with mingw32 DLL build

Folks, there is an issue pretty buried in the commits list that I suspect
should have wider visibility.

See r262188 and subsequent discussion. To summarize: it appears that
mingw32 was unable to correctly produce a static data member when
instantiated as a base class. The "fix" is to then explicitly
instantiate
the base class separately from its use in a base class.

I think this is unacceptable in this case because these base classes are
intended to be the primary way that users of LLVM will define new analysis
passes. That means we'll actually have to teach LLVM library users about
this pattern, not just the LLVM developers. =/

So my questions are:

1) Is there some more elegant way to fix this that doesn't require every
derived class to write an explicit instantiation definition of their base
class? If so, then the rest of my questions are moot.

2) If not, is this a problem with a native Windows 32-bit DLL build?

3) If this is a problem with the native Windows 32-bit DLL build, should we
back out of using this pattern of CRTP injection of the static data member
and just deal with the significant (and error prone) boiler plate? It seems
less error prone than the alternative.

4) If this is not a problem with the native Windows 32-bit DLL, then I
wonder how valuable it is to continue to support the conjunction of mingw32
and a DLL build? This seems to be a really high cost to carry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160301/6a5e4c0d/attachment.html>

First, I'd like to say it would be great if we could get away from relying
on these globally unique pass IDs represented as addresses of globals. Long
ago I tried to hack up a DLL build of LLVM and quickly discovered that
these IDs are the biggest source of dllimported data, which has to be
annotated with __declspec(dllimport/dllexport).

Here's the issue as I understand it:
- LLVM today supports using mingw64 compilers with -DBUILD_SHARED_LIBS=ON.
This produces a DLL per LLVM library, just like Unix.
- In lib/Analysis, each analysis inherits from a CRTP base class
AnalysisBase.
- AnalysisBase is very simple, it exists solely to provide the unique pass
ID in the form of a static data member.
- Because the ID is part of a template, it is only emitted when referenced.
This is the same on Windows and Unix, no compiler bug here.
  - Previously, each analysis would manually provide it's own ID, and the
ID would be emitted exactly once in the TU providing the analysis.
- The Windows loader is very simple, it does not merge symbols across DSOs
as it does on Unix. As a result, the IDs are not unique, leading to runtime
problems.
  - The auto-dll tools provided by mingw might not let you get this far,
though, I haven't actually seen the logs.

You introduced AnalysisBase to eliminate the boilerplate of declaring and
defining the ID of each analysis, and discovered that it just means we have
to put back a different kind of boilerplate in the form of explicit
template instantiations.

I don't think we can overcome the boilerplate here, if we want to support
BUILD_SHARED_LIBS on Windows, regardless of compiler. It's a limitation of
DLLs, and a useful one because it avoids doing tons of useless symbol
lookup at startup.

So, the options are:
1. Go back to declaring IDs in each analysis. It's tried and true and
doesn't have vague linkage like templates.
2. Push forward with extern template declarations, which is a new kind of
boilerplate that most users of LLVM probably don't understand very well.
3. Drop support for BUILD_SHARED_LIBS. This might not actually be viable if
we want to try to support transforms and analysis provided by loadable
plugins. I'm not sure what issues would arise here.

I guess I favor 1.

On Tue, Mar 1, 2016 at 9:01 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:
> Folks, there is an issue pretty buried in the commits list that I suspect
> should have wider visibility.
>
> See r262188 and subsequent discussion. To summarize: it appears that
> mingw32 was unable to correctly produce a static data member when
> instantiated as a base class. The "fix" is to then explicitly
instantiate
> the base class separately from its use in a base class.
>
> I think this is unacceptable in this case because these base classes are
> intended to be the primary way that users of LLVM will define new analysis
> passes. That means we'll actually have to teach LLVM library users
about
> this pattern, not just the LLVM developers. =/
>
> So my questions are:
>
> 1) Is there some more elegant way to fix this that doesn't require
every
> derived class to write an explicit instantiation definition of their base
> class? If so, then the rest of my questions are moot.
>
> 2) If not, is this a problem with a native Windows 32-bit DLL build?
>
> 3) If this is a problem with the native Windows 32-bit DLL build, should
> we back out of using this pattern of CRTP injection of the static data
> member and just deal with the significant (and error prone) boiler plate?
> It seems less error prone than the alternative.
>
> 4) If this is not a problem with the native Windows 32-bit DLL, then I
> wonder how valuable it is to continue to support the conjunction of mingw32
> and a DLL build? This seems to be a really high cost to carry.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160301/7ac52fe5/attachment.html>

On Tue, Mar 1, 2016 at 4:21 PM Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> First, I'd like to say it would be great if we could get away from
relying
> on these globally unique pass IDs represented as addresses of globals. Long
> ago I tried to hack up a DLL build of LLVM and quickly discovered that
> these IDs are the biggest source of dllimported data, which has to be
> annotated with __declspec(dllimport/dllexport).
>
> Here's the issue as I understand it:
> - LLVM today supports using mingw64 compilers with -DBUILD_SHARED_LIBS=ON.
> This produces a DLL per LLVM library, just like Unix.
> - In lib/Analysis, each analysis inherits from a CRTP base class
> AnalysisBase.
> - AnalysisBase is very simple, it exists solely to provide the unique pass
> ID in the form of a static data member.
> - Because the ID is part of a template, it is only emitted when
> referenced. This is the same on Windows and Unix, no compiler bug here.
>   - Previously, each analysis would manually provide it's own ID, and
the
> ID would be emitted exactly once in the TU providing the analysis.
> - The Windows loader is very simple, it does not merge symbols across DSOs
> as it does on Unix. As a result, the IDs are not unique, leading to runtime
> problems.
>   - The auto-dll tools provided by mingw might not let you get this far,
> though, I haven't actually seen the logs.
>
> You introduced AnalysisBase to eliminate the boilerplate of declaring and
> defining the ID of each analysis, and discovered that it just means we have
> to put back a different kind of boilerplate in the form of explicit
> template instantiations.
>
> I don't think we can overcome the boilerplate here, if we want to
support
> BUILD_SHARED_LIBS on Windows, regardless of compiler. It's a limitation
of
> DLLs, and a useful one because it avoids doing tons of useless symbol
> lookup at startup.
>
> So, the options are:
> 1. Go back to declaring IDs in each analysis. It's tried and true and
> doesn't have vague linkage like templates.
>
I don't disagree with your analysis, but I'd like to get a build bot
that
tests DLL builds with a host toolchain other than mingw32 if this is in
fact common to those configs (as no other bot broke that i saw)... But
that's a separate issue.

The base class isn't *just* providing the static variable. It is also
providing the accessor method. I really like using a method instead of raw
access to the static data member. It is also providing the name of the
pass. So what we'd likely end up with is still having the base class but
having a friend declaration and private static data member in the derived
class.

But before going there, I'd like to ask if there is another approach that
would work: do you see any ways to effectively cause the static data member
to be emitted reliably?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160301/63b84fd3/attachment.html>

On Tue, Mar 1, 2016 at 1:21 PM, Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> First, I'd like to say it would be great if we could get away from
relying
> on these globally unique pass IDs represented as addresses of globals. Long
> ago I tried to hack up a DLL build of LLVM and quickly discovered that
> these IDs are the biggest source of dllimported data, which has to be
> annotated with __declspec(dllimport/dllexport).
>
> Here's the issue as I understand it:
> - LLVM today supports using mingw64 compilers with -DBUILD_SHARED_LIBS=ON.
> This produces a DLL per LLVM library, just like Unix.
> - In lib/Analysis, each analysis inherits from a CRTP base class
> AnalysisBase.
> - AnalysisBase is very simple, it exists solely to provide the unique pass
> ID in the form of a static data member.
> - Because the ID is part of a template, it is only emitted when
> referenced. This is the same on Windows and Unix, no compiler bug here.
>   - Previously, each analysis would manually provide it's own ID, and
the
> ID would be emitted exactly once in the TU providing the analysis.
> - The Windows loader is very simple, it does not merge symbols across DSOs
> as it does on Unix. As a result, the IDs are not unique, leading to runtime
> problems.
>   - The auto-dll tools provided by mingw might not let you get this far,
> though, I haven't actually seen the logs.
>
> You introduced AnalysisBase to eliminate the boilerplate of declaring and
> defining the ID of each analysis, and discovered that it just means we have
> to put back a different kind of boilerplate in the form of explicit
> template instantiations.
>
> I don't think we can overcome the boilerplate here, if we want to
support
> BUILD_SHARED_LIBS on Windows, regardless of compiler. It's a limitation
of
> DLLs, and a useful one because it avoids doing tons of useless symbol
> lookup at startup.
>
> So, the options are:
> 1. Go back to declaring IDs in each analysis. It's tried and true and
> doesn't have vague linkage like templates.
>
+1

Looking at this from a documentation perspective, it seems the "tried and
true" method is easiest to document since we just say "declare this ID
in
your analysis" and "declare your pass' name" (both with
examples, of
course).
The current state is that we have to document "inherit from this CRTP base
class" and "oh, but if you want to manually specify this then here is
how
to do it" (I forget where, but having the ability to override this was the
reason given for the PassManager not doing all this internally to itself,
which would be a great simplification I think (there would just be nothing
to document)).
Hence the current state is strictly more to document, and has more
"magic"
which can cause problems for new users if they actually have to debug
anything.

Documentation is an absurdly effective way to make boilerplate a non-issue.

-- Sean Silva

> 2. Push forward with extern template declarations, which is a new kind of
> boilerplate that most users of LLVM probably don't understand very
well.
> 3. Drop support for BUILD_SHARED_LIBS. This might not actually be viable
> if we want to try to support transforms and analysis provided by loadable
> plugins. I'm not sure what issues would arise here.
>
> I guess I favor 1.
>
> On Tue, Mar 1, 2016 at 9:01 AM, Chandler Carruth <chandlerc at
gmail.com>
> wrote:
>
>> Folks, there is an issue pretty buried in the commits list that I
suspect
>> should have wider visibility.
>>
>> See r262188 and subsequent discussion. To summarize: it appears that
>> mingw32 was unable to correctly produce a static data member when
>> instantiated as a base class. The "fix" is to then explicitly
instantiate
>> the base class separately from its use in a base class.
>>
>> I think this is unacceptable in this case because these base classes
are
>> intended to be the primary way that users of LLVM will define new
analysis
>> passes. That means we'll actually have to teach LLVM library users
about
>> this pattern, not just the LLVM developers. =/
>>
>> So my questions are:
>>
>> 1) Is there some more elegant way to fix this that doesn't require
every
>> derived class to write an explicit instantiation definition of their
base
>> class? If so, then the rest of my questions are moot.
>>
>> 2) If not, is this a problem with a native Windows 32-bit DLL build?
>>
>> 3) If this is a problem with the native Windows 32-bit DLL build,
should
>> we back out of using this pattern of CRTP injection of the static data
>> member and just deal with the significant (and error prone) boiler
plate?
>> It seems less error prone than the alternative.
>>
>> 4) If this is not a problem with the native Windows 32-bit DLL, then I
>> wonder how valuable it is to continue to support the conjunction of
mingw32
>> and a DLL build? This seems to be a really high cost to carry.
>>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160301/ecaa31e4/attachment.html>