Antlr 4.10 upgrade

[alaindargelas]

No description provided.

[alaindargelas]

@hs-apotell, third_party/antlr contains your RTTI changes, the latest Antlr is supposed to fix the same + some other speedup
I tried to merge, but failed, I already updated https://github.com/alainmarcel/antlr4/tree/master and I put the new https://github.com/alainmarcel/antlr4/blob/master/antlr4-4.10.1-complete.jar file there.
Please update the third_party/antlr, once that is done I'll remove the fast_antlr and we can use the regular antlr.
We can also get rid of the indirection to alainmarcel/antlr and simply put https://github.com/antlr/antlr4 as a subsystem.
We can put the .jar file under Surelog/third_party/antlr_comp/antlr4-4.10.1-complete.jar

[hs-apotell]

We can also get rid of the indirection to alainmarcel/antlr and simply put https://github.com/antlr/antlr4 as a subsystem.

We cannot have a clone of antlr within Surelog because we use antlr for other parsing in our tool and that creates a conflict. Locally in our fork of Surelog we don't use antlr_fast for that reason. We will have to use a forked version of antlr and that would be a submodule (not a subsystem).

Also, I would recommend transferring the ownership of alainmarcel/antlr to chipsalliance. It won't break anything at the moment since nobody is referencing it.

Ping me back when that's done (or if you don't wish to transfer) and I shall follow up with the merge.

[hzeller]

The goal is to use upstream antlr, so any submodule will just point to there (and the submodule is just for the convenience of the people who don't want to install a local antlr)

We also don't use the Surelog antlr but just link against our local copy of upstream and then link Surelog against that which works well (which is why it is extra exciting to now use that upstream with is already faster than Alains' branch).

I think the alainmarcel/antlr was once considered to put to chipsalliance, but it is somewhat outside the scope of chipsalliance so was not accepted. If we all just use the upstream version of Antlr, things will get simpler (also, I am currently involved in getting some patches into Antlr, so there is more continued effort into getting things faster and even less RTTI'ed).

[hs-apotell]

The goal is to use upstream antlr, so any submodule will just point to there (and the submodule is just for the convenience of the people who don't want to install a local antlr)

Using antlr/antlr4 with no modifications is even better. I interpreted @alaindargelas comment as going back to what used to be a local copy of antlr modified for use with Surelog only.

If the plan is to use pristine antlr/antlr4 then what's there to merge? We just need to update the entry in .gitmodule to pull the relevant repository and we should be done. What am I missing??

[hzeller]

Exactly: changing .gitmodule to the upstream antlr and then have one directory with the pre-generated jar file (as that otherwise would require a whole lot more dependencies with maven and stuff).

[alaindargelas]

@hs-apotell, please submit the PR with the updated .gitmodule, .jar saved under Surelog/third_party/antlr_comp/antlr4-4.10.1-complete.jar and the changes to the Cmake to point to that by default, also remove the antlr_fast directory.

[hs-apotell]

Running into number of issues -

• antlr4 binaries available for download require java11 runtime. Previous version needed java8. The two versions are vastly different and there is no jre11 installation available for Windows.
• Cpp runtime on Windows has compile errors.
  https://github.com/antlr/antlr4/blob/ae1bf405f0071ad06dc7fc3105d074aad1130135/runtime/Cpp/runtime/src/ANTLRFileStream.cpp#L17
  There is no function antlrcpp::s2ws. It was deprecated.
• Cpp runtime doesn't support multi-configuration builds because it copies the output to <root>/dist folder. User has to manually go and delete the old library for build to succeed every time configuration is changed. This was fixed in the forked repository.
• Generated lib and pdb files are named differently and debugger doesn't find the symbol file when debugging.

Open blocker Issues on antlr4 repository -
antlr/antlr4#3467
antlr/antlr4#3651
antlr/antlr4#3648

[hzeller]

The antlrcpp::s2ws should now be fixed at head.

[hzeller]

Isn't Java8 EOL ?
Current Version is 18, current LTS is Version 17, and the previous LTS is Version 11. So I think it is reasonable to have them distribute for the oldest available LTS version.

[QuantamHD]

@hs-apotell Java 11+ based apps no longer require a JRE installation. Java 11 based apps are supposed to bundle a java implementation into their binary

https://stackoverflow.com/questions/52584888/how-to-use-jdk-without-jre-in-java-11#:~:text=Oracle%20no%20longer%20intends%20for,bundle%20their%20own%20Java%20implementation.

[hs-apotell]

The antlrcpp::s2ws should now be fixed at head.

Are we pulling in latest or staying with a release commit? I would recommend a release commit but that would mean waiting until the next release.

@hzeller @QuantamHD As for the Java releated concern - it seems that CMake picks Java 8 over Java 11, especially if there is no version specification required or only the Runtime is requested. Will need some time to test this and find a working solution.

There are still two other issues that are not even reported. I will open related bugs and possibly submit a PR too for antlr4 repository.

[hzeller]

I would track head of antlr4. The development stuff happens in their dev branch, so head is essentially the stable release (including minor fixes they added now). So no need to wait for the next release.

Can we CMake tell to use Java11 ? It sounds like a thing that some CMAKE variable will fix.

[hs-apotell]

Can we CMake tell to use Java11 ? It sounds like a thing that some CMAKE variable will fix.

Yes, that's what I was doing or at least I thought I was doing. I modified the CMakeLists.txt and set the recommended JAVA_HOME variable. CMake still wouldn't pick the correct Java version (I have multiple version installed on my system). As it turns out, JAVA_HOME needs to be set as environment variable only. Passing it to CMake as a variable assignment i.e. -D option doesn't work. With that resolved, the below is the only blocker.

antlr/antlr4#3663

Hopefully, this will get merged soon.

[hzeller]

Looks like antlr/antlr4#3663 is merged.

[hs-apotell]

Yes. I am off work this week. Will work on this following Monday.

[hs-apotell]

antlr/antlr4#3663 isn't merged into master yet. Do you want to track a specific commit for now??

[QuantamHD]

We should go with the head of dev

[hs-apotell]

Fixed by #2886