[PATCH] Bugfix for debug intrinsic handling in InstCombiner

[Eric Christopher]

On Tue, Apr 16, 2013 at 11:54 AM, Bob Wilson <bob.wilson at apple.com> wrote:
>
> On Apr 16, 2013, at 10:56 AM, Eric Christopher <echristo at gmail.com> wrote:
>
>> I'm not sure it's straightforward at all please tell me why you think so?
>
> I meant that the code in the patch is straightforward.  It is simply checking if a debug intrinsic call already exists and avoiding a duplicate if it's already there.  It sounds like you question whether this is the most general solution, but it's fixing a serious bug and is not likely to break anything, nor is it a new feature that is going to have implications throughout the compiler..
>

I'm questioning whether or not this should be an assert and the bug
fixed in a different location or if it's something that we can paper
over (which feels like this patch is doing) and chalk it up to the
"technical debt" associated with the current design.

I'm rather concerned about this approach as well, given how pervasive
and important our ability to support debug information is throughout
the compiler. Incremental fixes that get us to a desired goal are one
thing, however, I'm currently unconvinced that this is one and not
just a workaround for a problem somewhere else.

> I'm sure Adrian will respond to answer your questions about the circumstances that led to this happening.  If you have an idea for a more general solution, that would be great.  Until we have a more general solution, this is a safe fix that makes the situation better than it was before.
>

Given that I haven't gotten a description of what was wrong and what
went wrong I think that any sort of workaround as being
straightforward is not an obvious solution to anything. I speculated
as to a cause, I've still not heard one or seen a testcase and
description of how the problem arose. I'm happy to work with you for a
good solution as it is a serious bug that should be fixed.

-eric