[llvm-dev] Mips unconditionally uses fast-isel?

[Daniel Sanders via llvm-dev]

Thanks for the explanation. With all that complexity I can see why it's unreasonable to promise that they're the same thing.
________________________________
From: Robinson, Paul [Paul_Robinson at playstation.sony.com]
Sent: 19 November 2015 15:30
To: Daniel Sanders; llvm-dev at lists.llvm.org
Subject: RE: Mips unconditionally uses fast-isel?

> > Well, 'optnone' is already not identical to –O0, and given the nature of things, probably can't be

> It's not important to the topic but I'm curious about this since, in instruction selection at least, they both set SelectionDAGISel::OptLevel to CodeGenOpt::None and MCCodeGenInfo::OptLevel by one means or another. Could you point me at a difference between the two? I assume it's outside of instruction selection.

Without actually doing the research…  There are interactions between DAG construction, DAG Combine in particular, possibly CodeGenPrepare, and ISel, which I don't really understand. We tried to turn off all of DAG Combine, IIRC, but that totally didn't work, so instead we turn off individual bits and pieces.
We made a distinct effort to find all the IR and MachineFunction passes that don't run at O0 and turn them off individually (Chandler was particularly insistent that the pass manager should not be aware of 'optnone') and it's always possible that we missed some; those cases should be properly considered bugs.  And of course new passes get added from time to time, and typically people don't think about 'optnone' when they're doing that… again those are properly speaking bugs.

Regarding the "circuit-breaker" analogy, 'optnone' itself is a kind of user-visible circuit breaker, and making it not break the entire circuit could be considered a little strange.  But as I said, it does seem like a useful debugging tactic for 'optnone' itself.
--paulr

From: Daniel Sanders [mailto:Daniel.Sanders at imgtec.com]
Sent: Thursday, November 19, 2015 3:45 AM
To: Robinson, Paul; llvm-dev at lists.llvm.org
Subject: RE: Mips unconditionally uses fast-isel?

> Well, 'optnone' is already not identical to –O0, and given the nature of things, probably can't be

It's not important to the topic but I'm curious about this since, in instruction selection at least, they both set SelectionDAGISel::OptLevel to CodeGenOpt::None and MCCodeGenInfo::OptLevel by one means or another. Could you point me at a difference between the two? I assume it's outside of instruction selection.

> P.S. One nit, the "O0 + optnone" case should not have an asterisk, the FastISel flag is not manipulated if the opt level is already zero. Does not affect the strength of your argument, of course.

Bugs aside, you're right. I made this table using the Mips target so it was switching from O2 to O0.

I'll try to fix that everything-is-O2 bug today/tomorrow. It will probably be tomorrow due to other commitments.

> This is a different motivation than Daniel expressed, which is a more principled idea coming from the "optnone should exactly match –O0" misconception

It doesn't change anything but just for the record: While I do think that this should be true as far as possible, my opinion on -fast-isel=false is more rooted in the idea that emergency overrides shouldn't be overridable by normal use. As an analogy, circuit breakers shouldn't be overridable by a light switch.

From: Robinson, Paul [mailto:Paul_Robinson at playstation.sony.com]
Sent: 18 November 2015 16:04
To: Daniel Sanders; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Subject: RE: Mips unconditionally uses fast-isel?

Well, 'optnone' is already not identical to –O0, and given the nature of things, probably can't be; but I am persuaded that it's reasonable for it to honor the –fast-isel option as a debugging tactic.  I'll take an AI to make this happen.
Thanks,
--paulr
P.S. One nit, the "O0 + optnone" case should not have an asterisk, the FastISel flag is not manipulated if the opt level is already zero. Does not affect the strength of your argument, of course.

From: Daniel Sanders [mailto:Daniel.Sanders at imgtec.com]
Sent: Wednesday, November 18, 2015 2:19 AM
To: Robinson, Paul; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Subject: RE: Mips unconditionally uses fast-isel?


> -----Original Message-----

> From: Robinson, Paul [mailto:Paul_Robinson at playstation.sony.com]

> Sent: 17 November 2015 22:58

> To: Daniel Sanders; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>

> Subject: RE: Mips unconditionally uses fast-isel?

>

> > > The other thing that might work, is having TargetMachine remember how

> > > the fast-isel option got set, and make OptLevelChanger do the right

> > > thing. But that seems like a hack to work around Mips not obeying the

> > > specified optimization level, honestly.

> >

> > I think we should do that as well. I don't think it's right that optnone

> > enables Fast ISel even when it's been explicitly disabled. It should do

> > the same checks as addPassesToGenerateCode() does.

>

> Hm?  What you're asking for is that "-O2" and "-O2 -fast-isel=none" are

> identical, unless you have an 'optnone' function. Do you really have a

> use-case for controlling the codegen path for an 'optnone' function?

> The whole point of 'optnone' is to avoid optimizations.

> --paulr

>



No, that's already true. -O2 doesn't try to enable Fast ISel (unless the optnone attribute is given) so –fast-isel=false has no effect.



I'm saying that optnone means 'use –O0 for this function' and that optnone should

respect non-default values of the -fast-isel flag like –O0 does. This is the behaviour I'd expect:




-fast-isel=false


-fast-isel=default


-fast-isel=true


-O0


SelectionDAG


FastISel


FastISel


-O0 + optnone attribute


SelectionDAG*


FastISel


FastISel


-O1 + optnone attribute


SelectionDAG*


FastISel


FastISel


-O2 + optnone attribute


SelectionDAG*


FastISel


FastISel


-O3 + optnone attribute


SelectionDAG*


FastISel


FastISel


-O1


SelectionDAG


SelectionDAG


FastISel


-O2


SelectionDAG


SelectionDAG


FastISel


-O3


SelectionDAG


SelectionDAG


FastISel


The cells marked with '*' differ from the current behaviour.



In terms of code, I think this part of OptLevelChanger::OptLevelChanger():

   if (NewOptLevel == CodeGenOpt::None) {

      DEBUG(dbgs() << "\nEnable FastISel for Function "

            << IS.MF->getFunction()->getName() << "\n");

      IS.TM.setFastISel(true);

    }

Should be:

    if (NewOptLevel == CodeGenOpt::None) {

      DEBUG(dbgs() << "\nEnable FastISel for Function "

            << IS.MF->getFunction()->getName() << "\n");

      IS.TM.setFastISel(EnableFastISelOption != cl::BOU_FALSE);

    }

Where EnableFastISelOption has the same value as the global in LLVMTargetMachine.cpp



The main reason I'm asking for this is that I think it's weird to for optnone to use a different code generator

than  –O0. These hidden overrides exist to help us debug code generation problems and, faced with a code

generation bug, –fast-isel=false is useful for quickly determining whether it's in FastISel or somewhere else.

The current behaviour allows optnone to overrule the hidden option to force-disable FastISel which will give

misleading guidance for bugs that lie in functions with optnone.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151122/20540ced/attachment.html>