[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