Page MenuHomePhabricator

Support generic expansion of ordered vector reduction (PR36732)
ClosedPublic

Authored by RKSimon on Apr 6 2018, 6:55 AM.

Details

Summary

Without the fast math flags, the llvm.experimental.vector.reduce.fadd/fmul intrinsic expansions must be expanded in order.

This patch scalarizes the reduction, applying the accumulator at the start of the sequence: ((((Acc + Scl[0]) + Scl[1]) + Scl[2]) + ) ... + Scl[NumElts-1]

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Apr 6 2018, 6:55 AM
ABataev added inline comments.Apr 6 2018, 7:06 AM
include/llvm/Transforms/Utils/LoopUtils.h
517 ↗(On Diff #141335)

ArrayRef<Value *> RedOps = ArrayRef<Value *>()->ArrayRef<Value *> RedOps = None

lib/CodeGen/ExpandReductions.cpp
122–124 ↗(On Diff #141335)

auto->Value *

lib/Transforms/Utils/LoopUtils.cpp
1537 ↗(On Diff #141335)
  1. auto->auto &&
  2. Use explicit capturing
1560 ↗(On Diff #141335)

Use preincrement expression

For now, that the shuffle reduction is in loop utils, this patch is fine. But this really ought to be in target transform info.

lib/Transforms/Utils/LoopUtils.cpp
1531 ↗(On Diff #141335)

SVE will be able to do this with a single instruction, we should be able to override this based on target info.

For now, that the shuffle reduction is in loop utils, this patch is fine. But this really ought to be in target transform info.

I agree, I'll do that as an NFC once we've finalised the behaviour of reductions a bit more.

lib/Transforms/Utils/LoopUtils.cpp
1531 ↗(On Diff #141335)

This should be covered by TTI::useReductionIntrinsic

1537 ↗(On Diff #141335)

Even better, I'll remove the lambda entirely and inline it.

RKSimon updated this revision to Diff 141374.Apr 6 2018, 9:56 AM

Cleaned up based on @ABataev's comments.

@aemerson explained on D45336 that the strict/ordered reductions should always use the accumulator argument, so I've removed the is null or undef logic.

aemerson accepted this revision.Apr 9 2018, 2:41 AM
This revision is now accepted and ready to land.Apr 9 2018, 2:41 AM
This revision was automatically updated to reflect the committed changes.