This patch fixes the bug pointed out in the V10 patch review that the code
modified an input argument to vector extract with a variable element number.

I also added two gcc_asserts to the vector extract address code to signal an
internal error if the temporary base register was used for two different
purposes.  This shows up if you have a vector whose address is a PC-relative
address and the element number was variable.

Later patches will fix the case that I know of that generates the bad code, but
it is still important to make sure the same case doesn't happen in the future.

With this patch applied, the compiler will signal an error.  FWIW, I did build
all of Spec 2017 and Spec 2006 with this patch applied, but not the others, and
we did not get an assertion failure.

I have bootstrapped the compiler and there were no regression test failures on
a little endian Power8 system.

2019-12-20  Michael Meissner  <meiss...@linux.ibm.com>

        * config/rs6000/rs6000.c (rs6000_adjust_vec_address): Add
        assertion to make sure that we don't load an address into a
        temporary that is already used.
        (rs6000_split_vec_extract_var): Do not overwrite the element when
        masking it.  Use the base register temporary instead.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c  (revision 279549)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -6757,6 +6757,8 @@ rs6000_adjust_vec_address (rtx scalar_re
 
       else
        {
+         /* If we are called from rs6000_split_vec_extract_var, base_tmp may
+            be the same as element.  */
          if (TARGET_POWERPC64)
            emit_insn (gen_ashldi3 (base_tmp, element, GEN_INT (byte_shift)));
          else
@@ -6825,6 +6827,11 @@ rs6000_adjust_vec_address (rtx scalar_re
 
          else
            {
+             /* Make sure base_tmp is not the same as element_offset.  This
+                can happen if the element number is variable and the address
+                is not a simple address.  Otherwise we lose the offset, and
+                double the address.  */
+             gcc_assert (!reg_mentioned_p (base_tmp, element_offset));
              emit_move_insn (base_tmp, op1);
              emit_insn (gen_add2_insn (base_tmp, element_offset));
            }
@@ -6835,6 +6842,10 @@ rs6000_adjust_vec_address (rtx scalar_re
 
   else
     {
+      /* Make sure base_tmp is not the same as element_offset.  This can happen
+        if the element number is variable and the address is not a simple
+        address.  Otherwise we lose the offset, and double the address.  */
+      gcc_assert (!reg_mentioned_p (base_tmp, element_offset));
       emit_move_insn (base_tmp, addr);
       new_addr = gen_rtx_PLUS (Pmode, base_tmp, element_offset);
     }
@@ -6902,9 +6913,10 @@ rs6000_split_vec_extract_var (rtx dest,
       int num_elements = GET_MODE_NUNITS (mode);
       rtx num_ele_m1 = GEN_INT (num_elements - 1);
 
-      emit_insn (gen_anddi3 (element, element, num_ele_m1));
+      /* Make sure the element number is in bounds.  */
       gcc_assert (REG_P (tmp_gpr));
-      emit_move_insn (dest, rs6000_adjust_vec_address (dest, src, element,
+      emit_insn (gen_anddi3 (tmp_gpr, element, num_ele_m1));
+      emit_move_insn (dest, rs6000_adjust_vec_address (dest, src, tmp_gpr,
                                                       tmp_gpr, scalar_mode));
       return;
     }

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797

Reply via email to