Page MenuHomePhabricator

[InstCombine] Add constant vector support to getMinimumFPType for visitFPTrunc.
ClosedPublic

Authored by craig.topper on Feb 26 2018, 11:56 AM.

Details

Summary

This patch teaches lookThroughFPExtensions to support shrinking a vector of ConstantFPs. This should improve our ability to combine vector fptrunc with fp binops.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Feb 26 2018, 11:56 AM
spatel added inline comments.Feb 27 2018, 8:14 AM
lib/Transforms/InstCombine/InstCombineCasts.cpp
1426–1428 ↗(On Diff #135943)

The existing code has a bug, but I don't think there's a way to expose it other than as an inefficiency. We will (usually?) create a half constant and then extend it back to single because that's not the type we wanted in the first place.

For scalars, this is wasteful, but constant folding hides the problem. Maybe nobody cares for scalars, but now we're doing that extra work N times for vector constants. And the bug is now visible (so we should add a test like this where the constants can't shrink to the same narrow type):

define <2 x float> @not_half_shrinkable(<2 x float> %x) {
  %ext = fpext <2 x float> %x to <2 x double>
  %add = fadd <2 x double> %ext, <double 0.0, double 2049.0>
  %r = fptrunc <2 x double> %add to <2 x float>
  ret <2 x float>  %r
}
1471–1475 ↗(On Diff #135943)

The usual pattern has the vector constant check inside the helper function, so move the vector type check into 'shrinkFPConstantVector' and rename the functions?

Ideally, these wouldn't be instcombine-specific helpers. They'd be queries on constants. I recently made a similar change in rL325398.

test/Transforms/InstCombine/fpextend.ll
92–96 ↗(On Diff #135943)

I think we can remove the load/store/globals from all of the tests in this file as a preliminary cleanup.

craig.topper planned changes to this revision.Feb 27 2018, 9:40 AM
craig.topper retitled this revision from [InstCombine] Add constant vector support to lookThroughFPExtensions for visitFPTrunc. to [InstCombine] Add constant vector support to getMinimumFPType for visitFPTrunc..

The scalar code is now type based and doesn't new constants. Rework this patch to build on that and find a minimal safe type by looking at each element. Then turn that into a vector type.

spatel accepted this revision.Mar 5 2018, 7:57 AM

LGTM, but please add a TODO comment (and ideally a test to show it) that we don't handle vectors with undef elements.

This revision is now accepted and ready to land.Mar 5 2018, 7:57 AM
This revision was automatically updated to reflect the committed changes.