Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: LLVM version support policy #60

Closed
vtjnash opened this issue Jan 12, 2020 · 14 comments · Fixed by #209
Closed

RFC: LLVM version support policy #60

vtjnash opened this issue Jan 12, 2020 · 14 comments · Fixed by #209

Comments

@vtjnash
Copy link
Member

vtjnash commented Jan 12, 2020

Please don't break prior LLVM versions in the process (talking about that second commit, perhaps using an #ifdef there?).

Originally posted by @woachk in #55 (comment)

My unannounced personal policy (and thus effectively the de facto repo policy as primary maintainer) has been to only support the latest released version of LLVM on head of repo here. My rationale is that supporting multiple versions is additional work for the rare contributor to a repository that already sees little enough activity, and usually just drive-by PRs (in my impression). Additionally, since there's very few changes other than maintaining version compatibility, anyone using an older version can just checkout the last commit to work on that branch and be pretty well content. If someone had a strong interest in maintaining one version for longer, I'd happily create a branch for them—this has not come up yet in practice however (or, to be more accurate, I haven't received communication of such an interest).

I am proposing here making this an official documented policy (with mention on the README page), but first wanted to leave this open for RFC for an extended comment period, as it's not solely my decision—the ad hoc community should ultimately decide. But even as an ultra-low-activity repo, I think it's important to be up-front about expectations.

@woachk
Copy link
Contributor

woachk commented Jan 13, 2020

Even for a full-blown obfuscator, the API is stable enough nowadays that just #if LLVM_MAJOR_VERSION > X works well for example. (with being able to use the system LLVM, that actually has a purpose too)

@vonj
Copy link

vonj commented Feb 10, 2022

It might be a good idea to just decide this and write in the README exactly what version you support. (Or just say latest, but it would be good if you wrote what version you develop and test on, in case this project ever lags.)

I tried it with the LLVM in Ubuntu 20.04, and I get this error:

user@server:~/src/llvm-cbe/test/selectionsort$ ~/src/llvm-cbe/build/tools/llvm-cbe/llvm-cbe main.ll
/home/jakov/src/llvm-cbe/build/tools/llvm-cbe/llvm-cbe: main.ll:36:3: error: instruction expected to be numbered '%12'
  %13 = load i32, i32* %4, align 4, !dbg !33
  ^

this was on the main.c, compiled with:

clang -S -emit-llvm -g main.c

clang version 10.0.0-4ubuntu1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

@hikari-no-yume
Copy link
Collaborator

@vonj LLVM can't consume LLVM IR produced by newer versions of LLVM: you need to have compiled llvm-cbe with the same or a newer version of LLVM as Clang was compiled with. (This is a very common issue, so maybe I should write a FAQ…)

@vonj
Copy link

vonj commented Feb 10, 2022

Oh, my bad. I made sure I have only LLVM 10 installed (I had 8 installed too by mistake which got picked up by CMake)

Now a unit test runs fine (make CBEUnitTests && unittests/CWriterTest), but I still can't get a simple hello world to run:

clang -S -emit-llvm -g main.c
llvm-cbe  main.ll
/home/jakov/src/llvm-cbe/build/tools/llvm-cbe/llvm-cbe: main.ll:7:32: error: expected ')' at end of argument list
define dso_local i32 @main(i32 %0, i8** %1) #0 !dbg !7 {
                               ^

@hikari-no-yume
Copy link
Collaborator

Are you sure your clang version is not newer than the LLVM version you built llvm-cbe with?

@vonj
Copy link

vonj commented Feb 10, 2022 via email

@hikari-no-yume
Copy link
Collaborator

Try compiling to a bitcode (.bc) file instead of text IR (.ll) perhaps?

@vonj
Copy link

vonj commented Feb 12, 2022

I actually had remnants of LLVM-8 some more places. Now everything is LLVM-10, and llvm-cbe works, at least for trivial compilations. I have yet to figure out if I can use it for something substantial.

@dpaoliello
Copy link
Collaborator

Can I suggest using either tags or branches for versions?

So, we create a llvm-10 tag for commit a80a1b4 (i.e., just prior to the LLVM 16 change), then strip out all the #if checks, update the README to say that LLVM 16 is the only supported version and verify that in the CMake file. We can then either create a llvm-16 tag and keep moving it forward as we make changes OR create that tag just before merging in the change to push the project to LLVM 17 (when the time comes to that).

@dpaoliello
Copy link
Collaborator

@vtjnash I'd like there to be a decision on this before I complete the work with opaque pointers (#154) - I'm working through the last of the test failures but there's been quite a bit of churn in the code. Before I submit a PR, I'd like there to be a decision so that I can do some work in preparation:

  • If we do support multiple LLVM versions, then I'll extend the CI build to also test LLVM 10.
  • If we don't support multiple versions, then I'll like to remove all of the #if declarations.

@vtjnash
Copy link
Member Author

vtjnash commented Jul 18, 2023

I don't see any value in supporting an old LLVM version

@dpaoliello
Copy link
Collaborator

Plan going forward:

  • Only support a single LLVM version.
  • Each time we update the supported version create a new tag with the last commit that supported the old LLVM version.

@vtjnash
Copy link
Member Author

vtjnash commented Oct 14, 2024

Reopen as I don't think the primary item for closing it (documenting this in the readme) is done yet

@zlfn
Copy link
Contributor

zlfn commented Oct 14, 2024

In my opinion, this project should have two branches.

  1. Branch that adds bug fixes and features for the current LLVM version (LLVM-17 for now)
  2. Branch that adds features for the next LLVM version (LLVM-18 for now)

Adding of instructions that exist in LLVM-17 such as llvm.is.fpclass but are not implemented are managed in (1),
Adding of instructions and features added in LLVM-18 such as or disjoint are managed in (2).

If we determine that the codes of (2) are ready, create a LLVM-17 tag and merge the two branches.
After that, create LLVM-19 branch, continue this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants