Robinson, Paul via llvm-dev
2015-Nov-17 22:57 UTC
[llvm-dev] 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
Daniel Sanders via llvm-dev
2015-Nov-18 10:19 UTC
[llvm-dev] 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> 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/20151118/3196323f/attachment-0001.html>
Robinson, Paul via llvm-dev
2015-Nov-18 16:04 UTC
[llvm-dev] 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 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/20151118/fb84a588/attachment.html>
Pete Cooper via llvm-dev
2015-Nov-18 18:03 UTC
[llvm-dev] Mips unconditionally uses fast-isel?
Hi Daniel, Paul Thanks for looking in to this. I think its great to fix the inconsistency here. ...> On Nov 18, 2015, at 2:19 AM, Daniel Sanders via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > > -----Original Message----- > > From: Robinson, Paul [mailto:Paul_Robinson at playstation.sony.com <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 behavior.This table is excellent. Any chance you can find a good place in code to put it, or even somewhere in the docs? I did look to see if http://llvm.org/docs/WritingAnLLVMBackend.html#instruction-selector had a section on fast-isel but it surprisingly doesn’t. If it did, I think this table would have been good to put there to explain when a backend will get which selector. Cheers, Pete> > 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 <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/f3467810/attachment.html>