From: Kirill K. <kir...@sm...> - 2015-05-11 21:50:23
|
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 > >> >> > > >> >> > >> >> > >> > |