Menu
▾
▴
avr-llvm-devel — Developers mailing list.
You can subscribe to this list here.
| 2009 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
(26) |
Dec
(15) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2010 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
(2) |
Jul
|
Aug
|
Sep
(14) |
Oct
(16) |
Nov
(36) |
Dec
(3) |
| 2011 |
Jan
|
Feb
|
Mar
(1) |
Apr
(17) |
May
(9) |
Jun
(6) |
Jul
|
Aug
|
Sep
|
Oct
(1) |
Nov
(2) |
Dec
(4) |
| 2012 |
Jan
(22) |
Feb
(12) |
Mar
(39) |
Apr
(31) |
May
(42) |
Jun
(35) |
Jul
(32) |
Aug
(2) |
Sep
(5) |
Oct
|
Nov
|
Dec
(9) |
| 2013 |
Jan
|
Feb
|
Mar
|
Apr
(2) |
May
|
Jun
(121) |
Jul
(61) |
Aug
(7) |
Sep
(8) |
Oct
(6) |
Nov
|
Dec
(1) |
| 2014 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
(1) |
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
| 2015 |
Jan
(1) |
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Borja F. <bor...@gm...> - 2012-03-20 00:27:17
|
Ok I've just commited all patches and code related to this. Please try them out and let me know any issues. John you'll notice some patches duplicate some code compared to the interrupt.diff file because i added some code to the same files like that patch, you may want to fix this, I didnt know what to do. 2012/3/19 Borja Ferrer <bor...@gm...> > Indeed __flash is a type qualifier just like const or volatile, the syntax > you're all writing above is how it works. You can have an insane chain of > pointers like __flash const int * const __flash * * const __flash * __flash > ptr; and it's totally valid. The issue I'm talking about is, should __flash > qualify a variable with a const aswell? As we discussed above i think it > shouldn't. > > Anton, what you said in the last email is exactly what I think, as __flash > is a type qualifier, check for constness and add errors if the user doesn't > explicitly add them. But after all the usage is what you're all writing > above. I like the idea of adding const because it really models how flash > memory works, and as i said making __flash imply const is like cheating on > the C language. But well, we're all here to discuss about it so it's not a > closed decision in any sense. > > John thanks for the strings, I'll add them to my patch. If you want, once > it is commited you can add further changes or add any additional hints. I > still have to prepare the patch because I was waiting for this, I will > split it in 2 parts, one for the clang side and another for llvm. > > Oh and if anybody else can think of any additional checks let me know, we > should be here as robust as possible. > > > 2012/3/19 John Myers <ato...@gm...> > >> >> 1) Cannot write to flash memory. <-- emitted when trying to assign a >>> value to a flash var >>> >> The error for a const being assigned a value is "error: read-only >> variable is not assignable" so I would make the wording consistent and just >> do... >> >> "error: Flash variable is not assignable" ...or... >> "error: __flash qualified variable is not assignable" >> >> >>> 2) Flash variables are read only and should be declared with a const >>> qualifier. <-- emitted when doing something like __flash int, we want >>> __flash const int >>> >> >> Something like the below and then also have one of those clang fix-it >> hints showing const being added. >> >> "error: Flash variable requires const qualifier" >> >> 3) The pointee type of a flash pointer should be declared with a const >>> qualifier. <-- i cant think of a better way of saying it, this is emitted >>> when doing __flash int* ptr, we want __flash const int *ptr. >>> >> >> I think this can be the same as or similar to #2 and again use the fix-it >> hint to show explicitly where the const needs to be placed. >> > > |
|
From: Borja F. <bor...@gm...> - 2012-03-19 18:10:58
|
Indeed __flash is a type qualifier just like const or volatile, the syntax you're all writing above is how it works. You can have an insane chain of pointers like __flash const int * const __flash * * const __flash * __flash ptr; and it's totally valid. The issue I'm talking about is, should __flash qualify a variable with a const aswell? As we discussed above i think it shouldn't. Anton, what you said in the last email is exactly what I think, as __flash is a type qualifier, check for constness and add errors if the user doesn't explicitly add them. But after all the usage is what you're all writing above. I like the idea of adding const because it really models how flash memory works, and as i said making __flash imply const is like cheating on the C language. But well, we're all here to discuss about it so it's not a closed decision in any sense. John thanks for the strings, I'll add them to my patch. If you want, once it is commited you can add further changes or add any additional hints. I still have to prepare the patch because I was waiting for this, I will split it in 2 parts, one for the clang side and another for llvm. Oh and if anybody else can think of any additional checks let me know, we should be here as robust as possible. 2012/3/19 John Myers <ato...@gm...> > > 1) Cannot write to flash memory. <-- emitted when trying to assign a value >> to a flash var >> > The error for a const being assigned a value is "error: read-only variable > is not assignable" so I would make the wording consistent and just do... > > "error: Flash variable is not assignable" ...or... > "error: __flash qualified variable is not assignable" > > >> 2) Flash variables are read only and should be declared with a const >> qualifier. <-- emitted when doing something like __flash int, we want >> __flash const int >> > > Something like the below and then also have one of those clang fix-it > hints showing const being added. > > "error: Flash variable requires const qualifier" > > 3) The pointee type of a flash pointer should be declared with a const >> qualifier. <-- i cant think of a better way of saying it, this is emitted >> when doing __flash int* ptr, we want __flash const int *ptr. >> > > I think this can be the same as or similar to #2 and again use the fix-it > hint to show explicitly where the const needs to be placed. > |
|
From: John M. <ato...@gm...> - 2012-03-19 04:34:33
|
After looking at the Clang code some it looks like the address space __attribute__ *does* cause a type qualifier for the address space to be set. On Sun, Mar 18, 2012 at 1:47 PM, Anton Staaf <an...@so...> wrote: > On Sun, Mar 18, 2012 at 12:34 PM, John Myers <ato...@gm...> > wrote: > > I share Anton's concern about __flash not being a type qualifier. > > Will __flash behave as a type qualifier? > > int __flash * __flash c; > > If not and a __flash type qualifier had to be created then the constness > of > > the qualifier would be an inherent feature of the __flash type > qualifier. > > Yes, I think it's important that __flash be a qualifier. We can > probably decouple that requirement from whether or not __flash implies > constness. > > -Anton > |
|
From: John M. <ato...@gm...> - 2012-03-19 02:53:59
|
> 1) Cannot write to flash memory. <-- emitted when trying to assign a value
> to a flash var
>
The error for a const being assigned a value is "error: read-only variable
is not assignable" so I would make the wording consistent and just do...
"error: Flash variable is not assignable" ...or...
"error: __flash qualified variable is not assignable"
> 2) Flash variables are read only and should be declared with a const
> qualifier. <-- emitted when doing something like __flash int, we want
> __flash const int
>
Something like the below and then also have one of those clang fix-it hints
showing const being added.
"error: Flash variable requires const qualifier"
3) The pointee type of a flash pointer should be declared with a const
> qualifier. <-- i cant think of a better way of saying it, this is emitted
> when doing __flash int* ptr, we want __flash const int *ptr.
>
I think this can be the same as or similar to #2 and again use the fix-it
hint to show explicitly where the const needs to be placed.
|
|
From: Anton S. <an...@so...> - 2012-03-18 20:47:58
|
On Sun, Mar 18, 2012 at 12:34 PM, John Myers <ato...@gm...> wrote: > I share Anton's concern about __flash not being a type qualifier. > Will __flash behave as a type qualifier? > int __flash * __flash c; > If not and a __flash type qualifier had to be created then the constness of > the qualifier would be an inherent feature of the __flash type qualifier. Yes, I think it's important that __flash be a qualifier. We can probably decouple that requirement from whether or not __flash implies constness. -Anton |
|
From: Anton S. <an...@so...> - 2012-03-18 20:46:10
|
On Sun, Mar 18, 2012 at 11:48 AM, Borja Ferrer <bor...@gm...> wrote: > Ohhh, I misuderstood you Anton, I read the opposite thing xD > > Ok, so you and Eric are saying the same thing. As I said above, we wouldn't > benefit from the constantness optimizations, unless we set every flash > variable as const internally for the frontend, but that doesn't feel very > good to me. So doing __flash const int and __flash int would be the same > thing introducing redundancy in the language which is not standard at all. I > personally like the idea of making the user add the const so he doesn't > assign values accidentally to variables even if then we have a check for > erroring when this happens because he will get the typical error of "can't > assign a value to const variables" which is language specific and standard > and not a custom error added because the flash space cant be written which > is not language standard. > > So what do you think? Yes, I can see the reason for wanting to preserve the standard error messages over adding __flash specific error messages in as many cases as possible. Perhaps making __flash a type qualifier and having a single check that every instance of __flash is associated with a const qualifier would work? -Anton > > 2012/3/18 Anton Staaf <an...@so...> >> >> I'm confused, or have miss-communicated. If you look again at what I >> said, I explicitly said that __flash would imply const, not the other >> way around. Of coarse you want to be able to have const types that >> don't imply flash storage. And the examples I gave all used __flash, >> not const. No where did I suggest that const should imply __flash. >> >> Thanks, >> Anton >> >> On Sun, Mar 18, 2012 at 8:35 AM, Borja Ferrer <bor...@gm...> >> wrote: >> > Heh I knew Eric was going to get a bit mad about what you said Anton xDD >> > >> > Anton, those examples you wrote work exactly the same way as you >> > described. >> > However you can't make that everything that is declared with const to >> > mean >> > that it comes from flash memory, because it may be a ptr to ram, it's >> > not a >> > safe assumption to do. That way, __flash has to be used explicitly, so >> > the >> > user is telling the compiler to change the storage of that piece of data >> > from the generic address space to a custom one. >> > >> > Eric, about my decision of declaring variables always const, it is >> > because >> > they are really read-only by hardware, so writing to them is illegal and >> > this way you're defining the data type as it should be, always readonly. >> > Another reason is that by telling the compiler the data is not going to >> > change throughout execution you benefit from a decent amount of >> > optimizations and alias analysis, avoiding lots of memory reloads and >> > many >> > other goodies you get when using the const qualifier. >> > >> > Even if something is declared const, with a cast the constness can be >> > removed and one could write to the variable, but I've added an error to >> > handle this sort of behavior. In theory nobody should want to do this >> > because it would be going against what flash memory does, so it would be >> > more like wanting to trick the compiler, but erroring in this case would >> > show it's robust and that you cant really trick it. >> > >> > 2012/3/18 Weddington, Eric <Eri...@at...> >> > >> >> >> >> >> >> > -----Original Message----- >> >> > From: Anton Staaf [mailto:an...@so...] >> >> > Sent: Saturday, March 17, 2012 2:15 PM >> >> > To: Borja Ferrer >> >> > Cc: avr...@li... >> >> > Subject: Re: [avr-llvm-devel] Flash data >> >> > >> >> > Hmm, if __flash acts in the same way as const, as in it's a >> >> > qualifier. >> >> > Then could you make it a super set of const? In other words __flash >> >> > implies const? >> >> >> >> No, no, no and no. >> >> >> >> One can have const data, but not want it in flash. And having const >> >> imply a particular address space really does violate the C standard. >> >> >> >> However, what I do question is if the user writes __flash, are they >> >> really required to use const? It seems implicit in the use of __flash. >> >> >> >> Eric Weddington >> > >> > > > |
|
From: Anton S. <an...@so...> - 2012-03-18 20:42:34
|
Borja,
One thought I had was to use the word datum, instead of pointee.
in your third example.
-Anton
On Sun, Mar 18, 2012 at 12:03 PM, Borja Ferrer <bor...@gm...> wrote:
> Ok Eric :)
>
> Btw, can you think of better and more formal error messages as listed in
> message 2?
>
> 2012/3/18 Weddington, Eric <Eri...@at...>
>
>>
>>
>> > -----Original Message-----
>> > From: Borja Ferrer [mailto:bor...@gm...]
>> > Sent: Sunday, March 18, 2012 9:35 AM
>> > To: Weddington, Eric
>> > Cc: Anton Staaf; avr...@li...
>> > Subject: Re: [avr-llvm-devel] Flash data
>> >
>> > Heh I knew Eric was going to get a bit mad about what you said Anton
>> xDD
>> >
>> > Anton, those examples you wrote work exactly the same way as you
>> described.
>> > However you can't make that everything that is declared with const to
>> mean
>> > that it comes from flash memory, because it may be a ptr to ram, it's
>> not a
>> > safe assumption to do. That way, __flash has to be used explicitly, so
>> the
>> > user is telling the compiler to change the storage of that piece of
>> data from
>> > the generic address space to a custom one.
>> >
>> > Eric, about my decision of declaring variables always const, it is
>> because
>> > they are really read-only by hardware, so writing to them is illegal
>> and this
>> > way you're defining the data type as it should be, always readonly.
>> Another
>> > reason is that by telling the compiler the data is not going to change
>> > throughout execution you benefit from a decent amount of optimizations
>> and
>> > alias analysis, avoiding lots of memory reloads and many other goodies
>> you get
>> > when using the const qualifier.
>> >
>> > Even if something is declared const, with a cast the constness can be
>> removed
>> > and one could write to the variable, but I've added an error to handle
>> this
>> > sort of behavior. In theory nobody should want to do this because it
>> would be
>> > going against what flash memory does, so it would be more like wanting
>> to
>> > trick the compiler, but erroring in this case would show it's robust
>> and that
>> > you cant really trick it.
>> >
>>
>> Ok, it sounds reasonable to me. :-)
>>
>> Thanks for the explanation!
>>
>> Eric
>
>
|
|
From: John M. <ato...@gm...> - 2012-03-18 20:35:12
|
Hi Borja, On Mon, Mar 12, 2012 at 5:10 PM, Borja Ferrer <bor...@gm...> wrote: > I've started to work in adding support for storing data in flash memory. > As we discussed a year ago the best approach is to use named adress spaces > and clang and llvm supports them. During the weekend I've been working to > store the variables in the .progmem.data section. By default all static > variables are declared in assembly using the .comm directive, so I had to > change the way this works to save them in the mentioned section. > Unfortunately I couldn't find a way to do this without patching a file that > is target independent. > Can these patches be pushed back upstream to the LLVM trunk? --John |
|
From: John M. <ato...@gm...> - 2012-03-18 19:34:10
|
I share Anton's concern about *__flash* not being a type qualifier. Will *__flash* behave as a type qualifier? int __flash * __flash c; If not and a *__flash* type qualifier had to be created then the constness of the qualifier would be an inherent feature of the *__flash* type qualifier. |
|
From: Borja F. <bor...@gm...> - 2012-03-18 19:03:15
|
Ok Eric :) Btw, can you think of better and more formal error messages as listed in message 2? 2012/3/18 Weddington, Eric <Eri...@at...> > > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Sunday, March 18, 2012 9:35 AM > > To: Weddington, Eric > > Cc: Anton Staaf; avr...@li... > > Subject: Re: [avr-llvm-devel] Flash data > > > > Heh I knew Eric was going to get a bit mad about what you said Anton > xDD > > > > Anton, those examples you wrote work exactly the same way as you > described. > > However you can't make that everything that is declared with const to > mean > > that it comes from flash memory, because it may be a ptr to ram, it's > not a > > safe assumption to do. That way, __flash has to be used explicitly, so > the > > user is telling the compiler to change the storage of that piece of > data from > > the generic address space to a custom one. > > > > Eric, about my decision of declaring variables always const, it is > because > > they are really read-only by hardware, so writing to them is illegal > and this > > way you're defining the data type as it should be, always readonly. > Another > > reason is that by telling the compiler the data is not going to change > > throughout execution you benefit from a decent amount of optimizations > and > > alias analysis, avoiding lots of memory reloads and many other goodies > you get > > when using the const qualifier. > > > > Even if something is declared const, with a cast the constness can be > removed > > and one could write to the variable, but I've added an error to handle > this > > sort of behavior. In theory nobody should want to do this because it > would be > > going against what flash memory does, so it would be more like wanting > to > > trick the compiler, but erroring in this case would show it's robust > and that > > you cant really trick it. > > > > Ok, it sounds reasonable to me. :-) > > Thanks for the explanation! > > Eric > |
|
From: Weddington, E. <Eri...@at...> - 2012-03-18 18:54:15
|
> -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Sunday, March 18, 2012 9:35 AM > To: Weddington, Eric > Cc: Anton Staaf; avr...@li... > Subject: Re: [avr-llvm-devel] Flash data > > Heh I knew Eric was going to get a bit mad about what you said Anton xDD > > Anton, those examples you wrote work exactly the same way as you described. > However you can't make that everything that is declared with const to mean > that it comes from flash memory, because it may be a ptr to ram, it's not a > safe assumption to do. That way, __flash has to be used explicitly, so the > user is telling the compiler to change the storage of that piece of data from > the generic address space to a custom one. > > Eric, about my decision of declaring variables always const, it is because > they are really read-only by hardware, so writing to them is illegal and this > way you're defining the data type as it should be, always readonly. Another > reason is that by telling the compiler the data is not going to change > throughout execution you benefit from a decent amount of optimizations and > alias analysis, avoiding lots of memory reloads and many other goodies you get > when using the const qualifier. > > Even if something is declared const, with a cast the constness can be removed > and one could write to the variable, but I've added an error to handle this > sort of behavior. In theory nobody should want to do this because it would be > going against what flash memory does, so it would be more like wanting to > trick the compiler, but erroring in this case would show it's robust and that > you cant really trick it. > Ok, it sounds reasonable to me. :-) Thanks for the explanation! Eric |
|
From: Weddington, E. <Eri...@at...> - 2012-03-18 18:53:42
|
> -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Sunday, March 18, 2012 12:49 PM > To: Anton Staaf > Cc: Weddington, Eric; avr...@li... > Subject: Re: [avr-llvm-devel] Flash data > > Ohhh, I misuderstood you Anton, I read the opposite thing xD > My apologies too. I also mis-read you, and thought the opposite. My bad. :-( Eric |
|
From: Borja F. <bor...@gm...> - 2012-03-18 18:48:57
|
Ohhh, I misuderstood you Anton, I read the opposite thing xD Ok, so you and Eric are saying the same thing. As I said above, we wouldn't benefit from the constantness optimizations, unless we set every flash variable as const internally for the frontend, but that doesn't feel very good to me. So doing __flash const int and __flash int would be the same thing introducing redundancy in the language which is not standard at all. I personally like the idea of making the user add the const so he doesn't assign values accidentally to variables even if then we have a check for erroring when this happens because he will get the typical error of "can't assign a value to const variables" which is language specific and standard and not a custom error added because the flash space cant be written which is not language standard. So what do you think? 2012/3/18 Anton Staaf <an...@so...> > I'm confused, or have miss-communicated. If you look again at what I > said, I explicitly said that __flash would imply const, not the other > way around. Of coarse you want to be able to have const types that > don't imply flash storage. And the examples I gave all used __flash, > not const. No where did I suggest that const should imply __flash. > > Thanks, > Anton > > On Sun, Mar 18, 2012 at 8:35 AM, Borja Ferrer <bor...@gm...> > wrote: > > Heh I knew Eric was going to get a bit mad about what you said Anton xDD > > > > Anton, those examples you wrote work exactly the same way as you > described. > > However you can't make that everything that is declared with const to > mean > > that it comes from flash memory, because it may be a ptr to ram, it's > not a > > safe assumption to do. That way, __flash has to be used explicitly, so > the > > user is telling the compiler to change the storage of that piece of data > > from the generic address space to a custom one. > > > > Eric, about my decision of declaring variables always const, it is > because > > they are really read-only by hardware, so writing to them is illegal and > > this way you're defining the data type as it should be, always readonly. > > Another reason is that by telling the compiler the data is not going to > > change throughout execution you benefit from a decent amount of > > optimizations and alias analysis, avoiding lots of memory reloads and > many > > other goodies you get when using the const qualifier. > > > > Even if something is declared const, with a cast the constness can be > > removed and one could write to the variable, but I've added an error to > > handle this sort of behavior. In theory nobody should want to do this > > because it would be going against what flash memory does, so it would be > > more like wanting to trick the compiler, but erroring in this case would > > show it's robust and that you cant really trick it. > > > > 2012/3/18 Weddington, Eric <Eri...@at...> > > > >> > >> > >> > -----Original Message----- > >> > From: Anton Staaf [mailto:an...@so...] > >> > Sent: Saturday, March 17, 2012 2:15 PM > >> > To: Borja Ferrer > >> > Cc: avr...@li... > >> > Subject: Re: [avr-llvm-devel] Flash data > >> > > >> > Hmm, if __flash acts in the same way as const, as in it's a qualifier. > >> > Then could you make it a super set of const? In other words __flash > >> > implies const? > >> > >> No, no, no and no. > >> > >> One can have const data, but not want it in flash. And having const > >> imply a particular address space really does violate the C standard. > >> > >> However, what I do question is if the user writes __flash, are they > >> really required to use const? It seems implicit in the use of __flash. > >> > >> Eric Weddington > > > > > |
|
From: Anton S. <an...@so...> - 2012-03-18 17:45:02
|
I'm confused, or have miss-communicated. If you look again at what I
said, I explicitly said that __flash would imply const, not the other
way around. Of coarse you want to be able to have const types that
don't imply flash storage. And the examples I gave all used __flash,
not const. No where did I suggest that const should imply __flash.
Thanks,
Anton
On Sun, Mar 18, 2012 at 8:35 AM, Borja Ferrer <bor...@gm...> wrote:
> Heh I knew Eric was going to get a bit mad about what you said Anton xDD
>
> Anton, those examples you wrote work exactly the same way as you described.
> However you can't make that everything that is declared with const to mean
> that it comes from flash memory, because it may be a ptr to ram, it's not a
> safe assumption to do. That way, __flash has to be used explicitly, so the
> user is telling the compiler to change the storage of that piece of data
> from the generic address space to a custom one.
>
> Eric, about my decision of declaring variables always const, it is because
> they are really read-only by hardware, so writing to them is illegal and
> this way you're defining the data type as it should be, always readonly.
> Another reason is that by telling the compiler the data is not going to
> change throughout execution you benefit from a decent amount of
> optimizations and alias analysis, avoiding lots of memory reloads and many
> other goodies you get when using the const qualifier.
>
> Even if something is declared const, with a cast the constness can be
> removed and one could write to the variable, but I've added an error to
> handle this sort of behavior. In theory nobody should want to do this
> because it would be going against what flash memory does, so it would be
> more like wanting to trick the compiler, but erroring in this case would
> show it's robust and that you cant really trick it.
>
> 2012/3/18 Weddington, Eric <Eri...@at...>
>
>>
>>
>> > -----Original Message-----
>> > From: Anton Staaf [mailto:an...@so...]
>> > Sent: Saturday, March 17, 2012 2:15 PM
>> > To: Borja Ferrer
>> > Cc: avr...@li...
>> > Subject: Re: [avr-llvm-devel] Flash data
>> >
>> > Hmm, if __flash acts in the same way as const, as in it's a qualifier.
>> > Then could you make it a super set of const? In other words __flash
>> > implies const?
>>
>> No, no, no and no.
>>
>> One can have const data, but not want it in flash. And having const
>> imply a particular address space really does violate the C standard.
>>
>> However, what I do question is if the user writes __flash, are they
>> really required to use const? It seems implicit in the use of __flash.
>>
>> Eric Weddington
>
>
|
|
From: Borja F. <bor...@gm...> - 2012-03-18 15:35:14
|
Heh I knew Eric was going to get a bit mad about what you said Anton xDD Anton, those examples you wrote work exactly the same way as you described. However you can't make that everything that is declared with const to mean that it comes from flash memory, because it may be a ptr to ram, it's not a safe assumption to do. That way, __flash has to be used explicitly, so the user is telling the compiler to change the storage of that piece of data from the generic address space to a custom one. Eric, about my decision of declaring variables always const, it is because they are really read-only by hardware, so writing to them is illegal and this way you're defining the data type as it should be, always readonly. Another reason is that by telling the compiler the data is not going to change throughout execution you benefit from a decent amount of optimizations and alias analysis, avoiding lots of memory reloads and many other goodies you get when using the const qualifier. Even if something is declared const, with a cast the constness can be removed and one could write to the variable, but I've added an error to handle this sort of behavior. In theory nobody should want to do this because it would be going against what flash memory does, so it would be more like wanting to trick the compiler, but erroring in this case would show it's robust and that you cant really trick it. 2012/3/18 Weddington, Eric <Eri...@at...> > > > > -----Original Message----- > > From: Anton Staaf [mailto:an...@so...] > > Sent: Saturday, March 17, 2012 2:15 PM > > To: Borja Ferrer > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Flash data > > > > Hmm, if __flash acts in the same way as const, as in it's a qualifier. > > Then could you make it a super set of const? In other words __flash > > implies const? > > No, no, no and no. > > One can have const data, but not want it in flash. And having const > imply a particular address space really does violate the C standard. > > However, what I do question is if the user writes __flash, are they > really required to use const? It seems implicit in the use of __flash. > > Eric Weddington > |
|
From: Weddington, E. <Eri...@at...> - 2012-03-18 14:23:11
|
> -----Original Message----- > From: Anton Staaf [mailto:an...@so...] > Sent: Saturday, March 17, 2012 2:15 PM > To: Borja Ferrer > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Flash data > > Hmm, if __flash acts in the same way as const, as in it's a qualifier. > Then could you make it a super set of const? In other words __flash > implies const? No, no, no and no. One can have const data, but not want it in flash. And having const imply a particular address space really does violate the C standard. However, what I do question is if the user writes __flash, are they really required to use const? It seems implicit in the use of __flash. Eric Weddington |
|
From: Anton S. <an...@so...> - 2012-03-17 20:38:13
|
Hmm, if __flash acts in the same way as const, as in it's a qualifier.
Then could you make it a super set of const? In other words __flash
implies const? Then you could say:
int __flash a;
int __flash * b;
int __flash * __flash c;
This would be an int in flash, a pointer in ram to an int in flash,
and a pointer in flash that points to an int in flash. And all of the
constness would be guaranteed.
If __flash does not behave as a qualifier then I'm not sure how you
would achieve the third example above.
Thanks,
Anton
p.s. Still lurking around on this list. :) I hope to get back to
hacking on avr-llvm at some point.
On Thu, Mar 15, 2012 at 1:01 PM, Borja Ferrer <bor...@gm...> wrote:
> Ok I've finished working on all this. Native english speakers check out
> these strings for a more formal way of saying the following:
>
> 1) Cannot write to flash memory. <-- emitted when trying to assign a value
> to a flash var
> 2) Flash variables are read only and should be declared with a const
> qualifier. <-- emitted when doing something like __flash int, we want
> __flash const int
> 3) The pointee type of a flash pointer should be declared with a const
> qualifier. <-- i cant think of a better way of saying it, this is emitted
> when doing __flash int* ptr, we want __flash const int *ptr.
>
> Again, if anybody can think of any other sanity checks, list them here.
> Once these strings are verified i'll commit my work.
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> avr-llvm-devel mailing list
> avr...@li...
> https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel
>
|
|
From: Borja F. <bor...@gm...> - 2012-03-15 20:01:55
|
Ok I've finished working on all this. Native english speakers check out these strings for a more formal way of saying the following: 1) Cannot write to flash memory. <-- emitted when trying to assign a value to a flash var 2) Flash variables are read only and should be declared with a const qualifier. <-- emitted when doing something like __flash int, we want __flash const int 3) The pointee type of a flash pointer should be declared with a const qualifier. <-- i cant think of a better way of saying it, this is emitted when doing __flash int* ptr, we want __flash const int *ptr. Again, if anybody can think of any other sanity checks, list them here. Once these strings are verified i'll commit my work. |
|
From: Borja F. <bor...@gm...> - 2012-03-13 00:10:30
|
I've started to work in adding support for storing data in flash memory. As we discussed a year ago the best approach is to use named adress spaces and clang and llvm supports them. During the weekend I've been working to store the variables in the .progmem.data section. By default all static variables are declared in assembly using the .comm directive, so I had to change the way this works to save them in the mentioned section. Unfortunately I couldn't find a way to do this without patching a file that is target independent. Having this solved, or atleast for now if something else pops, I would like to add some sanity checks in the frontend for bad usage of using flash variables. Again, I think this will mean patching target independent files of clang, but I can't find a better way of doing it, I'm open to suggestions if anybody finds a way of avoiding this. The first checks I can think of is to force users to declare flash variables with the const qualifier, and pointers that point to flash memory to be const. Another one would be to error when writing a flash variable (only initialization is allowed). One could cast away the const and write to the var so this would have to be checked. On what other things should we check for? Btw, if somebody wants to work on this, I could move to the codegen side. |
|
From: Borja F. <bor...@gm...> - 2012-03-09 13:02:04
|
This is what I've found in atmega's 644pa datasheet: 1) when reading do LSByte first 2) when writing do MSByte first It explains the order is very important because there's some temp register when reading/writing some registers of the 16bit timers that gets activated with this particular order. Since these regs are present up to address 0xff then all in/out instructions should follow this order. Also LDS/STS instructions that use this address range should follow this order for addresses between 0x60 and 0xff, I think that making all sts/lds instructions follow this order instead of that specific range is ok, i can't think of a penalty of doing this. Now things get a bit harder for ld/st instructions because there's no way to know what is the actual address being used. Reversing the order is not good because it messes up postinc or predec operations, maybe only reverse the order if the address is volatile? just an idea. As an example IAR doesn't care at all, it only reverses in/out instructions, so unless I'm missing something it's going to produce wrong code. 2012/3/9 Weddington, Eric <Eri...@at...> > Hi Borja, > > Yes, there is a certain order. It's one way for reading, and another for > writing. And then, for the xmega devices, it gets switched around for > one of the operations. :-P > > Unfortunately, I can't remember off the top of my head which is which. > I'll have to go take a look at the AVR GCC backend code to refresh my > memory... > > Eric > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Wednesday, March 07, 2012 2:38 PM > > To: John Myers > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Current Status? > > > > Eric, one question. I vaguely remember that when reading or writing (i > can't > > remember if both) a 16 bit variable from RAM there was a certain > order, MSByte > > first then LSByte or the other way round. Can you clarify this > information for > > me and where is this specified and for which memory space this > happens. > > > > > > 2012/3/5 Borja Ferrer <bor...@gm...> > > > > > > Ah John one more thing, you may want to combine > SelectionDAGBuilder.diff > > and TargetCallingConv.diff into one file since they both are used for > the same > > purpose, you can name the patch SplitArgPiece > > > > > > 2012/3/5 Borja Ferrer <bor...@gm...> > > > > > > For now, spload should be the only pseudo that is > allowed to get > > into assembly, if you see any other pseudo it is a bug. Expanding > spload into > > real instructions requires a bit of work that is currently > unimplemented, so > > for now we'll have to live with it. I hope it's not a big deal for > you. > > > > About the patches, it's a good idea what you said, feel > free to do > > it when you have time. I still have to add some more patches about > interrupts. > > > > > > 2012/3/5 John Myers <ato...@gm...> > > > > > > On Sun, Mar 4, 2012 at 1:33 PM, Borja Ferrer > > <bor...@gm...> wrote: > > > > > > John check the lastest patch files I've > just commited > > to see if they patch correctly or i've messed them. > > Btw, have you been able to try the > backend? > > > > > > They all apply cleanly but you have to hunt > around to find > > which directory to start in for each file. I'm going to combine them > into one > > patch that can be applied from the root directory. > > > > > > I haven't really looked at the backend yet . I > did notice > > that there was a pseudo instruction (spload) that made it into the > assembly > > file which stopped avr-gcc from being able to assemble it though. > > > > main: > > push r28 > > push r29 > > spload r29:r28, SP > > sbiw r29:r28, 2 > > ldi r24, 0 > > ldi r25, 0 > > std Y+1, r24 > > std Y+2, r25 > > spload r29:r28, SP > > adiw r29:r28, 2 > > pop r29 > > pop r28 > > ret > > > > > > > > > > > > |
|
From: Weddington, E. <Eri...@at...> - 2012-03-09 04:24:11
|
Hi Borja, Yes, there is a certain order. It's one way for reading, and another for writing. And then, for the xmega devices, it gets switched around for one of the operations. :-P Unfortunately, I can't remember off the top of my head which is which. I'll have to go take a look at the AVR GCC backend code to refresh my memory... Eric > -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Wednesday, March 07, 2012 2:38 PM > To: John Myers > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Current Status? > > Eric, one question. I vaguely remember that when reading or writing (i can't > remember if both) a 16 bit variable from RAM there was a certain order, MSByte > first then LSByte or the other way round. Can you clarify this information for > me and where is this specified and for which memory space this happens. > > > 2012/3/5 Borja Ferrer <bor...@gm...> > > > Ah John one more thing, you may want to combine SelectionDAGBuilder.diff > and TargetCallingConv.diff into one file since they both are used for the same > purpose, you can name the patch SplitArgPiece > > > 2012/3/5 Borja Ferrer <bor...@gm...> > > > For now, spload should be the only pseudo that is allowed to get > into assembly, if you see any other pseudo it is a bug. Expanding spload into > real instructions requires a bit of work that is currently unimplemented, so > for now we'll have to live with it. I hope it's not a big deal for you. > > About the patches, it's a good idea what you said, feel free to do > it when you have time. I still have to add some more patches about interrupts. > > > 2012/3/5 John Myers <ato...@gm...> > > > On Sun, Mar 4, 2012 at 1:33 PM, Borja Ferrer > <bor...@gm...> wrote: > > > John check the lastest patch files I've just commited > to see if they patch correctly or i've messed them. > Btw, have you been able to try the backend? > > > They all apply cleanly but you have to hunt around to find > which directory to start in for each file. I'm going to combine them into one > patch that can be applied from the root directory. > > > I haven't really looked at the backend yet . I did notice > that there was a pseudo instruction (spload) that made it into the assembly > file which stopped avr-gcc from being able to assemble it though. > > main: > push r28 > push r29 > spload r29:r28, SP > sbiw r29:r28, 2 > ldi r24, 0 > ldi r25, 0 > std Y+1, r24 > std Y+2, r25 > spload r29:r28, SP > adiw r29:r28, 2 > pop r29 > pop r28 > ret > > > > > |
|
From: Borja F. <bor...@gm...> - 2012-03-08 11:51:58
|
Eric in case you're not following the "Current status?" thread, please check it out, I have a question for you there in the last message. 2012/3/6 Weddington, Eric <Eri...@at...> > > Agreed. ;-) > > Eric > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Tuesday, March 06, 2012 12:44 PM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > In my opinion the best thing should be to error out, it doesn't make > sense to > > set this attributes in non void functions. Technically from a codegen > > perspective it doesn't matter because registers are all saved and > restored > > unless the naked attrib is set so it wouldnt't make a difference > except for > > some useless instructions. Doing this kind of things shows that the > programmer > > doesn't understand what he's doing. > > I issued the warning to be sort of backwards compat with gcc, although > in this > > case we would be supporting wrong code. In conclusion, error out. > > > > > |
|
From: Borja F. <bor...@gm...> - 2012-03-07 21:38:14
|
Eric, one question. I vaguely remember that when reading or writing (i can't remember if both) a 16 bit variable from RAM there was a certain order, MSByte first then LSByte or the other way round. Can you clarify this information for me and where is this specified and for which memory space this happens. 2012/3/5 Borja Ferrer <bor...@gm...> > Ah John one more thing, you may want to combine SelectionDAGBuilder.diff<http://avr-llvm.svn.sourceforge.net/viewvc/avr-llvm/llvm/trunk/patches/SelectionDAGBuilder.diff?sortby=date&view=log>and > TargetCallingConv.diff<http://avr-llvm.svn.sourceforge.net/viewvc/avr-llvm/llvm/trunk/patches/TargetCallingConv.diff?sortby=date&view=log>into one file since they both are used for the same purpose, you can name > the patch SplitArgPiece > > > 2012/3/5 Borja Ferrer <bor...@gm...> > >> For now, spload should be the only pseudo that is allowed to get into >> assembly, if you see any other pseudo it is a bug. Expanding spload into >> real instructions requires a bit of work that is currently unimplemented, >> so for now we'll have to live with it. I hope it's not a big deal for you. >> >> About the patches, it's a good idea what you said, feel free to do it >> when you have time. I still have to add some more patches about interrupts. >> >> >> 2012/3/5 John Myers <ato...@gm...> >> >>> On Sun, Mar 4, 2012 at 1:33 PM, Borja Ferrer <bor...@gm...>wrote: >>> >>>> John check the lastest patch files I've just commited to see if they >>>> patch correctly or i've messed them. >>>> Btw, have you been able to try the backend? >>>> >>> >>> They all apply cleanly but you have to hunt around to find which >>> directory to start in for each file. I'm going to combine them into one >>> patch that can be applied from the root directory. >>> >>> >>> I haven't really looked at the backend yet . I did notice that there was >>> a pseudo instruction (spload) that made it into the assembly file which >>> stopped avr-gcc from being able to assemble it though. >>> >>> main: >>> push r28 >>> push r29 >>> spload r29:r28, SP >>> sbiw r29:r28, 2 >>> ldi r24, 0 >>> ldi r25, 0 >>> std Y+1, r24 >>> std Y+2, r25 >>> spload r29:r28, SP >>> adiw r29:r28, 2 >>> pop r29 >>> pop r28 >>> ret >>> >>> >> > |
|
From: Weddington, E. <Eri...@at...> - 2012-03-06 22:14:31
|
Agreed. ;-) Eric > -----Original Message----- > From: Borja Ferrer [mailto:bor...@gm...] > Sent: Tuesday, March 06, 2012 12:44 PM > To: Weddington, Eric > Cc: avr...@li... > Subject: Re: [avr-llvm-devel] Interrupt handling > > In my opinion the best thing should be to error out, it doesn't make sense to > set this attributes in non void functions. Technically from a codegen > perspective it doesn't matter because registers are all saved and restored > unless the naked attrib is set so it wouldnt't make a difference except for > some useless instructions. Doing this kind of things shows that the programmer > doesn't understand what he's doing. > I issued the warning to be sort of backwards compat with gcc, although in this > case we would be supporting wrong code. In conclusion, error out. > > |
|
From: Borja F. <bor...@gm...> - 2012-03-06 19:44:05
|
In my opinion the best thing should be to error out, it doesn't make sense to set this attributes in non void functions. Technically from a codegen perspective it doesn't matter because registers are all saved and restored unless the naked attrib is set so it wouldnt't make a difference except for some useless instructions. Doing this kind of things shows that the programmer doesn't understand what he's doing. I issued the warning to be sort of backwards compat with gcc, although in this case we would be supporting wrong code. In conclusion, error out. 2012/3/6 Weddington, Eric <Eri...@at...> > > Well, we don't have to slavishly follow GCC either. > > What do you think? > > If you were a programmer, would you want the compiler to warn you, then > not apply the attribute and continue compilation? > > Should the compiler assume that you did not want the attribute to be > applied in the case of a non-void function or one with arguments? > > The compiler really doesn't know what the fix should be. I would think, > that in this case, it really is equivalent to a syntax error. These > attributes are only allowed on a non-void, non-argument function. > Otherwise, the user needs to the fix the code to expressly say what > needs to be done. > > This is outside the realm of the C standard, since we're dealing with > function attributes and ISRs. Again, what do you think? > > Eric Weddington > > > -----Original Message----- > > From: Borja Ferrer [mailto:bor...@gm...] > > Sent: Tuesday, March 06, 2012 12:22 PM > > To: Weddington, Eric > > Cc: avr...@li... > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > It issues a warning, so, it doesn't apply the attribute and it doesn't > abort > > compilation. I checked gcc and it warns aswell. However i can error > out if > > it's better, i leave that up to you. > > > > > > 2012/3/6 Weddington, Eric <Eri...@at...> > > > > > > > > > > > -----Original Message----- > > > From: Borja Ferrer [mailto:bor...@gm...] > > > > > Sent: Tuesday, March 06, 2012 11:57 AM > > > To: Weddington, Eric > > > Cc: avr...@li... > > > Subject: Re: [avr-llvm-devel] Interrupt handling > > > > > > > > I've added some checks to the frontend when setting the > > signal/interrupt > > > attribute that are the following: > > > > > > 1) warn when the attrib is used in things that aren't function > > declarations. > > > 2) warn when the function has arguments > > > 3) warn when the function doesnt return a void > > > > > > If anybody can think of any more checks that are required let > me know. > > > > > > Oh, and here are the warning messages printed for cases 2 and > 3 > > respectively, > > > again, if somebody has a better descriptive string or with > better > > english I'll > > > change it: > > > > > > 2) "'%0' attribute only applies to functions with no > arguments" > > > 3) "'%0' attribute only applies to functions with a void > return type" > > > where %0 is either interrupt or signal > > > > > > > > > Semantics question: Do you mean that it issues just a *warning*? > Or does > > it error out on these? > > > > > > |
14 messages has been excluded from this view by a project administrator.
