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

[Tim Northover]

> 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.

It's difficult to know where to begin. Those interfaces may be behind
a "public:" access specifiers, but that doesn't mean they should be
used by just any code.

The LLVMContextImpl usage in particular is *completely* wrong. Apart
from anything else (and there are plenty of other issues with it),
you're changing any other module that uses that context by altering
its private data structures.

And the mutateType function is there as an escape hatch. To be used
when all other (safer) options have been exhausted and found wanting.
If your single pass more than doubles the total number of calls in
LLVM, you're probably Doing It Wrong.

> 2. Half float is a special type since most CPU targets don't support it in native way,

I don't think it's any more special than (say) i24 or even i8 (at
least on RISC cores) in that respect.

> it is more clear to handle it before any transformation passes.

I disagree. LLVM has these half operations for a reason, so that the
IR accurately describes what the front-end requested.

> 3. Handling half in DAGTypeLegalizer will add the complexity of type
> legalizer, it is difficult to mask/disable it in target machine configuration.

How so? I think it could be a matter of a simple call to setTypeAction
if half really is illegal on your target (x86 I'm assuming?).

The end point is that I think the problems with LLVMContextImpl and
mutateType are insurmountable. I just cannot see a path to a commit
that doesn't involve a complete rewrite (even if everyone suddenly
decided that a modulepass was the right way to do it after all). Given
that, I think the rewrite should take place in the legalizer, as it
does for other types.

Cheers.

Tim.