[PATCH] D113489: [AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:718
+             m_Intrinsic<Intrinsic::aarch64_sve_ptrue>(m_Value(Predicate))))
+    return None;
+  if (!match(Predicate, m_ConstantInt<AArch64SVEPredPattern::all>()))
----------------
paulwalker-arm wrote:
> MattDevereau wrote:
> > paulwalker-arm wrote:
> > > Can we use a masked load for this scenario also? As in masked_load doesn't care how the mask was constructed.
> > > 
> > > My feeling is `instCombineSVEMaskedLD1` and `instCombineSVELD1` are called in the wrong order.  It seems easier to always call  `instCombineSVEMaskedLD1` and then call `instCombineSVELD1` for the case when
> > > ```
> > > match(II.getOperand(0),
> > >              m_Intrinsic<Intrinsic::aarch64_sve_ptrue>(m_ConstantInt<AArch64SVEPredPattern::all>()))
> > > ```
> > > is true?
> > Using the match you suggested and always calling `instCombineSVEMaskedLD1` would end up instCombining and erasing `aarch64.sve.convert.from.svbool`, predicates I think? Is this ok?
> I don't understand your reasoning here.
> 
> `aarch64.sve.convert.from.svbool` is just an intrinsic that produces a predicate.  If that is the predicate operand to the `ld1` then it should end up being the mask operand of the masked_load.
> 
> For the case where the predicate operand is specifically `ptrue 31` we can drop the predicate operand and call `Builder.CreateLoad` with all other cases passing all the operands to `Builder.CreateMaskedLoad`.  Perhaps that means we don't even need instCombineSVELD1.
Nevermind, I realised I worded my question incorrectly and then even when worded correctly it made no sense either. I've changed the match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113489