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/combine] PR64304 wrong bitmask passed to force_to_mode in combine_simplify_rtx


as reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

given the following test:

unsigned char byte = 0;

void set_bit(unsigned int bit, unsigned char value) {
    unsigned char mask = (unsigned char)(1 << (bit & 7));
    if (!value) {
        byte &= (unsigned char)~mask;
    } else {
        byte |= mask;
    }
}

we should generate something like:

  set_bit:
        and     w0, w0, 7
        mov     w2, 1
        lsl     w2, w2, w0

while we are generating
        mov     w2, 1
        lsl     w2, w2, w0


the necessary "and     w0, w0, 7" deleted wrongly.

that because
(insn 2 5 3 2 (set (reg/v:SI 82 [ bit ])
        (reg:SI 0 x0 [ bit ])) bug.c:3 38 {*movsi_aarch64}
     (expr_list:REG_DEAD (reg:SI 0 x0 [ bit ])
        (nil)))
  (insn 7 4 8 2 (set (reg:SI 84 [ D.1482 ])
        (and:SI (reg/v:SI 82 [ bit ])
            (const_int 7 [0x7]))) bug.c:4 399 {andsi3}
     (expr_list:REG_DEAD (reg/v:SI 82 [ bit ])
        (nil)))
  (insn 9 8 10 2 (set (reg:SI 85 [ D.1482 ])
        (ashift:SI (reg:SI 86)
            (subreg:QI (reg:SI 84 [ D.1482 ]) 0))) bug.c:4 539 {*aarch64_ashl_sisd_or_int_si3}
     (expr_list:REG_DEAD (reg:SI 86)
        (expr_list:REG_DEAD (reg:SI 84 [ D.1482 ])
            (expr_list:REG_EQUAL (ashift:SI (const_int 1 [0x1])
                    (subreg:QI (reg:SI 84 [ D.1482 ]) 0))
                (nil)))))

are wrongly combined into

  (insn 9 8 10 2 (set (reg:QI 85 [ D.1482 ])
        (ashift:QI (subreg:QI (reg:SI 86) 0)
            (reg:QI 0 x0 [ bit ]))) bug.c:4 556 {*ashlqi3_insn}
     (expr_list:REG_DEAD (reg:SI 0 x0 [ bit ])
        (expr_list:REG_DEAD (reg:SI 86)
            (nil))))

thus, the generated assembly is lack of the necessary "and w0, x0, 7".

the root cause is at one place in combine pass, we are passing wrong bitmask to force_to_mode.

in this particular case, for QI mode, we should pass (1 << 8 - 1), while we are passing (1 << 3 - 1),
thus the combiner think we only need the lower 3 bits, that X & 7 is unnecessary. While for QI mode, we
want the lower 8 bits. we should remove the exp operator.

this should be a historical bug in combine pass?? while it's only triggered for target
where SHIFT_COUNT_TRUNCATED be true. it's long time hiding mostly because x86/arm will
not trigger this part of code.

bootstrap on x86 and gcc check OK.
bootstrap on aarch64 and bare-metal regression OK.
ok for trunk?

gcc/
  PR64303
  * combine.c (combine_simplify_rtx): Correct the bitmask passed to force_to_mode.
gcc/testsuite/
  PR64303
  * gcc.target/aarch64/pr64304.c: New testcase.
diff --git a/gcc/combine.c b/gcc/combine.c
index 1808f97..31a7fd0 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -5922,7 +5922,7 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest,
 	SUBST (XEXP (x, 1),
 	       force_to_mode (XEXP (x, 1), GET_MODE (XEXP (x, 1)),
 			      ((unsigned HOST_WIDE_INT) 1
-			       << exact_log2 (GET_MODE_BITSIZE (GET_MODE (x))))
+			       << GET_MODE_BITSIZE (GET_MODE (x)))
 			      - 1,
 			      0));
       break;
diff --git a/gcc/testsuite/gcc.target/aarch64/pr64304.c b/gcc/testsuite/gcc.target/aarch64/pr64304.c
new file mode 100644
index 0000000..5423bb3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr64304.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 --save-temps" } */
+
+unsigned char byte = 0;
+
+void
+set_bit (unsigned int bit, unsigned char value)
+{
+  unsigned char mask = (unsigned char) (1 << (bit & 7));
+
+  if (! value)
+    byte &= (unsigned char)~mask;
+  else
+    byte |= mask;
+  /* { dg-final { scan-assembler "and\tw\[0-9\]+, w\[0-9\]+, 7" } } */
+}
+
+/* { dg-final { cleanup-saved-temps } } */

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