ISRs for PIC16 [was [llvm]r79631 ...]

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

ISRs for PIC16 [was [llvm]r79631 ...]

sanjiv gupta-2
Chris Lattner wrote:

>
> On Aug 21, 2009, at 11:13 AM, [hidden email] wrote:
>
>>>> Add a pass to do call graph analyis to overlay the autos and frame
>>>> sections of
>>>> leaf functions. This pass will be extended to color other nodes of
>>>> the call tree
>>>> as well in future.
>>>
>>> Sanjiv, this commit, r79562 and r79563 are not the right way to tackle
>>> this problem, and are coming in the day of the release branch.  Please
>>> revert them and propose patches for these on llvmdev.
>>
>> Hi Chris
>>
>> The reason why we are doing these in backend is that cloning a function
>> is base on which functions call it and of course in frontend we don't
>> have the view of the entire call graph. We'll be happy to revert these,
>> but I would like to know exactly why in your view.
>
> We should discuss this on llvmdev, I think it came up before but there
> was no conclusive plan that was proposed.
The approach that we thought to for for PIC16 can be described in a
single line as below.

"Keep the functions called from ISR and main separate by cloning the  
shared ones".

>
> In short, I think that this sort of thing should be modeled as an
> attribute on function definitions, not declarations.  I don't
> understand why you need to clone entire call graphs, but this is best
> discussed on llvmdev.
It isn't cloning the entire call graph. It's cloning only the shared
ones as mentioned above.

The information that a function is ISR is encoded in the section string
for the lack of a better attribute.

Plus we also wanted to encode some information to inform codegen about
whether we are codegenerating an interrupt line function. Again the
section string is used to encode the same. Probably annotations might be
a better place but that does not work currently (and also, accessing
them slower). This information is required so that the codegen can emit
calls to appropriate intrinsics.

So, IMO, the approach is simple, but probably the implementation wasn't
clean.

Ideas welcome.
>
> Trying to cram things into 2.6 at the last minute is not acceptable.
> :)  For major things like this, it should have a real discussion about
> the design and the way forward and the implementation should have
> landed more than the day of the branch.
>
> -Chris
Agree. That was't a good thing on our part.

- Sanjiv



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

sanjiv gupta-2
Bringing it up again.

- Sanjiv
Sanjiv Gupta wrote:

> Chris Lattner wrote:
>  
>> We should discuss this on llvmdev, I think it came up before but there
>> was no conclusive plan that was proposed.
>>    
> The approach that we thought for PIC16 can be described in a
> single line as below.
>
> "Keep the functions called from ISR and main separate by cloning the  
> shared ones".
>
>  
>> In short, I think that this sort of thing should be modeled as an
>> attribute on function definitions, not declarations.  I don't
>> understand why you need to clone entire call graphs, but this is best
>> discussed on llvmdev.
>>    
> It isn't cloning the entire call graph. It's cloning only the shared
> ones as mentioned above.
>
> The information that a function is ISR is encoded in the section string
> for the lack of a better attribute.
>
> Plus we also wanted to encode some information to inform codegen about
> whether we are codegenerating an interrupt line function. Again the
> section string is used to encode the same. Probably annotations might be
> a better place but that does not work currently (and also, accessing
> them slower). This information is required so that the codegen can emit
> calls to appropriate intrinsics.
>
> So, IMO, the approach is simple, but probably the implementation wasn't
> clean.
>
> Ideas welcome.
>  

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Alireza.Moshtaghi

