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
