Skip to content

[mlgo] Call-graph reward calculation #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 0 commits into from
Jul 21, 2022
Merged

[mlgo] Call-graph reward calculation #1

merged 0 commits into from
Jul 21, 2022

Conversation

mtrofin
Copy link

@mtrofin mtrofin commented Jun 28, 2022

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.

@eopXD eopXD changed the base branch from main to mlgo-loop-unroll July 21, 2022 16:50
@eopXD
Copy link
Owner

eopXD commented Jul 21, 2022

Thank you for the patch and explanation. Merging this into the mlgo branch and I will follow up with an NFC patch to rename stuffs.

@eopXD eopXD merged this pull request into eopXD:mlgo-loop-unroll Jul 21, 2022
eopXD pushed a commit that referenced this pull request Nov 20, 2022
@c-rhodes
[AArch64][SVE] Fix bad PTEST(PG, OP(PG, ...)) optimization
ce3e7eb 
AArch64InstrInfo::optimizePTestInstr attempts to remove a PTEST of a
predicate generating operation that identically sets flags (implictly).

When the PTEST and the predicate-generating operation use the same mask
the PTEST is currently removed. This is incorrect since it doesn't
consider element size. PTEST operates on 8-bit predicates, but for
instructions like compare that also support 16/32/64-bit predicates, the
implicit PTEST performed by the instruction will consider fewer lanes
for these element sizes and could set different first or last active
flags.

For example, consider the following instruction sequence

  ptrue p0.b			; P0=1111-1111-1111-1111
  index z0.s, #0, #1		; Z0=<0,1,2,3>
  index z1.s, #1, #1		; Z1=<1,2,3,4>
  cmphi p1.s, p0/z, z1.s, z0.s  ; P1=0001-0001-0001-0001
				;       ^ last active
  ptest p0, p1.b		; P1=0001-0001-0001-0001
				;     ^ last active

where the compare generates a canonical all active 32-bit predicate (equivalent
to 'ptrue p1.s, all'). The implicit PTEST sets the last active flag, whereas
the PTEST instruction with the same mask doesn't.

This patch restricts the optimization to instructions operating on 8-bit
predicates. One caveat is the optimization is safe regardless of element
size for any active, this will be addressed in a later patch.

Reviewed By: bsmith

Differential Revision: https://reviews.llvm.org/D137716
eopXD pushed a commit that referenced this pull request Nov 20, 2022
@jayfoad
[GlobalISel] Better verification of G_UNMERGE_VALUES
96a661d 
Verify three cases of G_UNMERGE_VALUES separately:

1. Splitting a vector into subvectors (the converse of
   G_CONCAT_VECTORS).
2. Splitting a vector into its elements (the converse of
   G_BUILD_VECTOR).
3. Splitting a scalar into smaller scalars (the converse of
   G_MERGE_VALUES).

Previously #1 allowed strange combinations like this:
  %1:_(<2 x s16>),%2:_(<2 x s16>) = G_UNMERGE_VALUES %0(<2 x s32>)
This has been tightened up to check that the source and destination
element types match, and some MIR test cases updated accordingly.

Differential Revision: https://reviews.llvm.org/D111132
eopXD pushed a commit that referenced this pull request Mar 25, 2023
@dstenb
[DebugInfo] Merge partially matching chains of inlined locations
12a7aea 
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
@jlebar
Rewrite load-store-vectorizer.
2be0abb 
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
@oontvoo
[lld-macho][nfc]Add bounds on sections and subsections check before a…
595cd45 
…ttempting to dereferencing iterators.

Runnign some tests with asan built of LLD would throw errors similar to the following:

AddressSanitizer:DEADLYSIGNAL
    #0 0x55d8e6da5df7 in operator() /mnt/ssd/repo/lld/llvm-project/lld/MachO/Arch/ARM64.cpp:612
    #1 0x55d8e6daa514 in operator() /mnt/ssd/repo/lld/llvm-project/lld/MachO/Arch/ARM64.cpp:650

Differential Revision: https://reviews.llvm.org/D157027
eopXD pushed a commit that referenced this pull request Nov 14, 2023
@Michael137
Revert "[lldb][DWARFASTParserClang] Fetch constant value from variabl…
5f86b49 
…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
@mingmingl-llvm
Revert "[Support]Look up in top-level subcommand as a fallback when l…
2e912a2 
…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
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mtrofin @eopXD