[llvm-commits] [llvm] r94046 - /llvm/trunk/lib/CodeGen/DwarfEHPrepare.cpp

[Jim Grosbach]

On Jan 20, 2010, at 11:19 PM, Duncan Sands wrote:

> Hi Jim,
> 
>> +      // The SjLj dispatch block uses a switch instruction. This is effectively
>> +      // an unwind edge, so we can disregard it here. There will only ever
>> +      // be one dispatch, however, so if there are multiple switches, one
>> +      // of them truly is a normal edge, not an unwind edge.
>> +      if (SwitchOK && isa<SwitchInst>(PT)) {
>> +        SwitchOK = false;
>> +        continue;
>> +      }
> 
> it seems to me that this is only correct if SjLj *always* introduces a switch
> that branches to the edge.  Otherwise you can't know if a switch came from
> the original code or from SjLj lowering.  Can you please clarify.

Quite right. A dispatch switch instruction is indeed always used.


>  Also, rather than doing this:
> 
> +  bool usingSjLjEH = MAI->getExceptionHandlingType() == ExceptionHandling::SjLj;
> 
> you could pass it in as a boolean to the pass constructor.

Good idea. I'll have a look at that.

>  Finally, I don't
> much like passes magically knowing which other passes have run and what they
> have been up to - very GCC like :)  That said, I don't immediately see a nicer
> solution.

I totally agree it's a bit cringe-inducing. It's necessitated by the self-described hack in SelectionDAGISel::SelectAllBasicBlocks()  which tries to deal with eh.selectors that end up in blocks not an immediate unwind successor to an invoke. A hack has begat another hack, essentially.

I spent a good while trying to think of better options, but didn't come up with anything better. The best alternative I came up with was to remove the invokes, selectors, et.al. completely in the SjLj pass, but that would require a re-implementation all of the EH mechanisms that come afterwards to support the exception tables. While I'm not sure that wouldn't be a good thing in the long term, it's also a much larger project.

-Jim