This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch, avr, PR86635] Fix miscompilation with __memx and libgcc float function __gtsf2


The below patch fixes a miscompilation of function calls with__memx address space
arguments.

For code like

extern const  __memx float a;
extern const float b;

int diff () { return a > b; }

when compiled with -Os, a is never loaded and passed in as an argument
to the __gtsf2 libgcc function.

Turns out that mov<mode> for the variable a expands into

(insn 8 7 9 2 (parallel [
            (set (reg:SF 22 r22)
                (mem/u/c:SF (reg/f:PSI 47) [1 a+0 S4 A8 AS7]))
            (clobber (reg:SF 22 r22))
            (clobber (reg:QI 21 r21))
            (clobber (reg:HI 30 r30))
        ]) "test.c":4 36 {xloadsf_A}
     (expr_list:REG_DEAD (reg/f:PSI 47)
        (expr_list:REG_UNUSED (reg:HI 30 r30)
            (expr_list:REG_EQUAL (mem/u/c:SF (symbol_ref:PSI ("a") [flags 0xe40]  <var_decl 0x7fabf444c900 a>) [1 a+0 S4 A8 AS7])
                (nil)))))

The ud_dce pass sees this insn and deletes it as reg:SF r22 is both set
and clobbered.

Georg-Johann pointed out a similar issue (PR63633), and that was fixed
by introducing a pseudo as the target of set. This patch does the same -
adds an avr_emit2_fix_outputs for gen functions with 2 operands, that
detects hard reg conflicts with clobbered regs and substitutes pseudos
in their place.

The patch also adds a testcase to verify a is actually read. Reg testing
passed. Ok to commit to trunk?

Regards
Senthil

gcc/ChangeLog:

2018-07-25  Senthil Kumar Selvaraj  <senthilkumar.selvaraj@microchip.com>

	* config/avr/avr-protos.h (avr_emit2_fix_outputs): New prototype.
	* config/avr/avr.c (avr_emit2_fix_outputs): New function.
	* config/avr/avr.md (mov<mode>): Wrap gen_xload<mode>_A call
  with avr_emit2_fix_outputs.

gcc/testsuite/ChangeLog:

2018-07-25  Senthil Kumar Selvaraj  <senthilkumar.selvaraj@microchip.com>

	* gcc.target/avr/torture/pr86635.c: New test.


diff --git gcc/config/avr/avr-protos.h gcc/config/avr/avr-protos.h
index 5622e9035a0..f8db418582e 100644
--- gcc/config/avr/avr-protos.h
+++ gcc/config/avr/avr-protos.h
@@ -135,6 +135,7 @@ regmask (machine_mode mode, unsigned regno)
 }
 
 extern void avr_fix_inputs (rtx*, unsigned, unsigned);
+extern bool avr_emit2_fix_outputs (rtx (*)(rtx,rtx), rtx*, unsigned, unsigned);
 extern bool avr_emit3_fix_outputs (rtx (*)(rtx,rtx,rtx), rtx*, unsigned, unsigned);
 
 extern rtx lpm_reg_rtx;
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 81c35e7fbc2..996d5187c52 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -13335,6 +13335,34 @@ avr_emit3_fix_outputs (rtx (*gen)(rtx,rtx,rtx), rtx *op,
   return avr_move_fixed_operands (op, hreg, opmask);
 }
 
+/* Same as avr_emit3_fix_outputs, but for 2 operands */
+bool
+avr_emit2_fix_outputs (rtx (*gen)(rtx,rtx), rtx *op,
+                       unsigned opmask, unsigned rmask)
+{
+  const int n = 2;
+  rtx hreg[n];
+
+  /* It is letigimate for GEN to call this function, and in order not to
+     get self-recursive we use the following static kludge.  This is the
+     only way not to duplicate all expanders and to avoid ugly and
+     hard-to-maintain C-code instead of the much more appreciated RTL
+     representation as supplied by define_expand.  */
+  static bool lock = false;
+
+  gcc_assert (opmask < (1u << n));
+
+  if (lock)
+    return false;
+
+  avr_fix_operands (op, hreg, opmask, rmask);
+
+  lock = true;
+  emit_insn (gen (op[0], op[1]));
+  lock = false;
+
+  return avr_move_fixed_operands (op, hreg, opmask);
+}
 
 /* Worker function for movmemhi expander.
    XOP[0]  Destination as MEM:BLK
diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
index e619e695418..5ff4e490a21 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -672,7 +672,11 @@
              ; insn-emit does not depend on the mode, it's all about operands.  */
           emit_insn (gen_xload8qi_A (dest, src));
         else
-          emit_insn (gen_xload<mode>_A (dest, src));
+          if (!avr_emit2_fix_outputs (gen_xload<mode>_A, operands, 1 << 0,
+                                      regmask (<MODE>mode, 22)
+                                               | regmask (QImode, 21)
+                                               | regmask (HImode, REG_Z)))
+            FAIL;
 
         DONE;
       }
diff --git gcc/testsuite/gcc.target/avr/torture/pr86635.c gcc/testsuite/gcc.target/avr/torture/pr86635.c
new file mode 100644
index 00000000000..f91367f7e7a
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/torture/pr86635.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! avr_tiny } } } */
+/* { dg-options "-std=gnu99" } */
+
+extern const __memx float a;
+extern const float b;
+
+unsigned long diff () { return a > b; }
+
+/* { dg-final { scan-assembler "call __xload_4" } } */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]