[PATCH] D29240: IR: Consider two DISubprograms to be odr-equal if they have the same template parameters.

[Duncan P. N. Exon Smith via llvm-commits]

> On 2017-Feb-03, at 17:34, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2017-Feb-03, at 15:55, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> 
>> On Fri, Feb 3, 2017 at 3:34 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> I spent some more time on this and I understand the failure.
>> 
>> I'm really uncomfortable with the fix, though.  It seems partial -- it fixes the particular crash you found, but couldn't this happen with other fields in DISubprogram?
>> 
>> Maybe -- I guess that's what I was alluding to in my commit message. Do you think anything could break if I just remove the MDNodeSubsetEqualImpl specialization of DISubprogram?
> 
> Yes.  I suspect we'll get a big regression (compile-time? memory?) in -flto=full, and possible a regression in -flto=thin (which is what I was targeting at the time).  IIRC, the point of this was to (a) reduce number of nodes and (b) cut down on ValueMapper traffic (ironically, the same thing that it breaks).

(It's possible that this didn't affect -flto=full much, and the more recent changes to -flto=thin make this optimization unnecessary.)

> Of course, correctness should trump performance, but I'm pretty concerned about turning it off blind.  I'm also still looking for other options...
> 
>> Also, where does the ValueMapper run in this opt invocation?  Is that what -run-twice does?
>> 
>> Yes, -run-twice causes us to clone the module, which uses ValueMapper. See http://llvm-cs.pcc.me.uk/tools/opt/opt.cpp#719
> 
> Got it.
> 
>> Thanks,
>> -- 
>> -- 
>> Peter
>