Re: [Patch] 1/3 Add new builtin __builtin_flush_icache().

[David Daney <ddaney@avtrex.com>]

Richard Sandiford wrote:
> David Daney <ddaney@avtrex.com> writes:
>> +/* Expand a call to _builtin___clear_cache.  If the target does not
>> +   support the operation, return NULL_RTX so it expands as a call. */
>> +static rtx
>> +expand_builtin_clear_cache (tree exp)
>> +{
>> +  tree begin, end;
>> +  rtx begin_rtx, end_rtx;
>> +
>> +  if (!targetm.builtin_clear_cache_inline_p())
>> +    return NULL_RTX;
>> +
>> +  /* We must not expand to a library call if
>> +     __builtin_clear_cache_inline_p() returns true.  If we did, the
>> +     fallback library function in libgcc would expand to a call to
>> +     _builtin___clear_cache() and we would recurse infinitely.  */
>> +  if (!validate_arglist (exp, POINTER_TYPE, POINTER_TYPE, VOID_TYPE))
> 
> This comment seems to belong above the earlier code.  However, I'm only
> really replying because...
> 

This is the way I meant for it to be.  If 
targetm.builtin_clear_cache_inline_p() is false we just do the default 
expand to call the function in libgcc.

If we are expanding in-line, the it is an error if the arguments are not 
pointers.

The only reason it is an error is to prevent infinite recursion.  If 
that were not an issue, we could return NULL_RTX if validate_arglist failed.


>> +  /* Evaluate BEGIN and END. */
>> +  begin_rtx = expand_expr (begin, NULL_RTX, Pmode, EXPAND_NORMAL);
>> +  begin_rtx = convert_memory_address (Pmode, begin_rtx);
>> +
>> +  end_rtx = expand_expr (end, NULL_RTX, Pmode, EXPAND_NORMAL);
>> +  end_rtx = convert_memory_address (Pmode, end_rtx);
>> +
>> +#ifdef HAVE_clear_cache
>> +  if (HAVE_clear_cache)
>> +    emit_insn (gen_clear_cache (begin_rtx, end_rtx));
>> +#endif
> 
> When I said that the switch from general_operand to pmode_register_operand
> would need target-independent changes, I meant it would need changes to
> this bit of code.  I think you need to check the predicates of each
> clear_cache operand and force the operand into a register if it doesn't
> match.  Something like:
> 
>   if (HAVE_clear_cache)
>     {
>       icode = CODE_FOR_clear_cache;
>       if (!insn_data[icode].operand[0].predicate (begin_rtx, Pmode))
> 	begin_rtx = copy_to_mode_reg (Pmode, begin_rtx);
>       if (!insn_data[icode].operand[1].predicate (end_rtx, Pmode))
> 	end_rtx = copy_to_mode_reg (Pmode, end_rtx);
>       emit_insn (gen_clear_cache (begin_rtx, end_rtx));
>     }
> 
> (It's unfortunate that this construct has to be copied so often,
> but that's the way it is.)
> 
> As it stands, the code only seems to guarantee that the operand
> is an address operand, whereas the predicates in the MIPS patch
> claim that it will be a register.

The operands in the MIPS patch are both (match_operand:SI 0 
"general_operand").  Look at the part in mips.c where I do:

begin = force_reg(Pmode, begin);

If that is the wrong way to put something in a register I will change it.

David Daney