[PATCH] D96853: [clang][AVR] Support variable decorator '__flash'

Ben Shi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 4 06:40:06 PDT 2021


benshi001 marked an inline comment as done.
benshi001 added a comment.

In D96853#2661168 <https://reviews.llvm.org/D96853#2661168>, @Anastasia wrote:

> In D96853#2660443 <https://reviews.llvm.org/D96853#2660443>, @benshi001 wrote:
>
>> In D96853#2659266 <https://reviews.llvm.org/D96853#2659266>, @Anastasia wrote:
>>
>>> Btw is any pointer conversion between address space 1 and the default address space allowed? I.e. is the following code valid:
>>>
>>>   __flash static const int var1[] = {111, 222, 333};
>>>   void foo(int*);
>>>   ....
>>>   foo(var1);
>>>
>>> or
>>>
>>>   __flash static const int var1[] = {111, 222, 333};
>>>   void foo(int*);
>>>   ....
>>>   foo((int*)var1);
>>>
>>> ?
>>
>> No. clang denied with `error: passing 'const __attribute__((address_space(1))) int *' to parameter of type 'const int *' changes address space of pointer` while avr-gcc accepted it.
>>
>> It seems I need more efforts to make clang-avr fully compatible with avr-gcc.
>
> Ok, it makes sense for clang to align with gcc in general but do you know whether or where this conversions are described? By default if not specified explicitly implicit conversions are always disallowed in  `ISO/IEC DTR 18037` and explicit casts are always allowed but can yeild undefined behavior if address spaces don't overlap.

I do not think clang needs to fully follow avr-gcc's way. Since avr-gcc generates wrong assembly that c code.

  __flash static const int var1[] = {111, 222, 333};
  void foo(int*);
  ....
  foo(var1);

`LDD` (which is used access data space) is generated in th function `foo`, while `LPM` is needed for accessing `var1` (in the program space).
avr-gcc silently accepts the c code with any warning and generates wrong assembly. And I think clang should rise an error .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96853/new/

https://reviews.llvm.org/D96853



More information about the cfe-commits mailing list