2014-09-22 22:22 GMT+04:00 Jeff Law <l...@redhat.com>:
> On 09/21/14 06:34, Uros Bizjak wrote:
>>
>> On Fri, Sep 19, 2014 at 9:55 AM, Ilya Enkovich <enkovich....@gmail.com>
>> wrote:
>>
>>> Here is an updated version of this patch.
>>> @@ -25000,10 +25082,32 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx
>>> callarg1,
>>> }
>>>
>>> call = gen_rtx_CALL (VOIDmode, fnaddr, callarg1);
>>> +
>>> if (retval)
>>> - call = gen_rtx_SET (VOIDmode, retval, call);
>>> + {
>>> + /* For instrumented code we may have GPR + BR in parallel but
>>> + it will confuse DF and we need to put each reg
>>> + under EXPR_LIST. */
>>> + if (chkp_function_instrumented_p (current_function_decl))
>>> + chkp_put_regs_to_expr_list (retval);
>>
>>
>> I assume that this is OK from infrastructure POV. I'd like to ask
>> Jeff, if this approach is OK.
>
> This seems wrong. The documented way to handle multiple return values is to
> put them into a PARALLEL. If that's not working, I'd like Ilya to describe
> more precisely why. I missed this from patch #6.
Hi,
I did this change a couple of years ago and don't remember exactly
what problem was caused by PARALLEL. But from my comment it seems
parallel lead to values in BND0 and BND1 not to be actually defined by
call from DF point of view. I'll try to reproduce a problem I had.
>
>
>>>
>>> (define_c_enum "unspecv" [
>>> @@ -11549,7 +11550,9 @@
>>> (define_insn "*call_value"
>>> [(set (match_operand 0)
>>> (call (mem:QI (match_operand:W 1 "call_insn_operand" "<c>BwBz"))
>>> - (match_operand 2)))]
>>> + (match_operand 2)))
>>> + (set (reg:BND64 BND0_REG) (unspec [(reg:BND64 BND0_REG)]
>>> UNSPEC_BNDRET))
>>> + (set (reg:BND64 BND1_REG) (unspec [(reg:BND64 BND1_REG)]
>>> UNSPEC_BNDRET))]
>>
>>
>> Does these patterns depend on BND0/BND1 values? If not, please use
>> (const_int 0) for their arguments.
>>
>> OTOH, do you really need these sets here? Looking at the gcc internals
>> documentation (slightly unclear in this area), it should be enough to
>> list return values in EXPR_LIST. This question is somehow connected to
>> the new comment in ix86_expand_call, so perhaps Jeff can comment on
>> this approach.
>
> Excellent question. In fact, I'd hazard a guess this doesn't really do what
> Ilya wants. As written those additional sets occur in parallel with the
> call. ie, all the RHSs are evaluated in parallel, then the assignments to
> the LHSs occur in parallel. I suspect that's not what Ilya wants -- instead
> I think he wants those side effects to occur after the call.
>
> This kind of scheme also doesn't tend to "play well" with exception handling
> & scheduling becuase you can't guarantee the sets and the call are in the
> same block and scheduler as a single group.
How can the sets and the call no be in the same block/group if all of
them are parts of a single instruction?
Ilya
>
> See the block comment in the "call" expander on the PA port. The situation
> is not precisely the same, but it's a bit of a warning flag if we have the
> call expanders doing "other" assignments.
>
> jeff
>
>
