[PATCH] D78439: [AVR] Fix miscompilation of zext + add

[Ayke via Phabricator via llvm-commits]

aykevl created this revision.
aykevl added a reviewer: dylanmckay.
Herald added subscribers: Jim, hiraditya.
Herald added a project: LLVM.

Code like the following:

  define i32 @foo(i32 %a, i1 zeroext %b) addrspace(1) {
  entry:
    %conv = zext i1 %b to i32
    %add = add nsw i32 %conv, %a
    ret i32 %add
  }

Would compile to the following (incorrect) code:

  foo:
      mov     r18, r20
      clr     r19
      add     r22, r18
      adc     r23, r19
      sbci    r24, 0
      sbci    r25, 0
      ret

Those sbci instructions are clearly wrong, they should have been adc instructions.

This commit improves codegen to use adc instead:

  foo:
      mov     r18, r20
      clr     r19
      ldi     r20, 0
      ldi     r21, 0
      add     r22, r18
      adc     r23, r19
      adc     r24, r20
      adc     r25, r21
      ret

This code is not optimal (it could be just 5 instructions instead of the current 9) but at least it doesn't miscompile.

---

**WARNING**: I don't know what I'm doing here. While removing that pattern works, I don't know why it's there or whether I just introduced a different bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78439

Files:
  llvm/lib/Target/AVR/AVRInstrInfo.td
  llvm/test/CodeGen/AVR/add.ll


Index: llvm/test/CodeGen/AVR/add.ll
===================================================================
--- llvm/test/CodeGen/AVR/add.ll
+++ llvm/test/CodeGen/AVR/add.ll
@@ -45,6 +45,17 @@
     ret i16 %result
 }
 
+define i16 @add16_reg_reg_zext(i16 %a, i1 zeroext %b) {
+; CHECK-LABEL: add16_reg_reg_zext:
+; CHECK: mov r18, r22
+; CHECK: clr r19
+; CHECK: add r24, r18
+; CHECK: adc r25, r19
+    %zext = zext i1 %b to i16
+    %result = add i16 %a, %zext
+    ret i16 %result
+}
+
 define i32 @add32_reg_reg(i32 %a, i32 %b) {
 ; CHECK-LABEL: add32_reg_reg:
 ; CHECK: add r22, r18
@@ -65,6 +76,21 @@
     ret i32 %result
 }
 
+define i32 @add32_reg_reg_zext(i32 %a, i1 zeroext %b) {
+; CHECK-LABEL: add32_reg_reg_zext:
+; CHECK: mov r18, r20
+; CHECK: clr r19
+; CHECK: ldi r20, 0
+; CHECK: ldi r21, 0
+; CHECK: add r22, r18
+; CHECK: adc r23, r19
+; CHECK: adc r24, r20
+; CHECK: adc r25, r21
+    %zext = zext i1 %b to i32
+    %result = add i32 %a, %zext
+    ret i32 %result
+}
+
 define i64 @add64_reg_reg(i64 %a, i64 %b) {
 ; CHECK-LABEL: add64_reg_reg:
 ; CHECK: add r18, r10
@@ -91,3 +117,22 @@
     %result = add i64 %a, 5
     ret i64 %result
 }
+
+define i64 @add64_reg_reg_zext(i64 %a, i1 zeroext %b) {
+; CHECK-LABEL: add64_reg_reg_zext:
+; CHECK: mov r30, r16
+; CHECK: clr r31
+; CHECK: ldi r26, 0
+; CHECK: ldi r27, 0
+; CHECK: add r18, r30
+; CHECK: adc r19, r31
+; CHECK: adc r20, r26
+; CHECK: adc r21, r27
+; CHECK: adc r22, r26
+; CHECK: adc r23, r27
+; CHECK: adc r24, r26
+; CHECK: adc r25, r27
+    %zext = zext i1 %b to i64
+    %result = add i64 %a, %zext
+    ret i64 %result
+}
Index: llvm/lib/Target/AVR/AVRInstrInfo.td
===================================================================
--- llvm/lib/Target/AVR/AVRInstrInfo.td
+++ llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -2042,8 +2042,6 @@
           (SUBIWRdK i16:$src1, (imm16_neg_XFORM imm:$src2))>;
 def : Pat<(addc i16:$src1, imm:$src2),
           (SUBIWRdK i16:$src1, (imm16_neg_XFORM imm:$src2))>;
-def : Pat<(adde i16:$src1, imm:$src2),
-          (SBCIWRdK i16:$src1, (imm16_neg_XFORM imm:$src2))>;
 
 def : Pat<(add i8:$src1, imm:$src2),
           (SUBIRdK i8:$src1, (imm8_neg_XFORM imm:$src2))>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78439.258565.patch
Type: text/x-patch
Size: 2191 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200419/64d6f46f/attachment-0001.bin>