Senthil Kumar Selvaraj writes:
> 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?
Sent an out-of-date patch. Here's the right one.
>
> Regards
> Senthil
>
gcc/ChangeLog:
2018-07-25 Senthil Kumar Selvaraj <senthilkumar.selva...@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.selva...@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..033a428e9f3 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -672,7 +672,14 @@
; 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));
+ {
+ operands[0] = dest; operands[1] = 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" } } */
