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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D121768#3393897 <https://reviews.llvm.org/D121768#3393897>, @skan wrote:

> In D121768#3393895 <https://reviews.llvm.org/D121768#3393895>, @skan wrote:
>
>> In D121768#3393882 <https://reviews.llvm.org/D121768#3393882>, @craig.topper wrote:
>>
>>> In D121768#3393857 <https://reviews.llvm.org/D121768#3393857>, @skan wrote:
>>>
>>>> In D121768#3392554 <https://reviews.llvm.org/D121768#3392554>, @craig.topper wrote:
>>>>
>>>>> In D121768#3392153 <https://reviews.llvm.org/D121768#3392153>, @skan wrote:
>>>>>
>>>>>> 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, we can simply write such code
>>>>>>
>>>>>>   bool hasC() const { return false; }
>>>>>>
>>>>>> That's the flexibility and we could have more complicated cases.
>>>>>
>>>>> Since you would have to edit both the .td to make it non-trivial and then edit the .h file. Couldn't you instead rename the field in the .td so the autogenerated getter has a different name, and then add the custom getter using the old name?
>>>>
>>>> It's not straightforward vs my method. As I said, this is the simpliest case, non-trival interfaces allow us to untie and retie the dependencies in more complicated cases.  Another example is that we could add a knob like `-force-noC` to the backend, then define the interface like
>>>>
>>>>   bool hasC() const { return C && !ForceNoC; }
>>>>
>>>> so that we can control the dependency w/o building two compilers.
>>>
>>> I don't understand these use cases. How often are you doing these kinds of things? The dependencies in the compiler are supposed to be as loose as possible. BMI2 doesn't imply BMI1 for example.
>>
>> But AVX implies SSE.  In fact, it's the common case used in my development. Why shouldn't we have such usage?
>
> Let me tell another scenario. We could have different front ends for LLVM, e,g, clang, flang, aocc or anything else. Some of them are open-source and some of them are not.  We could use these front ends to emit the LLVM IR, then use LLVM middle/back end to optimize and tranlate it. New features can be transparent to the front end if backend can override the target features.  I have to say that "-mattr" is hard to use or not usable when LTO is enabled. At this time, adding knob in the backend is pretty useful. Flexibility is quite important.

So you want feature specific knobs that you have to pass through those front end command line interfaces with something like clang’s -mllvm?

In my opinion we should add more interfaces  in X86TargetParser.cpp so that all frontends can learn what features llvm supports without a hardcoded list like in Options.td. And those frontends should use the target-feature attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121768