This is an archive of the discontinued LLVM Phabricator instance.

Constant propogation through freeze instruction
ClosedPublic

Authored by mgudim on May 29 2023, 12:49 PM.

Details

Summary

The freeze instruction has not been handled by SCCPInstVisitor. This patch adds SCCPInstVisitor::visitFreezeInst(FreezeInst &I) method to handle freeze instructions.

Diff Detail

Event Timeline

mgudim created this revision.May 29 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2023, 12:49 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
mgudim requested review of this revision.May 29 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2023, 12:49 PM
nikic requested changes to this revision.May 29 2023, 1:00 PM
nikic added a subscriber: nikic.

You need to ensure the constant is not potentially undef or poison.

This revision now requires changes to proceed.May 29 2023, 1:00 PM
mgudim added a comment.EditedMay 29 2023, 2:24 PM

@nikic Thanks for taking a look!

My understanding is that if the argument of freeze instruction is undef then V0State.isUnknownOrUndef() will return true and we exit. In the value lattice undef and constant are separate states (llvm-project/llvm/include/llvm/Analysis/ValueLattice.h : 45)
Also, the test checks that freeze instructions with undef argument remain untouched %fr = freeze i32 undef - the test passed with my change.

I have also tested with poison: the instruction %fr = freeze i32 poison remains untouched. (but we could return any i32 value, according to semantics of freeze) Does this look right? Or am I still misunderstanding?

nikic added a comment.May 30 2023, 1:47 AM

The value lattice only distinguishes literal undef values. You can still have a constant that is potentially poison. Try something like freeze i64 add nuw (i64 ptrtoint (ptr @g to i64), i64 123).

mgudim updated this revision to Diff 526612.May 30 2023, 7:46 AM

Added the check for potentially poison values.

@nikic Thanks for explaining, I understand now. The check was necessary for the test case you suggested. I've updated the diff and included your test case.

nikic accepted this revision.May 30 2023, 8:09 AM

LGTM

This revision is now accepted and ready to land.May 30 2023, 8:09 AM
fhahn accepted this revision.May 30 2023, 8:17 AM
fhahn added a subscriber: fhahn.

LGTM, thanks!

llvm/lib/Transforms/Utils/SCCPSolver.cpp
1415

nit: fold into previous if?

mgudim updated this revision to Diff 526643.May 30 2023, 8:40 AM

Merged nested ifs into one.

mgudim marked an inline comment as done.May 30 2023, 8:42 AM

@fhahn @nikic Can someone please merge this patch for me? I don't have commit rights yet.

mgudim updated this revision to Diff 526699.May 30 2023, 10:15 AM

Build passed.

nikic added a comment.May 31 2023, 1:00 AM

Can you please share which Name <email> to use for attribution on the commit?

Can you please share which Name <email> to use for attribution on the commit?

Mikhail Gudim <mgudim@ventanamicro.com>

This revision was automatically updated to reflect the committed changes.
nikic reopened this revision.Jun 1 2023, 6:32 AM

Reverted due to buildbot failure:

FAILED: lib/TableGen/CMakeFiles/LLVMTableGen.dir/SetTheory.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/lib/TableGen -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/TableGen -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/include -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/include -nostdinc++ -isystem /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/include -isystem /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/include/c++/v1 -fsanitize=undefined -Wl,--rpath=/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/lib -L/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/lib -w -stdlib=libc++ -fPIC -fno
 -semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-class-memaccess -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fno-omit-frame-pointer -gline-tables-only -fsanitize=undefined -fno-sanitize=vptr,function -fno-sanitize-recover=all -fsanitize-blacklist=/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/sanitizers/ubsan_ignorelist.txt -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT lib/TableGen/CMakeFiles/LLVMTableGen.dir/SetTheory.cpp.o -MF lib/TableGen/CMakeFiles/LLVMTableGen.dir/SetTheory.cpp.o.d -o lib/TableGen/CMakeF
 iles/LLVMTableGen.dir/SetTheory.cpp.o -c /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/TableGen/SetTheory.cpp
