Page MenuHomePhabricator

[DAGCombine] Fix crash when store merging created an extract_subvector with invalid index

Authored by aemerson on Sep 8 2018, 6:28 AM.

Diff Detail


Event Timeline

aemerson created this revision.Sep 8 2018, 6:28 AM
niravd added inline comments.Sep 12 2018, 9:44 AM
13852 ↗(On Diff #164571)

Shouldn't the two NewIdx expressions should be equivalent (modulo integer overflow issues)?

NewIdx = ((unsigned long long) IdxC*MemVT.getVectorNumElements()) / Elts

aemerson updated this revision to Diff 165172.Sep 12 2018, 3:38 PM

You're right, that's a simplified expression.

aemerson marked an inline comment as done.Sep 12 2018, 3:38 PM
niravd accepted this revision.Sep 13 2018, 8:09 AM

We should make sure to do the multiplication as (unsigned long long) to give us 64-bits because vectors on the order of 2^16 are potentially reasonable and would cause an overflow at 32-bits.

Otherwise LGTM.

This revision is now accepted and ready to land.Sep 13 2018, 8:09 AM

Ok, I'm surprised that would be a realistic scenario but I'll make the change. Thanks.

This revision was automatically updated to reflect the committed changes.