[PATCH] D20907: [IfConversion] Bugfix: don't add Undef flag on use if reg is live.

Fri Jul 1 01:12:22 PDT 2016

jonpa added a comment.

> Returned, not necessary, but clobbered, yes.

OK, if the second tBL just clobbers it, I guess at that point %R0 is actually dead.

> To me the bug seems to come from the fact we thought it is live whereas it is not. Could you check why we think it is live before tMOVr?

IfConverter::IfConvertTriangle() gets to work on:

  BB#11: derived from LLVM BB %if.end5
      Live Ins: %R4 %R5 %R8 %R10 %R11
      Predecessors according to CFG: BB#9
  	%R9<def> = t2ADDri %R4, 32, pred:14, pred:%noreg, opt:%noreg
  	%R0<def> = t2ADDri %R4, 16, pred:14, pred:%noreg, opt:%noreg
  	tBL pred:14, pred:%noreg, <ga:@_ZN1B5m_fn1Ev>, <regmask %LR %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R11 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q4_Q5_Q6_Q7 %R4_R5 %R6_R7 %R8_R9 %R10_R11 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D8_D10_D12_D14 %D9_D11_D13_D15 %D9_D10 %D11_D12 %D13_D14 %D9_D10_D11_D12 %D11_D12_D13_D14>, %LR<imp-def,dead>, %SP<imp-use>, %R0<imp-use>, %SP<imp-def>
  	%R0<def> = tMOVr %R9, pred:14, pred:%noreg
  	tBL pred:14, pred:%noreg, <ga:@_ZN1CC1Ev>, <regmask %LR %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R0 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R11 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q4_Q5_Q6_Q7 %R4_R5 %R6_R7 %R8_R9 %R10_R11 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D8_D10_D12_D14 %D9_D11_D13_D15 %D9_D10 %D11_D12 %D13_D14 %D9_D10_D11_D12 %D11_D12_D13_D14>, %LR<imp-def,dead>, %SP<imp-use>, %R0<imp-use>, %SP<imp-def>
  	tBL pred:14, pred:%noreg, <ga:@_ZN1CanE1A>, <regmask %LR %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R11 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q4_Q5_Q6_Q7 %R4_R5 %R6_R7 %R8_R9 %R10_R11 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D8_D10_D12_D14 %D9_D11_D13_D15 %D9_D10 %D11_D12 %D13_D14 %D9_D10_D11_D12 %D11_D12_D13_D14>, %LR<imp-def,dead>, %SP<imp-use>, %R0<imp-use>, %SP<imp-def>
  	t2CMPri %R8<kill>, 0, pred:14, pred:%noreg, %CPSR<imp-def>
  	t2Bcc <BB#13>, pred:0, pred:%CPSR<kill>
      Successors according to CFG: BB#13(0x30000000 / 0x80000000 = 37.50%) BB#12(0x50000000 / 0x80000000 = 62.50%)

  BB#12: derived from LLVM BB %if.then9
      Live Ins: %R4 %R5 %R10 %R11
      Predecessors according to CFG: BB#11
  	%R0<def> = tMOVr %R11<kill>, pred:1, pred:%CPSR
  	tBL pred:14, pred:%noreg, <ga:@_ZN1F5m_fn3Ev>, <regmask %LR %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R11 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q4_Q5_Q6_Q7 %R4_R5 %R6_R7 %R8_R9 %R10_R11 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D8_D10_D12_D14 %D9_D11_D13_D15 %D9_D10 %D11_D12 %D13_D14 %D9_D10_D11_D12 %D11_D12_D13_D14>, %LR<imp-def,dead>, %SP<imp-use>, %R0<imp-use>, %SP<imp-def>
      Successors according to CFG: BB#13(?%)

  BB#13: derived from LLVM BB %cleanup
      Live Ins: %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R0 %R4 %R5 %R10 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D6_D8 %D7_D9 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %D14_D16 %D15_D17 %Q3_Q4 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q7_Q8 %Q1_Q2_Q3_Q4 %Q2_Q3_Q4_Q5 %Q3_Q4_Q5_Q6 %Q4_Q5_Q6_Q7 %Q5_Q6_Q7_Q8 %Q6_Q7_Q8_Q9 %Q7_Q8_Q9_Q10 %R0_R1 %R4_R5 %R10_R11 %D6_D7_D8 %D7_D8_D9 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D14_D15_D16 %D15_D16_D17 %D4_D6_D8 %D5_D7_D9 %D6_D8_D10 %D7_D9_D11 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D12_D14_D16 %D13_D15_D17 %D14_D16_D18 %D15_D17_D19 %D2_D4_D6_D8 %D3_D5_D7_D9 %D4_D6_D8_D10 %D5_D7_D9_D11 %D6_D8_D10_D12 %D7_D9_D11_D13 %D8_D10_D12_D14 %D9_D11_D13_D15 %D10_D12_D14_D16 %D11_D13_D15_D17 %D12_D14_D16_D18 %D13_D15_D17_D19 %D14_D16_D18_D20 %D15_D17_D19_D21 %D7_D8 %D9_D10 %D11_D12 %D13_D14 %D15_D16 %D5_D6_D7_D8 %D7_D8_D9_D10 %D9_D10_D11_D12 %D11_D12_D13_D14 %D13_D14_D15_D16 %D15_D16_D17_D18 %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %Q4 %Q5 %Q6 %Q7 %R4 %R5 %R10 %S16 %S17 %S18 %S19 %S20 %S21 %S22 %S23 %S24 %S25 %S26 %S27 %S28 %S29 %S30 %S31 %D6_D8 %D7_D9 %D8_D10 %D9_D11 %D10_D12 %D11_D13 %D12_D14 %D13_D15 %D14_D16 %D15_D17 %Q3_Q4 %Q4_Q5 %Q5_Q6 %Q6_Q7 %Q7_Q8 %Q1_Q2_Q3_Q4 %Q2_Q3_Q4_Q5 %Q3_Q4_Q5_Q6 %Q4_Q5_Q6_Q7 %Q5_Q6_Q7_Q8 %Q6_Q7_Q8_Q9 %Q7_Q8_Q9_Q10 %R4_R5 %R10_R11 %D6_D7_D8 %D7_D8_D9 %D8_D9_D10 %D9_D10_D11 %D10_D11_D12 %D11_D12_D13 %D12_D13_D14 %D13_D14_D15 %D14_D15_D16 %D15_D16_D17 %D4_D6_D8 %D5_D7_D9 %D6_D8_D10 %D7_D9_D11 %D8_D10_D12 %D9_D11_D13 %D10_D12_D14 %D11_D13_D15 %D12_D14_D16 %D13_D15_D17 %D14_D16_D18 %D15_D17_D19 %D2_D4_D6_D8 %D3_D5_D7_D9 %D4_D6_D8_D10 %D5_D7_D9_D11 %D6_D8_D10_D12 %D7_D9_D11_D13 %D8_D10_D12_D14 %D9_D11_D13_D15 %D10_D12_D14_D16 %D11_D13_D15_D17 %D12_D14_D16_D18 %D13_D15_D17_D19 %D14_D16_D18_D20 %D15_D17_D19_D21 %D7_D8 %D9_D10 %D11_D12 %D13_D14 %D15_D16 %D5_D6_D7_D8 %D7_D8_D9_D10 %D9_D10_D11_D12 %D11_D12_D13_D14 %D13_D14_D15_D16 %D15_D16_D17_D18
      Predecessors according to CFG: BB#12 BB#11
  	%R0<def> = t2MOVi 1, pred:14, pred:%noreg, opt:%noreg
  	%R9<def> = t2ADDri %R6, 28, pred:14, pred:%noreg, opt:%noreg
  Successors according to CFG: BB#14(?%)

Before predicating BB#12, it does

  // Initialize liveins to the first BB. These are potentially redefined by
  // predicated instructions.
  Redefs.init(TRI);
  Redefs.addLiveIns(*CvtBBI->BB);
  Redefs.addLiveIns(*NextBBI->BB);

Here %R0 gets inserted into Redefs, since it is live into BB#13!

First of all, if you ask me, I'd say that the Live Ins list is suspiciously long for BB#13. I have seen and reported bad live-in lists before: https://llvm.org/bugs/show_bug.cgi?id=25263. Another point is that %R0 is actually not really live-in to BB#13, since it is overwritten unconditionally by the t2MOVi.

Regardless of live-in lists correctness, I am thinking that IfConversion should learn to not think that a live-out reg is live into both successors here. After all, it should not be impossible to have e.g. unoptimized cases where you define a reg, and conditionally redefine it in another block, or?

BTW Quentin: I do not see your post here on Phabricator for some reason. Did you perhaps just reply to the e-mail (which perhaps should have worked...)?

http://reviews.llvm.org/D20907