Page MenuHomePhabricator

[AArch64] Fix assertion failure on widened f16 BUILD_VECTOR

Authored by bryanpkc on Aug 2 2018, 3:33 PM.



Ensure that NormalizedBuildVector returns a BUILD_VECTOR with operands of the
same type. This fixes an assertion failure in VerifySDNode.

Diff Detail


Event Timeline

bryanpkc created this revision.Aug 2 2018, 3:33 PM
SjoerdMeijer added inline comments.Aug 3 2018, 2:37 AM
6896 ↗(On Diff #158851)

Doesn't look like you're testing this; add a test for this?

43 ↗(On Diff #158851)

Nit: perhaps move some of these explanations to the implementation in NormalizeBuildVector? But definitely a brief explanation of the test would be good.

bryanpkc added inline comments.Aug 3 2018, 10:21 AM
6896 ↗(On Diff #158851)

Actually the test is exercising this path; widening adds i16 UNDEF nodes to a vector, but when normalizing the vector, they were left alone and not promoted.

The next block is in fact a preemptive fix, for which I couldn't construct a test case. However, zero-extending a 16-bit or smaller operand in an integer BUILD_VECTOR to 32 bits seems pretty safe to me, and is consistent with what we already do for constant operands in this function.

43 ↗(On Diff #158851)

OK, will do.

bryanpkc updated this revision to Diff 159092.Aug 3 2018, 2:21 PM

Removed impossible code path from NormalizeBuildVector.

SjoerdMeijer accepted this revision.Aug 6 2018, 1:56 AM

Thanks, looks reasonable to me.

This revision is now accepted and ready to land.Aug 6 2018, 1:56 AM
This revision was automatically updated to reflect the committed changes.