[llvm] r209386 - Override runOnMachineFunction for ARMISelDAGToDAG so that we can

[Bob Wilson]

On May 22, 2014, at 1:07 PM, Eric Christopher <echristo at gmail.com> wrote:

> On Thu, May 22, 2014 at 1:00 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>> 
>> On May 22, 2014, at 12:48 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 
>>> On Thu, May 22, 2014 at 12:46 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>>>> 
>>>> On May 22, 2014, at 11:29 AM, Tim Northover <t.p.northover at gmail.com> wrote:
>>>> 
>>>>>> Preparation for something bigger, basically the CGContext stuff from
>>>>>> another thread. The basic idea comes down to being able to link
>>>>>> together modules (or include in a single module) functions which
>>>>>> target different subtargets/subtarget features and still be able to
>>>>>> codegen and optimize them correctly. It's similar to some of the
>>>>>> function specific optimization that happened recently.
>>>>> 
>>>>> Oh, excellent! Looking forward to seeing more of that.
>>>> 
>>>> I’m concerned that this is the wrong approach. Especially as we start thinking about parallel compilation, I really think we need to be explicit about the context. We ought to be moving toward having runOnMachineFunction take a CGContext as an argument, with all the target information and other context accessible via the CGContext.  This patch seems to be further entrenching the current approach of having a single global target that is specified implicitly, not explicitly.
>>>> 
>>>> If I’ve missed some discussions that explain how this fits into the bigger picture of what we need to do, can you point me at it? If those discussions haven’t happen yet, this would be a good time to start.
>>> 
>>> This happened in the CGContext thread.
>>> 
>>> And yes, I agree, however, unless you want parallel pass managers then
>>> you'll _still_ need to do this. Also you still don't want to globally
>>> set what's legal based on a single target machine.
>>> 
>>> This is all scaffolding to make this work and is incremental.
>> 
>> I have not seen any discussion at all about how to move toward explicit use of CGContext. I just re-read the CGContext thread and didn’t see any discussion of that at all.
>> 
>> I understand we may not some incremental changes now, and that’s fine as long as we have a plan for where we’re trying to go in the longer term.
>> 
>> I definitely agree that we should not set what is legal based on a single target machine.  That is exactly what I’m concerned about. We need to get away from the whole idea that there is a single target machine. Your change here seems to be perpetuating that instead of fixing it.
> 
> There's single target machine as in X86, and single target machine as
> in Haswell. Right now I'm avoiding the issue of mixing ARM and X86 in
> the same module and assuming a single TM instance with different
> versions of the sub target.

Mixing ARM and x86 (or more realistically, ARM and Thumb) isn’t a priority but it would be good to do things in a way that will move us in that direction.

> This will sit at the higher level and each
> individual CGContext controlling the subtarget features for an
> individual function. Anything that needs the subtarget can ask for a
> CGContext/Subtarget from the target machine without needing the churn
> necessary to update all callers of passes and the correct one will get
> returned to it.

This seems backwards to me. You shouldn’t ask the target for a CGContext. It should be the other way around.

> Right now the scaffolding to do that is getting put
> into place so that those queries can be a simple and obvious change.
> The first thing for this is to make sure that the various passes and
> backends don't do things based on the initial subtarget which is the
> set of changes I've been making.

The fact that you refer to “the initial subtarget” is the root of my concern. We should get away from the implicit argument of “the current subtarget” and make that an explicit argument (via the CGContext) of everything in code-gen.

> Next will be making sure that
> subtargets really do own their own code generation while things like
> the register sets can stay on the target machine.
> 
> If we decide to mix targets in a single module at some point in the
> future then we'll need another set of indirection, but that can sit on
> top of it (and I'm not convinced we wouldn't want different pass
> managers still and just have a target machine for each actual target).
> 
> Make sense?

I think I understand what you’re trying to do. I just don’t agree that it is the right approach.