[llvm-commits] [llvm] r130206 - in /llvm/trunk: lib/Transforms/Scalar/GVN.cpp test/Transforms/GVN/rle.ll test/Transforms/SimplifyCFG/2006-08-03-Crash.ll

[Roman Divacky]

On Tue, Apr 26, 2011 at 01:08:57PM -0700, Chris Lattner wrote:
> On Apr 26, 2011, at 12:14 PM, Roman Divacky wrote:
> > this is causing some strange regressions here (with greedy regalloc but
> > otherwise vanilla llvm):
> > 
> > before:
> > 
> > cmpb    $69, 41(%esp)
> > 
> > after:
> > 
> > movl    %eax, %ecx
> > movb    %ch, %cl
> > cmpb    $69, %cl
> > 
> > and some more
> 
> That is pretty nasty codegen, but GVN is doing the right thing here, it is trading a load for some trivial integer operations.  This may not be the best for code size, but it is the right thing to do.  FWIW, this is one in a long series of patches I'm working on to fix PR6627 for you.
> 
> Here is what the code looked like before (after straightening out control flow):
> 
> 	movl	40(%esp), %eax
> 	cmpw	$267, %ax               # imm = 0x10B
> 	jne	.LBB7_9
> ...
> .LBB7_9:                                # %if.else
> 	cmpb	$127, %al
> 	jne	.LBB7_4
> # BB#10:                                # %land.lhs.true
> 	cmpb	$69, 41(%esp)
> 	jne	.LBB7_4
> # BB#11:                                # %land.lhs.true49
> 	cmpb	$76, 42(%esp)
> 	jne	.LBB7_4
> # BB#12:                                # %land.lhs.true57
> 	cmpb	$70, 43(%esp)
> 	jne	.LBB7_4
> # BB#13:                                # %if.then65
> 
> Now we have:
> 
> 	movl	40(%esp), %eax
> 	cmpw	$267, %ax               # imm = 0x10B
> 	jne	.LBB7_9
> ...
> .LBB7_9:                                # %if.else
> 	movl	%eax, %ecx
> 	cmpb	$127, %cl
> 	jne	.LBB7_4
> # BB#10:                                # %if.else
> 	movl	%eax, %ecx
> 	movb	%ch, %cl
> 	cmpb	$69, %cl
> 	jne	.LBB7_4
> # BB#11:                                # %if.else
> 	movl	%eax, %ecx
> 	shrl	$16, %ecx
> 	cmpb	$76, %cl
> 	jne	.LBB7_4
> # BB#12:                                # %land.lhs.true57
> 	andl	$-16777216, %eax        # imm = 0xFFFFFFFFFF000000
> 	cmpl	$1174405120, %eax       # imm = 0x46000000
> 	jne	.LBB7_4
> 
> 
> This instcombine patch is the next step along the way (which will probably improve this, but not get us all the way to the goal), but is causing some codegen pessimizations that I have to deal with before it can land:

Yes, I realized that this is actually an improvement after I sent the mail :)

Anyway, on FreeBSD boot loader I am seeing GVN hurting the codesize.
Removing GVN saves 84 bytes, disabling LoadPRE but keeping the rest
of GVN saves 36 bytes. Thats 0.5% - 1%.

I wonder what to do. If LoadPRE in GVN generally makes things worse
codesize wise maybe it should be disabled when optimizing for size?
Maybe disable GVN at -Os completely?

What do you think?