This is an archive of the discontinued LLVM Phabricator instance.

Destructor loads of struct that do not contains padding in InstCombine.
ClosedPublic

Authored by deadalnix on Nov 7 2015, 6:35 PM.

Details

Summary

For non padded structs, we can just proceed and deaggregate them. We don't want ot do this when there is padding in the struct as to not lose information about this padding (the subsequents passes would then try hard to preserve the padding, which is undesirable).

Also update extractvalue.ll and cast.ll so that they use structs with padding.

Remove the FIXME in the extractvalue of laod case as the non padded case is handled when processing the load, and we don't want to do it on the padded case.

Diff Detail

Event Timeline

deadalnix updated this revision to Diff 39646.Nov 7 2015, 6:35 PM
deadalnix retitled this revision from to Destructor loads of struct that do not contains padding in InstCombine..
deadalnix updated this object.
deadalnix added a subscriber: llvm-commits.
deadalnix updated this revision to Diff 40427.Nov 17 2015, 1:43 PM

rebase and ping

deadalnix updated this revision to Diff 42256.Dec 8 2015, 5:20 PM

Do not steal a bit from StructLayout.

Maybe that will make this less controversial. Yet I kind of feel bad over computing this again and again.

mehdi_amini accepted this revision.Dec 11 2015, 3:16 PM
mehdi_amini edited edge metadata.

LTGM. We can always revisit the caching later if it shows up on a profile.

lib/IR/DataLayout.cpp
71

Remove the empty line change

This revision is now accepted and ready to land.Dec 11 2015, 3:16 PM
deadalnix updated this revision to Diff 42610.Dec 11 2015, 5:05 PM
deadalnix edited edge metadata.

As per direction on IRC stealing a bit on the element count rather than on the struct size or recomputing.

mehdi_amini added inline comments.Dec 11 2015, 5:46 PM
include/llvm/IR/DataLayout.h
483

I think it needs a description, especially since it does not track padding in nested struct IIUC.

deadalnix updated this revision to Diff 42644.Dec 12 2015, 4:38 PM

Add comment on hasPadding to mention it doesn't include nested padding.

mehdi_amini added inline comments.Dec 12 2015, 4:59 PM
include/llvm/IR/DataLayout.h
493

no brief
s/wiether/whether/
s/it field/its fields/