[PATCH] D38359: [X86] Ignore DBG instructions in X86CmovConversion optimization to resolve PR34565

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 3 00:54:40 PDT 2017


chandlerc added a reviewer: rnk.
chandlerc added a comment.

In https://reviews.llvm.org/D38359#886781, @aaboud wrote:

> Thanks Chandler for reviewing the code.
>
> > Is there any way that the debug instructions can become invalid due to this transform? I'm not seeing anything obvious, but wanted to make sure you've thought about this too.
>
> This is how the code I am trying to fix looks like:
>
>   %vreg2<def,tied1> = CMOVB32rr %vreg6<tied0>, %vreg1, %EFLAGS<imp-use>
>   DBG_VALUE %vreg2, %noreg, !"b", <!DIExpression()>; GR32:%vreg2 line no:7
>   DBG_VALUE %vreg2, %noreg, !"b", <!DIExpression()>; GR32:%vreg2 line no:7
>   %vreg3<def,tied1> = CMOVB32rr %vreg0<tied0>, %vreg6, %EFLAGS<imp-use>
>
>
> I assume 3 facts at this point:
>
> 1. DBG_VALUE has only virtual register and cannot have physical register, that is true because we are still working with in SSA machine code.
> 2. DBG_VALUE instruction generates a void value, i.e., no instruction can use it (or refer to it).
> 3. When we replace all uses of the CMOV instruction with the new PHINode instruction, that also changes the virtual register in the DBG_VALUE.
>
>   Saying that, we can always move all DBG_VALUE instructions forward up to the end of Machine Basic Block, right?


Well, that's what I'm trying to make sure of...

I'm not an MI expert, but in the past I have found code motion at MI-time to be somewhat tricky.

Do we have kill flags set? If so, we could move a use of a virtual register across its kill... But maybe there is some reason to assure that this is not the case. Looping in Reid as he has been looking at DBG_VALUE a lot more closely than I have recently and may be able to quickly reassure both of us that this is safe. =]



================
Comment at: lib/Target/X86/X86CmovConversion.cpp:580-584
+  SmallVector<MachineInstr *, 2> DBGInstructions;
+  for (auto I = First->getIterator(), E = Last->getIterator(); I != E; I++) {
+    if (I->isDebugValue())
+      DBGInstructions.push_back(&*I);
+  }
----------------
Any particular reason you moved to a vector rather than doing the increment first, and then splicing?


https://reviews.llvm.org/D38359





More information about the llvm-commits mailing list