Skip to content

Commit

Permalink
[RISCV] Fix miscompile in SExtWRemoval due to early return ignoring o…
Browse files Browse the repository at this point in the history
…ther sources

This code is walking back through a worklist of sources. All of the sources need to be sign extending for the result to be true. We had a case which returned rather than continued, which causes a miscompile when another source was not sign extended. The flawed logic was introduced in Dec 22, by change 844430b.

This was recently exposed in a stage2 build of llvm-tablegen when we switched from using llvm::Optional to std::optional. The stars aligned in just the wrong way, and we started actively miscompiling idiomatic optional usage. std::optional<uint32_t> appears to use the top 32 bits of the word on RV64 for its tag.

Differential Revision: https://reviews.llvm.org/D143594
  • Loading branch information
preames committed Feb 8, 2023
1 parent 98e7670 commit db6bee5
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
5 changes: 3 additions & 2 deletions llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp
Expand Up @@ -175,8 +175,9 @@ static bool isSignExtendedW(Register SrcReg, const MachineRegisterInfo &MRI,

const AttributeSet &Attrs = CalleeFn->getAttributes().getRetAttrs();
unsigned BitWidth = IntTy->getBitWidth();
return (BitWidth <= 32 && Attrs.hasAttribute(Attribute::SExt)) ||
(BitWidth < 32 && Attrs.hasAttribute(Attribute::ZExt));
if ((BitWidth <= 32 && Attrs.hasAttribute(Attribute::SExt)) ||
(BitWidth < 32 && Attrs.hasAttribute(Attribute::ZExt)))
continue;
}

if (!AddRegDefToWorkList(CopySrcReg))
Expand Down
4 changes: 1 addition & 3 deletions llvm/test/CodeGen/RISCV/sextw-removal.ll
Expand Up @@ -1376,8 +1376,6 @@ define signext i32 @sextw_sh2add(i1 zeroext %0, ptr %1, i32 signext %2, i32 sign
}

; Negative test - an explicit sext.w *is* required
; FIXME: This is currently demonstrating an active miscompile as the high
; bits of s0 are *not* the sign extended zero of bit 32 on the untaken path.
define signext i32 @test19(i64 %arg, i1 zeroext %c1, i1 zeroext %c2, ptr %p) nounwind {
; CHECK-LABEL: test19:
; CHECK: # %bb.0: # %bb
Expand All @@ -1397,7 +1395,7 @@ define signext i32 @test19(i64 %arg, i1 zeroext %c1, i1 zeroext %c2, ptr %p) nou
; CHECK-NEXT: mv s0, a0
; CHECK-NEXT: .LBB23_2: # %bb7
; CHECK-NEXT: call side_effect@plt
; CHECK-NEXT: mv a0, s0
; CHECK-NEXT: sext.w a0, s0
; CHECK-NEXT: ld ra, 8(sp) # 8-byte Folded Reload
; CHECK-NEXT: ld s0, 0(sp) # 8-byte Folded Reload
; CHECK-NEXT: addi sp, sp, 16
Expand Down

0 comments on commit db6bee5

Please sign in to comment.