Robinson, Paul via llvm-dev
2015-Nov-18 18:42 UTC
[llvm-dev] Mips unconditionally uses fast-isel?
The driving goal of 'optnone' is to have an easy way for programmers to get an "-O0 like" debugging experience for selected functions, without making them build everything with –O0. To that end, we turn off as much optimization as we reasonably can, but in the context of a pipeline that is generally expecting optimizations to be enabled, in practice we can't exactly match –O0 because "things break" if we turn off everything. Luckily, exactly matching –O0 isn't a requirement. Contemplating the "things break" situation, I am entirely willing to believe (it may have actually happened) that problems can occur when using FastISel in a pipeline that isn't expecting it. So, for purposes of diagnosing these 'optnone' problems, it seems useful to be able to force 'optnone' not to use FastISel. This is a different motivation than Daniel expressed, which is a more principled idea coming from the "optnone should exactly match –O0" misconception; but the conclusion (that we should respect an explicit –fast-isel=false within 'optnone' functions) is the same. --paulr From: Eric Christopher [mailto:echristo at gmail.com] Sent: Wednesday, November 18, 2015 9:48 AM To: Robinson, Paul; Daniel Sanders; llvm-dev at lists.llvm.org Subject: Re: [llvm-dev] Mips unconditionally uses fast-isel? I'd have figured optnone was "no optimizations" not "use the entire O0 code path"? At least that seemed to be the intent when you added it Paul? -eric On Wed, Nov 18, 2015 at 8:05 AM Robinson, Paul via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: 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<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. _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151118/d11aa90d/attachment.html>
Eric Christopher via llvm-dev
2015-Nov-18 19:06 UTC
[llvm-dev] Mips unconditionally uses fast-isel?
On Wed, Nov 18, 2015 at 10:42 AM Robinson, Paul < Paul_Robinson at playstation.sony.com> wrote:> The driving goal of 'optnone' is to have an easy way for programmers to > get an "-O0 like" debugging experience for selected functions, without > making them build everything with –O0. > > To that end, we turn off as much optimization as we reasonably can, but in > the context of a pipeline that is generally expecting optimizations to be > enabled, in practice we can't exactly match –O0 because "things break" if > we turn off everything. Luckily, exactly matching –O0 isn't a requirement. > > > > Contemplating the "things break" situation, I am entirely willing to > believe (it may have actually happened) that problems can occur when using > FastISel in a pipeline that isn't expecting it. So, for purposes of > diagnosing these 'optnone' problems, it seems useful to be able to force > 'optnone' not to use FastISel. This is a different motivation than Daniel > expressed, which is a more principled idea coming from the "optnone should > exactly match –O0" misconception; but the conclusion (that we should > respect an explicit –fast-isel=false within 'optnone' functions) is the > same. > > Works for me, should probably document it under some documentation for thefunction attribute. Thanks! -eric> --paulr > > > > *From:* Eric Christopher [mailto:echristo at gmail.com] > *Sent:* Wednesday, November 18, 2015 9:48 AM > *To:* Robinson, Paul; Daniel Sanders; llvm-dev at lists.llvm.org > *Subject:* Re: [llvm-dev] Mips unconditionally uses fast-isel? > > > > I'd have figured optnone was "no optimizations" not "use the entire O0 > code path"? > > > > At least that seemed to be the intent when you added it Paul? > > > > -eric > > > > On Wed, Nov 18, 2015 at 8:05 AM Robinson, Paul via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > 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 > > > *Subject:* RE: Mips unconditionally uses fast-isel? > > > > > -----Original Message----- > > > From: Robinson, Paul [mailto:Paul_Robinson at playstation.sony.com > <Paul_Robinson at playstation.sony.com>] > > > Sent: 17 November 2015 22:58 > > > To: Daniel Sanders; 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. > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151118/43a3eeeb/attachment.html>
Robinson, Paul via llvm-dev
2015-Nov-18 23:24 UTC
[llvm-dev] Mips unconditionally uses fast-isel?
It looks like the Mips target is not ignoring –O0 completely; only for ISel purposes. In the test I mentioned originally (Mips/emergency-spill-slot-near-fp.ll) I can remove –fast-isel=false and 'optnone' from the test, and it passes; but it still needs –O0, or the spill that it's looking for doesn't happen. Once Mips pays attention to –O0 for ISel purposes, then it would need –fast-isel=false again, I think, because the test is not using FastISel now. --paulr From: Eric Christopher [mailto:echristo at gmail.com] Sent: Wednesday, November 18, 2015 11:07 AM To: Robinson, Paul; Daniel Sanders; llvm-dev at lists.llvm.org Subject: Re: [llvm-dev] Mips unconditionally uses fast-isel? On Wed, Nov 18, 2015 at 10:42 AM Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto:Paul_Robinson at playstation.sony.com>> wrote: The driving goal of 'optnone' is to have an easy way for programmers to get an "-O0 like" debugging experience for selected functions, without making them build everything with –O0. To that end, we turn off as much optimization as we reasonably can, but in the context of a pipeline that is generally expecting optimizations to be enabled, in practice we can't exactly match –O0 because "things break" if we turn off everything. Luckily, exactly matching –O0 isn't a requirement. Contemplating the "things break" situation, I am entirely willing to believe (it may have actually happened) that problems can occur when using FastISel in a pipeline that isn't expecting it. So, for purposes of diagnosing these 'optnone' problems, it seems useful to be able to force 'optnone' not to use FastISel. This is a different motivation than Daniel expressed, which is a more principled idea coming from the "optnone should exactly match –O0" misconception; but the conclusion (that we should respect an explicit –fast-isel=false within 'optnone' functions) is the same. Works for me, should probably document it under some documentation for the function attribute. Thanks! -eric --paulr From: Eric Christopher [mailto:echristo at gmail.com<mailto:echristo at gmail.com>] Sent: Wednesday, November 18, 2015 9:48 AM To: Robinson, Paul; Daniel Sanders; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> Subject: Re: [llvm-dev] Mips unconditionally uses fast-isel? I'd have figured optnone was "no optimizations" not "use the entire O0 code path"? At least that seemed to be the intent when you added it Paul? -eric On Wed, Nov 18, 2015 at 8:05 AM Robinson, Paul via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: 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<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. _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151118/9c3e34d0/attachment-0001.html>