[arm] PR target/85026: Fix ldrsh length estimate in Thumb state

Message ID 5AB52D22.3060104@foss.arm.com
State New
Headers show
Series
  • [arm] PR target/85026: Fix ldrsh length estimate in Thumb state
Related show

Commit Message

Kyrill Tkachov March 23, 2018, 4:36 p.m.
Hi all,

This bug has been reported against GCC 7.3.0 but it is latent in all release branches and on trunk.
We underestimate the length of the LRSH instruction in Thumb state.
Unlike other load instructions LDRSH can be encoded in 16 bits only when using a register offset.
In the testcase we have "ldrsh   r2, [r4]" being assigned a length of 2, which is wrong.
So we don't calculate branch ranges properly and cause the assembler error.

The fix is to make the unaligned_loadhis insn similar to the *arm_extendqihi_insn insn that outputs an LDRSB.
Just remove the wrong 2-byte alternative. I don't think this is worth inventing a new "register-offset-only" constraint.
This also makes the patch safer for backporting.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Committing to trunk and the branches after some soaking time.

Thanks,
Kyrill

2018-03-23  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/85026
     * config/arm/arm.md (unaligned_loadhis): Remove first alternative.
     Clean up attributes.

2018-03-23  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/85026
     * g++.dg/pr85026.C: New test.

Patch

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index f9365cd..ad5f387 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -4498,16 +4498,13 @@  (define_insn "unaligned_loadsi"
    (set_attr "type" "load1")])
 
 (define_insn "unaligned_loadhis"
-  [(set (match_operand:SI 0 "s_register_operand" "=l,r")
+  [(set (match_operand:SI 0 "s_register_operand" "=r")
 	(sign_extend:SI
-	  (unspec:HI [(match_operand:HI 1 "memory_operand" "Uw,Uh")]
+	  (unspec:HI [(match_operand:HI 1 "memory_operand" "Uh")]
 		     UNSPEC_UNALIGNED_LOAD)))]
   "unaligned_access"
   "ldrsh%?\t%0, %1\t@ unaligned"
-  [(set_attr "arch" "t2,any")
-   (set_attr "length" "2,4")
-   (set_attr "predicable" "yes")
-   (set_attr "predicable_short_it" "yes,no")
+  [(set_attr "predicable" "yes")
    (set_attr "type" "load_byte")])
 
 (define_insn "unaligned_loadhiu"
diff --git a/gcc/testsuite/g++.dg/pr85026.C b/gcc/testsuite/g++.dg/pr85026.C
new file mode 100644
index 0000000..e1e3ccd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr85026.C
@@ -0,0 +1,61 @@ 
+/* PR target/85026.  */
+/* { dg-do assemble } */
+/* { dg-options "-O2 -std=gnu++11" } */
+
+template <class> class a;
+class b;
+struct c {
+  typedef a<b> &g;
+};
+template <typename d> struct e { typedef typename d::f iter; };
+class h {
+public:
+  void __attribute__((noreturn)) i();
+} ab;
+template <class> class a {
+public:
+  typedef b *f;
+  b &operator[](unsigned m) {
+    if (ac)
+      ab.i();
+    return ad[m];
+  }
+  f n() { return ad; }
+  f m_fn3();
+  b *ad;
+  unsigned ac;
+};
+class b {
+public:
+  short j;
+  short k;
+  signed l;
+} __attribute__((__packed__));
+void o(a<b> &m, b &p2, b &p) {
+  p2 = p = m[0];
+  if (bool at = false)
+    ;
+  else
+    for (c::g au(m);; at = true)
+      if (bool av = false)
+        ;
+      else
+        for (e<a<int>>::iter aw = au.n(), ax = au.m_fn3(); ax;
+             av ? (void)0 : (void)0)
+          if (bool ay = 0)
+            ;
+          else
+            for (b az = *aw; !ay; ay = true) {
+              if (p2.j)
+                p2.j = az.j;
+              else if (p.j)
+                p.j = az.j;
+              if (p2.k)
+                p2.k = az.k;
+              else if (az.k > p.k)
+                p.k = az.k;
+              if (az.l < p2.l)
+                if (az.l > p.l)
+                  p.l = az.l;
+            }
+}