[PATCH] D30047: [GlobalISel] Split LowLevelType to a fundamental type in libSupport.

Ahmed Bougacha via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 22 10:07:47 PST 2017


ab added a comment.

In https://reviews.llvm.org/D30047#683232, @rovka wrote:

> In https://reviews.llvm.org/D30047#682820, @dsanders wrote:
>
> > Ok, let's go with the standalone method. I'd like to give it a more descriptive name though seeing as it's in the llvm namespace. How about getLLTForType()?
>


getLLTForType SGTM

>> For the header name, I agree with not having LowLevelTypes.h in both libraries. Let's go with GlobalISel/Utils.h.
> 
> That sounds good to me. @ab, what do you think?

Yeah, using the same name is weird, but here's the rationale: almost every single user of LLT should use CodeGen/LowLevelTypes.h; for them, that's the way to use LLT.  But I guess that illusion breaks down pretty quickly when they find the header almost completely empty...  Maybe we can rename Support/LowLevelTypes.h  Support/LowLevelTypeImpl.h or something?  Not that it sounds much better ;)

But specifically regarding Utils.h: I'm not a big fan of catch-all headers; maybe we can keep it single-purpose and call it CodeGen/LowLevelTypeUtils.h, or CodeGen/CodeGenLowLevelTypes.h, or something like that?

Not a big deal either way; if we don't have a better idea I'll stick to Utils.h.


https://reviews.llvm.org/D30047





More information about the llvm-commits mailing list