Details
Diff Detail
Event Timeline
LGTM with a (possibly unnecessary) nit
MultiSource/Applications/minisat/Main.cpp | ||
---|---|---|
79 | You're probably correct here but I don't know much about the non-linux targets that glibc supports (e.g. Hurd). I'd err on the side of caution and test for both __linux__ and __GLIBC__. |
Hi James,
I didn't see your comment in phabricator and I accidentally committed this. Do you want me to revert it?
I was going to say that we use LNT in order to run the LLVM test-suite, and given that CMake isn't supported at the moment, I'd like to have it committed.
Also, I'd like to use CMake with LNT and it's my intention to send any patch that'll allows us to do that with Musl, when the relevant review requests land.
Thanks,
Vasileios
From: James Molloy [james@jamesmolloy.co.uk]
Sent: 10 November 2015 15:15
To: reviews+D13935+public+6985f52afb85db86@reviews.llvm.org; Daniel Sanders; Vasileios Kalintiris
Cc: llvm-commits@lists.llvm.org
Subject: Re: [PATCH] D13935: [minisat] Include the fpu_control.h header only when the GLIBC macro is defined.
Hi,
I'd prefer if we used CMake to check for the existence of the fpu_control.h header - we have a proper configure system now, let's use it!
#if defined(GLIBC) || HAVE_FPU_CONTROL_H , or something?
Cheers,
James
You're probably correct here but I don't know much about the non-linux targets that glibc supports (e.g. Hurd). I'd err on the side of caution and test for both __linux__ and __GLIBC__.