Re: [avr-llvm-devel] Flash data

[Borja F. <bor...@gm...>]

John I've seen you've updated some patches, but remember to fix the code
you removed from a patch file.

2012/4/22 Borja Ferrer <bor...@gm...>

> Ok, thanks John.
>
>
> 2012/4/20 John Myers <ato...@gm...>
>
>> I haven't had time to fix it.
>> I'll probably be able to do it this weekend.
>>
>>
>> On Fri, Apr 20, 2012 at 8:02 AM, Borja Ferrer <bor...@gm...>wrote:
>>
>>> John ping?
>>>
>>>
>>> 2012/4/12 Borja Ferrer <bor...@gm...>
>>>
>>>> John I noticed that in your latest commit you removed a piece of valid
>>>> code inside lib/CodeGen/TargetInfo.cpp, although this code is for flash
>>>> variables, it goes under the code that handles interrupt attributes.
>>>>
>>>>   +  } else
>>>>  +    // Global variables that do not belong to the generic address
>>>> space and
>>>>  +    // have common linkage are set to have external linkage.
>>>>  +    if ((GV->getType()->getAddressSpace() != 0)
>>>>  +        && (GV->getLinkage() == llvm::GlobalValue::CommonLinkage))
>>>>  +      GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
>>>>  +}
>>>> You may want to place this bit in the interrupt patch or something, I
>>>> leave that up to you.
>>>>
>>>> Btw has anybody tried or tested the backend lately? I would like to
>>>> hear some opinions.
>>>>
>>>>
>>>> 2012/3/20 Borja Ferrer <bor...@gm...>
>>>>
>>>>> 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.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>