On 11/30/20 1:50 PM, Qing Zhao wrote:
> Hi, Jeff,
>
> Sorry for the late reply due to thanksgiving long weekend.
>
>> On Nov 25, 2020, at 1:37 PM, Jeff Law <l...@redhat.com
>> <mailto:l...@redhat.com>> wrote:
>>
>>
>>
>> On 11/19/20 8:59 AM, Qing Zhao via Gcc-patches wrote:
>>> Hi,
>>>
>>> PR97777 - ICE: in df_refs_verify, at df-scan.c:3991 with -O
>>> -ffinite-math-only -fzero-call-used-regs=all
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97777
>>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97777>
>>>
>>> Is a bug triggered by the new pass zero-call-used-regs, however,
>>> it’s an old bug in the pass “reg-stack”.
>>> This pass does not correctly maintain the df information after
>>> transformation.
>>>
>>> Since the transformation is reg-stack pass is quite complicate,
>>> involving both instruction changes and control
>>> Flow changes, I called “df_insn_rescan_all” after the transformation
>>> is done.
>>>
>>> The patch has been tested with bootstrap with
>>> --enable-checking=yes,rtl,df,extra, no regression.
>>>
>>> Okay for commit?
>>>
>>> Qing
>>>
>>> From c2573c6c8552b7b4c2eedb0684ce48b5c11436ec Mon Sep 17 00:00:00 2001
>>> From: qing zhao <qinz...@gcc.gnu.org>
>>> Date: Thu, 19 Nov 2020 16:46:50 +0100
>>> Subject: [PATCH] rtl-optimization: Fix data flow maintenance bug in
>>> reg-stack.c [pr97777]
>>>
>>> reg-stack pass does not maintain the data flow information correctly.
>>> call df_insn_rescan_all after the transformation is done.
>>>
>>> gcc/
>>> PR rtl-optimization/97777
>>> * reg-stack.c (rest_of_handle_stack_regs): call
>>> df_insn_rescan_all if reg_to_stack return true.
>>>
>>> gcc/testsuite/
>>> PR rtl-optimization/97777
>>> * gcc.target/i386/pr97777.c: New test.
>> I'd like to see more analysis here.
>>
>> ie, precisely what data is out of date and why?
>
> For the simple testing case, what happened is, for the insn #6:
>
> (gdb) call debug_rtx(insn)
> (insn 6 26 7 2 (set (reg:XF 8 st [84])
> (reg:XF 9 st(1) [85])) "t.c":4:10 134 {*movxf_internal}
> (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
> (nil)))
>
> After the following statement in reg-stack.c:
> 3080 control_flow_insn_deleted |= subst_stack_regs (insn,
> ®stack);
>
> This insn # 6 becomes:
> (gdb) call debug_rtx(insn)
> (insn 6 26 7 2 (set (reg:XF 8 st)
> (reg:XF 8 st)) "t.c":4:10 134 {*movxf_internal}
> (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
> (nil)))
>
> However, there is no any df maintenance routine (for example,
> df_insn_rescan, etc) is called for this changed insn.
So we are clearing the x87 registers with that option. Hence the change
in reg-stack behavior. I'm a bit surprised by this as I don't see
clearing the x87 registers as particularly helpful from a security
standpoint. But I haven't followed that discussion closely.
>
> As I checked, the transformation for this pass “stack” is quite
> complicated. In addition to the above register replacement,
> New insns might be inserted, and control flow might be changed, but
> for most of the transformations applied in this pass,
> There is no corresponding df maintenance routine is added for deferred
> df rescanning.
But this has been the case essentially for ever with reg-stack. So
what's unclear to me is why it's suddenly a problem now.
Jeff
