Reviving old port

2008-05-12 Thread Omar
Hi All,

BACKGROUND:
Downloaded the source code of the Xemics CoolRISC gcc port from Raisonance's
website (ftp://www.raisonance.com/pub/CoolRISC/cool3_2.zip). I
concluded that the port is based in gcc 3.2 from looking at the NEWS
file in the gcc folder.

I what to bring to this gcc port up to the latest version of gcc.

The motivations for doing this are:
 1- Gain experience on how to port gcc. This will be useful in the
not-so-distant future.
 2- Fix bugs in current version of port. For example: the #pragma
interrupt do not work correctly, the size of structures is limited to
255 (otherwise the generated code is broken).
 3- Try to improve the generated code density

CURRENT STATUS:
 1- I downloaded the latest version of gcc. Then I took the
target-specific files from the CoolRISC port (c816.md, c816.c and
c816.h) and add them to the latest gcc code under the gcc/config/c816
folder.
 2- Fixed multiple poised #defines in the c816.h file
 3- Changed references to gen_rtx ( RTXCODE, ...) to gen_rtx_RTXCODE
(...) in the c816.c file.
 4- Currently, I am able to compile the updated files. Furthermore, I
used the cc1 to do test compiles and the output asm code looks
promising.

NEXT STEPS:
1- Modify code to make use of newer gcc techniques: ie, make use of
define_constraint
instead of using the old #define method.
2- Better define the cpu using the md file's rtl macros (I want to
move away from the current approach of relying heavily in the c816.c
to do the rtl->asm transformations).

QUESTIONS:
 1-The c816's port is heavily using the c816.c instead of better
