[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:01:06 PDT 2017


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/a4e4898f/attachment-0001.html>


More information about the llvm-commits mailing list