[PATCH] D82483: [SVE] Code generation for fixed length vector adds.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm marked 4 inline comments as done.
paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14897
+    Operands.push_back(V);
+  }
+
----------------
cameron.mcinally wrote:
> Could we modify the assert to remove the branch?
> 
> ```
> assert(isa<CondCodeSDNode>(V) || V.getValueType().isScalableVector() &&
>            "Only scalable vectors are supported!");
> ```
> 
> Same with the code above too.
I can modify this case but the earlier code is not a straight copy so some kind of control flow is required. 


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1618
+  def : SVE_3_Op_Pat_SelZero<nxv2f64, int_op, nxv2i1, nxv2f64, nxv2f64, !cast<Pseudo>(NAME # _ZERO_D)>;
 }
 
----------------
cameron.mcinally wrote:
> Is having both ir_op and int_op problematic going forward? E.g. how to match an intrinsic with an undef merge.
> 
> X86 solves a similar problem with the tables in `lib/Target/X86/X86IntrinsicsInfo.h`. That might be something to consider long term. I'm not sure if it's a great fit though.
> 
> I don't think this needs to be changed now, but something to consider...
> 
I don't believe there's anything problematic as there's nothing to prevent adding SVE_3_Op_Pat_SelZero patterns for ir_op and SVE_3_Op_Pat for int_op.  It's just that this patch does not have tests for those cases so I've not added the patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82483