[PATCH] D38482: TargetMachine: Use LLVMTargetMachine in CodeGen

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D38482#886520, @chandlerc wrote:

> I generally like the direction of the cleanup. A few high level questions:
>
> 1. You say that this is to distinguish targets that *do* use LLVM's lib/CodeGen from those that do not. Do you mean entire targets that do not? Or do you mean just *components* of targets that do not depend on it like the target's MC layer?


Yes I mean entire targets.

> 
> 
> 2. If in #1 you mean actual targets -- are there any left now that the C backend is gone? What are these targets? I wonder if we could simplify things by collapsing a no longer necessary layer.

- There is none in open source, and I don't know of any internal ones at my company.
- According to the mailing list this may be good for SPIR-V (http://lists.llvm.org/pipermail/llvm-dev/2017-June/114593.html) which doesn't really need any of the CodeGen infrastructure.
- Indeed collapsing the two classes would slightly simplify the code, and I'd be perfectly fine with that.
- On the other hand it's kinda neat to have a CodeGen internal interface with `LLVMTargetMachine` and an outside interface with `TargetMachine`

> 
> 
> 3. If we can't eliminate the distinction here completely, are there better names than `TargetMachine` and `LLVMTargetMachine` that we could give these? I'm guessing a good name will be informed by the answers to the above two questions and the reason we're keeping the distinction in place.

I guess `CodeGenTarget` instead of `LLVMTarget` would be in line with this being for `lib/CodeGen`.

> 
> 
> 4. Any ideas how we could improve comments (or the design) to avoid regressing this (again) in the future?

I think if we think about this as CodeGen internal and external interfaces or if people actually start using it for SPIR-V it may be fine... But again if you rather feel to merge the two and get rid of the distinction that's fine with me too.


Repository:
  rL LLVM

https://reviews.llvm.org/D38482