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

[Wan, Xiaofei]

Hi, Tim, thanks for your reply:

1. This pass will be the first pass on IR before any other transformation passes, for sure it will change the module; it only use public interface in current code infrastructure. Could you please describe the main concern here? Thanks.
2. Half float is a special type since most CPU targets don't support it in native way, most target only use half as a storage type; this is why I don't want to handle it in DAGTypeLegalizer; it is more clear to handle it before any transformation passes.
3. Handling half in DAGTypeLegalizer will add the complexity of type legalizer, it is difficult to mask/disable it in target machine configuration. The PromoteHalf pass will not be configured as default pass, it has no impact to current code infrastructure. 

Thanks
Wan Xiaofei

-----Original Message-----
From: Tim Northover [mailto:t.p.northover at gmail.com] 
Sent: Friday, August 15, 2014 10:23 PM
To: Wan, Xiaofei; joe.abbey at gmail.com; srhines at google.com
Cc: t.p.northover at gmail.com; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] [Review Request] A module pass "PromoteHalf"

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