[PATCH] D154533: [DAG] Improve carry reconstruction in combineCarryDiamond.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Hi @deadalnix , it looks to me the test case exposes an actual bug.   A simplified test would be this:

  define i1 @f14(i32 %val0, i32 %val1, i32 %val2) {
    %t0 = call {i32, i1} @llvm.uadd.with.overflow.i32(i32 %val0, i32 %val1)
    %add0 = extractvalue {i32, i1} %t0, 0
    %obit0 = extractvalue {i32, i1} %t0, 1
    %t1 = call {i32, i1} @llvm.uadd.with.overflow.i32(i32 %add0, i32 %val2)
    %add1 = extractvalue {i32, i1} %t1, 0
    %obit1 = extractvalue {i32, i1} %t1, 1
  
    %res = or i1 %obit0, %obit1
    ret i1 %res
  }
  
  declare {i32, i1} @llvm.uadd.with.overflow.i32(i32, i32) nounwind readnone

This gets initially expanded into:

  t0: ch,glue = EntryToken
    t2: i32,ch = CopyFromReg t0, Register:i32 %0
    t4: i32,ch = CopyFromReg t0, Register:i32 %1
  t7: i32,i1 = uaddo t2, t4
          t6: i32,ch = CopyFromReg t0, Register:i32 %2
        t8: i32,i1 = uaddo t7, t6
      t9: i1 = or t7:1, t8:1
    t10: i32 = any_extend t9

but after DAGCombine with your change this ends up as:

      t2: i32,ch = CopyFromReg t0, Register:i32 %0
      t4: i32,ch = CopyFromReg t0, Register:i32 %1
      t6: i32,ch = CopyFromReg t0, Register:i32 %2
    t14: i32,i1 = uaddo_carry t2, t4, t6
  t10: i32 = any_extend t14:1

This looks wrong to me - the last operand of `uaddo_carry` is supposed to be a boolean flag indicating incoming carry, but here we're passing in one of the `i32` inputs in full.   Am I missing something here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154533/new/

https://reviews.llvm.org/D154533