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

DCE breaks by adding redundant terms #57120

Open
ThomasMayerl opened this issue Aug 12, 2022 · 6 comments
Open

DCE breaks by adding redundant terms #57120

ThomasMayerl opened this issue Aug 12, 2022 · 6 comments

Comments

@ThomasMayerl
Copy link

Sometimes, DCE breaks by adding a redundant term.

In the following example, the if statement always evaluates to false. LLVM (14) detects that and removes the dead code:

#include <stdbool.h>

void DCEMarker0_();

void f(bool s, bool sc, bool r, bool cr, bool c) {
    if ((((cr && sc && !r) || (c && !c)) && s && (!sc == !(r || r)))) {
        DCEMarker0_();
    }
}

Now, a redundant disjunction is inserted. LLVM is not able anymore to detect the dead code:

#include <stdbool.h>

void DCEMarker0_();

void f(bool s, bool sc, bool r, bool cr, bool c) {
    if (((((cr && sc && !r) || (c && !c)) && s && (!sc == !(r || (r || 0)))))) {
        DCEMarker0_();
    }
}

This is actually a regression: It worked fine up until LLVM 12.0.1. It has been an issue since LLVM 13.

This can also be seen via the following Compiler Explorer link: https://godbolt.org/z/asrdfj6fs

(Might be related to #56711)

@rotateright
Copy link
Contributor

Looks like it will reduce the same as #56711, but we can leave it open for now.

rotateright added a commit that referenced this issue Aug 13, 2022
(A | ?) | (A ^ B) --> (A | ?) | B
https://alive2.llvm.org/ce/z/dbNQw4

This extends the existing transform to peek through
another 'or' instruction for the common operand.

This is the underlying missing fold that should allow
issue #56711 and issue #57120 to reduce even more.
@rotateright
Copy link
Contributor

This was helped by 9d218b6, but not fixed completely.

It's now virtually the same pattern as #56653, so it would be fixed by https://reviews.llvm.org/D131356.

@rotateright
Copy link
Contributor

Fixed with the previously mentioned commit and:
15e3d86

@ThomasMayerl
Copy link
Author

I have found another case where this issue can still be reproduced:

DCE works in the following snippet:

#include <stdbool.h>

void DCEMarker0_();

void f(bool s, bool c, bool r, bool sc) {
    if (!sc && (c || sc) && (!0 == !(s || sc)) && r && !c) {
        DCEMarker0_();
    }
}

However, it does not work in the following snippet:

#include <stdbool.h>

void DCEMarker0_();

void f(bool s, bool c, bool r, bool sc) {
    if (!sc && (c || sc) && (!0 == !(s || (sc && 1))) && r && !c) {
        DCEMarker0_();
    }
}

(Compiled using -O3 and commit f3acb54)

@EugeneZelenko EugeneZelenko reopened this Aug 23, 2022
@rotateright
Copy link
Contributor

It would be better to open new issues for different examples. At first glance, the new example here has more in common with #57152 than with the original example in this report - it has multiple basic blocks:

define dso_local 
void @g(i1 noundef zeroext %0, i1 noundef zeroext %1, i1 noundef zeroext %2, i1 noundef zeroext %3) local_unnamed_addr #0 {
  %5 = xor i1 %1, true
  %6 = or i1 %5, %3
  br i1 %6, label %12, label %7

7:                                                ; preds = %4
  %8 = xor i1 %2, true
  %9 = or i1 %8, %0
  %10 = or i1 %9, %1
  br i1 %10, label %12, label %11

11:                                               ; preds = %7
  tail call void (...) @DCEMarker0_() #2
  br label %12

12:                                               ; preds = %4, %7, %11
  ret void
}

@rotateright
Copy link
Contributor

Also note: we do not have a general-purpose boolean logic solver, so you can create more complicated (and less realistic) code sequences that will not reduce, but it is unlikely that anyone will care enough to fix those unless it is shown in real code.

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

No branches or pull requests

4 participants