On Fri, 30 Mar 2018, Peter Bergner wrote: > On 3/29/18 9:35 AM, Alan Modra wrote: > > On Thu, Nov 16, 2017 at 03:27:01PM +0000, Tamar Christina wrote: > >> --- a/gcc/expr.c > >> +++ b/gcc/expr.c > >> @@ -2769,7 +2769,9 @@ copy_blkmode_to_reg (machine_mode mode, tree src) > >> > >> n_regs = (bytes + UNITS_PER_WORD - 1) / UNITS_PER_WORD; > >> dst_words = XALLOCAVEC (rtx, n_regs); > >> - bitsize = MIN (TYPE_ALIGN (TREE_TYPE (src)), BITS_PER_WORD); > >> + bitsize = BITS_PER_WORD; > >> + if (targetm.slow_unaligned_access (word_mode, TYPE_ALIGN (TREE_TYPE > >> (src)))) > >> + bitsize = MIN (TYPE_ALIGN (TREE_TYPE (src)), BITS_PER_WORD); > >> > >> /* Copy the structure BITSIZE bits at a time. */ > >> for (bitpos = 0, xbitpos = padding_correction; > > > > I believe this patch is wrong. Please revert. See the PR84762 testcase. > > > > There are two problems. Firstly, if padding_correction is non-zero, > > then xbitpos % BITS_PER_WORD is non-zero and in > > > > store_bit_field (dst_word, bitsize, xbitpos % BITS_PER_WORD, > > 0, 0, word_mode, > > extract_bit_field (src_word, bitsize, > > bitpos % BITS_PER_WORD, 1, > > NULL_RTX, word_mode, word_mode, > > false, NULL), > > false); > > > > the stored bit-field exceeds the destination register size. You could > > fix that by making bitsize the gcd of bitsize and padding_correction. > > > > However, that doesn't fix the second problem which is that the > > extracted bit-field can exceed the source size. That will result in > > rubbish being read into a register. > > FYI, I received an automated response saying Tamar is away on vacation > with no return date specified. That means he won't be able to revert > the patch. What do we do now?
The code before the change already looks fishy to me. x = expand_normal (src); so what do we expect this to expand to in general? Fortunately it seems there are exactly two callers so hopefully a gcc_assert (MEM_P (x)) would work? The fishy part is looking at TYPE_ALIGN (TREE_TYPE (src)). In case x is a MEM we should use MEM_ALIGN and if not then we shouldn't do anything but just use word_mode here.
