From: Borja F. <bor...@gm...> - 2012-04-20 15:03:09
|
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. >>>> >>> >>> >> > |