Up front I apologize for the lengthy email. Since our patch was not
accepted, Chris asked us to follow up with this issue on llvm-dev. For
the sake of completeness, let me give a bit of background and the
problems that we are facing. I tried to capture as much as possible here
so Please do give us feedback.
Don't take your stack for granted.... PIC16 has very poor pointer
handling, hence stack access are very limited and inefficient; so we do
static analysis of function frame size and statically allocate the
function frame in memory (including locals, arguments and spills); then,
to save memory, we overlay the frame of functions that are not on the
same call branch.
Consequently, functions are non reentrant. We can sacrifice recursion
(because of other limitations in pic16 architecture) but still
reentrancy is needed because ISR thread can call same function that is
also called from main thread; examples are stdlib functions and
intrinsic for math (div/mul/float/etc)
Here is what we did - but was rejected and we would like to know which
part (if any) we can keep and which part we must change:
1- Root ISR function is defined by user. Since the ISR is located at
certain address in program memory, we used the section attribute of
function to encode that the function needs to be at a section with fixed
address (our in-house linker takes care of the details). We did not add
new interrupt attribute because it seemed redundant because we can
encode whatever is needed in the section attribute in the front-end.
2- Added two passes at the end of llvm-ld. First pass does call graph
analysis to distinguish ISR thread from main thread and add few more
pieces of info to section attribute of the function to indicate whether
it is called from main thread or ISR thread; if called from both, the
function has to be cloned. To clone, code and data must be cloned. Not
having stack, and accessing local variables through their symbolic
reference, we need to also clone the local variables and modify the
cloned function to use different naming convention for local variables
(there are other ways to do this also but in practice, that requires two
different frameIndexes for one function one for locals and the other for
args and spills). So after this pass, the main thread and ISR thread are
completely disjoint; this takes care of stdlib functions as well because
they are defined in .bc or .a library files (there is a caveat on
function pointers that I'm going to explain later). The second pass does
simple function coloring to identify which function frames can be
overlaid. Color info is also added to section attribute.
3- Code generation is mostly similar for main vs ISR threads except for
function interface of root ISR; but the most important problem is the
intrinsic functions for non-native operations such as mul/div and
floating point. So while generating code for these calls, we use
different naming conventions depending on whether we are generating code
for a function on ISR-thread or main-thread (this information is in the
section attribute of the function). Two versions of intrinsics exist in
the pic16 specific library and the linker will use according to naming
convention.
4- There is a bit of cleanup left to be done in AsmPrinter and we are
good to go.
5- Handling of function pointers is not implemented yet. But since we
don't have dynamic linking, through alias analysis we should be able to
find targets of each pointer and when taking address of these functions,
depending on whether it is in isr or main thread, we can resolve the
problem efficiently. This can be done in a separate pass; the only thing
that we have to disallow is calling same function pointer from both the
ISR and main threads.
6- Handling of varargs requires that a separate frame be created for
arguments for each call site with varargs. This is going to be handled
while codegen and by the overlay algorithm to save memory.

If you have come this far, thank you for being interested in my brain
dysfunction. And I am really interested to know your input.

-Ali


> >> We should discuss this on llvmdev, I think it came up before but
there

> >> was no conclusive plan that was proposed.
> >>
> > The approach that we thought for PIC16 can be described in a
> > single line as below.
> >
> > "Keep the functions called from ISR and main separate by cloning the
> > shared ones".
> >
> >
> >> In short, I think that this sort of thing should be modeled as an
> >> attribute on function definitions, not declarations.  I don't
> >> understand why you need to clone entire call graphs, but this is
best
> >> discussed on llvmdev.
> >>
> > It isn't cloning the entire call graph. It's cloning only the shared
> > ones as mentioned above.
> >
> > The information that a function is ISR is encoded in the section
string
> > for the lack of a better attribute.
> >
> > Plus we also wanted to encode some information to inform codegen
about
> > whether we are codegenerating an interrupt line function. Again the
> > section string is used to encode the same. Probably annotations
might be
> > a better place but that does not work currently (and also, accessing
> > them slower). This information is required so that the codegen can
emit
> > calls to appropriate intrinsics.
> >
> > So, IMO, the approach is simple, but probably the implementation
wasn't
> > clean.
> >
> > Ideas welcome.
> >


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Jim Grosbach
Hi Ali,

Thanks for bringing this up. You're definitely under very tight design  
constraints from the hardware. I can certainly sympathize.

I think two design elements are being conflated here, and it would be  
worthwhile splitting them out. For correctness, you need to make sure  
any routines called from an ISR don't clobber equivalent routines  
called from mainline code. For efficiency, you want to overlay the  
static stack frames of functions as much as possible when you can  
prove those frames do not interfere. I believe you can solve these  
problems orthogonally with a bit of fiddling.

Parallel call trees for the ISR and the mainline have the following  
primary components:
*  Constructing the parallel trees themselves such that there's no  
overlap
*  Keeping things straight in the presence of user-written assembly
*  Indirect calls (function pointers)

To straightforwardly create parallel call trees, one for the mainline  
and one for the ISR, I believe it will be best to create two versions  
of every function that's compiled very early in the process, before  
any lowering of frames to static addresses or anything of that sort is  
done. The ISR-tree functions should be flagged as such and have their  
names mangled to reflect. Any call instructions in the ISR-tree  
function should be modified to call the equivalent ISR-tree function  
of the callee. Thus, with no further analysis, we have disjoint call  
trees for mainline code and ISR code. We also have a bunch of extra  
functions laying around, but they are unreachable, so existing  
optimizations should get rid of them for us.

When it comes time to allocate static memory space for the call  
frames, we simply have to do it twice. Once for the mainline  
functions, and once for the ISR functions. We can assert that nothing  
can overlap between the trees since we know that the trees are  
disjoint except via the ISR itself. Thus, we don't overlay anything  
from one tree with anything from the other tree. Varargs and should  
come along for the ride without any special handling since the  
functions are distinct well before those are lowered to reference  
static buffers.

User assembly throws a big wrench into the works, but it's not  
irreconcilable. I suggest requiring the user to flag assembly  
functions as being either for mainline or for ISR code. Doing so via a  
naming convention would likely tie in the easiest with the function  
cloning done above. With the convention being sufficiently magic  
(read: not legal C identifiers so as to avoid collisions), the  
assembler can issue diagnostics if a call instruction is seen that  
references a function not in the proper tree. That is, the  
disjointness of the trees will be enforceable even in user assembly  
with a bit of cooperation from the assembler.

Function pointers are where things get fun. To do these, we need to  
determine at run time whether we need to call the ISR or the mainline  
version of a function, and since the same pointer can be dereferenced  
from both trees, and we're not guaranteed the pointer assignment will  
take place in the same tree, we can't rely on static analysis to  
figure out which of the two versions to reference. We need to defer  
'til run time, and do so in a way that's not too horribly inefficient.  
We would likewise prefer not to have function pointers become  
unmanageably large, since they already essentially need to carry  
around a pointer to the function itself and a pointer to the argument  
frame for that function. Doubling that is not reasonable. If we use  
descriptor handles, however, we can reduce the load of what we're  
passing around at runtime to a single value. We build a list of  
functions whose addresses are taken, and create a descriptor table for  
them. Eventually, the descriptor handle will be relocated to the  
actual address of the proper half of the descriptor (since the  
invocation point knows at compile time which tree it's in).

Conceptually, something like this. Depending on what level of program  
memory access you have (sorry, hazy memory about what the enhanced  
PIC16 was or wasn't going to do about that), a trickier actual  
implementation may be better.
struct fptr_descriptor {
   void (*main_tree_fn)();
   void *main_tree_args;
   void (*isr_tree_fn)();
   void *isr_tree_args;
};

Each translation unit builds up a table of these, with function  
pointers being a relocation indicating the appropriate slot in the  
table. For global unreachable function removal to work, we just need  
to consider all functions in this table as being called (both  
versions). Good analysis should enable even better results, but the  
simple solution should still be pretty good.

All of this will work without any consideration of optimizing call  
frame allocation. That can be done completely separately without  
interfering (sorry for the pun) with any of these bits, including the  
function pointers, since we've kept the complete separation of ISR and  
mainline trees.

To summarize, make two copies of each function very early and keep the  
ISR and mainline trees separate all the way through. Unreachable  
function removal will get rid of any copies that aren't needed.  
Function pointers become a descriptor table that reference both  
copies, and the runtime invokes the proper bit based on which tree the  
call point is in.

Best regards,

   Jim

On Aug 24, 2009, at 12:12 PM, [hidden email] wrote:

>
> Up front I apologize for the lengthy email. Since our patch was not
> accepted, Chris asked us to follow up with this issue on llvm-dev. For
> the sake of completeness, let me give a bit of background and the
> problems that we are facing. I tried to capture as much as possible  
> here
> so Please do give us feedback.
> Don't take your stack for granted.... PIC16 has very poor pointer
> handling, hence stack access are very limited and inefficient; so we  
> do
> static analysis of function frame size and statically allocate the
> function frame in memory (including locals, arguments and spills);  
> then,
> to save memory, we overlay the frame of functions that are not on the
> same call branch.
> Consequently, functions are non reentrant. We can sacrifice recursion
> (because of other limitations in pic16 architecture) but still
> reentrancy is needed because ISR thread can call same function that is
> also called from main thread; examples are stdlib functions and
> intrinsic for math (div/mul/float/etc)
> Here is what we did - but was rejected and we would like to know which
> part (if any) we can keep and which part we must change:
> 1- Root ISR function is defined by user. Since the ISR is located at
> certain address in program memory, we used the section attribute of
> function to encode that the function needs to be at a section with  
> fixed
> address (our in-house linker takes care of the details). We did not  
> add
> new interrupt attribute because it seemed redundant because we can
> encode whatever is needed in the section attribute in the front-end.
> 2- Added two passes at the end of llvm-ld. First pass does call graph
> analysis to distinguish ISR thread from main thread and add few more
> pieces of info to section attribute of the function to indicate  
> whether
> it is called from main thread or ISR thread; if called from both, the
> function has to be cloned. To clone, code and data must be cloned. Not
> having stack, and accessing local variables through their symbolic
> reference, we need to also clone the local variables and modify the
> cloned function to use different naming convention for local variables
> (there are other ways to do this also but in practice, that requires  
> two
> different frameIndexes for one function one for locals and the other  
> for
> args and spills). So after this pass, the main thread and ISR thread  
> are
> completely disjoint; this takes care of stdlib functions as well  
> because
> they are defined in .bc or .a library files (there is a caveat on
> function pointers that I'm going to explain later). The second pass  
> does
> simple function coloring to identify which function frames can be
> overlaid. Color info is also added to section attribute.
> 3- Code generation is mostly similar for main vs ISR threads except  
> for
> function interface of root ISR; but the most important problem is the
> intrinsic functions for non-native operations such as mul/div and
> floating point. So while generating code for these calls, we use
> different naming conventions depending on whether we are generating  
> code
> for a function on ISR-thread or main-thread (this information is in  
> the
> section attribute of the function). Two versions of intrinsics exist  
> in
> the pic16 specific library and the linker will use according to naming
> convention.
> 4- There is a bit of cleanup left to be done in AsmPrinter and we are
> good to go.
> 5- Handling of function pointers is not implemented yet. But since we
> don't have dynamic linking, through alias analysis we should be able  
> to
> find targets of each pointer and when taking address of these  
> functions,
> depending on whether it is in isr or main thread, we can resolve the
> problem efficiently. This can be done in a separate pass; the only  
> thing
> that we have to disallow is calling same function pointer from both  
> the
> ISR and main threads.
> 6- Handling of varargs requires that a separate frame be created for
> arguments for each call site with varargs. This is going to be handled
> while codegen and by the overlay algorithm to save memory.
>
> If you have come this far, thank you for being interested in my brain
> dysfunction. And I am really interested to know your input.
>
> -Ali
>
>
>>>> We should discuss this on llvmdev, I think it came up before but
> there
>>>> was no conclusive plan that was proposed.
>>>>
>>> The approach that we thought for PIC16 can be described in a
>>> single line as below.
>>>
>>> "Keep the functions called from ISR and main separate by cloning the
>>> shared ones".
>>>
>>>
>>>> In short, I think that this sort of thing should be modeled as an
>>>> attribute on function definitions, not declarations.  I don't
>>>> understand why you need to clone entire call graphs, but this is
> best
>>>> discussed on llvmdev.
>>>>
>>> It isn't cloning the entire call graph. It's cloning only the shared
>>> ones as mentioned above.
>>>
>>> The information that a function is ISR is encoded in the section
> string
>>> for the lack of a better attribute.
>>>
>>> Plus we also wanted to encode some information to inform codegen
> about
>>> whether we are codegenerating an interrupt line function. Again the
>>> section string is used to encode the same. Probably annotations
> might be
>>> a better place but that does not work currently (and also, accessing
>>> them slower). This information is required so that the codegen can
> emit
>>> calls to appropriate intrinsics.
>>>
>>> So, IMO, the approach is simple, but probably the implementation
> wasn't
>>> clean.
>>>
>>> Ideas welcome.
>>>
>
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

John Regehr
> Function pointers are where things get fun. To do these, we need to
> determine at run time whether we need to call the ISR or the mainline
> version of a function

This sounds convenient but it may well be overkill.

On a PIC-class platform we can probably consider it to be a design flaw if
the programmer doesn't know whether a function pointer will be
dereferenced from interrupt context or not.  This suggests that for any
function whose address is taken, there could be a required annotation such
as ISR_ONLY or NONISR_ONLY.  The compiler could use this to do the right
thing without any heroic static analysis or dynamic binding.

John
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Jim Grosbach
On Aug 25, 2009, at 8:59 AM, John Regehr wrote:

>> Function pointers are where things get fun. To do these, we need to
>> determine at run time whether we need to call the ISR or the mainline
>> version of a function
>
> This sounds convenient but it may well be overkill.
>
> On a PIC-class platform we can probably consider it to be a design  
> flaw if
> the programmer doesn't know whether a function pointer will be
> dereferenced from interrupt context or not.  This suggests that for  
> any
> function whose address is taken, there could be a required  
> annotation such
> as ISR_ONLY or NONISR_ONLY.  The compiler could use this to do the  
> right
> thing without any heroic static analysis or dynamic binding.
>

That could work as well; however, for the PIC16, I'd still be tempted  
to go with a descriptor table for function pointers. Otherwise, a  
function pointer would need to be 32-bits wide (16 for the function  
itself, and 16 for the argument address). At that point, it's a very  
short step to supporting the runtime decision about which version of  
the function to call. Generally speaking, I've found that when  
possible, it's best for the compiler to just do the right thing  
without requiring additional annotations from the user. Every time  
I've strayed from that in a PIC target compiler, I've regretted it.

Regards,
   Jim
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Alireza.Moshtaghi
Right now, we are saving the address to frame right before the beginning
of function code space; so before calling the function pointer, we have
the frame pointer as well. That saves us two bytes of valuable ram -
compared to rom - per pointer (and with a bit of analysis we can only do
this for functions whose address is taken as to not waste rom space
either). Jim's proposal for adding second pointer for ISR function
pointers is generic and tempting, and with pointer to frame before each
function, we would only need two 16-bit pointers for each fptr; and it
eliminates need for static analysis of code to figure things out. But
still there is ram over usage that may not be favorable by the user of
the compiler. I think the benefits of this approach are enough to make
it fly but I have to look into that a little more.

Thanks,
-Ali

> -----Original Message-----
> From: [hidden email] [mailto:[hidden email]]
On

> Behalf Of Jim Grosbach
> Sent: Tuesday, August 25, 2009 9:30 AM
> To: John Regehr
> Cc: [hidden email]
> Subject: Re: [LLVMdev] ISRs for PIC16 [was [llvm]r79631 ...]
>
> On Aug 25, 2009, at 8:59 AM, John Regehr wrote:
>
> >> Function pointers are where things get fun. To do these, we need to
> >> determine at run time whether we need to call the ISR or the
mainline

> >> version of a function
> >
> > This sounds convenient but it may well be overkill.
> >
> > On a PIC-class platform we can probably consider it to be a design
> > flaw if
> > the programmer doesn't know whether a function pointer will be
> > dereferenced from interrupt context or not.  This suggests that for
> > any
> > function whose address is taken, there could be a required
> > annotation such
> > as ISR_ONLY or NONISR_ONLY.  The compiler could use this to do the
> > right
> > thing without any heroic static analysis or dynamic binding.
> >
>
> That could work as well; however, for the PIC16, I'd still be tempted
> to go with a descriptor table for function pointers. Otherwise, a
> function pointer would need to be 32-bits wide (16 for the function
> itself, and 16 for the argument address). At that point, it's a very
> short step to supporting the runtime decision about which version of
> the function to call. Generally speaking, I've found that when
> possible, it's best for the compiler to just do the right thing
> without requiring additional annotations from the user. Every time
> I've strayed from that in a PIC target compiler, I've regretted it.
>
> Regards,
>    Jim
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

sanjiv gupta-2
In reply to this post by Jim Grosbach
Jim Grosbach wrote:
> Hi Ali,
>
> Thanks for bringing this up. You're definitely under very tight design
> constraints from the hardware. I can certainly sympathize.
Jim,
First of all, thank you very much for understanding every detail of the
problem at our hand and coming up with a solution that addresses every
aspect of it. IMO, given the constraints, this is probably the best
design we can implement. In this email, my queries/observations mostly
relate to the implementation details.
> To straightforwardly create parallel call trees, one for the mainline
> and one for the ISR, I believe it will be best to create two versions
> of every function that's compiled very early in the process, before
> any lowering of frames to static addresses or anything of that sort is
> done.The ISR-tree functions should be flagged as such and have their
> names mangled to reflect. Any call instructions in the ISR-tree
> function should be modified to call the equivalent ISR-tree function
> of the callee.
In our earlier patch, we tried to do the same thing by writing an
llvm-ld pass. There we could clone the shared functions, their arguments
frames and local frames to construct parallel call trees. But now it
looks that clang will be a better place to do the same. For that, clang
will need the appropriate TargetInfo interfaces to allow precisely the
following things:
1. Allow targets to emit an ISR version of a function.
2. Allow targets to detect that they are emitting code for an ISR
version, so that call sites/goto sites can be modified.
And I assume that the DebugInfo for the renamed functions will
automatically fall in place.

My first choice would be clang to do it but was just curious to know if
there are some inherent limitations with the llvm-ld pass approach?
> When it comes time to allocate static memory space for the call
> frames, we simply have to do it twice. Once for the mainline
> functions, and once for the ISR functions. We can assert that nothing
> can overlap between the trees since we know that the trees are
> disjoint except via the ISR itself. Thus, we don't overlay anything
> from one tree with anything from the other tree.
True. To overlay within these two separate trees we used an llvm-ld pass
again. Though we overlaid only the leaf nodes in that pass but I think
using a runOnSCC we can extend this approach to overlay sections in such
a way that a function and any of its predecessor have separate memory
areas. Does that approach sound ok?

> User assembly throws a big wrench into the works, but it's not
> irreconcilable. I suggest requiring the user to flag assembly
> functions as being either for mainline or for ISR code. Doing so via a
> naming convention would likely tie in the easiest with the function
> cloning done above. With the convention being sufficiently magic
> (read: not legal C identifiers so as to avoid collisions), the
> assembler can issue diagnostics if a call instruction is seen that
> references a function not in the proper tree. That is, the
> disjointness of the trees will be enforceable even in user assembly
> with a bit of cooperation from the assembler.
We used this in the AsmPrinter to rename intrinsics called from ISR
code. As, codgen currently ties the libcall names in ISelLowering
constructors and then doesn't allow changing the name dynamically. So
we'll still need to handle the same in AsmPrinter.

Thanks
- Sanjiv


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Alireza.Moshtaghi
Extended thanks to the llvm community for feedback in advance, and
especially thanks to Jim for laying out a solution that captures all
aspects of the problems that we are facing. After some discussions with
our team, we have decided to do the following, but to avoid further
conflict with llvm standards, I would like to get the blessing of llvm
community, and in particular, Chris who at some point expressed concerns
about cloning the entire function hierarchy ...
Here I'm trying to stick to implementation and solicit your feedback
regarding whether we are doing them in the right place.

In summary, the discussion is comprised of three topics:
1) Separating the context of main vs isr functions
2) Function frame Overlay (function coloring)
3) Handling of Intrinsics for non-native operations

Implementation for (1)
I see Jim's suggestion - to duplicate all functions - at the center of
the solution. I think doing this in the front-end (clang) makes the
problem too widespread. An llvm-ld pass keeps the logic and
implementation all at one place which is more manageable; so we prefer
llvm-ld pass. Naturally by adopting a naming convention, there will be
no need to add information to section attribute.
With minor modification of what Jim suggested regarding function pointer
descriptors, code alterations for function pointers can be done either
in the same llvm-ld pass or while custom lowering CALL node. I prefer
the former because we have to modify all call sites in the duplicated
functions anyways; why not extend it to do the indirect calls as well?
And placement of function pointer descriptors can be done in AsmPrinter.

Implementation for (2)
This wouldn't be much different from what we did in the patch we sent:
Add an llvm-ld pass to color the functions (same color function frames
will be overlaid). The pass will add the color information to the
section attribute of the function, and this information will be used by
AsmPrinter to construct appropriate section information in assembly
output.

Implementation for (3)
Conceptually speaking, similar to (1) we have to separate the context of
intrinsic functions for mainline vs ISR. But in practice we can't take
the same approach as (1). Front-end does not generate custom calls for
non-native operations; so we generate calls to intrinsics while
lowering. We provide two sets of intrinsic functions in the port
specific library and appropriate function call will be generated
depending on whether we are in ISR or mainline code. The right way of
doing this is to keep all the logic in legalizer - use the existing
infrastructure and customize the library calls for each non-native
operation; however, this requires a patch on llc to allow customization
of soft floating point library calls. This patch currently breaks ARM
port so until we find time to refactor ARM, we will keep the
implementation in AsmPrinter (a little more work, but it is already
done).

I hope my descriptions are clear enough. Please do provide your
feedback, hopefully we can submit patches for these to be included in
llvm-2.8

Thanks,
-Ali

> -----Original Message-----
> From: Sanjiv Kumar Gupta - I00171
> Sent: Wednesday, August 26, 2009 11:41 AM
> To: Jim Grosbach
> Cc: Alireza Moshtaghi - C13012; [hidden email];
[hidden email]
> Subject: Re: [LLVMdev] ISRs for PIC16 [was [llvm]r79631 ...]
>
> Jim Grosbach wrote:
> > Hi Ali,
> >
> > Thanks for bringing this up. You're definitely under very tight
design
> > constraints from the hardware. I can certainly sympathize.
> Jim,
> First of all, thank you very much for understanding every detail of
the
> problem at our hand and coming up with a solution that addresses every
> aspect of it. IMO, given the constraints, this is probably the best
> design we can implement. In this email, my queries/observations mostly
> relate to the implementation details.
> > To straightforwardly create parallel call trees, one for the
mainline
> > and one for the ISR, I believe it will be best to create two
versions
> > of every function that's compiled very early in the process, before
> > any lowering of frames to static addresses or anything of that sort
is
> > done.The ISR-tree functions should be flagged as such and have their
> > names mangled to reflect. Any call instructions in the ISR-tree
> > function should be modified to call the equivalent ISR-tree function
> > of the callee.
> In our earlier patch, we tried to do the same thing by writing an
> llvm-ld pass. There we could clone the shared functions, their
arguments
> frames and local frames to construct parallel call trees. But now it
> looks that clang will be a better place to do the same. For that,
clang
> will need the appropriate TargetInfo interfaces to allow precisely the
> following things:
> 1. Allow targets to emit an ISR version of a function.
> 2. Allow targets to detect that they are emitting code for an ISR
> version, so that call sites/goto sites can be modified.
> And I assume that the DebugInfo for the renamed functions will
> automatically fall in place.
>
> My first choice would be clang to do it but was just curious to know
if
> there are some inherent limitations with the llvm-ld pass approach?
> > When it comes time to allocate static memory space for the call
> > frames, we simply have to do it twice. Once for the mainline
> > functions, and once for the ISR functions. We can assert that
nothing
> > can overlap between the trees since we know that the trees are
> > disjoint except via the ISR itself. Thus, we don't overlay anything
> > from one tree with anything from the other tree.
> True. To overlay within these two separate trees we used an llvm-ld
pass
> again. Though we overlaid only the leaf nodes in that pass but I think
> using a runOnSCC we can extend this approach to overlay sections in
such
> a way that a function and any of its predecessor have separate memory
> areas. Does that approach sound ok?
> > User assembly throws a big wrench into the works, but it's not
> > irreconcilable. I suggest requiring the user to flag assembly
> > functions as being either for mainline or for ISR code. Doing so via
a

> > naming convention would likely tie in the easiest with the function
> > cloning done above. With the convention being sufficiently magic
> > (read: not legal C identifiers so as to avoid collisions), the
> > assembler can issue diagnostics if a call instruction is seen that
> > references a function not in the proper tree. That is, the
> > disjointness of the trees will be enforceable even in user assembly
> > with a bit of cooperation from the assembler.
> We used this in the AsmPrinter to rename intrinsics called from ISR
> code. As, codgen currently ties the libcall names in ISelLowering
> constructors and then doesn't allow changing the name dynamically. So
> we'll still need to handle the same in AsmPrinter.
>
> Thanks
> - Sanjiv
>


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Alireza.Moshtaghi
> I hope my descriptions are clear enough. Please do provide your
> feedback, hopefully we can submit patches for these to be included in
> llvm-2.8

I meant 2.7 :)


