The new IR with And removes a use of the input variable, which is better for analysis.
Details
Diff Detail
Time | Test | |
---|---|---|
60 ms | x64 debian > Flang.Examples::feature-list-class.f90 Script:
--
: 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new -fc1 -load /var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so -plugin feature-list /var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-class.f90
| |
60 ms | x64 debian > Flang.Examples::feature-list-functions.f90 Script:
--
: 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new -fc1 -load /var/lib/buildkite-agent/builds/llvm-project/build/lib/flangFeatureList.so -plugin feature-list /var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-functions.f90 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/flang/test/Examples/feature-list-functions.f90
|
Event Timeline
llvm/test/Transforms/InstCombine/icmp-sub.ll | ||
---|---|---|
564 | Can you split the tests into two commits so we can see the diff from the patch? |
The new IR with and removes a use of the input variable, which is better for analysis.
"and" -> And (otherwise it looks like the conjugation, not the operator.
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
---|---|---|
4100 | Use ICmpInst::isEquality(). | |
4102 | Please add a vector test, this is probably going to crash. (Can use getScalarSizeInBits instead.) | |
4107 | I.getPrediate() -> Pred. | |
llvm/test/Transforms/InstCombine/icmp-sub.ll | ||
590 | Missing commuted test case (negation on RHS), multi-use test case and negative tests (e.g. non-equality predicate). |
address commet
1、Add more cases (also update on D146268)
2、update with getScalarSizeInBits and add related vector case
3、use ICmpInst::isEquality(Pred) to replace Pred == ICmpInst::ICMP_EQ
4、I.getPredicate() --> Pred
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
---|---|---|
4100 | isEquality() means "EQ" or "NE". You don't need to check for ICMP_NE separately. | |
llvm/test/Transforms/InstCombine/icmp-sub.ll | ||
596 | These end up not actually commuted due to complexity-based canonicalization. Grep for thwart complexity-based canonicalization for how to avoid it. |
Address comment
1、delete the repeat ICMP_NE check, as isEquality() already include it
2、use "thwart complexity-based canonicalization" for commuted case
How about this pattern ?
https://alive2.llvm.org/ce/z/Y-Po8g
Also reduce one usage but it works for any minuend.
https://github.com/llvm/llvm-project/issues/60818
Yes, I think this case is similar, but it seems the llvm doesn't have the issue end-to-end for C source, https://godbolt.org/z/EYMo9bd9c.
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
---|---|---|
4100 | Apply your comment, thanks |