Skip to content

(trunc nuw A) == (trunc nuw B) should not take multiple xors [InstCombine] #133344

Closed
@scottmcm

Description

@scottmcm

Take this input IR:

define noundef i1 @src(i8 noundef %a, i8 noundef %b) local_unnamed_addr {
  %at = trunc nuw i8 %a to i1
  %bt = trunc nuw i8 %b to i1
  %eq = icmp eq i1 %at, %bt
  ret i1 %eq
}

(That's the kind of thing you might see from looking at bools in Options or unions in Rust.)

Today, it "optimizes" to a lossy truncation and a couple of xors:

define noundef i1 @src(i8 noundef %a, i8 noundef %b) local_unnamed_addr #0 {
  %1 = xor i8 %b, %a
  %2 = trunc i8 %1 to i1
  %eq = xor i1 %2, true
  ret i1 %eq
}

Now, if the nuws weren't there that would make sense, but with the nuws it would be much better to simplify it to

define noundef i1 @tgt(i8 noundef %a, i8 noundef %b) local_unnamed_addr {
  %eq = icmp eq i8 %a, %b
  ret i1 %eq
}

instead. Alive2 proof: https://alive2.llvm.org/ce/z/X3Uh23

Notably, that does happen already if you truncate to something that's not i1, like https://alive2.llvm.org/ce/z/H-QrGZ

define noundef i1 @src(i8 noundef %a, i8 noundef %b) local_unnamed_addr {
  %at = trunc nuw i8 %a to i2
  %bt = trunc nuw i8 %b to i2
  %eq = icmp eq i2 %at, %bt
  ret i1 %eq
}

So hopefully this is just a matter of disabling an i1 special case in InstCombine when nuw is there.


For completeness, looks like this simplification is also legal when both sides are nsw: https://alive2.llvm.org/ce/z/vRMFDD

In mixed cases (one nuw and one nsw) this is NOT applicable, however: https://alive2.llvm.org/ce/z/kUl-ZL


cc @nikic -- I think this might be part of the enduring problem of Option<bool>::eq being terrible you've mentioned before.

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions