[PATCH] D89162: [SVE] Lower fixed length VECREDUCE_SEQ_FADD operation

Cameron McInally via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 23 08:03:46 PDT 2020


cameron.mcinally added a comment.

In D89162#2350199 <https://reviews.llvm.org/D89162#2350199>, @paulwalker-arm wrote:

> In D89162#2347814 <https://reviews.llvm.org/D89162#2347814>, @cameron.mcinally wrote:
>
>> The new tests would be broken without the legalisation changes, so I'm assuming that those are enough coverage. Maybe I'm missing something though...
>
> Are you sure?  I took your patch for a test drive and removed all but the `TLI.getOperationAction` related change from Legalize*.{cpp, h} and the tests passed.

That doesn't sound right. E.g. we need the scalarization code for <1 x i64>. I'll check again...



================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-fp-reduce.ll:65
+
+; NOTE: NEON expansions used to ensure ordering.
+  %op = load <32 x half>, <32 x half>* %a
----------------
paulwalker-arm wrote:
> What does this mean?
> 
> I was going to say you're missing VBITS_EQ_256 lines here but then noticed the output.  What's you're feeling on this?  I see two routes:
> 
> Stick with the expanded output for this patch, in which case just having `VBITS_EQ_256-COUNT-33: fadd` is good enough.
> 
> For better code generation we're just missing the ability to split VECREDUCE_SEQ_FADD operations, which I don't think should be that hard.
> 
> `VECREDUCE_SEQ_FADD A, X_512 => VECREDUCE_SEQ_FADD (VECREDUCE_SEQ_FADD X, X_LO), X_HI` ?
> 
> I'm happy with either approach.
> 
> For better code generation we're just missing the ability to split VECREDUCE_SEQ_FADD operations, which I don't think should be that hard.

Oh, you're right. I initially thought we couldn't preserve the ordering constraint if we split, but we could use the accumulator to chain them. Makes sense. I'll look into that...


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

https://reviews.llvm.org/D89162



More information about the llvm-commits mailing list