[llvm-commits] [llvm] r71610 - in /llvm/trunk: include/llvm/CodeGen/MachineFunction.h include/llvm/Intrinsics.td lib/CodeGen/MachineFunction.cpp lib/CodeGen/PrologEpilogInserter.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp lib/Target/ARM/ARMISelLowering.cpp lib/Target/ARM/ARMISelLowering.h lib/Target/ARM/ARMInstrInfo.cpp lib/Target/ARM/ARMInstrInfo.td

[Chris Lattner]

On May 12, 2009, at 4:59 PM, Jim Grosbach wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=71610&view=rev
> Log:
> Add support for GCC compatible builtin setjmp and longjmp  
> intrinsics. This is
> a supporting preliminary patch for GCC-compatible SjLJ exception  
> handling. Note that these intrinsics are not designed to be invoked  
> directly by the user, but
> rather used by the front-end as target hooks for exception handling.

All new non-target-specific intrinsics need to be documented in LangRef.

How about llvm.save.register.state() and  
llvm.restore.register.state().  LangRef should document that these are  
compatible with the GCC builtins.  Also, you should use GCCBuiltin<>  
in the .td file so that these work with the CBE etc.  Please make sure  
to update 'HasBuiltinSetjmp' and the accessors when you rename the  
intrinsic.


> +  // HasBuiltinSetjmp - true if the function uses builtin_setjmp.  
> Used to
> +  // adjust callee-saved register tracking.
> +  bool HasBuiltinSetjmp;
...
> +++ llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp Tue May 12  
> 18:59:14 2009
> @@ -180,7 +180,7 @@
>   std::vector<CalleeSavedInfo> CSI;
>   for (unsigned i = 0; CSRegs[i]; ++i) {
>     unsigned Reg = CSRegs[i];
> -    if (Fn.getRegInfo().isPhysRegUsed(Reg)) {
> +    if (Fn.getRegInfo().isPhysRegUsed(Reg) ||  
> Fn.doesHaveBuiltinSetjmp()) {

This is unfortunate, why does PEI need to know about this?  Shouldn't  
the 'Int_builtin_setjmp' use and def all these registers?


> +let Properties = [IntrNoMem] in {
> +def int_builtinsetjmp  : Intrinsic<[llvm_i32_ty],  [llvm_ptr_ty]>;
> +def int_builtinlongjmp : Intrinsic<[llvm_void_ty], [llvm_ptr_ty,  
> llvm_i32_ty]>;

If there is no reason to have both arguments, please drop the extra one.

> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +// SJLJ Exception handling intrinsics
> +//   setjmp() is a three instruction sequence to store the return  
> address

This is *not* setjmp and longjmp.  Please be very precise here.

-Chris