Page MenuHomePhabricator

Remove Clang support for '-fvectorize-slp-aggressive' which used LLVM's basic block vectorizer. This vectorizer has had no known users for many, many years and is completely surpassed by the normal '-fvectorize-slp'-controlled SLP vectorizer in LLVM.
ClosedPublic

Authored by chandlerc on Jun 29 2017, 4:29 PM.

Details

Summary

Given the long time for which this path has been unused, off by default,
and unnecessary this completely removes the flags. If users are deeply
concerned about the commandline compatibility, they can be added back
but it seems somewhat pointelss.

Hal proposed this back in 2014 to no objections:
http://lists.llvm.org/pipermail/llvm-dev/2014-November/079091.html

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc created this revision.Jun 29 2017, 4:29 PM
rsmith accepted this revision.Jun 29 2017, 4:33 PM

Removing a -cc1 flag is OK: we explicitly tell people that the -cc1 interface is subject to change without notice.

This revision is now accepted and ready to land.Jun 29 2017, 4:33 PM
hfinkel accepted this revision.Jun 29 2017, 4:35 PM
hfinkel added a subscriber: hfinkel.

LGTM, but you need to get the:

def fslp_vectorize_aggressive : Flag<["-"], "fslp-vectorize-aggressive">, Group<f_Group>,
  HelpText<"Enable the BB vectorization passes">;
def fno_slp_vectorize_aggressive : Flag<["-"], "fno-slp-vectorize-aggressive">, Group<f_Group>;

in include/clang/Driver/Options.td as well. There's corresponding code in lib/Driver/ToolChains/Clang.cpp:

// -fno-slp-vectorize-aggressive is default.
if (Args.hasFlag(options::OPT_fslp_vectorize_aggressive,
                 options::OPT_fno_slp_vectorize_aggressive, false))
  CmdArgs.push_back("-vectorize-slp-aggressive");

Removing a -cc1 flag is OK: we explicitly tell people that the -cc1 interface is subject to change without notice.

I'm assuming we can remove the regular flag as well. They'll just get an "unused argument" warning, which I think makes sense in this context. Do you think that we should do more? I know some people are using this flag, but I don't think many. We should remember to add something to the release notes.

chandlerc updated this revision to Diff 104775.Jun 29 2017, 4:41 PM

Remove the rest of the flag handling that I missed the first time grepping through...

chandlerc requested review of this revision.Jun 29 2017, 5:02 PM
chandlerc edited edge metadata.

Removing a -cc1 flag is OK: we explicitly tell people that the -cc1 interface is subject to change without notice.

What do you think about this now that it is a main flag?

rsmith accepted this revision.Jun 29 2017, 6:15 PM

LGTM

Per discussion on IRC, a follow-up patch from @nbjoerg will add the flag back with it just producing a warning that the flag has been removed and has no effect. While the usage of the driver flag is very low, the cost of keeping it around for compatibility is minuscule.

This revision is now accepted and ready to land.Jun 29 2017, 6:15 PM
This revision was automatically updated to reflect the committed changes.