This is an archive of the discontinued LLVM Phabricator instance.

[FastISel][X86] Teach how to select SSE2/AVX bitcasts between 128/256-bit vector types.
ClosedPublic

Authored by andreadb on Oct 1 2015, 7:53 AM.

Details

Summary

While looking at the -O0 codegen, I noticed that X86FastIsel doesn't know how to fast select bitcasts between 128-bit vetor types.

This patch is a (very) minor FastISel improvement.
It teaches FastISel the following two things:

  1. On SSE2, no instructions are needed for bitcasts between 128-bit vector types.
  2. On AVX, no instructions are needed for bitcasts between 256-bit vector types.

Example:

%1 = bitcast <4 x i32> %V to <2 x i64>

Before (-fast-isel -fast-isel-abort=1):

FastIsel miss:  %1 = bitcast <4 x i32> %V to <2 x i64>

Now we don't fall back to SelectionDAG and we correctly fold that computation propagating the register associated to %V.

Please let me know if okay to submit.

Thanks,
Andrea

Diff Detail

Event Timeline

andreadb updated this revision to Diff 36242.Oct 1 2015, 7:53 AM
andreadb retitled this revision from to [FastISel][X86] Teach how to select SSE2/AVX bitcasts between 128/256-bit vector types..
andreadb updated this object.
andreadb added reviewers: qcolombet, ributzka, nadav.
andreadb added a subscriber: llvm-commits.
qcolombet edited edge metadata.Oct 1 2015, 2:00 PM

Hi Andrea,

Couple of questions, see inline, otherwise looks quite reasonable to me.

Thanks,
-Quentin

lib/Target/X86/X86FastISel.cpp
3240

Why do we have to check that here?
We are interested in 128 and 256 bit vector types. I do not understand why the fact that f64 may be mapped on x87 need to be considered?

3247

Could you add test cases for these checks?
In particular the different SizeInBits part.

Hi Quentin,

lib/Target/X86/X86FastISel.cpp
3240

While it is true that X86ScalarSSEf64 is initialized to Subtarget->hasSSE2(), X86ScalarSSEf64 should only be used to reason on f64 types. So it is wrong to use it in this context.

I will change it to an explicit check for feature SSE2.

3247

My bad.. That check makes no sense since source and destination bit widths must always be identical for bitcast operations.
I will remove that redundant check.

qcolombet added inline comments.Oct 1 2015, 2:44 PM
lib/Target/X86/X86FastISel.cpp
3247

Hehe, thanks for checking, I was wondering if the data layout may break that assumption and if it does, when!

andreadb updated this revision to Diff 36308.Oct 1 2015, 3:01 PM
andreadb edited edge metadata.

Patch Updated.

Replaced the check for X86ScalarSSEf64 with a more appropriate check for SSE2.
Removed redundant check for type sizes.

Please let me know if okay to commit.
Thanks!

qcolombet accepted this revision.Oct 1 2015, 5:47 PM
qcolombet edited edge metadata.

Thanks Andrea!

LGTM.

Cheers,
-Quentin

This revision is now accepted and ready to land.Oct 1 2015, 5:47 PM
This revision was automatically updated to reflect the committed changes.