[llvm-commits] [llvm] r79250 - in /llvm/trunk: include/llvm/CodeGen/Passes.h lib/CodeGen/AsmPrinter/DwarfException.cpp lib/CodeGen/AsmPrinter/DwarfException.h lib/CodeGen/LLVMTargetMachine.cpp lib/CodeGen/SjLjEHPrepare.cpp

[Jim Grosbach]

On Aug 17, 2009, at 1:50 PM, Evan Cheng wrote:

>
> On Aug 17, 2009, at 9:41 AM, Jim Grosbach wrote:
>
>> Author: grosbach
>> Date: Mon Aug 17 11:41:22 2009
>> New Revision: 79250
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=79250&view=rev
>> Log:
>> Move the sjlj exception handling conversions to a back-end pass  
>> where they
>> more properly belong. This allows removing the front-end  
>> conditionalized
>> SJLJ code, and cleans up the generated IR considerably. All of the
>> infrastructure code (calling _Unwind_SjLj_Register/Unregister, etc)  
>> is
>> added by the SjLjEHPrepare pass.
>
> Cool. Are we passing c++ tests now?

We're doing better, yes. I'm running the tests to get a better feel  
for just how much better. It's not perfect yet, but it's better.  
There's a rethrow problem that's being squirrely at the moment.

>
> Some preliminary (most cosmetic) comments inline below.
>

r79287.

>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp Mon Aug 17  
>> 11:41:22 2009
>> @@ -241,8 +241,11 @@
>>  // handle.
>>  switch (getTargetAsmInfo()->getExceptionHandlingType())
>>  {
>> -  // SjLj piggy-backs on dwarf for this bit
>>  case ExceptionHandling::SjLj:
>> +    // SjLj piggy-backs on dwarf for this bit. The cleanups done  
>> apply to both
>> +    PM.add(createDwarfEHPass(getTargetLowering(),  
>> OptLevel==CodeGenOpt::None));
>> +    PM.add(createSjLjEHPass(getTargetLowering()));
>
> What part of the dwarf EH pass is needed for SJLJ EH? Is it possible  
> to factor that part out?

It's mostly cleanup to make sure things are as the DwarfException  
stuff expects it (selector calls always in landing pads and things  
like that). It should be very possible to factor it out. I ran out of  
steam around 1am and didn't get that far.

>>
>> +#include <set>
>
> Can this be replaced with SmallPtrSet.h?

Don't think it's needed at all, actually. Removed.

I'll poke about in a bit and see if there's other headers that can be  
removed, too.

>>
>> +// live across unwind edges.  Each value that is live across an  
>> unwind edge
>> +// we spill into a stack location, guaranteeing that there is  
>> nothing live
>> +// across the unwind edge.  This process also splits all critical  
>> edges
>> +// coming out of invoke's.
>
> Function level comment should start with  
> "splitLiveRangesLiveAcrossInvokes -".
>

Done.

>> +      std::set<InvokeInst*> InvokesWithStoreInserted;
>
> This std::set is not used anywhere?
>

Fixed.

>>
>> +bool SjLjEHPass::insertSjLjEHSupport(Function &F) {
>> +  std::vector<ReturnInst*> Returns;
>> +  std::vector<UnwindInst*> Unwinds;
>> +  std::vector<InvokeInst*> Invokes;
>
> SmallVector please.
>
>>
>> +    std::vector<CallInst*> EH_Selectors;
>> +    std::vector<CallInst*> EH_Exceptions;
>
> SmallVector.

Will do.


>
>> +  for (Function::iterator BB = F.begin(), E = F.end(); BB != E; + 
>> +BB) {
>> +  //  for (unsigned i = 0, e = Invokes.size(); i != e; ++i) {
>> +//      BasicBlock *Pad = Invokes[0]->getUnwindDest();
>
> Please delete this line?

Ack. Oops. Done.