From add59ecb34e3003311b7e2318b16a0ef10c76d79 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Fri, 10 Jul 2020 12:47:58 -0400 Subject: [PATCH 01/14] Re-land [CodeView] Add full repro to LF_BUILDINFO record This patch adds some missing information to the LF_BUILDINFO which allows for rebuilding an .OBJ without any external dependency but the .OBJ itself (other than the compiler executable). Some tools need this information to reproduce a build without any knowledge of the build system. The LF_BUILDINFO therefore stores a full path to the compiler, the PWD (which is the CWD at program startup), a relative or absolute path to the TU, and the full CC1 command line. The command line needs to be freestanding (not depend on any environment variable). In the same way, MSVC doesn't store the provided command-line, but an expanded version (somehow their equivalent of CC1) which is also freestanding. For more information see PR36198 and D43002. Differential Revision: https://reviews.llvm.org/D80833 --- .../CodeGen/debug-info-codeview-buildinfo.c | 26 ++++ lld/COFF/PDB.cpp | 69 ++++++++++ .../COFF/Inputs/pdb_lines_1_relative.yaml | 127 ++++++++++++++---- .../COFF/Inputs/pdb_lines_2_relative.yaml | 93 ++++++++++--- lld/test/COFF/pdb-relative-source-lines.test | 36 ++++- lld/test/COFF/pdb-relative-source-lines2.test | 66 +++++++++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | 38 +++++- llvm/test/DebugInfo/COFF/build-info.ll | 2 +- .../test/DebugInfo/COFF/global-type-hashes.ll | 3 +- llvm/test/DebugInfo/COFF/types-basic.ll | 12 +- .../test/DebugInfo/COFF/types-data-members.ll | 12 +- 11 files changed, 428 insertions(+), 56 deletions(-) create mode 100644 clang/test/CodeGen/debug-info-codeview-buildinfo.c create mode 100644 lld/test/COFF/pdb-relative-source-lines2.test diff --git a/clang/test/CodeGen/debug-info-codeview-buildinfo.c b/clang/test/CodeGen/debug-info-codeview-buildinfo.c new file mode 100644 index 00000000000000..3434f5f8657917 --- /dev/null +++ b/clang/test/CodeGen/debug-info-codeview-buildinfo.c @@ -0,0 +1,26 @@ +// UNSUPPORTED: s390x +// RUN: %clang_cl /c /Z7 /Fo%t.obj -- %s +// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s +// RUN: %clang_cl /c /Z7 /Fo%t.obj -fdebug-compilation-dir . -- %s +// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix RELATIVE + +int main() { return 42; } + +// CHECK: Types (.debug$T) +// CHECK: ============================================================ +// CHECK: 0x[[PWD:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: [[PWDVAL:.+]] +// CHECK: 0x[[FILEPATH:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: [[FILEPATHVAL:.+[\\/]debug-info-codeview-buildinfo.c]] +// CHECK: 0x[[ZIPDB:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: +// CHECK: 0x[[TOOL:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: [[TOOLVAL:.+[\\/]clang.*]] +// CHECK: 0x[[CMDLINE:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: "-cc1 +// CHECK: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}] +// CHECK: 0x[[PWD]]: `[[PWDVAL]]` +// CHECK: 0x[[TOOL]]: `[[TOOLVAL]]` +// CHECK: 0x[[FILEPATH]]: `[[FILEPATHVAL]]` +// CHECK: 0x[[ZIPDB]]: `` +// CHECK: 0x[[CMDLINE]]: `"-cc1 + +// RELATIVE: Types (.debug$T) +// RELATIVE: ============================================================ +// RELATIVE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}] +// RELATIVE: 0x{{.+}}: `.` diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp index 49d04add5be04f..5738eae7d6c40b 100644 --- a/lld/COFF/PDB.cpp +++ b/lld/COFF/PDB.cpp @@ -250,6 +250,72 @@ static void addTypeInfo(pdb::TpiStreamBuilder &tpiBuilder, }); } +// LF_BUILDINFO records might contain relative paths, and we want to make them +// absolute. We do this remapping only after the type records were merged, +// because the full types graph isn't known during merging. In addition, we plan +// to multi-thread the type merging, and the change below needs to be done +// atomically, single-threaded. + +// A complication could arise when a LF_STRING_ID record already exists with the +// same content as the new absolutized path. In that case, we simply redirect +// LF_BUILDINFO's CurrentDirectory index to reference the existing LF_STRING_ID +// record. + +static void remapBuildInfo(TypeCollection &idTable) { + SimpleTypeSerializer s; + idTable.ForEachRecord([&](TypeIndex ti, const CVType &type) { + if (type.kind() != LF_BUILDINFO) + return; + BuildInfoRecord bi; + cantFail(TypeDeserializer::deserializeAs(const_cast(type), bi)); + + auto makeAbsoluteRecord = + [&](BuildInfoRecord::BuildInfoArg recordType) -> Optional { + TypeIndex recordTi = bi.getArgs()[recordType]; + if (recordTi.isNoneType()) + return None; + CVType recordRef = idTable.getType(recordTi); + + StringIdRecord record; + cantFail(TypeDeserializer::deserializeAs(recordRef, record)); + + SmallString<128> abolutizedPath(record.getString()); + pdbMakeAbsolute(abolutizedPath); + + if (abolutizedPath == record.getString()) + return None; // The path is already absolute. + + record.String = abolutizedPath; + ArrayRef recordData = s.serialize(record); + + // Replace the previous LF_STRING_ID record + if (!idTable.replaceType(recordTi, CVType(recordData), + /*Stabilize=*/true)) + return recordTi; + return None; + }; + + Optional curDirTI = + makeAbsoluteRecord(BuildInfoRecord::CurrentDirectory); + Optional buildToolTI = + makeAbsoluteRecord(BuildInfoRecord::BuildTool); + + if (curDirTI || buildToolTI) { + // This new record is already there. We don't want duplicates, so + // re-serialize the BuildInfoRecord instead. + if (curDirTI) + bi.ArgIndices[BuildInfoRecord::CurrentDirectory] = *curDirTI; + if (buildToolTI) + bi.ArgIndices[BuildInfoRecord::BuildTool] = *buildToolTI; + + ArrayRef biData = s.serialize(bi); + bool r = idTable.replaceType(ti, CVType(biData), /*Stabilize=*/true); + assert(r && "Didn't expect two build records pointing to the same OBJ!"); + (void)r; + } + }); +} + static bool remapTypeIndex(TypeIndex &ti, ArrayRef typeIndexMap) { if (ti.isSimple()) return true; @@ -988,6 +1054,9 @@ void PDBLinker::addObjectsToPDB() { builder.getStringTableBuilder().setStrings(pdbStrTab); t1.stop(); + // Remap the contents of the LF_BUILDINFO record. + remapBuildInfo(tMerger.getIDTable()); + // Construct TPI and IPI stream contents. ScopedTimer t2(tpiStreamLayoutTimer); addTypeInfo(builder.getTpiBuilder(), tMerger.getTypeTable()); diff --git a/lld/test/COFF/Inputs/pdb_lines_1_relative.yaml b/lld/test/COFF/Inputs/pdb_lines_1_relative.yaml index 947de419d6b8b0..9a6b192e1d0d25 100644 --- a/lld/test/COFF/Inputs/pdb_lines_1_relative.yaml +++ b/lld/test/COFF/Inputs/pdb_lines_1_relative.yaml @@ -19,6 +19,7 @@ sections: Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ] Alignment: 4 SectionData: '' + SizeOfRawData: 0 - Name: .xdata Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] Alignment: 4 @@ -38,7 +39,6 @@ sections: - Name: '.debug$S' Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: 04000000F10000002F0000002D003C1100000000D0000700000000000000581B000000000000636C616E672076657273696F6E20372E302E30200000F1000000300000002A0047110000000000000000000000001B000000000000000000000002100000000000000000006D61696E0002004F11F20000003000000000000000000000001B00000000000000030000002400000000000000020000000C000000030000001100000004000000F400000030000000010000001001EA6429BCE282CCF3F0E3CD93B216EB410000110000001001061EB73ABB642532857A4F1D9CBAC3230000F30000001C000000002E5C7064625F6C696E65735F312E63002E5C666F6F2E6800000000 Subsections: - !Symbols Records: @@ -46,15 +46,15 @@ sections: Compile3Sym: Flags: [ ] Machine: X64 - FrontendMajor: 7 + FrontendMajor: 11 FrontendMinor: 0 FrontendBuild: 0 FrontendQFE: 0 - BackendMajor: 7000 + BackendMajor: 11000 BackendMinor: 0 BackendBuild: 0 BackendQFE: 0 - Version: 'clang version 7.0.0 ' + Version: 'clang version 11.0.0 (https://github.com/llvm/llvm-project.git 77dad72eae974338ddc13d74783c012ccbb8c5ac)' - !Symbols Records: - Kind: S_GPROC32_ID @@ -65,8 +65,17 @@ sections: FunctionType: 4098 Flags: [ ] DisplayName: main + - Kind: S_FRAMEPROC + FrameProcSym: + TotalFrameBytes: 40 + PaddingFrameBytes: 0 + OffsetToPadding: 0 + BytesOfCalleeSavedRegisters: 0 + OffsetOfExceptionHandler: 0 + SectionIdOfExceptionHandler: 0 + Flags: [ ] - Kind: S_PROC_ID_END - ScopeEndSym: + ScopeEndSym: {} - !Lines CodeSize: 27 Flags: [ ] @@ -87,15 +96,15 @@ sections: LineStart: 4 IsStatement: false EndDelta: 0 - Columns: + Columns: [] - !FileChecksums Checksums: - FileName: '.\pdb_lines_1.c' Kind: MD5 - Checksum: EA6429BCE282CCF3F0E3CD93B216EB41 + Checksum: 9A64DD4298487888B1D99F825D520C5E - FileName: '.\foo.h' Kind: MD5 - Checksum: 061EB73ABB642532857A4F1D9CBAC323 + Checksum: A9D05E6DC184DE20A57797E24F8B0E97 - !StringTable Strings: - '.\pdb_lines_1.c' @@ -103,23 +112,27 @@ sections: - '' - '' - '' + - !Symbols + Records: + - Kind: S_BUILDINFO + BuildInfoSym: + BuildId: 4105 Relocations: - - VirtualAddress: 100 + - VirtualAddress: 184 SymbolName: main Type: IMAGE_REL_AMD64_SECREL - - VirtualAddress: 104 + - VirtualAddress: 188 SymbolName: main Type: IMAGE_REL_AMD64_SECTION - - VirtualAddress: 124 + - VirtualAddress: 240 SymbolName: main Type: IMAGE_REL_AMD64_SECREL - - VirtualAddress: 128 + - VirtualAddress: 244 SymbolName: main Type: IMAGE_REL_AMD64_SECTION - Name: '.debug$T' Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: 0400000006000112000000000E0008107400000000000000001000001200011600000000011000006D61696E00F3F2F10E0008100300000000000000001000000E0001160000000003100000666F6F00 Types: - Kind: LF_ARGLIST ArgList: @@ -148,6 +161,25 @@ sections: ParentScope: 0 FunctionType: 4099 Name: foo + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: . + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: pdb_lines_1.c + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: 'buildninjaRel\bin\clang-cl.exe' + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: '"-cc1" "-triple" "x86_64-pc-windows-msvc19.26.28806" "-emit-obj" "-mrelax-all" "-mincremental-linker-compatible" "-disable-free" "-main-file-name" "pdb_lines_1.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model" "posix" "-mframe-pointer=none" "-relaxed-aliasing" "-fmath-errno" "-fno-rounding-math" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "x86-64" "-mllvm" "-x86-asm-syntax=intel" "-D_MT" "-flto-visibility-public-std" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-stack-protector" "2" "-fms-volatile" "-fdiagnostics-format" "msvc" "-gcodeview" "-debug-info-kind=limited" "-resource-dir" "D:\\llvm-project\\buildninjaRel\\lib\\clang\\11.0.0" "-internal-isystem" "D:\\llvm-project\\buildninjaRel\\lib\\clang\\11.0.0\\include" "-internal-isystem" "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\ATLMFC\\include" "-internal-isystem" "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\include" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\NETFXSDK\\4.8\\include\\um" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\um" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\cppwinrt" "-fdebug-compilation-dir" "." "-ferror-limit" "19" "-fmessage-length=146" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.26.28806" "-fdelayed-template-parsing" "-fcolor-diagnostics" "-faddrsig" "-x" "c"' + - Kind: LF_BUILDINFO + BuildInfo: + ArgIndices: [ 4101, 4103, 4102, 0, 4104 ] - Name: .pdata Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] Alignment: 4 @@ -160,8 +192,12 @@ sections: SymbolName: main Type: IMAGE_REL_AMD64_ADDR32NB - VirtualAddress: 8 - SymbolName: .xdata + SymbolTableIndex: 6 Type: IMAGE_REL_AMD64_ADDR32NB + - Name: .llvm_addrsig + Characteristics: [ IMAGE_SCN_LNK_REMOVE ] + Alignment: 1 + SectionData: 0A1D - Name: .xdata Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_READ ] Alignment: 4 @@ -169,7 +205,6 @@ sections: - Name: '.debug$S' Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: 04000000F10000002F000000290047110000000000000000000000000F00000000000000000000000410000000000000000000666F6F0002004F1100F20000003000000000000000000000000F000000180000000300000024000000000000000200000004000000030000000900000004000000 Subsections: - !Symbols Records: @@ -181,8 +216,17 @@ sections: FunctionType: 4100 Flags: [ ] DisplayName: foo + - Kind: S_FRAMEPROC + FrameProcSym: + TotalFrameBytes: 40 + PaddingFrameBytes: 0 + OffsetToPadding: 0 + BytesOfCalleeSavedRegisters: 0 + OffsetOfExceptionHandler: 0 + SectionIdOfExceptionHandler: 0 + Flags: [ ] - Kind: S_PROC_ID_END - ScopeEndSym: + ScopeEndSym: {} - !Lines CodeSize: 15 Flags: [ ] @@ -203,7 +247,7 @@ sections: LineStart: 4 IsStatement: false EndDelta: 0 - Columns: + Columns: [] Relocations: - VirtualAddress: 44 SymbolName: foo @@ -211,10 +255,10 @@ sections: - VirtualAddress: 48 SymbolName: foo Type: IMAGE_REL_AMD64_SECTION - - VirtualAddress: 68 + - VirtualAddress: 100 SymbolName: foo Type: IMAGE_REL_AMD64_SECREL - - VirtualAddress: 72 + - VirtualAddress: 104 SymbolName: foo Type: IMAGE_REL_AMD64_SECTION - Name: .pdata @@ -229,7 +273,7 @@ sections: SymbolName: foo Type: IMAGE_REL_AMD64_ADDR32NB - VirtualAddress: 8 - SymbolName: .xdata + SymbolTableIndex: 11 Type: IMAGE_REL_AMD64_ADDR32NB symbols: - Name: .text @@ -301,7 +345,7 @@ symbols: StorageClass: IMAGE_SYM_CLASS_EXTERNAL - Name: .xdata Value: 0 - SectionNumber: 10 + SectionNumber: 11 SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC @@ -331,22 +375,22 @@ symbols: ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 264 + Length: 396 NumberOfRelocations: 4 NumberOfLinenumbers: 0 - CheckSum: 2204933783 + CheckSum: 3390249978 Number: 7 - Name: '.debug$S' Value: 0 - SectionNumber: 11 + SectionNumber: 12 SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 116 + Length: 148 NumberOfRelocations: 4 NumberOfLinenumbers: 0 - CheckSum: 2691661839 + CheckSum: 1236081121 Number: 5 Selection: IMAGE_COMDAT_SELECT_ASSOCIATIVE - Name: '.debug$T' @@ -356,10 +400,10 @@ symbols: ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 80 + Length: 2028 NumberOfRelocations: 0 NumberOfLinenumbers: 0 - CheckSum: 3541780432 + CheckSum: 2043733667 Number: 8 - Name: .pdata Value: 0 @@ -375,7 +419,7 @@ symbols: Number: 9 - Name: .pdata Value: 0 - SectionNumber: 12 + SectionNumber: 13 SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC @@ -386,6 +430,24 @@ symbols: CheckSum: 3642757804 Number: 5 Selection: IMAGE_COMDAT_SELECT_ASSOCIATIVE + - Name: .llvm_addrsig + Value: 0 + SectionNumber: 10 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 2 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 2582217811 + Number: 10 + - Name: '@feat.00' + Value: 0 + SectionNumber: -1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC - Name: main Value: 0 SectionNumber: 1 @@ -398,4 +460,11 @@ symbols: SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: .file + Value: 0 + SectionNumber: -2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_FILE + File: pdb_lines_1.c ... diff --git a/lld/test/COFF/Inputs/pdb_lines_2_relative.yaml b/lld/test/COFF/Inputs/pdb_lines_2_relative.yaml index 1b051d82d9a46b..71ce5d63f50870 100644 --- a/lld/test/COFF/Inputs/pdb_lines_2_relative.yaml +++ b/lld/test/COFF/Inputs/pdb_lines_2_relative.yaml @@ -15,6 +15,7 @@ sections: Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ] Alignment: 4 SectionData: '' + SizeOfRawData: 0 - Name: .drectve Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ] Alignment: 1 @@ -22,7 +23,6 @@ sections: - Name: '.debug$S' Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: 04000000F10000002F0000002D003C1100000000D0000700000000000000581B000000000000636C616E672076657273696F6E20372E302E30200000F10000002F0000002900471100000000000000000000000001000000000000000000000002100000000000000000006261720002004F1100F2000000200000000000000000000000010000000000000001000000140000000000000002000000F400000018000000010000001001DF91CB3A2B8D917486574BB50CAC4CC70000F300000014000000002E5C7064625F6C696E65735F322E6300000000 Subsections: - !Symbols Records: @@ -30,15 +30,15 @@ sections: Compile3Sym: Flags: [ ] Machine: X64 - FrontendMajor: 7 + FrontendMajor: 11 FrontendMinor: 0 FrontendBuild: 0 FrontendQFE: 0 - BackendMajor: 7000 + BackendMajor: 11000 BackendMinor: 0 BackendBuild: 0 BackendQFE: 0 - Version: 'clang version 7.0.0 ' + Version: 'clang version 11.0.0 (https://github.com/llvm/llvm-project.git 77dad72eae974338ddc13d74783c012ccbb8c5ac)' - !Symbols Records: - Kind: S_GPROC32_ID @@ -49,8 +49,17 @@ sections: FunctionType: 4098 Flags: [ ] DisplayName: bar + - Kind: S_FRAMEPROC + FrameProcSym: + TotalFrameBytes: 0 + PaddingFrameBytes: 0 + OffsetToPadding: 0 + BytesOfCalleeSavedRegisters: 0 + OffsetOfExceptionHandler: 0 + SectionIdOfExceptionHandler: 0 + Flags: [ ] - Kind: S_PROC_ID_END - ScopeEndSym: + ScopeEndSym: {} - !Lines CodeSize: 1 Flags: [ ] @@ -63,35 +72,39 @@ sections: LineStart: 2 IsStatement: false EndDelta: 0 - Columns: + Columns: [] - !FileChecksums Checksums: - FileName: '.\pdb_lines_2.c' Kind: MD5 - Checksum: DF91CB3A2B8D917486574BB50CAC4CC7 + Checksum: 4CC58B73BFD5AB52F87CFB3C604BB288 - !StringTable Strings: - '.\pdb_lines_2.c' - '' - '' - '' + - !Symbols + Records: + - Kind: S_BUILDINFO + BuildInfoSym: + BuildId: 4103 Relocations: - - VirtualAddress: 100 + - VirtualAddress: 184 SymbolName: bar Type: IMAGE_REL_AMD64_SECREL - - VirtualAddress: 104 + - VirtualAddress: 188 SymbolName: bar Type: IMAGE_REL_AMD64_SECTION - - VirtualAddress: 124 + - VirtualAddress: 240 SymbolName: bar Type: IMAGE_REL_AMD64_SECREL - - VirtualAddress: 128 + - VirtualAddress: 244 SymbolName: bar Type: IMAGE_REL_AMD64_SECTION - Name: '.debug$T' Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: 0400000006000112000000000E0008100300000000000000001000000E000116000000000110000062617200 Types: - Kind: LF_ARGLIST ArgList: @@ -108,6 +121,29 @@ sections: ParentScope: 0 FunctionType: 4097 Name: bar + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: . + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: pdb_lines_2.c + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: 'buildninjaRel\bin\clang-cl.exe' + - Kind: LF_STRING_ID + StringId: + Id: 0 + String: '"-cc1" "-triple" "x86_64-pc-windows-msvc19.26.28806" "-emit-obj" "-mrelax-all" "-mincremental-linker-compatible" "-disable-free" "-main-file-name" "pdb_lines_2.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model" "posix" "-mframe-pointer=none" "-relaxed-aliasing" "-fmath-errno" "-fno-rounding-math" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "x86-64" "-mllvm" "-x86-asm-syntax=intel" "-D_MT" "-flto-visibility-public-std" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-stack-protector" "2" "-fms-volatile" "-fdiagnostics-format" "msvc" "-gcodeview" "-debug-info-kind=limited" "-resource-dir" "D:\\llvm-project\\buildninjaRel\\lib\\clang\\11.0.0" "-internal-isystem" "D:\\llvm-project\\buildninjaRel\\lib\\clang\\11.0.0\\include" "-internal-isystem" "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\ATLMFC\\include" "-internal-isystem" "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\include" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\NETFXSDK\\4.8\\include\\um" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\um" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" "-internal-isystem" "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\cppwinrt" "-fdebug-compilation-dir" "." "-ferror-limit" "19" "-fmessage-length=146" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.26.28806" "-fdelayed-template-parsing" "-fcolor-diagnostics" "-faddrsig" "-x" "c"' + - Kind: LF_BUILDINFO + BuildInfo: + ArgIndices: [ 4099, 4101, 4100, 0, 4102 ] + - Name: .llvm_addrsig + Characteristics: [ IMAGE_SCN_LNK_REMOVE ] + Alignment: 1 + SectionData: '' symbols: - Name: .text Value: 0 @@ -164,10 +200,10 @@ symbols: ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 216 + Length: 348 NumberOfRelocations: 4 NumberOfLinenumbers: 0 - CheckSum: 2383431754 + CheckSum: 2408981505 Number: 5 - Name: '.debug$T' Value: 0 @@ -176,15 +212,40 @@ symbols: ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 44 + Length: 1992 NumberOfRelocations: 0 NumberOfLinenumbers: 0 - CheckSum: 179171995 + CheckSum: 1158086003 Number: 6 + - Name: .llvm_addrsig + Value: 0 + SectionNumber: 7 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 0 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 7 + - Name: '@feat.00' + Value: 0 + SectionNumber: -1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC - Name: bar Value: 0 SectionNumber: 1 SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_FUNCTION StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: .file + Value: 0 + SectionNumber: -2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_FILE + File: pdb_lines_2.c ... diff --git a/lld/test/COFF/pdb-relative-source-lines.test b/lld/test/COFF/pdb-relative-source-lines.test index 547056785962ad..632aa48cb6cd42 100644 --- a/lld/test/COFF/pdb-relative-source-lines.test +++ b/lld/test/COFF/pdb-relative-source-lines.test @@ -15,7 +15,9 @@ int main(void) { void bar(void) { } -$ clang-cl -Xclang -fdebug-compilation-dir -Xclang . -c -Z7 pdb_lines*.c +$ clang-cl -fdebug-compilation-dir . -no-canonical-prefixes -c -Z7 pdb_lines*.c +$ obj2yaml pdb_lines_1.obj > pdb_lines_1_relative.yaml +$ obj2yaml pdb_lines_2.obj > pdb_lines_2_relative.yaml /pdbsourcepath: only sets the directory that relative paths are considered relative to, so this test needs to pass relative paths to lld-link for: @@ -33,9 +35,9 @@ RUN: cd %t RUN: yaml2obj %S/Inputs/pdb_lines_1_relative.yaml -o %t/pdb_lines_1_relative.obj RUN: yaml2obj %S/Inputs/pdb_lines_2_relative.yaml -o %t/pdb_lines_2_relative.obj RUN: ./lld-link -debug "-pdbsourcepath:c:\src" -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj -RUN: llvm-pdbutil pdb2yaml -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck %s +RUN: llvm-pdbutil pdb2yaml -ipi-stream -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck %s RUN: ./lld-link -debug "-pdbsourcepath:/usr/src" -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj -RUN: llvm-pdbutil pdb2yaml -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck --check-prefix=POSIX %s +RUN: llvm-pdbutil pdb2yaml -ipi-stream -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck --check-prefix=POSIX %s CHECK-LABEL: - Module: 'c:\src\pdb_lines_1_relative.obj' CHECK-NEXT: ObjFile: 'c:\src\pdb_lines_1_relative.obj' @@ -70,6 +72,20 @@ CHECK-NEXT: - 'c:\src\out.pdb' CHECK-NEXT: - cmd CHECK-NEXT: - '-debug -pdbsourcepath:c:\src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj' +CHECK-LABEL: IpiStream: + +CHECK: - Kind: LF_STRING_ID +CHECK-NEXT: StringId: +CHECK-NEXT: Id: 0 +CHECK-NEXT: String: 'c:\src' +CHECK-NEXT: - Kind: LF_STRING_ID +CHECK-NEXT: StringId: +CHECK-NEXT: Id: 0 +CHECK-NEXT: String: pdb_lines_1.c +CHECK-NEXT: - Kind: LF_STRING_ID +CHECK-NEXT: StringId: +CHECK-NEXT: Id: 0 +CHECK-NEXT: String: 'c:\src\buildninjaRel\bin\clang-cl.exe' POSIX-LABEL: - Module: '/usr/src/pdb_lines_1_relative.obj' POSIX-NEXT: ObjFile: '/usr/src/pdb_lines_1_relative.obj' @@ -103,3 +119,17 @@ POSIX-NEXT: - pdb POSIX-NEXT: - '/usr/src/out.pdb' POSIX-NEXT: - cmd POSIX-NEXT: - '-debug -pdbsourcepath:/usr/src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj' + +POSIX-LABEL: IpiStream: +POSIX: - Kind: LF_STRING_ID +POSIX-NEXT: StringId: +POSIX-NEXT: Id: 0 +POSIX-NEXT: String: '/usr/src' +POSIX-NEXT: - Kind: LF_STRING_ID +POSIX-NEXT: StringId: +POSIX-NEXT: Id: 0 +POSIX-NEXT: String: pdb_lines_1.c +POSIX-NEXT: - Kind: LF_STRING_ID +POSIX-NEXT: StringId: +POSIX-NEXT: Id: 0 +POSIX-NEXT: String: '/usr/src/buildninjaRel/bin/clang-cl.exe' diff --git a/lld/test/COFF/pdb-relative-source-lines2.test b/lld/test/COFF/pdb-relative-source-lines2.test new file mode 100644 index 00000000000000..955f7bc1e453a1 --- /dev/null +++ b/lld/test/COFF/pdb-relative-source-lines2.test @@ -0,0 +1,66 @@ +REQUIRES: system-windows + +Test the linker line tables on roughly the following example: + +==> foo.h <== +void bar(void); +inline void foo(void) { + bar(); +} +==> pdb_lines_1.c <== +#include "foo.h" +int main(void) { + foo(); + return 42; +} +==> pdb_lines_2.c <== +void bar(void) { +} + +$ clang-cl -fdebug-compilation-dir . -no-canonical-prefixes -c -Z7 pdb_lines*.c +$ obj2yaml pdb_lines_1.obj > pdb_lines_1_relative.yaml +$ obj2yaml pdb_lines_2.obj > pdb_lines_2_relative.yaml + +/pdbsourcepath: only sets the directory that relative paths are considered +relative to, so this test needs to pass relative paths to lld-link for: +1. The input obj files +2. The /pdb: switch +3. The lld-link invocation itself +To achieve this, put all inputs of the lld-link invocation (including lld-link +itself) in a temp directory that's cwd and then make sure to only use relative +arguments when calling ./lld-link below. +RUN: rm -rf %t +RUN: mkdir %t +RUN: cp lld-link %t/lld-link +RUN: cd %t + +Test the convoluted case at the end of remapBuildInfo() in lld/COFF/PDB.cpp +The only drawback right now is that this edge case will create LF_BUILDINFO +records with front references in the IPI stream. However the Visual Studio +debugger takes the .PDB thusly created without any problems. +Tested on VS2015, 2017 and 2019. + +RUN: yaml2obj %S/Inputs/pdb_lines_1_relative.yaml -o %t/pdb_lines_1_relative.obj +RUN: sed -e "s|String: \.|String: "c:\\\src"|" < %S/Inputs/pdb_lines_2_relative.yaml > %t/pdb_lines_2_relative.yaml +RUN: yaml2obj pdb_lines_2_relative.yaml -o %t/pdb_lines_2_relative.obj +RUN: ./lld-link -debug "-pdbsourcepath:c:\src" -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj +RUN: llvm-pdbutil pdb2yaml -ipi-stream -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck --check-prefix=EXISTING %s + +EXISTING-LABEL: IpiStream: + +EXISTING: - Kind: LF_STRING_ID +EXISTING-NEXT: StringId: +EXISTING-NEXT: Id: 0 +EXISTING-NEXT: String: . +EXISTING-NEXT: - Kind: LF_STRING_ID +EXISTING-NEXT: StringId: +EXISTING-NEXT: Id: 0 +EXISTING-NEXT: String: pdb_lines_1.c +EXISTING: - Kind: LF_STRING_ID +EXISTING-NEXT: StringId: +EXISTING-NEXT: Id: 0 +EXISTING-LABEL: String: 'c:\src' +EXISTING-NEXT: - Kind: LF_STRING_ID +EXISTING-NEXT: StringId: +EXISTING-NEXT: Id: 0 +EXISTING-NEXT: String: pdb_lines_2.c diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp index f7041c0cc92631..cf3c38c57f6d7b 100644 --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp @@ -77,6 +77,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" +#include "llvm/Support/Program.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Target/TargetLoweringObjectFile.h" @@ -831,6 +832,31 @@ static TypeIndex getStringIdTypeIdx(GlobalTypeTableBuilder &TypeTable, return TypeTable.writeLeafType(SIR); } +static std::string flattenCommandLine(ArrayRef Args, + StringRef MainFilename) { + std::string FlatCmdLine; + raw_string_ostream OS(FlatCmdLine); + StringRef LastArg; + for (StringRef Arg : Args) { + if (Arg.empty()) + continue; + // The command-line shall not contain the file to compile. + if (Arg == MainFilename && LastArg != "-main-file-name") + continue; + // Also remove the output file. + if (Arg == "-o" || LastArg == "-o") { + LastArg = Arg; + continue; + } + if (!LastArg.empty()) + OS << " "; + llvm::sys::printArg(OS, Arg, /*Quote=*/true); + LastArg = Arg; + } + OS.flush(); + return FlatCmdLine; +} + void CodeViewDebug::emitBuildInfo() { // First, make LF_BUILDINFO. It's a sequence of strings with various bits of // build info. The known prefix is: @@ -851,8 +877,16 @@ void CodeViewDebug::emitBuildInfo() { getStringIdTypeIdx(TypeTable, MainSourceFile->getDirectory()); BuildInfoArgs[BuildInfoRecord::SourceFile] = getStringIdTypeIdx(TypeTable, MainSourceFile->getFilename()); - // FIXME: Path to compiler and command line. PDB is intentionally blank unless - // we implement /Zi type servers. + // FIXME: PDB is intentionally blank unless we implement /Zi type servers. + BuildInfoArgs[BuildInfoRecord::TypeServerPDB] = + getStringIdTypeIdx(TypeTable, ""); + if (Asm->TM.Options.MCOptions.Argv0 != nullptr) { + BuildInfoArgs[BuildInfoRecord::BuildTool] = + getStringIdTypeIdx(TypeTable, Asm->TM.Options.MCOptions.Argv0); + BuildInfoArgs[BuildInfoRecord::CommandLine] = getStringIdTypeIdx( + TypeTable, flattenCommandLine(Asm->TM.Options.MCOptions.CommandLineArgs, + MainSourceFile->getFilename())); + } BuildInfoRecord BIR(BuildInfoArgs); TypeIndex BuildInfoIndex = TypeTable.writeLeafType(BIR); diff --git a/llvm/test/DebugInfo/COFF/build-info.ll b/llvm/test/DebugInfo/COFF/build-info.ll index 94f006c3b09356..983aa22214bce9 100644 --- a/llvm/test/DebugInfo/COFF/build-info.ll +++ b/llvm/test/DebugInfo/COFF/build-info.ll @@ -5,7 +5,7 @@ ; CHECK-NEXT: 0x{{.*}}: `D:\src\scopes\clang` ; CHECK-NEXT: : `` ; CHECK-NEXT: 0x{{.*}}: `D:\src\scopes\foo.cpp` -; CHECK-NEXT: : `` +; CHECK-NEXT: 0x{{.*}}: `` ; CHECK-NEXT: : `` ; CHECK: {{.*}} | S_BUILDINFO [size = 8] BuildId = `[[INFO_IDX]]` diff --git a/llvm/test/DebugInfo/COFF/global-type-hashes.ll b/llvm/test/DebugInfo/COFF/global-type-hashes.ll index 70f9df156a5beb..3c6c27301b20aa 100644 --- a/llvm/test/DebugInfo/COFF/global-type-hashes.ll +++ b/llvm/test/DebugInfo/COFF/global-type-hashes.ll @@ -295,7 +295,8 @@ attributes #2 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-ma ; YAML: - 4470750F2E319329 ; YAML: - 0FB556FD1FAB66D7 ; YAML: - 5970EFB4874D0F3F -; YAML: - EDB1D74C120CF44A +; YAML: - D8EF11198C33843F +; YAML: - D81F744D7366282B ; ... diff --git a/llvm/test/DebugInfo/COFF/types-basic.ll b/llvm/test/DebugInfo/COFF/types-basic.ll index 81e0c25d17cdbb..6455452d125ae9 100644 --- a/llvm/test/DebugInfo/COFF/types-basic.ll +++ b/llvm/test/DebugInfo/COFF/types-basic.ll @@ -511,14 +511,22 @@ ; ASM: .asciz "t.cpp" # StringData ; ASM: .byte 242 ; ASM: .byte 241 -; ASM: # BuildInfo (0x1015) +; ASM: # StringId (0x1015) +; ASM: .short 0xa # Record length +; ASM: .short 0x1605 # Record kind: LF_STRING_ID +; ASM: .long 0x0 # Id +; ASM: .byte 0 # StringData +; ASM: .byte 243 +; ASM: .byte 242 +; ASM: .byte 241 +; ASM: # BuildInfo (0x1016) ; ASM: .short 0x1a # Record length ; ASM: .short 0x1603 # Record kind: LF_BUILDINFO ; ASM: .short 0x5 # NumArgs ; ASM: .long 0x1013 # Argument: D:\src\llvm\build ; ASM: .long 0x0 # Argument ; ASM: .long 0x1014 # Argument: t.cpp -; ASM: .long 0x0 # Argument +; ASM: .long 0x1015 # Argument ; ASM: .long 0x0 # Argument ; ASM: .byte 242 ; ASM: .byte 241 diff --git a/llvm/test/DebugInfo/COFF/types-data-members.ll b/llvm/test/DebugInfo/COFF/types-data-members.ll index 87fde74b989c43..1e699efdf8ed38 100644 --- a/llvm/test/DebugInfo/COFF/types-data-members.ll +++ b/llvm/test/DebugInfo/COFF/types-data-members.ll @@ -727,14 +727,22 @@ ; ASM: .asciz "t.cpp" # StringData ; ASM: .byte 242 ; ASM: .byte 241 -; ASM: # BuildInfo (0x1022) +; ASM: # StringId (0x1022) +; ASM: .short 0xa # Record length +; ASM: .short 0x1605 # Record kind: LF_STRING_ID +; ASM: .long 0x0 # Id +; ASM: .byte 0 # StringData +; ASM: .byte 243 +; ASM: .byte 242 +; ASM: .byte 241 +; ASM: # BuildInfo (0x1023) ; ASM: .short 0x1a # Record length ; ASM: .short 0x1603 # Record kind: LF_BUILDINFO ; ASM: .short 0x5 # NumArgs ; ASM: .long 0x1020 # Argument: D:\src\llvm\build ; ASM: .long 0x0 # Argument ; ASM: .long 0x1021 # Argument: t.cpp -; ASM: .long 0x0 # Argument +; ASM: .long 0x1022 # Argument ; ASM: .long 0x0 # Argument ; ASM: .byte 242 ; ASM: .byte 241 From 8c8a2fd1f015525d048444610a6e27c66aa96293 Mon Sep 17 00:00:00 2001 From: Anastasia Stulova Date: Fri, 10 Jul 2020 19:04:49 +0100 Subject: [PATCH 02/14] [OpenCL] Fixed typo for ctor stub name in UsersManual --- clang/docs/UsersManual.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index 4b4e28a8b65c4c..8615a77596b4f3 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -3132,7 +3132,7 @@ Global objects must be constructed before the first kernel using the global obje is executed and destroyed just after the last kernel using the program objects is executed. In OpenCL v2.0 drivers there is no specific API for invoking global constructors. However, an easy workaround would be to enqueue a constructor -initialization kernel that has a name ``@_GLOBAL__sub_I_``. +initialization kernel that has a name ``_GLOBAL__sub_I_``. This kernel is only present if there are any global objects to be initialized in the compiled binary. One way to check this is by passing ``CL_PROGRAM_KERNEL_NAMES`` to ``clGetProgramInfo`` (OpenCL v2.0 s5.8.7). @@ -3148,7 +3148,7 @@ before running any kernels in which the objects are used. clang -cl-std=clc++ test.cl If there are any global objects to be initialized, the final binary will contain -the ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued. +the ``_GLOBAL__sub_I_test.cl`` kernel to be enqueued. Global destructors can not be invoked in OpenCL v2.0 drivers. However, all memory used for program scope objects is released on ``clReleaseProgram``. From e337350be9d6efd72027c331f95ef33df61fdc43 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 10 Jul 2020 11:10:56 -0700 Subject: [PATCH 03/14] This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The intent of that change was to do the same work for the computation of the locations of the children of ValueObjectVariable as was done for the root ValueObjectVariable. This original patch did that by moving the computation from ValueObjectVariable to ValueObject. That fixed the problem but caused a handful of swift-lldb testsuite failures and a crash or two. The problem is that synthetic value objects can sometimes represent objects in target memory, and other times they might be made up wholly in lldb memory, with pointers from one synthetic object to another, and so the ValueObjectVariable computation was not appropriate. This patch delegates the computation to the root of the ValueObject in question. That solves the problem for ValueObjectVariable while not messing up the computation for ValueObjectConstResult or ValueObjectSynthetic. Differential Revision: https://reviews.llvm.org/D83450 --- lldb/include/lldb/Core/ValueObject.h | 7 ++- lldb/include/lldb/Core/ValueObjectVariable.h | 2 + lldb/source/Core/ValueObject.cpp | 52 ------------------ lldb/source/Core/ValueObjectVariable.cpp | 55 ++++++++++++++++++++ 4 files changed, 63 insertions(+), 53 deletions(-) diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h index 7c8a01beea56d6..0080368fd99657 100644 --- a/lldb/include/lldb/Core/ValueObject.h +++ b/lldb/include/lldb/Core/ValueObject.h @@ -963,9 +963,14 @@ class ValueObject : public UserID { void SetPreferredDisplayLanguageIfNeeded(lldb::LanguageType); +protected: + virtual void DoUpdateChildrenAddressType(ValueObject &valobj) { return; }; + private: virtual CompilerType MaybeCalculateCompleteType(); - void UpdateChildrenAddressType(); + void UpdateChildrenAddressType() { + GetRoot()->DoUpdateChildrenAddressType(*this); + } lldb::ValueObjectSP GetValueForExpressionPath_Impl( llvm::StringRef expression_cstr, diff --git a/lldb/include/lldb/Core/ValueObjectVariable.h b/lldb/include/lldb/Core/ValueObjectVariable.h index a0417e5e7d85ab..b7e262574a14d8 100644 --- a/lldb/include/lldb/Core/ValueObjectVariable.h +++ b/lldb/include/lldb/Core/ValueObjectVariable.h @@ -67,6 +67,8 @@ class ValueObjectVariable : public ValueObject { protected: bool UpdateValue() override; + + void DoUpdateChildrenAddressType(ValueObject &valobj) override; CompilerType GetCompilerTypeImpl() override; diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index f13ad63dfb61ad..3a775b07e5e1f2 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -140,58 +140,6 @@ ValueObject::ValueObject(ExecutionContextScope *exe_scope, // Destructor ValueObject::~ValueObject() {} -void ValueObject::UpdateChildrenAddressType() { - Value::ValueType value_type = m_value.GetValueType(); - ExecutionContext exe_ctx(GetExecutionContextRef()); - Process *process = exe_ctx.GetProcessPtr(); - const bool process_is_alive = process && process->IsAlive(); - const uint32_t type_info = GetCompilerType().GetTypeInfo(); - const bool is_pointer_or_ref = - (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0; - - switch (value_type) { - case Value::eValueTypeFileAddress: - // If this type is a pointer, then its children will be considered load - // addresses if the pointer or reference is dereferenced, but only if - // the process is alive. - // - // There could be global variables like in the following code: - // struct LinkedListNode { Foo* foo; LinkedListNode* next; }; - // Foo g_foo1; - // Foo g_foo2; - // LinkedListNode g_second_node = { &g_foo2, NULL }; - // LinkedListNode g_first_node = { &g_foo1, &g_second_node }; - // - // When we aren't running, we should be able to look at these variables - // using the "target variable" command. Children of the "g_first_node" - // always will be of the same address type as the parent. But children - // of the "next" member of LinkedListNode will become load addresses if - // we have a live process, or remain a file address if it was a file - // address. - if (process_is_alive && is_pointer_or_ref) - SetAddressTypeOfChildren(eAddressTypeLoad); - else - SetAddressTypeOfChildren(eAddressTypeFile); - break; - case Value::eValueTypeHostAddress: - // Same as above for load addresses, except children of pointer or refs - // are always load addresses. Host addresses are used to store freeze - // dried variables. If this type is a struct, the entire struct - // contents will be copied into the heap of the - // LLDB process, but we do not currently follow any pointers. - if (is_pointer_or_ref) - SetAddressTypeOfChildren(eAddressTypeLoad); - else - SetAddressTypeOfChildren(eAddressTypeHost); - break; - case Value::eValueTypeLoadAddress: - case Value::eValueTypeScalar: - case Value::eValueTypeVector: - SetAddressTypeOfChildren(eAddressTypeLoad); - break; - } -} - bool ValueObject::UpdateValueIfNeeded(bool update_format) { bool did_change_formats = false; diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp index 8199c6101395f9..0d1e7b047a0ac1 100644 --- a/lldb/source/Core/ValueObjectVariable.cpp +++ b/lldb/source/Core/ValueObjectVariable.cpp @@ -242,9 +242,64 @@ bool ValueObjectVariable::UpdateValue() { m_resolved_value.SetContext(Value::eContextTypeInvalid, nullptr); } } + return m_error.Success(); } +void ValueObjectVariable::DoUpdateChildrenAddressType(ValueObject &valobj) { + Value::ValueType value_type = valobj.GetValue().GetValueType(); + ExecutionContext exe_ctx(GetExecutionContextRef()); + Process *process = exe_ctx.GetProcessPtr(); + const bool process_is_alive = process && process->IsAlive(); + const uint32_t type_info = valobj.GetCompilerType().GetTypeInfo(); + const bool is_pointer_or_ref = + (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0; + + switch (value_type) { + case Value::eValueTypeFileAddress: + // If this type is a pointer, then its children will be considered load + // addresses if the pointer or reference is dereferenced, but only if + // the process is alive. + // + // There could be global variables like in the following code: + // struct LinkedListNode { Foo* foo; LinkedListNode* next; }; + // Foo g_foo1; + // Foo g_foo2; + // LinkedListNode g_second_node = { &g_foo2, NULL }; + // LinkedListNode g_first_node = { &g_foo1, &g_second_node }; + // + // When we aren't running, we should be able to look at these variables + // using the "target variable" command. Children of the "g_first_node" + // always will be of the same address type as the parent. But children + // of the "next" member of LinkedListNode will become load addresses if + // we have a live process, or remain a file address if it was a file + // address. + if (process_is_alive && is_pointer_or_ref) + valobj.SetAddressTypeOfChildren(eAddressTypeLoad); + else + valobj.SetAddressTypeOfChildren(eAddressTypeFile); + break; + case Value::eValueTypeHostAddress: + // Same as above for load addresses, except children of pointer or refs + // are always load addresses. Host addresses are used to store freeze + // dried variables. If this type is a struct, the entire struct + // contents will be copied into the heap of the + // LLDB process, but we do not currently follow any pointers. + if (is_pointer_or_ref) + valobj.SetAddressTypeOfChildren(eAddressTypeLoad); + else + valobj.SetAddressTypeOfChildren(eAddressTypeHost); + break; + case Value::eValueTypeLoadAddress: + case Value::eValueTypeScalar: + case Value::eValueTypeVector: + valobj.SetAddressTypeOfChildren(eAddressTypeLoad); + break; + } +} + + + bool ValueObjectVariable::IsInScope() { const ExecutionContextRef &exe_ctx_ref = GetExecutionContextRef(); if (exe_ctx_ref.HasFrameRef()) { From fdb7856d54a1f81bab0ac0c8a4e984620589e699 Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Fri, 10 Jul 2020 11:16:33 -0700 Subject: [PATCH 04/14] Revert "[NFC] Derive from PassInfoMixin for no-op/printing passes" This reverts commit 8039d2c3bf14585ef37dc9343bf393ecad9aead9 as it breaks the modules build on macOS. --- llvm/include/llvm/IR/IRPrintingPasses.h | 17 +- llvm/lib/IR/LegacyPassManager.cpp | 202 +++++++++++------------- llvm/lib/Passes/PassBuilder.cpp | 15 +- 3 files changed, 115 insertions(+), 119 deletions(-) diff --git a/llvm/include/llvm/IR/IRPrintingPasses.h b/llvm/include/llvm/IR/IRPrintingPasses.h index 3a1c489ee09f24..230db988f737be 100644 --- a/llvm/include/llvm/IR/IRPrintingPasses.h +++ b/llvm/include/llvm/IR/IRPrintingPasses.h @@ -19,10 +19,17 @@ #define LLVM_IR_IRPRINTINGPASSES_H #include "llvm/ADT/StringRef.h" -#include "llvm/IR/PassManager.h" #include namespace llvm { +class Pass; +class Function; +class FunctionPass; +class Module; +class ModulePass; +class PreservedAnalyses; +class raw_ostream; +template class AnalysisManager; /// Create and return a pass that writes the module to the specified /// \c raw_ostream. @@ -64,7 +71,7 @@ extern bool shouldPrintAfterPass(StringRef); /// /// Note: This pass is for use with the new pass manager. Use the create...Pass /// functions above to create passes for use with the legacy pass manager. -class PrintModulePass : public PassInfoMixin { +class PrintModulePass { raw_ostream &OS; std::string Banner; bool ShouldPreserveUseListOrder; @@ -75,13 +82,15 @@ class PrintModulePass : public PassInfoMixin { bool ShouldPreserveUseListOrder = false); PreservedAnalyses run(Module &M, AnalysisManager &); + + static StringRef name() { return "PrintModulePass"; } }; /// Pass for printing a Function as LLVM's text IR assembly. /// /// Note: This pass is for use with the new pass manager. Use the create...Pass /// functions above to create passes for use with the legacy pass manager. -class PrintFunctionPass : public PassInfoMixin { +class PrintFunctionPass { raw_ostream &OS; std::string Banner; @@ -90,6 +99,8 @@ class PrintFunctionPass : public PassInfoMixin { PrintFunctionPass(raw_ostream &OS, const std::string &Banner = ""); PreservedAnalyses run(Function &F, AnalysisManager &); + + static StringRef name() { return "PrintFunctionPass"; } }; } // End llvm namespace diff --git a/llvm/lib/IR/LegacyPassManager.cpp b/llvm/lib/IR/LegacyPassManager.cpp index 4189aea46294c2..1d9c44f385fb42 100644 --- a/llvm/lib/IR/LegacyPassManager.cpp +++ b/llvm/lib/IR/LegacyPassManager.cpp @@ -33,6 +33,7 @@ #include #include using namespace llvm; +using namespace llvm::legacy; // See PassManagers.h for Pass Manager infrastructure overview. @@ -386,66 +387,6 @@ class FunctionPassManagerImpl : public Pass, void FunctionPassManagerImpl::anchor() {} char FunctionPassManagerImpl::ID = 0; - -//===----------------------------------------------------------------------===// -// FunctionPassManagerImpl implementation -// -bool FunctionPassManagerImpl::doInitialization(Module &M) { - bool Changed = false; - - dumpArguments(); - dumpPasses(); - - for (ImmutablePass *ImPass : getImmutablePasses()) - Changed |= ImPass->doInitialization(M); - - for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) - Changed |= getContainedManager(Index)->doInitialization(M); - - return Changed; -} - -bool FunctionPassManagerImpl::doFinalization(Module &M) { - bool Changed = false; - - for (int Index = getNumContainedManagers() - 1; Index >= 0; --Index) - Changed |= getContainedManager(Index)->doFinalization(M); - - for (ImmutablePass *ImPass : getImmutablePasses()) - Changed |= ImPass->doFinalization(M); - - return Changed; -} - -void FunctionPassManagerImpl::releaseMemoryOnTheFly() { - if (!wasRun) - return; - for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { - FPPassManager *FPPM = getContainedManager(Index); - for (unsigned Index = 0; Index < FPPM->getNumContainedPasses(); ++Index) { - FPPM->getContainedPass(Index)->releaseMemory(); - } - } - wasRun = false; -} - -// Execute all the passes managed by this top level manager. -// Return true if any function is modified by a pass. -bool FunctionPassManagerImpl::run(Function &F) { - bool Changed = false; - - initializeAllAnalysisInfo(); - for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { - Changed |= getContainedManager(Index)->runOnFunction(F); - F.getContext().yield(); - } - - for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) - getContainedManager(Index)->cleanup(); - - wasRun = true; - return Changed; -} } // namespace legacy } // namespace llvm @@ -465,7 +406,7 @@ class MPPassManager : public Pass, public PMDataManager { // Delete on the fly managers. ~MPPassManager() override { for (auto &OnTheFlyManager : OnTheFlyManagers) { - legacy::FunctionPassManagerImpl *FPP = OnTheFlyManager.second; + FunctionPassManagerImpl *FPP = OnTheFlyManager.second; delete FPP; } } @@ -510,7 +451,7 @@ class MPPassManager : public Pass, public PMDataManager { for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) { ModulePass *MP = getContainedPass(Index); MP->dumpPassStructure(Offset + 1); - MapVector::const_iterator I = + MapVector::const_iterator I = OnTheFlyManagers.find(MP); if (I != OnTheFlyManagers.end()) I->second->dumpPassStructure(Offset + 2); @@ -530,7 +471,7 @@ class MPPassManager : public Pass, public PMDataManager { private: /// Collection of on the fly FPPassManagers. These managers manage /// function passes that are required by module passes. - MapVector OnTheFlyManagers; + MapVector OnTheFlyManagers; }; char MPPassManager::ID = 0; @@ -593,33 +534,6 @@ class PassManagerImpl : public Pass, void PassManagerImpl::anchor() {} char PassManagerImpl::ID = 0; - -//===----------------------------------------------------------------------===// -// PassManagerImpl implementation - -// -/// run - Execute all of the passes scheduled for execution. Keep track of -/// whether any of the passes modifies the module, and if so, return true. -bool PassManagerImpl::run(Module &M) { - bool Changed = false; - - dumpArguments(); - dumpPasses(); - - for (ImmutablePass *ImPass : getImmutablePasses()) - Changed |= ImPass->doInitialization(M); - - initializeAllAnalysisInfo(); - for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { - Changed |= getContainedManager(Index)->runOnModule(M); - M.getContext().yield(); - } - - for (ImmutablePass *ImPass : getImmutablePasses()) - Changed |= ImPass->doFinalization(M); - - return Changed; -} } // namespace legacy } // namespace llvm @@ -1400,15 +1314,12 @@ AnalysisResolver::findImplPass(Pass *P, AnalysisID AnalysisPI, Function &F) { return PM.getOnTheFlyPass(P, AnalysisPI, F); } -namespace llvm { -namespace legacy { - //===----------------------------------------------------------------------===// // FunctionPassManager implementation /// Create new Function pass manager FunctionPassManager::FunctionPassManager(Module *m) : M(m) { - FPM = new legacy::FunctionPassManagerImpl(); + FPM = new FunctionPassManagerImpl(); // FPM is the top level manager. FPM->setTopLevelManager(FPM); @@ -1447,8 +1358,36 @@ bool FunctionPassManager::doInitialization() { bool FunctionPassManager::doFinalization() { return FPM->doFinalization(*M); } -} // namespace legacy -} // namespace llvm + +//===----------------------------------------------------------------------===// +// FunctionPassManagerImpl implementation +// +bool FunctionPassManagerImpl::doInitialization(Module &M) { + bool Changed = false; + + dumpArguments(); + dumpPasses(); + + for (ImmutablePass *ImPass : getImmutablePasses()) + Changed |= ImPass->doInitialization(M); + + for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) + Changed |= getContainedManager(Index)->doInitialization(M); + + return Changed; +} + +bool FunctionPassManagerImpl::doFinalization(Module &M) { + bool Changed = false; + + for (int Index = getNumContainedManagers() - 1; Index >= 0; --Index) + Changed |= getContainedManager(Index)->doFinalization(M); + + for (ImmutablePass *ImPass : getImmutablePasses()) + Changed |= ImPass->doFinalization(M); + + return Changed; +} /// cleanup - After running all passes, clean up pass manager cache. void FPPassManager::cleanup() { @@ -1460,6 +1399,35 @@ void FPPassManager::cleanup() { } } +void FunctionPassManagerImpl::releaseMemoryOnTheFly() { + if (!wasRun) + return; + for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { + FPPassManager *FPPM = getContainedManager(Index); + for (unsigned Index = 0; Index < FPPM->getNumContainedPasses(); ++Index) { + FPPM->getContainedPass(Index)->releaseMemory(); + } + } + wasRun = false; +} + +// Execute all the passes managed by this top level manager. +// Return true if any function is modified by a pass. +bool FunctionPassManagerImpl::run(Function &F) { + bool Changed = false; + + initializeAllAnalysisInfo(); + for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { + Changed |= getContainedManager(Index)->runOnFunction(F); + F.getContext().yield(); + } + + for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) + getContainedManager(Index)->cleanup(); + + wasRun = true; + return Changed; +} //===----------------------------------------------------------------------===// // FPPassManager implementation @@ -1586,7 +1554,7 @@ MPPassManager::runOnModule(Module &M) { // Initialize on-the-fly passes for (auto &OnTheFlyManager : OnTheFlyManagers) { - legacy::FunctionPassManagerImpl *FPP = OnTheFlyManager.second; + FunctionPassManagerImpl *FPP = OnTheFlyManager.second; Changed |= FPP->doInitialization(M); } @@ -1647,7 +1615,7 @@ MPPassManager::runOnModule(Module &M) { // Finalize on-the-fly passes for (auto &OnTheFlyManager : OnTheFlyManagers) { - legacy::FunctionPassManagerImpl *FPP = OnTheFlyManager.second; + FunctionPassManagerImpl *FPP = OnTheFlyManager.second; // We don't know when is the last time an on-the-fly pass is run, // so we need to releaseMemory / finalize here FPP->releaseMemoryOnTheFly(); @@ -1668,9 +1636,9 @@ void MPPassManager::addLowerLevelRequiredPass(Pass *P, Pass *RequiredPass) { RequiredPass->getPotentialPassManagerType()) && "Unable to handle Pass that requires lower level Analysis pass"); - legacy::FunctionPassManagerImpl *FPP = OnTheFlyManagers[P]; + FunctionPassManagerImpl *FPP = OnTheFlyManagers[P]; if (!FPP) { - FPP = new legacy::FunctionPassManagerImpl(); + FPP = new FunctionPassManagerImpl(); // FPP is the top level manager. FPP->setTopLevelManager(FPP); @@ -1701,7 +1669,7 @@ void MPPassManager::addLowerLevelRequiredPass(Pass *P, Pass *RequiredPass) { /// its runOnFunction() for function F. std::tuple MPPassManager::getOnTheFlyPass(Pass *MP, AnalysisID PI, Function &F) { - legacy::FunctionPassManagerImpl *FPP = OnTheFlyManagers[MP]; + FunctionPassManagerImpl *FPP = OnTheFlyManagers[MP]; assert(FPP && "Unable to find on the fly pass"); FPP->releaseMemoryOnTheFly(); @@ -1710,8 +1678,32 @@ std::tuple MPPassManager::getOnTheFlyPass(Pass *MP, AnalysisID PI, Changed); } -namespace llvm { -namespace legacy { +//===----------------------------------------------------------------------===// +// PassManagerImpl implementation + +// +/// run - Execute all of the passes scheduled for execution. Keep track of +/// whether any of the passes modifies the module, and if so, return true. +bool PassManagerImpl::run(Module &M) { + bool Changed = false; + + dumpArguments(); + dumpPasses(); + + for (ImmutablePass *ImPass : getImmutablePasses()) + Changed |= ImPass->doInitialization(M); + + initializeAllAnalysisInfo(); + for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) { + Changed |= getContainedManager(Index)->runOnModule(M); + M.getContext().yield(); + } + + for (ImmutablePass *ImPass : getImmutablePasses()) + Changed |= ImPass->doFinalization(M); + + return Changed; +} //===----------------------------------------------------------------------===// // PassManager implementation @@ -1736,8 +1728,6 @@ void PassManager::add(Pass *P) { bool PassManager::run(Module &M) { return PM->run(M); } -} // namespace legacy -} // namespace llvm //===----------------------------------------------------------------------===// // PMStack implementation @@ -1828,4 +1818,4 @@ void FunctionPass::assignPassManager(PMStack &PMS, PM->add(this); } -legacy::PassManagerBase::~PassManagerBase() {} +PassManagerBase::~PassManagerBase() {} diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index 771cdfd17aa542..4d6c30b87a99bd 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -295,16 +295,11 @@ const PassBuilder::OptimizationLevel PassBuilder::OptimizationLevel::Oz = { namespace { -// The following passes/analyses have custom names, otherwise their name will -// include `(anonymous namespace)`. These are special since they are only for -// testing purposes and don't live in a header file. - /// No-op module pass which does nothing. -struct NoOpModulePass : PassInfoMixin { +struct NoOpModulePass { PreservedAnalyses run(Module &M, ModuleAnalysisManager &) { return PreservedAnalyses::all(); } - static StringRef name() { return "NoOpModulePass"; } }; @@ -320,7 +315,7 @@ class NoOpModuleAnalysis : public AnalysisInfoMixin { }; /// No-op CGSCC pass which does nothing. -struct NoOpCGSCCPass : PassInfoMixin { +struct NoOpCGSCCPass { PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &, LazyCallGraph &, CGSCCUpdateResult &UR) { return PreservedAnalyses::all(); @@ -342,7 +337,7 @@ class NoOpCGSCCAnalysis : public AnalysisInfoMixin { }; /// No-op function pass which does nothing. -struct NoOpFunctionPass : PassInfoMixin { +struct NoOpFunctionPass { PreservedAnalyses run(Function &F, FunctionAnalysisManager &) { return PreservedAnalyses::all(); } @@ -361,7 +356,7 @@ class NoOpFunctionAnalysis : public AnalysisInfoMixin { }; /// No-op loop pass which does nothing. -struct NoOpLoopPass : PassInfoMixin { +struct NoOpLoopPass { PreservedAnalyses run(Loop &L, LoopAnalysisManager &, LoopStandardAnalysisResults &, LPMUpdater &) { return PreservedAnalyses::all(); @@ -387,7 +382,7 @@ AnalysisKey NoOpCGSCCAnalysis::Key; AnalysisKey NoOpFunctionAnalysis::Key; AnalysisKey NoOpLoopAnalysis::Key; -} // namespace +} // End anonymous namespace. void PassBuilder::invokePeepholeEPCallbacks( FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) { From 90b1a710aede2b276cda47538142fef6f5253361 Mon Sep 17 00:00:00 2001 From: Lei Huang Date: Wed, 8 Jul 2020 17:07:34 -0500 Subject: [PATCH 05/14] [PowerPC] Enable default support of quad precision operations Summary: Remove option guarding support of quad precision operations. Reviewers: nemanjai, #powerpc, steven.zhang Reviewed By: nemanjai, #powerpc, steven.zhang Subscribers: qiucf, wuzish, nemanjai, hiraditya, kbarton, shchenz, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D83437 --- llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 148 +++++++++--------- .../CodeGen/PowerPC/builtins-ppc-p9-f128.ll | 5 +- llvm/test/CodeGen/PowerPC/constant-pool.ll | 3 +- llvm/test/CodeGen/PowerPC/f128-aggregates.ll | 7 +- llvm/test/CodeGen/PowerPC/f128-arith.ll | 3 +- llvm/test/CodeGen/PowerPC/f128-bitcast.ll | 10 +- llvm/test/CodeGen/PowerPC/f128-compare.ll | 3 +- llvm/test/CodeGen/PowerPC/f128-conv.ll | 4 +- llvm/test/CodeGen/PowerPC/f128-fma.ll | 3 +- llvm/test/CodeGen/PowerPC/f128-passByValue.ll | 3 +- llvm/test/CodeGen/PowerPC/f128-rounding.ll | 3 +- .../CodeGen/PowerPC/f128-truncateNconv.ll | 4 +- .../CodeGen/PowerPC/f128-vecExtractNconv.ll | 4 +- .../CodeGen/PowerPC/float-load-store-pair.ll | 16 +- llvm/test/CodeGen/PowerPC/fp-strict-f128.ll | 3 +- .../global-address-non-got-indirect-access.ll | 4 +- .../CodeGen/PowerPC/pcrel-got-indirect.ll | 4 +- llvm/test/CodeGen/PowerPC/ppc64-P9-setb.ll | 2 +- llvm/test/CodeGen/PowerPC/recipest.ll | 18 +-- 19 files changed, 107 insertions(+), 140 deletions(-) diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index 229c5a76010c67..49140bab513435 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -117,9 +117,6 @@ cl::desc("disable sibling call optimization on ppc"), cl::Hidden); static cl::opt DisableInnermostLoopAlign32("disable-ppc-innermost-loop-align32", cl::desc("don't always align innermost loop to 32 bytes on ppc"), cl::Hidden); -static cl::opt EnableQuadPrecision("enable-ppc-quad-precision", -cl::desc("enable quad precision float support on ppc"), cl::Hidden); - static cl::opt UseAbsoluteJumpTables("ppc-use-absolute-jumptables", cl::desc("use absolute jump tables on ppc"), cl::Hidden); @@ -1004,61 +1001,59 @@ PPCTargetLowering::PPCTargetLowering(const PPCTargetMachine &TM, setOperationAction(ISD::SRL, MVT::v1i128, Legal); setOperationAction(ISD::SRA, MVT::v1i128, Expand); - if (EnableQuadPrecision) { - addRegisterClass(MVT::f128, &PPC::VRRCRegClass); - setOperationAction(ISD::FADD, MVT::f128, Legal); - setOperationAction(ISD::FSUB, MVT::f128, Legal); - setOperationAction(ISD::FDIV, MVT::f128, Legal); - setOperationAction(ISD::FMUL, MVT::f128, Legal); - setOperationAction(ISD::FP_EXTEND, MVT::f128, Legal); - // No extending loads to f128 on PPC. - for (MVT FPT : MVT::fp_valuetypes()) - setLoadExtAction(ISD::EXTLOAD, MVT::f128, FPT, Expand); - setOperationAction(ISD::FMA, MVT::f128, Legal); - setCondCodeAction(ISD::SETULT, MVT::f128, Expand); - setCondCodeAction(ISD::SETUGT, MVT::f128, Expand); - setCondCodeAction(ISD::SETUEQ, MVT::f128, Expand); - setCondCodeAction(ISD::SETOGE, MVT::f128, Expand); - setCondCodeAction(ISD::SETOLE, MVT::f128, Expand); - setCondCodeAction(ISD::SETONE, MVT::f128, Expand); - - setOperationAction(ISD::FTRUNC, MVT::f128, Legal); - setOperationAction(ISD::FRINT, MVT::f128, Legal); - setOperationAction(ISD::FFLOOR, MVT::f128, Legal); - setOperationAction(ISD::FCEIL, MVT::f128, Legal); - setOperationAction(ISD::FNEARBYINT, MVT::f128, Legal); - setOperationAction(ISD::FROUND, MVT::f128, Legal); - - setOperationAction(ISD::SELECT, MVT::f128, Expand); - setOperationAction(ISD::FP_ROUND, MVT::f64, Legal); - setOperationAction(ISD::FP_ROUND, MVT::f32, Legal); - setTruncStoreAction(MVT::f128, MVT::f64, Expand); - setTruncStoreAction(MVT::f128, MVT::f32, Expand); - setOperationAction(ISD::BITCAST, MVT::i128, Custom); - // No implementation for these ops for PowerPC. - setOperationAction(ISD::FSIN , MVT::f128, Expand); - setOperationAction(ISD::FCOS , MVT::f128, Expand); - setOperationAction(ISD::FPOW, MVT::f128, Expand); - setOperationAction(ISD::FPOWI, MVT::f128, Expand); - setOperationAction(ISD::FREM, MVT::f128, Expand); - - // Handle constrained floating-point operations of fp128 - setOperationAction(ISD::STRICT_FADD, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FSUB, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FMUL, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FDIV, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FMA, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FSQRT, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FP_EXTEND, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FP_ROUND, MVT::f64, Legal); - setOperationAction(ISD::STRICT_FP_ROUND, MVT::f32, Legal); - setOperationAction(ISD::STRICT_FRINT, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FNEARBYINT, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FFLOOR, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FCEIL, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FTRUNC, MVT::f128, Legal); - setOperationAction(ISD::STRICT_FROUND, MVT::f128, Legal); - } + addRegisterClass(MVT::f128, &PPC::VRRCRegClass); + setOperationAction(ISD::FADD, MVT::f128, Legal); + setOperationAction(ISD::FSUB, MVT::f128, Legal); + setOperationAction(ISD::FDIV, MVT::f128, Legal); + setOperationAction(ISD::FMUL, MVT::f128, Legal); + setOperationAction(ISD::FP_EXTEND, MVT::f128, Legal); + // No extending loads to f128 on PPC. + for (MVT FPT : MVT::fp_valuetypes()) + setLoadExtAction(ISD::EXTLOAD, MVT::f128, FPT, Expand); + setOperationAction(ISD::FMA, MVT::f128, Legal); + setCondCodeAction(ISD::SETULT, MVT::f128, Expand); + setCondCodeAction(ISD::SETUGT, MVT::f128, Expand); + setCondCodeAction(ISD::SETUEQ, MVT::f128, Expand); + setCondCodeAction(ISD::SETOGE, MVT::f128, Expand); + setCondCodeAction(ISD::SETOLE, MVT::f128, Expand); + setCondCodeAction(ISD::SETONE, MVT::f128, Expand); + + setOperationAction(ISD::FTRUNC, MVT::f128, Legal); + setOperationAction(ISD::FRINT, MVT::f128, Legal); + setOperationAction(ISD::FFLOOR, MVT::f128, Legal); + setOperationAction(ISD::FCEIL, MVT::f128, Legal); + setOperationAction(ISD::FNEARBYINT, MVT::f128, Legal); + setOperationAction(ISD::FROUND, MVT::f128, Legal); + + setOperationAction(ISD::SELECT, MVT::f128, Expand); + setOperationAction(ISD::FP_ROUND, MVT::f64, Legal); + setOperationAction(ISD::FP_ROUND, MVT::f32, Legal); + setTruncStoreAction(MVT::f128, MVT::f64, Expand); + setTruncStoreAction(MVT::f128, MVT::f32, Expand); + setOperationAction(ISD::BITCAST, MVT::i128, Custom); + // No implementation for these ops for PowerPC. + setOperationAction(ISD::FSIN, MVT::f128, Expand); + setOperationAction(ISD::FCOS, MVT::f128, Expand); + setOperationAction(ISD::FPOW, MVT::f128, Expand); + setOperationAction(ISD::FPOWI, MVT::f128, Expand); + setOperationAction(ISD::FREM, MVT::f128, Expand); + + // Handle constrained floating-point operations of fp128 + setOperationAction(ISD::STRICT_FADD, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FSUB, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FMUL, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FDIV, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FMA, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FSQRT, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FP_EXTEND, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FP_ROUND, MVT::f64, Legal); + setOperationAction(ISD::STRICT_FP_ROUND, MVT::f32, Legal); + setOperationAction(ISD::STRICT_FRINT, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FNEARBYINT, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FFLOOR, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FCEIL, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FTRUNC, MVT::f128, Legal); + setOperationAction(ISD::STRICT_FROUND, MVT::f128, Legal); setOperationAction(ISD::FP_EXTEND, MVT::v2f32, Custom); setOperationAction(ISD::BSWAP, MVT::v8i16, Legal); setOperationAction(ISD::BSWAP, MVT::v4i32, Legal); @@ -1307,20 +1302,18 @@ PPCTargetLowering::PPCTargetLowering(const PPCTargetMachine &TM, setTargetDAGCombine(ISD::VSELECT); } - if (EnableQuadPrecision) { - setLibcallName(RTLIB::LOG_F128, "logf128"); - setLibcallName(RTLIB::LOG2_F128, "log2f128"); - setLibcallName(RTLIB::LOG10_F128, "log10f128"); - setLibcallName(RTLIB::EXP_F128, "expf128"); - setLibcallName(RTLIB::EXP2_F128, "exp2f128"); - setLibcallName(RTLIB::SIN_F128, "sinf128"); - setLibcallName(RTLIB::COS_F128, "cosf128"); - setLibcallName(RTLIB::POW_F128, "powf128"); - setLibcallName(RTLIB::FMIN_F128, "fminf128"); - setLibcallName(RTLIB::FMAX_F128, "fmaxf128"); - setLibcallName(RTLIB::POWI_F128, "__powikf2"); - setLibcallName(RTLIB::REM_F128, "fmodf128"); - } + setLibcallName(RTLIB::LOG_F128, "logf128"); + setLibcallName(RTLIB::LOG2_F128, "log2f128"); + setLibcallName(RTLIB::LOG10_F128, "log10f128"); + setLibcallName(RTLIB::EXP_F128, "expf128"); + setLibcallName(RTLIB::EXP2_F128, "exp2f128"); + setLibcallName(RTLIB::SIN_F128, "sinf128"); + setLibcallName(RTLIB::COS_F128, "cosf128"); + setLibcallName(RTLIB::POW_F128, "powf128"); + setLibcallName(RTLIB::FMIN_F128, "fminf128"); + setLibcallName(RTLIB::FMAX_F128, "fmaxf128"); + setLibcallName(RTLIB::POWI_F128, "__powikf2"); + setLibcallName(RTLIB::REM_F128, "fmodf128"); // With 32 condition bits, we don't need to sink (and duplicate) compares // aggressively in CodeGenPrep. @@ -8308,7 +8301,7 @@ SDValue PPCTargetLowering::LowerFP_TO_INT(SDValue Op, SelectionDAG &DAG, const SDLoc &dl) const { // FP to INT conversions are legal for f128. - if (EnableQuadPrecision && (Op->getOperand(0).getValueType() == MVT::f128)) + if (Op->getOperand(0).getValueType() == MVT::f128) return Op; // Expand ppcf128 to i32 by hand for the benefit of llvm-gcc bootstrap on @@ -8576,7 +8569,7 @@ SDValue PPCTargetLowering::LowerINT_TO_FP(SDValue Op, return LowerINT_TO_FPVector(Op, DAG, dl); // Conversions to f128 are legal. - if (EnableQuadPrecision && (Op.getValueType() == MVT::f128)) + if (Op.getValueType() == MVT::f128) return Op; if (Subtarget.hasQPX() && Op.getOperand(0).getValueType() == MVT::v4i1) { @@ -9104,10 +9097,9 @@ SDValue PPCTargetLowering::LowerBITCAST(SDValue Op, SelectionDAG &DAG) const { SDLoc dl(Op); SDValue Op0 = Op->getOperand(0); - if (!EnableQuadPrecision || - (Op.getValueType() != MVT::f128 ) || + if ((Op.getValueType() != MVT::f128) || (Op0.getOpcode() != ISD::BUILD_PAIR) || - (Op0.getOperand(0).getValueType() != MVT::i64) || + (Op0.getOperand(0).getValueType() != MVT::i64) || (Op0.getOperand(1).getValueType() != MVT::i64)) return SDValue(); @@ -16373,7 +16365,7 @@ bool PPCTargetLowering::isFMAFasterThanFMulAndFAdd(const Function &F, case Type::DoubleTyID: return true; case Type::FP128TyID: - return EnableQuadPrecision && Subtarget.hasP9Vector(); + return Subtarget.hasP9Vector(); default: return false; } diff --git a/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-f128.ll b/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-f128.ll index 366493ae76b265..bd8d6099c40f64 100644 --- a/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-f128.ll +++ b/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-f128.ll @@ -1,6 +1,5 @@ -; RUN: llc -verify-machineinstrs -mcpu=pwr9 -enable-ppc-quad-precision \ -; RUN: -mtriple=powerpc64le-unknown-unknown -ppc-vsr-nums-as-vr \ -; RUN: -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: llc -verify-machineinstrs -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ +; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s @A = common global fp128 0xL00000000000000000000000000000000, align 16 @B = common global fp128 0xL00000000000000000000000000000000, align 16 diff --git a/llvm/test/CodeGen/PowerPC/constant-pool.ll b/llvm/test/CodeGen/PowerPC/constant-pool.ll index 797fc74672a28e..4355cfa6ba211f 100644 --- a/llvm/test/CodeGen/PowerPC/constant-pool.ll +++ b/llvm/test/CodeGen/PowerPC/constant-pool.ll @@ -1,6 +1,5 @@ ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \ -; RUN: -mcpu=future -enable-ppc-quad-precision -ppc-asm-full-reg-names \ -; RUN: < %s | FileCheck %s +; RUN: -mcpu=future -ppc-asm-full-reg-names < %s | FileCheck %s define float @FloatConstantPool() { ; CHECK-LABEL: FloatConstantPool: diff --git a/llvm/test/CodeGen/PowerPC/f128-aggregates.ll b/llvm/test/CodeGen/PowerPC/f128-aggregates.ll index 006ad745f60732..094d29e2f258dc 100644 --- a/llvm/test/CodeGen/PowerPC/f128-aggregates.ll +++ b/llvm/test/CodeGen/PowerPC/f128-aggregates.ll @@ -1,9 +1,8 @@ ; RUN: llc -relocation-model=pic -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ -; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: -verify-machineinstrs -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s \ +; RUN: | FileCheck %s ; RUN: llc -relocation-model=pic -mcpu=pwr9 -mtriple=powerpc64-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ -; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s \ +; RUN: -verify-machineinstrs -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s \ ; RUN: | FileCheck -check-prefix=CHECK-BE %s ; Testing homogeneous aggregates. diff --git a/llvm/test/CodeGen/PowerPC/f128-arith.ll b/llvm/test/CodeGen/PowerPC/f128-arith.ll index a957e0e6bdaa9c..40b123bb9276be 100644 --- a/llvm/test/CodeGen/PowerPC/f128-arith.ll +++ b/llvm/test/CodeGen/PowerPC/f128-arith.ll @@ -1,5 +1,4 @@ -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs \ ; RUN: -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s ; Function Attrs: norecurse nounwind diff --git a/llvm/test/CodeGen/PowerPC/f128-bitcast.ll b/llvm/test/CodeGen/PowerPC/f128-bitcast.ll index 68069e542ffd61..fca24f5fd54106 100644 --- a/llvm/test/CodeGen/PowerPC/f128-bitcast.ll +++ b/llvm/test/CodeGen/PowerPC/f128-bitcast.ll @@ -1,10 +1,8 @@ -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs \ ; RUN: -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ -; RUN: -ppc-asm-full-reg-names \ -; RUN: -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-BE +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64-unknown-unknown -verify-machineinstrs \ +; RUN: -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \ +; RUN: FileCheck %s --check-prefix=CHECK-BE ; Function Attrs: norecurse nounwind readnone define i64 @getPart1(fp128 %in) local_unnamed_addr { diff --git a/llvm/test/CodeGen/PowerPC/f128-compare.ll b/llvm/test/CodeGen/PowerPC/f128-compare.ll index c876878f05faca..5376b3b3f1c5ae 100644 --- a/llvm/test/CodeGen/PowerPC/f128-compare.ll +++ b/llvm/test/CodeGen/PowerPC/f128-compare.ll @@ -1,5 +1,4 @@ -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs \ ; RUN: -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s @a_qp = common global fp128 0xL00000000000000000000000000000000, align 16 diff --git a/llvm/test/CodeGen/PowerPC/f128-conv.ll b/llvm/test/CodeGen/PowerPC/f128-conv.ll index 4c64341d634972..2cb3174925457f 100644 --- a/llvm/test/CodeGen/PowerPC/f128-conv.ll +++ b/llvm/test/CodeGen/PowerPC/f128-conv.ll @@ -1,6 +1,6 @@ ; RUN: llc -relocation-model=pic -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -ppc-vsr-nums-as-vr \ -; RUN: -verify-machineinstrs -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: -ppc-vsr-nums-as-vr -verify-machineinstrs -ppc-asm-full-reg-names < %s \ +; RUN: | FileCheck %s @mem = global [5 x i64] [i64 56, i64 63, i64 3, i64 5, i64 6], align 8 @umem = global [5 x i64] [i64 560, i64 100, i64 34, i64 2, i64 5], align 8 diff --git a/llvm/test/CodeGen/PowerPC/f128-fma.ll b/llvm/test/CodeGen/PowerPC/f128-fma.ll index 8f76520d32bd58..f63ae04699f496 100644 --- a/llvm/test/CodeGen/PowerPC/f128-fma.ll +++ b/llvm/test/CodeGen/PowerPC/f128-fma.ll @@ -1,6 +1,5 @@ ; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -ppc-vsr-nums-as-vr \ -; RUN: -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s define void @qpFmadd(fp128* nocapture readonly %a, fp128* nocapture %b, fp128* nocapture readonly %c, fp128* nocapture %res) { diff --git a/llvm/test/CodeGen/PowerPC/f128-passByValue.ll b/llvm/test/CodeGen/PowerPC/f128-passByValue.ll index cbccaea3bce189..8b2db6b0351097 100644 --- a/llvm/test/CodeGen/PowerPC/f128-passByValue.ll +++ b/llvm/test/CodeGen/PowerPC/f128-passByValue.ll @@ -1,5 +1,4 @@ -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -ppc-vsr-nums-as-vr \ +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -ppc-vsr-nums-as-vr \ ; RUN: -verify-machineinstrs -ppc-asm-full-reg-names < %s | FileCheck %s ; Function Attrs: norecurse nounwind readnone diff --git a/llvm/test/CodeGen/PowerPC/f128-rounding.ll b/llvm/test/CodeGen/PowerPC/f128-rounding.ll index 063eb1456fd83d..56f63be5734e86 100644 --- a/llvm/test/CodeGen/PowerPC/f128-rounding.ll +++ b/llvm/test/CodeGen/PowerPC/f128-rounding.ll @@ -1,5 +1,4 @@ -; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -verify-machineinstrs \ +; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs \ ; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s diff --git a/llvm/test/CodeGen/PowerPC/f128-truncateNconv.ll b/llvm/test/CodeGen/PowerPC/f128-truncateNconv.ll index ebded9dcccbe5c..10d56fb2c47a6a 100644 --- a/llvm/test/CodeGen/PowerPC/f128-truncateNconv.ll +++ b/llvm/test/CodeGen/PowerPC/f128-truncateNconv.ll @@ -1,6 +1,6 @@ ; RUN: llc -relocation-model=pic -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -verify-machineinstrs -enable-ppc-quad-precision \ -; RUN: -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: -verify-machineinstrs -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s \ +; RUN: | FileCheck %s @f128Array = global [4 x fp128] [fp128 0xL00000000000000004004C00000000000, fp128 0xLF000000000000000400808AB851EB851, diff --git a/llvm/test/CodeGen/PowerPC/f128-vecExtractNconv.ll b/llvm/test/CodeGen/PowerPC/f128-vecExtractNconv.ll index bae676cd09cd34..8542b107223376 100644 --- a/llvm/test/CodeGen/PowerPC/f128-vecExtractNconv.ll +++ b/llvm/test/CodeGen/PowerPC/f128-vecExtractNconv.ll @@ -1,10 +1,10 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown -ppc-vsr-nums-as-vr \ ; RUN: -relocation-model=pic -ppc-asm-full-reg-names -verify-machineinstrs \ -; RUN: -enable-ppc-quad-precision < %s | FileCheck %s +; RUN: < %s | FileCheck %s ; RUN: llc -mcpu=pwr9 -mtriple=powerpc64-unknown-unknown -ppc-vsr-nums-as-vr \ ; RUN: -ppc-asm-full-reg-names -verify-machineinstrs \ -; RUN: -enable-ppc-quad-precision < %s | FileCheck %s -check-prefix=CHECK-BE +; RUN: < %s | FileCheck %s -check-prefix=CHECK-BE ; Vector extract DWord and convert to quad precision. diff --git a/llvm/test/CodeGen/PowerPC/float-load-store-pair.ll b/llvm/test/CodeGen/PowerPC/float-load-store-pair.ll index a8ed39ee9ce0e6..caeff71553343d 100644 --- a/llvm/test/CodeGen/PowerPC/float-load-store-pair.ll +++ b/llvm/test/CodeGen/PowerPC/float-load-store-pair.ll @@ -52,24 +52,22 @@ define signext i32 @test() nounwind { ; CHECK-NEXT: addis 3, 2, a10@toc@ha ; CHECK-NEXT: lfd 10, a10@toc@l(3) ; CHECK-NEXT: addis 3, 2, a11@toc@ha -; CHECK-NEXT: addis 6, 2, a17@toc@ha +; CHECK-NEXT: lfd 11, a11@toc@l(3) +; CHECK-NEXT: addis 3, 2, a12@toc@ha ; CHECK-NEXT: addis 5, 2, a16@toc@ha +; CHECK-NEXT: addis 6, 2, a17@toc@ha ; CHECK-NEXT: addi 6, 6, a17@toc@l -; CHECK-NEXT: addi 5, 5, a16@toc@l ; CHECK-NEXT: lxvx 34, 0, 6 +; CHECK-NEXT: lfd 12, a12@toc@l(3) +; CHECK-NEXT: addis 3, 2, a13@toc@ha +; CHECK-NEXT: addi 5, 5, a16@toc@l ; CHECK-NEXT: addis 4, 2, a15@toc@ha ; CHECK-NEXT: lxvx 0, 0, 5 ; CHECK-NEXT: ld 4, a15@toc@l(4) -; CHECK-NEXT: li 5, 168 -; CHECK-NEXT: lfd 11, a11@toc@l(3) -; CHECK-NEXT: addis 3, 2, a12@toc@ha -; CHECK-NEXT: lfd 12, a12@toc@l(3) -; CHECK-NEXT: addis 3, 2, a13@toc@ha +; CHECK-NEXT: li 5, 152 ; CHECK-NEXT: lfd 13, a13@toc@l(3) ; CHECK-NEXT: addis 3, 2, a14@toc@ha ; CHECK-NEXT: ld 3, a14@toc@l(3) -; CHECK-NEXT: stxvx 34, 1, 5 -; CHECK-NEXT: li 5, 152 ; CHECK-NEXT: stxvx 0, 1, 5 ; CHECK-NEXT: std 4, 144(1) ; CHECK-NEXT: std 3, 136(1) diff --git a/llvm/test/CodeGen/PowerPC/fp-strict-f128.ll b/llvm/test/CodeGen/PowerPC/fp-strict-f128.ll index 21ddb799141d08..cab949a6ebd2a5 100644 --- a/llvm/test/CodeGen/PowerPC/fp-strict-f128.ll +++ b/llvm/test/CodeGen/PowerPC/fp-strict-f128.ll @@ -1,5 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -verify-machineinstrs -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s -mtriple=powerpc64le-unknown-linux -mcpu=pwr9 -enable-ppc-quad-precision=true | FileCheck %s +; RUN: llc -verify-machineinstrs -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \ +; RUN: < %s -mtriple=powerpc64le-unknown-linux -mcpu=pwr9 | FileCheck %s declare fp128 @llvm.experimental.constrained.fadd.f128(fp128, fp128, metadata, metadata) declare fp128 @llvm.experimental.constrained.fsub.f128(fp128, fp128, metadata, metadata) diff --git a/llvm/test/CodeGen/PowerPC/global-address-non-got-indirect-access.ll b/llvm/test/CodeGen/PowerPC/global-address-non-got-indirect-access.ll index f29d9bd251ca47..5c49760a9f45a7 100644 --- a/llvm/test/CodeGen/PowerPC/global-address-non-got-indirect-access.ll +++ b/llvm/test/CodeGen/PowerPC/global-address-non-got-indirect-access.ll @@ -1,7 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \ -; RUN: -mcpu=future -enable-ppc-quad-precision -ppc-asm-full-reg-names \ -; RUN: -ppc-vsr-nums-as-vr < %s | FileCheck %s +; RUN: -mcpu=future -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s \ +; RUN: | FileCheck %s @_ZL13StaticBoolVar = internal unnamed_addr global i8 0, align 1 @_ZL19StaticSignedCharVar = internal unnamed_addr global i8 0, align 1 diff --git a/llvm/test/CodeGen/PowerPC/pcrel-got-indirect.ll b/llvm/test/CodeGen/PowerPC/pcrel-got-indirect.ll index 7f7659b356ee74..c838308f3aa654 100644 --- a/llvm/test/CodeGen/PowerPC/pcrel-got-indirect.ll +++ b/llvm/test/CodeGen/PowerPC/pcrel-got-indirect.ll @@ -1,7 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \ -; RUN: -mcpu=future -enable-ppc-quad-precision -ppc-asm-full-reg-names \ -; RUN: -ppc-vsr-nums-as-vr < %s | FileCheck %s +; RUN: -mcpu=future -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s \ +; RUN: | FileCheck %s %struct.Struct = type { i8, i16, i32 } diff --git a/llvm/test/CodeGen/PowerPC/ppc64-P9-setb.ll b/llvm/test/CodeGen/PowerPC/ppc64-P9-setb.ll index feb20465c510c4..4762b10af7dd48 100644 --- a/llvm/test/CodeGen/PowerPC/ppc64-P9-setb.ll +++ b/llvm/test/CodeGen/PowerPC/ppc64-P9-setb.ll @@ -1,5 +1,5 @@ ; RUN: llc -verify-machineinstrs -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \ -; RUN: -enable-ppc-quad-precision -ppc-asm-full-reg-names < %s | FileCheck %s +; RUN: -ppc-asm-full-reg-names < %s | FileCheck %s ; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mtriple=powerpc64le-unknown-unknown \ ; RUN: -ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-PWR8 \ ; RUN: -implicit-check-not "\" diff --git a/llvm/test/CodeGen/PowerPC/recipest.ll b/llvm/test/CodeGen/PowerPC/recipest.ll index 4276d944af8f1b..7ceddd95e5735a 100644 --- a/llvm/test/CodeGen/PowerPC/recipest.ll +++ b/llvm/test/CodeGen/PowerPC/recipest.ll @@ -1176,14 +1176,7 @@ define fp128 @hoo5_fmf(fp128 %a) #1 { ; ; CHECK-P9-LABEL: hoo5_fmf: ; CHECK-P9: # %bb.0: -; CHECK-P9-NEXT: mflr 0 -; CHECK-P9-NEXT: std 0, 16(1) -; CHECK-P9-NEXT: stdu 1, -32(1) -; CHECK-P9-NEXT: bl sqrtl -; CHECK-P9-NEXT: nop -; CHECK-P9-NEXT: addi 1, 1, 32 -; CHECK-P9-NEXT: ld 0, 16(1) -; CHECK-P9-NEXT: mtlr 0 +; CHECK-P9-NEXT: xssqrtqp 2, 2 ; CHECK-P9-NEXT: blr %r = call reassoc ninf afn fp128 @llvm.sqrt.f128(fp128 %a) ret fp128 %r @@ -1216,14 +1209,7 @@ define fp128 @hoo5_safe(fp128 %a) #1 { ; ; CHECK-P9-LABEL: hoo5_safe: ; CHECK-P9: # %bb.0: -; CHECK-P9-NEXT: mflr 0 -; CHECK-P9-NEXT: std 0, 16(1) -; CHECK-P9-NEXT: stdu 1, -32(1) -; CHECK-P9-NEXT: bl sqrtl -; CHECK-P9-NEXT: nop -; CHECK-P9-NEXT: addi 1, 1, 32 -; CHECK-P9-NEXT: ld 0, 16(1) -; CHECK-P9-NEXT: mtlr 0 +; CHECK-P9-NEXT: xssqrtqp 2, 2 ; CHECK-P9-NEXT: blr %r = call fp128 @llvm.sqrt.f128(fp128 %a) ret fp128 %r From a4f0c58c6e3f4ec0f5bb8c5232a77dd452df0fb5 Mon Sep 17 00:00:00 2001 From: cgyurgyik Date: Fri, 10 Jul 2020 14:28:20 -0400 Subject: [PATCH 06/14] [libc] Add strchr implementation. Fixes bug in memchr. Summary: [libc] Adds strchr implementation with unit tests. Fixes signed character bug in memchr. Reviewers: sivachandra, PaulkaToast Reviewed By: sivachandra Subscribers: mgorny, tschuett, ecnelises, libc-commits Tags: #libc-project Differential Revision: https://reviews.llvm.org/D83353 --- libc/config/linux/aarch64/entrypoints.txt | 1 + libc/config/linux/x86_64/entrypoints.txt | 1 + libc/src/string/CMakeLists.txt | 9 +++ libc/src/string/memchr.cpp | 3 +- libc/src/string/strchr.cpp | 26 +++++++ libc/src/string/strchr.h | 18 +++++ libc/test/src/string/CMakeLists.txt | 10 +++ libc/test/src/string/memchr_test.cpp | 9 +++ libc/test/src/string/strchr_test.cpp | 87 +++++++++++++++++++++++ 9 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 libc/src/string/strchr.cpp create mode 100644 libc/src/string/strchr.h create mode 100644 libc/test/src/string/strchr_test.cpp diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt index 1eb9e8a995160d..5d607b82892ffc 100644 --- a/libc/config/linux/aarch64/entrypoints.txt +++ b/libc/config/linux/aarch64/entrypoints.txt @@ -10,6 +10,7 @@ set(TARGET_LIBC_ENTRYPOINTS libc.src.string.strcat libc.src.string.strlen libc.src.string.memchr + libc.src.string.strchr ) set(TARGET_LIBM_ENTRYPOINTS diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt index 322bf050c716eb..fc62fbb880b413 100644 --- a/libc/config/linux/x86_64/entrypoints.txt +++ b/libc/config/linux/x86_64/entrypoints.txt @@ -28,6 +28,7 @@ set(TARGET_LIBC_ENTRYPOINTS libc.src.string.strlen libc.src.string.strcmp libc.src.string.memchr + libc.src.string.strchr # sys/mman.h entrypoints libc.src.sys.mman.mmap diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt index 05deee10d71498..8d22a139006050 100644 --- a/libc/src/string/CMakeLists.txt +++ b/libc/src/string/CMakeLists.txt @@ -52,6 +52,15 @@ add_entrypoint_object( memchr.h ) +add_entrypoint_object( + strchr + SRCS + strchr.cpp + HDRS + strchr.h + DEPENDS + .strlen +) # Helper to define a function with multiple implementations # - Computes flags to satisfy required/rejected features and arch, diff --git a/libc/src/string/memchr.cpp b/libc/src/string/memchr.cpp index eee3d4971edc4e..303f78185f49c3 100644 --- a/libc/src/string/memchr.cpp +++ b/libc/src/string/memchr.cpp @@ -15,7 +15,8 @@ namespace __llvm_libc { // TODO: Look at performance benefits of comparing words. void *LLVM_LIBC_ENTRYPOINT(memchr)(const void *src, int c, size_t n) { const unsigned char *str = reinterpret_cast(src); - for (; n && *str != c; --n, ++str) + const unsigned char ch = c; + for (; n && *str != ch; --n, ++str) ; return n ? const_cast(str) : nullptr; } diff --git a/libc/src/string/strchr.cpp b/libc/src/string/strchr.cpp new file mode 100644 index 00000000000000..73dc7e7b1c9655 --- /dev/null +++ b/libc/src/string/strchr.cpp @@ -0,0 +1,26 @@ +//===-- Implementation of strchr ------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/string/strchr.h" +#include "src/string/strlen.h" + +#include "src/__support/common.h" + +namespace __llvm_libc { + +// TODO: Look at performance benefits of comparing words. +char *LLVM_LIBC_ENTRYPOINT(strchr)(const char *src, int c) { + unsigned char *str = + const_cast(reinterpret_cast(src)); + const unsigned char ch = c; + for (; *str && *str != ch; ++str) + ; + return *str == ch ? reinterpret_cast(str) : nullptr; +} + +} // namespace __llvm_libc diff --git a/libc/src/string/strchr.h b/libc/src/string/strchr.h new file mode 100644 index 00000000000000..6f106afaf695c1 --- /dev/null +++ b/libc/src/string/strchr.h @@ -0,0 +1,18 @@ +//===-- Implementation header for strchr ------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_LIBC_SRC_STRING_STRCHR_H +#define LLVM_LIBC_SRC_STRING_STRCHR_H + +namespace __llvm_libc { + +char *strchr(const char *src, int c); + +} // namespace __llvm_libc + +#endif // LLVM_LIBC_SRC_STRING_STRCHR_H diff --git a/libc/test/src/string/CMakeLists.txt b/libc/test/src/string/CMakeLists.txt index 0055591be5c716..be4836818212d8 100644 --- a/libc/test/src/string/CMakeLists.txt +++ b/libc/test/src/string/CMakeLists.txt @@ -52,6 +52,16 @@ add_libc_unittest( libc.src.string.memchr ) +add_libc_unittest( + strchr_test + SUITE + libc_string_unittests + SRCS + strchr_test.cpp + DEPENDS + libc.src.string.strchr +) + # Tests all implementations that can run on the host. function(add_libc_multi_impl_test name) get_property(fq_implementations GLOBAL PROPERTY ${name}_implementations) diff --git a/libc/test/src/string/memchr_test.cpp b/libc/test/src/string/memchr_test.cpp index 370aa2fa9f1328..005e02c6b3d4d9 100644 --- a/libc/test/src/string/memchr_test.cpp +++ b/libc/test/src/string/memchr_test.cpp @@ -111,3 +111,12 @@ TEST(MemChrTest, SingleRepeatedCharacterShouldReturnFirst) { // Should return original string since X is first character. ASSERT_STREQ(call_memchr(dups, 'X', size), dups); } + +TEST(MemChrTest, SignedCharacterFound) { + char c = -1; + const size_t size = 1; + char src[size] = {c}; + const char *actual = call_memchr(src, c, size); + // Should find the first character 'c'. + ASSERT_EQ(actual[0], c); +} diff --git a/libc/test/src/string/strchr_test.cpp b/libc/test/src/string/strchr_test.cpp new file mode 100644 index 00000000000000..37b37338579977 --- /dev/null +++ b/libc/test/src/string/strchr_test.cpp @@ -0,0 +1,87 @@ +//===-- Unittests for strchr ----------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/string/strchr.h" +#include "utils/UnitTest/Test.h" + +TEST(StrChrTest, FindsFirstCharacter) { + const char *src = "abcde"; + const char *src_copy = src; + + // Should return original string since 'a' is the first character. + ASSERT_STREQ(__llvm_libc::strchr(src, 'a'), "abcde"); + // Source string should not change. + ASSERT_STREQ(src, src_copy); +} + +TEST(StrChrTest, FindsMiddleCharacter) { + const char *src = "abcde"; + const char *src_copy = src; + + // Should return characters after (and including) 'c'. + ASSERT_STREQ(__llvm_libc::strchr(src, 'c'), "cde"); + // Source string should not change. + ASSERT_STREQ(src, src_copy); +} + +TEST(StrChrTest, FindsLastCharacterThatIsNotNullTerminator) { + const char *src = "abcde"; + const char *src_copy = src; + + // Should return 'e' and null-terminator. + ASSERT_STREQ(__llvm_libc::strchr(src, 'e'), "e"); + // Source string should not change. + ASSERT_STREQ(src, src_copy); +} + +TEST(StrChrTest, FindsNullTerminator) { + const char *src = "abcde"; + const char *src_copy = src; + + // Should return null terminator. + ASSERT_STREQ(__llvm_libc::strchr(src, '\0'), ""); + // Source string should not change. + ASSERT_STREQ(src, src_copy); +} + +TEST(StrChrTest, CharacterNotWithinStringShouldReturnNullptr) { + // Since 'z' is not within the string, should return nullptr. + ASSERT_STREQ(__llvm_libc::strchr("123?", 'z'), nullptr); +} + +TEST(StrChrTest, TheSourceShouldNotChange) { + const char *src = "abcde"; + const char *src_copy = src; + // When the character is found, the source string should not change. + __llvm_libc::strchr(src, 'd'); + ASSERT_STREQ(src, src_copy); + // Same case for when the character is not found. + __llvm_libc::strchr(src, 'z'); + ASSERT_STREQ(src, src_copy); + // Same case for when looking for nullptr. + __llvm_libc::strchr(src, '\0'); + ASSERT_STREQ(src, src_copy); +} + +TEST(StrChrTest, ShouldFindFirstOfDuplicates) { + // '1' is duplicated in the string, but it should find the first copy. + ASSERT_STREQ(__llvm_libc::strchr("abc1def1ghi", '1'), "1def1ghi"); + + const char *dups = "XXXXX"; + // Should return original string since 'X' is the first character. + ASSERT_STREQ(__llvm_libc::strchr(dups, 'X'), dups); +} + +TEST(StrChrTest, EmptyStringShouldOnlyMatchNullTerminator) { + // Null terminator should match. + ASSERT_STREQ(__llvm_libc::strchr("", '\0'), ""); + // All other characters should not match. + ASSERT_STREQ(__llvm_libc::strchr("", 'Z'), nullptr); + ASSERT_STREQ(__llvm_libc::strchr("", '3'), nullptr); + ASSERT_STREQ(__llvm_libc::strchr("", '*'), nullptr); +} From e541e1b757237172c247904b670c9894d6b3759d Mon Sep 17 00:00:00 2001 From: Sidharth Baveja Date: Fri, 10 Jul 2020 18:38:08 +0000 Subject: [PATCH 07/14] [NFC] Separate Peeling Properties into its own struct (re-land after minor fix) Summary: This patch separates the peeling specific parameters from the UnrollingPreferences, and creates a new struct called PeelingPreferences. Functions which used the UnrollingPreferences struct for peeling have been updated to use the PeelingPreferences struct. Author: sidbav (Sidharth Baveja) Reviewers: Whitney (Whitney Tsang), Meinersbur (Michael Kruse), skatkov (Serguei Katkov), ashlykov (Arkady Shlykov), bogner (Justin Bogner), hfinkel (Hal Finkel), anhtuyen (Anh Tuyen Tran), nikic (Nikita Popov) Reviewed By: Meinersbur (Michael Kruse) Subscribers: fhahn (Florian Hahn), hiraditya (Aditya Kumar), llvm-commits, LLVM Tag: LLVM Differential Revision: https://reviews.llvm.org/D80580 --- .../llvm/Analysis/TargetTransformInfo.h | 42 ++++++++---- .../llvm/Analysis/TargetTransformInfoImpl.h | 3 + llvm/include/llvm/CodeGen/BasicTTIImpl.h | 8 +++ .../llvm/Transforms/Utils/UnrollLoop.h | 13 +++- llvm/lib/Analysis/TargetTransformInfo.cpp | 5 ++ .../AArch64/AArch64TargetTransformInfo.cpp | 5 ++ .../AArch64/AArch64TargetTransformInfo.h | 3 + .../AMDGPU/AMDGPUTargetTransformInfo.cpp | 14 ++++ .../Target/AMDGPU/AMDGPUTargetTransformInfo.h | 8 +++ .../lib/Target/ARM/ARMTargetTransformInfo.cpp | 5 ++ llvm/lib/Target/ARM/ARMTargetTransformInfo.h | 2 + .../Hexagon/HexagonTargetTransformInfo.cpp | 7 +- .../Hexagon/HexagonTargetTransformInfo.h | 3 + .../Target/NVPTX/NVPTXTargetTransformInfo.cpp | 5 ++ .../Target/NVPTX/NVPTXTargetTransformInfo.h | 4 ++ .../Target/PowerPC/PPCTargetTransformInfo.cpp | 4 ++ .../Target/PowerPC/PPCTargetTransformInfo.h | 2 + .../SystemZ/SystemZTargetTransformInfo.cpp | 4 ++ .../SystemZ/SystemZTargetTransformInfo.h | 3 + .../Scalar/LoopUnrollAndJamPass.cpp | 12 ++-- llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp | 67 ++++++++++++------- llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp | 25 +++---- 22 files changed, 186 insertions(+), 58 deletions(-) diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h index 695b7d6061c064..b6698eefdb0114 100644 --- a/llvm/include/llvm/Analysis/TargetTransformInfo.h +++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h @@ -450,11 +450,6 @@ class TargetTransformInfo { /// transformation will select an unrolling factor based on the current cost /// threshold and other factors. unsigned Count; - /// A forced peeling factor (the number of bodied of the original loop - /// that should be peeled off before the loop body). When set to 0, the - /// unrolling transformation will select a peeling factor based on profile - /// information and other factors. - unsigned PeelCount; /// Default unroll count for loops with run-time trip count. unsigned DefaultUnrollRuntimeCount; // Set the maximum unrolling factor. The unrolling factor may be selected @@ -488,19 +483,10 @@ class TargetTransformInfo { bool Force; /// Allow using trip count upper bound to unroll loops. bool UpperBound; - /// Allow peeling off loop iterations. - bool AllowPeeling; - /// Allow peeling off loop iterations for loop nests. - bool AllowLoopNestsPeeling; /// Allow unrolling of all the iterations of the runtime loop remainder. bool UnrollRemainder; /// Allow unroll and jam. Used to enable unroll and jam for the target. bool UnrollAndJam; - /// Allow peeling basing on profile. Uses to enable peeling off all - /// iterations basing on provided profile. - /// If the value is true the peeling cost model can decide to peel only - /// some iterations and in this case it will set this to false. - bool PeelProfiledIterations; /// Threshold for unroll and jam, for inner loop size. The 'Threshold' /// value above is used during unroll and jam for the outer loop size. /// This value is used in the same manner to limit the size of the inner @@ -534,6 +520,28 @@ class TargetTransformInfo { /// intrinsic is supported. bool emitGetActiveLaneMask() const; + // Parameters that control the loop peeling transformation + struct PeelingPreferences { + /// A forced peeling factor (the number of bodied of the original loop + /// that should be peeled off before the loop body). When set to 0, the + /// a peeling factor based on profile information and other factors. + unsigned PeelCount; + /// Allow peeling off loop iterations. + bool AllowPeeling; + /// Allow peeling off loop iterations for loop nests. + bool AllowLoopNestsPeeling; + /// Allow peeling basing on profile. Uses to enable peeling off all + /// iterations basing on provided profile. + /// If the value is true the peeling cost model can decide to peel only + /// some iterations and in this case it will set this to false. + bool PeelProfiledIterations; + }; + + /// Get target-customized preferences for the generic loop peeling + /// transformation. The caller will initialize \p PP with the current + /// target-independent defaults with information from \p L and \p SE. + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + PeelingPreferences &PP) const; /// @} /// \name Scalar Target Information @@ -1282,6 +1290,8 @@ class TargetTransformInfo::Concept { virtual bool isLoweredToCall(const Function *F) = 0; virtual void getUnrollingPreferences(Loop *L, ScalarEvolution &, UnrollingPreferences &UP) = 0; + virtual void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + PeelingPreferences &PP) = 0; virtual bool isHardwareLoopProfitable(Loop *L, ScalarEvolution &SE, AssumptionCache &AC, TargetLibraryInfo *LibInfo, @@ -1560,6 +1570,10 @@ class TargetTransformInfo::Model final : public TargetTransformInfo::Concept { UnrollingPreferences &UP) override { return Impl.getUnrollingPreferences(L, SE, UP); } + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + PeelingPreferences &PP) override { + return Impl.getPeelingPreferences(L, SE, PP); + } bool isHardwareLoopProfitable(Loop *L, ScalarEvolution &SE, AssumptionCache &AC, TargetLibraryInfo *LibInfo, HardwareLoopInfo &HWLoopInfo) override { diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h index ca7106ab98aaf3..0ce975d6d4b5d3 100644 --- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h +++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h @@ -150,6 +150,9 @@ class TargetTransformInfoImplBase { void getUnrollingPreferences(Loop *, ScalarEvolution &, TTI::UnrollingPreferences &) {} + void getPeelingPreferences(Loop *, ScalarEvolution &, + TTI::PeelingPreferences &) {} + bool isLegalAddImmediate(int64_t Imm) { return false; } bool isLegalICmpImmediate(int64_t Imm) { return false; } diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h index c6a9a65ae6c1bc..f9d32eadd23e2b 100644 --- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h +++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h @@ -451,6 +451,14 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase { UP.BEInsns = 2; } + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + PP.PeelCount = 0; + PP.AllowPeeling = true; + PP.AllowLoopNestsPeeling = false; + PP.PeelProfiledIterations = true; + } + bool isHardwareLoopProfitable(Loop *L, ScalarEvolution &SE, AssumptionCache &AC, TargetLibraryInfo *LibInfo, diff --git a/llvm/include/llvm/Transforms/Utils/UnrollLoop.h b/llvm/include/llvm/Transforms/Utils/UnrollLoop.h index 1970cefcefba83..bb3d02b9595649 100644 --- a/llvm/include/llvm/Transforms/Utils/UnrollLoop.h +++ b/llvm/include/llvm/Transforms/Utils/UnrollLoop.h @@ -94,6 +94,7 @@ bool UnrollRuntimeLoopRemainder( void computePeelCount(Loop *L, unsigned LoopSize, TargetTransformInfo::UnrollingPreferences &UP, + TargetTransformInfo::PeelingPreferences &PP, unsigned &TripCount, ScalarEvolution &SE); bool canPeel(Loop *L); @@ -119,6 +120,8 @@ bool computeUnrollCount(Loop *L, const TargetTransformInfo &TTI, unsigned MaxTripCount, bool MaxOrZero, unsigned &TripMultiple, unsigned LoopSize, TargetTransformInfo::UnrollingPreferences &UP, + TargetTransformInfo::PeelingPreferences &PP, + bool &UseUpperBound); void simplifyLoopAfterUnroll(Loop *L, bool SimplifyIVs, LoopInfo *LI, @@ -133,9 +136,13 @@ TargetTransformInfo::UnrollingPreferences gatherUnrollingPreferences( BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI, int OptLevel, Optional UserThreshold, Optional UserCount, Optional UserAllowPartial, Optional UserRuntime, - Optional UserUpperBound, Optional UserAllowPeeling, - Optional UserAllowProfileBasedPeeling, - Optional UserFullUnrollMaxCount); + Optional UserUpperBound, Optional UserFullUnrollMaxCount); + +TargetTransformInfo::PeelingPreferences +gatherPeelingPreferences(Loop *L, ScalarEvolution &SE, + const TargetTransformInfo &TTI, + Optional UserAllowPeeling, + Optional UserAllowProfileBasedPeeling); unsigned ApproximateLoopSize(const Loop *L, unsigned &NumCalls, bool &NotDuplicatable, bool &Convergent, diff --git a/llvm/lib/Analysis/TargetTransformInfo.cpp b/llvm/lib/Analysis/TargetTransformInfo.cpp index 87c6f83938ed55..2f051e53790b15 100644 --- a/llvm/lib/Analysis/TargetTransformInfo.cpp +++ b/llvm/lib/Analysis/TargetTransformInfo.cpp @@ -327,6 +327,11 @@ void TargetTransformInfo::getUnrollingPreferences( return TTIImpl->getUnrollingPreferences(L, SE, UP); } +void TargetTransformInfo::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + PeelingPreferences &PP) const { + return TTIImpl->getPeelingPreferences(L, SE, PP); +} + bool TargetTransformInfo::isLegalAddImmediate(int64_t Imm) const { return TTIImpl->isLegalAddImmediate(Imm); } diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp index be0c51b83a25b4..cf6de797727bed 100644 --- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp @@ -859,6 +859,11 @@ void AArch64TTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, getFalkorUnrollingPreferences(L, SE, UP); } +void AArch64TTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} + Value *AArch64TTIImpl::getOrCreateResultFromMemIntrinsic(IntrinsicInst *Inst, Type *ExpectedType) { switch (Inst->getIntrinsicID()) { diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h index 6b1e5d5083e2c4..1f029689a60e67 100644 --- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h +++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h @@ -153,6 +153,9 @@ class AArch64TTIImpl : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); + Value *getOrCreateResultFromMemIntrinsic(IntrinsicInst *Inst, Type *ExpectedType); diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp index 8783427b50029d..542a5f006c0f74 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp @@ -236,6 +236,10 @@ void AMDGPUTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, } } +void AMDGPUTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} unsigned GCNTTIImpl::getHardwareNumberOfRegisters(bool Vec) const { // The concept of vector registers doesn't really exist. Some packed vector // operations operate on the normal 32-bit registers. @@ -997,6 +1001,11 @@ void GCNTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, CommonTTI.getUnrollingPreferences(L, SE, UP); } +void GCNTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + CommonTTI.getPeelingPreferences(L, SE, PP); +} + unsigned R600TTIImpl::getHardwareNumberOfRegisters(bool Vec) const { return 4 * 128; // XXX - 4 channels. Should these count as vector instead? } @@ -1103,3 +1112,8 @@ void R600TTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP) { CommonTTI.getUnrollingPreferences(L, SE, UP); } + +void R600TTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + CommonTTI.getPeelingPreferences(L, SE, PP); +} diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h index b8a027c79bfc23..3364a9bcaccbba 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h @@ -61,6 +61,9 @@ class AMDGPUTTIImpl final : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); }; class GCNTTIImpl final : public BasicTTIImplBase { @@ -146,6 +149,9 @@ class GCNTTIImpl final : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); + TTI::PopcntSupportKind getPopcntSupport(unsigned TyWidth) { assert(isPowerOf2_32(TyWidth) && "Ty width must be power of 2"); return TTI::PSK_FastHardware; @@ -264,6 +270,8 @@ class R600TTIImpl final : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); unsigned getHardwareNumberOfRegisters(bool Vec) const; unsigned getNumberOfRegisters(bool Vec) const; unsigned getRegisterBitWidth(bool Vector) const; diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp index 44dfb9e8c1296e..74b1331216a05a 100644 --- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp +++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp @@ -1582,6 +1582,11 @@ void ARMTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, UP.Force = true; } +void ARMTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} + bool ARMTTIImpl::useReductionIntrinsic(unsigned Opcode, Type *Ty, TTI::ReductionFlags Flags) const { return ST->hasMVEIntegerOps(); diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.h b/llvm/lib/Target/ARM/ARMTargetTransformInfo.h index 5d914227c96813..537a546361eebe 100644 --- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.h +++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.h @@ -251,6 +251,8 @@ class ARMTTIImpl : public BasicTTIImplBase { bool emitGetActiveLaneMask() const; + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); bool shouldBuildLookupTablesForConstant(Constant *C) const { // In the ROPI and RWPI relocation models we can't have pointers to global // variables or functions in constant data, so don't convert switches to diff --git a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp index 76df4e8e19314f..80c8736cb74a06 100644 --- a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp +++ b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp @@ -78,12 +78,17 @@ HexagonTTIImpl::getPopcntSupport(unsigned IntTyWidthInBit) const { void HexagonTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP) { UP.Runtime = UP.Partial = true; +} + +void HexagonTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); // Only try to peel innermost loops with small runtime trip counts. if (L && L->empty() && canPeel(L) && SE.getSmallConstantTripCount(L) == 0 && SE.getSmallConstantMaxTripCount(L) > 0 && SE.getSmallConstantMaxTripCount(L) <= 5) { - UP.PeelCount = 2; + PP.PeelCount = 2; } } diff --git a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h index 3365c5bf1cb14c..5fe397486402e9 100644 --- a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h +++ b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h @@ -64,6 +64,9 @@ class HexagonTTIImpl : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); + /// Bias LSR towards creating post-increment opportunities. bool shouldFavorPostInc() const; diff --git a/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp b/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp index 5c14d0f1a24d52..3873c73fb2e03b 100644 --- a/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp @@ -155,3 +155,8 @@ void NVPTXTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, UP.Partial = UP.Runtime = true; UP.PartialThreshold = UP.Threshold / 4; } + +void NVPTXTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} diff --git a/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h b/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h index 88156f68728478..cb832031f1add9 100644 --- a/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h +++ b/llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h @@ -95,6 +95,10 @@ class NVPTXTTIImpl : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); + bool hasVolatileVariant(Instruction *I, unsigned AddrSpace) { // Volatile loads/stores are only supported for shared and global address // spaces, or for generic AS that maps to them. diff --git a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp index f2c746a1429975..53556ffc267df9 100644 --- a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp +++ b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp @@ -568,6 +568,10 @@ void PPCTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, BaseT::getUnrollingPreferences(L, SE, UP); } +void PPCTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} // This function returns true to allow using coldcc calling convention. // Returning true results in coldcc being used for functions which are cold at // all call sites when the callers of the functions are not calling any other diff --git a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h index b831789d3e6e89..d998521084e1c0 100644 --- a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h +++ b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h @@ -66,6 +66,8 @@ class PPCTTIImpl : public BasicTTIImplBase { TargetLibraryInfo *LibInfo); void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); bool isLSRCostLess(TargetTransformInfo::LSRCost &C1, TargetTransformInfo::LSRCost &C2); diff --git a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp index 36141426e27d90..864200e5f71cc4 100644 --- a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp +++ b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp @@ -294,6 +294,10 @@ void SystemZTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, UP.Force = true; } +void SystemZTTIImpl::getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP) { + BaseT::getPeelingPreferences(L, SE, PP); +} bool SystemZTTIImpl::isLSRCostLess(TargetTransformInfo::LSRCost &C1, TargetTransformInfo::LSRCost &C2) { diff --git a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h index d20541774da1ba..7f8f7f6f923fff 100644 --- a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h +++ b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h @@ -50,6 +50,9 @@ class SystemZTTIImpl : public BasicTTIImplBase { void getUnrollingPreferences(Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP); + void getPeelingPreferences(Loop *L, ScalarEvolution &SE, + TTI::PeelingPreferences &PP); + bool isLSRCostLess(TargetTransformInfo::LSRCost &C1, TargetTransformInfo::LSRCost &C2); /// @} diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp index f0ece1faa5fd5c..285cba6ee2054d 100644 --- a/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp +++ b/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp @@ -158,7 +158,8 @@ static bool computeUnrollAndJamCount( const SmallPtrSetImpl &EphValues, OptimizationRemarkEmitter *ORE, unsigned OuterTripCount, unsigned OuterTripMultiple, unsigned OuterLoopSize, unsigned InnerTripCount, - unsigned InnerLoopSize, TargetTransformInfo::UnrollingPreferences &UP) { + unsigned InnerLoopSize, TargetTransformInfo::UnrollingPreferences &UP, + TargetTransformInfo::PeelingPreferences &PP) { // First up use computeUnrollCount from the loop unroller to get a count // for unrolling the outer loop, plus any loops requiring explicit // unrolling we leave to the unroller. This uses UP.Threshold / @@ -168,7 +169,8 @@ static bool computeUnrollAndJamCount( bool UseUpperBound = false; bool ExplicitUnroll = computeUnrollCount( L, TTI, DT, LI, SE, EphValues, ORE, OuterTripCount, MaxTripCount, - /*MaxOrZero*/ false, OuterTripMultiple, OuterLoopSize, UP, UseUpperBound); + /*MaxOrZero*/ false, OuterTripMultiple, OuterLoopSize, UP, PP, + UseUpperBound); if (ExplicitUnroll || UseUpperBound) { // If the user explicitly set the loop as unrolled, dont UnJ it. Leave it // for the unroller instead. @@ -282,7 +284,9 @@ tryToUnrollAndJamLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, OptimizationRemarkEmitter &ORE, int OptLevel) { TargetTransformInfo::UnrollingPreferences UP = gatherUnrollingPreferences(L, SE, TTI, nullptr, nullptr, OptLevel, None, - None, None, None, None, None, None, None); + None, None, None, None, None); + TargetTransformInfo::PeelingPreferences PP = + gatherPeelingPreferences(L, SE, TTI, None, None); if (AllowUnrollAndJam.getNumOccurrences() > 0) UP.UnrollAndJam = AllowUnrollAndJam; if (UnrollAndJamThreshold.getNumOccurrences() > 0) @@ -367,7 +371,7 @@ tryToUnrollAndJamLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, // Decide if, and by how much, to unroll bool IsCountSetExplicitly = computeUnrollAndJamCount( L, SubLoop, TTI, DT, LI, SE, EphValues, &ORE, OuterTripCount, - OuterTripMultiple, OuterLoopSize, InnerTripCount, InnerLoopSize, UP); + OuterTripMultiple, OuterLoopSize, InnerTripCount, InnerLoopSize, UP, PP); if (UP.Count <= 1) return LoopUnrollResult::Unmodified; // Unroll factor (Count) must be less or equal to TripCount. diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp index ec56610e41e53f..87f40bb7ba8528 100644 --- a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp +++ b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp @@ -193,9 +193,7 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI, int OptLevel, Optional UserThreshold, Optional UserCount, Optional UserAllowPartial, Optional UserRuntime, - Optional UserUpperBound, Optional UserAllowPeeling, - Optional UserAllowProfileBasedPeeling, - Optional UserFullUnrollMaxCount) { + Optional UserUpperBound, Optional UserFullUnrollMaxCount) { TargetTransformInfo::UnrollingPreferences UP; // Set up the defaults @@ -206,7 +204,6 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( UP.PartialThreshold = 150; UP.PartialOptSizeThreshold = 0; UP.Count = 0; - UP.PeelCount = 0; UP.DefaultUnrollRuntimeCount = 8; UP.MaxCount = std::numeric_limits::max(); UP.FullUnrollMaxCount = std::numeric_limits::max(); @@ -218,10 +215,7 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( UP.AllowExpensiveTripCount = false; UP.Force = false; UP.UpperBound = false; - UP.AllowPeeling = true; - UP.AllowLoopNestsPeeling = false; UP.UnrollAndJam = false; - UP.PeelProfiledIterations = true; UP.UnrollAndJamInnerLoopThreshold = 60; UP.MaxIterationsCountToAnalyze = UnrollMaxIterationsCountToAnalyze; @@ -249,8 +243,6 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( UP.MaxCount = UnrollMaxCount; if (UnrollFullMaxCount.getNumOccurrences() > 0) UP.FullUnrollMaxCount = UnrollFullMaxCount; - if (UnrollPeelCount.getNumOccurrences() > 0) - UP.PeelCount = UnrollPeelCount; if (UnrollAllowPartial.getNumOccurrences() > 0) UP.Partial = UnrollAllowPartial; if (UnrollAllowRemainder.getNumOccurrences() > 0) @@ -259,10 +251,6 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( UP.Runtime = UnrollRuntime; if (UnrollMaxUpperBound == 0) UP.UpperBound = false; - if (UnrollAllowPeeling.getNumOccurrences() > 0) - UP.AllowPeeling = UnrollAllowPeeling; - if (UnrollAllowLoopNestsPeeling.getNumOccurrences() > 0) - UP.AllowLoopNestsPeeling = UnrollAllowLoopNestsPeeling; if (UnrollUnrollRemainder.getNumOccurrences() > 0) UP.UnrollRemainder = UnrollUnrollRemainder; if (UnrollMaxIterationsCountToAnalyze.getNumOccurrences() > 0) @@ -281,16 +269,45 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences( UP.Runtime = *UserRuntime; if (UserUpperBound.hasValue()) UP.UpperBound = *UserUpperBound; - if (UserAllowPeeling.hasValue()) - UP.AllowPeeling = *UserAllowPeeling; - if (UserAllowProfileBasedPeeling.hasValue()) - UP.PeelProfiledIterations = *UserAllowProfileBasedPeeling; if (UserFullUnrollMaxCount.hasValue()) UP.FullUnrollMaxCount = *UserFullUnrollMaxCount; return UP; } +TargetTransformInfo::PeelingPreferences +llvm::gatherPeelingPreferences(Loop *L, ScalarEvolution &SE, + const TargetTransformInfo &TTI, + Optional UserAllowPeeling, + Optional UserAllowProfileBasedPeeling) { + TargetTransformInfo::PeelingPreferences PP; + + // Default values + PP.PeelCount = 0; + PP.AllowPeeling = true; + PP.AllowLoopNestsPeeling = false; + PP.PeelProfiledIterations = true; + + // Get Target Specifc Values + TTI.getPeelingPreferences(L, SE, PP); + + // User Specified Values using cl::opt + if (UnrollPeelCount.getNumOccurrences() > 0) + PP.PeelCount = UnrollPeelCount; + if (UnrollAllowPeeling.getNumOccurrences() > 0) + PP.AllowPeeling = UnrollAllowPeeling; + if (UnrollAllowLoopNestsPeeling.getNumOccurrences() > 0) + PP.AllowLoopNestsPeeling = UnrollAllowLoopNestsPeeling; + + // User Specifed values provided by argument + if (UserAllowPeeling.hasValue()) + PP.AllowPeeling = *UserAllowPeeling; + if (UserAllowProfileBasedPeeling.hasValue()) + PP.PeelProfiledIterations = *UserAllowProfileBasedPeeling; + + return PP; +} + namespace { /// A struct to densely store the state of an instruction after unrolling at @@ -761,7 +778,8 @@ bool llvm::computeUnrollCount( ScalarEvolution &SE, const SmallPtrSetImpl &EphValues, OptimizationRemarkEmitter *ORE, unsigned &TripCount, unsigned MaxTripCount, bool MaxOrZero, unsigned &TripMultiple, unsigned LoopSize, - TargetTransformInfo::UnrollingPreferences &UP, bool &UseUpperBound) { + TargetTransformInfo::UnrollingPreferences &UP, + TargetTransformInfo::PeelingPreferences &PP, bool &UseUpperBound) { // Check for explicit Count. // 1st priority is unroll count set by "unroll-count" option. @@ -863,8 +881,8 @@ bool llvm::computeUnrollCount( } // 4th priority is loop peeling. - computePeelCount(L, LoopSize, UP, TripCount, SE); - if (UP.PeelCount) { + computePeelCount(L, LoopSize, UP, PP, TripCount, SE); + if (PP.PeelCount) { UP.Runtime = false; UP.Count = 1; return ExplicitUnroll; @@ -1067,8 +1085,9 @@ static LoopUnrollResult tryToUnrollLoop( TargetTransformInfo::UnrollingPreferences UP = gatherUnrollingPreferences( L, SE, TTI, BFI, PSI, OptLevel, ProvidedThreshold, ProvidedCount, ProvidedAllowPartial, ProvidedRuntime, ProvidedUpperBound, - ProvidedAllowPeeling, ProvidedAllowProfileBasedPeeling, ProvidedFullUnrollMaxCount); + TargetTransformInfo::PeelingPreferences PP = gatherPeelingPreferences( + L, SE, TTI, ProvidedAllowPeeling, ProvidedAllowProfileBasedPeeling); // Exit early if unrolling is disabled. For OptForSize, we pick the loop size // as threshold later on. @@ -1142,7 +1161,7 @@ static LoopUnrollResult tryToUnrollLoop( bool UseUpperBound = false; bool IsCountSetExplicitly = computeUnrollCount( L, TTI, DT, LI, SE, EphValues, &ORE, TripCount, MaxTripCount, MaxOrZero, - TripMultiple, LoopSize, UP, UseUpperBound); + TripMultiple, LoopSize, UP, PP, UseUpperBound); if (!UP.Count) return LoopUnrollResult::Unmodified; // Unroll factor (Count) must be less or equal to TripCount. @@ -1157,7 +1176,7 @@ static LoopUnrollResult tryToUnrollLoop( LoopUnrollResult UnrollResult = UnrollLoop( L, {UP.Count, TripCount, UP.Force, UP.Runtime, UP.AllowExpensiveTripCount, - UseUpperBound, MaxOrZero, TripMultiple, UP.PeelCount, UP.UnrollRemainder, + UseUpperBound, MaxOrZero, TripMultiple, PP.PeelCount, UP.UnrollRemainder, ForgetAllSCEV}, LI, &SE, &DT, &AC, &TTI, &ORE, PreserveLCSSA, &RemainderLoop); if (UnrollResult == LoopUnrollResult::Unmodified) @@ -1189,7 +1208,7 @@ static LoopUnrollResult tryToUnrollLoop( // If the loop was peeled, we already "used up" the profile information // we had, so we don't want to unroll or peel again. if (UnrollResult != LoopUnrollResult::FullyUnrolled && - (IsCountSetExplicitly || (UP.PeelProfiledIterations && UP.PeelCount))) + (IsCountSetExplicitly || (PP.PeelProfiledIterations && PP.PeelCount))) L->setLoopAlreadyUnrolled(); return UnrollResult; diff --git a/llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp b/llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp index 43dfaf3e50dc9c..c653aacbee6cc2 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp @@ -279,19 +279,20 @@ static unsigned countToEliminateCompares(Loop &L, unsigned MaxPeelCount, // Return the number of iterations we want to peel off. void llvm::computePeelCount(Loop *L, unsigned LoopSize, TargetTransformInfo::UnrollingPreferences &UP, + TargetTransformInfo::PeelingPreferences &PP, unsigned &TripCount, ScalarEvolution &SE) { assert(LoopSize > 0 && "Zero loop size is not allowed!"); - // Save the UP.PeelCount value set by the target in - // TTI.getUnrollingPreferences or by the flag -unroll-peel-count. - unsigned TargetPeelCount = UP.PeelCount; - UP.PeelCount = 0; + // Save the PP.PeelCount value set by the target in + // TTI.getPeelingPreferences or by the flag -unroll-peel-count. + unsigned TargetPeelCount = PP.PeelCount; + PP.PeelCount = 0; if (!canPeel(L)) return; // Only try to peel innermost loops by default. // The constraint can be relaxed by the target in TTI.getUnrollingPreferences // or by the flag -unroll-allow-loop-nests-peeling. - if (!UP.AllowLoopNestsPeeling && !L->empty()) + if (!PP.AllowLoopNestsPeeling && !L->empty()) return; // If the user provided a peel count, use that. @@ -299,13 +300,13 @@ void llvm::computePeelCount(Loop *L, unsigned LoopSize, if (UserPeelCount) { LLVM_DEBUG(dbgs() << "Force-peeling first " << UnrollForcePeelCount << " iterations.\n"); - UP.PeelCount = UnrollForcePeelCount; - UP.PeelProfiledIterations = true; + PP.PeelCount = UnrollForcePeelCount; + PP.PeelProfiledIterations = true; return; } // Skip peeling if it's disabled. - if (!UP.AllowPeeling) + if (!PP.AllowPeeling) return; unsigned AlreadyPeeled = 0; @@ -354,8 +355,8 @@ void llvm::computePeelCount(Loop *L, unsigned LoopSize, LLVM_DEBUG(dbgs() << "Peel " << DesiredPeelCount << " iteration(s) to turn" << " some Phis into invariants.\n"); - UP.PeelCount = DesiredPeelCount; - UP.PeelProfiledIterations = false; + PP.PeelCount = DesiredPeelCount; + PP.PeelProfiledIterations = false; return; } } @@ -367,7 +368,7 @@ void llvm::computePeelCount(Loop *L, unsigned LoopSize, return; // Do not apply profile base peeling if it is disabled. - if (!UP.PeelProfiledIterations) + if (!PP.PeelProfiledIterations) return; // If we don't know the trip count, but have reason to believe the average // trip count is low, peeling should be beneficial, since we will usually @@ -387,7 +388,7 @@ void llvm::computePeelCount(Loop *L, unsigned LoopSize, (LoopSize * (*PeelCount + 1) <= UP.Threshold)) { LLVM_DEBUG(dbgs() << "Peeling first " << *PeelCount << " iterations.\n"); - UP.PeelCount = *PeelCount; + PP.PeelCount = *PeelCount; return; } LLVM_DEBUG(dbgs() << "Requested peel count: " << *PeelCount << "\n"); From a0b549602612fa2577068bcdcae3bfbc6c9c3264 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 10 Jul 2020 20:58:09 +0200 Subject: [PATCH 08/14] [PredicateInfo] Add test for multiple branches on same condition (NFC) This illustrates a case where RenamedOp does not correspond to the value used in the condition, which it ideally should. --- .../Util/PredicateInfo/branch-on-same-cond.ll | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 llvm/test/Transforms/Util/PredicateInfo/branch-on-same-cond.ll diff --git a/llvm/test/Transforms/Util/PredicateInfo/branch-on-same-cond.ll b/llvm/test/Transforms/Util/PredicateInfo/branch-on-same-cond.ll new file mode 100644 index 00000000000000..7cf52d1bed3c7a --- /dev/null +++ b/llvm/test/Transforms/Util/PredicateInfo/branch-on-same-cond.ll @@ -0,0 +1,64 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -S -print-predicateinfo < %s 2>&1 >/dev/null | FileCheck %s + +; FIXME: RenamedOp should be %cmp or %x in all cases here, +; which is the value used in the condition. +define i32 @test(i32 %x) { +; CHECK-LABEL: @test( +; CHECK-NEXT: entry: +; CHECK-NEXT: br label [[BB1:%.*]] +; CHECK: bb1: +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[X:%.*]], 0 +; CHECK: RenamedOp: [[CMP]] +; CHECK: [[CMP_0:%.*]] = call i1 @llvm.ssa.copy.{{.*}}(i1 [[CMP]]) +; CHECK: RenamedOp: [[X]] +; CHECK: [[X_0:%.*]] = call i32 @llvm.ssa.copy.{{.*}}(i32 [[X]]) +; CHECK-NEXT: br i1 [[CMP]], label [[BB2:%.*]], label [[EXIT1:%.*]] +; CHECK: bb2: +; CHECK: RenamedOp: [[CMP_0]] +; CHECK: [[CMP_0_1:%.*]] = call i1 @llvm.ssa.copy.{{.*}}(i1 [[CMP_0]]) +; CHECK: RenamedOp: [[X]] +; CHECK: [[X_0_1:%.*]] = call i32 @llvm.ssa.copy.{{.*}}(i32 [[X_0]]) +; CHECK: RenamedOp: [[X_0]] +; CHECK: [[X_0_4:%.*]] = call i32 @llvm.ssa.copy.{{.*}}(i32 [[X_0]]) +; CHECK-NEXT: br i1 [[CMP_0]], label [[BB3:%.*]], label [[EXIT2:%.*]] +; CHECK: bb3: +; CHECK: RenamedOp: [[X]] +; CHECK: [[X_0_1_2:%.*]] = call i32 @llvm.ssa.copy.{{.*}}(i32 [[X_0_1]]) +; CHECK: RenamedOp: [[X_0_1]] +; CHECK: [[X_0_1_3:%.*]] = call i32 @llvm.ssa.copy.{{.*}}(i32 [[X_0_1]]) +; CHECK-NEXT: br i1 [[CMP_0_1]], label [[EXIT3:%.*]], label [[EXIT4:%.*]] +; CHECK: exit1: +; CHECK-NEXT: ret i32 0 +; CHECK: exit2: +; CHECK-NEXT: ret i32 [[X_0_4]] +; CHECK: exit3: +; CHECK-NEXT: ret i32 [[X_0_1_2]] +; CHECK: exit4: +; CHECK-NEXT: ret i32 [[X_0_1_3]] +; +entry: + br label %bb1 + +bb1: + %cmp = icmp eq i32 %x, 0 + br i1 %cmp, label %bb2, label %exit1 + +bb2: + br i1 %cmp, label %bb3, label %exit2 + +bb3: + br i1 %cmp, label %exit3, label %exit4 + +exit1: + ret i32 0 + +exit2: + ret i32 %x + +exit3: + ret i32 %x + +exit4: + ret i32 %x +} From dafc3106d2069b806a10e072306a2196f1cda585 Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Fri, 10 Jul 2020 13:22:11 -0400 Subject: [PATCH 09/14] [Sema] Emit a -Wformat warning for printf("%s", (void*)p) Its dangerous to assume that the opaque pointer points to a null-terminated string, and this has an easy fix (casting to char*). rdar://62432331 --- clang/lib/AST/FormatString.cpp | 1 - clang/test/Sema/format-strings.c | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp index e9f6b88631af5b..83b952116a5e01 100644 --- a/clang/lib/AST/FormatString.cpp +++ b/clang/lib/AST/FormatString.cpp @@ -419,7 +419,6 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const { QualType pointeeTy = PT->getPointeeType(); if (const BuiltinType *BT = pointeeTy->getAs()) switch (BT->getKind()) { - case BuiltinType::Void: case BuiltinType::Char_U: case BuiltinType::UChar: case BuiltinType::Char_S: diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c index 11acfcd1b9a62d..e8cd478d25126a 100644 --- a/clang/test/Sema/format-strings.c +++ b/clang/test/Sema/format-strings.c @@ -290,8 +290,11 @@ void test11(void *p, char *s) { printf("%0p", p); // expected-warning{{flag '0' results in undefined behavior with 'p' conversion specifier}} printf("%s", s); // no-warning printf("%+s", p); // expected-warning{{flag '+' results in undefined behavior with 's' conversion specifier}} + // expected-warning@-1 {{format specifies type 'char *' but the argument has type 'void *'}} printf("% s", p); // expected-warning{{flag ' ' results in undefined behavior with 's' conversion specifier}} + // expected-warning@-1 {{format specifies type 'char *' but the argument has type 'void *'}} printf("%0s", p); // expected-warning{{flag '0' results in undefined behavior with 's' conversion specifier}} + // expected-warning@-1 {{format specifies type 'char *' but the argument has type 'void *'}} } void test12(char *b) { @@ -707,3 +710,7 @@ void PR30481() { // This caused crashes due to invalid casts. printf(1 > 0); // expected-warning{{format string is not a string literal}} expected-warning{{incompatible integer to pointer conversion}} expected-note@format-strings.c:*{{passing argument to parameter here}} expected-note{{to avoid this}} } + +void test_printf_opaque_ptr(void *op) { + printf("%s", op); // expected-warning{{format specifies type 'char *' but the argument has type 'void *'}} +} From ecfa01e956a49ed349683f1cfcfbbec423114b68 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Fri, 10 Jul 2020 14:47:00 -0400 Subject: [PATCH 10/14] [lldb] on s390x fix override issue Summary: This fixes an override issue by marking a function as const so that the signature maps to the signature of the function in the base class. This is the original error: In file included from /root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp:11: /root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h:79:10: error: 'size_t lldb_private::process_linux::NativeRegisterContextLinux_s390x::GetGPRSize()' marked 'override', but does not override 79 | size_t GetGPRSize() override { return sizeof(m_regs); } | ^~~~~~~~~~ Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D83580 --- .../Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h index 69714e400113d0..5ed78810a18bdc 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h @@ -76,7 +76,7 @@ class NativeRegisterContextLinux_s390x : public NativeRegisterContextLinux { Status WriteFPR() override; void *GetGPRBuffer() override { return &m_regs; } - size_t GetGPRSize() override { return sizeof(m_regs); } + size_t GetGPRSize() const override { return sizeof(m_regs); } void *GetFPRBuffer() override { return &m_fp_regs; } size_t GetFPRSize() override { return sizeof(m_fp_regs); } From 9ff310d5bfa56bb5f29645e2d2fee12115c3ddb7 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Fri, 10 Jul 2020 14:27:24 -0400 Subject: [PATCH 11/14] AArch64: Fix unused variables --- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp index ec9683a560f8f5..11a8d5def4296f 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp +++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp @@ -885,7 +885,6 @@ bool AArch64CallLowering::lowerTailCall( const auto &Forwards = FuncInfo->getForwardedMustTailRegParms(); // Do the actual argument marshalling. - SmallVector PhysRegs; OutgoingArgHandler Handler(MIRBuilder, MRI, MIB, AssignFnFixed, AssignFnVarArg, true, FPDiff); if (!handleAssignments(MIRBuilder, OutArgs, Handler)) @@ -1003,7 +1002,6 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, TRI->emitReservedArgRegCallError(MF); // Do the actual argument marshalling. - SmallVector PhysRegs; OutgoingArgHandler Handler(MIRBuilder, MRI, MIB, AssignFnFixed, AssignFnVarArg, false); if (!handleAssignments(MIRBuilder, OutArgs, Handler)) From 9bf6354301ac4e1c7a00e4ef46decba38840fe62 Mon Sep 17 00:00:00 2001 From: aartbik Date: Fri, 10 Jul 2020 12:23:03 -0700 Subject: [PATCH 12/14] [mlir] [VectorOps] Allow AXPY to be expressed as special case of OUTERPRODUCT This specialization allows sharing more code where an AXPY follows naturally in cases where an OUTERPRODUCT on a scalar would be generated. Reviewed By: nicolasvasilache Differential Revision: https://reviews.llvm.org/D83453 --- mlir/include/mlir/Dialect/Vector/VectorOps.td | 50 +++++++++++++------ .../Vector/CPU/test-outerproduct-f32.mlir | 24 +++++++++ .../Vector/CPU/test-outerproduct-i64.mlir | 24 +++++++++ mlir/lib/Dialect/Vector/VectorOps.cpp | 42 +++++++++++----- mlir/lib/Dialect/Vector/VectorTransforms.cpp | 47 ++++++++++------- mlir/test/Dialect/Vector/invalid.mlir | 23 ++++++++- .../Vector/vector-contract-transforms.mlir | 47 +++++++++++++++++ 7 files changed, 212 insertions(+), 45 deletions(-) diff --git a/mlir/include/mlir/Dialect/Vector/VectorOps.td b/mlir/include/mlir/Dialect/Vector/VectorOps.td index b205e5a2e28672..57490c3780416c 100644 --- a/mlir/include/mlir/Dialect/Vector/VectorOps.td +++ b/mlir/include/mlir/Dialect/Vector/VectorOps.td @@ -91,7 +91,7 @@ def Vector_ContractionOp : Example: ```mlir - // Simple dot product (K = 0). + // Simple DOT product (K = 0). #contraction_accesses = [ affine_map<(i) -> (i)>, affine_map<(i) -> (i)>, @@ -668,19 +668,36 @@ def Vector_InsertStridedSliceOp : } def Vector_OuterProductOp : - Vector_Op<"outerproduct", [NoSideEffect, SameOperandsAndResultElementType]>, - Arguments<(ins AnyVector:$lhs, AnyVector:$rhs, Variadic:$acc)>, + Vector_Op<"outerproduct", [NoSideEffect, + PredOpTrait<"lhs operand and result have same element type", + TCresVTEtIsSameAsOpBase<0, 0>>, + PredOpTrait<"rhs operand and result have same element type", + TCresVTEtIsSameAsOpBase<0, 1>>]>, + Arguments<(ins AnyVector:$lhs, AnyType:$rhs, Variadic:$acc)>, Results<(outs AnyVector)> { let summary = "vector outerproduct with optional fused add"; let description = [{ - Takes 2 1-D vectors and returns the 2-D vector containing the outer-product. + Takes 2 1-D vectors and returns the 2-D vector containing the outer-product, + as illustrated below: + ``` + outer | [c, d] + ------+------------ + [a, | [ [a*c, a*d], + b] | [b*c, b*d] ] + ``` + This operation also accepts a 1-D vector lhs and a scalar rhs. In this + case a simple AXPY operation is performed, which returns a 1-D vector. + ``` + [a, b] * c = [a*c, b*c] + ``` - An optional extra 2-D vector argument may be specified in which case the - operation returns the sum of the outer-product and the extra vector. In this - multiply-accumulate scenario, the rounding mode is that obtained by - guaranteeing that a fused-multiply add operation is emitted. When lowered to - the LLVMIR dialect, this form emits `llvm.intr.fma`, which is guaranteed to - lower to actual `fma` instructions on x86. + An optional extra vector argument with the same shape as the output + vector may be specified in which case the operation returns the sum of + the outer-product and the extra vector. In this multiply-accumulate + scenario for floating-point arguments, the rounding mode is enforced + by guaranteeing that a fused-multiply add operation is emitted. When + lowered to the LLVMIR dialect, this form emits `llvm.intr.fma`, which + is guaranteed to lower to actual `fma` instructions on x86. Example: @@ -691,6 +708,10 @@ def Vector_OuterProductOp : %3 = vector.outerproduct %0, %1, %2: vector<4xf32>, vector<8xf32>, vector<4x8xf32> return %3: vector<4x8xf32> + + %6 = vector.outerproduct %4, %5: vector<10xf32>, f32 + return %6: vector<10xf32> + ``` }]; let builders = [ @@ -702,12 +723,13 @@ def Vector_OuterProductOp : VectorType getOperandVectorTypeLHS() { return lhs().getType().cast(); } - VectorType getOperandVectorTypeRHS() { - return rhs().getType().cast(); + Type getOperandTypeRHS() { + return rhs().getType(); } VectorType getOperandVectorTypeACC() { - return (llvm::size(acc()) == 0) ? VectorType() : - (*acc().begin()).getType().cast(); + return (llvm::size(acc()) == 0) + ? VectorType() + : (*acc().begin()).getType().cast(); } VectorType getVectorType() { return getResult().getType().cast(); diff --git a/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-f32.mlir b/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-f32.mlir index a6470c656f8cc7..9b86976e490101 100644 --- a/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-f32.mlir +++ b/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-f32.mlir @@ -11,6 +11,8 @@ !vector_type_Y = type vector<3xf32> !vector_type_Z = type vector<2x3xf32> +!vector_type_R = type vector<7xf32> + func @vector_outerproduct_splat_8x8(%fa: f32, %fb: f32, %fc: f32) -> !vector_type_C { %a = splat %fa: !vector_type_A %b = splat %fb: !vector_type_B @@ -33,6 +35,7 @@ func @vector_outerproduct_vec_2x3_acc(%x : !vector_type_X, } func @entry() { + %f0 = constant 0.0: f32 %f1 = constant 1.0: f32 %f2 = constant 2.0: f32 %f3 = constant 3.0: f32 @@ -72,5 +75,26 @@ func @entry() { // // CHECK: ( ( 6, 8, 10 ), ( 12, 16, 20 ) ) + %3 = vector.broadcast %f0 : f32 to !vector_type_R + %4 = vector.insert %f1, %3[1] : f32 into !vector_type_R + %5 = vector.insert %f2, %4[2] : f32 into !vector_type_R + %6 = vector.insert %f3, %5[3] : f32 into !vector_type_R + %7 = vector.insert %f4, %6[4] : f32 into !vector_type_R + %8 = vector.insert %f5, %7[5] : f32 into !vector_type_R + %9 = vector.insert %f10, %8[6] : f32 into !vector_type_R + + %o = vector.broadcast %f1 : f32 to !vector_type_R + + %axpy1 = vector.outerproduct %9, %f2 : !vector_type_R, f32 + %axpy2 = vector.outerproduct %9, %f2, %o : !vector_type_R, f32 + + vector.print %axpy1 : !vector_type_R + vector.print %axpy2 : !vector_type_R + // + // axpy operations: + // + // CHECK: ( 0, 2, 4, 6, 8, 10, 20 ) + // CHECK: ( 1, 3, 5, 7, 9, 11, 21 ) + return } diff --git a/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-i64.mlir b/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-i64.mlir index 12a71d85b3f7ba..c2724aec9b373b 100644 --- a/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-i64.mlir +++ b/mlir/integration_test/Dialect/Vector/CPU/test-outerproduct-i64.mlir @@ -11,6 +11,8 @@ !vector_type_Y = type vector<3xi64> !vector_type_Z = type vector<2x3xi64> +!vector_type_R = type vector<7xi64> + func @vector_outerproduct_splat_8x8(%ia: i64, %ib: i64, %ic: i64) -> !vector_type_C { %a = splat %ia: !vector_type_A %b = splat %ib: !vector_type_B @@ -33,6 +35,7 @@ func @vector_outerproduct_vec_2x3_acc(%x : !vector_type_X, } func @entry() { + %i0 = constant 0: i64 %i1 = constant 1: i64 %i2 = constant 2: i64 %i3 = constant 3: i64 @@ -72,5 +75,26 @@ func @entry() { // // CHECK: ( ( 6, 8, 10 ), ( 12, 16, 20 ) ) + %3 = vector.broadcast %i0 : i64 to !vector_type_R + %4 = vector.insert %i1, %3[1] : i64 into !vector_type_R + %5 = vector.insert %i2, %4[2] : i64 into !vector_type_R + %6 = vector.insert %i3, %5[3] : i64 into !vector_type_R + %7 = vector.insert %i4, %6[4] : i64 into !vector_type_R + %8 = vector.insert %i5, %7[5] : i64 into !vector_type_R + %9 = vector.insert %i10, %8[6] : i64 into !vector_type_R + + %o = vector.broadcast %i1 : i64 to !vector_type_R + + %axpy1 = vector.outerproduct %9, %i2 : !vector_type_R, i64 + %axpy2 = vector.outerproduct %9, %i2, %o : !vector_type_R, i64 + + vector.print %axpy1 : !vector_type_R + vector.print %axpy2 : !vector_type_R + // + // axpy operations: + // + // CHECK: ( 0, 2, 4, 6, 8, 10, 20 ) + // CHECK: ( 1, 3, 5, 7, 9, 11, 21 ) + return } diff --git a/mlir/lib/Dialect/Vector/VectorOps.cpp b/mlir/lib/Dialect/Vector/VectorOps.cpp index cdf09c4a8f6827..ca85625c7e4c86 100644 --- a/mlir/lib/Dialect/Vector/VectorOps.cpp +++ b/mlir/lib/Dialect/Vector/VectorOps.cpp @@ -1203,10 +1203,13 @@ static ParseResult parseOuterProductOp(OpAsmParser &parser, "expected at least 2 operands"); VectorType vLHS = tLHS.dyn_cast(); VectorType vRHS = tRHS.dyn_cast(); - if (!vLHS || !vRHS) - return parser.emitError(parser.getNameLoc(), "expected 2 vector types"); - VectorType resType = VectorType::get({vLHS.getDimSize(0), vRHS.getDimSize(0)}, - vLHS.getElementType()); + if (!vLHS) + return parser.emitError(parser.getNameLoc(), + "expected vector type for operand #1"); + VectorType resType = + vRHS ? VectorType::get({vLHS.getDimSize(0), vRHS.getDimSize(0)}, + vLHS.getElementType()) + : VectorType::get({vLHS.getDimSize(0)}, vLHS.getElementType()); return failure( parser.resolveOperand(operandsInfo[0], tLHS, result.operands) || parser.resolveOperand(operandsInfo[1], tRHS, result.operands) || @@ -1216,19 +1219,32 @@ static ParseResult parseOuterProductOp(OpAsmParser &parser, } static LogicalResult verify(OuterProductOp op) { + Type tRHS = op.getOperandTypeRHS(); VectorType vLHS = op.getOperandVectorTypeLHS(), - vRHS = op.getOperandVectorTypeRHS(), + vRHS = tRHS.dyn_cast(), vACC = op.getOperandVectorTypeACC(), vRES = op.getVectorType(); + if (vLHS.getRank() != 1) return op.emitOpError("expected 1-d vector for operand #1"); - if (vRHS.getRank() != 1) - return op.emitOpError("expected 1-d vector for operand #2"); - if (vRES.getRank() != 2) - return op.emitOpError("expected 2-d vector result"); - if (vLHS.getDimSize(0) != vRES.getDimSize(0)) - return op.emitOpError("expected #1 operand dim to match result dim #1"); - if (vRHS.getDimSize(0) != vRES.getDimSize(1)) - return op.emitOpError("expected #2 operand dim to match result dim #2"); + + if (vRHS) { + // Proper OUTER operation. + if (vRHS.getRank() != 1) + return op.emitOpError("expected 1-d vector for operand #2"); + if (vRES.getRank() != 2) + return op.emitOpError("expected 2-d vector result"); + if (vLHS.getDimSize(0) != vRES.getDimSize(0)) + return op.emitOpError("expected #1 operand dim to match result dim #1"); + if (vRHS.getDimSize(0) != vRES.getDimSize(1)) + return op.emitOpError("expected #2 operand dim to match result dim #2"); + } else { + // An AXPY operation. + if (vRES.getRank() != 1) + return op.emitOpError("expected 1-d vector result"); + if (vLHS.getDimSize(0) != vRES.getDimSize(0)) + return op.emitOpError("expected #1 operand dim to match result dim #1"); + } + if (vACC && vACC != vRES) return op.emitOpError("expected operand #3 of same type as result type"); return success(); diff --git a/mlir/lib/Dialect/Vector/VectorTransforms.cpp b/mlir/lib/Dialect/Vector/VectorTransforms.cpp index ebad34fcd59369..aa5264ae0d33cc 100644 --- a/mlir/lib/Dialect/Vector/VectorTransforms.cpp +++ b/mlir/lib/Dialect/Vector/VectorTransforms.cpp @@ -1262,7 +1262,7 @@ class TransposeOpLowering : public OpRewritePattern { /// %0 = vector.extract %lhs[0] /// %1 = vector.broadcast %0 /// %2 = vector.extract %acc[0] -/// %3 = vector.fma %1, %arg1, %2 +/// %3 = vector.fma %1, %rhs, %2 /// %4 = vector.insert %3, %z[0] /// .. /// %x = vector.insert %.., %..[N-1] @@ -1275,36 +1275,49 @@ class OuterProductOpLowering : public OpRewritePattern { PatternRewriter &rewriter) const override { auto loc = op.getLoc(); - VectorType rhsType = op.getOperandVectorTypeRHS(); + VectorType lhsType = op.getOperandVectorTypeLHS(); + VectorType rhsType = op.getOperandTypeRHS().dyn_cast(); VectorType resType = op.getVectorType(); Type eltType = resType.getElementType(); + bool isInt = eltType.isa(); Value acc = (op.acc().empty()) ? nullptr : op.acc()[0]; + if (!rhsType) { + // Special case: AXPY operation. + Value b = rewriter.create(loc, lhsType, op.rhs()); + rewriter.replaceOp(op, genMult(loc, op.lhs(), b, acc, isInt, rewriter)); + return success(); + } + Value result = rewriter.create(loc, resType, rewriter.getZeroAttr(resType)); for (int64_t d = 0, e = resType.getDimSize(0); d < e; ++d) { auto pos = rewriter.getI64ArrayAttr(d); Value x = rewriter.create(loc, eltType, op.lhs(), pos); - Value b = rewriter.create(loc, rhsType, x); - Value m; - if (acc) { - Value e = rewriter.create(loc, rhsType, acc, pos); - if (eltType.isa()) - m = rewriter.create( - loc, rewriter.create(loc, b, op.rhs()), e); - else - m = rewriter.create(loc, b, op.rhs(), e); - } else { - if (eltType.isa()) - m = rewriter.create(loc, b, op.rhs()); - else - m = rewriter.create(loc, b, op.rhs()); - } + Value a = rewriter.create(loc, rhsType, x); + Value r = nullptr; + if (acc) + r = rewriter.create(loc, rhsType, acc, pos); + Value m = genMult(loc, a, op.rhs(), r, isInt, rewriter); result = rewriter.create(loc, resType, m, result, pos); } rewriter.replaceOp(op, result); return success(); } + +private: + static Value genMult(Location loc, Value x, Value y, Value acc, bool isInt, + PatternRewriter &rewriter) { + if (acc) { + if (isInt) + return rewriter.create(loc, rewriter.create(loc, x, y), + acc); + return rewriter.create(loc, x, y, acc); + } + if (isInt) + return rewriter.create(loc, x, y); + return rewriter.create(loc, x, y); + } }; /// Progressive lowering of ConstantMaskOp. diff --git a/mlir/test/Dialect/Vector/invalid.mlir b/mlir/test/Dialect/Vector/invalid.mlir index 84d596bf512f84..916403800fe151 100644 --- a/mlir/test/Dialect/Vector/invalid.mlir +++ b/mlir/test/Dialect/Vector/invalid.mlir @@ -187,7 +187,7 @@ func @outerproduct_num_operands(%arg0: f32) { // ----- func @outerproduct_non_vector_operand(%arg0: f32) { - // expected-error@+1 {{expected 2 vector types}} + // expected-error@+1 {{expected vector type for operand #1}} %1 = vector.outerproduct %arg0, %arg0 : f32, f32 } @@ -228,6 +228,27 @@ func @outerproduct_operand_2_dim_generic(%arg0: vector<4xf32>, %arg1: vector<8xf // ----- +func @outerproduct_axpy_operand(%arg0: vector<4x8xf32>, %arg1: f32) { + // expected-error@+1 {{expected 1-d vector for operand #1}} + %1 = vector.outerproduct %arg0, %arg1 : vector<4x8xf32>, f32 +} + +// ----- + +func @outerproduct_axpy_result_generic(%arg0: vector<4xf32>, %arg1: f32) { + // expected-error@+1 {{expected 1-d vector result}} + %1 = "vector.outerproduct" (%arg0, %arg1) : (vector<4xf32>, f32) -> (vector<4x8xf32>) +} + +// ----- + +func @outerproduct_axpy_operand_dim_generic(%arg0: vector<8xf32>, %arg1: f32) { + // expected-error@+1 {{expected #1 operand dim to match result dim #1}} + %1 = "vector.outerproduct" (%arg0, %arg1) : (vector<8xf32>, f32) -> (vector<16xf32>) +} + +// ----- + func @outerproduct_operand_3_result_type_generic(%arg0: vector<4xf32>, %arg1: vector<8xf32>, %arg2: vector<4x16xf32>) { // expected-error@+1 {{expected operand #3 of same type as result type}} %1 = "vector.outerproduct" (%arg0, %arg1, %arg2) : (vector<4xf32>, vector<8xf32>, vector<4x16xf32>) -> (vector<4x8xf32>) diff --git a/mlir/test/Dialect/Vector/vector-contract-transforms.mlir b/mlir/test/Dialect/Vector/vector-contract-transforms.mlir index f6f215a506161b..82faadf100e9ad 100644 --- a/mlir/test/Dialect/Vector/vector-contract-transforms.mlir +++ b/mlir/test/Dialect/Vector/vector-contract-transforms.mlir @@ -326,6 +326,53 @@ func @outerproduct_acc_int(%arg0: vector<2xi32>, return %0: vector<2x3xi32> } +// CHECK-LABEL: func @axpy_fp( +// CHECK-SAME: %[[A:.*0]]: vector<16xf32>, +// CHECK-SAME: %[[B:.*1]]: f32) +// CHECK: %[[T0:.*]] = splat %[[B]] : vector<16xf32> +// CHECK: %[[T1:.*]] = mulf %[[A]], %[[T0]] : vector<16xf32> +// CHECK: return %[[T1]] : vector<16xf32> +func @axpy_fp(%arg0: vector<16xf32>, %arg1: f32) -> vector<16xf32> { + %0 = vector.outerproduct %arg0, %arg1: vector<16xf32>, f32 + return %0: vector<16xf32> +} + +// CHECK-LABEL: func @axpy_fp_add( +// CHECK-SAME: %[[A:.*0]]: vector<16xf32>, +// CHECK-SAME: %[[B:.*1]]: f32, +// CHECK-SAME: %[[C:.*2]]: vector<16xf32>) +// CHECK: %[[T0:.*]] = splat %[[B]] : vector<16xf32> +// CHECK: %[[T1:.*]] = vector.fma %[[A]], %[[T0]], %[[C]] : vector<16xf32> +// CHECK: return %[[T1]] : vector<16xf32> +func @axpy_fp_add(%arg0: vector<16xf32>, %arg1: f32, %arg2 : vector<16xf32>) -> vector<16xf32> { + %0 = vector.outerproduct %arg0, %arg1, %arg2: vector<16xf32>, f32 + return %0: vector<16xf32> +} + +// CHECK-LABEL: func @axpy_int( +// CHECK-SAME: %[[A:.*0]]: vector<16xi32>, +// CHECK-SAME: %[[B:.*1]]: i32) +// CHECK: %[[T0:.*]] = splat %[[B]] : vector<16xi32> +// CHECK: %[[T1:.*]] = muli %[[A]], %[[T0]] : vector<16xi32> +// CHECK: return %[[T1]] : vector<16xi32> +func @axpy_int(%arg0: vector<16xi32>, %arg1: i32) -> vector<16xi32> { + %0 = vector.outerproduct %arg0, %arg1: vector<16xi32>, i32 + return %0: vector<16xi32> +} + +// CHECK-LABEL: func @axpy_int_add( +// CHECK-SAME: %[[A:.*0]]: vector<16xi32>, +// CHECK-SAME: %[[B:.*1]]: i32, +// CHECK-SAME: %[[C:.*2]]: vector<16xi32>) +// CHECK: %[[T0:.*]] = splat %[[B]] : vector<16xi32> +// CHECK: %[[T1:.*]] = muli %[[A]], %[[T0]] : vector<16xi32> +// CHECK: %[[T2:.*]] = addi %[[T1]], %[[C]] : vector<16xi32> +// CHECK: return %[[T2]] : vector<16xi32> +func @axpy_int_add(%arg0: vector<16xi32>, %arg1: i32, %arg2: vector<16xi32>) -> vector<16xi32> { + %0 = vector.outerproduct %arg0, %arg1, %arg2: vector<16xi32>, i32 + return %0: vector<16xi32> +} + // CHECK-LABEL: func @transpose23 // CHECK-SAME: %[[A:.*]]: vector<2x3xf32> // CHECK: %[[Z:.*]] = constant dense<0.000000e+00> : vector<3x2xf32> From ea201e83e292f39c3ee7fe8810a348ee98000398 Mon Sep 17 00:00:00 2001 From: David Goldman Date: Thu, 9 Jul 2020 17:32:05 -0400 Subject: [PATCH 13/14] [AST][ObjC] Fix crash when printing invalid objc categories Summary: If no valid interface definition was found previously we would crash. With this change instead we just print `<>` in place of the NULL interface. In the future this could be improved by saving the invalid interface's name and using that. Reviewers: sammccall, gribozavr Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D83513 --- clang/lib/AST/DeclPrinter.cpp | 13 ++++++++-- clang/unittests/AST/DeclPrinterTest.cpp | 34 +++++++++++++++++-------- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 4df6512e6c76c1..2e48b2b46c4dae 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -1374,7 +1374,12 @@ void DeclPrinter::VisitObjCProtocolDecl(ObjCProtocolDecl *PID) { } void DeclPrinter::VisitObjCCategoryImplDecl(ObjCCategoryImplDecl *PID) { - Out << "@implementation " << *PID->getClassInterface() << '(' << *PID <<")\n"; + Out << "@implementation "; + if (const auto *CID = PID->getClassInterface()) + Out << *CID; + else + Out << "<>"; + Out << '(' << *PID << ")\n"; VisitDeclContext(PID, false); Out << "@end"; @@ -1382,7 +1387,11 @@ void DeclPrinter::VisitObjCCategoryImplDecl(ObjCCategoryImplDecl *PID) { } void DeclPrinter::VisitObjCCategoryDecl(ObjCCategoryDecl *PID) { - Out << "@interface " << *PID->getClassInterface(); + Out << "@interface "; + if (const auto *CID = PID->getClassInterface()) + Out << *CID; + else + Out << "<>"; if (auto TypeParams = PID->getTypeParamList()) { PrintObjCTypeParams(TypeParams); } diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp index 018e99237ae947..939c8b52c12c1e 100644 --- a/clang/unittests/AST/DeclPrinterTest.cpp +++ b/clang/unittests/AST/DeclPrinterTest.cpp @@ -76,14 +76,16 @@ ::testing::AssertionResult PrintedDeclMatches(StringRef Code, const std::vector &Args, const DeclarationMatcher &NodeMatch, StringRef ExpectedPrinted, StringRef FileName, - PrintingPolicyModifier PolicyModifier = nullptr) { + PrintingPolicyModifier PolicyModifier = nullptr, + bool AllowError = false) { PrintMatch Printer(PolicyModifier); MatchFinder Finder; Finder.addMatcher(NodeMatch, &Printer); std::unique_ptr Factory( newFrontendActionFactory(&Finder)); - if (!runToolOnCodeWithArgs(Factory->create(), Code, Args, FileName)) + if (!runToolOnCodeWithArgs(Factory->create(), Code, Args, FileName) && + !AllowError) return testing::AssertionFailure() << "Parsing error in \"" << Code.str() << "\""; @@ -170,16 +172,12 @@ PrintedDeclCXX1ZMatches(StringRef Code, const DeclarationMatcher &NodeMatch, "input.cc"); } -::testing::AssertionResult PrintedDeclObjCMatches( - StringRef Code, - const DeclarationMatcher &NodeMatch, - StringRef ExpectedPrinted) { +::testing::AssertionResult +PrintedDeclObjCMatches(StringRef Code, const DeclarationMatcher &NodeMatch, + StringRef ExpectedPrinted, bool AllowError = false) { std::vector Args(1, ""); - return PrintedDeclMatches(Code, - Args, - NodeMatch, - ExpectedPrinted, - "input.m"); + return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.m", + /*PolicyModifier=*/nullptr, AllowError); } } // unnamed namespace @@ -1321,3 +1319,17 @@ TEST(DeclPrinter, TestObjCProtocol2) { namedDecl(hasName("P1")).bind("id"), "@protocol P1\n@end")); } + +TEST(DeclPrinter, TestObjCCategoryInvalidInterface) { + ASSERT_TRUE(PrintedDeclObjCMatches( + "@interface I (Extension) @end", + namedDecl(hasName("Extension")).bind("id"), + "@interface <>(Extension)\n@end", /*AllowError=*/true)); +} + +TEST(DeclPrinter, TestObjCCategoryImplInvalidInterface) { + ASSERT_TRUE(PrintedDeclObjCMatches( + "@implementation I (Extension) @end", + namedDecl(hasName("Extension")).bind("id"), + "@implementation <>(Extension)\n@end", /*AllowError=*/true)); +} From 169c83208f37f8e5539b1386030d9f3e4b15f16a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 10 Jul 2020 12:45:38 -0700 Subject: [PATCH 14/14] [ldb/Reproducers] Add YamlRecorder and MultiProvider This patch does several things that are all closely related: - It introduces a new YamlRecorder as a counterpart to the existing DataRecorder. As the name suggests the former serializes data as yaml while the latter uses raw texts or bytes. - It introduces a new MultiProvider base class which can be backed by either a DataRecorder or a YamlRecorder. - It reimplements the CommandProvider in terms of the new MultiProvider. Finally, it adds unit testing coverage for the MultiProvider, a naive YamlProvider built on top of the new YamlRecorder and the existing MutliLoader. Differential revision: https://reviews.llvm.org/D83441 --- lldb/include/lldb/Utility/Reproducer.h | 90 +++++++++++++++-- lldb/source/API/SBDebugger.cpp | 2 +- lldb/source/Utility/Reproducer.cpp | 35 +------ lldb/unittests/Utility/ReproducerTest.cpp | 118 +++++++++++++++++++++- 4 files changed, 203 insertions(+), 42 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index 5d810460a0c5ba..ab673e5e00192e 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -159,6 +159,9 @@ class WorkingDirectoryProvider : public Provider { static char ID; }; +/// The recorder is a small object handed out by a provider to record data. It +/// is commonly used in combination with a MultiProvider which is meant to +/// record information for multiple instances of the same source of data. class AbstractRecorder { protected: AbstractRecorder(const FileSpec &filename, std::error_code &ec) @@ -181,6 +184,7 @@ class AbstractRecorder { bool m_record; }; +/// Recorder that records its data as text to a file. class DataRecorder : public AbstractRecorder { public: DataRecorder(const FileSpec &filename, std::error_code &ec) @@ -199,24 +203,88 @@ class DataRecorder : public AbstractRecorder { } }; -class CommandProvider : public Provider { +/// Recorder that records its data as YAML to a file. +class YamlRecorder : public AbstractRecorder { +public: + YamlRecorder(const FileSpec &filename, std::error_code &ec) + : AbstractRecorder(filename, ec) {} + + static llvm::Expected> + Create(const FileSpec &filename); + + template void Record(const T &t) { + if (!m_record) + return; + llvm::yaml::Output yout(m_os); + // The YAML traits are defined as non-const because they are used for + // serialization and deserialization. The cast is safe because + // serialization doesn't modify the object. + yout << const_cast(t); + m_os.flush(); + } +}; + +/// The MultiProvider is a provider that hands out recorder which can be used +/// to capture data for different instances of the same object. The recorders +/// can be passed around or stored as an instance member. +/// +/// The Info::file for the MultiProvider contains an index of files for every +/// recorder. Use the MultiLoader to read the index and get the individual +/// files. +template +class MultiProvider : public repro::Provider { +public: + MultiProvider(const FileSpec &directory) : Provider(directory) {} + + T *GetNewRecorder() { + std::size_t i = m_recorders.size() + 1; + std::string filename = (llvm::Twine(V::Info::name) + llvm::Twine("-") + + llvm::Twine(i) + llvm::Twine(".yaml")) + .str(); + auto recorder_or_error = + T::Create(this->GetRoot().CopyByAppendingPathComponent(filename)); + if (!recorder_or_error) { + llvm::consumeError(recorder_or_error.takeError()); + return nullptr; + } + + m_recorders.push_back(std::move(*recorder_or_error)); + return m_recorders.back().get(); + } + + void Keep() override { + std::vector files; + for (auto &recorder : m_recorders) { + recorder->Stop(); + files.push_back(recorder->GetFilename().GetPath()); + } + + FileSpec file = this->GetRoot().CopyByAppendingPathComponent(V::Info::file); + std::error_code ec; + llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text); + if (ec) + return; + llvm::yaml::Output yout(os); + yout << files; + } + + void Discard() override { m_recorders.clear(); } + +private: + std::vector> m_recorders; +}; + +class CommandProvider : public MultiProvider { public: struct Info { static const char *name; static const char *file; }; - CommandProvider(const FileSpec &directory) : Provider(directory) {} - - DataRecorder *GetNewDataRecorder(); - - void Keep() override; - void Discard() override; + CommandProvider(const FileSpec &directory) + : MultiProvider(directory) {} static char ID; - -private: - std::vector> m_data_recorders; }; /// The generator is responsible for the logic needed to generate a @@ -360,6 +428,8 @@ class Reproducer { mutable std::mutex m_mutex; }; +/// Loader for data captured with the MultiProvider. It will read the index and +/// return the path to the files in the index. template class MultiLoader { public: MultiLoader(std::vector files) : m_files(files) {} diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 978d0befa5ff5a..5f62987f37dad1 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -313,7 +313,7 @@ SBError SBDebugger::SetInputFile(SBFile file) { repro::DataRecorder *recorder = nullptr; if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) - recorder = g->GetOrCreate().GetNewDataRecorder(); + recorder = g->GetOrCreate().GetNewRecorder(); FileSP file_sp = file.m_opaque_sp; diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index eb7d96baab572a..7620ab2c389d60 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -272,40 +272,15 @@ DataRecorder::Create(const FileSpec &filename) { return std::move(recorder); } -DataRecorder *CommandProvider::GetNewDataRecorder() { - std::size_t i = m_data_recorders.size() + 1; - std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") + - llvm::Twine(i) + llvm::Twine(".yaml")) - .str(); - auto recorder_or_error = - DataRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename)); - if (!recorder_or_error) { - llvm::consumeError(recorder_or_error.takeError()); - return nullptr; - } - - m_data_recorders.push_back(std::move(*recorder_or_error)); - return m_data_recorders.back().get(); -} - -void CommandProvider::Keep() { - std::vector files; - for (auto &recorder : m_data_recorders) { - recorder->Stop(); - files.push_back(recorder->GetFilename().GetPath()); - } - - FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file); +llvm::Expected> +YamlRecorder::Create(const FileSpec &filename) { std::error_code ec; - llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text); + auto recorder = std::make_unique(std::move(filename), ec); if (ec) - return; - yaml::Output yout(os); - yout << files; + return llvm::errorCodeToError(ec); + return std::move(recorder); } -void CommandProvider::Discard() { m_data_recorders.clear(); } - void VersionProvider::Keep() { FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file); std::error_code ec; diff --git a/lldb/unittests/Utility/ReproducerTest.cpp b/lldb/unittests/Utility/ReproducerTest.cpp index 1ada0b32827cda..5a9dea3450f0ef 100644 --- a/lldb/unittests/Utility/ReproducerTest.cpp +++ b/lldb/unittests/Utility/ReproducerTest.cpp @@ -9,6 +9,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/Error.h" #include "llvm/Testing/Support/Error.h" @@ -31,8 +32,25 @@ class DummyProvider : public repro::Provider { static char ID; }; +class YamlMultiProvider + : public MultiProvider { +public: + struct Info { + static const char *name; + static const char *file; + }; + + YamlMultiProvider(const FileSpec &directory) : MultiProvider(directory) {} + + static char ID; +}; + const char *DummyProvider::Info::name = "dummy"; const char *DummyProvider::Info::file = "dummy.yaml"; +const char *YamlMultiProvider::Info::name = "mutli"; +const char *YamlMultiProvider::Info::file = "mutli.yaml"; +char DummyProvider::ID = 0; +char YamlMultiProvider::ID = 0; class DummyReproducer : public Reproducer { public: @@ -42,7 +60,25 @@ class DummyReproducer : public Reproducer { using Reproducer::SetReplay; }; -char DummyProvider::ID = 0; +struct YamlData { + YamlData() : i(-1) {} + YamlData(int i) : i(i) {} + int i; +}; + +inline bool operator==(const YamlData &LHS, const YamlData &RHS) { + return LHS.i == RHS.i; +} + +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(YamlData) + +namespace llvm { +namespace yaml { +template <> struct MappingTraits { + static void mapping(IO &io, YamlData &Y) { io.mapRequired("i", Y.i); }; +}; +} // namespace yaml +} // namespace llvm TEST(ReproducerTest, SetCapture) { DummyReproducer reproducer; @@ -144,3 +180,83 @@ TEST(GeneratorTest, GetOrCreate) { auto &provider_alt = generator.GetOrCreate(); EXPECT_EQ(&provider, &provider_alt); } + +TEST(GeneratorTest, YamlMultiProvider) { + SmallString<128> root; + std::error_code ec = llvm::sys::fs::createUniqueDirectory("reproducer", root); + ASSERT_FALSE(static_cast(ec)); + + auto cleanup = llvm::make_scope_exit( + [&] { EXPECT_FALSE(llvm::sys::fs::remove_directories(root.str())); }); + + YamlData data0(0); + YamlData data1(1); + YamlData data2(2); + YamlData data3(3); + + { + DummyReproducer reproducer; + EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec(root.str())), Succeeded()); + + auto &generator = *reproducer.GetGenerator(); + auto *provider = generator.Create(); + ASSERT_NE(nullptr, provider); + + auto *recorder = provider->GetNewRecorder(); + ASSERT_NE(nullptr, recorder); + recorder->Record(data0); + recorder->Record(data1); + + recorder = provider->GetNewRecorder(); + ASSERT_NE(nullptr, recorder); + recorder->Record(data2); + recorder->Record(data3); + + generator.Keep(); + } + + { + DummyReproducer reproducer; + EXPECT_THAT_ERROR(reproducer.SetReplay(FileSpec(root.str())), Succeeded()); + + auto &loader = *reproducer.GetLoader(); + std::unique_ptr> multi_loader = + repro::MultiLoader::Create(&loader); + + // Read the first file. + { + llvm::Optional file = multi_loader->GetNextFile(); + EXPECT_TRUE(static_cast(file)); + + auto buffer = llvm::MemoryBuffer::getFile(*file); + EXPECT_TRUE(static_cast(buffer)); + + yaml::Input yin((*buffer)->getBuffer()); + std::vector data; + yin >> data; + + ASSERT_EQ(data.size(), 2U); + EXPECT_THAT(data, testing::ElementsAre(data0, data1)); + } + + // Read the second file. + { + llvm::Optional file = multi_loader->GetNextFile(); + EXPECT_TRUE(static_cast(file)); + + auto buffer = llvm::MemoryBuffer::getFile(*file); + EXPECT_TRUE(static_cast(buffer)); + + yaml::Input yin((*buffer)->getBuffer()); + std::vector data; + yin >> data; + + ASSERT_EQ(data.size(), 2U); + EXPECT_THAT(data, testing::ElementsAre(data2, data3)); + } + + // There is no third file. + llvm::Optional file = multi_loader->GetNextFile(); + EXPECT_FALSE(static_cast(file)); + } +}