[PATCH] D141870: [DAG] Fold Op(vecreduce(a), vecreduce(b)) into vecreduce(Op(a,b))

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 05:54:10 PST 2023


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jobnoorman.

LGTM

In D141870#4109408 <https://reviews.llvm.org/D141870#4109408>, @dmgreen wrote:

> Thanks - Now using a FlagInserter to copy the flags. The flags for integer ops are not passed through, as nsw/nuw/etc may not apply to the new operations.

Ah, right - I was only thinking of FMF flags in this context. That's worth a header comment on `reassociateReduction`. It looks like we drop all flags in `reassociateOps`, but that's not called from FP opcodes.

We could also make it less likely to go wrong by using a default flags param and/or naming that param "FMFFlags" or something like that. 
For example, we have functions like this:

  SDValue getMemBasePlusOffset(SDValue Base, SDValue Offset, const SDLoc &DL,
                               const SDNodeFlags Flags = SDNodeFlags());


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

https://reviews.llvm.org/D141870



More information about the llvm-commits mailing list