@@ -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))
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 <law@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.