This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Merge definitions of get_some_local_dynamic_name
- From: Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>
- To: gcc-patches at gcc dot gnu dot org
- Cc: rdsandiford at googlemail dot com
- Date: Mon, 06 Oct 2014 15:14:35 +0200
- Subject: Re: RFA: Merge definitions of get_some_local_dynamic_name
- Authentication-results: sourceware.org; auth=none
- References: <87bnqxna3y dot fsf at googlemail dot com> <yddr3zmwcjd dot fsf at lokon dot CeBiTec dot Uni-Bielefeld dot DE> <87ppf6x9d9 dot fsf at googlemail dot com> <yddd2b4bqpf dot fsf at CeBiTec dot Uni-Bielefeld dot DE> <87d2b4y6wk dot fsf at googlemail dot com>
Hi Richard,
> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>> Hi Richard,
>>>> It seems the new get_some_local_dynamic_name implementation in
>>>> function.c lost the non-NULL check the sparc.c version had. I'm
>>>> currently testing the following patch:
>>>
>>> Could you do a:
>>>
>>> call debug_rtx (...)
>>>
>>> on the insn that contains a null pointer? Normally insn patterns
>>> shouldn't contain nulls, so I was wondering whether this was some
>>> SPARC-specific construct.
>>
>> proved a bit difficult to do: at the default -O2, insn was optimized
>> away, at -g3 -O0, I only got
>>
>> can't compute CFA for this frame
>>
>> with gdb 7.8 even after recompiling all of the gcc dir with -g3 -O0.
>>
>> Here's what I find after inserting the call in the source:
>>
>> (insn 30 6 28 (sequence [
>> (call_insn:TI 8 6 7 (parallel [
>> (set (reg:SI 8 %o0)
>> (call (mem:SI (unspec:SI [
>> (symbol_ref:SI ("__tls_get_addr"))
>> ] UNSPEC_TLSLDM) [0 S4 A32])
>> (const_int 1 [0x1])))
>> (clobber (reg:SI 15 %o7))
>> ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 390 {tldm_call32}
>> (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000])
>> (nil))
>> (expr_list (use (reg:SI 8 %o0))
>> (nil)))
>> (insn 7 8 28 (set (reg:SI 8 %o0)
>> (plus:SI (reg:SI 23 %l7)
>> (unspec:SI [
>> (reg:SI 8 %o0 [112])
>> ] UNSPEC_TLSLDM))) 388 {tldm_add32}
>> (nil))
>> ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 -1
>> (nil))
>
> Bah, a sequence. Hadn't thought of that.
>
> IMO it's a bug for a walk on a PATTERN to pull in non-PATTERN parts
> of an insn. We should really be looking at the patterns of the two
> subinsns instead and ignore the other stuff. Let me have a think
> about it.
did you come to a conclusion here?
> Now that we know the underlying cause, I personally wouldn't mind the
> null check being committed as a workaround. If you do though, please
> could you add a comment saying that this is for SEQUENCEs?
I've had the following in my local tree for some weeks now to keep SPARC
bootstrap working:
2014-10-06 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
* final.c (get_some_local_dynamic_name): Guard against NULL
const_rtx.
# HG changeset patch
# Parent 5768379d027521837a7be746630c3f53cc5bce83
Fix SPARC bootstrap: get_some_local_dynamic_name
diff --git a/gcc/final.c b/gcc/final.c
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1739,7 +1739,8 @@ get_some_local_dynamic_name ()
FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
{
const_rtx x = *iter;
- if (GET_CODE (x) == SYMBOL_REF)
+ /* NULL check to guard against SEQUENCEs. */
+ if (x && GET_CODE (x) == SYMBOL_REF)
{
if (SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
return some_local_dynamic_name = XSTR (x, 0);
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University