nikic added a comment.
From a very cursory look, this looks pretty nice. It looks like we pretty much
get rid of AMX-specific knowledge in the middle-end entirely, which is the
point of target extension types.
Regarding bitcasts, it's worth mentioning that there was some recent discussion
about allowing bitcasts on target types as one of the supported properties --
however, I think we wouldn't actually want to do that for AMX, because of the
special properties it has. In particular, if we allow bitcasts, we should also
allow bitcast optimizations, and those are generally illegal for AMX (as seen
with the various checks this patch removes).
Something this is missing is auto-upgrade support in the bitcode reader.
Nitpick: I wonder whether the type should be called `target("x86.amx")` -- it
seems like lowercase names are more idiomatic in LLVM? No strong feelings
either way though.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5338
+ if (TargetExtTy->getName() == "x86.AMX")
+ return true;
+ return false;
----------------
Probably worthwhile to add an overload like `Type::isTargetExtTy(StringRef)`?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5373
+ Builder.CreateIntrinsic(Intrinsic::x86_bitconvert_vector_to_tile,
+ {}, {ArgValue});
+ } else {
----------------
Something I don't really get is why we need the separate bitconvert intrinsic,
and the existing cast intrinsic does not work. Do they have different semantics?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141899/new/
https://reviews.llvm.org/D141899
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
