<div class="socmaildefaultfont" dir="ltr" style="font-family:Arial, Helvetica, sans-serif;font-size:10.5pt" ><div dir="ltr" > </div>
<div dir="ltr" >Vedant,</div>
<div dir="ltr" > </div>
<div dir="ltr" >I attached an updated .diff with a filecheck regression test. Look good?</div>
<div dir="ltr" > </div>
<div dir="ltr" >I am far from a TableGen expert so I don't think I am the one to clean up emitEnums(), just had the bug fix and wanted to share it.</div>
<div dir="ltr" > </div>
<div dir="ltr" >Tyler</div>
<div dir="ltr" > </div>
<div dir="ltr" > </div>
<blockquote data-history-content-modified="1" dir="ltr" style="border-left:solid #aaaaaa 2px; margin-left:5px; padding-left:5px; direction:ltr; margin-right:0px" >----- Original message -----<br>From: Vedant Kumar <vsk@apple.com><br>To: Tyler Kenney/Marlborough/IBM@IBMUS<br>Cc: llvm-commits@lists.llvm.org<br>Subject: Re: [PATCH] TableGen InstrMapping Bug fix<br>Date: Mon, Nov 7, 2016 6:40 PM<br> 
<div><font face="Default Monospace,Courier New,Courier,monospace" size="2" >Your fix looks correct but should really have a test.<br><br>We don't have any existing tests which run -gen-instr-info, but it should be<br>possible. We'd just need to hit the emitEnums path with an entry that sets<br>FieldValues to {a, a, a}.<br><br>Side note: emitEnums() really needs to be cleaned up. It looks like it has<br>cubic runtime in the worst case, and it does what looks like a totally<br>unnecessary copy to initialize FieldValues.<br><br>best,<br>vedant<br><br>> On Nov 2, 2016, at 7:55 AM, Tyler Kenney via llvm-commits <llvm-commits@lists.llvm.org> wrote:<br>><br>>  <br>>  <br>> I believe I've found a bug in CodeGenMapTable.cpp.emitEnums(). I'm developing an out-of-tree backend and I was getting an error on a tablegen header for a duplicate enum value. emitEnums() was not properly deleting duplicate entries, but does with the attached the patch. Apparently none of the in-tree targets have enough InstrMapping's to trigger this bug, but my backend defines many.<br>>  <br>> I don't think there is a way to provide a test case for this without providing a whole new backend, so hopefully we can agree that it's not necessary. I've confirmed that all default targets still build with this patch applied.<br>>  <br>> Tyler<br>>  <br>>  <br>><br>> <CodeGenMapTable.diff>_______________________________________________<br>> llvm-commits mailing list<br>> llvm-commits@lists.llvm.org<br>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank" >http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></font><br> </div></blockquote>
<div dir="ltr" > </div></div><BR>