Page MenuHomePhabricator

Fixed a failure in cost calculation for vector GEP
Needs ReviewPublic

Authored by delena on Nov 25 2015, 2:06 AM.



Cost calculation for vector GEP failed with due to invalid cast to GEP index operand.
The bug is fixed, added a test.

Diff Detail


Event Timeline

delena updated this revision to Diff 41116.Nov 25 2015, 2:06 AM
delena retitled this revision from to Fixed a failure in cost calculation for vector GEP.
delena updated this object.
delena added reviewers: gilr, jingyue.
delena set the repository for this revision to rL LLVM.
delena added subscribers: llvm-commits, hfinkel.
jingyue added inline comments.Nov 25 2015, 9:49 AM

I don't understand why you use getSplatValue here. getSplatValue returns the element type only when all the elements in the array/vector are the same. Are you supposed to also protect the cases where the elements are different?


This if-check seems unnecessary. The one at Line 424 already checks for ConstantDataVector.

delena added inline comments.Nov 25 2015, 11:44 PM

It may be a vector of constants. In this case the ConstIdx = 0.
For structs, the index is always splat or a scalar constant.


Constant and ConstantDataVector are different types. In case of all-zero-vector it is "Constant".

jingyue requested changes to this revision.Nov 29 2015, 12:14 PM
jingyue edited edge metadata.

LGTM with some minors


It makes sense now. Please add a comment explaining why we need to use getSplatValue. Thanks!


ConstantDataVector is a subclass of Constant. Constant::getSplatValue checks if the constant is a ConstantDataVector and if so invokes ConstantDataVector::getSplatValue. So I think it's no longer necessary to check for ConstantDataVector in Line 421-422.

This revision now requires changes to proceed.Nov 29 2015, 12:14 PM
delena updated this revision to Diff 41371.Nov 30 2015, 12:52 AM
delena edited edge metadata.
delena marked 2 inline comments as done.

Jingyue, thank you for the review. I addressed all comments and updated the patch.

jingyue resigned from this revision.Feb 5 2016, 5:48 PM
jingyue removed a reviewer: jingyue.

Committed in r254408.