Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstCombine] a && (a | b) is not simplified #69038

Open
k-arrows opened this issue Oct 14, 2023 · 9 comments · May be fixed by #69840
Open

[InstCombine] a && (a | b) is not simplified #69038

k-arrows opened this issue Oct 14, 2023 · 9 comments · May be fixed by #69840

Comments

@k-arrows
Copy link

https://alive2.llvm.org/ce/z/Z5Ly98

define i32 @src(i32 %0, i32 %1) {
%2:
  %3 = icmp ne i32 %0, 0
  %4 = or i32 %1, %0
  %5 = icmp ne i32 %4, 0
  %6 = and i1 %3, %5
  %7 = zext i1 %6 to i32
  ret i32 %7
}
=>
define i32 @tgt(i32 %0, i32 %1) {
%2:
  %3 = icmp ne i32 %0, 0
  %4 = zext i1 %3 to i32
  ret i32 %4
}
Transformation seems to be correct!

Reproducer:
https://godbolt.org/z/YEqW7sE6n

int foo(int a, int b)
{
   return a && (a | b);
}
@elhewaty
Copy link
Member

I will Work on this.

@elhewaty
Copy link
Member

@k-arrows Where is the function responsible for this optimization?
In InstCombmineAndOrXor.cpp the optimizations are about logical operators, not the logical ones.

@k-arrows
Copy link
Author

@k-arrows Where is the function responsible for this optimization? In InstCombmineAndOrXor.cpp the optimizations are about logical operators, not the logical ones.

Thank you for the question, but I'm not a developer of LLVM, so it's not appropriate for me to answer this question.

@nikic @goldsteinn @dtcxzyw
Sorry to bother you, but do you have any suggestions for this question?

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 22, 2023

@k-arrows Where is the function responsible for this optimization? In InstCombmineAndOrXor.cpp the optimizations are about logical operators, not the logical ones.

@elhewaty I guess it has been fixed by #69840. I will update the regression tests soon.

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 22, 2023

@k-arrows Where is the function responsible for this optimization? In InstCombmineAndOrXor.cpp the optimizations are about logical operators, not the logical ones.

InstCombinerImpl::visitAnd calls simplifyAndInst. Then simplifyAndInst will simplify the and inst if LHS implies RHS/!RHS.

if (Op0->getType()->isIntOrIntVectorTy(1)) {
if (std::optional<bool> Implied = isImpliedCondition(Op0, Op1, Q.DL)) {
// If Op0 is true implies Op1 is true, then Op0 is a subset of Op1.
if (*Implied == true)
return Op0;
// If Op0 is true implies Op1 is false, then they are not true together.
if (*Implied == false)
return ConstantInt::getFalse(Op0->getType());
}
if (std::optional<bool> Implied = isImpliedCondition(Op1, Op0, Q.DL)) {
// If Op1 is true implies Op0 is true, then Op1 is a subset of Op0.
if (*Implied)
return Op1;
// If Op1 is true implies Op0 is false, then they are not true together.
if (!*Implied)
return ConstantInt::getFalse(Op1->getType());
}
}

@dtcxzyw dtcxzyw self-assigned this Oct 22, 2023
@elhewaty
Copy link
Member

@dtcxzyw can you suggest an interesting issue for me to work on?

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 22, 2023

@elhewaty
Some missed-optimizations: #69803 #69050 #68227
Canonicalization of splat: #63868
Crash: #65107

Besides, there are many crashes reported by fuzzers. I guess you may not be interested in these issues since it is hard to reduce test cases and find bugs :(

@elhewaty
Copy link
Member

@dtcxzyw Can you please assign them to me, except the crash one till I get familiar with the system more?

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 22, 2023

@dtcxzyw Can you please assign them to me, except the crash one till I get familiar with the system more?

I am sorry I cannot assign all these issues to you simultaneously. Someone may also be interested in these issues. You can pick one (or two) issues and leave comments on them :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants