[PATCH] TableGen InstrMapping Bug fix

[Vedant Kumar via llvm-commits]

> On Nov 23, 2016, at 5:50 PM, Vedant Kumar <vsk at apple.com> wrote:
> 
> 
>> On Nov 22, 2016, at 12:47 PM, Tyler Kenney <tjkenney at us.ibm.com> wrote:
>> 
>> 
>> Vedant,
>> 
>> I attached an updated .diff with a filecheck regression test. Look good?
> 
> Hm, I don't see the diff anywhere. Maybe it got scrubbed?
> 
> 
>> 
>> 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.
> 
> Fair enough :).

This is what I had in mind:

  for (auto &Entry : ColFieldValueMap) {                                           
    std::vector<Init*> FieldValues;                                                
    const auto &InitialFieldValues = Entry.second;                                 
                                                                                   
    // Delete duplicate entries from ColFieldValueMap                              
    for (Init *Val : InitialFieldValues)                                           
      if (std::find(FieldValues.begin(), FieldValues.end(), Val) == FieldValues.end())
        FieldValues.push_back(Val); 

vedant

> 
> vedant
> 
> 
>> 
>> Tyler
>> 
>> 
>> ----- Original message -----
>> From: Vedant Kumar <vsk at apple.com>
>> To: Tyler Kenney/Marlborough/IBM at IBMUS
>> Cc: llvm-commits at lists.llvm.org
>> Subject: Re: [PATCH] TableGen InstrMapping Bug fix
>> Date: Mon, Nov 7, 2016 6:40 PM
>> 
>> Your fix looks correct but should really have a test.
>> 
>> We don't have any existing tests which run -gen-instr-info, but it should be
>> possible. We'd just need to hit the emitEnums path with an entry that sets
>> FieldValues to {a, a, a}.
>> 
>> Side note: emitEnums() really needs to be cleaned up. It looks like it has
>> cubic runtime in the worst case, and it does what looks like a totally
>> unnecessary copy to initialize FieldValues.
>> 
>> best,
>> vedant
>> 
>>> On Nov 2, 2016, at 7:55 AM, Tyler Kenney via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> 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.
>>> 
>>> Tyler
>>> 
>>> 
>>> 
>>> <CodeGenMapTable.diff>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> 
>> 
>> 
>