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

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 21 06:43:33 PST 2017


rovka added a comment.

In https://reviews.llvm.org/D30047#679944, @dsanders wrote:

> I've been trying to preserve the current style of LLT usage but having seen this I realize there's no need to do that.
>
> I think we should make llvm::getLLT() a member of Type (see example usage below and https://reviews.llvm.org/differential/diff/88897/) but aside from that and a couple nits I think this seems like a reasonable way to fix the layering problem.


Not sure if this has been decided yet or not, but here's my 2 cents anyway:

I find it a bit weird to have IR level types know about low level types, we usually have it the other way around (e.g. EVT::getTypeForEVT). I prefer a standalone method as defined here, but OTOH having a LowLevelTypes.h in both CodeGen and Support might be confusing. How about we move it to GlobalISel/Utils.h? Or think of a different name for it (FWIW clang uses CodeGenTypes.h for the correspondence between AST types and LLVM types).


https://reviews.llvm.org/D30047





More information about the llvm-commits mailing list