[PATCH] [Review Request] A module pass "PromoteHalf"

Tim Northover t.p.northover at gmail.com
Fri Aug 15 07:23:28 PDT 2014


Hi,

I'm really worried about this. The pass looks like it's modifying information it has no right to even access (LLVMContextImpl is otherwise used *entirely* within the lib/IR directory) and completely butchering the Module in the process. The pervasive use of mutateType is also a big concern; it's a very risky operation, only used in about half a dozen places currently. It shouldn't be necessary here.

At an even larger scale, this seems like an odd place to handle the half type. Illegal types are not an uncommon problem and we usually do promotion during the LegalizeTypes DAG phase. I'm not going to argue that doing it at IR level approach is worse (moving the whole lot to IR might be a good idea longer term, and help us to deprecate the DAG), but doing it just for half is inconsistent.

Perhaps the first thing that needs to be resolved is why you decided against using the existing Legalize framework. Did you try and find it lacking in some areas? We ought to be able to improve it, if so.

Cheers.

Tim.

http://reviews.llvm.org/D4926






More information about the llvm-commits mailing list