[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 05:53:16 PDT 2022


skan added a comment.

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

>> Your method can not get rid of non-trivial feature like useAA, which has an virtual interface.
>
> I gave an example above of exactly this:
>
>   bool enablePostRAScheduler() const override { return usePostRAScheduler(); }
>
>
>
>> 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.
>
> Could you be more specific? I can't imagine how consistent getter/field naming prevents performance or testing work.
>
> Still seeing no justification for the added complexity of 2 new fields to SubtargetFeature, 3 new SubtargetFeature subclasses, in order to avoid a couple of function renames. We already have custom predicates.

Let me tell the scenario:

The features defined in TD file have dependencies between each other. The simpliest case is that both feature A and feature B depends on feature C due to any historic HW reason. When we disable C by passing
knobs like `-mattr=-C` or `-mnoC`, feature A and B will be disabled too.  However, it's possible either A or B does not depends on C in SW, namely, compiler could emit ISA in A while not emitting ISA in C.
Handwritten (non-trivial) interface gives us an quick way to untie such dependencies. If we'd like to disable C w/o affecting A and B by simply writting such code

  bool hasC() const { return false; }

That's the flexibility. And this is the simpliest case, we could have more complicated cases.

Besides, I think adding two bits to a class is very common in a DSL (tablgen).  It's worthy b/c flexibility is kept and code is reduced at same time.


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