[PATCH] D121768: [X86][tablgen] Auto-generate trivial fields and trivial interfaces for target features

[Tomas Matheson via Phabricator via llvm-commits]

tmatheson added a comment.

> Then I think it's important to have the flexibility to customize the interface, at least for now.

To be clear, I think that flexibility is a *bad* thing in this situation and not justified. Autogenerated code is already kind of magic, so it should be repetitive, boilerplate and consistent. Allowing people to tweak the code generation to behave inconsistently some of the time is a recipe for confusion. It will also encourage more deviation in future. If this is to be used in other backends as well, I would prefer a solution that resulted in a 1-1 relationship between getter and field. IIUC, this is in agreement with what @craig.topper is saying here:

> Every feature bit is exposed directly to FeatureBits vector at the MC layer using the name of the tablegen def. It is to our advantage to have consistent naming between the Def, the Subtarget field name, and the getter. Thus the concept of the getter being different than the Subtarget field doesn’t make sense because it means the MC layer is still wrong

To give a concrete example using `CMov` again, either the feature or the existing getter can be simply renamed, making it clear that `hasCMov()` is not simply checking for the feature:

  /// Autogenerated getter for the autogenerated field CMovFeature (not necessarily a good name).
  /// Renamed to avoid changing the name of hasCMov()
  bool hasCMovFeature() const { return HasCMovFeature; }
  
  /// People still have the flexibility to create a custom interface in a way that is explicit and obvious to the reader
  bool hasCMov() const { return hasCMov() || X86SSELevel >= SSE1 || is64Bit(); }

Given the above, I haven't seen any examples to justify the added complexity to the tablegen classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121768