[PATCH] D16141: [TableGen] Fix sort order of asm operand classes

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 18 16:15:47 PST 2016


Thanks for working on this!

FWIW this fix looks right to me, but I'm not sure I've ever committed
to tablegen.  Please track down someone that knows the code better to
chime in.

> +  /// operator< - Compare two classes. This does not produce a total ordering,
> +  /// but does guarantee that subclasses are sorted before their parents, and
> +  /// that the ordering is transitive.

Since you're changing the comment anyway, can you update the style
(remove the function name at the start of it)?

> On 2016-Jan-13, at 04:01, Oliver Stannard <oliver.stannard at arm.com> wrote:
> 
> olista01 created this revision.
> olista01 added reviewers: dblaikie, dexonsmith.
> olista01 added a subscriber: llvm-commits.
> olista01 set the repository for this revision to rL LLVM.
> Herald added a subscriber: aemerson.
> 
> This is a fix for https://llvm.org/bugs/show_bug.cgi?id=22796.
> 
> The previous implementation of ClassInfo::operator< allowed cycles of classes such that x < y < z < x, meaning that a list of them cannot be correctly sorted, and the sort order could differ with different standard libraries.
> 
> The original implementation sorted classes by ValueName if they were otherwise equal. This isn't strictly necessary, but some backends seem to accidentally rely on it. If I reverse this comparison I get 8 test failures spread across the AArch64, Mips and X86 backends, so I have left it in until those backends can be fixed.
> 
> There was one case in the X86 backend where the observable behaviour of the assembler is changed by this patch. This was because some of the memory asm operands were not marked as children of X86MemAsmOperand.
> 
> This bug caused my patch to add the ARMv8.2 FP16 AArch32 scalar instructions (http://reviews.llvm.org/rL255762) to fail on windows.
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D16141
> 
> Files:
>  lib/Target/X86/X86InstrInfo.td
>  utils/TableGen/AsmMatcherEmitter.cpp
> 
> <D16141.44733.patch>



More information about the llvm-commits mailing list