[PATCH] Fix seq_cost prototype to use signed int

[PATCH] Fix seq_cost prototype to use signed int

[Jiong Wang]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]


All other cost helper functions are using signed int to hold cost
while seq_cost is using unsigned int.

This fix this. bootstrap OK on x86.

OK for trunk?

2015-09-08  Jiong Wang  <jiong.wang@arm.com>

gcc/
  * rtl.h (seq_cost): Change return type from "unsigned" to "int".
  * rtlanal.c (seq_cost): Likewise.
  
-- 
Regards,
Jiong


[-- Attachment #2: seq_cost.patch --]
[-- Type: text/x-diff, Size: 948 bytes --]

diff --git a/gcc/rtl.h b/gcc/rtl.h
index ac56133..ded054c 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3050,7 +3050,7 @@ extern rtx_insn *find_first_parameter_load (rtx_insn *, rtx_insn *);
 extern bool keep_with_call_p (const rtx_insn *);
 extern bool label_is_jump_target_p (const_rtx, const rtx_insn *);
 extern int insn_rtx_cost (rtx, bool);
-extern unsigned seq_cost (const rtx_insn *, bool);
+extern int seq_cost (const rtx_insn *, bool);

 /* Given an insn and condition, return a canonical description of
    the test being made.  */
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index ef98f4b..2f14b93 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5160,10 +5160,10 @@ insn_rtx_cost (rtx pat, bool speed)

 /* Returns estimate on cost of computing SEQ.  */

-unsigned
+int
 seq_cost (const rtx_insn *seq, bool speed)
 {
-  unsigned cost = 0;
+  int cost = 0;
   rtx set;

   for (; seq; seq = NEXT_INSN (seq))

Re: [PATCH] Fix seq_cost prototype to use signed int

[Jeff Law]

On 09/08/2015 06:17 AM, Jiong Wang wrote:
>
> All other cost helper functions are using signed int to hold cost
> while seq_cost is using unsigned int.
>
> This fix this. bootstrap OK on x86.
>
> OK for trunk?
>
> 2015-09-08  Jiong Wang  <jiong.wang@arm.com>
>
> gcc/
>    * rtl.h (seq_cost): Change return type from "unsigned" to "int".
>    * rtlanal.c (seq_cost): Likewise.
Why not go the other way and start making things unsigned -- for a cost 
function like this, unsigned seems more natural to me.

jeff

Re: [PATCH] Fix seq_cost prototype to use signed int

[Jiong Wang]

Jeff Law writes:

> On 09/08/2015 06:17 AM, Jiong Wang wrote:
>>
>> All other cost helper functions are using signed int to hold cost
>> while seq_cost is using unsigned int.
>>
>> This fix this. bootstrap OK on x86.
>>
>> OK for trunk?
>>
>> 2015-09-08  Jiong Wang  <jiong.wang@arm.com>
>>
>> gcc/
>>    * rtl.h (seq_cost): Change return type from "unsigned" to "int".
>>    * rtlanal.c (seq_cost): Likewise.
> Why not go the other way and start making things unsigned -- for a cost 
> function like this, unsigned seems more natural to me.

I was using "(unsigned) -1" to represent maximum cost, then later known
there is MAX_COST macro and found it's actually signed type, and quick
search shows most of the code in gcc/rtlanal.c, are using "int", so I am
changing seq_cost which seems to be the only cost helper using unsigned.

And looks like there is no consistent type to hold cost value across
gcc, some are using "unsigned" while others are using "int", I guess
they are surviving because gcc disable -Wconversion by default.

-- 
Regards,
Jiong

Re: [PATCH] Fix seq_cost prototype to use signed int

[Jeff Law]

On 09/08/2015 09:17 AM, Jiong Wang wrote:
>
> Jeff Law writes:
>
>> On 09/08/2015 06:17 AM, Jiong Wang wrote:
>>>
>>> All other cost helper functions are using signed int to hold cost
>>> while seq_cost is using unsigned int.
>>>
>>> This fix this. bootstrap OK on x86.
>>>
>>> OK for trunk?
>>>
>>> 2015-09-08  Jiong Wang  <jiong.wang@arm.com>
>>>
>>> gcc/
>>>     * rtl.h (seq_cost): Change return type from "unsigned" to "int".
>>>     * rtlanal.c (seq_cost): Likewise.
>> Why not go the other way and start making things unsigned -- for a cost
>> function like this, unsigned seems more natural to me.
>
> I was using "(unsigned) -1" to represent maximum cost, then later known
> there is MAX_COST macro and found it's actually signed type, and quick
> search shows most of the code in gcc/rtlanal.c, are using "int", so I am
> changing seq_cost which seems to be the only cost helper using unsigned.
Understood.  But the natural type should be unsigned as far as I can 
tell.  The fact that we're using signed types all over the place is 
probably a historical wart.

So I'd start by changing the MAX_COST macro to an unsigned type, then 
fix any fallout from that.  That should be a patch unto itself.  Then we 
can have additional follow-up patches to fix the types of costing 
related variables, parameters & return values.

Jeff