<html><body><p>Yes please, I do not have commit authority.<br><br>If you prefer the code snippet you sent earlier, that works for me too. It looks to me like both solutions would fix the bug (although I didn't test yours), but yours would provide a few other enhancements as well. Whichever works, so if you could commit one of them along with the reg test, that would be great.<br><br>Thanks,<br>Tyler<br><br><br><img width="16" height="16" src="cid:1__=0ABB0AEFDFD9F1AC8f9e8a93df938690918c0AB@" border="0" alt="Inactive hide details for Vedant Kumar ---11/30/2016 06:27:36 PM---I'm sorry for the delay. This LGTM. Let me know if you'd lik"><font color="#424282">Vedant Kumar ---11/30/2016 06:27:36 PM---I'm sorry for the delay. This LGTM. Let me know if you'd like me to commit this for you.</font><br><br><font size="2" color="#5F5F5F">From: </font><font size="2">Vedant Kumar <vsk@apple.com></font><br><font size="2" color="#5F5F5F">To: </font><font size="2">Tyler Kenney/Marlborough/IBM@IBMUS</font><br><font size="2" color="#5F5F5F">Cc: </font><font size="2">llvm-commits@lists.llvm.org</font><br><font size="2" color="#5F5F5F">Date: </font><font size="2">11/30/2016 06:27 PM</font><br><font size="2" color="#5F5F5F">Subject: </font><font size="2">Re: [PATCH] TableGen InstrMapping Bug fix</font><br><font size="2" color="#5F5F5F">Sent by: </font><font size="2">vsk@apple.com</font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br><tt>I'm sorry for the delay. This LGTM.<br><br>Let me know if you'd like me to commit this for you.<br><br>thanks,<br>vedant<br><br><br>> On Nov 23, 2016, at 3:03 PM, Tyler Kenney <tjkenney@us.ibm.com> wrote:<br>> <br>> Hm, I may have forgotten it. I attached it again, but I'll also copy it below just in case.<br>> <br>> Tyler<br>> <br>> <br>> Index: test/TableGen/DuplicateFieldValues.td<br>> ===================================================================<br>> --- test/TableGen/DuplicateFieldValues.td (revision 0)<br>> +++ test/TableGen/DuplicateFieldValues.td (revision 0)<br>> @@ -0,0 +1,84 @@<br>> +// RUN: llvm-tblgen -gen-instr-info -I%S/../../include %s | FileCheck %s<br>> +<br>> +// CHECK: ABCForm_A<br>> +// CHECK-NOT: ABCForm_A<br>> +<br>> +//<br>> +// include Target.td for InstrMapping class and define minimally required objects<br>> +//<br>> +<br>> +include "llvm/Target/Target.td"<br>> +<br>> +class DFVReg<string n> : Register<n> {<br>> + let Namespace = "DFV";<br>> +}<br>> +<br>> +def R0 : DFVReg<"r0">;<br>> +def DFVRegClass : RegisterClass<"DFV",[i32],0,(add R0)>;<br>> +def DFVInstrInfo : InstrInfo;<br>> +<br>> +def DFVTest : Target {<br>> + let InstructionSet = DFVInstrInfo;<br>> +}<br>> +<br>> +//<br>> +// Define a number of a InstrMappings with repeated ValueCol fields<br>> +//<br>> +<br>> +class ABCRel;<br>> +<br>> +def getAFormFromBForm : InstrMapping {<br>> + let FilterClass = "ABCRel";<br>> + let RowFields = ["BaseName"];<br>> + let ColFields = ["ABCForm"];<br>> + let KeyCol = ["B"];<br>> + let ValueCols = [["A"]];<br>> +}<br>> +<br>> +def getAFormFromCForm : InstrMapping {<br>> + let FilterClass = "ABCRel";<br>> + let RowFields = ["BaseName"];<br>> + let ColFields = ["ABCForm"];<br>> + let KeyCol = ["C"];<br>> + let ValueCols = [["A"]];<br>> +}<br>> +<br>> +def getAFormFromDForm : InstrMapping {<br>> + let FilterClass = "ABCRel";<br>> + let RowFields = ["BaseName"];<br>> + let ColFields = ["ABCForm"];<br>> + let KeyCol = ["D"];<br>> + let ValueCols = [["A"]];<br>> +}<br>> +<br>> +def getAFormFromEForm : InstrMapping {<br>> + let FilterClass = "ABCRel";<br>> + let RowFields = ["BaseName"];<br>> + let ColFields = ["ABCForm"];<br>> + let KeyCol = ["E"];<br>> + let ValueCols = [["A"]];<br>> +}<br>> +<br>> +class I : Instruction {<br>> + let Namespace = "DFV";<br>> + let OutOperandList = (outs);<br>> + let InOperandList = (ins);<br>> +<br>> + string BaseName = "";<br>> + string ABCForm = "";<br>> +}<br>> +<br>> +class isAForm { string ABCForm = "A"; }<br>> +class isBForm { string ABCForm = "B"; }<br>> +class isCForm { string ABCForm = "C"; }<br>> +class isDForm { string ABCForm = "D"; }<br>> +class isEForm { string ABCForm = "E"; }<br>> +<br>> +let BaseName = "0" in {<br>> + def A0 : I, ABCRel, isAForm;<br>> + def B0 : I, ABCRel, isBForm;<br>> + def C0 : I, ABCRel, isCForm;<br>> + def D0 : I, ABCRel, isDForm;<br>> + def E0 : I, ABCRel, isEForm;<br>> +}<br>> +<br>> Index: utils/TableGen/CodeGenMapTable.cpp<br>> ===================================================================<br>> --- utils/TableGen/CodeGenMapTable.cpp (revision 285805)<br>> +++ utils/TableGen/CodeGenMapTable.cpp (working copy)<br>> @@ -542,6 +542,7 @@<br>> for (unsigned j = i+1; j < FieldValues.size(); j++) {<br>> if (CurVal == FieldValues[j]) {<br>> FieldValues.erase(FieldValues.begin()+j);<br>> + --j;<br>> }<br>> }<br>> }<br>> <br>> (See attached file: TableGenInstrMappingDuplicateBugFix.diff)<br>> <br>> <graycol.gif>Vedant Kumar ---11/23/2016 05:51:53 PM---> On Nov 22, 2016, at 12:47 PM, Tyler Kenney <tjkenney@us.ibm.com> wrote: ><br>> <br>> From: Vedant Kumar <vsk@apple.com><br>> To: Tyler Kenney/Marlborough/IBM@IBMUS<br>> Cc: llvm-commits@lists.llvm.org<br>> Date: 11/23/2016 05:51 PM<br>> Subject: Re: [PATCH] TableGen InstrMapping Bug fix<br>> Sent by: vsk@apple.com<br>> <br>> <br>> <br>> <br>> <br>> > On Nov 22, 2016, at 12:47 PM, Tyler Kenney <tjkenney@us.ibm.com> wrote:<br>> > <br>> > <br>> > Vedant,<br>> > <br>> > I attached an updated .diff with a filecheck regression test. Look good?<br>> <br>> Hm, I don't see the diff anywhere. Maybe it got scrubbed?<br>> <br>> <br>> > <br>> > 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.<br>> <br>> Fair enough :).<br>> <br>> vedant<br>> <br>> <br>> > <br>> > Tyler<br>> > <br>> > <br>> > ----- 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>> > <br>> > 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>> > > </tt><tt><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></tt><tt><br>> > <br>> > <br>> > <br>> <br>> <br>> <br>> <TableGenInstrMappingDuplicateBugFix.diff><br><br></tt><br><BR>
</body></html>