-Ali

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Chris Lattner-2
In reply to this post by Alireza.Moshtaghi
On Aug 27, 2009, at 2:02 PM, [hidden email] wrote:
> Extended thanks to the llvm community for feedback in advance, and
> especially thanks to Jim for laying out a solution that captures all
> aspects of the problems that we are facing. After some discussions  
> with
> our team, we have decided to do the following, but to avoid further
> conflict with llvm standards, I would like to get the blessing of llvm
> community, and in particular, Chris who at some point expressed  
> concerns
> about cloning the entire function hierarchy ...

Ok, fwiw, I think that Jim's outlined approach is a good one.

> Here I'm trying to stick to implementation and solicit your feedback
> regarding whether we are doing them in the right place.
>
> In summary, the discussion is comprised of three topics:
> 1) Separating the context of main vs isr functions
> 2) Function frame Overlay (function coloring)
> 3) Handling of Intrinsics for non-native operations
>
> Implementation for (1)
> I see Jim's suggestion - to duplicate all functions - at the center of
> the solution. I think doing this in the front-end (clang) makes the
> problem too widespread. An llvm-ld pass keeps the logic and
> implementation all at one place which is more manageable; so we prefer
> llvm-ld pass. Naturally by adopting a naming convention, there will be
> no need to add information to section attribute.
> With minor modification of what Jim suggested regarding function  
> pointer
> descriptors, code alterations for function pointers can be done either
> in the same llvm-ld pass or while custom lowering CALL node. I prefer
> the former because we have to modify all call sites in the duplicated
> functions anyways; why not extend it to do the indirect calls as well?

