diff mbox

[optabs] PR63442 libgcc_cmp_return_mode not always return word_mode

Message ID 544E558C.3030309@arm.com
State New
Headers show

Commit Message

Jiong Wang Oct. 27, 2014, 2:24 p.m. UTC 
On 24/10/14 19:41, Jeff Law wrote:
> On 10/24/14 08:09, Jiong Wang wrote:
>> ping~
>>
>> thanks.
>>
>> Regards,
>> Jiong
>>
>> On 17/10/14 13:04, Jiong Wang wrote:
>>> the cause should be one minor bug in prepare_cmp_insn.
>>>
>>> the last mode parameter "pmode" of "prepare_cmp_insn" should match the
>>> mode of the first parameter "x", while during the recursive call of
>>> "prepare_cmp_insn",
>>> x is with mode of targetm.libgcc_cmp_return_mode () and pmode is
>>> assign to word_mode.
>>>
>>> generally this is OK, because default libgcc_cmp_return_mode hook
>>> always return word_mode,
>>> but AArch64 has a target private implementation which always return
>>> SImode, so there is a
>>> mismatch which cause a ICE later.
>>>
>>> this minor issue is hidding because nearly all other targets use
>>> default hook, and the
>>> compare is rarely invoked.
>>>
>>> Thanks
>>>
>>> gcc/
>>>      PR target/63442
>>>      * optabs.c (prepare_cmp_insn): Use target hook
>>> "libgcc_cmp_return_mode" instead of word_mode.
> This is fine once you have run it through a bootstrap and regression test.
>
> Any reason not to use RET_MODE since that's already set up with the
> result of the target hook?

Ah, it's better to reuse ret_mode, thanks for your carefulness.

patch updated, bootstrapping OK on x86-64 (--enable-languages=c,c++), no regression. (on top of 216667 which is nearly up-to-date)

ok for trunk?

gcc/
   PR target/63442
   * optabs.c (prepare_cmp_insn): Use "ret_mode" instead of "word_mode".

Comments

Jeff Law Oct. 27, 2014, 6:34 p.m. UTC | #1 
On 10/27/14 08:24, Jiong Wang wrote:
>
> On 24/10/14 19:41, Jeff Law wrote:
>> On 10/24/14 08:09, Jiong Wang wrote:
>>> ping~
>>>
>>> thanks.
>>>
>>> Regards,
>>> Jiong
>>>
>>> On 17/10/14 13:04, Jiong Wang wrote:
>>>> the cause should be one minor bug in prepare_cmp_insn.
>>>>
>>>> the last mode parameter "pmode" of "prepare_cmp_insn" should match the
>>>> mode of the first parameter "x", while during the recursive call of
>>>> "prepare_cmp_insn",
>>>> x is with mode of targetm.libgcc_cmp_return_mode () and pmode is
>>>> assign to word_mode.
>>>>
>>>> generally this is OK, because default libgcc_cmp_return_mode hook
>>>> always return word_mode,
>>>> but AArch64 has a target private implementation which always return
>>>> SImode, so there is a
>>>> mismatch which cause a ICE later.
>>>>
>>>> this minor issue is hidding because nearly all other targets use
>>>> default hook, and the
>>>> compare is rarely invoked.
>>>>
>>>> Thanks
>>>>
>>>> gcc/
>>>>      PR target/63442
>>>>      * optabs.c (prepare_cmp_insn): Use target hook
>>>> "libgcc_cmp_return_mode" instead of word_mode.
>> This is fine once you have run it through a bootstrap and regression
>> test.
>>
>> Any reason not to use RET_MODE since that's already set up with the
>> result of the target hook?
>
> Ah, it's better to reuse ret_mode, thanks for your carefulness.
>
> patch updated, bootstrapping OK on x86-64 (--enable-languages=c,c++), no
> regression. (on top of 216667 which is nearly up-to-date)
>
> ok for trunk?
>
> gcc/
>    PR target/63442
>    * optabs.c (prepare_cmp_insn): Use "ret_mode" instead of "word_mode".
OK for the trunk once the bootstrap and regression test are complete.

jeff
diff mbox

Patch

diff --git a/gcc/optabs.c b/gcc/optabs.c
index 2df84d2..ed415fa 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -4270,7 +4270,7 @@  prepare_cmp_insn (rtx x, rtx y, enum rtx_code comparison, rtx size,
 	    y = const0_rtx;
 	}
 
-      *pmode = word_mode;
+      *pmode = ret_mode;
       prepare_cmp_insn (x, y, comparison, NULL_RTX, unsignedp, methods,
 			ptest, pmode);
     }