Richard Sandiford <[email protected]> writes:
> Rainer Orth <[email protected]> writes:
>> Hi Richard,
>>> Rainer Orth <[email protected]> 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?
>
> Sorry, forgot to come back to this. I have a patch that iterates over
> PATTERNs of a SEQUENCE if the SEQUENCE (rather than its containing insn)
> is the topmost iterated rtx. So if PATTERN (insn) is a SEQUENCE:
>
> FOR_EACH_SUBRTX (...., insn, x)
> ...
>
> will iterate over the insns in the SEQUENCE (including pattern, notes,
> jump label, etc.), whereas:
>
> FOR_EACH_SUBRTX (...., PATTERN (insn), x)
> ...
>
> would only iterate over the patterns of the insns in the SEQUENCE.
Does this work for you? I tested it on x86_64-linux-gnu but obviously
that's not particularly useful for SEQUENCEs.
Thanks,
Richard
gcc/
* rtlanal.c (generic_subrtx_iterator <T>::add_subrtxes_to_queue):
Add the parts of an insn in reverse order, with the pattern at
the top of the queue. Detect when we're iterating over a SEQUENCE
pattern and in that case just consider patterns of subinstructions.
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c 2014-09-25 16:40:44.944406590 +0100
+++ gcc/rtlanal.c 2014-10-07 13:13:57.698132753 +0100
@@ -128,29 +128,58 @@ generic_subrtx_iterator <T>::add_subrtxe
value_type *base,
size_t end, rtx_type x)
{
- const char *format = GET_RTX_FORMAT (GET_CODE (x));
+ enum rtx_code code = GET_CODE (x);
+ const char *format = GET_RTX_FORMAT (code);
size_t orig_end = end;
- for (int i = 0; format[i]; ++i)
- if (format[i] == 'e')
- {
- value_type subx = T::get_value (x->u.fld[i].rt_rtx);
- if (__builtin_expect (end < LOCAL_ELEMS, true))
- base[end++] = subx;
- else
- base = add_single_to_queue (array, base, end++, subx);
- }
- else if (format[i] == 'E')
- {
- int length = GET_NUM_ELEM (x->u.fld[i].rt_rtvec);
- rtx *vec = x->u.fld[i].rt_rtvec->elem;
- if (__builtin_expect (end + length <= LOCAL_ELEMS, true))
- for (int j = 0; j < length; j++)
- base[end++] = T::get_value (vec[j]);
- else
- for (int j = 0; j < length; j++)
- base = add_single_to_queue (array, base, end++,
- T::get_value (vec[j]));
- }
+ if (__builtin_expect (INSN_P (x), false))
+ {
+ /* Put the pattern at the top of the queue, since that's what
+ we're likely to want most. It also allows for the SEQUENCE
+ code below. */
+ for (int i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; --i)
+ if (format[i] == 'e')
+ {
+ value_type subx = T::get_value (x->u.fld[i].rt_rtx);
+ if (__builtin_expect (end < LOCAL_ELEMS, true))
+ base[end++] = subx;
+ else
+ base = add_single_to_queue (array, base, end++, subx);
+ }
+ }
+ else
+ for (int i = 0; format[i]; ++i)
+ if (format[i] == 'e')
+ {
+ value_type subx = T::get_value (x->u.fld[i].rt_rtx);
+ if (__builtin_expect (end < LOCAL_ELEMS, true))
+ base[end++] = subx;
+ else
+ base = add_single_to_queue (array, base, end++, subx);
+ }
+ else if (format[i] == 'E')
+ {
+ unsigned int length = GET_NUM_ELEM (x->u.fld[i].rt_rtvec);
+ rtx *vec = x->u.fld[i].rt_rtvec->elem;
+ if (__builtin_expect (end + length <= LOCAL_ELEMS, true))
+ for (unsigned int j = 0; j < length; j++)
+ base[end++] = T::get_value (vec[j]);
+ else
+ for (unsigned int j = 0; j < length; j++)
+ base = add_single_to_queue (array, base, end++,
+ T::get_value (vec[j]));
+ if (code == SEQUENCE && end == length)
+ /* If the subrtxes of the sequence fill the entire array then
+ we know that no other parts of a containing insn are queued.
+ The caller is therefore iterating over the sequence as a
+ PATTERN (...), so we also want the patterns of the
+ subinstructions. */
+ for (unsigned int j = 0; j < length; j++)
+ {
+ typename T::rtx_type x = T::get_rtx (base[j]);
+ if (INSN_P (x))
+ base[j] = T::get_value (PATTERN (x));
+ }
+ }
return end - orig_end;
}
