Meador Inge
2012-Aug-03 19:56 UTC
[LLVMdev] Proposal to merge SimplifyLibCalls into InstCombiner
On 08/02/2012 11:11 AM, Chris Lattner wrote:>> A new self-contained `LibCallSimplifier` class will be created. An instance >> of the class will be instantiated when running the `InstCombiner` pass. It's >> folding functionality will be invoked from `InstCombiner::tryOptimizeCall` and >> the implementation will be table-driven like `SimplifyLibCalls`. All of the >> existing fortified library call simplifiers will be migrated to this new class >> at this step. > > Ok, the idea is that other general parts of the compiler can use the LibCallSimplifier > interface as well? That makes sense to me.Yup, that's it.>> An option for enabling/disabling library call simplification in `InstCombiner` >> will be available. For backwards compatibility perhaps it should remain >> '-simplify-libcalls'. The `NumSimplified` and `NumAnnotated` statistics shall >> be added to `InstCombiner`. > > There is no need to preserver the -simplify-libcalls flag. It's an internal > implementation detail of the compiler, one that is better left in the past :)Do you think we still need some way to disable just the library call simplification now that it is a part of the instruction combiner?>> 1. What should be done about the `SimplifyFortifiedLibCalls` use in >> `CodeGenPrepare`? > > I think it stays where it is. CGP runs right before the code generator > kicks in. The idea here is that we want to leave the fortified libcalls > in the IR as long as possible on the off chance that instcombine or something > else can expose range information about underlying buffers... however, if we > get all the way to code gen, then we failed to find a length, so we remove them.Thanks for the explanation. As a part of the merger I really want `SimplifyFortifiedLibCalls` to go away as well, since the fortified library call folding is being rolled into `LibCallSimplifier`. I will look at applying `LibCallSimplifier` to the CGP case as well (the conditions for when it is OK to fold is slightly different for CGP). Thanks for the feedback! -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software
Chris Lattner
2012-Aug-03 20:15 UTC
[LLVMdev] Proposal to merge SimplifyLibCalls into InstCombiner
On Aug 3, 2012, at 12:56 PM, Meador Inge <meadori at codesourcery.com> wrote:>>> An option for enabling/disabling library call simplification in `InstCombiner` >>> will be available. For backwards compatibility perhaps it should remain >>> '-simplify-libcalls'. The `NumSimplified` and `NumAnnotated` statistics shall >>> be added to `InstCombiner`. >> >> There is no need to preserver the -simplify-libcalls flag. It's an internal >> implementation detail of the compiler, one that is better left in the past :) > > Do you think we still need some way to disable just the library call > simplification now that it is a part of the instruction combiner?It should just be part of instcombine, but be gated by what TargetLibraryInfo says. If you want to turn off these libcall optimizations, it should be by marking the libcalls as off-limits in TLI.> >>> 1. What should be done about the `SimplifyFortifiedLibCalls` use in >>> `CodeGenPrepare`? >> >> I think it stays where it is. CGP runs right before the code generator >> kicks in. The idea here is that we want to leave the fortified libcalls >> in the IR as long as possible on the off chance that instcombine or something >> else can expose range information about underlying buffers... however, if we >> get all the way to code gen, then we failed to find a length, so we remove them. > > Thanks for the explanation. As a part of the merger I really want > `SimplifyFortifiedLibCalls` to go away as well, since the fortified > library call folding is being rolled into `LibCallSimplifier`. I will > look at applying `LibCallSimplifier` to the CGP case as well (the conditions > for when it is OK to fold is slightly different for CGP).Ok, please just tackle this as a follow-on step. Thanks Meador! -Chris
Meador Inge
2012-Nov-08 04:31 UTC
[LLVMdev] Proposal to merge SimplifyLibCalls into InstCombiner
On 08/03/2012 03:15 PM, Chris Lattner wrote:> On Aug 3, 2012, at 12:56 PM, Meador Inge <meadori at codesourcery.com> wrote: >>>> An option for enabling/disabling library call simplification in `InstCombiner` >>>> will be available. For backwards compatibility perhaps it should remain >>>> '-simplify-libcalls'. The `NumSimplified` and `NumAnnotated` statistics shall >>>> be added to `InstCombiner`. >>> >>> There is no need to preserver the -simplify-libcalls flag. It's an internal >>> implementation detail of the compiler, one that is better left in the past :) >> >> Do you think we still need some way to disable just the library call >> simplification now that it is a part of the instruction combiner? > > It should just be part of instcombine, but be gated by what TargetLibraryInfo says. > If you want to turn off these libcall optimizations, it should be by marking the > libcalls as off-limits in TLI.There is also the matter of -fno-builtin, which I missed in my original proposal. Patch posted here: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121105/155370.html. -- Meador Inge CodeSourcery / Mentor Embedded