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

Improvement for 64bit Windows port #1011

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Conversation

watsk
Copy link
Contributor

@watsk watsk commented Mar 9, 2024

Improvement for 64bit Windows port

Description

  1. 64bit TickType_t is supported. (MSVC and MinGW)
    The case configTICK_TYPE_WIDTH_IN_BITS == TICK_TYPE_WIDTH_64_BITS is added in portmacro.h.
    It is straightforward extension from 16bit and 32bit.
    This change is based on the following discussion on the FreeRTOS forum.
    https://forums.freertos.org/t/64bit-application-on-freertos-windows-port/19110

  2. Unnecessary compiler warning is disabled locally. (MinGW-w64 only)
    #pragma for ignoring the warning cast-function-type is added in port.c.
    MinGW-w64 reports this warning for the for the cast operation from TaskFunction_t to LPTHREAD_START_ROUTINE.
    This warning should be ignored because it is unavoidable. TaskFunction_t is an essential type in FreeRTOS and LPTHREAD_START_ROUTINE is an essential type in win32api.
    This pragma option affects only one line. Other part is not affected.

Test Steps

Define configTICK_TYPE_WIDTH_IN_BITS as TICK_TYPE_WIDTH_64_BITS in FreeRTOSConfig.h and build FreeRTOS by MinGW-w64 or x64 platform on MSVC(Visual Studio).

Regression tests are performed.
All combinations of tick type width and compiler type have been tested.

  1. TICK_TYPE_WIDTH_32_BITS on MinGW(32bit) and Win32 on MSVC
  2. TICK_TYPE_WIDTH_64_BITS on MinGW(32bit) and Win32 on MSVC
  3. TICK_TYPE_WIDTH_32_BITS on MinGW-w64 and x64 on MSVC
  4. TICK_TYPE_WIDTH_64_BITS on MinGW-w64 and x64 on MSVC

MingW Demo: CodeCoverage configuration runs successfully in all cases. No error is reported in the console. No regression is found on coverage score.
MSVC Demo: Demo application runs successfully. No error is reported in the console.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Especially it is introduced for 64bit compiler.(x64 platform on MSVC and MinGW-w64)
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.00%. Comparing base (4732b96) to head (cb85579).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1011   +/-   ##
=======================================
  Coverage   93.00%   93.00%           
=======================================
  Files           6        6           
  Lines        3200     3200           
  Branches      879      879           
=======================================
  Hits         2976     2976           
  Misses        111      111           
  Partials      113      113           
Flag Coverage Δ
unittests 93.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chinglee-iot
Copy link
Member

@watsk
Thank you for creating this PR. I will look into it and discuss with you.

@chinglee-iot chinglee-iot self-requested a review March 12, 2024 05:13
@watsk
Copy link
Contributor Author

watsk commented Mar 12, 2024

@chinglee-iot
Thank you for reviewing this PR. If you have any question, please let me know.

chinglee-iot
chinglee-iot previously approved these changes Mar 15, 2024
Copy link
Member

@chinglee-iot chinglee-iot left a comment

Choose a reason for hiding this comment

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

Thank you for creating this PR. The information is very clear to me.

@@ -246,8 +246,19 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack,
FALSE, /* Start not signalled. */
NULL ); /* No name. */


#ifdef __x86_64__
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed only on x64 and not on Win32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aggarg
Thank you for pointing out. You are correct. Win32 is also needed.

