On Thu, Sep 25, 2014 at 7:26 PM, Jeff Law <[email protected]> wrote:
> On 09/24/14 13:39, Uros Bizjak wrote:
>>
>> Hello!
>>
>> The failure was caused by barrier detection code, which failed to
>> detect barrier after call insn was to be split when
>> NOTE_CALL_ARG_LOCATION was present. This problem caused
>> -fcompare-debug failure.
>>
>> Digging a bit deeped, and as hinted in the PR, the handling of
>> barriers in try_split seems to be broken. The code is emitting extra
>> barrier for non-debug compiles, but it "forgots" to remove the
>> existing one, leading to duplicated barriers. The barrier is not
>> detected at all for debug build.
>>
>> I have removed special handling of barriers here (also, the comment in
>> removed code was not helpful at all), and this solved -fcompare-debug
>> failure.
>>
>> The patch was also bootstrapped and regression tested on
>> x86_64-linux-gnu {,-m32} which in -m32 mode splits x87 FP jump insns,
>> and there were no regressions. However, I am not too familiar with
>> rtl-optimization part and I am not confident that this code surgery is
>> fully correct, so this is the reason for RFC status of the patch.
>>
>> 2014-09-24 Uros Bizjak <[email protected]>
>>
>> PR rtl-optimization/63348
>> * emit-rtl.c (try_split): Do not emit extra barrier.
>
> Good grief, the code you're removing pre-dates any version control we have.
> ie, it's in the first revision of emit-rtl.c from 1992. Egad.
>
> It's going to be a hell of a time figuring out why that code exists in the
> first place. I don't like removing code if we don't know why the code
> exists... Any reason you picked that route rather than looking forward
> through the NOTEs to see if they're followed by a suitable BARRIER?
I have tried with alternative patch that just skipped the NOTE:
--cut here--
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (revision 215606)
+++ emit-rtl.c (working copy)
@@ -3622,6 +3622,10 @@ try_split (rtx pat, rtx uncast_trial, int last)
int njumps = 0;
rtx call_insn = NULL_RTX;
+ if (after && NOTE_P (after)
+ && NOTE_KIND (after) == NOTE_INSN_CALL_ARG_LOCATION)
+ after = NEXT_INSN (after);
+
/* We're not good at redistributing frame information. */
if (RTX_FRAME_RELATED_P (trial))
return trial;
--cut here--
and resulted in:
(call_insn 184 190 185 (parallel [
(call (mem:SI (reg:SI 25 $25 [217]) [0 S4 A32])
(const_int 16 [0x10]))
(clobber (reg:SI 31 $31))
(clobber (reg:SI 28 $28))
]) pr43670.c:29 595 {call_split}
(expr_list:REG_NORETURN (const_int 0 [0])
(nil))
(expr_list (use (reg:SI 79 $fakec))
(expr_list (use (reg:SI 28 $28))
(nil))))
(barrier 185 184 175)
(note 175 185 130 (expr_list:REG_DEP_TRUE (concat:SI (pc)
(unspec:SI [
(reg:SI 28 $28)
(const:SI (unspec:SI [
(symbol_ref:SI ("abort") [flags 0x41]
<function_decl 0x7fe1e3af2e58 abort>)
] 227))
(reg:SI 79 $fakec)
] UNSPEC_LOAD_CALL))
(nil)) NOTE_INSN_CALL_ARG_LOCATION)
(barrier 130 175 174)
I have noticed that the barrier is always there, since without -g, we have:
(call_insn 76 82 77 (parallel [
(call (mem:SI (reg:SI 25 $25 [217]) [0 S4 A32])
(const_int 16 [0x10]))
(clobber (reg:SI 31 $31))
(clobber (reg:SI 28 $28))
]) pr43670.c:29 595 {call_split}
(expr_list:REG_NORETURN (const_int 0 [0])
(nil))
(expr_list (use (reg:SI 79 $fakec))
(expr_list (use (reg:SI 28 $28))
(nil))))
(barrier 77 76 37)
(barrier 37 77 40)
and considering the fact that the code didn't process barriers
correctly with -g, I simply removed the emission. The - probably
stalled - comment was not helpful at all.
Uros.
