During the cc0 transition work I had disabled most, if not all, of the optional
patterns in the H8 backend. Then I converted the critical patterns to get a
baseline state. Then I could re-add the disabled patterns as they were
converted
and verify we weren't going backwards from a correctness standpoint.
One of the more interesting cases are the bra/bc bra/bs patterns which are
specific to the H8/SX processor. They are integrated branch on bit-set and
branch on bit-clear patterns. They save a little time and space, but they're
not
hugely important.
Imagine my surprise when I turned them back on a saw a few tests failing.
Looking at things in the debugger I suspected length computation problems
(again). Sure enough a -dp dump showed the bogus lengths for instances of the
bra/bc bra/bs patterns.
Unfortunately, I don't see a really good way to fix them as the port is
structured to call out to a function to compute the length of the memory address
operand. This results in a default and minimum length of 0x7fffffff. I was
able
to use the same trick as the PA port and fix the minimum length, but getting the
default length fixed for these patterns proved too elusive.
At this point, I think we're best off just disabling them. If someone really
cares, they can revisit and try to get the computation correct. Given there's
only a few addressing modes for the memory operand and only two affected
patterns
it probably wouldn't be too gross to avoid the function call.
Anyway, this is the patch to disable the two patterns, which provides a higher
correctness baseline state (in terms of testsuite state).
Committed to the trunk,
Jeff
commit 6dda86084439af4f5315a5c3aaee732a610e3551
Author: Jeff Law <l...@redhat.com>
Date: Sat May 30 21:53:28 2020 -0600
Disable brabc/brabs patterns as their length computation is horribly broken
and leads to incorrect code generation.
* config/h8300/jumpcall.md (brabs, brabc): Disable patterns.
diff --git a/gcc/config/h8300/jumpcall.md b/gcc/config/h8300/jumpcall.md
index 7208fb6d86b..3917cf18920 100644
--- a/gcc/config/h8300/jumpcall.md
+++ b/gcc/config/h8300/jumpcall.md
@@ -77,6 +77,16 @@
[(set_attr "type" "branch")
(set_attr "cc" "none")])
+;; The brabc/brabs patterns have been disabled because their length computation
+;; is horribly broken. When we call out to a function via a SYMBOL_REF we get
+;; bogus default and minimum lengths. The trick used by the PA port seems to
+;; fix the minimum, but not the default length. The broken lengths can lead
+;; to bogusly using a short jump when a long jump was needed and thus
+;; incorrect code.
+;;
+;; Given the restricted addressing modes for operand 1, we could probably just
+;; open-code the necessary length computation in the two affected patterns
+;; rather than using a function call. I think that would fix this problem.
(define_insn "*brabc"
[(set (pc)
(if_then_else (eq (zero_extract (match_operand:QI 1
"bit_memory_operand" "WU")
@@ -85,7 +95,7 @@
(const_int 0))
(label_ref (match_operand 0 "" ""))
(pc)))]
- "TARGET_H8300SX"
+ "0 && TARGET_H8300SX"
{
switch (get_attr_length (insn)
- h8300_insn_length_from_table (insn, operands))
@@ -110,7 +120,7 @@
(const_int 0))
(label_ref (match_operand 0 "" ""))
(pc)))]
- "TARGET_H8300SX"
+ "0 && TARGET_H8300SX"
{
switch (get_attr_length (insn)
- h8300_insn_length_from_table (insn, operands))
