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

Kan Shengchen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 18 02:49:43 PDT 2022


skan added a comment.

In D121768#3391687 <https://reviews.llvm.org/D121768#3391687>, @tmatheson wrote:

>> 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.

Your method can not get rid of non-trivial feature like useAA, which has an virtual interface.  In addition, as a LLVM backend developer, I often need to hack these interfaces to do some performance tuning or testing work, so the flexibility is quite important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121768



More information about the llvm-commits mailing list