Page MenuHomePhabricator

[X86] Move combineLoopMAddPattern and combineLoopSADPattern to an IR pass before SelecitonDAG.
ClosedPublic

Authored by craig.topper on Mon, Mar 23, 4:38 PM.

Details

Summary

These transforms rely on a vector reduction flag on the SDNode
set by SelectionDAGBuilder. This flag exists because SelectionDAG
can't see across basic blocks so SelectionDAGBuilder is looking
across and saving the info. X86 is the only target that uses this
flag currently. By removing the X86 code we can remove the flag
and the SelectionDAGBuilder code.

This pass adds a dedicated IR pass for X86 that looks across the
blocks and transforms the IR into a form that the X86 SelectionDAG
can finish.

An advantage of this new approach is that we can enhance it to
shrink the phi nodes and final reduction tree based on the zeroes
that we need to concatenate to bring the partially reduced
reduction back up to the original width.

Diff Detail

Event Timeline

craig.topper created this revision.Mon, Mar 23, 4:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Mar 23, 4:38 PM

There's still 1 other use of the flag in DAGCombiner IIUC, but if we can find a way to get rid of that too, that would be great.

llvm/lib/Target/X86/X86PartialReduction.cpp
70

Is this identical to the existing code in SelectionDAGBuilder? Could we hoist that to VectorUtils.cpp, so we don't have duplicate code?

craig.topper marked an inline comment as done.Thu, Mar 26, 10:31 AM

There's still 1 other use of the flag in DAGCombiner IIUC, but if we can find a way to get rid of that too, that would be great.

Isn't the use in DAGCombiner just a check to prevent reassociation so we don't lose the flag?

llvm/lib/Target/X86/X86PartialReduction.cpp
70

Isn't the code in SelectionDAGBuilder just there to set the flag? Once that's gone then there won't be any duplication.

There's still 1 other use of the flag in DAGCombiner IIUC, but if we can find a way to get rid of that too, that would be great.

Isn't the use in DAGCombiner just a check to prevent reassociation so we don't lose the flag?

Ah, I didn't make that connection. So we can delete that use and the SelectionDAGBuilder code in this patch, or is there some reason to make that a different step?

Remove the vector reduction flag an associated SelectionDAGBuilder code

spatel accepted this revision.Thu, Mar 26, 12:37 PM

LGTM

This revision is now accepted and ready to land.Thu, Mar 26, 12:37 PM
This revision was automatically updated to reflect the committed changes.