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

[Jolanta Jensen via Phabricator via llvm-commits]

jolanta.jensen 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()});
----------------
mgabka wrote:
> to me this does not look right, what are you trying to do here?
> why not use getModule function?
Changed to getModule(). It's not a function defined in llvm::IntrinsicInst but inherited and I did not find it looking for something suitable. So I reused existing snippet of code.


================
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.
+
----------------
mgabka wrote:
> type, should be "replace with"
Fixed.


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


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


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-combine-to-u-forms.ll:2012
+
+; SVE2 - Uniform DSP operations
+
----------------
mgabka wrote:
> 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.
Added +sve2 to the attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152005