From: Borja F. <bor...@gm...> - 2012-04-12 11:20:45
|
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. >>> >> >> > |