llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.84k stars 11.47k forks source link

[InstCombine] Failure to merge similar comparisons that share a common true/false condition #59266

Open RKSimon opened 1 year ago

RKSimon commented 1 year ago

Noticed while looking at the 'IsBigEndian' checks in D138872

bool src(bool c, int x) {
  if ((c && x == 15) || (!c && x == 0))
    return true;
  return false;
}
bool tgt(bool c, int x) {
  if (x == (c ? 15 : 0))
    return true;
  return false;
}

https://godbolt.org/z/eEbqfsGM1 https://godbolt.org/z/f96znedee (General Case)

----------------------------------------
define i1 @src(i1 %c, i32 %x) {
  %cmp = icmp eq i32 %x, 15
  %or.cond = and i1 %cmp, %c
  %cmp3 = icmp ne i32 %x, 0
  %or = or i1 %cmp3, %c
  %or.cond7 = xor i1 %or.cond, %or
  %not.or.cond7 = xor i1 %or.cond7, 1
  ret i1 %not.or.cond7
}
=>
define i1 @tgt(i1 %c, i32 %x) {
  %cond = select i1 %c, i32 15, i32 0
  %cmp = icmp eq i32 %cond, %x
  ret i1 %cmp
}
Transformation seems to be correct!
rotateright commented 1 year ago

Neat! (sigh)

I should code it with the select because that's shorter.

I didn't follow through to see if this changes anything, but we miss a logic canonicalization: https://alive2.llvm.org/ce/z/JHN2p4 We're probably better off avoiding a generic 'xor' op if we can replace with 'and' or 'or', so leaning towards "tgt".

bcl5980 commented 1 year ago

And maybe we should add a transform: C ? (Y != X) : (Z != X) --> (C ? Y : Z) != X C ? (Y == X) : (Z == X) --> (C ? Y : Z) == X

https://alive2.llvm.org/ce/z/-frXfs

Candidate Patch: https://reviews.llvm.org/D139076

bcl5980 commented 1 year ago

And add this transform looks can partial fix the issue: (C & X) | ~(C | Y) -> C ? X : ~Y

https://alive2.llvm.org/ce/z/4yLh_i

Candidate Patch: https://reviews.llvm.org/D139080

RKSimon commented 1 year ago

Another slight variant:

bool src2(bool c, int x) {
  if ((c && x == 15))
    return true;
  if ((!c && x == 0))
    return true;
  return false;
}

define i1 @src2(i1 %c, i32 %x) {
entry:
  %cmp = icmp eq i32 %x, 15
  %or.cond = and i1 %cmp, %c
  %cmp3 = icmp ne i32 %x, 0
  %0 = or i1 %cmp3, %c
  %not. = xor i1 %0, true
  %retval.0 = or i1 %or.cond, %not.
  ret i1 %retval.0
}
bcl5980 commented 1 year ago

Neat! (sigh)

I should code it with the select because that's shorter.

I didn't follow through to see if this changes anything, but we miss a logic canonicalization: https://alive2.llvm.org/ce/z/JHN2p4 We're probably better off avoiding a generic 'xor' op if we can replace with 'and' or 'or', so leaning towards "tgt".

I try to do this in D139299

RKSimon commented 1 year ago

I try to do this in D139299

D139299 landed at 10c3df728cb6d445b6ab9e2d62487e4ecf55693b

RKSimon commented 1 year ago

The general case of src2 looks to be the only outstanding case https://alive2.llvm.org/ce/z/JYgjTz

----------------------------------------
define i1 @src(i1 %c, i32 %x, i32 %t, i32 %f) {
%entry:
  %cmp = icmp eq i32 %x, %t
  %or.cond = and i1 %cmp, %c
  br i1 %or.cond, label %return, label %if.end

%if.end:
  %c.not = xor i1 %c, 1
  %cmp3 = icmp eq i32 %x, %f
  %or.cond8 = and i1 %cmp3, %c.not
  br label %return

%return:
  %retval.0 = phi i1 [ 1, %entry ], [ %or.cond8, %if.end ]
  ret i1 %retval.0
}
=>
define i1 @tgt(i1 %c, i32 %x, i32 %t, i32 %f) {
%entry:
  %cond = select i1 %c, i32 %t, i32 %f
  %cmp = icmp eq i32 %cond, %x
  ret i1 %cmp
}
Transformation seems to be correct!
bcl5980 commented 1 year ago

The general case of src2 looks to be the only outstanding case https://alive2.llvm.org/ce/z/JYgjTz

----------------------------------------
define i1 @src(i1 %c, i32 %x, i32 %t, i32 %f) {
%entry:
  %cmp = icmp eq i32 %x, %t
  %or.cond = and i1 %cmp, %c
  br i1 %or.cond, label %return, label %if.end

%if.end:
  %c.not = xor i1 %c, 1
  %cmp3 = icmp eq i32 %x, %f
  %or.cond8 = and i1 %cmp3, %c.not
  br label %return

%return:
  %retval.0 = phi i1 [ 1, %entry ], [ %or.cond8, %if.end ]
  ret i1 %retval.0
}
=>
define i1 @tgt(i1 %c, i32 %x, i32 %t, i32 %f) {
%entry:
  %cond = select i1 %c, i32 %t, i32 %f
  %cmp = icmp eq i32 %cond, %x
  ret i1 %cmp
}
Transformation seems to be correct!

simplifycfg doesn't work for this transform because of the i1 tpye. If return value is not i1, the branch will be removed. But we still can not get the best result. We may need add some other patterns to fix the general version.