[llvm-commits] r133708 strikes me again? - the case clang stalls

[Jay Foad]

On 9 August 2011 12:12, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> 2011/8/9 Jay Foad <jay.foad at gmail.com>:
>> On 9 August 2011 11:40, Jay Foad <jay.foad at gmail.com> wrote:
>>> On 9 August 2011 06:58, John McCall <rjmccall at apple.com> wrote:
>>>> I think your patch to LLVM is a good idea;  replaceAllUsesWith should
>>>> not be assuming a fully-formed AST.  As a slight optimization, I would
>>>> suggest doing this instead:
>>>>  if (Succ->empty()) continue;
>>>> Jay, does that seem reasonable?
>>>
>>> Sure, I'm fine with either your or Takumi's fix. Thanks for taking the
>>> time to investigate.
>>
>> Hang on... if we need to cope with half-baked IR, won't your fix still
>> fall over on a BB that contains some phi nodes but nothing else?
>
> Jay, yeah, I was afraid that case. (it was the reason I proposed
> checking iterator end)

I like your fix, Takumi.

Personally I would write it like this:

    // N.B. Succ might not be a complete BasicBlock, so don't assume
that it ends with a non-phi instruction.
    for (iterator II = Succ->begin(), IE = Succ->end(); II != IE; ++II) {
      PHINode *PN = dyn_cast<PHINode>(II);
      if (!PN)
        break;
      ...
    }

but that's just bike-shedding.

Jay.