This is an archive of the discontinued LLVM Phabricator instance.

[TypePromotion] Don't treat bitcast as a Source
ClosedPublic

Authored by dmgreen on Jun 4 2023, 11:18 AM.

Details

Summary

This removes BitCasts from isSource in Type Promotion, as I don't believe they need to be treated as Sources. They will usually be from floats or hoisted constants, where constants will be handled already.

This fixes #62513, but didn't otherwise cause any differences in the tests I ran.

Diff Detail

Event Timeline

dmgreen created this revision.Jun 4 2023, 11:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 11:18 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
dmgreen requested review of this revision.Jun 4 2023, 11:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 11:18 AM
samparker accepted this revision.Jun 5 2023, 3:07 AM

Okay!

This revision is now accepted and ready to land.Jun 5 2023, 3:07 AM
fhahn accepted this revision.Jun 5 2023, 5:39 AM
fhahn added a subscriber: fhahn.

LGTM, thanks for looking into this!

This revision was automatically updated to reflect the committed changes.
srj added a subscriber: srj.Jun 6 2023, 4:42 PM

This appears to have broken arm64 codegen for macOS in Halide:

Assertion failed: (VT.getSizeInBits() == Operand.getValueSizeInBits() && "Cannot BITCAST between types of different sizes!"), function getNode, file SelectionDAG.cpp, line 5715.

I'm investigating to see if this is a latent pre-existing issue in Halide (ie, it just happened to work before) or an actual injection.

I think this is also causing failures in our CI for the Fuchsia Toolchain. I'm bisecting now to confirm, but it seems most likely given the blamelist + the earlier report. Can you revert this until a fix is ready?

See the bot here: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8779118297575483793/overview

Unfortunately our CI isn't set up to capture the reproducer in the runtimes tests correctly, so we can't provide you w/ something easy to reduce.

FAILED: libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o 
/b/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=aarch64-unknown-linux-gnu --sysroot=/b/s/w/ir/x/w/cipd/linux -DLIBCXX_BUILDING_LIBCXXABI -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS="" -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_LINK_PTHREAD_LIB -D_LIBCPP_LINK_RT_LIB -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/s/w/ir/x/w/llvm-llvm-project/libcxx/src -I/b/s/w/ir/x/w/staging/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/compiler-rt/lib/msan/libcxx_msan_aarch64/include/c++/v1 -I/b/s/w/ir/x/w/llvm-llvm-project/libcxxabi/include -fsanitize=memory -fsanitize-memory-track-origins -fno-sanitize-memory-param-retval  -Xclang -fdepfile-entry=/b/s/w/ir/x/w/staging/llvm_build/./lib/../lib/clang/17/share/msan_ignorelist.txt --target=aarch64-unknown-linux-gnu -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++20 -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -MD -MT libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o -MF libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o.d -o libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o -c /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/locale.cpp
Invalid bitcast
  %173 = bitcast <8 x i1> %172 to i32
Invalid bitcast
  %174 = bitcast <8 x i1> %_msprop_icmp_s66 to i32
in function _ZNKSt3__15ctypeIcE9do_narrowEPKcS3_cPc
fatal error: error in backend: Broken function found, compilation aborted!
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/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=aarch64-unknown-linux-gnu --sysroot=/b/s/w/ir/x/w/cipd/linux -DLIBCXX_BUILDING_LIBCXXABI -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS= -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_LINK_PTHREAD_LIB -D_LIBCPP_LINK_RT_LIB -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/s/w/ir/x/w/llvm-llvm-project/libcxx/src -I/b/s/w/ir/x/w/staging/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/compiler-rt/lib/msan/libcxx_msan_aarch64/include/c++/v1 -I/b/s/w/ir/x/w/llvm-llvm-project/libcxxabi/include -fsanitize=memory -fsanitize-memory-track-origins -fno-sanitize-memory-param-retval -Xclang -fdepfile-entry=/b/s/w/ir/x/w/staging/llvm_build/./lib/../lib/clang/17/share/msan_ignorelist.txt --target=aarch64-unknown-linux-gnu -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++20 -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -MD -MT libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o -MF libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o.d -o libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.o -c /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/locale.cpp
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module '/b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/locale.cpp'.
4.	Running pass 'Module Verifier' on function '@_ZNKSt3__15ctypeIcE9do_narrowEPKcS3_cPc'
#0 0x0000aaaac7ed7194 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/b/s/w/ir/x/w/staging/llvm_build/./bin/clang+++0x7927194)
clang++: error: clang frontend command failed with exit code 70 (use -v to see invocation)
Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project c9998ec145972714d581e19fd1c9cb276b836387)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /b/s/w/ir/x/w/staging/llvm_build/./bin
clang++: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang++: note: diagnostic msg: /b/s/w/ir/x/w/staging/llvm_build/clang-crashreports/locale-e001fb.cpp
clang++: note: diagnostic msg: /b/s/w/ir/x/w/staging/llvm_build/clang-crashreports/locale-e001fb.sh
clang++: note: diagnostic msg: 

********************
ninja: build stopped: subcommand failed.
srj added a comment.Jun 6 2023, 5:29 PM

I think this is also causing failures in our CI for the Fuchsia Toolchain. I'm bisecting now to confirm, but it seems most likely given the blamelist + the earlier report. Can you revert this until a fix is ready?

Agreed, this should probably be reverted pending a fix.

paulkirth reopened this revision.Jun 6 2023, 6:10 PM

I've reverted this for now.

This revision is now accepted and ready to land.Jun 6 2023, 6:10 PM

Thanks, I was able to reproduce the problem form the error message. I'll take a look and put up a new version.

dmgreen updated this revision to Diff 529540.Jun 8 2023, 2:32 AM

I've limited bitcasts to only when the input and output types are the same, in order to capture constants but leave other bitcasts alone. This prevents v8i1->i8 bitcasts being promoted. (Also half->i16 but I didn't see those causing a problem, and ptrs will be too large to be promoted).

samparker accepted this revision.Jun 8 2023, 2:38 AM

Cheers Dave.

This revision was automatically updated to reflect the committed changes.