This is an archive of the discontinued LLVM Phabricator instance.

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

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

Diff Detail

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–2238

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

2254–2255

Why don't you reuse EltPtr?

2487

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–2238

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

2254–2255

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).

2487

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–2238

Could throw in an assert to that effect if you like

2254–2255

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.