[llvm-commits] [PATCH] fix bug in InstCombiner::visitPHINode()

[Frits van Bommel]

On Sat, Apr 16, 2011 at 2:46 PM, Jay Foad <jay.foad at gmail.com> wrote:
> I found this bug in InstCombiner::visitPHINode() by code inspection. I
> have no idea what symptoms it might cause, and I don't have a test
> case, but it looks obviously wrong to me: InValNo is an incoming value
> number, but it is used as an operand number, which is not the same
> thing.

It looks like the only symptom would be a missed optimization.
The incorrect code checks whether the later non-PHI incoming values
are all equal to either a basic block[1] or a previous incoming PHI
node[2], depending on whether the incoming value number is odd or
even.
The original value is also re-checked, and that check will only
succeed if it was the first incoming value
(getOperandNumForIncomingValue(0) == 0), so in all other cases it will
simply choose not to do anything.
The 'first incoming value' case was apparently the only case currently
tested, which was why this didn't show up in 'make check'

[1]: Which won't happen because they're not valid incoming values.
[2]: Which won't happen because they're treated differently by PHIsEqualValue().

> Tested with "make check". OK to apply?

LGTM.
I've constructed some tests for this which I'll check in afterwards.