Page MenuHomePhabricator

[opaque pointer types] Alloca: use getAllocatedType() instead of getType()->getElementType().
ClosedPublic

Authored by eddyb on Jan 17 2016, 11:24 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

eddyb updated this revision to Diff 45111.Jan 17 2016, 11:24 AM
eddyb retitled this revision from to [opaque pointer types] Alloca: use getAllocatedType() instead of getType()->getPointerElementType()..
eddyb updated this object.
eddyb added a reviewer: mjacob.
eddyb added subscribers: dblaikie, llvm-commits.
eddyb retitled this revision from [opaque pointer types] Alloca: use getAllocatedType() instead of getType()->getPointerElementType(). to [opaque pointer types] Alloca: use getAllocatedType() instead of getType()->getElementType()..Jan 17 2016, 12:33 PM
mjacob added inline comments.Jan 17 2016, 12:59 PM
lib/Transforms/Scalar/ScalarReplAggregates.cpp
2237 ↗(On Diff #45111)

Are you sure this is right? I don't think we can assume that AI and OtherPtr have the same type.

2254 ↗(On Diff #45111)

Why don't you reuse EltPtr?

2486 ↗(On Diff #45111)

Why don't you reuse SrcField?

eddyb marked 2 inline comments as done.Jan 17 2016, 1:11 PM
eddyb added inline comments.
lib/Transforms/Scalar/ScalarReplAggregates.cpp
2237 ↗(On Diff #45111)

OtherPtr is ensured above to have the type PointerType::get(AI->getAllocatedType(), AddrSpace).

2254 ↗(On Diff #45111)

Wrong type, NewElts is an array of AllocaInst*, and I believe EltPtr is assigned something that is not an AllocaInst* (or used to, looking again at it doesn't seem to be the case atm).

2486 ↗(On Diff #45111)

See previous comment. In this case, SrcField *does* get assigned, so I can't change its type to AllocaInst*.

dblaikie accepted this revision.Jan 17 2016, 1:32 PM
dblaikie added a reviewer: dblaikie.

Thanks for all the work/patches posted here. Will be getting to the reviews as quick as I can.

This one looks good, please commit!

lib/Transforms/Scalar/ScalarReplAggregates.cpp
2237 ↗(On Diff #45111)

Could throw in an assert to that effect if you like

2254 ↗(On Diff #45111)

Feel free to correct the type (since, as you say, it's no longer reused for other things) & reuse it if you like

This revision is now accepted and ready to land.Jan 17 2016, 1:32 PM
eddyb updated this revision to Diff 45134.Jan 17 2016, 3:38 PM
eddyb marked 2 inline comments as done.
eddyb edited edge metadata.

Reuse EltPtr variable, with a more precise type.

eddyb marked an inline comment as done.Jan 17 2016, 3:39 PM
eddyb updated this revision to Diff 45137.Jan 17 2016, 4:01 PM

Assert that OtherPtr still has the same pointer type as the alloca.

eddyb marked an inline comment as done.Jan 17 2016, 4:01 PM
mjacob accepted this revision.Jan 17 2016, 4:03 PM
mjacob edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.