Fix PR33028

[hliao]

Summary

• '-verify-machineinstrs' complains allocatable live-in on MBB which is not entry or landing-pad.
• Previous implementation relies on the live-in to model the hardware behavior updating EAX when fallback code path is taken in the following CFG:

thisMBB
 |   \
 |   mainMBB
 |   /
sinkMBB

The new implementation needs to create an explicit MBB and a pseudo
instruction to model that behavior in the following CFG so that we won't have
EAX as live-in:

    thisMBB
     /   \
fallMBB  mainMBB
     \   /
     sinkMBB

[RKSimon]

Thank you for looking at this.

Please can you regenerate the diff with context?

Also, please add -verify-machineinstrs to the test cmds in rtm.ll

[hliao]

new patch is updated

[RKSimon]

Please can you regenerate the diff with context?

[hliao]

patch is updated following review page by running 'svn diff --diff-cmd=diff -x -U999999'

[RKSimon]

I think this is sound, but I'm not an expert on any of this. Any other comments?

lib/Target/X86/X86ISelLowering.cpp

24758 ↗

(On Diff #99245)

Isn't this comment for fallMBB now?

[hliao]

I updated the comment following the fix made by this change. If there's no other comments, I will commit it. If any one has objection, please let me know ASAP.

[hliao]

lib/Target/X86/X86ISelLowering.cpp

24758 ↗

(On Diff #99245)

comment is revised

[RKSimon]

LGTM

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

static MachineBasicBlock *emitXBegin(MachineInstr &MI, MachineBasicBlock *MBB,
   MachineFunction::iterator I = ++MBB->getIterator();
 
   // For the v = xbegin(), we generate
   //
   // thisMBB:
   //  xbegin sinkMBB
   //
   // mainMBB:
-  //  eax = -1
+  //  s0 = -1
+  //
+  // fallBB:
+  //  eax = # XABORT_DEF
+  //  s1 = eax
   //
   // sinkMBB:
-  //  v = eax
+  //  v = phi(s0/mainBB, s1/fallBB)
 
   MachineBasicBlock *thisMBB = MBB;
   MachineFunction *MF = MBB->getParent();
   MachineBasicBlock *mainMBB = MF->CreateMachineBasicBlock(BB);
+  MachineBasicBlock *fallMBB = MF->CreateMachineBasicBlock(BB);
   MachineBasicBlock *sinkMBB = MF->CreateMachineBasicBlock(BB);
   MF->insert(I, mainMBB);
+  MF->insert(I, fallMBB);
   MF->insert(I, sinkMBB);
 
   // Transfer the remainder of BB and its successor edges to sinkMBB.
   sinkMBB->splice(sinkMBB->begin(), MBB,
                   std::next(MachineBasicBlock::iterator(MI)), MBB->end());
   sinkMBB->transferSuccessorsAndUpdatePHIs(MBB);
 
+  MachineRegisterInfo &MRI = MF->getRegInfo();
+  unsigned DstReg = MI.getOperand(0).getReg();
+  const TargetRegisterClass *RC = MRI.getRegClass(DstReg);
+  unsigned mainDstReg = MRI.createVirtualRegister(RC);
+  unsigned fallDstReg = MRI.createVirtualRegister(RC);
+
   // thisMBB:
-  //  xbegin sinkMBB
+  //  xbegin fallMBB
   //  # fallthrough to mainMBB
-  //  # abortion to sinkMBB
-  BuildMI(thisMBB, DL, TII->get(X86::XBEGIN_4)).addMBB(sinkMBB);
+  //  # abortion to fallMBB
+  BuildMI(thisMBB, DL, TII->get(X86::XBEGIN_4)).addMBB(fallMBB);
   thisMBB->addSuccessor(mainMBB);
-  thisMBB->addSuccessor(sinkMBB);
+  thisMBB->addSuccessor(fallMBB);
 
   // mainMBB:
-  //  EAX = -1
-  BuildMI(mainMBB, DL, TII->get(X86::MOV32ri), X86::EAX).addImm(-1);
+  //  mainDstReg := -1
+  BuildMI(mainMBB, DL, TII->get(X86::MOV32ri), mainDstReg).addImm(-1);
+  BuildMI(mainMBB, DL, TII->get(X86::JMP_1)).addMBB(sinkMBB);
   mainMBB->addSuccessor(sinkMBB);
 
-  // sinkMBB:
-  // EAX is live into the sinkMBB
-  sinkMBB->addLiveIn(X86::EAX);
-  BuildMI(*sinkMBB, sinkMBB->begin(), DL, TII->get(TargetOpcode::COPY),
-          MI.getOperand(0).getReg())
+  // fallMBB:
+  //  ; pseudo instruction to model hardware's definition from XABORT
+  //  EAX := XABORT_DEF
+  //  fallDstReg := EAX
+  BuildMI(fallMBB, DL, TII->get(X86::XABORT_DEF));
+  BuildMI(fallMBB, DL, TII->get(TargetOpcode::COPY), fallDstReg)
       .addReg(X86::EAX);
+  fallMBB->addSuccessor(sinkMBB);
+
+  // sinkMBB:
+  //  DstReg := phi(mainDstReg/mainBB, fallDstReg/fallBB)
+  BuildMI(*sinkMBB, sinkMBB->begin(), DL, TII->get(X86::PHI), DstReg)
+      .addReg(mainDstReg).addMBB(mainMBB)
+      .addReg(fallDstReg).addMBB(fallMBB);
 
   MI.eraseFromParent();
   return sinkMBB;
 }
 
 // FIXME: When we get size specific XMM0 registers, i.e. XMM0_V16I8
 // or XMM0_V32I8 in AVX all of this code can be replaced with that
 // in the .td file.

llvm/trunk/lib/Target/X86/X86InstrTSX.td

 
 let isBranch = 1, isTerminator = 1, Defs = [EAX] in {
 def XBEGIN_2 : Ii16PCRel<0xc7, MRM_F8, (outs), (ins brtarget16:$dst),
                          "xbegin\t$dst", []>, OpSize16;
 def XBEGIN_4 : Ii32PCRel<0xc7, MRM_F8, (outs), (ins brtarget32:$dst),
                          "xbegin\t$dst", []>, OpSize32;
 }
 
+// Psuedo instruction to fake the definition of EAX on the fallback code path.
+let isPseudo = 1, Defs = [EAX] in {
+def XABORT_DEF : I<0, Pseudo, (outs), (ins), "# XABORT DEF", []>;
+}
+
 def XEND : I<0x01, MRM_D5, (outs), (ins),
              "xend", [(int_x86_xend)]>, TB, Requires<[HasRTM]>;
 
 let Defs = [EFLAGS] in
 def XTEST : I<0x01, MRM_D6, (outs), (ins),
               "xtest", [(set EFLAGS, (X86xtest))]>, TB, Requires<[HasRTM]>;
 
 def XABORT : Ii8<0xc6, MRM_F8, (outs), (ins i8imm:$imm),

llvm/trunk/test/CodeGen/X86/rtm.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+rtm | FileCheck %s --check-prefix=X86
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+rtm | FileCheck %s --check-prefix=X64
+; RUN: llc -verify-machineinstrs < %s -mtriple=i686-unknown-unknown -mattr=+rtm | FileCheck %s --check-prefix=X86
+; RUN: llc -verify-machineinstrs < %s -mtriple=x86_64-unknown-unknown -mattr=+rtm | FileCheck %s --check-prefix=X64
 
 declare i32 @llvm.x86.xbegin() nounwind
 declare void @llvm.x86.xend() nounwind
 declare void @llvm.x86.xabort(i8) nounwind
 declare void @f1()
 
 define i32 @test_xbegin() nounwind uwtable {
 ; X86-LABEL: test_xbegin:
 ; X86:       # BB#0: # %entry
 ; X86-NEXT:    xbegin .LBB0_2
 ; X86-NEXT:  # BB#1: # %entry
 ; X86-NEXT:    movl $-1, %eax
-; X86-NEXT:  .LBB0_2: # %entry
+; X86:       .LBB0_2: # %entry
+; X86-NEXT:  # XABORT DEF
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: test_xbegin:
 ; X64:       # BB#0: # %entry
 ; X64-NEXT:    xbegin .LBB0_2
 ; X64-NEXT:  # BB#1: # %entry
 ; X64-NEXT:    movl $-1, %eax
-; X64-NEXT:  .LBB0_2: # %entry
+; X64:       .LBB0_2: # %entry
+; X64-NEXT:  # XABORT DEF
 ; X64-NEXT:    retq
 entry:
   %0 = tail call i32 @llvm.x86.xbegin() nounwind
   ret i32 %0
 }
 
 define void @test_xend() nounwind uwtable {
 ; X86-LABEL: test_xend: