This is an archive of the discontinued LLVM Phabricator instance.

[Review Request] A module pass "PromoteHalf"
Needs ReviewPublic

Authored by shawfly on Aug 15 2014, 6:13 AM.

Details

Reviewers
joe.abbey
srhines
Summary

When Opts.NativeHalfType is set to 1, Clang will generate IR for half float. Currently, most CPUs don't support fp16 register, fp16 should be promoted to fp32 for computation, then store back the result to fp16.

This is a module pass to transform operations on fp16 to operations on fp32; different back-end could add this pass as needed.

Diff Detail

Event Timeline

shawfly updated this revision to Diff 12553.Aug 15 2014, 6:13 AM
shawfly retitled this revision from to [Review Request] A module pass "PromoteHalf".
shawfly updated this object.
shawfly edited the test plan for this revision. (Show Details)
shawfly added reviewers: joe.abbey, srhines.
shawfly added a subscriber: Unknown Object (MLST).

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.

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