[PATCH] D121768: [tablgen][X86] Auto-generate trival fields and interace for target features

[Tomas Matheson via Phabricator via llvm-commits]

tmatheson added a comment.

> This patch makes a distinction between trivial/nontrivial members & interface b/c some targets has some tricky interfaces, e.g
>
>   bool hasCMov() const { return HasCMov || X86SSELevel >= SSE1 || is64Bit(); }
>   bool useAA() const override { return UseAA; }
>   bool hasLAHFSAHF() const { return HasLAHFSAHF64 || !is64Bit(); }
>
> It seems that you patch handle them incorrectly by now.

It would be better to take this opportunity to fix these inconsistencies between the getter methods and the fields, rather than adding complexity to the tablegen to keep them. If `HasCMov` is false it is counterintuitive for `hasCMov` to return true. I handled this by the getter always matching the field name, e.g.

  bool enablePostRAScheduler() const override { return usePostRAScheduler(); }
  bool useNEONForSinglePrecisionFP() const { return hasNEON() && hasNEONForFP(); }



> A field may be set to false by a feature while the default value of the field can either be true or false at the same time. So I provide a way to avoid generating field that is not zero-initialized. But your assumption is that the default value of the field must be true if a feature set it to false.

How many examples of this are there, and are they intentional? This also seems like an inconsistency that could lead to confusion. If a field has default value false and enabling the feature sets it to false, at what point is it set to true and which takes precedence?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121768/new/

https://reviews.llvm.org/D121768