[PATCH] D156439: [TLI][AArch64] Extend ReplaceWithVeclib to replace vector FREM instructions for scalable vectors

[mgabka via Phabricator via llvm-commits]

mgabka added inline comments.


================
Comment at: llvm/lib/CodeGen/ReplaceWithVeclib.cpp:92-111
+  CallInst *Replacement = nullptr;
+  if (CI) {
+    // Intrinsics handling.
+    SmallVector<Value *> Args(CI->args());
+    // Preserve the operand bundles.
+    SmallVector<OperandBundleDef, 1> OpBundles;
+    CI->getOperandBundlesAsDefs(OpBundles);
----------------
mgabka wrote:
> this can be written in much more compacted way for example:
> 
> ```
> SmallVector<Value *> Args(I.operand_values());
> SmallVector<OperandBundleDef, 1> OpBundles;
> // Preserve the operand bundles if it is an intrinsic call.
> if (CI)
>   CI->getOperandBundlesAsDefs(OpBundles);
> // for masked calls to frem add a mask operand =
> else if (Masked)
>        Args.push_back(IRBuilder.getAllOnesMask(NumElements));
> 
> CallInst *Replacement = IRBuilder.CreateCall(TLIFunc, Args, OpBundles);
> if (isa<FPMathOperator>(Replacement)) {
>     // Preserve fast math flags for FP math.
>     Replacement->copyFastMathFlags(&I);
>   }
> ```
I was wrong here, when the Instruction is a CI has the call as the first operand so it needs to be a bit different, I also think that it would be good to handle always mask (assuming it is always the last operand)

```
SmallVector<Value *> Args;
SmallVector<OperandBundleDef, 1> OpBundles;
// Preserve the operand bundles and copy arguments if it is an intrinsic call.
if (CI) {
  Args.assign(CI->arg_begin(), CI->arg_end());
  CI->getOperandBundlesAsDefs(OpBundles);
 }
else
  Args.assign(I.op_begin(), I.op_end());
// if mask is requested we need to add it
if (Masked)
  Args.push_back(IRBuilder.getAllOnesMask(NumElements));

CallInst *Replacement = IRBuilder.CreateCall(TLIFunc, Args, OpBundles);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156439