Page MenuHomePhabricator

[GlobalISel] Simplify G_ICMP to true/false when the result is known
ClosedPublic

Authored by paquette on May 14 2021, 5:07 PM.

Details

Summary

Use existing KnownBits helpers from KnownBits.h to simplify G_ICMPs.

E.g.

x == x -> true
x != x -> false
load(x) > 1 -> true (when the load is known to be greater than 1)

And so on.

Diff Detail

Event Timeline

paquette created this revision.May 14 2021, 5:07 PM
paquette requested review of this revision.May 14 2021, 5:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2021, 5:07 PM
Herald added a subscriber: wdng. · View Herald Transcript
foad accepted this revision.May 17 2021, 1:25 AM
foad added a subscriber: foad.

LGTM.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
3974

Not really related, but I don't understand why getICmpTrueVal takes an IsFP argument.

This revision is now accepted and ready to land.May 17 2021, 1:25 AM
arsenm added inline comments.May 17 2021, 6:26 AM
llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir
5

Don't need the IR section

phosek added a subscriber: phosek.May 18 2021, 10:16 PM

We're seeing a test failure in prelegalizer-combiner-icmp-to-true-false-known-bits.mir on our 2-stage LTO builder:

FAIL: LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir (2029 of 43749)
******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/llc -debugify-and-strip-all-safe -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="icmp_to_true_false_known_bits" -global-isel -verify-machineinstrs /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir -o - | /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir
--
Exit Code: 2

Command Output (stderr):
--
LLVM ERROR: Invalid rule identifier
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir

--

Would it be possible to revert the change unless the issue can be addressed quickly?

dyung added a subscriber: dyung.May 19 2021, 1:13 AM

We're seeing a test failure in prelegalizer-combiner-icmp-to-true-false-known-bits.mir on our 2-stage LTO builder:

FAIL: LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir (2029 of 43749)
******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/llc -debugify-and-strip-all-safe -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="icmp_to_true_false_known_bits" -global-isel -verify-machineinstrs /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir -o - | /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir
--
Exit Code: 2

Command Output (stderr):
--
LLVM ERROR: Invalid rule identifier
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir

--

Would it be possible to revert the change unless the issue can be addressed quickly?

We are also seeing this test fail in our internal linux release without asserts build bot.

Here is our failure with a more detailed stack trace:

FAIL: LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir (1 of 1)
******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir' FAILED ********************
Script:
--
: 'RUN: at line 2';   /home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc -debugify-and-strip-all-safe -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="icmp_to_true_false_known_bits" -global-isel -verify-machineinstrs /home/dyung/src/upstream/llvm_clean_git/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir -o - | /home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/FileCheck /home/dyung/src/upstream/llvm_clean_git/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.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/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc -debugify-and-strip-all-safe -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule=icmp_to_true_false_known_bits -global-isel -verify-machineinstrs /home/dyung/src/upstream/llvm_clean_git/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir -o -
1.      Running pass 'Function Pass Manager' on module '/home/dyung/src/upstream/llvm_clean_git/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir'.
2.      Running pass 'AArch64PreLegalizerCombiner' on function '@eq_true'
 #0 0x000055a575f4f641 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x2acd641)
 #1 0x000055a575f4d1f4 llvm::sys::RunSignalHandlers() (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x2acb1f4)
 #2 0x000055a575f4d36b SignalHandler(int) Signals.cpp:0:0
 #3 0x00007fc4948bc3c0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x153c0)
 #4 0x00007fc49437818b raise /build/glibc-eX1tMB/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007fc494357859 abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:81:7
 #6 0x000055a575ebb07a llvm::report_fatal_error(llvm::Twine const&, bool) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x2a3907a)
 #7 0x000055a575ebb19e (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x2a3919e)
 #8 0x000055a573e73961 (anonymous namespace)::AArch64PreLegalizerCombiner::runOnMachineFunction(llvm::MachineFunction&) AArch64PreLegalizerCombiner.cpp:0:0
 #9 0x000055a57539a0dc llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x1f180dc)
#10 0x000055a5757b2a70 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x2330a70)
#11 0x000055a5757b40a9 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x23320a9)
#12 0x000055a5757b1ca0 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x232fca0)
#13 0x000055a573c27630 main (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x7a5630)
#14 0x00007fc4943590b3 __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:342:3
#15 0x000055a573ce81ee _start (/home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/llc+0x8661ee)
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/dyung/src/upstream/892497c806306a4b7185ead16d60b0ebcca0a304-linux/bin/FileCheck /home/dyung/src/upstream/llvm_clean_git/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir

And another +1 for this. This fails on all our bots except the one that build clang for mac/arm. Example: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8846830947115936304/+/steps/package_clang/0/stdout?format=raw

Given that this has been reported several hours ago, I reverted this for now in 52a779762688dee1a6042dc41f35e1f7a7048b85 . Sorry about the inconvenience.

gulfem added a subscriber: gulfem.May 19 2021, 8:51 AM

Oh, sorry about that and thanks for taking care of the revert.

Will fix + recommit shortly.