[PATCH] D152005: [SVE ACLE] Implement IR combines to convert intrinsics used for _m C/C++ builtins

[mgabka via Phabricator via llvm-commits]

mgabka added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1292
+
+  auto *Mod = II.getParent()->getParent()->getParent();
+  auto *NewDecl = Intrinsic::getDeclaration(Mod, IID, {II.getType()});
----------------
to me this does not look right, what are you trying to do here?
why not use getModule function?


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-combine-to-u-forms.ll:11
+
+; Replace SVE merging intrinsics to their equivalent undef (_u) variants when they take an all active predicate.
+
----------------
type, should be "replace with"


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-combine-to-u-forms.ll:13
+
+; Float arithmetics
+
----------------
to me the singular form is more correct in this context


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-combine-to-u-forms.ll:726
+
+; Integer arithmetics
+
----------------
to me the singular form is more correct in this context


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-combine-to-u-forms.ll:2012
+
+; SVE2 - Uniform DSP operations
+
----------------
if these are SVE2 instructions (and they are implemented like that currently in LLVM) then the attached attribute to these functions isn't correct, it should have sve2 attribute attached otherwise those intrinsics can not be code generated. you can try to run llc on this file and check it yourself  , you will observe a compiler crash with the current version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152005