Bug 41076 - [avr] pessimal code for logical OR of 8-bit fields
Summary: [avr] pessimal code for logical OR of 8-bit fields
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.1
: P4 normal
Target Milestone: ---
Assignee: Georg-Johann Lay
URL:
Keywords: missed-optimization
: 60145 (view as bug list)
Depends on:
Blocks: 65964
  Show dependency treegraph
 
Reported: 2009-08-15 07:42 UTC by Philip Blundell
Modified: 2015-05-01 13:55 UTC (History)
2 users (show)

See Also:
Host:
Target: avr
Build:
Known to work:
Known to fail: 4.8.0, 4.9.1
Last reconfirmed: 2014-10-07 00:00:00


Attachments
Some test cases (208 bytes, text/plain)
2011-09-11 20:59 UTC, Georg-Johann Lay
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Blundell 2009-08-15 07:42:54 UTC
This trivial function:

unsigned short f(unsigned char a, unsigned char b)
{
  return a | (b << 8);
}

generates the code:

f:
/* prologue: function */
/* frame size = 0 */
	mov r21,r22
	ldi r20,lo8(0)
	mov r18,r24
	ldi r19,lo8(0)
	or r18,r20
	or r19,r21
	mov r24,r18
	mov r25,r19
/* epilogue start */
	ret

In other words, GCC is first zero-extending the two 8-bit quantities to 16 bits, then performing a 16-bit logical OR operation on the result.  For each half of the OR operation, though, one side is a constant zero and hence these operations are redundant.

In this particular trivial case, the function could be implemented as:

mov r25, r21
ret

with all the other operations omitted.
Comment 1 Richard Biener 2009-08-15 09:55:08 UTC
because this is what C says you are doing:

  return (unsigned short) ((int) a | ((int) b << 8));

which is harder to transform back into what you expect.
Comment 2 Georg-Johann Lay 2011-05-16 14:20:25 UTC
Author: gjl
Date: Mon May 16 14:20:19 2011
New Revision: 173792

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173792
Log:
	PR target/27663
	PR target/41076
	* config/avr/predicates.md (const_8_16_24_operand): New predicate.
	* config/avr/avr.md ("*ior<mode>qi.byte0",
	"*ior<mode>qi.byte1-3"): New define_insn_and_split patterns.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/predicates.md
Comment 3 Georg-Johann Lay 2011-09-11 20:59:28 UTC
Created attachment 25242 [details]
Some test cases
Comment 4 Richard Biener 2012-03-22 08:26:23 UTC
GCC 4.7.0 is being released, adjusting target milestone.
Comment 5 Richard Biener 2012-07-02 13:52:40 UTC
Fixed I suppose.
Comment 6 Georg-Johann Lay 2013-01-22 22:31:11 UTC
(In reply to comment #5)
> Fixed I suppose.

Unfortunately, not

extern unsigned char read8 (void);

unsigned short read16 (void)
{
    unsigned char lo, hi;

    hi = read8();
    lo = read8();
    return lo | hi << 8;
}


with 4.8.0 -S -Os the compiler needs 5 instructions (29, 37, 30, 38) for an operation that is basically a no-op:


read16:
	push r28	 ;  31	pushqi1/1	[length = 1]
/* prologue: function */
/* frame size = 0 */
/* stack size = 1 */
.L__stack_usage = 1
	call read8	 ;  5	call_value_insn/2	[length = 2]
	mov r28,r24	 ;  6	movqi_insn/1	[length = 1]
	call read8	 ;  7	call_value_insn/2	[length = 2]
	mov r18,r28	 ;  28	movqi_insn/1	[length = 1]
	ldi r19,0	 ;  29	movqi_insn/1	[length = 1]
	mov r19,r18	 ;  37	*ashlhi3_const/3	[length = 2]
	clr r18
	or r18,r24	 ;  30	iorqi3/1	[length = 1]
	movw r24,r18	 ;  38	*movhi/1	[length = 1]
/* epilogue start */
	pop r28	 ;  34	popqi	[length = 1]
	ret	 ;  35	return_from_epilogue	[length = 1]
Comment 7 Georg-Johann Lay 2014-02-17 16:35:13 UTC
*** Bug 60145 has been marked as a duplicate of this bug. ***