search

JuliaHubOSS / llvm-cbe

resurrected LLVM "C Backend", with improvements
Other
826 stars 141 forks source link

llvm 13 fixes #140

Closed jfm535 closed 3 years ago

jfm535 commented 3 years ago

these fixes allow it to compile under llvm trunk (commit hash bbea64250f65480d787e1c5ff45c4de3ec2dcda8)

hikari-no-yume commented 3 years ago

Hi, thanks for the changes! I assume these may help with LLVM 12 compatibility, too?

It seems these break LLVM 10 compatibility, which is a bit annoying for me personally, since that's the version I'm currently using. But that can be solved one way or another.

jfm535 commented 3 years ago

these changes should help with llvm 12 compatability. And sorry for breaking llvm 10 compatability these were the changes needed to get the code to compile to a usable binary

hikari-no-yume commented 3 years ago

Thanks a lot for the changes to use conditionals!

For the getElementCount/getNumElements thing, it's a bit awkward to use conditionals every time they're used, though. If there's not a method that works on both the older and newer versions, then maybe the best solution is to make a small helper function that will do one or the other for its argument.

jfm535 commented 3 years ago

it should now compile under llvm 10

jfm535 commented 3 years ago

Thanks a lot for the changes to use conditionals!

For the getElementCount/getNumElements thing, it's a bit awkward to use conditionals every time they're used, though. If there's not a method that works on both the older and newer versions, then maybe the best solution is to make a small helper function that will do one or the other for its argument.

I agree with this statement and i will see if i can setup a helper function now

hikari-no-yume commented 3 years ago

Once I fixed a typo, it did build for me again on LLVM 10, and the pytest-based tests still pass for me. Thanks a lot! Overall code changes look good to me now.

I'll give @vtjnash a chance to look at this before clicking the big button.

Note: if merging this, please use the squash option. The commit history would be very messy with a rebase or true merge.

jfm535 commented 3 years ago

sorry for the typo Im suprised my ide let that slip through fixed now

MagneFire commented 3 years ago

Hi, I just tried to compile this with LLVM 12 installed but it seems to fail with the following error:

llvm-cbe/lib/Target/CBackend/CBackend.cpp:2335:85: error: no matching function for call to ‘llvm::MCContext::MCContext(llvm::Triple, const llvm::MCAsmInfo*&, const llvm::MCRegisterInfo*&, std::nullptr_t)’
 2335 |   TCtx = new MCContext(llvm::Triple(TheModule->getTargetTriple()),TAsm, MRI, nullptr);

It seems that this guard https://github.com/JuliaComputingOSS/llvm-cbe/pull/140/files#diff-18ffe28a234c38c6d09cd7a1e882d3bfa7a18faf6d119346b5417d476619461dR2334 is incorrect. Simply changing that to #if LLVM_VERSION_MAJOR > 12 seems to fix compilation with LLVM 12.

jfm535 commented 3 years ago

Hi, I just tried to compile this with LLVM 12 installed but it seems to fail with the following error:

llvm-cbe/lib/Target/CBackend/CBackend.cpp:2335:85: error: no matching function for call to ‘llvm::MCContext::MCContext(llvm::Triple, const llvm::MCAsmInfo*&, const llvm::MCRegisterInfo*&, std::nullptr_t)’
 2335 |   TCtx = new MCContext(llvm::Triple(TheModule->getTargetTriple()),TAsm, MRI, nullptr);

It seems that this guard https://github.com/JuliaComputingOSS/llvm-cbe/pull/140/files#diff-18ffe28a234c38c6d09cd7a1e882d3bfa7a18faf6d119346b5417d476619461dR2334 is incorrect. Simply changing that to #if LLVM_VERSION_MAJOR > 12 seems to fix compilation with LLVM 12.

oh thanks tbh I didnt know where certain parts were version dependent so i mostly guessed but pushed the fix for llvm 12 now

jfm535 commented 3 years ago

squashed a number of commits to cleanup the history