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

lib: fix compile error #312

Merged
merged 1 commit into from
Oct 21, 2024
Merged

lib: fix compile error #312

merged 1 commit into from
Oct 21, 2024

Conversation

wyr-7
Copy link
Contributor

@wyr-7 wyr-7 commented Oct 10, 2024

lib/errno.h: defined(__ARMCC_VERSION) for use GCC compile
lib/autmoic:fix compiler error:
nuttx/include/metal/compiler/gcc/atomic.h:19:13: error: conflicting type qualifiers for 'atomic_flag'
19 | typedef int atomic_flag;
| ^~~~~~~~~~~
In file included from nuttx/include/nuttx/net/netdev_lowerhalf.h:38,
from virtio/virtio-net.c:33:
prebuilts/gcc/linux/arm/lib/gcc/arm-none-eabi/13.2.1/include/
stdatomic.h:233:3: note: previous declaration of 'atomic_flag' with
type 'atomic_flag'
233 | } atomic_flag;
| ^~~~~~~~~~~
nuttx/include/metal/compiler/gcc/atomic.h:20:14: error: conflicting
type qualifiers for 'atomic_char'
20 | typedef char atomic_char;
^~~~~~~~~~~

@arnopo
Copy link
Contributor

arnopo commented Oct 16, 2024

Please detail the issue you are facing. Your commit partially reverts 9855f84.

@arnopo
Copy link
Contributor

arnopo commented Oct 16, 2024

I have conducted further investigation.
The 9855f84 t tried to address the removal of __CC_ARM from armcc v5 to armcc v6 in a wrong way

Therefore, we need to rework the fix introduce by commit 9855f84.

First, could you close #314 and address it here ?

Then it seems that we can use __ARMCC_VERSION to detect armcc compiler (v5 and V6)

Could you replace
if defined(__CC_ARM) || defined(__arm__)
by
if defined(__ARMCC_VERSION)

And verify that it works in you case?
I will then test it with the armcc compiler

@arnopo
Copy link
Contributor

arnopo commented Oct 17, 2024

@wyr-7 It will be nice to address it for this release as it fix a bug.
the code freeze is tomorrow evening do you think you can send an update or should i do it?

@CV-Bowen
Copy link
Contributor

@arnopo We will update this PR today evening (Your Time).

@wyr-7 wyr-7 changed the title lib/errno.h: fix compile error lib: fix compile error Oct 18, 2024
@wyr-7
Copy link
Contributor Author

wyr-7 commented Oct 18, 2024

@wyr-7 It will be nice to address it for this release as it fix a bug. the code freeze is tomorrow evening do you think you can send an update or should i do it?

@arnopo Ok,thank you. I have send an update.

Copy link
Contributor

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

one comment + Please, could you also update lib/error.h (see 9855f84#diff-aa1a7db6b5986ae989644bcaefc8a3076f07aa97be4d7176ec93f788b9c21ea2L17)

lib/atomic.h Outdated
#elif defined(HAVE_STDATOMIC_H) && !defined(__CC_ARM) && !defined(__arm__) && \
!defined(__STDC_NO_ATOMICS__)
#elif defined(HAVE_STDATOMIC_H) && !defined(__STDC_NO_ATOMICS__) && \
(!defined(__ARMCC_VERSION) || defined(__GNUC__))
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to add the || defined(GNUC) ?
I would expect that you only replace!defined(__CC_ARM) && !defined(__arm__)by !defined(__ARMCC_VERSION)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arnopo It was considered that other compilers did not meet this condition, but now it seems that it is not necessary, I have removed it.

@arnopo
Copy link
Contributor

arnopo commented Oct 18, 2024

Please, could you also update lib/error.h (see 9855f84#diff-aa1a7db6b5986ae989644bcaefc8a3076f07aa97be4d7176ec93f788b9c21ea2L17)

could you also address this remaing issue?

lib/errno.h: defined(__ARMCC_VERSION) for use GCC compile
lib/autmoic:fix compiler error:
nuttx/include/metal/compiler/gcc/atomic.h:19:13: error: conflicting type
qualifiers for 'atomic_flag'
19 | typedef int atomic_flag;
| ^~~~~~~~~~~
In file included from nuttx/include/nuttx/net/netdev_lowerhalf.h:38,
from virtio/virtio-net.c:33:
prebuilts/gcc/linux/arm/lib/gcc/arm-none-eabi/13.2.1/include/
stdatomic.h:233:3: note: previous declaration of 'atomic_flag' with
type 'atomic_flag'
233 | } atomic_flag;
| ^~~~~~~~~~~
nuttx/include/metal/compiler/gcc/atomic.h:20:14: error: conflicting
type qualifiers for 'atomic_char'
20 | typedef char atomic_char;
^~~~~~~~~~~

Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
@wyr-7
Copy link
Contributor Author

wyr-7 commented Oct 18, 2024

Please, could you also update lib/error.h (see 9855f84#diff-aa1a7db6b5986ae989644bcaefc8a3076f07aa97be4d7176ec93f788b9c21ea2L17)

could you also address this remaing issue?

@arnopo Ok, I have update lib/compiler.h too.

Copy link
Contributor

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

I will need to test it with armcc before merging it but look got o go for this release

Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@wyr-7
Copy link
Contributor Author

wyr-7 commented Oct 21, 2024

I will need to test it with armcc before merging it but look got o go for this release

Ok, thanks.

@arnopo
Copy link
Contributor

arnopo commented Oct 21, 2024

Tested with success on Keil with arm compiler 6.21

@arnopo arnopo merged commit 7f351a5 into OpenAMP:main Oct 21, 2024
6 checks passed
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.

5 participants