Skip to content
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

Support APPLE and LINUX platforms in CPP Preprocessing #3229

Merged
merged 2 commits into from Jan 26, 2024

Conversation

Shaikh-Ubaid
Copy link
Member

fixes #3217

@Shaikh-Ubaid
Copy link
Member Author

Output of the added test on MacOS:

% lfortran integration_tests/cpp_pre_03.f90 --cpp
OSX
Ok

@Shaikh-Ubaid
Copy link
Member Author

Also see #3217 (comment).

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as ready for review January 26, 2024 20:46
} else if (compiler_options.platform == Platform::macOS_ARM
|| compiler_options.platform == Platform::macOS_Intel) {
md.expansion = "1";
macro_definitions["__APPLE__"] = md;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add other C++ defines, here is our relevant code:

Platform get_platform()
{
#if defined(_WIN32)                                           
    return Platform::Windows;
#elif defined(__APPLE__)                                               
#    ifdef __aarch64__       
    return Platform::macOS_ARM;
#    else                                          
    return Platform::macOS_Intel;  
#    endif
#elif defined(__FreeBSD__)      
    return Platform::FreeBSD;
#elif defined(__OpenBSD__)            
    return Platform::OpenBSD;     
#else
    return Platform::Linux;                 
#endif          
}

So let's add:

__FreeBSD__
__OpenBSD__

and also platform defines:

__aarch64__
__x86_64__

That should cover most cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Added.

% cat integration_tests/cpp_pre_03.f90 
program cpp_pre_03
#ifdef _WIN32
    print *,"Windows"
#endif
#ifdef __linux__
    print *,"Linux"
#endif
#ifdef __APPLE__
    print *,"OSX"
#endif
#ifdef __aarch64__
    print *,"Apple ARM"
#endif
#ifdef __x86_64__
    print *,"Apple AMD"
#endif
#ifdef __FreeBSD__
    print *,"FreeBSD"
#endif
#ifdef __OpenBSD__
    print *,"OpenBSD"
#endif
print *, "Ok"
end program
% gfortran integration_tests/cpp_pre_03.f90 -cpp
% ./a.out 
 Ok
% lfortran integration_tests/cpp_pre_03.f90 --cpp
OSX
Apple ARM
Ok

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this is good. We'll have to eventually revamp how we handle the architecture / platform "triplet", since linux can also be "arm", and so on. But for now this is good to get us started.

Thanks!

@Shaikh-Ubaid Shaikh-Ubaid merged commit d809161 into lfortran:main Jan 26, 2024
21 checks passed
@Shaikh-Ubaid Shaikh-Ubaid deleted the support_platform_in_cpp branch January 26, 2024 21:25
macro_definitions["__aarch64__"] = md;
} else {
md.expansion = "1";
macro_definitions["__x86_64__"] = md;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to define this on Linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CPP: create platform defines
3 participants