Skip to content

Conversation

@raaiq1
Copy link
Owner

@raaiq1 raaiq1 commented Jun 28, 2022

No description provided.

Walter Erquinigo and others added 30 commits June 15, 2022 12:07
…lPTProcessTrace class

We have two different "process trace" implementations: per thread and per core. As a way to simplify the collector who uses both, I'm creating a base abstract class that is used by these implementations. This effectively simplify a good chunk of code.

Differential Revision: https://reviews.llvm.org/D125503
… buffer perf_event configuration

We were setting some events to be written in the data buffer of the
perf_event, but we don't need that.

Besides that, we don't need the data buffer to be larger than 1, so we
can reduce its size.

Differential Revision: https://reviews.llvm.org/D125850
… context switch traces

- Add collection of context switches per cpu grouped with the per-cpu intel pt traces.
- Move the state handling from the interl pt trace class to the PerfEvent one.
- Add support for stopping and enabling perf event groups.
- Return context switch entries as part of the jLLDBTraceGetState response.
- Move the triggers of whenever the process stopped or resumed. Now the will-resume notification is in a better location, which will ensure that we'll capture the instructions that will be executed.
- Remove IntelPTSingleBufferTraceUP. The unique pointer was useless.
- Add unit tests

Differential Revision: https://reviews.llvm.org/D125897
…nd tsc information from lldb-server.

- Add a warnings field in the jLLDBGetState response, for warnings to be delivered to the client for troubleshooting. This removes the need to silently log lldb-server's llvm::Errors and not expose them easily to the user
- Simplify the tscPerfZeroConversion struct and schema. It used to extend a base abstract class, but I'm doubting that we'll ever add other conversion mechanisms because all modern kernels support perf zero. It is also the one who is supposed to work with the timestamps produced by the context switch trace, so expecting it is imperative.
- Force tsc collection for cpu tracing.
- Add a test checking that tscPerfZeroConversion is returned by the GetState request
- Add a pre-check for cpu tracing that makes sure that perf zero values are available.

Differential Revision: https://reviews.llvm.org/D125932
… perf conversion in the client

- Add logging for when the live state of the process is refreshed
- Move error handling of the live state refreshing to Trace from TraceIntelPT. This allows refreshing to fail either at the plug-in level or at the base class level. The error is cached and it can be gotten every time RefreshLiveProcessState is invoked.
- Allow DoRefreshLiveProcessState to handle plugin-specific parameters.
- Add some encapsulation to prevent TraceIntelPT from accessing variables belonging to Trace.

Test done via logging:

