ian-ross / arb-fft

Pure Haskell arbitrary length FFT library
https://www.skybluetrades.net/haskell-fft-index.html
BSD 3-Clause "New" or "Revised" License
11 stars 2 forks source link

setup.hs doesn't correctly test for llvm being installed #5

Closed cartazio closed 10 years ago

cartazio commented 10 years ago

i've got llvm installed as opt-3.4 llvm-config-3.4 and llc-3.4 and my ghc settings file points to them appropriately. A better way to test for llvm support is to shell out and compile a toy hs module with -fllvm and check the error code response.

i hit this in the process of trying to repro alex's segfault issue

carter repoScratcher/arb-fft-0.2.0.2 » cabal configure --enable-tests --ghc-options="-optlo-O3 -fllvm"
Resolving dependencies...
Warning: LLVM is not installed.
Building arb-fft with native code generator (slower).
Configuring arb-fft-0.2.0.2...
cartazio commented 10 years ago

also its really bad to override user supplied flags! (unless you're actually always setting them correclty and they're not)

mind you when i say "bad" i mean duncan coutts will be sad and need to explain the errors of your ways : )

ian-ross commented 10 years ago

I'm quite prepared to believe the approach I've taken for testing for LLVM isn't right. And I guess Setup.hs should just crap out with an error if the LLVM flag is set and there's no LLVM installed.

ian-ross commented 10 years ago

I don't think I agree with your comment that "A better way to test for llvm support is to shell out and compile a toy hs module with -fllvm and check the error code response". That's what I would do if I was using autotools. But I would hope that there's a better way of doing it within Cabal. (Also, avoiding that sort of nastiness is one of the reasons that things like LLVM come with pkg-config tools like llvm-config!)

One other thing: I didn't realise before what you meant when you said your "GHC settings file". I want to have a look at some other packages to see how well they support "weird GHC developer setups" (just kidding). Are the values stored in that settings file accessible in any way via Cabal's Distribution.... API?

cartazio commented 10 years ago

LLVM-General does it in a way you'll like even less

https://github.com/bscarlet/llvm-general/blob/master/llvm-general/Setup.hs#L27-L30

llvmConfigNames = [
  "llvm-config-" ++ (intercalate "." . map show . versionBranch $ llvmVersion),
  "llvm-config"
 ]

and unlike llvm-general, you have no good reason to mandate a fixed llvm version right now.

honestly, the sanest approach might be to NOT auto set -fllvm, but instead, issue a warning during setup like "you seem to be building arb-fft using -fasm rather than -fllvm, current benchmarks indicate better performance with -fllvm"

cartazio commented 10 years ago

the way I look up where the settings file is is I do

ghc-pkg list ; then the first line of output is something like /usr/local/lib/ghc-7.8.20140130/package.conf.d then cd .. then cat settings.

If you don't want to do an icky hack, throw a warning if arb-fft isn't being built with -fllvm (and if benchmarks validate that -fllvm yields better perf), don't get clever with doing a simple but fragile setup.hs hack :)

ian-ross commented 10 years ago

Yeah, I've seen that llvm-general code. It made me shudder.

I think you're probably right. I was trying to be a bit too clever, but it seems like a simple and obvious warning if LLVM isn't being used is the best thing. I'll do that.

cartazio commented 10 years ago

glad I could help! And thanks for being super responsive :)

cartazio commented 10 years ago

note that the warning should only happen as a pre/post configure hook, not during builds, I think

ian-ross commented 10 years ago

Yep. Definitely.