Seems reasonable to me.

> Implementation for (2)
> This wouldn't be much different from what we did in the patch we sent:
> Add an llvm-ld pass to color the functions (same color function frames
> will be overlaid). The pass will add the color information to the
> section attribute of the function, and this information will be used  
> by
> AsmPrinter to construct appropriate section information in assembly
> output.

Ok.

> Implementation for (3)
> Conceptually speaking, similar to (1) we have to separate the  
> context of
> intrinsic functions for mainline vs ISR. But in practice we can't take
> the same approach as (1). Front-end does not generate custom calls for
> non-native operations; so we generate calls to intrinsics while
> lowering.

Right.

> We provide two sets of intrinsic functions in the port
> specific library and appropriate function call will be generated
> depending on whether we are in ISR or mainline code. The right way of
> doing this is to keep all the logic in legalizer - use the existing
> infrastructure and customize the library calls for each non-native
> operation; however, this requires a patch on llc to allow  
> customization
> of soft floating point library calls. This patch currently breaks ARM
> port so until we find time to refactor ARM, we will keep the
> implementation in AsmPrinter (a little more work, but it is already
> done).

I don't really understand your proposal here, what exactly would  
change to support this?

-Chris

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

sanjiv gupta-2
Chris Lattner wrote:

>
>> We provide two sets of intrinsic functions in the port
>> specific library and appropriate function call will be generated
>> depending on whether we are in ISR or mainline code. The right way of
>> doing this is to keep all the logic in legalizer - use the existing
>> infrastructure and customize the library calls for each non-native
>> operation; however, this requires a patch on llc to allow customization
>> of soft floating point library calls. This patch currently breaks ARM
>> port so until we find time to refactor ARM, we will keep the
>> implementation in AsmPrinter (a little more work, but it is already
>> done).
>
> I don't really understand your proposal here, what exactly would
> change to support this?
>
> -Chris
>
http://llvm.org/viewvc/llvm-project?rev=77974&view=rev

