Page MenuHomePhabricator

[VPlan] Handle scalarized values in VPTransformState.
ClosedPublic

Authored by fhahn on Nov 29 2020, 1:32 PM.

Details

Summary

This patch adds plumbing to handle scalarized values directly in
VPTransformState.

Diff Detail

Event Timeline

fhahn created this revision.Nov 29 2020, 1:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2020, 1:32 PM
fhahn requested review of this revision.Nov 29 2020, 1:32 PM
gilr added inline comments.Jan 19 2021, 7:37 AM
llvm/lib/Transforms/Vectorize/VPlan.cpp
220

Why is this commented out?

llvm/lib/Transforms/Vectorize/VPlan.h
273

Better to use using?

fhahn updated this revision to Diff 318223.Jan 21 2021, 8:48 AM

Address Gil's comments, thanks!

fhahn updated this revision to Diff 318225.Jan 21 2021, 8:51 AM
fhahn marked an inline comment as done.

Remove unnecessary variable

llvm/lib/Transforms/Vectorize/VPlan.cpp
220

That was some leftovers from earlier experiments. I removed the line for now.

gilr added inline comments.Jan 23 2021, 11:59 PM
llvm/lib/Transforms/Vectorize/VPlan.h
318

Implementation?

fhahn updated this revision to Diff 318836.Jan 24 2021, 5:18 AM

Move void set(VPValue *Def, Value *IRDef, Value *V, const VPIteration &Instance); declaration to D92284, where it is actually defined & used.

fhahn added inline comments.Jan 24 2021, 5:19 AM
llvm/lib/Transforms/Vectorize/VPlan.h
318

Good catch, this accidentally slipped into this patch when I split them up. I moved the declaration to D92284 where it is implemented & used. Thanks!

gilr requested changes to this revision.Jan 24 2021, 11:47 PM

LGTM, with a nit. Thanks!

llvm/lib/Transforms/Vectorize/VPlan.cpp
223

The assert is missing an "error message", but checking hasVectorValue() seems a bit too defensive. Any reason to assert here? (and not assert at line 230?)

This revision now requires changes to proceed.Jan 24 2021, 11:47 PM
gilr accepted this revision.Jan 24 2021, 11:48 PM
This revision is now accepted and ready to land.Jan 24 2021, 11:48 PM
fhahn updated this revision to Diff 318937.Jan 25 2021, 3:14 AM

Thanks Gil! Rebased before committing.

This revision was landed with ongoing or failed builds.Jan 25 2021, 6:23 AM
This revision was automatically updated to reflect the committed changes.