Skip to content

Commit db6bee5

Browse files
committed
[RISCV] Fix miscompile in SExtWRemoval due to early return ignoring other 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
1 parent 98e7670 commit db6bee5

File tree

2 files changed

+4
-5
lines changed

2 files changed

+4
-5
lines changed

llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,9 @@ static bool isSignExtendedW(Register SrcReg, const MachineRegisterInfo &MRI,
175175

176176
const AttributeSet &Attrs = CalleeFn->getAttributes().getRetAttrs();
177177
unsigned BitWidth = IntTy->getBitWidth();
178-
return (BitWidth <= 32 && Attrs.hasAttribute(Attribute::SExt)) ||
179-
(BitWidth < 32 && Attrs.hasAttribute(Attribute::ZExt));
178+
if ((BitWidth <= 32 && Attrs.hasAttribute(Attribute::SExt)) ||
179+
(BitWidth < 32 && Attrs.hasAttribute(Attribute::ZExt)))
180+
continue;
180181
}
181182

182183
if (!AddRegDefToWorkList(CopySrcReg))

llvm/test/CodeGen/RISCV/sextw-removal.ll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,8 +1376,6 @@ define signext i32 @sextw_sh2add(i1 zeroext %0, ptr %1, i32 signext %2, i32 sign
13761376
}
13771377

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

0 commit comments

Comments
 (0)