JuliaHubOSS / llvm-cbe

resurrected LLVM "C Backend", with improvements
Other
846 stars 146 forks source link

RFC: LLVM version support policy #60

Closed vtjnash closed 2 weeks ago

vtjnash commented 4 years ago

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 https://github.com/JuliaComputing/llvm-cbe/pull/55#issuecomment-573349601

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 commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

vonj commented 2 years ago

Yes, as far as I can tell. I removed all llvm packages except version 10

10 feb. 2022 kl. 13:02 skrev Andrea @.***>:

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

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

hikari-no-yume commented 2 years ago

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

vonj commented 2 years ago

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 commented 1 year ago

Can I suggest using either tags or branches for versions?

So, we create a llvm-10 tag for commit a80a1b420821634d07d127585faf9910cede2785 (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 commented 1 year ago

@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:

vtjnash commented 1 year ago

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

dpaoliello commented 1 year ago

Plan going forward:

vtjnash commented 2 weeks ago

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

zlfn commented 2 weeks ago

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.