r187969 - The only useful loop unrolling flag to give realistically is
Hal Finkel
hfinkel at anl.gov
Thu Aug 8 04:46:11 PDT 2013
----- 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.
-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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the cfe-commits
mailing list