This is needed for an upcoming change to how we translate conditional branches which might generate these.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
2100–2106 | Can generalize for not-i1 by checking the target boolean contents |
One of the globalisel code quality regressions vs selectiondag that I'm tracking comes from code like this:
%40:_(s1) = G_ICMP intpred(uge), %35:_(s32), %37:_ %41:_(s1) = G_ICMP intpred(uge), %36:_(s32), %38:_ %42:_(s1) = G_OR %40:_, %41:_ %43:_(s1) = G_CONSTANT i1 true %44:sreg_64_xexec(s1) = G_XOR %42:_, %43:_ %46:sreg_64_xexec(s64) = SI_IF %44:sreg_64_xexec(s1), %bb.3, implicit-def $exec, implicit-def $scc, implicit $exec
So... is there any chance of extending this patch to be able to invert a whole tree of comparisons combined with ANDs and ORs?
llvm/lib/CodeGen/GlobalISel/Utils.cpp | ||
---|---|---|
708 | One of these parameters is isVector, which is getting lost here |
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
2153–2165 | I think this isConstTrue logic should be merged into a separate function (at least the getScalarSizeInBits() == 1 case and check TLI for the extended version should be together) | |
2172 | I guess technically TargetBooleanContents also depends on isFloat in the fcmp case |
FYI, I reverted this (and D86413 which depends on it) in 8693ddc74371dedc742c9f3d3e4eda1da72c13ea to keep the build green, as it was causing surprising crashes when running ninja check-llvm-codegen-aarch64-globalisel in release builds.
In case it goes away, the buildbot error is:
FAIL: LLVM :: CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir (33345 of 67618) ******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir' FAILED ******************** Script: -- : 'RUN: at line 2'; /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc -mtriple aarch64-apple-ios -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="not_cmp_fold" /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir -o - -verify-machineinstrs | /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/FileCheck /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir -- Exit Code: 2 Command Output (stderr): -- LLVM ERROR: Invalid rule identifier PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace. Stack dump: 0. Program arguments: /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc -mtriple aarch64-apple-ios -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule=not_cmp_fold /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir -o - -verify-machineinstrs 1. Running pass 'Function Pass Manager' on module '/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir'. 2. Running pass 'AArch64PreLegalizerCombiner' on function '@icmp' #0 0x00005651667a183c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x25cc83c) #1 0x000056516679f694 llvm::sys::RunSignalHandlers() (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x25ca694) #2 0x000056516679f7e3 SignalHandler(int) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x25ca7e3) #3 0x00007f71603c58a0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x128a0) #4 0x00007f715f293f47 raise (/lib/x86_64-linux-gnu/libc.so.6+0x3ef47) #5 0x00007f715f2958b1 abort (/lib/x86_64-linux-gnu/libc.so.6+0x408b1) #6 0x000056516671a1d6 llvm::report_fatal_error(llvm::Twine const&, bool) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x25451d6) #7 0x000056516671a2f8 (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x25452f8) #8 0x0000565164a62751 (anonymous namespace)::AArch64PreLegalizerCombiner::runOnMachineFunction(llvm::MachineFunction&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x88d751) #9 0x0000565165d08b78 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x1b33b78) #10 0x00005651660d9af7 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x1f04af7) #11 0x00005651660da211 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x1f05211) #12 0x00005651660d8e6f llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x1f03e6f) #13 0x000056516491a6e7 compileModule(char**, llvm::LLVMContext&) (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x7456e7) #14 0x0000565164871cf7 main (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x69ccf7) #15 0x00007f715f276b97 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b97) #16 0x000056516491380a _start (/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/llc+0x73e80a) FileCheck error: '<stdin>' is empty. FileCheck command line: /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/build/stage1/bin/FileCheck /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-invert-cmp.mir
Thanks, it seems the test doesn't work without asserts enabled. I've re-committed it fixed in 520ab710fb6f9829b4e70fda1dcc91ed4f614d0a
Thanks!
Usually, tests that are marked # REQUIRES: asserts are done so because they make use of some code that is only available behind NDEBUG. However, in this case, it's outright crashing on certain inputs when not in debug mode. At a first glance, that doesn't seem like a correct fix to me -- after all, the crashing bug is still there -- but I'm not familiar with this code, so I'm probably missing something?
There's no bug in the actual patch, the crash is because the testing flag --aarch64prelegalizercombinerhelper-only-enable-rule doesn't work without assertions enabled. It's not an elegant failure mode, but nothing to worry I think.
Can generalize for not-i1 by checking the target boolean contents