[mlgo] Initial draft for MLGO on loop unroll #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
2022/06/28 sync-up TODO-s
|
eopXD
force-pushed
the
mlgo-loop-unroll
branch
2 times, most recently
from
55683de to
0bd3e34
Compare
eopXD
force-pushed
the
mlgo-loop-unroll
branch
from
156e472 to
421c2d4
Compare
|
Rebase to latest main. |
eopXD
force-pushed
the
mlgo-loop-unroll
branch
2 times, most recently
from
cf0f4cc to
4896438
Compare
eopXD
force-pushed
the
mlgo-loop-unroll
branch
from
4896438 to
13b36da
Compare
The analysis will be used to extract data from loop, either to log data for training or feed it into the machine learning advisor
…TFUtils::Logger The changes reference MLRegallocEvictAdvisor.
Calculate 2 values characterizing a call graph, starting at an externally-callable (non-local linkage) function. One value is a latency estimator, the other is a icache pressure estimator. They both use basic block frequencies. When traversing the call graph, the MBB frequencies of callees are weighed by the frequency of the callsite, thus, ultimately, the frequencies used are relative to the entrypoint (graph start). The calculations are roughly similar, the difference is that for cache pressure, we treat frequencies larger than 1 as 1 - i.e. probability of execution (if a MBB is in a hot loop, its prob of execution is 1, regardless of how hot the loop is). The values are serialized in a section of the object file. This is because the cache pressure uses actual MBB size in its calculation - which is computed by the assembler. See test/Transforms/Inline/ML/reward-o3.ll and the Inputs/parse_reward.py accompanying it.
- Simplify MLGOLoopUnrollAnalysis::flush - Log the loops correctly under LoopUnrollPass and not in class LoopUnroll - Change log key to $(FUNCTION_NAME)__$(LOOP_NAME), this guarantees unique key - Call flush in dtor of LoopUnrollPass, this lets flush to be only called once
- Move logging into shouldPartialUnroll - Simplify logged feature for now - Minor renaming
Encountered segmentation fault for LoopBound conversion. Remove them for now.
- Create ModelUnderTrainingRunner - Create option to specify model path, log path and mode - Separate training mode from current heuristc calculation
eopXD
force-pushed
the
mlgo-loop-unroll
branch
from
13b36da to
757967c
Compare
eopXD
pushed a commit
that referenced
this pull request
Mar 25, 2023
For example, if you have a chain of inlined funtions like this:
1 #include <stdlib.h>
2 int g1 = 4, g2 = 6;
3
4 static inline void bar(int q) {
5 if (q > 5)
6 abort();
7 }
8
9 static inline void foo(int q) {
10 bar(q);
11 }
12
13 int main() {
14 foo(g1);
15 foo(g2);
16 return 0;
17 }
with optimizations you could end up with a single abort call for the two
inlined instances of foo(). When merging the locations for those inlined
instances you would previously end up with a 0:0 location in main().
Leaving out that inlined chain from the location for the abort call
could make troubleshooting difficult in some cases.
This patch changes DILocation::getMergedLocation() to try to handle such
cases. The function is rewritten to first find a common starting point
for the two locations (same subprogram and inlined-at location), and
then in reverse traverses the inlined-at chain looking for matches in
each subprogram. For each subprogram, the merge function will find the
nearest common scope for the two locations, and matching line and
column (or set them to 0 if not matching).
In the example above, you will for the abort call get a location in
bar() at 6:5, inlined in foo() at 10:3, inlined in main() at 0:0 (since
the two inlined functions are on different lines, but in the same
scope).
I have not seen anything in the DWARF standard that would disallow
inlining a non-zero location at 0:0 in the inlined-at function, and both
LLDB and GDB seem to accept these locations (with D142552 needed for
LLDB to handle cases where the file, line and column number are all 0).
One incompatibility with GDB is that it seems to ignore 0-line locations
in some cases, but I am not aware of any specific issue that this patch
produces related to that.
With x86-64 LLDB (trunk) you previously got:
frame #0: 0x00007ffff7a44930 libc.so.6`abort
frame #1: 0x00005555555546ec a.out`main at merge.c:0
and will now get:
frame #0: 0x[...] libc.so.6`abort
frame #1: 0x[...] a.out`main [inlined] bar(q=<unavailable>) at merge.c:6:5
frame #2: 0x[...] a.out`main [inlined] foo(q=<unavailable>) at merge.c:10:3
frame llvm#3: 0x[...] a.out`main at merge.c:0
and with x86-64 GDB (11.1) you will get:
(gdb) bt
#0 0x00007ffff7a44930 in abort () from /lib64/libc.so.6
#1 0x00005555555546ec in bar (q=<optimized out>) at merge.c:6
#2 foo (q=<optimized out>) at merge.c:10
llvm#3 0x00005555555546ec in main ()
Reviewed By: aprantl, dblaikie
Differential Revision: https://reviews.llvm.org/D142556
eopXD
pushed a commit
that referenced
this pull request
Jun 6, 2023
The motivation for this change is a workload generated by the XLA compiler targeting nvidia GPUs. This kernel has a few hundred i8 loads and stores. Merging is critical for performance. The current LSV doesn't merge these well because it only considers instructions within a block of 64 loads+stores. This limit is necessary to contain the O(n^2) behavior of the pass. I'm hesitant to increase the limit, because this pass is already one of the slowest parts of compiling an XLA program. So we rewrite basically the whole thing to use a new algorithm. Before, we compared every load/store to every other to see if they're consecutive. The insight (from tra@) is that this is redundant. If we know the offset from PtrA to PtrB, then we don't need to compare PtrC to both of them in order to tell whether C may be adjacent to A or B. So that's what we do. When scanning a basic block, we maintain a list of chains, where we know the offset from every element in the chain to the first element in the chain. Each instruction gets compared only to the leaders of all the chains. In the worst case, this is still O(n^2), because all chains might be of length 1. To prevent compile time blowup, we only consider the 64 most recently used chains. Thus we do no more comparisons than before, but we have the potential to make much longer chains. This rewrite affects many tests. The changes to tests fall into two categories. 1. The old code had what appears to be a bug when deciding whether a misaligned vectorized load is fast. Suppose TTI reports that load <i32 x 4> align 4 has relative speed 1, and suppose that load i32 align 4 has relative speed 32. The intent of the code seems to be that we prefer the scalar load, because it's faster. But the old code would choose the vectorized load. accessIsMisaligned would set RelativeSpeed to 0 for the scalar load (and not even call into TTI to get the relative speed), because the scalar load is aligned. After this patch, we will prefer the scalar load if it's faster. 2. This patch changes the logic for how we vectorize. Usually this results in vectorizing more. Explanation of changes to tests: - AMDGPU/adjust-alloca-alignment.ll: #1 - AMDGPU/flat_atomic.ll: #2, we vectorize more. - AMDGPU/int_sideeffect.ll: #2, there are two possible locations for the call to @foo, and the pass is brittle to this. Before, we'd vectorize in case 1 and not case 2. Now we vectorize in case 2 and not case 1. So we just move the call. - AMDGPU/adjust-alloca-alignment.ll: #2, we vectorize more - AMDGPU/insertion-point.ll: #2 we vectorize more - AMDGPU/merge-stores-private.ll: #1 (undoes changes from git rev 86f9117, which appear to have hit the bug from #1) - AMDGPU/multiple_tails.ll: #1 - AMDGPU/vect-ptr-ptr-size-mismatch.ll: Fix alignment (I think related to #1 above). - AMDGPU CodeGen: I have difficulty commenting on these changes, but many of them look like #2, we vectorize more. - NVPTX/4x2xhalf.ll: Fix alignment (I think related to #1 above). - NVPTX/vectorize_i8.ll: We don't generate <3 x i8> vectors on NVPTX because they're not legal (and eventually get split) - X86/correct-order.ll: #2, we vectorize more, probably because of changes to the chain-splitting logic. - X86/subchain-interleaved.ll: #2, we vectorize more - X86/vector-scalar.ll: #2, we can now vectorize scalar float + <1 x float> - X86/vectorize-i8-nested-add-inseltpoison.ll: Deleted the nuw test because it was nonsensical. It was doing `add nuw %v0, -1`, but this is equivalent to `add nuw %v0, 0xffff'ffff`, which is equivalent to asserting that %v0 == 0. - X86/vectorize-i8-nested-add.ll: Same as nested-add-inseltpoison.ll Differential Revision: https://reviews.llvm.org/D149893
eopXD
pushed a commit
that referenced
this pull request
Sep 21, 2023
The new ACLE PR#225[1] now combines the slice parameters for some builtins. This patch is the #2 of 3 patches to update the interface. Slice specifies the ZA slice number directly and needs to be explicity implemented by the "user" with the base register plus the immediate offset [1]https://github.com/ARM-software/acle/pull/225/files
eopXD
pushed a commit
that referenced
this pull request
Nov 14, 2023
…e defintion if available (llvm#71004)" This reverts commit ef3feba. This caused an LLDB test failure on Linux for `lang/cpp/symbols/TestSymbols.test_dwo`: ``` make: Leaving directory '/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/lang/cpp/symbols/TestSymbols.test_dwo' runCmd: expression -- D::i PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. HandleCommand(command = "expression -- D::i") 1. <user expression 0>:1:4: current parser token 'i' 2. <lldb wrapper prefix>:44:1: parsing function body '$__lldb_expr' 3. <lldb wrapper prefix>:44:1: in compound statement ('{}') Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb08b87 1 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb067ae 2 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb0923f 3 libpthread.so.0 0x00007fbd07ab7140 ``` And a failure in `TestCallStdStringFunction.py` on Linux aarch64: ``` -- Exit Code: -11 Command Output (stdout): -- lldb version 18.0.0git (https://github.com/llvm/llvm-project.git revision ef3feba) clang revision ef3feba llvm revision ef3feba -- Command Output (stderr): -- PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. HandleCommand(command = "expression str") 1. <lldb wrapper prefix>:45:34: current parser token ';' 2. <lldb wrapper prefix>:44:1: parsing function body '$__lldb_expr' 3. <lldb wrapper prefix>:44:1: in compound statement ('{}') #0 0x0000ffffb72a149c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c749c) #1 0x0000ffffb729f458 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c5458) #2 0x0000ffffb72a1bd0 SignalHandler(int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c7bd0) llvm#3 0x0000ffffbdd9e7dc (linux-vdso.so.1+0x7dc) llvm#4 0x0000ffffb71799d8 lldb_private::plugin::dwarf::SymbolFileDWARF::FindGlobalVariables(lldb_private::ConstString, lldb_private::CompilerDeclContext const&, unsigned int, lldb_private::VariableList&) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x579f9d8) llvm#5 0x0000ffffb7197508 DWARFASTParserClang::FindConstantOnVariableDefinition(lldb_private::plugin::dwarf::DWARFDIE) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x57bd508) ```
eopXD
pushed a commit
that referenced
this pull request
Nov 14, 2023
The const.cpp testcase fails when running in MSVC mode, while it does
succeed in MinGW mode.
In MSVC mode, there are more constructor invocations than expected, as
the printout looks like this:
A(1), this = 0000025597930000
A(1), this = 0000025597930000
f: this = 0000025597930000, val = 1
A(1), this = 0000025597930000
f: this = 0000025597930000, val = 1
~A, this = 0000025597930000, val = 1
~A, this = 0000025597930000, val = 1
~A, this = 0000025597930000, val = 1
While the expected printout looks like this:
A(1), this = 000002C903E10000
f: this = 000002C903E10000, val = 1
f: this = 000002C903E10000, val = 1
~A, this = 000002C903E10000, val = 1
Reapplying llvm#70991 with the XFAIL changed to check the host triple, not
the target triple. On an MSVC based build of Clang, but with the default
target triple set to PS4/PS5, we will still see the failure. And a Linux
based build of Clang that targets PS4/PS5 won't see the issue.
eopXD
pushed a commit
that referenced
this pull request
Nov 14, 2023
…ooking options for a custom subcommand (llvm#71975) …ooking options for a custom subcommand. (llvm#71776)" This reverts commit b88308b. The build-bot is unhappy (https://lab.llvm.org/buildbot/#/builders/186/builds/13096), `GroupingAndPrefix` fails after `TopLevelOptInSubcommand` (the newly added test). Revert while I look into this (might be related with test sharding but not sure) ``` [----------] 3 tests from CommandLineTest [ RUN ] CommandLineTest.TokenizeWindowsCommandLine2 [ OK ] CommandLineTest.TokenizeWindowsCommandLine2 (0 ms) [ RUN ] CommandLineTest.TopLevelOptInSubcommand [ OK ] CommandLineTest.TopLevelOptInSubcommand (0 ms) [ RUN ] CommandLineTest.GroupingAndPrefix #0 0x00ba8118 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x594118) #1 0x00ba5914 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x591914) #2 0x00ba89c4 SignalHandler(int) (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5949c4) llvm#3 0xf7828530 __default_sa_restorer /build/glibc-9MGTF6/glibc-2.31/signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:67:0 llvm#4 0x00af91f0 (anonymous namespace)::CommandLineParser::ResetAllOptionOccurrences() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e51f0) llvm#5 0x00af8e1c llvm::cl::ResetCommandLineParser() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x4e4e1c) llvm#6 0x0077cda0 (anonymous namespace)::CommandLineTest_GroupingAndPrefix_Test::TestBody() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x168da0) llvm#7 0x00bc5adc testing::Test::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b1adc) llvm#8 0x00bc6cc0 testing::TestInfo::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b2cc0) llvm#9 0x00bc7880 testing::TestSuite::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5b3880) llvm#10 0x00bd7974 testing::internal::UnitTestImpl::RunAllTests() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c3974) llvm#11 0x00bd6ebc testing::UnitTest::Run() (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x5c2ebc) llvm#12 0x00bb1058 main (/home/tcwg-buildbot/worker/clang-armv7-global-isel/stage1/unittests/Support/./SupportTests+0x59d058) llvm#13 0xf78185a4 __libc_start_main /build/glibc-9MGTF6/glibc-2.31/csu/libc-start.c:342:3 ```
eopXD
pushed a commit
that referenced
this pull request
Nov 15, 2023
… functions (llvm#72069) Fixes a bug introduced by commit f95b2f1 ("Reland [InstrProf][compiler-rt] Enable MC/DC Support in LLVM Source-based Code Coverage (1/3)") The InstrProfiling pass was refactored when introducing support for MC/DC such that the creation of the data variable was abstracted and called only once per function from ::run(). Because ::run() only iterated over functions there were not fully inlined, and because it only created the data variable for the first intrinsic that it saw, data variables corresponding to functions fully inlined into other instrumented callers would end up without a data variable, resulting in loss of coverage information. This patch does the following: 1.) Move the call of createDataVariable() to getOrCreateRegionCounters() so that the creation of the data variable will happen indirectly either from ::new() or during profile intrinsic lowering when it is needed. This effectively restores the behavior prior to the refactor and ensures that all data variables are created when needed (and not duplicated). 2.) Process all MC/DC bitmap parameter intrinsics in ::run() prior to calling getOrCreateRegionCounters(). This ensures bitmap regions are created for each function including functions that are fully inlined. It also ensures that the bitmap region is created for each function prior to the creation of the data variable because it is referenced by the data variable. Again, duplication is prevented if the same parameter intrinsic is inlined into multiple functions. 3.) No longer pass the MC/DC intrinsic to createDataVariable(). This decouples the creation of the data variable from a specific MC/DC intrinsic. Instead, with #2 above, store the number of bitmap bytes required in the PerFunctionProfileData in the ProfileDataMap along with the function's CounterRegion and BitmapRegion variables. This ties the bitmap information directly to the function to which it belongs, and the data variable created for that function can reference that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR shows day-to-day development for the initial works for MLGO on loop unroll.
When we validate the workflow, further polishing PR-s will be created.