[SwiftError] Use IMPLICIT_DEF as a definition for unreachable VReg uses

[fzhinkin]

Summary

SwiftErrorValueTracking creates vregs at swifterror use sites and then
connects it with appropriate definitions after instruction selection.
To propagate swifterror values SwiftErrorValueTracking::propagateVRegs
iterates over basic blocks in RPO, but some vregs previously created
at use sites may be located in blocks that became unreachable after
instruction selection. Because of that there will no definition for
such vregs and that may cause issues down the pipeline.

To ensure that all vregs created by the SwiftErrorValueTracking will
be defined propagateVRegs was updated to insert IMPLICIT_DEF at the
beginning of unreachable blocks containing swifterror uses.

Related issue: https://github.com/llvm/llvm-project/issues/59751

[compnerd]

llvm/lib/CodeGen/SwiftErrorValueTracking.cpp

259–261

I think that we can simplify this to:

for (const auto *Use : VRegUpwardsUse) {
  const MachineBasicBlock *UseBB = Use.first.first;
  Register VReg = Use.second;

266

Should this be an expensive assert?

llvm/test/CodeGen/AArch64/swift-error-unreachable-use.ll

2

Please expand this into three explicit invocations:

llc -mtriple aarch64-unknown-windows-msvc %s -filetype asm -o - | FileCheck %s
llc -mtriple aarch64-unknown-linux-gnu %s -filetype asm -o - | FileCheck %s
llc -mtriple aarch64-apple-macosx10.15 %s -filetype asm -o - | FileCheck %s

The test should actually validate that the output is correct.

[fzhinkin]

Cleaned up the code, updated test.

[fzhinkin]

llvm/lib/CodeGen/SwiftErrorValueTracking.cpp

266

Thanks for suggestion, fixed.

llvm/test/CodeGen/AArch64/swift-error-unreachable-use.ll

2

Updated the test, thanks for suggestion!

[fzhinkin]

ping

[fzhinkin]

Rebased

[fzhinkin]

ping

[compnerd]

llvm/test/CodeGen/AArch64/swift-error-unreachable-use.ll

28

Why not use a regex group to handle the comment leader? These are identical.

[fzhinkin]

Cleaned up the test

llvm/test/CodeGen/AArch64/swift-error-unreachable-use.ll

28

Thank you for suggestion, I updated the test.

[fzhinkin]

Rebase

[fzhinkin]

Rebase

[fzhinkin]

ping

llvm/lib/CodeGen/SwiftErrorValueTracking.cpp

for (const auto *SwiftErrorVal : SwiftErrorVals) {
       }
 
       // We did not have a definition in this block before: store the phi's vreg
       // as this block downward exposed def.
       if (!UpwardsUse)
         setCurrentVReg(MBB, SwiftErrorVal, PHIVReg);
     }
   }
+
+  // Create implicit defs for upward uses from unreachable blocks
+  MachineRegisterInfo &MRI = MF->getRegInfo();
+  for (const auto &Use : VRegUpwardsUse) {
+    const MachineBasicBlock *UseBB = Use.first.first;
+    Register VReg = Use.second;
+    if (!MRI.def_begin(VReg).atEnd())
+      continue;
+
+#ifdef EXPENSIVE_CHECKS
+    assert(std::find(RPOT.begin(), RPOT.end(), UseBB) == RPOT.end() &&
+           "Reachable block has VReg upward use without definition.");
+#endif
+
+    MachineBasicBlock *UseBBMut = MF->getBlockNumbered(UseBB->getNumber());
+
+    BuildMI(*UseBBMut, UseBBMut->getFirstNonPHI(), DebugLoc(),
+            TII->get(TargetOpcode::IMPLICIT_DEF), VReg);
+  }
 }
 
 void SwiftErrorValueTracking::preassignVRegs(
     MachineBasicBlock *MBB, BasicBlock::const_iterator Begin,
     BasicBlock::const_iterator End) {
   if (!TLI->supportSwiftError() || SwiftErrorVals.empty())
     return;
 

llvm/test/CodeGen/AArch64/swift-error-unreachable-use.ll

+; RUN: llc -mtriple aarch64-unknown-windows-msvc %s -filetype asm -o - | FileCheck %s
+; RUN: llc -mtriple aarch64-unknown-linux-gnu %s -filetype asm -o - | FileCheck %s
+; RUN: llc -mtriple aarch64-apple-macosx %s -filetype asm -o - | FileCheck %s
+; Regression test for https://github.com/llvm/llvm-project/issues/59751
+
+define void @"func"(i32** swifterror %0) #0 {
+; CHECK-LABEL: func:
+; CHECK:       {{.*}}%bb.0:
+; CHECK-NEXT:    b {{\.?}}LBB0_2
+; CHECK-NEXT:  {{\.?}}LBB0_1:{{.*}}%thirtythree
+; CHECK-NEXT:  {{.*}}=>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    b {{\.?}}LBB0_1
+; CHECK-NEXT:  {{\.?}}LBB0_2:{{.*}}%thirtyeight
+; CHECK-NEXT:    b {{\.?}}LBB0_3
+; CHECK-NEXT:  {{\.?}}LBB0_3:{{.*}}%thirtythree.preheader
+; CHECK-NEXT:    b {{\.?}}LBB0_1
+  br label %thirtyeight
+
+five:
+  br label %UelOc2l.exit
+
+common.ret:
+  ret void
+
+UelOc2l.exit:
+  %a = getelementptr inbounds [754 x i8*], [754 x i8*]* undef, i32 undef, i32 undef
+  %b = load i8*, i8** %a, align 8
+  %c = bitcast i8* %b to void ()*
+  call void %c()
+  br label %common.ret
+
+thirtythree:
+  br i1 false, label %UelOc2l.exit, label %thirtythree
+
+thirtyeight:
+  br i1 undef, label %thirtyeight, label %thirtythree
+}
+
+attributes #0 = { noinline optnone }