[PATCH] D152005: [SVE ACLE] Implement IR combines to convert intrinsics used for _m C/C++ builtins
Jolanta Jensen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 6 10:05:56 PDT 2023
jolanta.jensen added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1331-1350
+instCombineSVEAllActive2VA(InstCombiner &IC, IntrinsicInst &II) {
+ auto *OpPredicate = II.getOperand(0);
+ auto *OpA = II.getOperand(1);
+ auto *OpB = II.getOperand(2);
+ if (!match(OpPredicate, m_Intrinsic<Intrinsic::aarch64_sve_ptrue>(
+ m_ConstantInt<AArch64SVEPredPattern::all>())))
+ return std::nullopt;
----------------
paulwalker-arm wrote:
> Rather than have nested switch statements perhaps you can pass the new `IntrinsicID` into this function?
>
> Since both this function and `instCombineSVEAllActive3VA` don't change the original call's operands, I think you should be able to make use of II.args(). I'm not sure if there's a variant of `CreateIntrinsic` that'll take this directly but if not, you should be able to create a `SmallVector` from it and pass that to `CreateIntrinsic`.
Fixed. So much better now :)
================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1338-1339
+ return std::nullopt;
+ IRBuilder<> Builder(II.getContext());
+ Builder.SetInsertPoint(&II);
+ switch (II.getIntrinsicID()) {
----------------
paulwalker-arm wrote:
> `InstCombiner` comes with its own `IRBuilder` already set up. It should be as simple as replacing your new `Builder.Create...`lines with `IC.Builder.Create...`. You'll see the existing SVE functions have been updated similarly.
Changed to InstCombiner own IRBuilder.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152005/new/
https://reviews.llvm.org/D152005
More information about the llvm-commits
mailing list