[PATCH] D123449: [AArch64][SelectionDAG] Refactor to support more scalable vector extending stores

[Allen zhong via Phabricator via llvm-commits]

Allen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7780
   if (StVT.isScalableVector())
-    report_fatal_error("Cannot scalarize scalable vector stores");
 
----------------
paulwalker-arm wrote:
> Allen wrote:
> > Allen wrote:
> > > paulwalker-arm wrote:
> > > > Thanks for the change @Allen.  Before accepting I just wanted to query this line.  What's the circumstance where it now triggers? (I tried `llvm-lit -v ../llvm-project/llvm/test/CodeGen/AArch64` without this change and nothing jumped out)  I ask because it's not immediate clear to me that the callers are happy for the scalarisation request to be ignored, hence the original error message.
> > > hi @paulwalker-arm，If we revert the following change, then we'll have 5 fail cases. I think none of them are Legal should also be work, as we pick them out are taken into account of the performance.
> > > 
> > > ```
> > > +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
> > > @@ -1214,12 +1214,6 @@ AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM,
> > >      }
> > >  
> > >      // Then, selectively enable those which we directly support.
> > > -    setTruncStoreAction(MVT::nxv2i64, MVT::nxv2i8, Legal);
> > > -    setTruncStoreAction(MVT::nxv2i64, MVT::nxv2i16, Legal);
> > > -    setTruncStoreAction(MVT::nxv2i64, MVT::nxv2i32, Legal);
> > > -    setTruncStoreAction(MVT::nxv4i32, MVT::nxv4i8, Legal);
> > > -    setTruncStoreAction(MVT::nxv4i32, MVT::nxv4i16, Legal);
> > > -    setTruncStoreAction(MVT::nxv8i16, MVT::nxv8i8, Legal);
> > > ```
> > > 
> > > the related failed cases
> > > ```
> > >  LLVM :: CodeGen/AArch64/spillfill-sve.ll
> > >   LLVM :: CodeGen/AArch64/sve-forward-st-to-ld.ll
> > >   LLVM :: CodeGen/AArch64/sve-split-store.ll
> > >   LLVM :: CodeGen/AArch64/sve-st1-addressing-mode-reg-reg.ll
> > >   LLVM :: CodeGen/AArch64/sve-trunc.ll
> > > ```
> > @paulwalker-arm, need I revert this line's change ?
> Your `setTruncStoreAction` related changes are fine.  I'm specifically only talking about the single line `report_fatal_error("Cannot scalarize scalable vector stores");`-> `return SDValue();` change.  This looks unnecessary and potentially wrong.  If I try this patch but with the `report_fatal_error` left intact I do not see any test failures.
sorry, may be I don't express clear!

I mean that, If I only keep the changes of setTruncStoreAction(VT, InnerVT, Expand) , but without picking out the ones, which are legal, then it will crash at report_fatal_error with above 5 test cases. 
So I change  report_fatal_error("Cannot scalarize scalable vector stores") into return SDValue() to avoid it.


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

https://reviews.llvm.org/D123449