Page MenuHomePhabricator

[LV] Delete unneeded scalar GEP creation code
ClosedPublic

Authored by mssimpso on Mar 3 2017, 12:16 PM.

Details

Summary

The code for generating scalar base pointers in vectorizeMemoryInstruction is not needed. We currently scalarize all GEPs and maintain the scalarized values in VectorLoopValueMap. The GEP cloning in this unneeded code is the same as that in scalarizeInstruction. The test cases that changed as a result of this patch changed because we were able to reuse the scalarized GEP that we previously generated instead of cloning a new one.

I plan to follow this patch up with a few more that will eventually merge pointer induction variable widening with the rest of the induction widening code.

Diff Detail

Repository
rL LLVM

Event Timeline

mssimpso created this revision.Mar 3 2017, 12:16 PM
mssimpso updated this revision to Diff 90535.Mar 3 2017, 2:42 PM

Added back the bitcast look-through via getGEPInstruction and the DEBUG check to be safe.

mssimpso updated this revision to Diff 90538.Mar 3 2017, 2:59 PM

Fixed a bug.

mkuper added inline comments.Mar 6 2017, 6:18 PM
lib/Transforms/Vectorize/LoopVectorize.cpp
2979 ↗(On Diff #90538)

Do we really need to pay attention to the GEP anymore?
I mean, if we already scalarize everything correctly, wouldn't just always using "getScalarValue(Ptr, 0, 0)" be enough?

mssimpso added inline comments.Mar 7 2017, 8:34 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
2979 ↗(On Diff #90538)

That was my initial thought, but then I started second guessing myself for some reason. You're right, though; I think we can just use getScalarValue(Ptr, 0, 0) here and get rid of the GEP check (and probably the DEBUG stuff as well).

I have a follow-on patch that also removes the vector GEP creation from here. It moves it into the main vectorization loop. I'll update this patch and post the other so we can take a look at both of them at the same time.

mssimpso updated this revision to Diff 90894.Mar 7 2017, 11:14 AM

Revert to the original version of the patch - just use getScalarValue(Ptr, 0, 0).

This revision was automatically updated to reflect the committed changes.