clang++: /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/Utils/SCCPSolver.cpp:442: llvm::ValueLatticeElement &llvm::SCCPInstVisitor::getValueState(llvm::Value *): Assertion `!V->getType()->isStructTy() && "Should use getStructValueState"' 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:
0.      Program arguments: /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang++ -fsanitize=undefined -L/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/lib -w -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-class-memaccess -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fno-omit-frame-pointer -gline-tables-only -fsanitize=undefined -fno-sanitize=vptr,function -fno-sanitize-recover=all -fsanitize-blacklist=/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/sanitizers/ubsan_ignorelist.txt -ffunction-sections -fdata-sections -O3 -fno-exceptions -funwind-tables -fno-rtt
 i -std=c++17 -fdiagnostics-color -Wl,--rpath=/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/lib -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/lib/TableGen -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/TableGen -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/include -I/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/include -nostdinc++ -isystem /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/include -isystem /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_build_ubsan/include/c++/v1 -stdlib=libc++ -DNDEBUG -UNDEBUG -c -MD -MT lib/TableGen/CMakeFiles/LLVMTableGen.dir/SetTheory.cpp.o -MF lib/TableGen/CMakeFiles/LLVMTableGen.dir/SetTheory.cpp.o.d -fcolor-diagnostics -o lib/TableGen/CMakeFiles/LL
 VMTableGen.dir/SetTheory.cpp.o /b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/TableGen/SetTheory.cpp
1.      <eof> parser at end of file
2.      Optimizer
 #0 0x0000aaaad4f56648 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x7606648)
 #1 0x0000aaaad4f547a8 llvm::sys::RunSignalHandlers() (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x76047a8)
 #2 0x0000aaaad4ec6ab4 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x0000ffffb1d768bc (linux-vdso.so.1+0x8bc)
 #4 0x0000ffffb18610c0 (/lib/aarch64-linux-gnu/libc.so.6+0x810c0)
 #5 0x0000ffffb181ab2c raise (/lib/aarch64-linux-gnu/libc.so.6+0x3ab2c)
 #6 0x0000ffffb18074bc abort (/lib/aarch64-linux-gnu/libc.so.6+0x274bc)
 #7 0x0000ffffb1814434 (/lib/aarch64-linux-gnu/libc.so.6+0x34434)
 #8 0x0000ffffb181449c (/lib/aarch64-linux-gnu/libc.so.6+0x3449c)
 #9 0x0000aaaad62c2634 llvm::SCCPInstVisitor::getValueState(llvm::Value*) SCCPSolver.cpp:0:0
#10 0x0000aaaad62c5664 llvm::SCCPInstVisitor::visitFreezeInst(llvm::FreezeInst&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x8975664)
#11 0x0000aaaad62c9a8c llvm::SCCPInstVisitor::solve() (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x8979a8c)
#12 0x0000aaaad63b56ec llvm::SCCPPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x8a656ec)
#13 0x0000aaaad49cf3ac llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x707f3ac)
#14 0x0000aaaad40c5bd0 llvm::CGSCCToFunctionPassAdaptor::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x6775bd0)
#15 0x0000aaaad40c0b80 llvm::PassManager<llvm::LazyCallGraph::SCC, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x6770b80)
#16 0x0000aaaad40c41a8 llvm::DevirtSCCRepeatedPass::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x67741a8)
#17 0x0000aaaad40c2960 llvm::ModuleToPostOrderCGSCCPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x6772960)
#18 0x0000aaaad49ce6ac llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x707e6ac)
#19 0x0000aaaad61e3a88 llvm::ModuleInlinerWrapperPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x8893a88)
#20 0x0000aaaad49ce6ac llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x707e6ac)
#21 0x0000aaaad564a520 (anonymous namespace)::EmitAssemblyHelper::RunOptimizationPipeline(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>&, std::unique_ptr<llvm::ToolOutputFile, std::default_delete<llvm::ToolOutputFile>>&) BackendUtil.cpp:0:0
#22 0x0000aaaad56430f4 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x7cf30f4)
#23 0x0000aaaad5a45474 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) CodeGenAction.cpp:0:0
#24 0x0000aaaad7039f5c clang::ParseAST(clang::Sema&, bool, bool) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x96e9f5c)
#25 0x0000aaaad5963f8c clang::FrontendAction::Execute() (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x8013f8c)
#26 0x0000aaaad58e8894 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x7f98894)
#27 0x0000aaaad5a3f09c clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x80ef09c)
#28 0x0000aaaad2d3f560 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang+++0x53ef560)
#29 0x0000aaaad2d3b530 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#30 0x0000aaaad57b7b18 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::$_1>(long) Job.cpp:0:0
This revision is now accepted and ready to land.Jun 1 2023, 6:32 AM
mgudim updated this revision to Diff 527962.Jun 2 2023, 1:53 PM

Added a check if type is struct (followed visitSelect as an example)
added test freeze_struct

mgudim added a comment.EditedJun 2 2023, 1:56 PM

there is this failure on windows:

$ ":" "RUN: at line 1"
$ "c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\stage1\bin\flang-new.exe" "-S" "-emit-llvm" "-o" "-" "C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\llvm\flang\test\Driver\compiler_options.f90"
$ "c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\stage1\bin\filecheck.exe" "C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\llvm\flang\test\Driver\compiler_options.f90"
# command stderr:
C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\llvm\flang\test\Driver\compiler_options.f90:3:10: error: CHECK: expected string not found in input
! CHECK: [[OPTSVAR:@_QQcl\.[0-9a-f]+]] = linkonce constant [[[OPTSLEN:[0-9]+]] x i8] c"{{.*}}flang-new{{(\.exe)?}} -S -emit-llvm -o - {{.*}}compiler_options.f90"
         ^
<stdin>:1:1: note: scanning from here
; ModuleID = 'FIRModule'
^

Input file: <stdin>
Check file: C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc-2stage\llvm\flang\test\Driver\compiler_options.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
         1: ; ModuleID = 'FIRModule'
check:3     X~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
         2: source_filename = "FIRModule"
check:3     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         3: target datalayout = "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128"
check:3     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         4: target triple = "aarch64-pc-windows-msvc19.34.31937"
check:3     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         5: 
check:3     ~
         6: @_QQcl.46849559950f708ee332311df8227b62 = internal constant [203 x i8] c"c:\\users\\tcwg\\llvm-worker\\clang-arm64-windows-msvc-2stage\\stage1\\bin\\flang-new.exe -S -emit-llvm -o - C:\\Users\\tcwg\\llvm-worker\\clang-arm64-windows-msvc-2stage\\llvm\\flang\\test\\Driver\\compiler_options.f90"
check:3     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         .
         .
         .
>>>>>>

error: command failed with exit status: 1

Any ideas what this may be? Seems like test is expecting "linkonce" but instead it finds "internal". How can this be related to my change? Was it a known failure? Is there a way to check?

mgudim added a comment.Jun 2 2023, 2:01 PM

Is there a way to trigger all the tests before merging?

This revision was automatically updated to reflect the committed changes.