----- Original Message ----- > ----- Original Message ----- > > Author: chandlerc > > Date: Thu Aug 8 03:34:35 2013 > > New Revision: 187969 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=187969&view=rev > > Log: > > The only useful loop unrolling flag to give realistically is > > '-fno-unroll-loops'. The option to the backend is even called > > 'DisableUnrollLoops'. This is precisely the form that Clang > > *didn't* > > support. > > Maybe we should hook up the positive form to enabling the > more-aggressive unrolling options: -unroll-allow-partial > -unroll-runtime -- doing these kinds of unrolling is not always a > win (I think it gives a mean regression on x86_64, but it gives a > mean speedup on my A2 cores), but I believe that gcc does equivalent > things when -funroll-loops is given. > > Also, even with this change, -fno-unroll-loops with -O2 or -O3 might > not really prevent all loop unrolling because the loop vectorizer > also unrolls loops based on ILP (and register pressure) > considerations. I think that passing -force-vector-unroll=1 would > turn this off.
Chandler, thoughts? -Hal > > -Hal > > > We didn't recognize the flag, we didn't pass it to the CC1 > > layer, and even if we did we wouldn't use it. Clang only inspected > > the > > positive form of the flag, and only did so to enable loop unrolling > > when > > the optimization level wasn't high enough. This only occurs for an > > optimization level that even has a chance of running the loop > > unroller > > when optimizing for size. > > > > This commit wires up the 'no' variant, and switches the code to > > actually > > follow the standard flag pattern of using the last flag and > > allowing > > a flag in either direction to override the default. > > > > I think this is still wrong. I don't know why we disable the loop > > unroller entirely *from Clang* when optimizing for size, as the > > loop > > unrolling pass *already has special logic* for the case where the > > function is attributed as optimized for size! We should really be > > trusting that. Maybe in a follow-up patch, I don't really want to > > change > > behavior here. > > > > Modified: > > cfe/trunk/include/clang/Driver/Options.td > > cfe/trunk/lib/Driver/Tools.cpp > > cfe/trunk/lib/Frontend/CompilerInvocation.cpp > > cfe/trunk/test/Driver/clang_f_opts.c > > > > Modified: cfe/trunk/include/clang/Driver/Options.td > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=187969&r1=187968&r2=187969&view=diff > > ============================================================================== > > --- cfe/trunk/include/clang/Driver/Options.td (original) > > +++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 8 03:34:35 > > 2013 > > @@ -791,6 +791,8 @@ def ftrap_function_EQ : Joined<["-"], "f > > def funit_at_a_time : Flag<["-"], "funit-at-a-time">, > > Group<f_Group>; > > def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>, > > HelpText<"Turn on loop unroller">, Flags<[CC1Option]>; > > +def fno_unroll_loops : Flag<["-"], "fno-unroll-loops">, > > Group<f_Group>, > > + HelpText<"Turn off loop unroller">, Flags<[CC1Option]>; > > def funsigned_bitfields : Flag<["-"], "funsigned-bitfields">, > > Group<f_Group>; > > def funsigned_char : Flag<["-"], "funsigned-char">, > > Group<f_Group>; > > def funwind_tables : Flag<["-"], "funwind-tables">, > > Group<f_Group>; > > > > Modified: cfe/trunk/lib/Driver/Tools.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=187969&r1=187968&r2=187969&view=diff > > ============================================================================== > > --- cfe/trunk/lib/Driver/Tools.cpp (original) > > +++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 8 03:34:35 2013 > > @@ -2977,7 +2977,8 @@ void Clang::ConstructJob(Compilation &C, > > CmdArgs.push_back("-fwrapv"); > > } > > Args.AddLastArg(CmdArgs, options::OPT_fwritable_strings); > > - Args.AddLastArg(CmdArgs, options::OPT_funroll_loops); > > + Args.AddLastArg(CmdArgs, options::OPT_funroll_loops, > > + options::OPT_fno_unroll_loops); > > > > Args.AddLastArg(CmdArgs, options::OPT_pthread); > > > > > > Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=187969&r1=187968&r2=187969&view=diff > > ============================================================================== > > --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) > > +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Aug 8 > > 03:34:35 > > 2013 > > @@ -351,8 +351,9 @@ static bool ParseCodeGenArgs(CodeGenOpti > > Opts.OptimizeSize = getOptimizationLevelSize(Args); > > Opts.SimplifyLibCalls = !(Args.hasArg(OPT_fno_builtin) || > > Args.hasArg(OPT_ffreestanding)); > > - Opts.UnrollLoops = Args.hasArg(OPT_funroll_loops) || > > - (Opts.OptimizationLevel > 1 && > > !Opts.OptimizeSize); > > + Opts.UnrollLoops = > > + Args.hasFlag(OPT_funroll_loops, OPT_fno_unroll_loops, > > + (Opts.OptimizationLevel > 1 && > > !Opts.OptimizeSize)); > > > > Opts.Autolink = !Args.hasArg(OPT_fno_autolink); > > Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose); > > > > Modified: cfe/trunk/test/Driver/clang_f_opts.c > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang_f_opts.c?rev=187969&r1=187968&r2=187969&view=diff > > ============================================================================== > > --- cfe/trunk/test/Driver/clang_f_opts.c (original) > > +++ cfe/trunk/test/Driver/clang_f_opts.c Thu Aug 8 03:34:35 2013 > > @@ -37,6 +37,13 @@ > > // FP-CONTRACT-FAST-CHECK: -ffp-contract=fast > > // FP-CONTRACT-OFF-CHECK: -ffp-contract=off > > > > +// RUN: %clang -### -S -funroll-loops %s 2>&1 | FileCheck > > -check-prefix=CHECK-UNROLL-LOOPS %s > > +// RUN: %clang -### -S -fno-unroll-loops %s 2>&1 | FileCheck > > -check-prefix=CHECK-NO-UNROLL-LOOPS %s > > +// RUN: %clang -### -S -fno-unroll-loops -funroll-loops %s 2>&1 | > > FileCheck -check-prefix=CHECK-UNROLL-LOOPS %s > > +// RUN: %clang -### -S -funroll-loops -fno-unroll-loops %s 2>&1 | > > FileCheck -check-prefix=CHECK-NO-UNROLL-LOOPS %s > > +// CHECK-UNROLL-LOOPS: "-funroll-loops" > > +// CHECK-NO-UNROLL-LOOPS: "-fno-unroll-loops" > > + > > // RUN: %clang -### -S -fvectorize %s 2>&1 | FileCheck > > -check-prefix=CHECK-VECTORIZE %s > > // RUN: %clang -### -S -fno-vectorize -fvectorize %s 2>&1 | > > FileCheck -check-prefix=CHECK-VECTORIZE %s > > // RUN: %clang -### -S -fno-vectorize %s 2>&1 | FileCheck > > -check-prefix=CHECK-NO-VECTORIZE %s > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > -- > Hal Finkel > Assistant Computational Scientist > Leadership Computing Facility > Argonne National Laboratory > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
