bscarlet / llvm-general

Rich LLVM bindings for Haskell (with transfer of LLVM IR to and from C++, detailed compilation pass control, etc.)
http://hackage.haskell.org/package/llvm-general
132 stars 38 forks source link

add type annotations for references in the llvm-3.2 branch #108

Closed tmcdonell closed 9 years ago

tmcdonell commented 10 years ago

RE: https://github.com/bscarlet/llvm-general/commit/7842578c4fc24aed6534dd723fc655d5d0fa74c5

Can you please apply this change to the llvm-3.2 branch as well. I use libNVVM to generate GPU code from LLVM IR, but currently this still only accepts LLVM 3.2 syntax.

My stuff also generates CPU code, and is thus compatible with the latest versions of llvm-general as well. I'd like to keep compatibility on both sides.

bscarlet commented 10 years ago

Hmm. I'd only been maintaining the latest two llvm releases (currently 3.3 and 3.4). Bringing the 3.2 up to date would be quite a time sink, keeping it up to date more so. The inertia of NVIDIA aside, two releases seemed a reasonable limit.

I'm curious you say the 3.2 "syntax". Do you need to link against the old version, or pass it binary IR or, as your comment suggests, pass it text IR?

tmcdonell commented 10 years ago

I can deal with the APIs being a little different (and currently do), because that just happens at the boundary --- lowering the LLVM from the pure AST into IR, etc. This change affects the shared code of actually generating the LLVM, which is shared between the CPU and GPU versions, hence it is a bit more problematic for me.

The NVIDIA library can take the IR as either binary or text representation. However, if I give it something generated by an LLVM tool version 3.3 or above --- as either binary or text --- it fails. When I say "syntax", this is perhaps not the right term, but here's an example: using the text representation, it doesn't like the new way that attributes are handled, e.g.:

  %6 = tail call float @llvm.cos.f32(float %5) #1
  ...
attributes #0 = { nounwind }

It wants the old style:

  %6 = tail call float @llvm.cos.f32(float %5) nounwind

Ideally NVIDIA would bring their tools up to speed as well, but I don't think we can do anything about that ourselves.

bscarlet commented 10 years ago

I'll leave this request open rather than reject it, but there are many other tasks I need to handle before I can take on resuscitating the 3.2 branch, so it'll be while. I don't want to cherry-pick just this one change (lest the relationship of this branch to the other become hopelessly tangled); so the work required is to go back to the point I stopped keeping the branches in sync and cherry-pick each change after that from the master, adapting each change to the slightly-different 3.2 world, and making sure the tests pass at each commit.

tmcdonell commented 10 years ago

Ah, I didn't intend to make you resurrect the entire branch, but I can see your argument as to why this is a better approach than just cherry picking this one patch.

Thanks for taking your time to do this properly. In the mean time I'll continue trying to work around the differences in whatever way I can.

bscarlet commented 9 years ago

According to http://docs.nvidia.com/cuda/nvvm-ir-spec/ The current version of NVVM is based on llvm-3.4.

Thank you for your patience. I'll keep this experience in mind when deciding when to obsolete more versions.

tmcdonell commented 9 years ago

Ah, looks NVIDIA have finally updated their tools. Good catch, and thanks for considering this.