[PATCH] Remove redundent register mov by improving TwoAddressInstructionPass

Quentin Colombet qcolombet at apple.com
Fri Feb 27 16:30:21 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: test/CodeGen/X86/twoaddr-coalesce-3.ll:22
@@ +21,3 @@
+; CHECK-LABEL: foo:
+; CHECK: [[LOOP1:.LBB0_[0-9]+]]: # %for.body
+; CHECK-NOT: mov
----------------
chandlerc wrote:
> qcolombet wrote:
> > wmi wrote:
> > > qcolombet wrote:
> > > > Like I said, you shouldn’t use basic block labels, but rely on branch instruction on other block.
> > > > E.g, on my machine the label looks like this: LBB0_…, i.e., no leading ‘.’.
> > > > 
> > > > So, what I was saying was to use something like:
> > > > 
> > > > ; End of the first block.
> > > > CHECK: jp
> > > > ; We enter the loop
> > > > CHECK loop boby
> > > > ; The loop body is done
> > > > CHECK jp
> > > Yes, I understand your concern. However using branch instruction in previous block as boundary will introduce extra code in loop preheader, including mov. Like the following movl total(%rip), ..., I can make the test right, but it introduces extra complexity. 
> > > 
> > > 	jle	.LBB0_4
> > > # BB#1:                                 # %for.body.lr.ph
> > > 	xorl	%edx, %edx
> > > 	movl	total(%rip), %ecx
> > > 	.align	16, 0x90
> > > .LBB0_2:                                # %for.body
> > >                                         # =>This Inner Loop Header: Depth=1
> > > 	movl	%edx, %esi
> > > 	shrl	$31, %esi
> > > 	addl	%edx, %esi
> > > 	sarl	%esi
> > > 	addl	%esi, %ecx
> > > 	incl	%edx
> > > 	cmpl	%eax, %edx
> > > 	jl	.LBB0_2
> > > 
> > > Do you like CHECK: [[LOOP:[.]?LBB0_[0-9]+]]; ? If not, I will follow your way to use last branch as the boundary.
> > In that case, I would use 'movl total' as an anchor.
> > 
> > Another way to avoid this label problem is to specify a -mtriple instead of just a -march. Anyhow, I am not sure labels are stable between debug and assert builds. Same thing for the assembly comments, i.e., I am not sure your output will contain:  # %for.body.
> > 
> > The bottom line is that I really think you shouldn't rely on labels, but I may be wrong of course!
> FWIW, I agree that relying on specific label naming isn't a great strategy. However, I think for assembly tests, the instruction comments are very valuable and we should always keep them stable. For example, all of the shuffle testing using the comments rather than checking magic numbers. So I would suggest
> 
> [[LOOP1:^[.\w]+:]]: # %for.body
> 
> Essentially, match the specific x86 pattern for labels, any label, and the comment to identify which one.
Agreed.

I was not sure the comment were printed in release mode, but now that I think about this it happens only if the would pipeline is in release mode.

http://reviews.llvm.org/D7806

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list