```
(lldb) b main
Breakpoint 1: where = a.out`main + 20 at main.cpp:27:20, address = 0x00000000004023d9
(lldb) r
Process 2359706 launched: '/home/wallace/a.out' (x86_64)
Process 2359706 stopped
* thread #1, name = 'a.out', stop reason = breakpoint 1.1
    frame #0: 0x00000000004023d9 a.out`main at main.cpp:27:20
   24   };
   25
   26   int main() {
-> 27     std::vector<int> vvv;
   28     for (int i = 0; i < 100000; i++)
   29       vvv.push_back(i);
   30
(lldb) process trace start                                                                                        (lldb) log enable lldb target -F(lldb) n
Process 2359706 stopped
* thread #1, name = 'a.out', stop reason = step over
    frame #0: 0x00000000004023e8 a.out`main at main.cpp:28:12
   25
   26   int main() {
   27     std::vector<int> vvv;
-> 28     for (int i = 0; i < 100000; i++)
   29       vvv.push_back(i);
   30
   31     std::deque<int> dq1 = {1, 2, 3};
(lldb) thread trace dump instructions -c 2 -t                                                                     Trace.cpp:RefreshLiveProcessState                            Trace::RefreshLiveProcessState invoked
TraceIntelPT.cpp:DoRefreshLiveProcessState                   TraceIntelPT found tsc conversion information
thread #1: tid = 2359706
  a.out`std::vector<int, std::allocator<int>>::vector() + 26 at stl_vector.h:395:19
    54: [tsc=unavailable] 0x0000000000403a7c    retq
```

See the logging lines at the end of the dump. They indicate that refreshing happened and that perf conversion information was found.

Differential Revision: https://reviews.llvm.org/D125943
This allows to set printAsJSON through the create function.

Differential Revision: https://reviews.llvm.org/D127891
…nodes are different vector types.

Currently in `combineVectorShuffle()`, we update the shuffle mask if either
input vector comes from a scalar_to_vector, and we keep the respective input
vectors in its permuted form by producing PPCISD::SCALAR_TO_VECTOR_PERMUTED.
However, it is possible that we end up in a situation where both input vectors
to the vector_shuffle are scalar_to_vector, and are different vector types.
In situations like this, the shuffle mask is updated incorrectly as the current
code assumes both scalar_to_vector inputs are the same vector type.

This patch skips the combines for vector_shuffle if both input vectors are
scalar_to_vector, and if they are of different vector types. A follow up patch
will focus on fixing this issue afterwards, in order to correctly update the
shuffle mask.

Differential Revision: https://reviews.llvm.org/D127818
This adds information for DRs 126 through 146.
When an unconnected unit number is used in a BACKSPACE statement
with ERR=, IOSTAT=, &/or IOMSG= control specifiers, don't crash,
but let the program deal with the error.

Differential Revision: https://reviews.llvm.org/D127782
…e trace load and save

:q!
This diff is massive, but it's because it connects the client with lldb-server
and also ensures that the postmortem case works.

- Flatten the postmortem trace schema. The reason is that the schema has become quite complex due to the new multicore case, which defeats the original purpose of having a schema that could work for every trace plug-in. At this point, it's better that each trace plug-in defines it's own full schema. This means that the only common field is "type".
-- Because of this new approach, I merged the "common" trace load and saving functionalities into the IntelPT one. This simplified the code quite a bit. If we eventually implement another trace plug-in, we can see then what we could reuse.
-- The new schema, which is flattened, has now better comments and is parsed better. A change I did was to disallow hex addresses, because they are a bit error prone. I'm asking now to print the address in decimal.
-- Renamed "intel" to "GenuineIntel" in the schema because that's what you see in /proc/cpuinfo.
- Implemented reading the context switch trace data buffer. I had to do
some refactors to do that cleanly.
-- A major change that I did here was to simplify the perf_event circular buffer reading logic. It was too complex. Maybe the original Intel author had something different in mind.
- Implemented all the necessary bits to read trace.json files with per-core data.
- Implemented all the necessary bits to save to disk per-core trace session.
- Added a test that ensures that parsing and saving to disk works.

Differential Revision: https://reviews.llvm.org/D126015
Reviewed By: var-const, Mordante, #libc

Spies: H-G-Hristov, sstefan1, libcxx-commits, mgorny

Differential Revision: https://reviews.llvm.org/D127130
Disable or canonicalize compiler options that are not relevant in
explicit module builds, similar to what we already did for the modules
cache path. This reduces uninteresting differences between
command-lines, which is particularly useful if there is a tool that can
cache the compilations.

Differential Revision: https://reviews.llvm.org/D127883
A REWIND of a unit that's in the middle of a record due to a READ
or WRITE statement with ADVANCE='NO' needs to reset the left tab
limit so that the next transfer takes place at the beginning of
the first record.

Differential Revision: https://reviews.llvm.org/D127783
https://lab.llvm.org/buildbot/#/builders/17/builds/23269 breaks because
we are doing some asm calls that only work on x86

https://lab.llvm.org/buildbot/#/builders/68/builds/34092/steps/6/logs/stdio
breaks because some comparators where being done incorrectly.
Remove the `hasPrototype()` restriction so that old style K&R
declarations of main work too.

For example the following has 2 params but no prototype.

```
int main(argc, argv)
    int argc;
    char *argv[];
{
  return 0;
}
```

Also, use `getNumParams()` over `param_size()` which seems to be a more
direct way to get at the same information.

Also, add missing tests for this mangling.

Differential Revision: https://reviews.llvm.org/D127888
There's code in EditCharacterInput() that causes that template function
to silently return false if it is invoked at the end of the input file.
This overrides other checks that properly call SignalEnd() later.

Differential Revision: https://reviews.llvm.org/D127786
Position inquiries need to account for offsets in records to be
accurate in the case of non-advancing I/O.

Differential Revision: https://reviews.llvm.org/D127789
…ated USEs

When generic resolution finds its specific procedure in a module,
and that specific procedure is not use-associated into the local scope
(perhaps because it was PRIVATE, perhaps because the generic was
use-associated with ONLY:), we create a new use-association with
a renaming.  The name constructed for this renaming needs to be
additionally qualified with the module name of the specific procedure
in order to avoid clashing with another specific of the same name
that may have previously been use-associated in the same way from
a distinct module.

Differential Revision: https://reviews.llvm.org/D127790
I'm emitting "x'y" because the space-separated apostrophes are
misinterpreted as being adjacent repeated quotation marks.
Fix to ensure no space skipping is applied when checking for
repeated quotation marks.

Differential Revision: https://reviews.llvm.org/D127792
… check it

NCOPIES= is currently a std::size_t in the API.  If a negative value is
used, the memory allocation will fail.  Change it to be a signed integer,
and crash with a message instead if it be negative.

Differential Revision: https://reviews.llvm.org/D127795
…ggregate

When translating from a llvm::ConstantAggregate with vector type, we
should lower to insertelement operations (if needed) rather than using
insertvalue.

Differential Revision: https://reviews.llvm.org/D127534
Instead of casting the incoming operand into VectorType to check if it's
scalable or not.
This is the place I missed to fix in f088b99.

Differential Revision: https://reviews.llvm.org/D127535
If any of the operands for ICmpOp is a vector, returns a vector<Nxi1>
, rather than an i1 type result.

Differential Revision: https://reviews.llvm.org/D127536
This reverts commit b10579d.

Make sure that the lldb-target-fuzzer exists before adding the
custom fuzz-lldb-target.
The predicate that determines image control statements needs
to distinguish STOP (which is) from ERROR STOP (which isn't).

Differential Revision: https://reviews.llvm.org/D127796
jinge90 and others added 25 commits June 18, 2022 18:01
…device (#6324)

This patch aims to fix post-commit issue for #6312
Building libimf SYCL device library depends on sycl-compiler

Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: bb-sycl <bb-sycl@intel.com>
* [SYCL] Add inline to get_local_linear_id

These changes make sycl::detail::get_local_linear_id and its
specializations inline.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Co-authored-by: Sergey Semenov <sergey.semenov@intel.com>
The joint_group_conflict test uses compiler flags for the test that are
not recognized when running on Windows. As such, the test is marked as
unsupported for Windows.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
This PR is adds part of the CUDA-backend spec interop proposed in KhronosGroup/SYCL-Docs#197. The changes work with the CUDA CTS interop checks KhronosGroup/SYCL-CTS#336.

This PR just adds the event interop. 

llvm-test-suite: intel/llvm-test-suite#1053
Fixed off-by-one error introduced in #6201 that would cause queue synchronization to synchronize all streams when no stream has been used. The code worked correctly, but this can in some cases impact performance.
DPC++ implementation has been renamed in Khronos SYCL-CTS project from
Intel_SYCL to DPCPP. These instructions are outdated for 10 months
already.

Khronos SYCL-CTS README file provides updated instructions and this
guide already refers to the README file, so removed part duplicates the
information.

Fixed linked to the README section with the test build build and run
instructions.
This patch fix the casting of integer coordinates in image samplers.
This solves #6285, allowing to re-enable the tests disabled in intel/llvm-test-suite#1052.
Uplift GPU RT version for Linux to 22.24.23453

Co-authored-by: GitHub Actions <actions@github.com>
…6242)

* Make esimd implementation of fmod compatible with std::fmod
The driver option -f[no-]sycl-esimd-force-stateless-mem is added.

-fsycl-esimd-force-stateless-mem enables the automatic conversion of stateful memory
accesses via SYCL accessors or surface-index to stateless within ESIMD kernels.
It also disables those ESIMD intrinsics that use stateful accesses that cannot be converted to stateless.
-fsycl-esimd-force-stateless-mem defines the macro __ESIMD_FORCE_STATELESS_MEM
to map the calls of ESIMD API using accessors to calls of API using pointers.
It also passes a switch to sycl-post-link to signal it that it should
ignore the buffer_t attribute and use svmptr_t.

-fno-sycl-esimd-force-stateless-mem is used to tell the compiler not to convert
stateful memory accesses to stateless. Default behavior.

Draft of the design document/proposal for this change-set: #6187
Uplift GPU RT version for Linux to 22.24.23453

Co-authored-by: GitHub Actions <actions@github.com>
This commit adds various PI definitions and replaces some uses of OpenCL header definitions from the SYCL runtime library.

To further isolate the remaining uses of the OpenCL headers, the includes of cl.h are moved from common.hpp to the dependent files.

Tests updates at: intel/llvm-test-suite#1061
Add Windows artifacts cleanup
…s used (#6295)

This revives the patch for #6152 (which was reverted in #6232).

In this patch, the new change is that there is a new target for spir64_fpga and the maximum bitsize limit for that target is 2048.
Unfortunately, for the host, there is now an explicit check in Sema for the same bitsize.

When -fintefpga is specified with -fsycl, we allow a maximum bitsize of 2048.
This patch allows safelen = 0 with ivdep attribute and emits suppressible warning when safelen of 0 or 1 is used

Current behaviour:

ivdep with safelen of 1 is allowed with no warning. Safelen of 0 causes an error.

Desired behaviour:

safelen of 1 and 0 should both be allowed but will have no effect on the loop.

Both should emit a warning. The warning should be suppressible with a -Wno flag.

Justification:

The safelen parameter of an ivdep will sometimes be set based on a template argument.
It makes this type of code much cleaner if the setting of 0 is permitted.

The warning for both the 0 and 1 case is helpful for users who might misunderstand the
meaning of the safelen parameter and set a value of 1 (or possibly 0) thinking it will have some effect.
The warning should be suppressible for those developers using the templated coding pattern described
above who understand and accept that a safelen of 0 will have no effect.

Signed-off-by: Soumi Manna soumi.manna@intel.com
kernel_arg_accessor_ptr metadata is conveying to the BE the
requirement that argument be implemented using a surface.

This is different from information conveyed by metadatas
kernel_arg_exclusive_ptr and kernel_arg_runtime_aligned.

Therefore, we cannot replace kernel_arg_accessor_ptr.

Signed-off-by: Elizabeth Andrews <elizabeth.andrews@intel.com>
Fixed a bug that can cause transfer stream to be reused for compute, messing up the synchronization.
PR #6295 modified Driver/sycl-offload.c incorrectly. It used a triple specific to Linux, and this failed on Windows.
This PR fixes that by checking a wildcard pattern for the other triple values.
* Release notes for commit range f34ba2c..4043dda
* Update known issues:
1. cuda prefetch issue seems to be fixed by:
#5043
2. Performance issues with assert seem to be fixed by:
#4505
#4516
This PR fixes an issue found in the SYCL-CTS cuda interop tests. 
Where the context had not been properly retained when creating a native event with CUDA interop.
According to the spec, sycl::event() constructed via empty constructor should be initialized with the default context. But simply doing so in that constructor is too expensive - many of those events are created temporarily, never needing the context. So instead, we are adding the context lazily, when it might be needed. The performance impact of both approaches has been measured, the simple direct setting of the context on all empty constructed events is deleterious whereas the method here, setting it lazily, has no appreciable degradation on performance.

There are a couple places in the existing codebase where the lack of a context was used as a proxy indicating event state. This has had to change. Also, the setContextImpl routine was not only setting the context, but setting the atomic state to incomplete. This is no longer desirable. I've moved the setting of that state to its own call on the impl, and the appropriate calls now invoke it.
#6314)

Resetting command lists only in synchronization points caused
performance regression because we reuse less command lists and
new command lists are getting created more often which adds
performance overhead.

Return the old behavior back: reset signalled command lists
in the getAvailableCommandList.
@raaiq1 raaiq1 merged commit 6c85afe into raaiq1:sycl Jun 28, 2022
raaiq1 pushed a commit that referenced this pull request Jul 18, 2022
…ned form

The DWARF spec says:

 Any debugging information entry representing the declaration of an object,
 module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and
 DW_AT_decl_column attributes, each of whose value is an unsigned integer
							 ^^^^^^^^
 constant.

If however, a producer happens to emit DW_AT_decl_file /
DW_AT_decl_line using a signed integer form, llvm-dwarfdump crashes,
like so:

     (... snip ...)
     0x000000b4:   DW_TAG_structure_type
                     DW_AT_name      ("test_struct")
                     DW_AT_byte_size (136)
                     DW_AT_decl_file (llvm-dwarfdump: (... snip ...)/llvm/include/llvm/ADT/Optional.h:197: T& llvm::optional_detail::OptionalStorage<T, true>::getValue() &
 [with T = long unsigned int]: Assertion `hasVal' failed.
     PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
     Stack dump:
     0.      Program arguments: /opt/rocm/llvm/bin/llvm-dwarfdump ./testsuite/outputs/gdb.rocm/lane-pc-vega20/lane-pc-vega20-kernel.so
      #0 0x000055cc8e78315f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
      #1 0x000055cc8e780d3d SignalHandler(int) Signals.cpp:0:0
      #2 0x00007f8f2cae8420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
      #3 0x00007f8f2c58d00b raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
      #4 0x00007f8f2c56c859 abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:81:7
      #5 0x00007f8f2c56c729 get_sysdep_segment_value /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:509:8
      #6 0x00007f8f2c56c729 _nl_load_domain /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:970:34
      #7 0x00007f8f2c57dfd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
      #8 0x000055cc8e58ceb9 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2e0eb9)
      #9 0x000055cc8e58bec3 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2dfec3)
     #10 0x000055cc8e5b28a3 llvm::DWARFCompileUnit::dump(llvm::raw_ostream&, llvm::DIDumpOptions) (.part.21) DWARFCompileUnit.cpp:0:0

Likewise with DW_AT_call_file / DW_AT_call_line.

The problem is that the code in llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
dumping these attributes assumes that
FormValue.getAsUnsignedConstant() returns an armed optional.  If in
debug mode, we get an assertion line the above.  If in release mode,
and asserts are compiled out, then we proceed as if the optional had a
value, running into undefined behavior, printing whatever random
value.

Fix this by checking whether the optional returned by
FormValue.getAsUnsignedConstant() has a value, like done in other
places.

In addition, DWARFVerifier.cpp is validating DW_AT_call_file /
DW_AT_decl_file, but not AT_call_line / DW_AT_decl_line.  This commit
fixes that too.

The llvm-dwarfdump/X86/verify_file_encoding.yaml testcase is extended
to cover these cases.  Current llvm-dwarfdump crashes running the
newly-extended test.

"make check-llvm-tools-llvm-dwarfdump" shows no regressions, on x86-64
GNU/Linux.

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D129392
raaiq1 pushed a commit that referenced this pull request Nov 8, 2022
Found by msan -fsanitize-memory-use-after-dtor.

==8259==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55dbec54d2b8 in dtorRecord(clang::interp::Block*, char*, clang::interp::Descriptor*) clang/lib/AST/Interp/Descriptor.cpp:150:22
    #1 0x55dbec54bfcf in dtorArrayDesc(clang::interp::Block*, char*, clang::interp::Descriptor*) clang/lib/AST/Interp/Descriptor.cpp:97:7
    #2 0x55dbec508578 in invokeDtor clang/lib/AST/Interp/InterpBlock.h:79:7
    #3 0x55dbec508578 in clang::interp::Program::~Program() clang/lib/AST/Interp/Program.h:55:19
    #4 0x55dbec50657a in operator() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:55:5
    #5 0x55dbec50657a in std::__msan::unique_ptr<clang::interp::Program, std::__msan::default_delete<clang::interp::Program>>::~unique_ptr() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:261:7
    #6 0x55dbec5035a1 in clang::interp::Context::~Context() clang/lib/AST/Interp/Context.cpp:27:22
    #7 0x55dbebec1daa in operator() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:55:5
    #8 0x55dbebec1daa in std::__msan::unique_ptr<clang::interp::Context, std::__msan::default_delete<clang::interp::Context>>::~unique_ptr() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:261:7
    #9 0x55dbebe285f9 in clang::ASTContext::~ASTContext() clang/lib/AST/ASTContext.cpp:1038:40
    #10 0x55dbe941ff13 in llvm::RefCountedBase<clang::ASTContext>::Release() const llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:101:7
    #11 0x55dbe94353ef in release llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:159:38
    #12 0x55dbe94353ef in release llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:224:7
    #13 0x55dbe94353ef in ~IntrusiveRefCntPtr llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:191:27
    #14 0x55dbe94353ef in clang::CompilerInstance::setASTContext(clang::ASTContext*) clang/lib/Frontend/CompilerInstance.cpp:178:3
    #15 0x55dbe95ad0ad in clang::FrontendAction::EndSourceFile() clang/lib/Frontend/FrontendAction.cpp:1100:8
    #16 0x55dbe9445fcf in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) clang/lib/Frontend/CompilerInstance.cpp:1047:11
    intel#17 0x55dbe6b3afef in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:266:25
    intel#18 0x55dbe6b13288 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) clang/tools/driver/cc1_main.cpp:250:15
    intel#19 0x55dbe6b0095f in ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) clang/tools/driver/driver.cpp:319:12
    intel#20 0x55dbe6aff41c in clang_main(int, char**) clang/tools/driver/driver.cpp:395:12
    intel#21 0x7f9be07fa632 in __libc_start_main
    intel#22 0x55dbe6a702e9 in _start

  Member fields were destroyed
    #0 0x55dbe6a7da5d in __sanitizer_dtor_callback_fields compiler-rt/lib/msan/msan_interceptors.cpp:949:5
    #1 0x55dbec5094ac in ~SmallVectorImpl llvm/include/llvm/ADT/SmallVector.h:479:7
    #2 0x55dbec5094ac in ~SmallVectorImpl llvm/include/llvm/ADT/SmallVector.h:612:3
    #3 0x55dbec5094ac in llvm::SmallVector<clang::interp::Record::Base, 8u>::~SmallVector() llvm/include/llvm/ADT/SmallVector.h:1207:3
    #4 0x55dbec508e79 in clang::interp::Record::~Record() clang/lib/AST/Interp/Record.h:24:7
    #5 0x55dbec508612 in clang::interp::Program::~Program() clang/lib/AST/Interp/Program.h:49:26
    #6 0x55dbec50657a in operator() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:55:5
    #7 0x55dbec50657a in std::__msan::unique_ptr<clang::interp::Program, std::__msan::default_delete<clang::interp::Program>>::~unique_ptr() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:261:7
    #8 0x55dbec5035a1 in clang::interp::Context::~Context() clang/lib/AST/Interp/Context.cpp:27:22
    #9 0x55dbebec1daa in operator() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:55:5
    #10 0x55dbebec1daa in std::__msan::unique_ptr<clang::interp::Context, std::__msan::default_delete<clang::interp::Context>>::~unique_ptr() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:261:7
    #11 0x55dbebe285f9 in clang::ASTContext::~ASTContext() clang/lib/AST/ASTContext.cpp:1038:40
    #12 0x55dbe941ff13 in llvm::RefCountedBase<clang::ASTContext>::Release() const llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:101:7
    #13 0x55dbe94353ef in release llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:159:38
    #14 0x55dbe94353ef in release llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:224:7
    #15 0x55dbe94353ef in ~IntrusiveRefCntPtr llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:191:27
    #16 0x55dbe94353ef in clang::CompilerInstance::setASTContext(clang::ASTContext*) clang/lib/Frontend/CompilerInstance.cpp:178:3
    intel#17 0x55dbe95ad0ad in clang::FrontendAction::EndSourceFile() clang/lib/Frontend/FrontendAction.cpp:1100:8
    intel#18 0x55dbe9445fcf in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) clang/lib/Frontend/CompilerInstance.cpp:1047:11
    intel#19 0x55dbe6b3afef in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:266:25
    intel#20 0x55dbe6b13288 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) clang/tools/driver/cc1_main.cpp:250:15
    intel#21 0x55dbe6b0095f in ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) clang/tools/driver/driver.cpp:319:12
    intel#22 0x55dbe6aff41c in clang_main(int, char**) clang/tools/driver/driver.cpp:395:12
    intel#23 0x7f9be07fa632 in __libc_start_main
    intel#24 0x55dbe6a702e9 in _start
raaiq1 pushed a commit that referenced this pull request Nov 8, 2022
For the following program,
  $ cat t.c
  struct t {
   int (__attribute__((btf_type_tag("rcu"))) *f)();
   int a;
  };
  int foo(struct t *arg) {
    return arg->a;
  }
Compiling with 'clang -g -O2 -S t.c' will cause a failure like below:
  clang: /home/yhs/work/llvm-project/clang/lib/Sema/SemaType.cpp:6391: void {anonymous}::DeclaratorLocFiller::VisitParenTypeLoc(clang::ParenTypeLoc):
         Assertion `Chunk.Kind == DeclaratorChunk::Paren' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  ......
  #5 0x00007f89e4280ea5 abort (/lib64/libc.so.6+0x21ea5)
  #6 0x00007f89e4280d79 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d79)
  #7 0x00007f89e42a6456 (/lib64/libc.so.6+0x47456)
  #8 0x00000000045c2596 GetTypeSourceInfoForDeclarator((anonymous namespace)::TypeProcessingState&, clang::QualType, clang::TypeSourceInfo*) SemaType.cpp:0:0
  #9 0x00000000045ccfa5 GetFullTypeForDeclarator((anonymous namespace)::TypeProcessingState&, clang::QualType, clang::TypeSourceInfo*) SemaType.cpp:0:0
  ......

The reason of the failure is due to the mismatch of TypeLoc and D.getTypeObject().Kind. For example,
the TypeLoc is
  BTFTagAttributedType 0x88614e0 'int  btf_type_tag(rcu)()' sugar
  |-ParenType 0x8861480 'int ()' sugar
  | `-FunctionNoProtoType 0x8861450 'int ()' cdecl
  |   `-BuiltinType 0x87fd500 'int'
while corresponding D.getTypeObject().Kind points to DeclaratorChunk::Paren, and
this will cause later assertion.

To fix the issue, similar to AttributedTypeLoc, let us skip BTFTagAttributedTypeLoc in
GetTypeSourceInfoForDeclarator().

Differential Revision: https://reviews.llvm.org/D136807
raaiq1 pushed a commit that referenced this pull request Dec 20, 2022
The Assignment Tracking debug-info feature is outlined in this RFC:

https://discourse.llvm.org/t/
rfc-assignment-tracking-a-better-way-of-specifying-variable-locations-in-ir

Add initial revision of assignment tracking analysis pass
---------------------------------------------------------
This patch squashes five individually reviewed patches into one:

    #1 https://reviews.llvm.org/D136320
    #2 https://reviews.llvm.org/D136321
    #3 https://reviews.llvm.org/D136325
    #4 https://reviews.llvm.org/D136331
    #5 https://reviews.llvm.org/D136335

Patch #1 introduces 2 new files: AssignmentTrackingAnalysis.h and .cpp. The
two subsequent patches modify those files only. Patch #4 plumbs the analysis
into SelectionDAG, and patch #5 is a collection of tests for the analysis as
a whole.

The analysis was broken up into smaller chunks for review purposes but for the
most part the tests were written using the whole analysis. It would be possible
to break up the tests for patches #1 through #3 for the purpose of landing the
patches seperately. However, most them would require an update for each
patch. In addition, patch #4 - which connects the analysis to SelectionDAG - is
required by all of the tests.

If there is build-bot trouble, we might try a different landing sequence.

Analysis problem and goal
-------------------------

Variables values can be stored in memory, or available as SSA values, or both.
Using the Assignment Tracking metadata, it's not possible to determine a
variable location just by looking at a debug intrinsic in
isolation. Instructions without any metadata can change the location of a
variable. The meaning of dbg.assign intrinsics changes depending on whether
there are linked instructions, and where they are relative to those
instructions. So we need to analyse the IR and convert the embedded information
into a form that SelectionDAG can consume to produce debug variable locations
in MIR.

The solution is a dataflow analysis which, aiming to maximise the memory
location coverage for variables, outputs a mapping of instruction positions to
variable location definitions.

API usage
---------

The analysis is named `AssignmentTrackingAnalysis`. It is added as a required
pass for SelectionDAGISel when assignment tracking is enabled.

The results of the analysis are exposed via `getResults` using the returned
`const FunctionVarLocs *`'s const methods:

    const VarLocInfo *single_locs_begin() const;
    const VarLocInfo *single_locs_end() const;
    const VarLocInfo *locs_begin(const Instruction *Before) const;
    const VarLocInfo *locs_end(const Instruction *Before) const;
    void print(raw_ostream &OS, const Function &Fn) const;

Debug intrinsics can be ignored after running the analysis. Instead, variable
location definitions that occur between an instruction `Inst` and its
predecessor (or block start) can be found by looping over the range:

    locs_begin(Inst), locs_end(Inst)

Similarly, variables with a memory location that is valid for their lifetime
can be iterated over using the range:

    single_locs_begin(), single_locs_end()

Further detail
--------------

For an explanation of the dataflow implementation and the integration with
SelectionDAG, please see the reviews linked at the top of this commit message.

Reviewed By: jmorse
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.