Page MenuHomePhabricator

[MIPS MSA] Avoid some DAG combines for vector shifts
ClosedPublic

Authored by Petar.Avramovic on Feb 14 2019, 2:29 AM.

Details

Summary

DAG combiner combines two shifts (for different shift amounts) into
shift + and with bitmask. Vector bitmasks require either quite a few
instructions or extra constant pool space. Avoid such combines as
leaving two vector shifts as they are produces better end results.

Diff Detail

Repository
rL LLVM

Event Timeline

Petar.Avramovic edited the summary of this revision. (Show Details)Feb 14 2019, 2:47 AM
RKSimon added a subscriber: craig.topper.

Intel targets tend to only accept vector shifts on Port0, while vector logic can use Port0/1/5 - not sure how much of an issue that would be @craig.topper ? Some AMD targets are almost as bad while others (Jaguar) can issue vector immediate shifts to any vector integer pipe.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6694 ↗(On Diff #186805)

A comment would be nice

test/CodeGen/X86/combine-shl.ll
574 ↗(On Diff #186805)

This comment needs adjusting.

I think only Skylake CPUs have more than 1 shift execution port if I remember right. I'm not sure what the impact would be. Can we limit this to MIPS MSA using the TLI.shouldFoldShiftPairToMask hook? Not sure why the call to that hook is made before we know the other value is a constant.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6689 ↗(On Diff #186805)

Doesn't this have the same issue?

spatel added a subscriber: spatel.Feb 18 2019, 6:38 AM
Petar.Avramovic marked an inline comment as done.

Restrict changes to MIPS backend only.
Use shouldFoldShiftPairToMask to avoid all attempts
to combine shifts if operands have vector type.

This revision is now accepted and ready to land.Feb 20 2019, 5:16 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2019, 5:42 AM