To allow targets to choose the libcall name on the fly at codegen time.

- Sanjiv

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

Chris Lattner-2

On Aug 27, 2009, at 7:44 PM, Sanjiv Gupta wrote:

> Chris Lattner wrote:
>>
>>> We provide two sets of intrinsic functions in the port
>>> specific library and appropriate function call will be generated
>>> depending on whether we are in ISR or mainline code. The right way  
>>> of
>>> doing this is to keep all the logic in legalizer - use the existing
>>> infrastructure and customize the library calls for each non-native
>>> operation; however, this requires a patch on llc to allow  
>>> customization
>>> of soft floating point library calls. This patch currently breaks  
>>> ARM
>>> port so until we find time to refactor ARM, we will keep the
>>> implementation in AsmPrinter (a little more work, but it is already
>>> done).
>>
>> I don't really understand your proposal here, what exactly would  
>> change to support this?
>>
>> -Chris
>>
> http://llvm.org/viewvc/llvm-project?rev=77974&view=rev
>
> To allow targets to choose the libcall name on the fly at codegen  
> time.

That patch looks fine to me if the indentation is improved :)

-Chris
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: ISRs for PIC16 [was [llvm]r79631 ...]

sanjiv gupta-2
Chris Lattner wrote:

>
> On Aug 27, 2009, at 7:44 PM, Sanjiv Gupta wrote:
>
>> Chris Lattner wrote:
>>>
>>>> We provide two sets of intrinsic functions in the port
>>>> specific library and appropriate function call will be generated
>>>> depending on whether we are in ISR or mainline code. The right way of
>>>> doing this is to keep all the logic in legalizer - use the existing
>>>> infrastructure and customize the library calls for each non-native
>>>> operation; however, this requires a patch on llc to allow
>>>> customization
>>>> of soft floating point library calls. This patch currently breaks ARM
>>>> port so until we find time to refactor ARM, we will keep the
>>>> implementation in AsmPrinter (a little more work, but it is already
>>>> done).
>>>
>>> I don't really understand your proposal here, what exactly would
>>> change to support this?
>>>
>>> -Chris
>>>
>> http://llvm.org/viewvc/llvm-project?rev=77974&view=rev
>>
>> To allow targets to choose the libcall name on the fly at codegen time.
>
> That patch looks fine to me if the indentation is improved :)
>
> -Chris
Indentation is fine, but last time I tried that it broke some ARM tests.
So, I will reapply this only when we work out those failures.

- Sanjiv

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev