[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