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

Fri Mar 18 22:00:16 PDT 2022

skan added a comment.

In D121768#3393936 <https://reviews.llvm.org/D121768#3393936>, @craig.topper wrote:

> In D121768#3393935 <https://reviews.llvm.org/D121768#3393935>, @skan wrote:
>
>> In D121768#3393923 <https://reviews.llvm.org/D121768#3393923>, @craig.topper wrote:
>>
>>> In D121768#3393914 <https://reviews.llvm.org/D121768#3393914>, @skan wrote:
>>>
>>>> In D121768#3393896 <https://reviews.llvm.org/D121768#3393896>, @craig.topper wrote:
>>>>
>>>>> But I guarantee the compiler will break if you force SSE to false and keep AVX. So that’s not a reasonable case.
>>>>
>>>> It's a check in `X86ISelLowering.cpp`, we can relax it.
>>>
>>> Yes, but if you did that work then you should probably change the avx to not imply sse.
>>>
>>> The dependencies we have are largely due to implementation details of the features within the compiler. If we relax those implementation details we should relax the dependencies. For the most part I don’t think you can just relax a dependency without making additional code changes.
>>>
>>> AVX512 implementation has quite a few special cases to avoid assuming dependencies except where it was really necessary. It would be simpler if avx512bw implied avx512 dq for example. But no documentation exists of dependencies.
>>
>> I agree that the correct approach is to relax the dependencies too. But it takes more effort and not quick enough.
>
> I'm saying that a dependency currently exists, that changing the subtarget "has" function will most likely result in a crash or a miscompile without other changes. Do you disagree with that?

I agree, but we can improve.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121768