This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Attempt to mask vectors before zero extension instead of after.
ClosedPublic

Authored by RKSimon on Aug 5 2015, 8:24 AM.

Details

Summary

For cases where we TRUNCATE and then ZERO_EXTEND to a larger size (often from vector legalization), see if we can mask the source data and then ZERO_EXTEND (instead of after a ANY_EXTEND). This can help avoid having to generate a larger mask, and possibly applying it to several sub-vectors.

(zext (truncate x)) -> (zext (and(x, m))

This is the first of a number of minor patches to help improve the conversion of byte masks to clear mask shuffles.

Ulrich - this includes some changes in SystemZ, please can you check them? A quick search indicates they make sense but I want to make sure.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 31353.Aug 5 2015, 8:24 AM
RKSimon retitled this revision from to [DAGCombiner] Attempt to mask vectors before zero extension instead of after..
RKSimon updated this object.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: llvm-commits.
uweigand edited edge metadata.Aug 5 2015, 3:17 PM

Ulrich - this includes some test changes in SystemZ, please can you check them? A quick search inducates they make sense but I want to make sure.

Well, the changed assembly is still *correct*, but it is somewhat less efficient than what we had before.

The problem is that we do have special "zero-extend" instructions (llgcr/llghr) which should be preferred over a generic rotate-then-insert-selected-bits (risbg) when possible. However, the instruction patterns for the zero-extend instructions attempt to match the SelectionDAG patterns the middle end used to generate for those scenarios. If we change those, the back end really ought to adapted to match the new patterns as well.

Unfortunately I'm currently travelling; I'll be able to look into this in more detail when I'm back mid next week.

Thanks Ulrich - have you had a chance to look at this?

It looks like the logic in SystemZDAGToDAGISel::tryRISBGZero requires generalizing a little beyond the AND(ANY_EXTEND(V)) pattern, I can update the patch with a candidate fix if you wish?

chandlerc edited edge metadata.Aug 13 2015, 2:39 PM

Very nice. LGTM when you have the SystemZ stuff sorted out.

test/CodeGen/X86/vec_cast2.ll
100 ↗(On Diff #31353)

This looks like just a boring re-generation of the test change. Land it separately (and first)?

RKSimon added inline comments.Aug 13 2015, 2:44 PM
test/CodeGen/X86/vec_cast2.ll
100 ↗(On Diff #31353)

Done in rL244765 - I haven't gotten around to rebasing yet though.

RKSimon updated this revision to Diff 32142.Aug 14 2015, 2:28 AM
RKSimon updated this object.
RKSimon edited edge metadata.

updated with SystemZ zero-extension candidate fix.

Sorry for the delay in getting back to you on this. Thanks for taking care of the SystemZ part yourself!

SystemZ changes LGTM.

This revision was automatically updated to reflect the committed changes.