Page MenuHomePhabricator

Don't constant fold through zero-length fields
Needs ReviewPublic

Authored by tjablin on Aug 22 2014, 1:35 PM.

Details

Reviewers
hfinkel
Summary

Previously, ConstantFoldLoadThroughGEPIndices and ConstantFoldLoadThroughGEPConstantExpr would return zeroinitializer when a constant GEP expression indexed to a zero-length field. The patch adds checks to these two methods based on isEmptyTy to avoid these scenarios. The patch also includes a simple testcase. I have verified that the testcase fails with r216246 and passes after applying the patch.

Diff Detail

Event Timeline

tjablin updated this revision to Diff 12855.Aug 22 2014, 1:35 PM
tjablin retitled this revision from to Don't constant fold through zero-length fields.
tjablin updated this object.
tjablin edited the test plan for this revision. (Show Details)
tjablin added a subscriber: Unknown Object (MLST).Aug 22 2014, 1:37 PM
tjablin updated this revision to Diff 12863.Aug 22 2014, 4:00 PM

Fix a related bug in ConstantFoldLoadThroughBitcast and add an additional test case.

Could you please upload a full-context diff as described here: http://llvm.org/docs/Phabricator.html

If this is well-defined behavior (I realize that indexing off the end of an array is allowed, but whether that is permitted to alias other structure members is unclear to me), then I don't see why an empty type is a special case here. Shouldn't we have the same problem with any off-the-end array access where the array is not the last aggregate member?

test/Transforms/GlobalOpt/empty-type-field.ll
4

Remove this line (it is unneeded).

tjablin updated this revision to Diff 13807.Sep 17 2014, 4:19 PM
tjablin updated this revision to Diff 13808.Sep 17 2014, 4:40 PM

I've updated the patch by replacing the CHECK-NOT test cases with CHECK test cases and included a new test demonstrating that indexing past the end of an array that is not the last field in a structure is already handled correctly.

chandlerc resigned from this revision.Mar 29 2015, 1:36 PM
chandlerc edited reviewers, added: hfinkel; removed: chandlerc.

Leaving this to Hal, I don't understand the specific bug you're fixing well enough to be a good reviewer.