Bug 49687 - [4.6 Regression][avr] Missed optimization for widening MUL
Summary: [4.6 Regression][avr] Missed optimization for widening MUL
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.1
: P3 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
: 39250 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-07-09 09:36 UTC by Georg-Johann Lay
Modified: 2011-09-09 17:57 UTC (History)
2 users (show)

See Also:
Host:
Target: avr
Build:
Known to work:
Known to fail: 4.6.1
Last reconfirmed: 2011-07-14 18:45:17


Attachments
C Test case (111 bytes, text/plain)
2011-07-09 09:37 UTC, Georg-Johann Lay
Details
Test case results using GCC 4.3.3 (WinAVR 20100110) (675 bytes, application/octet-stream)
2011-07-14 18:41 UTC, Eric Weddington
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Georg-Johann Lay 2011-07-09 09:36:23 UTC
With avr-gcc 4.6.1 and 
-Os -mmcu=atmega88 -S -dp

int mul8_55 (char x)
{
    return x * 55;
}

gets compiled to 

mul8_55:
	ldi r25,lo8(55)	 ;  7	*movqi/2	[length = 1]
	muls r24,r25	 ;  8	mulqihi3	[length = 3]
	movw r24,r0
	clr r1
	ret	 ;  26	return	[length = 1]

which is fine.  If the constant is 126, however, the result is bloated and might be related to PR36467, i.e. MUL* is better than shift.

mul8_126:
	clr r25	 ;  7	extendqihi2/1	[length = 3]
	sbrc r24,7
	com r25
	movw r18,r24	 ;  28	*movhi/1	[length = 1]
	lsl r18	 ;  33	*ashlhi3_const/2	[length = 2]
	rol r19
	lsr r25	 ;  34	*ashlhi3_const/5	[length = 5]
	mov r25,r24
	clr r24
	ror r25
	ror r24
	sub r24,r18	 ;  12	subhi3/1	[length = 2]
	sbc r25,r19
	ret	 ;  31	return	[length = 1]

If the constant is 155, a MUL is invented, but a widening MUL would be smarter:

mul8_155:
	mov r20,r24	 ;  6	extendqihi2/2	[length = 4]
	clr r21
	sbrc r20,7
	com r21
	ldi r18,lo8(155)	 ;  7	*movhi/4	[length = 2]
	ldi r19,hi8(155)
	mul r20,r18	 ;  8	*mulhi3_enh	[length = 7]
	movw r24,r0
	mul r20,r19
	add r25,r0
	mul r21,r18
	add r25,r0
	clr r1
	ret	 ;  26	return	[length = 1]
Comment 1 Georg-Johann Lay 2011-07-09 09:37:40 UTC
Created attachment 24724 [details]
C Test case
Comment 2 Eric Weddington 2011-07-14 18:41:56 UTC
Created attachment 24758 [details]
Test case results using GCC 4.3.3 (WinAVR 20100110)
Comment 3 Eric Weddington 2011-07-14 18:45:17 UTC
The test case results using GCC 4.3.3 (WinAVR 20100110) shows that the mul8_126 case is smaller:

.global	mul8_126
	.type	mul8_126, @function
mul8_126:
/* prologue: function */
/* frame size = 0 */
	mov r18,r24	 ;  2	*movqi/1	[length = 1]
	ldi r24,lo8(126)	 ;  6	*movqi/2	[length = 1]
	muls r18,r24	 ;  7	mulqihi3	[length = 3]
	movw r18,r0
	clr r1
	movw r24,r18	 ;  32	*movhi/1	[length = 1]
/* epilogue start */
	ret	 ;  30	return	[length = 1]
	.size	mul8_126, .-mul8_126

So this is a regression wrt to gcc 4.6.1.

The case of mul8_155 shows the identical problem as for gcc 4.6.1.
Comment 4 Georg-Johann Lay 2011-07-20 17:23:31 UTC
Author: gjl
Date: Wed Jul 20 17:23:28 2011
New Revision: 176527

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176527
Log:

	PR target/36467
	PR target/49687
	* config/avr/avr.md (mulhi3): Use register_or_s9_operand for
	operand2 and expand appropriately if there is a CONST_INT in
	operand2.
	(usmulqihi3): New insn.
	(*sumulqihi3): New insn.
	(*osmulqihi3): New insn.
	(*oumulqihi3): New insn.
	(*muluqihi3.uconst): New insn_and_split.
	(*muluqihi3.sconst): New insn_and_split.
	(*mulsqihi3.sconst): New insn_and_split.
	(*mulsqihi3.uconst): New insn_and_split.
	(*mulsqihi3.oconst): New insn_and_split.
	(*ashifthi3.signx.const): New insn_and_split.
	(*ashifthi3.signx.const7): New insn_and_split.
	(*ashifthi3.zerox.const): New insn_and_split.
	(mulsqihi3): New insn.
	(muluqihi3): New insn.
	(muloqihi3): New insn.
	* config/avr/predicates.md (const_2_to_7_operand): New.
	(const_2_to_6_operand): New.
	(u8_operand): New.
	(s8_operand): New.
	(o8_operand): New.
	(s9_operand): New.
	(register_or_s9_operand): New.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/predicates.md
