Bug 35013 - Incomplete check in RTL for "pm()" annotation
Summary: Incomplete check in RTL for "pm()" annotation
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.2.2
: P3 normal
Target Milestone: 4.5.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-29 09:33 UTC by Wouter van Gulik
Modified: 2009-12-29 21:22 UTC (History)
3 users (show)

See Also:
Host:
Target: avr-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-04-03 22:28:59


Attachments
Patch (941 bytes, patch)
2008-02-16 22:06 UTC, Andy Hutchinson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wouter van Gulik 2008-01-29 09:33:13 UTC
All credits for finding the root of the evil go to Andrew Hutchinson 
His original e-mail:

There is a bug!

Backend (avr part) creates special "pm()" annotation by looking at RTL. This is done in avr.c by this function:

static bool
avr_assemble_integer (rtx x, unsigned int size, int aligned_p)
{
debug_rtx(x); /*ADDED TO DEBUG*/
fprintf(stderr,"size=%d align=%d\n\n",size,aligned_p); /*ADDED TO DEBUG*/
 if (size == POINTER_SIZE / BITS_PER_UNIT && aligned_p
     && ((GET_CODE (x) == SYMBOL_REF && SYMBOL_REF_FUNCTION_P (x))
     || GET_CODE (x) == LABEL_REF))
   {
     fputs ("\t.word\tpm(", asm_out_file);
     output_addr_const (asm_out_file, x);
     fputs (")\n", asm_out_file);
     return true;
   }
 return default_assemble_integer (x, size, aligned_p);
}

(I added 2 lines to debug it)

You will see it looks symbol_ref or label_ref - otherwise it does not use pm.

So I ran it with following testcase to see what argument get sent:

//Dummy func
int table[]= {1,2};
char ctable[]= {3,4};
void foo(void) {}
 
//Table with address manipulation
void (* const pFuncTable[]) (void) = {
  foo + 0,
  (foo + 1),
  foo +2
};

....and we get the following:

(const_int 1 [0x1])
size=2 align=1

(const_int 2 [0x2])
size=2 align=1

(const_int 3 [0x3])
size=1 align=1

(const_int 4 [0x4])
size=1 align=1

(symbol_ref:HI ("foo") [flags 0x3] <function_decl 0x7fdcf030 foo>)
size=2 align=1

(const:HI (plus:HI (symbol_ref:HI ("foo") [flags 0x3] <function_decl 0x7fdcf030 foo>)
       (const_int 1 [0x1])))
size=2 align=1

(const:HI (plus:HI (symbol_ref:HI ("foo") [flags 0x3] <function_decl 0x7fdcf030 foo>)
       (const_int 2 [0x2])))
size=2 align=1

You will see last two cases are an expression with outer code as const:HI so it will not match, it does not add pm().
Comment 1 Andy Hutchinson 2008-02-16 22:06:23 UTC
Created attachment 15169 [details]
Patch

The attached  patch allows function address expressions of the form address+k to be correctly recognized as program memory addresses and thus force use of pm() assembler syntax.

This has not been extensively tested but assembler appears to be correct for this bugs testcase and a similar issue found in:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27192

Note, odd addresses will be accepted by C and only cause linker warning.
Comment 2 Andy Hutchinson 2009-12-24 19:54:16 UTC
Subject: Bug 35013

Author: hutchinsonandy
Date: Thu Dec 24 19:53:57 2009
New Revision: 155459

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155459
Log:
2009-12-24  Andy Hutchinson  <hutchinsonandy@gcc.gnu.org>

	PR target/35013, 27192
	* config/avr/avr.c (print_operand_address): Print correct program
	memory address.
	Add warning for large device offset addresses.
	(avr_assemble_integer): Ditto.
	(print_operand): Add warnings for incorrect addressing.
	(out_movqi_r_mr): Tag assembler with new address codes.
	(out_movhi_r_mr): Ditto.
	(out_movsi_r_mr): Ditto.
	(out_movqi_mr_r): Ditto.
	(out_movhi_mr_r): Ditto.
	(out_movsi_mr_r): Ditto.
	* config/avr/predicates.md (text_segment_operand): New predicate.
	* config/avr/avr.md (jump): Tag assembler with new address codes.
	(call_insn): Ditto.
	(call_value_insn): Ditto.
	(*tablejump_lib): Ditto.
	(*cbi): Ditto.
	(*sbi): Ditto.
	(indirect_jump): New define_expand.
	(jcindirect_jump): New pattern for constant expression jump.
	(njcindirect_jump): Renamed old indirect_jump.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/predicates.md

Comment 3 Andy Hutchinson 2009-12-24 19:58:10 UTC
Fixed 4.5