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

[Alireza.Moshtaghi at microchip.com]

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; clattner at apple.com;
llvmdev at cs.uiuc.edu
> 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
>