[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

Tue Feb 21 18:54:39 PST 2023

zixuan-wu added a comment.

In D141899#4065375 <https://reviews.llvm.org/D141899#4065375>, @nikic wrote:

> In D141899#4061237 <https://reviews.llvm.org/D141899#4061237>, @zixuan-wu wrote:
>
>> With considering https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility I think we need make consensus to choose one option from following 2 options.
>>
>> 1. Remove X86amx type in IR totally. (what I am doing now)
>> 2. Without removing X86amx type in IR, just upgrade the x86amx type to target extension and also upgrade bitcast llvm instruction to intrinsic(required). It also includes changing the testcase to target extension type.
>
> I believe the right option is:
>
> 3. Remove x86_amx type from the (in-memory) IR representation, but support an auto-upgrade for bitcode only.
>
> We do need bitcode auto-upgrade support as a matter of policy, and we shouldn't support both type representation at the same time, that would defeat the point of the change.
>
> In D141899#4061174 <https://reviews.llvm.org/D141899#4061174>, @LuoYuanke wrote:
>
>> I think target extension type is nice, if it is introduced 2 years ago I would vote for it. However my concern is the compatibility issue as I explained. We need to be compatible to the IR that built by previous compiler, and be compatible to the 3rd party software that based on the x86_amx type. I can't predict more risks for now if we replace an LLVM IR type, but I believe there is big risk hidden.
>
> Due to bitcode auto-upgrade, compatibility with old IR is retained. As long as we avoid some of the API changes here, the impact on downstream code should be fairly minimal. (Though as already pointed out, downstream impact generally doesn't figure into LLVM design decisions anyway.)

Deal. And because I am on busy for a long time and it is also better to let intel guy handle x86-related feature, I am happy with the patch being commandeered.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141899