I added #pragma only for x64 because I observed that MinGW32 does not report warning and I thought it is x64 unique.
But I found that MinGW32 version on my PC is too old and the latest MinGW32 reports same warning as MinGW64.
I modified __x86_64__ to __GNUC__.
(I didn't delete #ifdef because MSVC does not recognize this pragma directive.)

Could you review modification?
I tested again all combinations of TickType_t width and compiler width on MinGW and MSVC.
No problem was found.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for your contribution. Looks good!

…tion.

Before modification: Compiler warning was ignored only on MinGW64
After modification: Compiler warning is ignored on MinGW32 and MinGW64
Reason of modification: The cast warning here is unavoidable not only on MinGW64 but also on MinGW32.
"__GNUC__" macro is used because MSVC does not recognize this #pragma directive.
Copy link

sonarcloud bot commented Mar 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@chinglee-iot chinglee-iot merged commit 6dcce92 into FreeRTOS:main Mar 18, 2024
18 checks passed
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
* Add IPv6 Demo (FreeRTOS#937)

* Add demo changes

* Update kernel and library paths

* Update main.c

* Run uncrustify

* Fix spell checker

* CI check file headers update

* Add IPv6/v4 UDP echo server with zero copy/non-zero copy versions

* Add VS proj file changes to include the UDP echo sample code

* readme update

---------

Co-authored-by: Tony Josi <tonyjosi@amazon.com>

* Update Backward Compatibility Flag (FreeRTOS#954)

* Update Backward Compatibility Flag

* Update FreeRTOS_GetUDPPayloadBuffer_ByIPType

* Update FreeRTOS_IPStart to FreeRTOS_IPInit_Multi

* Update Application APIs

* Remove ipconfigCOMPATIBLE_WITH_SINGLE

* Update Static Lib files (FreeRTOS#956)

* Update Static Lib files

* making vApplicationIPNetworkEventHook backward compatible in demos

* Update CI check file headers

---------

Co-authored-by: Tony Josi <tonyjosi@amazon.com>

* Add WinPCap NetworkInterface Changes (FreeRTOS#958)

* Update winpcap network interface

* Run uncrustify

* Update function to include NetworkInterface_t parameters

* Adding compatibility for xApplicationDNSQueryHook with latest dev branch for old demos (FreeRTOS#957)

* adding compatibility for xApplicationDNSQueryHook with latest dev branch

* adding tcp echo server source

* removing unused sub demos

* fix build issues (FreeRTOS#969)

* Update demo to latest +TCP dev/IPv6_integration (FreeRTOS#978)

* remove macro namings

* rename sin_addr to sin_address.ulIP_IPv4 for ipv6 demo

* replace in6addr_any with FreeRTOS_in6addr_any

* replace mainCREATE_UDP_ECHO_SERVER_TASK with mainCREATE_UDP_ECHO_TASKS_SINGLE

* handle removal of sin_addr macro to sin_address.ulIP_IPv4

* updating +TCP repo to latest dev/IPv6_integration

* minor update to more clear code

* more sin_addr to sin_address.ulIP_IPv4 replacements

* fix makefiles for qemu and posix demos

* review feedback changes

* Update FreeRTOS-Plus-TCP for RC2

* Change from PR (FreeRTOS#994)

* Update FreeRTOS-Plus-TCP for RC2

* Update copyright

* Ignore WinPCap for files header check failure.

* Update checker

* Update manifest

* Point manifest to latest commit

* Fix Spell-checker

* Update doxygen

* Update xApplicationDHCPHook for backward compatibility  (FreeRTOS#999)

* Update xApplicationDHCPHook for backward compatability

* Update IPv6

* Update VisualStudio Static Project files

* Update pxEndPoint error (FreeRTOS#1002)

* Update IPv6 demo ReadMe (FreeRTOS#1004)

* Update ReadMe

* Update setup requirement

* Update UDP demo info

* Update comment

* TCP demo changes post build separation (FreeRTOS#1011)

* adding sin_family to dest adddr for FreeRTOS_sendto

* updating FreeRTOS_bind to input sin_family post build separation changes

* updating FreeRTOS_connect to input sin_family post build separation changes

* minor fix

* updating copyright year

* updating file headers

* updating +TCP submodule

* updating file headers

* updating file headers

* updating manifest file to have latest +TCP submodule hash

* Fix issue with posix demo while running with ipconfigIPv4_BACKWARD_COMPATIBLE enabled for +TCP stack (FreeRTOS#1027)

* Update the submodule pointer to IPv6 main

* Update manifest with latest TCP commit

* Update file checker exception

* Ignore Visual studio project file from file header checker

---------

Co-authored-by: Tony Josi <tonyjosi@amazon.com>
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.

3 participants