Pass llvm.experimental.noalias.scope.decl to IntrinsicLowering so that it strips out these intrinsics

[operasfantom]

Summary:

New llvm.experimental.noalias.scope.decl intrinsic appeared in LLVM 12.0 which purposes to identify where a noalias scope is declared and accepts metadata as an argument. Execution of that call usually causes an error as shown in the example of stacktrace below:

klee: /mnt/c/Work/klee-simd-runtime/lib/Core/Executor.cpp:1232: const klee::Cell &klee::Executor::eval(klee::KInstruction *, unsigned int, klee::ExecutionState &) const: Assertion `vnumber != -1 && "Invalid operand to eval(), not a value or constant!"' failed.
 #0 0x00007fa241a35ef3 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/lib/x86_64-linux-gnu/libLLVM-12.so.1+0xbd8ef3)
 #1 0x00007fa241a341e2 llvm::sys::RunSignalHandlers() (/lib/x86_64-linux-gnu/libLLVM-12.so.1+0xbd71e2)
 #2 0x00007fa241a3655f (/lib/x86_64-linux-gnu/libLLVM-12.so.1+0xbd955f)
 #3 0x00007fa240960090 (/lib/x86_64-linux-gnu/libc.so.6+0x43090)
 #4 0x00007fa24096000b raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007fa24093f859 abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:81:7
 #6 0x00007fa24093f729 get_sysdep_segment_value /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:509:8
 #7 0x00007fa24093f729 _nl_load_domain /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:970:34
 #8 0x00007fa240950fd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #9 0x000000000044a853 klee::Executor::eval(klee::KInstruction*, unsigned int, klee::ExecutionState&) const /mnt/c/Work/klee-simd-runtime/lib/Core/Executor.cpp:1235:15
#10 0x0000000000452ad3 klee::Executor::executeInstruction(klee::ExecutionState&, klee::KInstruction*) /mnt/c/Work/klee-simd-runtime/lib/Core/Executor.cpp:2411:27
#11 0x000000000045b037 klee::Executor::run(klee::ExecutionState&) /mnt/c/Work/klee-simd-runtime/lib/Core/Executor.cpp:3528:5
#12 0x000000000045e271 klee::Executor::runFunctionAsMain(llvm::Function*, int, char**, char**) /mnt/c/Work/klee-simd-runtime/lib/Core/Executor.cpp:4440:3
#13 0x00000000004274a2 main /mnt/c/Work/klee-simd-runtime/tools/klee/main.cpp:1530:19
#14 0x00007fa240941083 __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:342:3
#15 0x000000000042200e _start (/mnt/c/Work/klee-simd-runtime/cmake-build-debug/bin/klee+0x42200e)
Aborted

Fortunately, IntrinsicLowering is able to strip those out along with assume and var_annotation intrinsics as shown in the snippet of code from llvm/lib/CodeGen/IntrinsicLowering.cpp below:

case Intrinsic::assume:
case Intrinsic::experimental_noalias_scope_decl:
case Intrinsic::var_annotation:
  break;   // Strip out these intrinsics

Checklist:

• [x] The PR addresses a single issue. If it can be divided into multiple independent PRs, please do so.
• [x] The PR is divided into a logical sequence of commits OR a single commit is sufficient.
• [x] There are no unnecessary commits (e.g. commits fixing issues in a previous commit in the same PR).
• [x] Each commit has a meaningful message documenting what it does.
• [x] All messages added to the codebase, all comments, as well as commit messages are spellchecked.
• [x] The code is commented OR not applicable/necessary.
• [x] The patch is formatted via clang-format OR not applicable (if explicitly overridden leave unchecked and explain).
• [x] There are test cases for the code you added or modified OR no such test cases are required.

[codecov[bot]]

Codecov Report

Merging #1542 (204195e) into master (64dfbcd) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1542   +/-   ##
=======================================
  Coverage   70.88%   70.88%           
=======================================
  Files         156      156           
  Lines       18541    18541           
  Branches     4111     4111           
=======================================
  Hits        13142    13142           
  Misses       3592     3592           
  Partials     1807     1807           

Impacted Files	Coverage Δ
lib/Module/IntrinsicCleaner.cpp	91.97% <ø> (ø)

[ccadar]

LGTM, but it would be nice to have a test case in test/Intrinsics (or enhance an existing one in that directory)

[operasfantom]

LGTM, but it would be nice to have a test case in test/Intrinsics (or enhance an existing one in that directory)

Fully agree, it would be.

I considered that, but other missing intrinsics had been added in a single commit with the only test for llvm.minnum.f32. Would it be inaccurate to have a test cast for newly cleared intrinsic and not to have any test for the rest? It would be nice to have separate tests for all of them since we have seen recent changes in IntrinsicLowering class. For my part, I will add a test soon.

[ccadar]

Thanks, @operasfantom . I agree, we haven't done a good job requesting tests for various intrinsics, but it would be useful to have at least one test covering each of them. Thanks for adding a test for this intrinsic.

[operasfantom]

it would be useful to have at least one test covering each of them.

Agree, this may be done later, in a separate follow-up request. So far, due to my busy schedule, the relevant issue may be created and marked with the label "good first issue". What do you think about that, @ccadar?

[ccadar]

Thanks again, @operasfantom , let's merge this!