-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add clang to compiler.h #2397
base: master
Are you sure you want to change the base?
Add clang to compiler.h #2397
Conversation
src/common/tusb_compiler.h
Outdated
@@ -175,6 +175,11 @@ | |||
#pragma GCC poison tud_vendor_control_request_cb | |||
#endif | |||
|
|||
#if defined (__clang__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you do the #ifdef #else instead of undef (ust move this up). undef is rather confusing since people may only look at the first define.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify @hathach
Like this?
#if defined(__GNUC__) || defined (__clang__)
#define TU_ATTR_ALIGNED(Bytes) __attribute__ ((aligned(Bytes)))
#define TU_ATTR_SECTION(sec_name) __attribute__ ((section(#sec_name)))
#define TU_ATTR_PACKED __attribute__ ((packed))
#if defined(__GNUC__)
#define TU_ATTR_WEAK __attribute__ ((weak))
#else
#define TU_ATTR_WEAK __attribute__ ((weak_import))
#endif
...
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, but the otherway around
#if defined(__clang__)
#define TU_ATTR_WEAK __attribute__ ((weak_import))
#else
#define TU_ATTR_WEAK __attribute__ ((weak))
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All fixed :)
Not sure why pre-commit CI is failing 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit test probably compile using clang, and has affected by this attribute. I will check later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still haven't got time to test this out, will be off for a couple of week for TET holidays. Please be patient, thank you.
@Ryzee119 I am back from TET holidays, which compiler you are using. If it is arm clang, I could give it a try (was planning to support it anwyay). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recently I switch to use weak as the default implementation of callback to be compatible with keil #2239
https://github.com/hathach/tinyusb/blob/master/src/device/usbd.c#L48
the other callback header can work with weak_import, but this style seems to require actual weak and cause the compile issue with unit-test (since test include mocking of dcd layer).
Maybe I should use weakref/alias or unify them all to default implemenation, will revise later. We probably need to keep this open until then. Thank you.
How it goes with #2606 ? |
so far I only tested arm-clang with cdc_msc example, it doesn't seem to need weak_import yet, but I admit we need more testinng. the conlfict with unit-test is resolved (unit test switch back to gcc) |
So sorry i missed this discussion somehow! I'm using clang + llvm cross compiling for x86 type embedded system with OHCI backend.
^ Yep this does the trick for me. I was having problems with the OHCI tusb_app_virt_to_phys/tusb_app_phys_to_virt weak functions. So I just need this patch and implement weak stub functions.
Ill probably supersede this PR. I need to stick these two stubs somewhere for my needs. Can you suggest a good location?
|
Describe the PR
I'm using the clang compiler which for our purposes the compiler attributes match GCC from what I can read.
Although
TU_ATTR_WEAK
had to be changed to__attribute__ ((weak_import))
or I was gettinglld: error: duplicate symbol
errors on all theTU_ATTR_WEAK
functions.