Re: [Kaldi-users] Kaldi compiles now under VS 2013

[Daniel P. <dp...@gm...>]

BTW, this is one of many bugs that I specifically pointed out to the
Visual Studio people while at Microsoft (and I looked it up just now
because I remembered checking the standard before, so I knew it was
there).  Their attitude was that compliance with the standard doesn't
really matter; what really matters is to not break the Windows build.
Eventually I think I gained a bad reputation in that circle as not
being sufficiently "with the program".  Anyway, I'm willing to patch
Kaldi to support their broken compiler as far as is reasonable, but it
doesn't make me very happy.

Dan


On Mon, May 11, 2015 at 7:06 PM, Daniel Povey <dp...@gm...> wrote:
> It isn't a gray area in the standard; see
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4296.pdf
> the last paragraph on page 367:
>   If a name does not depend on a template-parameter (as defined in
> 14.6.2), a declaration (or set of declarations) for that name shall be
> in scope at the point where the name appears in the template
> definition; the name is bound to the declaration (or declarations)
> found at that point and this binding is not affected by declarations
> that are visible at the point of instantiation.
>
> Dan
>
>
> On Mon, May 11, 2015 at 2:50 PM, Kirill Katsnelson
> <kir...@sm...> wrote:
>> I thought it was but this really appears to be a grey area in the standard. But indeed, from the practical standpoint, I think the best thing to do is just make all compilers happy. I can figure out if this is really undefined C++ later, if there is a theoretical interest.
>>
>>  -kkm
>>
>>> -----Original Message-----
>>> From: Daniel Povey [mailto:dp...@gm...]
>>> Sent: 2015-05-11 1444
>>> To: Jan Trmal
>>> Cc: Kirill Katsnelson; kal...@li...; kaldi-
>>> dev...@li...; 洪孝宗
>>> Subject: Re: [Kaldi-users] Kaldi compiles now under VS 2013
>>>
>>> OK fine.
>>> I'm pretty sure this is a bug in MSVC, that the namespace lookup for a
>>> templated function happens in the context in which the template is
>>> evaluated (as opposed to where it was defined).  But it makes sense to
>>> patch it anyway.
>>> Dan
>>>
>>>
>>> On Mon, May 11, 2015 at 2:09 PM, Jan Trmal <jt...@gm...> wrote:
>>> > Ok, I will have a.look tmrw. I think I've resolved the types issue
>>> > (last one you mentioned). I generated the patch for openfst, that
>>> > defines the int types within the openfst namespace - originally they
>>> > exist on.the highest scope (i.e. ::int32) The patch has something
>>> like
>>> > namespace fst {
>>> >   typedef int32 ::int32;
>>> > }
>>> >
>>> > Doping this, the lookup works fine even under the ms c++.
>>> > To me, this is optimal solution as we have to patch the openfst
>>> anyway
>>> > (and my change wont break any code).
>>> > You can.have a look ať the openfstwin-1.3.3.patch in tools/extras.
>>> > I am open to any suggestion or general discussion on how this should
>>> > be made better.
>>> > Y.
>>> >
>>> > On May 11, 2015 9:03 PM, "Daniel Povey" <dp...@gm...> wrote:
>>> >>
>>> >> > Well, the build was not "painless" at all. In fact, the perl
>>> script
>>> >> > generated a solution with 600+ projects that simply failed to
>>> >> > build. VS was churning out dependencies on these, and crashed in
>>> >> > about 10 minutes of just sitting there with a 100% CPU use. Not a
>>> >> > single file was compiled, so I think it was just autogenerating
>>> dependencies at this point.
>>> >>
>>> >> Build systems on Windows are pretty broken.  I think in Microsoft
>>> >> they use some other system, not the one used in VS, which is not
>>> >> publicly released.
>>> >>
>>> >> > I did not try to build from the command line though. Am I
>>> >> > understanding it would compile?
>>> >>
>>> >> No, not everything would compile.  Sometimes the VS compiler just
>>> crashes.
>>> >>
>>> >> > What I did not like about the existing system is that I could not
>>> >> > plug in CUDA or intel compiler. It is extremely rigid in the
>>> option
>>> >> > part, and to change compiler options you are most likely to change
>>> >> > the perl code that generates the project files, or modify the
>>> >> > different MSBuild include files (called "property sheets" in this
>>> >> > context) that are interplaying quite non-trivially. No, I have
>>> >> > spent a few days on the build system not for a love of tinkering
>>> >> > with MSBuils at all! :)
>>> >>
>>> >> What we committed was just a hack to try to get something working.
>>> >> IIRC we just looked at the build files generated by VS to try to
>>> >> guess what the format of those files was supposed to be; I'm not
>>> sure
>>> >> that they are really intended to be human readable/writable (unless
>>> >> that human really loves XML).
>>> >>
>>> >> > Anyway, you can look at it at
>>> >> > https://github.com/kkm000/kaldi/tree/winbuild/msbuild. (NB I have
>>> >> > not updated the branch in a while) If you think this is not an
>>> >> > acceptable solution, I'm just dropping the ball, no offence taken,
>>> >> > naturally. I can maintain this as an "unofficial port" for a
>>> while.
>>> >> > In any case, build is the least problem, as Dan has pointed out
>>> already...
>>> >> >
>>> >> > The biggest (in the number of changed lines) changes I had to make
>>> >> > were in the namespace using declarations, because of conflicts
>>> >> > between the types
>>> >> > int32 and friends from different namespaces. I ended up reading
>>> the
>>> >> > C++ standard to understand whether gcc, icl or msvc is "correct"
>>> in
>>> >> > different cases they are complaining about, and the standard
>>> >> > appears not to define any behavior there. These changes I would
>>> >> > prefer accepted into the mainline of development, because they are
>>> harder to maintain in a fork.
>>> >>
>>> >> In any case where you have to do something like typedef kaldi::int32
>>> >> int32, we should definitely get this committed- talk to Yenda.
>>> >> Actually, this is really a bug in OpenFST which they haven't fixed
>>> >> yet.  There is a "proper" POSIX way to get types like int32, which
>>> we
>>> >> use in Kaldi, but in OpenFst they do it in a different, ad-hoc way
>>> >> which sometimes generates different types (e.g. on a system where
>>> int
>>> >> and long int are both 32-bit).  So the Kaldi int32 and the OpenFst
>>> >> int32 end up being different, incompatible types.
>>> >>
>>> >> Dan
>>> >>
>>> >>
>>> >> >> -----Original Message-----
>>> >> >> From: Jan Trmal [mailto:jt...@gm...]
>>> >> >> Sent: 2015-05-11 0201
>>> >> >> To: Dan Povey
>>> >> >> Cc: Kirill Katsnelson; kal...@li...;
>>> >> >> kaldi- us...@li...; 洪孝宗
>>> >> >> Subject: Re: [Kaldi-users] Kaldi compiles now under VS 2013
>>> >> >>
>>> >> >> Guys, the migration to git is "unofficially" done -- unless we
>>> >> >> find some problems with it, the repo (https://github.com/kaldi-
>>> >> >> asr/kaldi.git) will stay as it is now.
>>> >> >>
>>> >> >> Kirill,  there is no reason why wait. We should start working on
>>> it.
>>> >> >> I propose you could first send the Kaldi code patches -- I
>>> >> >> actually worked on this and I'm able to compile Kaldi, so this
>>> >> >> should be quite straightforward, provided you will generate the
>>> diff against the HEAD.
>>> >> >>
>>> >> >> Then we can start looking at the  "build system". I understand
>>> >> >> your reasons and I think they might be legit. My feeling is,
>>> >> >> however, that a lot of people are not using MSBuild and will
>>> >> >> expect the MSVC solution, just because this is how they usually
>>> >> >> work. I haven't tested it, but you should be able to use the
>>> >> >> current SLN to build the binaries from command line -- that
>>> should
>>> >> >> be completely painless -- but again, I haven't tested it.
>>> >> >> y.
>>> >> >>
>>> >> >>
>>> >> >> On Thu, May 7, 2015 at 10:53 PM, Daniel Povey <dp...@gm...>
>>> wrote:
>>> >> >>
>>> >> >>
>>> >> >>       >> To the extent that your patches are simple fixes and
>>> >> >> minor changes to
>>> >> >>       >> the code, I think we could apply them.  Perhaps you
>>> could
>>> >> >> work with
>>> >> >>       >> Yenda to get your changes checked?
>>> >> >>       >
>>> >> >>       > Easy enough, and as easy to hold till the migration is
>>> done.
>>> >> >> Was there a change in the plans?
>>> >> >>
>>> >> >>       Not really a change in the plans, but we don't have a
>>> definite
>>> >> >>       timeline for migrating to git, and it could be that we'll
>>> >> >> keep the svn
>>> >> >>       as upstream for a while.  Your changes seem reasonable, but
>>> >> >> I think it
>>> >> >>       would be better to get things started now rather than
>>> later.
>>> >> >> The
>>> >> >>       git/svn issues should not be that hard.  Just start sending
>>> >> >> patches to
>>> >> >>       Yenda and we'll handle your changes bit by bit.
>>> >> >>       Dan
>>> >> >>
>>> >> >>
>>> >> >>
>>> >> >>       >
>>> >> >>       >> If you update your git repo to
>>> >> >>       >> point to the current code, then a patch should be
>>> >> >> applicable by the
>>> >> >>       >> program "patch" to the svn repo too.
>>> >> >>       >
>>> >> >>       > There are many changes, and I certainly want to split the
>>> >> >> patch so it is readable. One megapatch is not reviewable.
>>> >> >>       >
>>> >> >>       >> Regarding your build approach, you could send me and
>>> >> >> Yenda some details
>>> >> >>       >> about how you did it, and we could consider whether to
>>> >> >> support that.
>>> >> >>       >
>>> >> >>       > There's a Powershell script that takes information from
>>> >> >> every src/*/Makefile into a simple declarative MSBuild script
>>> >> >> $dirname.kwproj, and a top-level MSBuild "makefile" that drives
>>> >> >> the whole thing. A little bit more complex than that to allow for
>>> >> >> user- defined options, but generally this is it. This supports
>>> >> >> building libraries, tools, tests and running the tests with
>>> >> >> command line switches. The whole thing pretty much mimics a linux
>>> >> >> make process but using MS tools.
>>> >> >>       >
>>> >> >>       >> I don't think it makes sense to try to maintain a
>>> >> >> parallel
>>> >> >> "windows-
>>> >> >>       >> compatible" version of the scripts, if there are larger
>>> >> >> changes
>>> >> >>       >> required there.
>>> >> >>       >
>>> >> >>       > I was targeting for no changes at all. There maybe a few
>>> >> >> very small patches that supposed not to break compatibility.
>>> >> >>       >
>>> >> >>       >> Anyway you depend on cygwin for scripting, and the set
>>> >> >>       >> of people who want to run cygwin and build with MSVC is
>>> >> >> probably
>>> >> >>       >> limited enough that I don't think it makes sense for us
>>> >> >> to try to
>>> >> >>       >> support it.
>>> >> >>       >
>>> >> >>       > This is not as simple a choice as it seems at first. Alex
>>> >> >> Hung just posted a trick to build CUDA under Cygwin; before he
>>> did
>>> >> >> I thought it was not possible. MKL is another story. I would
>>> >> >> rather build under native Windows than try to pull this monster
>>> >> >> into the Cygwin build environment.
>>> >> >>       >
>>> >> >>       >  -kkm
>>> >> >>       >
>>> >> >>
>>> >> >>
>>> >> >