[PATCH, RFC]: Next stage1, refactoring: propagating rtx subclasses

[Jeff Law]

On 04/25/2015 05:49 AM, Richard Sandiford wrote:
>
>> @@ -2099,9 +2107,9 @@ fix_crossing_conditional_branches (void)
>>   		  emit_label (new_label);
>>
>>   		  gcc_assert (GET_CODE (old_label) == LABEL_REF);
>> -		  old_label = JUMP_LABEL (old_jump);
>> -		  new_jump = emit_jump_insn (gen_jump (old_label));
>> -		  JUMP_LABEL (new_jump) = old_label;
>> +		  old_label_insn = JUMP_LABEL_AS_INSN (old_jump);
>> +		  new_jump = emit_jump_insn (gen_jump (old_label_insn));
>> +		  JUMP_LABEL (new_jump) = old_label_insn;
>>
>>   		  last_bb = EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb;
>>   		  new_bb = create_basic_block (new_label, new_jump, last_bb);
>
> I think the eventual aim would be to have rtx_jump_insn member functions
> that get and set the jump label as an rtx_insn, with JUMP_LABEL_AS_INSN
> being a stepping stone towards that.  In cases like this it might make
> more sense to ensure old_jump has the right type (rtx_jump_insn) and go
> straight to the member functions, rather than switching to JUMP_LABEL_AS_INSN
> now and then having to rewrite it later.
I'm comfortable with either way, so long as we get there.  I know that 
David certainly found it easier to introduce "scaffolding" early in this 
patch series, then exploit it, then tear down the scaffolding near the 
end of a patch series.

>> diff --git a/gcc/is-a.h b/gcc/is-a.h
>> index 58917eb..4fb9dde 100644
>> --- a/gcc/is-a.h
>> +++ b/gcc/is-a.h
>> @@ -46,6 +46,11 @@ TYPE as_a <TYPE> (pointer)
>>
>>         do_something_with (as_a <cgraph_node *> *ptr);
>>
>> +TYPE assert_as_a <TYPE> (pointer)
>> +
>> +    Like as_a <TYPE> (pointer), but uses assertion, which is enabled even in
>> +    non-checking (release) build.
>> +
>>   TYPE safe_as_a <TYPE> (pointer)
>>
>>       Like as_a <TYPE> (pointer), but where pointer could be NULL.  This
>> @@ -193,6 +198,17 @@ as_a (U *p)
>>     return is_a_helper <T>::cast (p);
>>   }
>>
>> +/* Same as above, but checks the condition even in release build.  */
>> +
>> +template <typename T, typename U>
>> +inline T
>> +assert_as_a (U *p)
>> +{
>> +  gcc_assert (is_a <T> (p));
>> +  return is_a_helper <T>::cast (p);
>> +}
>> +
>> +
>>   /* Similar to as_a<>, but where the pointer can be NULL, even if
>>      is_a_helper<T> doesn't check for NULL.  */
>
> This preserves the behaviour of the original code but I'm not sure
> it's worth it.  I doubt the distinction between:
>
>    gcc_assert (JUMP_P (x));
>
> and:
>
>    gcc_checking_assert (JUMP_P (x));
>
> was ever very scientific.  Seems like we should take this refactoring as
> an opportunity to make the checking more consistent.
Without some guidelines I suspect usage of gcc_check_assert would be 
highly inconsistent.

And ultimately we want to get away from the helpers as much as possible, 
instead relying on the static typesystem to detect errors at compile 
time.  So unless there's a compelling reason, I'd prefer not to add more 
"support" for these helpers.

[ snip]

>
> That seems pretty heavy-weight for LRA-local code.  Also, the long-term
> plan is for INSN_LIST and rtx_insn to be in separate hierarchies,
> at which point we'd have no alias-safe way to distinguish them.
That's certainly what I think ought to happen.  INSN_LIST should turn 
into a standard vector or forward list.  For the use cases in GCC, 
either ought to be acceptable.

Jeff