describing the cpu in the c816.md file. For instance, looking at the
define_insn movhi:
;; movhi
(define_insn "movhi"
 [(set (match_operand:HI 0 "general_operand" "=g")
   (match_operand:HI 1 "general_operand" "g"))]
 ""
 "*
   {
 return output_move(insn, operands);
   }
 ")

 I noticed that predicates and constraints are essentially disabled,
and the output_move function in the c816.c  does all the work. Also,
since c816 can only move bytes, it looks to me like this definition is
incorrectly since it is basically lying to gcc and making it believe
that there is a two byte move. Is a define_split (or a define_expand)
more appropriate in this situation?

 2- I am trying to sort-out define_expand from define_split. Looks
like they can be used to perform similar tasks. By looking at other
ports I came to the following conclusions (I gather this from reading
multiple posts, please correct me if this statements are not true):
 2.1  define_split should be used to break an operation into simpler
operations *of a different mode*. That is, to describe a movhi in
terms of two movqi I should use a define_split instead of a
define_expand.
 2.2 the define_expand expects the mode of the matching rtl macro and
the output operations to be preserve. Otherwise gcc could get confuse
since the mode will be change for no apparent reason.

Thanks for your help.
Best regards,
-Omar


real.h: REAL_VALUE_TO_TARGET_DOUBLE

2008-10-02 Thread Omar Torres
Hi All,
 Shouldn't this macro:
#define REAL_VALUE_TO_TARGET_DOUBLE(IN, OUT) \
  real_to_target (OUT, &(IN), mode_for_size (64, MODE_FLOAT, 0))

 be using DOUBLE_TYPE_SIZE instead of the hard coded '64'? Am I missing
something here?

In the target I am currently working, DOUBLE_TYPE_SIZE is defined as
32-bit, so the hard-coded '64' is causing trouble. I am planning on
doing this change on my local source tree, but wanted to discuss with
the community to see if there is some implementations details somewhere
that I need to consider as well.

Best regards,
-Omar


Re: real.h: REAL_VALUE_TO_TARGET_DOUBLE

2008-10-03 Thread Omar Torres
On Thu, Oct 2, 2008 at 11:59 PM, Ian Lance Taylor <[EMAIL PROTECTED]> wrote:
> "Omar Torres" <[EMAIL PROTECTED]> writes:
>
>>  Shouldn't this macro:
>> #define REAL_VALUE_TO_TARGET_DOUBLE(IN, OUT) \
>>   real_to_target (OUT, &(IN), mode_for_size (64, MODE_FLOAT, 0))
>>
>>  be using DOUBLE_TYPE_SIZE instead of the hard coded '64'? Am I missing
>> something here?
>
> That would certainly be more logical.  In order to make that change,
> you would have to consider each use of the macro to confirm that the
> change would be valid.
>
>
>> In the target I am currently working, DOUBLE_TYPE_SIZE is defined as
>> 32-bit, so the hard-coded '64' is causing trouble. I am planning on
>> doing this change on my local source tree, but wanted to discuss with
>> the community to see if there is some implementations details somewhere
>> that I need to consider as well.
>
> As far as I can see, all but one use of this macro is in target
> dependent code.  Are you having trouble from the single target
> independent use?  Or perhaps your target dependent code should just
> use REAL_VALUE_TO_TARGET_SINGLE instead?
>
> Ian
>
 Both.
 Yes, I can use REAL_VALUE_TO_TARGET_SINGLE in the target dependent
files to work around this.
 I do not see a similar solution for the target independent code,
since the size mismatch cause the incorrect format (encode/decode
functions) to be use.

Thanks,
-Omar


Re: real.h: REAL_VALUE_TO_TARGET_DOUBLE

2008-10-03 Thread Omar Torres
On Fri, Oct 3, 2008 at 1:49 PM, Richard Henderson <[EMAIL PROTECTED]> wrote:
> The size of the C type "double" is controlled by DOUBLE_TYPE_SIZE,
> not the size of the compiler mode "DFmode".  This macro is referring
> to the latter -- a double-precision floating point mode.
>
>
> r~
>

Richard,
 I do not fully understand your comments, but I will like to. So your
pacience is greatly appreciated.
 Here are some more specific's about the target I am working with:
float maps to 3 byte (TQF) and and double maps to 4 byte (SF). This is
how my target/target-modes.def file looks like:
  FLOAT_MODE (TQF, 3, cool_float_format);
  RESET_FLOAT_FORMAT (SF, cool_float_format);

 Now, given this information, looks like the macro in question
(REAL_VALUE_TO_TARGET_DOUBLE) is implicitly forcing a 64-bit (DF)
encoding into my target, which do not really exists. I understand my
target is an outliner, but shouldn't the DOUBLE_TYPE_SIZE be used in
this macro to cover targets like mine?

Thanks,
-Omar


divmodsi4

2008-10-09 Thread Omar Torres
Hi All,
 I am having trouble distinguishing div vs mod while implementing the
divmodsi4 instruction. The gccint documentation states:
"If an instruction that just produces a quotient or just a remainder
exists and is more efficient than the instruction that produces both,
write the output routine of 'divmodm4' to call find_reg_note and look
for a REG_UNUSED note on the quotient or remainder and generate the
appropriate instruction."

 The problem is that both, the quotient and reminder, registers are
getting marked with a REG_UNUSED note:

 (insn 12 11 17 (parallel [
(set (reg:SI 1 %r3 [33])
(div:SI (reg:SI 1 %r3 [30])
(reg:SI 5 %iph [orig:31 current ] [31])))
(set (reg:SI 5 %iph [34])
(mod:SI (reg:SI 1 %r3 [30])
(reg:SI 5 %iph [orig:31 current ] [31])))
]) 56 {*divmodsi4} (insn_list:REG_DEP_TRUE 10
(insn_list:REG_DEP_TRUE 11 (nil)))
(expr_list:REG_UNUSED (reg:SI 5 %iph [34])
(expr_list:REG_UNUSED (reg:QI 2 %r2)
(expr_list:REG_UNUSED (reg:QI 1 %r3)
(nil)

Any suggestions on how I might be able to work around this?
Thanks!
-Omar


Here is my expander and  matching patter:
(define_expand "divmodsi4"
  [(parallel [(set (match_operand:SI 0 "register_operand" "")
   (div:SI (match_operand:SI 1 "general_operand" "")
   (match_operand:SI 2 "general_operand" "")))
  (set (match_operand:SI 3 "nonimmediate_operand" "")
   (mod:SI (match_dup 1) (match_dup 2)))])]
  ""
  "")

(define_insn "*divmodsi4"
  [(set (match_operand:SI 0 "register_operand" "=f")
(div:SI (match_operand:SI 1 "general_operand" "g")
(match_operand:SI 2 "general_operand" "g")))
   (set (match_operand:SI 3 "nonimmediate_operand" "=g")
(mod:SI (match_dup 1) (match_dup 2)))]
  "!TARGET_SOFTLIB"
  "*
return output_divmod (insn, operands, 1);
  ")


Re: divmodsi4

2008-10-13 Thread Omar Torres
On Fri, Oct 10, 2008 at 10:40 AM, Dave Korn <[EMAIL PROTECTED]> wrote:
> Ian Lance Taylor wrote on 10 October 2008 15:53:
>
>> "Omar Torres" <[EMAIL PROTECTED]> writes:
>>
>>>  The problem is that both, the quotient and reminder, registers are
>>> getting marked with a REG_UNUSED note:
>>>
>>>  (insn 12 11 17 (parallel [
>>> (set (reg:SI 1 %r3 [33])
>>> (div:SI (reg:SI 1 %r3 [30])
>>> (reg:SI 5 %iph [orig:31 current ] [31])))
>>> (set (reg:SI 5 %iph [34])
>>> (mod:SI (reg:SI 1 %r3 [30])
>>> (reg:SI 5 %iph [orig:31 current ] [31])))
>>> ]) 56 {*divmodsi4} (insn_list:REG_DEP_TRUE 10
>>> (insn_list:REG_DEP_TRUE 11 (nil)))
>>> (expr_list:REG_UNUSED (reg:SI 5 %iph [34])
>>> (expr_list:REG_UNUSED (reg:QI 2 %r2)
>>> (expr_list:REG_UNUSED (reg:QI 1 %r3)
>>> (nil)
>>>
>>> Any suggestions on how I might be able to work around this?
>>
>> This makes it sounds like the whole insn is useless.  I wonder why it
>> hasn't simply been deleted?
>
>  Is it not unusual that the insn sets r1 in SImode, but the note says it is
> unused in QImode?  Does that mean only the lowest byte is unused?
>
>cheers,
>  DaveK
> --
> Can't think of a witty .sigline today
>
>

 Hi, thanks for the comments.

 A closer look revealed that this insn is not deleted because it is
needed to (partially) set (reg:SI 33).
 But, as Dave suggested, only the lower part of the quotient is used
afterwards. Below is the src code and the corresponding basic block.

 I suspect that a workaround will be to look not only for the presence
of the REG_UNUSED note, but check check the reg's mode as well, to
successfully determine whether the quotient or remainder  is needed in
divmodsi4.

 Do you agree this will be a practical approach, or is it fundamentally flawed?

Thanks,
-Omar


(machine's word size is 8-bits)

int Resistance (int volt, int current)
{
return ((long) volt<<10)/current;
}

;; basic block 2, loop depth 0, count 0
;; prev block 0, next block 1
;; pred:   ENTRY [100.0%]  (fallthru)
;; succ:   EXIT [100.0%]  (fallthru)
;; Registers live at start:  1 [%r3] 2 [%r2] 3 [%r1] 4 [%r0] 11 [%i2h]
13 [%i3h] 16 [%ap_hi] 18 [%fp_hi]
(note 6 2 3 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 3 6 4 2 (set (reg/v:HI 26 [ volt ])
(reg:HI 1 %r3 [ volt ])) 1 {movhi} (nil)
(nil))
(insn 4 3 5 2 (set (reg/v:HI 27 [ current ])
(reg:HI 3 %r1 [ current ])) 1 {movhi} (nil)
(nil))
(note 5 4 8 2 NOTE_INSN_FUNCTION_BEG)
(insn 8 5 9 2 (set (reg:SI 29 [ volt ])
(sign_extend:SI (reg/v:HI 26 [ volt ]))) 149 {extendhisi2} (nil)
(nil))
(insn 9 8 10 2 (set (reg:SI 1 %r3)
(ashift:SI (reg:SI 29 [ volt ])
(const_int 10 [0xa]))) 71 {*ashlsi3} (nil)
(nil))
(insn 10 9 11 2 (set (reg:SI 30)
(reg:SI 1 %r3)) 2 {movsi} (nil)
(expr_list:REG_EQUAL (ashift:SI (reg:SI 29 [ volt ])
(const_int 10 [0xa]))
(nil)))
(insn 11 10 12 2 (set (reg:SI 31 [ current ])
(sign_extend:SI (reg/v:HI 27 [ current ]))) 149 {extendhisi2} (nil)
(nil))
(insn 12 11 17 2 (parallel [
(set (reg:SI 33)
(div:SI (reg:SI 30)
(reg:SI 31 [ current ])))
(set (reg:SI 34)
(mod:SI (reg:SI 30)
(reg:SI 31 [ current ])))
]) 56 {*divmodsi4} (nil)
(nil))
(note 17 12 20 2 NOTE_INSN_FUNCTION_END)
(insn 20 17 26 2 (set (reg/i:HI 1 %r3 [  ])
(subreg:HI (reg:SI 33) 2)) 1 {movhi} (insn_list:REG_DEP_TRUE 12 (nil))
(expr_list:REG_DEAD (reg:SI 33)
(nil)))
(insn 26 20 0 2 (use (reg/i:HI 1 %r3 [  ])) -1
(insn_list:REG_DEP_TRUE 20 (nil))
(nil))
;; Registers live at end:  1 [%r3] 2 [%r2] 11 [%i2h] 13 [%i3h] 16
[%ap_hi] 18 [%fp_hi]


error: unable to emulate 'DI'

2008-10-15 Thread Omar Torres
Hi All,

I have a similar issue to what is reported here
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20143):
/Applications/avr/avr-src/gcc/unwind.h:59: error: unable to emulate 'DI'

As you clearly expressed by Paul, the underline issue that the target
only support data types up to 32-bits, while gcc is expecting up to
64-bit support.

Any suggestions on how to workaround this?

Thanks,
 -Omar


Re: error: unable to emulate 'DI'

2008-10-16 Thread Omar Torres
Hi Andrew,
 Looks like Paul did submitted a patch here:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20675

Can you or someone else take a look and comment on it?

Thanks!
-Omar

On Thu, Oct 16, 2008 at 4:08 AM, Andrew Haley <[EMAIL PROTECTED]> wrote:
> Omar Torres wrote:
>
>> I have a similar issue to what is reported here
>> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20143):
>> /Applications/avr/avr-src/gcc/unwind.h:59: error: unable to emulate 'DI'
>>
>> As clearly expressed by Paul, the underline issue that the target
>> only support data types up to 32-bits, while gcc is expecting up to
>> 64-bit support.
>>
>> Any suggestions on how to workaround this?
>
> Paul Schlie said:
>
> "The further good news is that upon reviewing the DWARF2 spec in detail,
> and GCC's implementation, it's clear that a 32-bit DWARF data structure
> is sufficient to support any target with 32-bit or less data type sizes.
>
> "(which seems that it may be reasonably easy to support by declaring the
> DWARF data structure size as a function of the size of target's the maxim
> declared data type size; although possibly limited to a practical minimum
> of 32-bits, which seems much more reasonable for smaller targets)"
>
> but no-one has ever AFAIAA submitted a patch that does this.
>
> Andrew.
>
>
>


Re: error: unable to emulate 'DI'

2008-10-16 Thread Omar Torres
On Thu, Oct 16, 2008 at 10:02 AM, Andrew Haley <[EMAIL PROTECTED]> wrote:
> Omar Torres wrote:
>> Hi Andrew,
>>  Looks like Paul did submitted a patch here:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20675
>>
>> Can you or someone else take a look and comment on it?
>
> Oh my goodness, that is a huge patch.  It's also incorrect, as
> far as I can see: LONG_LONG_TYPE_SIZE is never less than 64 bits,
> so this test always returns true.  There's a discussion in Section
> 6.2.5. of the rationale in C99 that explains why long long is defined
> to be this way.
>
> Andrew.
>

LONG_LONG_TYPE_SIZE is in fact defined as 32-bit in the port I am
working. I inherited this GCC port, so I do not now whether or not
this is fully compliant with C99 standard. I believe the reason is to
reduce code size (this is an 8-bit word target, 64-bit operations are
very expensive).

The core is aimed, at low-power embedded applications without an OS
(an event-driven scheduler is used instead). A very old GCC port has
been sucessfuly used for years, all I am trying to do is to bring that
old port to a more current version of GCC.

Tahnks,
-Omar


Re: error: unable to emulate 'DI'

2008-10-16 Thread Omar Torres
On Thu, Oct 16, 2008 at 10:42 AM, Andrew Haley <[EMAIL PROTECTED]> wrote:
>
>> LONG_LONG_TYPE_SIZE is in fact defined as 32-bit in the port I am
>> working. I inherited this GCC port, so I do not now whether or not
>> this is fully compliant with C99 standard.
>
> You do now.

Yes, thanks.

>
>> I believe the reason is to reduce code size (this is an 8-bit word
>> target, 64-bit operations are very expensive).
>
>> The core is aimed, at low-power embedded applications without an OS
>> (an event-driven scheduler is used instead). A very old GCC port has
>> been sucessfuly used for years, all I am trying to do is to bring that
>> old port to a more current version of GCC.
>
> Sure, I understand that, and I also note one or two ports define
> LONG_LONG_TYPE_SIZE otherwise.
>
 I see avr port's situation:
 ./avr/avr.h:#define LONG_LONG_TYPE_SIZE (INT_TYPE_SIZE == 8 ? 32 : 64)

 I need to investigate how do they get pass the boot-strapping part of
gcc... Mean while, if anyone have any insight, I am all ears.

> Andrew.
>

Thanks,
-0mar


Re: error: unable to emulate 'DI'

2008-10-16 Thread Omar Torres
On Thu, Oct 16, 2008 at 11:59 AM, Andrew Haley <[EMAIL PROTECTED]> wrote:
> Paul Schlie wrote:
>> Andrew Haley wrote:
>>> Omar Torres wrote:
>>>> Hi Andrew,
>>>>  Looks like Paul did submitted a patch here:
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20675
>>>>
>>>> Can you or someone else take a look and comment on it?
>>> Oh my goodness, that is a huge patch.  It's also incorrect, as
>>> far as I can see: LONG_LONG_TYPE_SIZE is never less than 64 bits,
>>> so this test always returns true.  There's a discussion in Section
>>> 6.2.5. of the rationale in C99 that explains why long long is defined
>>> to be this way.
>>
>> Yes, a target port which which does not support 64 bit operations
>> could not be strictly C99 compliant, however don't see any reason
>> to forbid such as may be desired with this understanding; as such
>> a port would remain compliant otherwise and fully useful in this
>> respect, and remain fully compliant with earlier standards.
>
> I don't understand this.  If a target will generate inefficient code
> for long long, that is surely no reason not to support it, it's a
> reason to avoid using it.  How does the availability of long long
> cause any kind of a problem?

 Agree, and it turns out that long long is *never* used in the
intended application, so the original port author decided not to
bother implementing 64-bit support.

 Since implementing 64-bit support for this target is out of the
question in the immediate future, I am in the look for an alternative
solution.

Realized just now that the original port have a comment on how to work
around this same issue:
  # We do not have DF or DI types, so fake out the libgcc2 compilation.
  TARGET_LIBGCC2_CFLAGS = -DDF=SF -DDI=SI -D__DF__=__SF__ -D__DI__=__SI__

Looks like GCC now sucessfuly complies my x-compiler. Now, the hard
part: check that it does the "right thing".


Thanks for your help!
-Omar


Error building gcc 4.5.2 for AVR

2011-01-24 Thread Omar Choudary
4.5.2/include
 /usr/local/avr/lib/gcc/avr/4.5.2/include-fixed
 /usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-mmcu=at90usb1287' '-o' 'SCD.elf'
 /usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/bin/as -v
-mmcu=at90usb1287 -o /tmp/ccswYC86.o /tmp/ccPZdrIF.s
GNU assembler version 2.21 (avr) using BFD version (GNU Binutils) 2.21
COMPILER_PATH=/usr/local/avr/libexec/gcc/avr/4.5.2/:/usr/local/avr/libexec/gcc/avr/4.5.2/:/usr/local/avr/libexec/gcc/avr/:/usr/local/avr/lib/gcc/avr/4.5.2/:/usr/local/avr/lib/gcc/avr/:/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/bin/
LIBRARY_PATH=/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/lib/avr51/:/usr/local/avr/lib/gcc/avr/4.5.2/:/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/lib/
COLLECT_GCC_OPTIONS='-v' '-mmcu=at90usb1287' '-o' 'SCD.elf'
 /usr/local/avr/libexec/gcc/avr/4.5.2/collect2 -m avr51 -Tdata
0x800100 -o SCD.elf
/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/lib/avr51/crtusb1286.o
-L/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/lib/avr51
-L/usr/local/avr/lib/gcc/avr/4.5.2
-L/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/lib -Map=SCD.map
SCD.o EMV.o halSCD.o scdIO.o utils.o terminal.o /tmp/ccAv4Kbo.o
/tmp/ccswYC86.o -lgcc -lc -lgcc
/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/bin/ld: cannot find -lgcc
/usr/local/avr/lib/gcc/avr/4.5.2/../../../../avr/bin/ld: cannot find -lgcc
collect2: ld returned 1 exit status


Thanks,
 Omar


Re: Reviving old port

2008-05-13 Thread Omar Torres
Richard,
  Thanks a lot for the detailed explanation. I need some time to digest,
since this is my first experience doing a gcc backend port.

> Probably most of that was just telling you what you already knew, sorry.
Not really, I am new to most of these concepts. At this stage, this is
exactly the amount of details I am looking for. Thanks a lot for your
patience and level of detail.

> Good luck with the port,
>
> Richard

Regards,
-Omar


splitting const_int's

2008-05-19 Thread Omar Torres
Hi All,
  Since the architecture I am working with only support byte size moves,
I created a define_split break HI-move insn's into two  QI-moves.

For example, the following insn:
(insn 84 5 9 (set (reg:HI 1 %r3)
 (const_int 32767 [0x7fff])) 3 {*movhi} (nil)
 (nil))

I split it into:
(set (reg:QI 1 %r3)
  (const_int 127 [0x7f])
(set (reg:QI 2 %r2)
  (const_int 255 [0x255])

I expect that the simplified RTL will be detected by the movqi pattern:
(define_insn "movqi"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=r,m")
(match_operand:QI 1 "general_operand"  "g,r"))]
   ""
   "move\t%0,%1")


The split split is working as I expect, but the problem is that the
movqi is not recognizing the the second part of the move:
error: unrecognizable insn:
(insn 94 93 9 (set (reg:QI 2 %r2 [orig:1+1 ] [1])
 (const_int 255 [0xff])) -1 (nil)
 (nil))


After further investigating this, I was able find why the movqi is not
recognizing this patter. The const_int 255 fails this statement in the
general_operand predicate:
   if (GET_CODE (op) == CONST_INT
   && mode != VOIDmode
   && trunc_int_for_mode (INTVAL (op), mode) != INTVAL (op))
 return 0;
When I debug this I can see that trunc_int_for_mode is returning -1
while the INTVAL macro returns 255.

Any suggestions on how to fix this issue?

Below is the complete define_split I am referring above.

(define_split
   [(set (match_operand:HI 0 "nonimmediate_operand" "")
 (match_operand:HI 1 "general_operand"  ""))]
   "reload_completed
&& REG_P (operands[0])"
[(set (match_dup 2) (match_dup 4))
  (set (match_dup 3) (match_dup 5))]
  "{
   gcc_assert (REG_P (operands[0]));
   operands[2] = gen_highpart(QImode, operands[0]);
   operands[3] = gen_lowpart (QImode, operands[0]);

   if (REG_P (operands[1])) {
   operands[4] = gen_highpart (QImode, operands[1]);
   operands[5] = gen_lowpart  (QImode, operands[1]);
   }
   else if (MEM_P (operands[1]) || CONST == GET_CODE (operands[1])) {
   int ioff;
   rtx base, off;
   base = XEXP (operands[1],0);
   off  = gen_rtx_CONST_INT (VOIDmode, 0);
   if (CONST == GET_CODE (base)) {
   base = XEXP(base,0);
   }
   if (PLUS == GET_CODE (base)) {
off  = XEXP (base,1);
base = XEXP (base,0);
   }
   gcc_assert (REG == GET_CODE (base)
   && CONST_INT == GET_CODE (off));
   ioff = INTVAL (off);
   gcc_assert (254 >= ioff
   && 0 <= ioff);
   operands[4] = gen_rtx_MEM (QImode,
  gen_rtx_PLUS (Pmode,
base,
gen_rtx_CONST_INT   
(QImode, ioff + 1)));
   operands[5] = gen_rtx_MEM (QImode,
  gen_rtx_PLUS (Pmode,
base,

gen_rtx_CONST_INT(QImode, ioff)));
   }
   else if (LABEL_REF == GET_CODE (operands[1])
  || SYMBOL_REF == GET_CODE (operands[1])) {
   operands[4] = simplify_gen_subreg(QImode,operands[1],HImode,0);
   operands[5] = simplify_gen_subreg(QImode,operands[1],HImode,1);
   }
   else if (CONST_INT == GET_CODE (operands[1])) {
   int val = INTVAL (operands[1]);
   int low = val & 0xff;
   int high = (val>>8) & 0xff;
   operands[4] = gen_rtx_CONST_INT (QImode, high);
   operands[5] = gen_rtx_CONST_INT (QImode, low);
   }
   else {
   fprintf (stderr, \"\nUnrecongnized:\");
   debug_rtx (operands[1]);
   }
  }")


Thanks,
-Omar


Re: splitting const_int's

2008-05-19 Thread Omar Torres
Just correcting a typo...

Omar Torres wrote:
> I split it into:
  (set (reg:QI 1 %r3)
   (const_int 127 [0x7f])
  (set (reg:QI 2 %r2)
   (const_int 255 [0xFF])


Re: splitting const_int's

2008-05-19 Thread Omar Torres
Richard Sandiford wrote:
>
> You might as well make the first operand a register_operand and
> avoid the REG_P check.
  I agree. I implemented this change and it works as expected.

>More importantly:
>
>>operands[4] = simplify_gen_subreg(QImode,operands[1],HImode,0);
>>operands[5] = simplify_gen_subreg(QImode,operands[1],HImode,1);
>
> ..this code is designed to handle REGs and CONST_INTs correctly,
> and avoid the problem you were seeing.  (As Eric says, gen_int_mode
> is the canonical way of generating a correct CONST_INT in cases where
> you do need to create one manually.  In this case it's simpler to use
> simplify_gen_subreg though.)
  I modified the code as below and now I do not have the problems I had
before.

   else if (CONST_INT == GET_CODE (operands[1])
  || REG_P (operands[1])) {
   operands[4] = simplify_gen_subreg(QImode,operands[1],HImode,0);
   operands[5] = simplify_gen_subreg(QImode,operands[1],HImode,1);
   }

> I notice you're generating subregs of symbolic constants,
> but I'm not sure that's really supported.
   I did this so that I can generate rtl code that I can latter trap
with the define_insn's below.

(define_insn "movqi_hiword"
  [(set (match_operand:QI 0 "register_operand" "")
(subreg:QI (match_operand:HI 1 "general_operand" "") 0))]
  ""
  "move\t%0,#HIWORD(%1)" )

(define_insn "*movqi_lowword"
  [(set (match_operand:QI 0 "register_operand" "")
(subreg:QI (match_operand:HI 1 "general_operand" "") 1))]
  ""
  "move\t%0,#LOWWORD(%1)" )

  Do you recommend a better way to do this?



> As far as the MEM_P case goes:
[...]
>
> you can use:
>
> operands[4] = adjust_address (operands[1], QImode, 1);
> operands[5] = adjust_address (operands[1], QImode, 0);
>
  I also implemented this and it works nicely. Many thanks!

> The CONST handling looks suspicious here.  CONSTs aren't memory references,
> but you split them into memory references.
  In my debugging session, all cases I say CONSTs they were used for
memory. I will look at this in more detail.


> Also, you need to beware of cases in which operands[1] overlaps
> operands[0].  The splitter says:
>
>  [(set (match_dup 2) (match_dup 4))
>   (set (match_dup 3) (match_dup 5))]
>
> and operands[2] is always the highpart:
>
>operands[2] = gen_highpart(QImode, operands[0]);
>
> but consider the case in which operands[1] (and thus operands[4])
> is a memory reference that uses the high part of operands[0] as
> a base register.  In that case, the base register will be modified
> by the first split instruction and have the wrong value in the
> second split instruction.  See other ports for the canonical way
> of handling this.

Thanks for the heads up. Could you point me to a specific target/file?


Best regards,
-Omar


Re: splitting const_int's

2008-05-19 Thread Omar Torres
Richard Sandiford wrote:
> "Omar Torres" <[EMAIL PROTECTED]> writes:
> More importantly:
>
>>operands[4] = simplify_gen_subreg(QImode,operands[1],HImode,0);
>>operands[5] = simplify_gen_subreg(QImode,operands[1],HImode,1);
>
> ..this code is designed to handle REGs and CONST_INTs correctly,
> and avoid the problem you were seeing.  (As Eric says, gen_int_mode
> is the canonical way of generating a correct CONST_INT in cases where
> you do need to create one manually.  In this case it's simpler to use
> simplify_gen_subreg though.)

Now, the insn:
(insn 84 5 9 (set (reg:HI 1 %r3)
 (const_int 32767 [0x7fff])) 3 {*movhi} (nil)
 (nil))

get's split into
  (set (reg:QI 1 %r3)
   (const_int 127 [0x7f])
  (set (reg:QI 2 %r2)
   (const_int -1 [0x])

and my movqi patter is able to match both of this patterns as expected.

But, now the output asm is
  move %r2,#0x
Instead of the expected:
  move %r2,#0xff

I am not fully familiar with how to fix this, but seems like there are
at least two ways of fixing this:
  1- Detect the sign in the rtl template and manipulate the operand so
that the asm value is outputted. That is, use something like this:

(define_insn "movqi"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=r,m")
(match_operand:QI 1 "general_operand"  "g,r"))]
   ""
   "*
   if (CONST_INT == GET_CODE (operands[1])) {
 if (0 > INTVAL(operands[1])) {
   fprintf (stderr, \"\nOperand 1 Value: %d, %d\", INTVAL
(operands[1]),0xff & INTVAL (operands[1]) );
   operands[1] = gen_rtx_CONST_INT (VOIDmode, 0xff & INTVAL
(operands[1]));
 }
   }
  return \"move.a\t%0,%1\";
  ")

  2- Use a special letter in the asm template, to communicate with the
print_operand() function.

  Is there any other way of doing this? If not, is there any advantage
of using one option over the other above?

Thanks,
-Omar


Re: splitting const_int's

2008-05-20 Thread Omar Torres
Richard Sandiford wrote:
> Also, you need to beware of cases in which operands[1] overlaps
> operands[0].  The splitter says:
>
>  [(set (match_dup 2) (match_dup 4))
>   (set (match_dup 3) (match_dup 5))]
>
> and operands[2] is always the highpart:
>
>operands[2] = gen_highpart(QImode, operands[0]);
>
> but consider the case in which operands[1] (and thus operands[4])
> is a memory reference that uses the high part of operands[0] as
> a base register.  In that case, the base register will be modified
> by the first split instruction and have the wrong value in the
> second split instruction.  See other ports for the canonical way
> of handling this.
>
> Richard

By looking at other ports, I learned that I can detect when this happens
by using the reg_overlap_mentioned_p(). Here is one case:
(insn 43 115 74 (set (reg:HI 7 %i0h)
 (mem/s/j:HI (plus:HI (reg/f:HI 7 %i0h [orig:39 source ] [39])
 (const_int 2 [0x2])) [0 .r+0 S2 A8])) 3
{*movhi} (nil)
 (nil))

   I need to tell the compiler not to use as destination the same base
register when doing index operations. Any suggestions on how do I that?

Thanks for your help.
-Omar


How to post to GCC lists?

2008-05-22 Thread Omar Torres
Dear All,
  I recently started working in a GCC backend, and as a result I expect
to be actively participating in the ML.
   That said, I have questions on how to properly and efficiently do a
post. I want to remove the annoyances that at the end just unnecessary
waste everyone's time.

1- I noticed that when I reply to posts, the "References" are not
preserved, which leads to messages in the same threat not to be linked
together. I am using Thunderbird as my email client. When I go to
View>Headers>All the References field looks accurate. Is there a trick
to make this work? Should I use instead a different email client better
suit for GCC ML?

2- Currently, (1) is the only thing I have run cross. I am sure there
are other issues that those of you with more experience have already
discovered and solve. Any other Dos/Don'ts or tips/tricks when posting
to GCC ML?

Thanks,
-Omar


Re: Few question regarding the implementation of splitting HImode patterns

2008-05-23 Thread Omar Torres
Mohamed Shafi wrote:
> Hello Omar,
>
> I saw your mail to gcc mailing list regarding splitting of HImode
> patterns into QImode patterns. I am also involved in porting. My
> problem is similar to yours. But i have to split SImode patterns into
> HImode patterns.
>
> I am sure that you have modified your define_split patterns after
> receiving the suggestions from the mailing list. Could you just mail
> me the finalized define_split pattern of HImode.
>
> One thing that i noticed in your split pattern is that you are not
> handling cases where operand[0] is memory, i.e store patterns.  How
> are you handling this? Do you have a define_insn for this case?
>
> I hope you don't mind me asking these questions.
>
> Thank you for your time.
>
> Regards,
> Shafi
>
>

Hi Mohamed,
  I added the gcc mailing list to the threat.

My current implementation looks like this:
;; movhi
(define_expand "movhi"
   [(set (match_operand:HI 0 "nonimmediate_operand" "")
 (match_operand:HI 1 "general_operand"  ""))]
""
{
   if (c816_expand_move (HImode, operands)) {
   DONE;
   }
})

;; =&r creates an early clobber.
;; It prevent insn where the target register
;; is the same as the base register used for memory addressing...
;; This is needed so that the split produce correct code.
(define_insn "*movhi"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=&r,m")
 (match_operand:HI 1 "general_operand"  "g,r"))]
""
"#")

(define_split
   [(set (match_operand:HI 0 "nonimmediate_operand" "")
 (match_operand:HI 1 "general_operand"  ""))]
   "reload_completed"
[(set (match_dup 2) (match_dup 4))
  (set (match_dup 3) (match_dup 5))]
  "{
   gcc_assert (REG_P (operands[0]) || MEM_P (operands[0]));

#ifdef DEBUG_OVERLAP
   if (reg_overlap_mentioned_p(operands[0], operands[1])){
   fprintf (stderr, \"\nOperands Overlap:\n\");
   debug_rtx (curr_insn);
   }
#endif

   if (REG_P (operands[0])) {
   operands[2] = gen_highpart(QImode, operands[0]);
   operands[3] = gen_lowpart (QImode, operands[0]);
   }
   else if (MEM_P (operands[0])) {
   operands[2] = adjust_address (operands[0], QImode, 0);
   operands[3] = adjust_address (operands[0], QImode, 1);
   }

   if (MEM_P (operands[1])) {// || CONST == GET_CODE (operands[1])) {
   operands[4] = adjust_address (operands[1], QImode, 0);
   operands[5] = adjust_address (operands[1], QImode, 1);
   }
   else if (LABEL_REF == GET_CODE (operands[1])
  || SYMBOL_REF == GET_CODE (operands[1])) {//
   operands[4] = simplify_gen_subreg(QImode, operands[1], HImode, 0);
   operands[5] = simplify_gen_subreg(QImode, operands[1], HImode, 1);
   }
   else if (CONST_INT == GET_CODE (operands[1])
  || REG_P (operands[1])) {
   operands[4] = simplify_gen_subreg(QImode, operands[1], HImode, 0);
   operands[5] = simplify_gen_subreg(QImode, operands[1], HImode, 1);
   }
   else {
   error(\"Unrecognized code in operands[1]\");
   fputs(\"\nrtx code is: \", stderr);
   debug_rtx(curr_insn);
   abort();
   }
  }")


The purpose of the expand is to load Label or Symbol references into
base registers for index addressing. I decided to use the expand since
the force_reg() was failing when I called from the split.

The c816_expand_move() is implemented as follows:
bool
c816_expand_move (enum machine_mode mode, rtx *operands) {
   if (!no_new_pseudos) {
 int i;
 for (i = 0; i < 2; i++) {
   if (MEM_P (operands[i]) || CONST == GET_CODE (operands[i])) {
rtx base = XEXP (operands[i],0);
if (CONST == GET_CODE(base)) {
  base = XEXP (base,0);
}
if (PLUS == GET_CODE (base)) {
  rtx off = XEXP (base,1);
  base = XEXP (base,0);
  if (SYMBOL_REF == GET_CODE (base)
  || LABEL_REF == GET_CODE (base)) {
rtx newreg = force_reg (Pmode, base);
operands[i] = gen_rtx_MEM (mode,
   gen_rtx_PLUS (Pmode,
 newreg,
 off));
  }
}
else if (SYMBOL_REF == GET_CODE (base)
 || LABEL_REF == GET_CODE(base)) {
  rtx newreg = force_reg (Pmode, base);
  operands[i] = gen_rtx_MEM (mode, newreg);
}
   }// if MEM_P
 }// for
   }// if !no_new_pseudos
   return false;
}

In your case, you might want to look at other gcc ports for 16-bit
machines. One I have look from time to time is the MSP430 port
(http://mspgcc.sourceforge.net/).

Hope this helps.
Good Luck,
-Omar


insn does not satisfy its constraints

2008-08-28 Thread Omar Torres
Hi All,
  I am getting the error message bellow while hacking my gcc backend.
Any suggestions on how to fix this?

error: insn does not satisfy its constraints:
(insn 25 50 26 2 (set (reg:HI 1 %r0 [33])
 (plus:HI (mem/s/j:HI (plus:HI (reg:HI 3 %r2)
 (reg/v/f:HI 11 %i2h [orig:28 cell ] [28])) [0
.dod S2 A8])
 (reg:HI 1 %r0 [43]))) 8 {*addhi3} (insn_list:REG_DEP_TRUE 3
(insn_list:REG_DEP_TRUE 24 (nil)))
 (nil))


The md description for this instruction is:

;; addhi3
(define_expand "addhi3"
   [(set (match_operand:HI 0 "register_operand" "")
(plus:HI (match_operand:HI 1 "cool_addhi_operand"  "")
 (match_operand:HI 2 "cool_addhi_operand"  "")))]
   ""
   "")

(define_insn "*addhi3"
   [(set (match_operand:HI 0 "register_operand""=r ,r  ,r")
(plus:HI (match_operand:HI 1 "cool_addhi_operand" "%0 ,rim,r")
 (match_operand:HI 2 "cool_addhi_operand" "rim,0  ,r")))]
   ""
  {
operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 1);
operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 0);

operands[5] = simplify_gen_subreg (QImode, operands[1], HImode, 1);
operands[6] = simplify_gen_subreg (QImode, operands[1], HImode, 0);

operands[7] = simplify_gen_subreg (QImode, operands[2], HImode, 1);
operands[8] = simplify_gen_subreg (QImode, operands[2], HImode, 0);

if (!which_alternative)
return "ADD\t%3,%7\n\tADDC\t%4,%8";
else if (1 == which_alternative)
return "ADD\t%3,%5\n\tADDC\t%4,%6";
else
return "ADD\t%3,%5,%8\n\tADDC\t%4,%5,%8";
   })

Regards,
  -Omar


Re: insn does not satisfy its constraints

2008-08-29 Thread Omar Torres
shafi wrote:
>Operand 0 is a register
>Operand 1 is a memory
>Operand 2 is a register
>
>
>> The md description for this instruction is:
>>
>> ;; addhi3
>> (define_expand "addhi3"
>>   [(set (match_operand:HI 0 "register_operand" "")
>>  (plus:HI (match_operand:HI 1 "cool_addhi_operand"  "")
>>   (match_operand:HI 2 "cool_addhi_operand"  "")))]
>>   ""
>>   "")
>>
>> (define_insn "*addhi3"
>>   [(set (match_operand:HI 0 "register_operand""=r ,r  ,r")
>>  (plus:HI (match_operand:HI 1 "cool_addhi_operand" "%0 ,rim,r")
>>   (match_operand:HI 2 "cool_addhi_operand" "rim,0  ,r")))]
>>   ""
>>
>
>Do you have an option where operand 0 is reg and operand 1 is mem and
> operand 2 is reg?
>
My purpose is to describe the three possible scenarios:
1)  Operand 0 is a register
 Operand 1 is the same register as operand 0
 Operand 2 is a register, immediate or memory

2)  Operand 0 is a register
 Operand 1 is a register, immediate or memory
  Operand 2 is the same register as operand 0

3)  Operand 0 is a register
 Operand 2 is a register
 Operand 3 is also a register


>I am not sure what rim is for?
>
rim = is a short cut for r, m, i. I think is is allow to mix several
constrains like this, right?


Thanks,
-Omar