Comment 5 Georg-Johann Lay 2011-07-28 08:03:12 UTC
Author: gjl
Date: Thu Jul 28 08:03:07 2011
New Revision: 176862

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176862
Log:
	
	PR target/49687
	* config/avr/t-avr (LIB1ASMFUNCS): Remove _xmulhisi3_exit.
	Add _muluhisi3, _mulshisi3, _usmulhisi3.
	* config/avr/libgcc.S (__mulsi3): Rewrite.
	(__mulhisi3): Rewrite.
	(__umulhisi3): Rewrite.
	(__usmulhisi3): New.
	(__muluhisi3): New.
	(__mulshisi3): New.
	(__mulohisi3): New.
	(__mulqi3, __mulqihi3, __umulqihi3, __mulhi3): Use DEFUN/ENDF to
	declare.
	* config/avr/predicates.md (pseudo_register_operand): Rewrite.
	(pseudo_register_or_const_int_operand): New.
	(combine_pseudo_register_operand): New.
	(u16_operand): New.
	(s16_operand): New.
	(o16_operand): New.
	* config/avr/avr.c (avr_rtx_costs): Handle costs for mult:SI.
	* config/avr/avr.md (QIHI, QIHI2): New mode iterators.
	(any_extend, any_extend2): New code iterators.
	(extend_prefix): New code attribute.
	(mulsi3): Rewrite. Turn insn to expander.
	(mulhisi3): Ditto.
	(umulhisi3): Ditto.
	(usmulhisi3): New expander.
	(*mulsi3): New insn-and-split.
	(mulu<mode>si3): New insn-and-split.
	(muls<mode>si3): New insn-and-split.
	(mulohisi3): New insn-and-split.
	(*uumulqihisi3, *uumulhiqisi3, *uumulhihisi3, *uumulqiqisi3,
	*usmulqihisi3, *usmulhiqisi3, *usmulhihisi3, *usmulqiqisi3,
	*sumulqihisi3, *sumulhiqisi3, *sumulhihisi3, *sumulqiqisi3,
	*ssmulqihisi3, *ssmulhiqisi3, *ssmulhihisi3, *ssmulqiqisi3): New
	insn-and-split.
	(*mulsi3_call): Rewrite.
	(*mulhisi3_call): Rewrite.
	(*umulhisi3_call): Rewrite.
	(*usmulhisi3_call): New insn.
	(*muluhisi3_call): New insn.
	(*mulshisi3_call): New insn.
	(*mulohisi3_call): New insn.
	(extendqihi2): Use combine_pseudo_register_operand as predicate
	for operand 1.
	(extendqisi2): Ditto.
	(zero_extendqihi2): Ditto.
	(zero_extendqisi2): Ditto.
	(zero_extendhisi2): Ditto.
	(extendhisi2): Ditto. Don't early-clobber operand 0.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/libgcc.S
    trunk/gcc/config/avr/predicates.md
    trunk/gcc/config/avr/t-avr
Comment 6 Georg-Johann Lay 2011-07-29 11:27:45 UTC
Author: gjl
Date: Fri Jul 29 11:27:39 2011
New Revision: 176923

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176923
Log:
	
	PR target/49687
	* config/avr/avr.md (mulsi3, *mulsi3, mulu<mode>si3,
	muls<mode>si3, mulohisi3, mulhisi3, umulhisi3, usmulhisi3,
	*<any_extend:extend_prefix><any_extend2:extend_prefix>mul<QIHI:mode><QIHI2:mode>si3):
	Add X to register footprint: Clobber r26/r27.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.md
Comment 7 Georg-Johann Lay 2011-07-29 11:49:30 UTC
With this milestone the work for better support for widening multiplication is finished -- fir the case when MUL instruction is available.

Similar work for the case when no hardware multiplier is available needs still to be done.

The preferred approach in that case is to do the extension explicitely in libgcc prior to the multiplication.  Respective code was already included in libgcc but dead because no one calls it up to now.
Comment 8 Georg-Johann Lay 2011-08-11 07:50:42 UTC
Author: gjl
Date: Thu Aug 11 07:50:37 2011
New Revision: 177648

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177648
Log:
	
	PR target/49687
	* config/avr/avr.md (smulqi3_highpart): New insn.
	(umulqi3_highpart): New insn.
	(*subqi3.ashiftrt7): New insn.
	(smulhi3_highpart): New expander.
	(umulhi3_highpart): Nex expander.
	(*smulhi3_highpart_call): New insn.
	(*umulhi3_highpart_call): New insn.
	(extend_u): New code attribute.
	(extend_prefix): Rename code attribute to extend_su.
	* config/avr/avr.c (avr_rtx_costs): Report costs of highpart of
	widening QI/HI multiply.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
Comment 9 Georg-Johann Lay 2011-08-11 21:43:47 UTC
*** Bug 39250 has been marked as a duplicate of this bug. ***
Comment 10 Georg-Johann Lay 2011-09-09 17:57:34 UTC
Closed for the 4.7.0 Milestone.