-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable CTRE_FORCE_INLINE when clang or gcc is integrated in MSVC #172
base: main
Are you sure you want to change the base?
Conversation
What about ICC and EDG? |
I'd guess if they're properly integrated it's the same behavior? Is there a way to detect those? |
Are you sure there is no other way how to detect MSVC without negating other compilers? That seems brittle to me. |
Not sure, I honestly just ran into this issue only because I just managed to get clang working for msvc and I only caught it because of debugging the assembly. There may be an easier way, but apparently as is, clang basically ignores __forceinline because it doesn't know what that is, kind of surprised it didn't error out or give a warning. I'm not finding anything other than _MSC_VER being noted as the way to detect the MSVC compiler, but clearly that wasn't quite right. Well, apparently _MSC_VER isn't specific to MSVC, intel's compiler apparently also defines it. Might be that _MSC_VER could be the IDE in general? Could be the MSVC compiler, could be the intel compiler (I don't know if intel's compiler has weird quirks like MSVC or they're defining both). This bit might need to be slightly more complicated than it ought to be. |
I wouldn't necessarily trust this, _MSC_BUILD seems to be different between whether I'm using MSVC specifically, or another compiler, that might be a short-hand way of checking. |
Looks like MSVC might always define |
Intel's compiler appears to use __forceinline? I'm not sure what the EDG compiler wants, I'm guessing since MSVC uses it __forceinline is what it would expect? |
p1007r1 appears to do this: #if defined(__clang__) || defined(__GNUC__)
__attribute__((always_inline))
#elif defined(_MSC_VER)
__forceinline
#endif Which would seem to avoid the weird MVSC issue, but wouldn't handle other compilers the same. Could rewrite like: #if defined(__clang__) || defined(__GNUC__)
__attribute__((always_inline))
#elif defined(_MSC_VER)
__forceinline
#else
__attribute__((always_inline))
#endif To keep everything else the same. |
fa89757
to
4b8647e
Compare
…ntegrated in MSVC
4b8647e
to
9946baf
Compare
Here's a question, is there a particular reason why CTLL_FORCE_INLINE and CTRE_FORCE_INLINE are defined differently besides
Having inline already in the function signature? Seems CTLL_FORCE_INLINE and CTRE_FORCE_INLINE would be the same thing? |
CTLL and CTRE can be used separately. |
Right, I guess my question is about the macro's usage, are CTLL_FORCE_INLINE and CTRE_FORCE_INLINE meant to be different really in any way? It looks as though from the way CTLL_FORCE_INLINE used you're adding an inline before the macro, where for CTRE_FORCE_INLINE you don't? |
They should be same, but they probably are out of sync. |
Ah, ok was worried I was missing something weird about inlining because of how the signature would end up. |
@hanickadot not having luck on finding anything specific about the intel or edg compilers for force inline. Best I've run into is: I found the attribute always_inline mentioned here for intel: Best I can gauge without having access to either is intel and edg's compiler can parse msvc's attribute syntax. EDG reads as though it might ignore / not implement them. Not found __forceinline for eaither, so both might really use: #define CTRE_FORCE_INLINE inline __attribute__((always_inline)) So it might look like: #if defined(__INTEL_COMPILER) || defined(__EDG__) || defined(__clang__) || defined(__GNUC__)
#define CTRE_FORCE_INLINE inline __attribute__((always_inline))
#elif defined(_MSC_VER)
#define CTRE_FORCE_INLINE __forceinline
#else
#define CTRE_FORCE_INLINE inline __attribute__((always_inline))
#endif |
I know this is an old thread, but chiming in here - because _MSC_VER is defined when clang is building in msvc compatibility mode (via clang-cl), the addition of the changes in 9572913 to add
|
oh no :( |
When other compilers are integrated into the MSVC their compiler macros are defined in addition to _MSC_VER. Which confusingly means _MSC_VER only means the MSVC compiler if and only if no other compiler is detected.