This is an archive of the discontinued LLVM Phabricator instance.

Make TargetLowering::getPointerTy() taking DataLayout as an argument
ClosedPublic

Authored by mehdi_amini on Jul 7 2015, 11:32 PM.

Details

Summary

This change is part of a series of commits dedicated to have a single
DataLayout during compilation by using always the one owned by the
module.

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Make TargetLowering::getPointerTy() taking DataLayout as an argument.
mehdi_amini updated this object.
mehdi_amini added a reviewer: echristo.
rafael added inline comments.Jul 8 2015, 6:09 AM
lib/CodeGen/TargetLoweringBase.cpp
881

Why can't this use getDataLayout?

mehdi_amini updated this revision to Diff 29266.Jul 8 2015, 8:49 AM

What about also making getPointerTy() define in the header, it is non-virtual and can be inlined.
I'm also not sure why it has to be in this class, it seems it could be a free function?

echristo edited edge metadata.Jul 8 2015, 3:24 PM

One inline question...

-eric

include/llvm/CodeGen/BasicTTIImpl.h
149

My C++ knowledge may be lacking here, but why can't we use getDataLayout here?

mehdi_amini added inline comments.Jul 8 2015, 3:35 PM
include/llvm/CodeGen/BasicTTIImpl.h
149

C++ weirdness explained here: https://isocpp.org/wiki/faq/templates#nondependent-name-lookup-members

Now, we could also write this->DL. But I may be better adding a using TargetTransformInfoImplBase::DL in the class itself instead of clobbering every single use.

echristo accepted this revision.Jul 8 2015, 3:55 PM
echristo edited edge metadata.

LGTM.

I have no preference on reorganizing getPointerTy.

-eric

include/llvm/CodeGen/BasicTTIImpl.h
149

*sigh* Right. OK. Thanks!

This revision is now accepted and ready to land.Jul 8 2015, 3:55 PM