Diana Picus via llvm-dev
2017-Nov-29 10:23 UTC
[llvm-dev] Do I need to implement CCAssignFnForCall for porting GlobalISel to AVR target?
Hi Leslie, Long story short - no, you don't need CCAssignFnForCall. In fact, if you look at the X86CallLowering, you'll see that it doesn't use that either, since it can use CC_X86 directly. For ARM it made sense to have a CCAssignFnForCall so we could reuse the code in ARMISelLowering. I don't know the AVR backend very well, so I can't help you figure out what is most appropriate for the AVR target. Having a CCAssignFn available would make it easier to use the existing infrastructure, but in principle you should be able to get away without one if you override ValueHandler::assignArg. After a cursory look at AVRISelLowering, I see it has some custom code for analyzing arguments - you'll probably have to somehow incorporate that logic into assignArg. Hope that helps, Diana On 29 November 2017 at 05:00, Leslie Zhai <lesliezhai at llvm.org.cn> wrote:> Hi LLVM developers, > > I am porting GlobalISel to AVR target by learning ARM target's source code > > > commit c82e7ec9e25c934568eb9c75cf13226177e98dba > Author: Diana Picus <diana.picus at linaro.org> > Date: Fri Dec 16 10:35:20 2016 +0000 > > [ARM] Expose methods to get the CCAssignFn. NFCI > > Add two public methods to ARMTargetLowering: CCAssignFnForCall and > CCAssignFnForReturn, which are just calling the already existing private > method > CCAssignFnForNode. These will come in handy for GlobalISel on ARM. > > We also replace all calls to CCAssignFnForNode in ARMISelLowering.cpp, > because > the new methods are friendlier to the reader. > > > There are: > > CCAssignFnForCall > > and > > CCAssignFnForReturn > > But there is only CCAssignFnForReturn for AVR, do I need to implement > CCAssignFnForCall for AVR target? > > > diff --git a/lib/Target/AVR/AVRCallingConv.td > b/lib/Target/AVR/AVRCallingConv.td > index 68dbce0..d460925 100644 > --- a/lib/Target/AVR/AVRCallingConv.td > +++ b/lib/Target/AVR/AVRCallingConv.td > @@ -10,6 +10,18 @@ > > //===----------------------------------------------------------------------===// > > > //===----------------------------------------------------------------------===// > +// AVR Calling Convention > +//===----------------------------------------------------------------------===// > + > +def CC_AVR : CallingConv > +<[ > +]>; > + > +def CC_AVR_BUILTIN : CallingConv > +<[ > +]>; > + > +//===----------------------------------------------------------------------===// > // AVR Return Value Calling Convention > > //===----------------------------------------------------------------------===// > > diff --git a/lib/Target/AVR/AVRISelLowering.cpp > b/lib/Target/AVR/AVRISelLowering.cpp > index a49da15..44f0ed6 100644 > --- a/lib/Target/AVR/AVRISelLowering.cpp > +++ b/lib/Target/AVR/AVRISelLowering.cpp > @@ -1324,12 +1324,13 @@ SDValue AVRTargetLowering::LowerCallResult( > // Return Value Calling Convention Implementation > > //===----------------------------------------------------------------------===// > > -CCAssignFn *AVRTargetLowering::CCAssignFnForReturn(CallingConv::ID CC) > const { > +CCAssignFn *AVRTargetLowering::CCAssignFnForReturn(CallingConv::ID CC, > + bool Return) const { > switch (CC) { > case CallingConv::AVR_BUILTIN: > - return RetCC_AVR_BUILTIN; > + return Return ? RetCC_AVR_BUILTIN : CC_AVR_BUILTIN; > default: > - return RetCC_AVR; > + return Return ? RetCC_AVR : CC_AVR; > } > } > > diff --git a/lib/Target/AVR/AVRISelLowering.h > b/lib/Target/AVR/AVRISelLowering.h > index c90c65c..43777a2 100644 > --- a/lib/Target/AVR/AVRISelLowering.h > +++ b/lib/Target/AVR/AVRISelLowering.h > @@ -127,6 +127,8 @@ public: > unsigned getRegisterByName(const char* RegName, EVT VT, > SelectionDAG &DAG) const override; > > + CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC, bool Return = true) > const; > + > private: > SDValue getAVRCmp(SDValue LHS, SDValue RHS, ISD::CondCode CC, SDValue > &AVRcc, > SelectionDAG &DAG, SDLoc dl) const; > @@ -140,8 +142,6 @@ private: > SDValue LowerSETCC(SDValue Op, SelectionDAG &DAG) const; > SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) const; > > - CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC) const; > - > bool CanLowerReturn(CallingConv::ID CallConv, > MachineFunction &MF, bool isVarArg, > const SmallVectorImpl<ISD::OutputArg> &Outs, > > > Please give me some hint, thanks a lot! > > -- > Regards, > Leslie Zhai - https://reviews.llvm.org/p/xiangzhai/ > > >
Leslie Zhai via llvm-dev
2017-Nov-30 05:36 UTC
[llvm-dev] Do I need to implement CCAssignFnForCall for porting GlobalISel to AVR target?
Hi Diana, Thanks for your kind response! Yes, ARM target is more complex than AVR to getEffectiveCallingConv ID https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMISelLowering.cpp#L1559 I need to read ARM's code carefully. 在 2017年11月29日 18:23, Diana Picus 写道:> Hi Leslie, > > Long story short - no, you don't need CCAssignFnForCall. > > In fact, if you look at the X86CallLowering, you'll see that it > doesn't use that either, since it can use CC_X86 directly. For ARM it > made sense to have a CCAssignFnForCall so we could reuse the code in > ARMISelLowering. > > I don't know the AVR backend very well, so I can't help you figure out > what is most appropriate for the AVR target. Having a CCAssignFn > available would make it easier to use the existing infrastructure, but > in principle you should be able to get away without one if you > override ValueHandler::assignArg. After a cursory look at > AVRISelLowering, I see it has some custom code for analyzing arguments > - you'll probably have to somehow incorporate that logic into > assignArg. > > Hope that helps, > Diana > > On 29 November 2017 at 05:00, Leslie Zhai <lesliezhai at llvm.org.cn> wrote: >> Hi LLVM developers, >> >> I am porting GlobalISel to AVR target by learning ARM target's source code >> >> >> commit c82e7ec9e25c934568eb9c75cf13226177e98dba >> Author: Diana Picus <diana.picus at linaro.org> >> Date: Fri Dec 16 10:35:20 2016 +0000 >> >> [ARM] Expose methods to get the CCAssignFn. NFCI >> >> Add two public methods to ARMTargetLowering: CCAssignFnForCall and >> CCAssignFnForReturn, which are just calling the already existing private >> method >> CCAssignFnForNode. These will come in handy for GlobalISel on ARM. >> >> We also replace all calls to CCAssignFnForNode in ARMISelLowering.cpp, >> because >> the new methods are friendlier to the reader. >> >> >> There are: >> >> CCAssignFnForCall >> >> and >> >> CCAssignFnForReturn >> >> But there is only CCAssignFnForReturn for AVR, do I need to implement >> CCAssignFnForCall for AVR target? >> >> >> diff --git a/lib/Target/AVR/AVRCallingConv.td >> b/lib/Target/AVR/AVRCallingConv.td >> index 68dbce0..d460925 100644 >> --- a/lib/Target/AVR/AVRCallingConv.td >> +++ b/lib/Target/AVR/AVRCallingConv.td >> @@ -10,6 +10,18 @@ >> >> //===----------------------------------------------------------------------===// >> >> >> //===----------------------------------------------------------------------===// >> +// AVR Calling Convention >> +//===----------------------------------------------------------------------===// >> + >> +def CC_AVR : CallingConv >> +<[ >> +]>; >> + >> +def CC_AVR_BUILTIN : CallingConv >> +<[ >> +]>; >> + >> +//===----------------------------------------------------------------------===// >> // AVR Return Value Calling Convention >> >> //===----------------------------------------------------------------------===// >> >> diff --git a/lib/Target/AVR/AVRISelLowering.cpp >> b/lib/Target/AVR/AVRISelLowering.cpp >> index a49da15..44f0ed6 100644 >> --- a/lib/Target/AVR/AVRISelLowering.cpp >> +++ b/lib/Target/AVR/AVRISelLowering.cpp >> @@ -1324,12 +1324,13 @@ SDValue AVRTargetLowering::LowerCallResult( >> // Return Value Calling Convention Implementation >> >> //===----------------------------------------------------------------------===// >> >> -CCAssignFn *AVRTargetLowering::CCAssignFnForReturn(CallingConv::ID CC) >> const { >> +CCAssignFn *AVRTargetLowering::CCAssignFnForReturn(CallingConv::ID CC, >> + bool Return) const { >> switch (CC) { >> case CallingConv::AVR_BUILTIN: >> - return RetCC_AVR_BUILTIN; >> + return Return ? RetCC_AVR_BUILTIN : CC_AVR_BUILTIN; >> default: >> - return RetCC_AVR; >> + return Return ? RetCC_AVR : CC_AVR; >> } >> } >> >> diff --git a/lib/Target/AVR/AVRISelLowering.h >> b/lib/Target/AVR/AVRISelLowering.h >> index c90c65c..43777a2 100644 >> --- a/lib/Target/AVR/AVRISelLowering.h >> +++ b/lib/Target/AVR/AVRISelLowering.h >> @@ -127,6 +127,8 @@ public: >> unsigned getRegisterByName(const char* RegName, EVT VT, >> SelectionDAG &DAG) const override; >> >> + CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC, bool Return = true) >> const; >> + >> private: >> SDValue getAVRCmp(SDValue LHS, SDValue RHS, ISD::CondCode CC, SDValue >> &AVRcc, >> SelectionDAG &DAG, SDLoc dl) const; >> @@ -140,8 +142,6 @@ private: >> SDValue LowerSETCC(SDValue Op, SelectionDAG &DAG) const; >> SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) const; >> >> - CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC) const; >> - >> bool CanLowerReturn(CallingConv::ID CallConv, >> MachineFunction &MF, bool isVarArg, >> const SmallVectorImpl<ISD::OutputArg> &Outs, >> >> >> Please give me some hint, thanks a lot! >> >> -- >> Regards, >> Leslie Zhai - https://reviews.llvm.org/p/xiangzhai/ >> >> >>-- Regards, Leslie Zhai - https://reviews.llvm.org/p/xiangzhai/