[llvm] r298701 - Don't build up std::vectors with constant sizes when an array suffices.

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 24 11:08:19 PDT 2017


Thanks!

On Fri, Mar 24, 2017 at 11:07 AM Benjamin Kramer <benny.kra at gmail.com>
wrote:

> r298719 should help.
>
> On Fri, Mar 24, 2017 at 7:01 PM, Vitaly Buka <vitalybuka at google.com>
> wrote:
> > Could you please fix or revert the patch?
> >
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/3684/steps/build%20clang%2Fmsan/logs/stdio
> >
> > FAILED:
> >
> lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/SIMachineScheduler.cpp.o
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build0/bin/clang++
> > -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_DEBUG -D_GNU_SOURCE
> > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> > -Ilib/Target/AMDGPU
> >
> -I/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU
> > -Iinclude
> >
> -I/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include
> > -nostdinc++ -isystem
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include
> > -isystem
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1
> > -lc++abi
> >
> -Wl,--rpath=/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/lib
> >
> -L/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/lib
> > -fsanitize=memory -w -stdlib=libc++ -fPIC -fvisibility-inlines-hidden
> > -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter
> -Wwrite-strings
> > -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long
> > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> > -Wstring-conversion -fno-omit-frame-pointer -gline-tables-only
> > -fsanitize=memory -fcolor-diagnostics -ffunction-sections -fdata-sections
> > -O3    -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT
> >
> lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/SIMachineScheduler.cpp.o
> > -MF
> >
> lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/SIMachineScheduler.cpp.o.d
> > -o
> >
> lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/SIMachineScheduler.cpp.o
> > -c
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:1830:9:
> > error: constexpr variable 'Variants' must be initialized by a constant
> > expression
> >         Variants[] = {
> >         ^            ~
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:1831:7:
> > note: non-constexpr constructor
> 'pair<llvm::SISchedulerBlockCreatorVariant,
> > llvm::SISchedulerBlockSchedulerVariant, false>' cannot be used in a
> constant
> > expression
> >       { LatenciesAlone, BlockRegUsageLatency },
> >       ^
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1/utility:436:5:
> > note: declared here
> >     pair(_U1&& __u1, _U2&& __u2)
> >     ^
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:1851:9:
> > error: constexpr variable 'Variants' must be initialized by a constant
> > expression
> >         Variants[] = {
> >         ^            ~
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:1853:7:
> > note: non-constexpr constructor
> 'pair<llvm::SISchedulerBlockCreatorVariant,
> > llvm::SISchedulerBlockSchedulerVariant, false>' cannot be used in a
> constant
> > expression
> >       { LatenciesAlone, BlockRegUsage },
> >       ^
> >
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_msan/include/c++/v1/utility:436:5:
> > note: declared here
> >     pair(_U1&& __u1, _U2&& __u2)
> >     ^
> > 2 errors generated.
> > [464/1124] Building XCoreGenRegisterInfo.inc...
> > [465/1124] Building XCoreGenInstrInfo.inc...
> > [466/1124] Building X86GenRegisterInfo.inc...
> > [467/
> >
> >
> > On Fri, Mar 24, 2017 at 7:24 AM Benjamin Kramer via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: d0k
> >> Date: Fri Mar 24 09:11:47 2017
> >> New Revision: 298701
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=298701&view=rev
> >> Log:
> >> Don't build up std::vectors with constant sizes when an array suffices.
> >>
> >> NFC.
> >>
> >> Modified:
> >>     llvm/trunk/include/llvm/Support/FormatProviders.h
> >>     llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp
> >>     llvm/trunk/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
> >>
> >> Modified: llvm/trunk/include/llvm/Support/FormatProviders.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatProviders.h?rev=298701&r1=298700&r2=298701&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/Support/FormatProviders.h (original)
> >> +++ llvm/trunk/include/llvm/Support/FormatProviders.h Fri Mar 24
> 09:11:47
> >> 2017
> >> @@ -370,8 +370,7 @@ template <typename IterT> class format_p
> >>        return Default;
> >>      }
> >>
> >> -    std::vector<const char *> Delims = {"[]", "<>", "()"};
> >> -    for (const char *D : Delims) {
> >> +    for (const char *D : {"[]", "<>", "()"}) {
> >>        if (Style.front() != D[0])
> >>          continue;
> >>        size_t End = Style.find_first_of(D[1]);
> >>
> >> Modified: llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp?rev=298701&r1=298700&r2=298701&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp (original)
> >> +++ llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp Fri Mar 24
> >> 09:11:47 2017
> >> @@ -1825,7 +1825,9 @@ void SIScheduleDAGMI::schedule()
> >>    // if VGPR usage is extremely high, try other good performing
> variants
> >>    // which could lead to lower VGPR usage
> >>    if (Best.MaxVGPRUsage > 180) {
> >> -    std::vector<std::pair<SISchedulerBlockCreatorVariant,
> >> SISchedulerBlockSchedulerVariant>> Variants = {
> >> +    static constexpr std::pair<SISchedulerBlockCreatorVariant,
> >> +                               SISchedulerBlockSchedulerVariant>
> >> +        Variants[] = {
> >>        { LatenciesAlone, BlockRegUsageLatency },
> >>  //      { LatenciesAlone, BlockRegUsage },
> >>        { LatenciesGrouped, BlockLatencyRegUsage },
> >> @@ -1844,7 +1846,9 @@ void SIScheduleDAGMI::schedule()
> >>    // if VGPR usage is still extremely high, we may spill. Try other
> >> variants
> >>    // which are less performing, but that could lead to lower VGPR
> usage.
> >>    if (Best.MaxVGPRUsage > 200) {
> >> -    std::vector<std::pair<SISchedulerBlockCreatorVariant,
> >> SISchedulerBlockSchedulerVariant>> Variants = {
> >> +    static constexpr std::pair<SISchedulerBlockCreatorVariant,
> >> +                               SISchedulerBlockSchedulerVariant>
> >> +        Variants[] = {
> >>  //      { LatenciesAlone, BlockRegUsageLatency },
> >>        { LatenciesAlone, BlockRegUsage },
> >>  //      { LatenciesGrouped, BlockLatencyRegUsage },
> >>
> >> Modified: llvm/trunk/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp?rev=298701&r1=298700&r2=298701&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp (original)
> >> +++ llvm/trunk/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp Fri Mar 24
> >> 09:11:47 2017
> >> @@ -37,15 +37,10 @@ class X86EVEX2VEXTablesEmitter {
> >>    std::vector<Entry> EVEX2VEX256;
> >>
> >>    // Represents a manually added entry to the tables
> >> -  class ManualEntry {
> >> -  public:
> >> -    std::string EVEXInstStr;
> >> -    std::string VEXInstStr;
> >> +  struct ManualEntry {
> >> +    StringLiteral EVEXInstStr;
> >> +    StringLiteral VEXInstStr;
> >>      bool Is128Bit;
> >> -
> >> -    ManualEntry(std::string EVEXInstStr, std::string VEXInstStr, bool
> >> Is128Bit)
> >> -        : EVEXInstStr(EVEXInstStr), VEXInstStr(VEXInstStr),
> >> Is128Bit(Is128Bit) {
> >> -    }
> >>    };
> >>
> >>  public:
> >> @@ -59,81 +54,30 @@ private:
> >>    // X86EvexToVexCompressTableEntry
> >>    void printTable(const std::vector<Entry> &Table, raw_ostream &OS);
> >>
> >> -  // List of EVEX instructions that match VEX instructions by the
> >> encoding
> >> -  // but do not perform the same operation.
> >> -  const std::vector<std::string> ExceptionList = {
> >> -      "VCVTQQ2PD",
> >> -      "VCVTQQ2PS",
> >> -      "VPMAXSQ",
> >> -      "VPMAXUQ",
> >> -      "VPMINSQ",
> >> -      "VPMINUQ",
> >> -      "VPMULLQ",
> >> -      "VPSRAQ",
> >> -      "VDBPSADBW",
> >> -      "VRNDSCALE",
> >> -      "VSCALEFPS"
> >> -  };
> >> -
> >>    bool inExceptionList(const CodeGenInstruction *Inst) {
> >> +    // List of EVEX instructions that match VEX instructions by the
> >> encoding
> >> +    // but do not perform the same operation.
> >> +    static constexpr StringLiteral ExceptionList[] = {
> >> +        "VCVTQQ2PD",
> >> +        "VCVTQQ2PS",
> >> +        "VPMAXSQ",
> >> +        "VPMAXUQ",
> >> +        "VPMINSQ",
> >> +        "VPMINUQ",
> >> +        "VPMULLQ",
> >> +        "VPSRAQ",
> >> +        "VDBPSADBW",
> >> +        "VRNDSCALE",
> >> +        "VSCALEFPS"
> >> +    };
> >>      // Instruction's name starts with one of the entries in the
> exception
> >> list
> >> -    for (const std::string& InstStr : ExceptionList) {
> >> +    for (StringRef InstStr : ExceptionList) {
> >>        if (Inst->TheDef->getName().startswith(InstStr))
> >>          return true;
> >>      }
> >>      return false;
> >>    }
> >>
> >> -  // Some VEX instructions were duplicated to multiple EVEX versions
> due
> >> the
> >> -  // introduction of mask variants, and thus some of the EVEX versions
> >> have
> >> -  // different encoding than the VEX instruction. In order to maximize
> >> the
> >> -  // compression we add these entries manually.
> >> -  const std::vector<ManualEntry> ManuallyAddedEntries = {
> >> -    // EVEX-Inst              VEX-Inst           Is128-bit
> >> -    {"VMOVDQU8Z128mr",      "VMOVDQUmr",       true},
> >> -    {"VMOVDQU8Z128rm",      "VMOVDQUrm",       true},
> >> -    {"VMOVDQU8Z128rr",      "VMOVDQUrr",       true},
> >> -    {"VMOVDQU8Z128rr_REV",  "VMOVDQUrr_REV",   true},
> >> -    {"VMOVDQU16Z128mr",     "VMOVDQUmr",       true},
> >> -    {"VMOVDQU16Z128rm",     "VMOVDQUrm",       true},
> >> -    {"VMOVDQU16Z128rr",     "VMOVDQUrr",       true},
> >> -    {"VMOVDQU16Z128rr_REV", "VMOVDQUrr_REV",   true},
> >> -    {"VMOVDQU8Z256mr",      "VMOVDQUYmr",      false},
> >> -    {"VMOVDQU8Z256rm",      "VMOVDQUYrm",      false},
> >> -    {"VMOVDQU8Z256rr",      "VMOVDQUYrr",      false},
> >> -    {"VMOVDQU8Z256rr_REV",  "VMOVDQUYrr_REV",  false},
> >> -    {"VMOVDQU16Z256mr",     "VMOVDQUYmr",      false},
> >> -    {"VMOVDQU16Z256rm",     "VMOVDQUYrm",      false},
> >> -    {"VMOVDQU16Z256rr",     "VMOVDQUYrr",      false},
> >> -    {"VMOVDQU16Z256rr_REV", "VMOVDQUYrr_REV",  false},
> >> -
> >> -    {"VPERMILPDZ128mi",     "VPERMILPDmi",     true},
> >> -    {"VPERMILPDZ128ri",     "VPERMILPDri",     true},
> >> -    {"VPERMILPDZ128rm",     "VPERMILPDrm",     true},
> >> -    {"VPERMILPDZ128rr",     "VPERMILPDrr",     true},
> >> -    {"VPERMILPDZ256mi",     "VPERMILPDYmi",    false},
> >> -    {"VPERMILPDZ256ri",     "VPERMILPDYri",    false},
> >> -    {"VPERMILPDZ256rm",     "VPERMILPDYrm",    false},
> >> -    {"VPERMILPDZ256rr",     "VPERMILPDYrr",    false},
> >> -
> >> -    {"VPBROADCASTQZ128m",   "VPBROADCASTQrm",  true},
> >> -    {"VPBROADCASTQZ128r",   "VPBROADCASTQrr",  true},
> >> -    {"VPBROADCASTQZ256m",   "VPBROADCASTQYrm", false},
> >> -    {"VPBROADCASTQZ256r",   "VPBROADCASTQYrr", false},
> >> -
> >> -    {"VBROADCASTSDZ256m",   "VBROADCASTSDYrm", false},
> >> -    {"VBROADCASTSDZ256r",   "VBROADCASTSDYrr", false},
> >> -
> >> -    {"VEXTRACTF64x2Z256mr", "VEXTRACTF128mr",  false},
> >> -    {"VEXTRACTF64x2Z256rr", "VEXTRACTF128rr",  false},
> >> -    {"VEXTRACTI64x2Z256mr", "VEXTRACTI128mr",  false},
> >> -    {"VEXTRACTI64x2Z256rr", "VEXTRACTI128rr",  false},
> >> -
> >> -    {"VINSERTF64x2Z256rm",  "VINSERTF128rm",   false},
> >> -    {"VINSERTF64x2Z256rr",  "VINSERTF128rr",   false},
> >> -    {"VINSERTI64x2Z256rm",  "VINSERTI128rm",   false},
> >> -    {"VINSERTI64x2Z256rr",  "VINSERTI128rr",   false}
> >> -  };
> >>  };
> >>
> >>  void X86EVEX2VEXTablesEmitter::printTable(const std::vector<Entry>
> >> &Table,
> >> @@ -153,6 +97,57 @@ void X86EVEX2VEXTablesEmitter::printTabl
> >>         << ", X86::" << Pair.second->TheDef->getName() << " },\n";
> >>    }
> >>
> >> +  // Some VEX instructions were duplicated to multiple EVEX versions
> due
> >> the
> >> +  // introduction of mask variants, and thus some of the EVEX versions
> >> have
> >> +  // different encoding than the VEX instruction. In order to maximize
> >> the
> >> +  // compression we add these entries manually.
> >> +  static constexpr ManualEntry ManuallyAddedEntries[] = {
> >> +      // EVEX-Inst            VEX-Inst           Is128-bit
> >> +      {"VMOVDQU8Z128mr",      "VMOVDQUmr",       true},
> >> +      {"VMOVDQU8Z128rm",      "VMOVDQUrm",       true},
> >> +      {"VMOVDQU8Z128rr",      "VMOVDQUrr",       true},
> >> +      {"VMOVDQU8Z128rr_REV",  "VMOVDQUrr_REV",   true},
> >> +      {"VMOVDQU16Z128mr",     "VMOVDQUmr",       true},
> >> +      {"VMOVDQU16Z128rm",     "VMOVDQUrm",       true},
> >> +      {"VMOVDQU16Z128rr",     "VMOVDQUrr",       true},
> >> +      {"VMOVDQU16Z128rr_REV", "VMOVDQUrr_REV",   true},
> >> +      {"VMOVDQU8Z256mr",      "VMOVDQUYmr",      false},
> >> +      {"VMOVDQU8Z256rm",      "VMOVDQUYrm",      false},
> >> +      {"VMOVDQU8Z256rr",      "VMOVDQUYrr",      false},
> >> +      {"VMOVDQU8Z256rr_REV",  "VMOVDQUYrr_REV",  false},
> >> +      {"VMOVDQU16Z256mr",     "VMOVDQUYmr",      false},
> >> +      {"VMOVDQU16Z256rm",     "VMOVDQUYrm",      false},
> >> +      {"VMOVDQU16Z256rr",     "VMOVDQUYrr",      false},
> >> +      {"VMOVDQU16Z256rr_REV", "VMOVDQUYrr_REV",  false},
> >> +
> >> +      {"VPERMILPDZ128mi",     "VPERMILPDmi",     true},
> >> +      {"VPERMILPDZ128ri",     "VPERMILPDri",     true},
> >> +      {"VPERMILPDZ128rm",     "VPERMILPDrm",     true},
> >> +      {"VPERMILPDZ128rr",     "VPERMILPDrr",     true},
> >> +      {"VPERMILPDZ256mi",     "VPERMILPDYmi",    false},
> >> +      {"VPERMILPDZ256ri",     "VPERMILPDYri",    false},
> >> +      {"VPERMILPDZ256rm",     "VPERMILPDYrm",    false},
> >> +      {"VPERMILPDZ256rr",     "VPERMILPDYrr",    false},
> >> +
> >> +      {"VPBROADCASTQZ128m",   "VPBROADCASTQrm",  true},
> >> +      {"VPBROADCASTQZ128r",   "VPBROADCASTQrr",  true},
> >> +      {"VPBROADCASTQZ256m",   "VPBROADCASTQYrm", false},
> >> +      {"VPBROADCASTQZ256r",   "VPBROADCASTQYrr", false},
> >> +
> >> +      {"VBROADCASTSDZ256m",   "VBROADCASTSDYrm", false},
> >> +      {"VBROADCASTSDZ256r",   "VBROADCASTSDYrr", false},
> >> +
> >> +      {"VEXTRACTF64x2Z256mr", "VEXTRACTF128mr",  false},
> >> +      {"VEXTRACTF64x2Z256rr", "VEXTRACTF128rr",  false},
> >> +      {"VEXTRACTI64x2Z256mr", "VEXTRACTI128mr",  false},
> >> +      {"VEXTRACTI64x2Z256rr", "VEXTRACTI128rr",  false},
> >> +
> >> +      {"VINSERTF64x2Z256rm",  "VINSERTF128rm",   false},
> >> +      {"VINSERTF64x2Z256rr",  "VINSERTF128rr",   false},
> >> +      {"VINSERTI64x2Z256rm",  "VINSERTI128rm",   false},
> >> +      {"VINSERTI64x2Z256rr",  "VINSERTI128rr",   false}
> >> +  };
> >> +
> >>    // Print the manually added entries
> >>    for (const ManualEntry &Entry : ManuallyAddedEntries) {
> >>      if ((Table == EVEX2VEX128 && Entry.Is128Bit) ||
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/bf5a1127/attachment.html>


More information about the llvm-commits mailing list