[llvm-commits] [llvm] r140083 - /llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp

[Duncan Sands]

Hi Bill,

> If we are extracting a basic block that ends in an invoke call, we must also
> extract the landing pad block. Otherwise, there will be a situation where the
> invoke's unwind edge lands on a non-landing pad.
>
> We also forbid the user from extracting the landing pad block by itself. Again,
> this is not a valid transformation.
>
> Modified:
>      llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp?rev=140083&r1=140082&r2=140083&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp Mon Sep 19 18:00:52 2011
> @@ -664,7 +664,13 @@
>     //  * Pass in uses as args
>     // 3) Move code region, add call instr to func
>     //
> -  BlocksToExtract.insert(code.begin(), code.end());
> +  for (std::vector<BasicBlock*>::const_iterator
> +         I = code.begin(), E = code.end(); I != E; ++I) {
> +    BasicBlock *BB = *I;
> +    BlocksToExtract.insert(BB);
> +    if (InvokeInst *II = dyn_cast<InvokeInst>(BB->getTerminator()))
> +      BlocksToExtract.insert(II->getUnwindDest());
> +  }

I think the design of ExtractCodeRegion is that the caller should already have
taken care of this kind of thing.  For example, take a look at the following
lines (// Assumption: this is a single-entry code region, ...).
>
>     Values inputs, outputs;
>
> @@ -788,6 +794,7 @@
>   /// ExtractBasicBlock - slurp a basic block into a brand new function
>   ///
>   Function* llvm::ExtractBasicBlock(BasicBlock *BB, bool AggregateArgs) {
> +  if (BB->isLandingPad()) return 0;

Wouldn't it be better to put this logic in CodeExtractor::isEligible?

Ciao, Duncan.