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

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 24 11:07:27 PDT 2017


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


More information about the llvm-commits mailing list